mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] drivers: treewide: Do not use NULL as driver_data
@ 2025-09-16  9:32 Sascha Hauer
  2025-09-16  9:38 ` Ahmad Fatoum
  0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2025-09-16  9:32 UTC (permalink / raw)
  To: Barebox List

Several drivers in the tree use an enum for the chip type casted to int
as driver_data. This comes with the problem that device_get_match_data()
returns NULL for these devices and a potential error check bails out
then.

This patch changes the enums used by the drivers to start from 1 instead
of 0 so that the return value of device_get_match_data() can safely
checked for being NULL.

Most drivers do not check the return value anyway, but fec_imx.c does
which causes the driver to no longer work on i.MX27. This issue is fixed
here.

Fixes: Fixes: 20d87123a6 ("treewide: replace dev_get_drvdata with device_get_match_data")
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 drivers/mtd/nand/raw/nand_mxs.c | 2 +-
 drivers/net/fec_imx.h           | 2 +-
 drivers/net/fsl-fman.c          | 2 +-
 drivers/regulator/fan53555.c    | 2 +-
 include/linux/mfd/axp20x.h      | 2 +-
 include/mfd/bd71837.h           | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_mxs.c b/drivers/mtd/nand/raw/nand_mxs.c
index 02f8546b87..4293f4e0e0 100644
--- a/drivers/mtd/nand/raw/nand_mxs.c
+++ b/drivers/mtd/nand/raw/nand_mxs.c
@@ -38,7 +38,7 @@
 #include "internals.h"
 
 enum gpmi_type {
-	GPMI_MXS,
+	GPMI_MXS = 1,
 	GPMI_IMX6,
 };
 
diff --git a/drivers/net/fec_imx.h b/drivers/net/fec_imx.h
index 1aaff87fdd..33b8db2745 100644
--- a/drivers/net/fec_imx.h
+++ b/drivers/net/fec_imx.h
@@ -114,7 +114,7 @@ struct buffer_descriptor {
 };
 
 enum fec_type {
-	FEC_TYPE_IMX27,
+	FEC_TYPE_IMX27 = 1,
 	FEC_TYPE_IMX28,
 	FEC_TYPE_IMX6,
 };
diff --git a/drivers/net/fsl-fman.c b/drivers/net/fsl-fman.c
index bde054d751..ac716c7468 100644
--- a/drivers/net/fsl-fman.c
+++ b/drivers/net/fsl-fman.c
@@ -123,7 +123,7 @@ struct fsl_fman_mdio {
 };
 
 enum fman_port_type {
-	FMAN_PORT_TYPE_RX = 0,	/* RX Port */
+	FMAN_PORT_TYPE_RX = 1,	/* RX Port */
 	FMAN_PORT_TYPE_TX,	/* TX Port */
 };
 
diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
index 2dcc6f5394..7477543915 100644
--- a/drivers/regulator/fan53555.c
+++ b/drivers/regulator/fan53555.c
@@ -44,7 +44,7 @@ enum {
 };
 
 enum fan53555_vendor {
-	FAN53526_VENDOR_FAIRCHILD = 0,
+	FAN53526_VENDOR_FAIRCHILD = 1,
 	FAN53555_VENDOR_FAIRCHILD,
 	FAN53555_VENDOR_ROCKCHIP,	/* RK8600, RK8601 */
 	RK8602_VENDOR_ROCKCHIP,		/* RK8602, RK8603 */
diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
index 93d303c459..0ba4568faf 100644
--- a/include/linux/mfd/axp20x.h
+++ b/include/linux/mfd/axp20x.h
@@ -11,7 +11,7 @@
 #include <poweroff.h>
 
 enum axp20x_variants {
-	AXP152_ID = 0,
+	AXP152_ID = 1,
 	AXP202_ID,
 	AXP209_ID,
 	AXP221_ID,
diff --git a/include/mfd/bd71837.h b/include/mfd/bd71837.h
index 4e285a647a..ab4cc57c88 100644
--- a/include/mfd/bd71837.h
+++ b/include/mfd/bd71837.h
@@ -7,7 +7,7 @@
 #define BD718XX_REGULATOR_DRIVER "bd718x7_regulator"
 
 enum {
-	ROHM_CHIP_TYPE_BD71837 = 0,
+	ROHM_CHIP_TYPE_BD71837 = 1,
 	ROHM_CHIP_TYPE_BD71847,
 	ROHM_CHIP_TYPE_BD70528,
 	ROHM_CHIP_TYPE_AMOUNT
-- 
2.47.3




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drivers: treewide: Do not use NULL as driver_data
  2025-09-16  9:32 [PATCH] drivers: treewide: Do not use NULL as driver_data Sascha Hauer
@ 2025-09-16  9:38 ` Ahmad Fatoum
  2025-09-17 14:02   ` Sascha Hauer
  0 siblings, 1 reply; 4+ messages in thread
From: Ahmad Fatoum @ 2025-09-16  9:38 UTC (permalink / raw)
  To: Sascha Hauer, Barebox List

Hello Sascha,

On 9/16/25 11:32 AM, Sascha Hauer wrote:
> Several drivers in the tree use an enum for the chip type casted to int
> as driver_data. This comes with the problem that device_get_match_data()
> returns NULL for these devices and a potential error check bails out
> then.
> 
> This patch changes the enums used by the drivers to start from 1 instead
> of 0 so that the return value of device_get_match_data() can safely
> checked for being NULL.
> 
> Most drivers do not check the return value anyway, but fec_imx.c does
> which causes the driver to no longer work on i.MX27. This issue is fixed
> here.

If they don't check the return value, why bother changing it and risk a
regression?

Thanks,
Ahmad

> 
> Fixes: Fixes: 20d87123a6 ("treewide: replace dev_get_drvdata with device_get_match_data")
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/raw/nand_mxs.c | 2 +-
>  drivers/net/fec_imx.h           | 2 +-
>  drivers/net/fsl-fman.c          | 2 +-
>  drivers/regulator/fan53555.c    | 2 +-
>  include/linux/mfd/axp20x.h      | 2 +-
>  include/mfd/bd71837.h           | 2 +-
>  6 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_mxs.c b/drivers/mtd/nand/raw/nand_mxs.c
> index 02f8546b87..4293f4e0e0 100644
> --- a/drivers/mtd/nand/raw/nand_mxs.c
> +++ b/drivers/mtd/nand/raw/nand_mxs.c
> @@ -38,7 +38,7 @@
>  #include "internals.h"
>  
>  enum gpmi_type {
> -	GPMI_MXS,
> +	GPMI_MXS = 1,
>  	GPMI_IMX6,
>  };
>  
> diff --git a/drivers/net/fec_imx.h b/drivers/net/fec_imx.h
> index 1aaff87fdd..33b8db2745 100644
> --- a/drivers/net/fec_imx.h
> +++ b/drivers/net/fec_imx.h
> @@ -114,7 +114,7 @@ struct buffer_descriptor {
>  };
>  
>  enum fec_type {
> -	FEC_TYPE_IMX27,
> +	FEC_TYPE_IMX27 = 1,
>  	FEC_TYPE_IMX28,
>  	FEC_TYPE_IMX6,
>  };
> diff --git a/drivers/net/fsl-fman.c b/drivers/net/fsl-fman.c
> index bde054d751..ac716c7468 100644
> --- a/drivers/net/fsl-fman.c
> +++ b/drivers/net/fsl-fman.c
> @@ -123,7 +123,7 @@ struct fsl_fman_mdio {
>  };
>  
>  enum fman_port_type {
> -	FMAN_PORT_TYPE_RX = 0,	/* RX Port */
> +	FMAN_PORT_TYPE_RX = 1,	/* RX Port */
>  	FMAN_PORT_TYPE_TX,	/* TX Port */
>  };
>  
> diff --git a/drivers/regulator/fan53555.c b/drivers/regulator/fan53555.c
> index 2dcc6f5394..7477543915 100644
> --- a/drivers/regulator/fan53555.c
> +++ b/drivers/regulator/fan53555.c
> @@ -44,7 +44,7 @@ enum {
>  };
>  
>  enum fan53555_vendor {
> -	FAN53526_VENDOR_FAIRCHILD = 0,
> +	FAN53526_VENDOR_FAIRCHILD = 1,
>  	FAN53555_VENDOR_FAIRCHILD,
>  	FAN53555_VENDOR_ROCKCHIP,	/* RK8600, RK8601 */
>  	RK8602_VENDOR_ROCKCHIP,		/* RK8602, RK8603 */
> diff --git a/include/linux/mfd/axp20x.h b/include/linux/mfd/axp20x.h
> index 93d303c459..0ba4568faf 100644
> --- a/include/linux/mfd/axp20x.h
> +++ b/include/linux/mfd/axp20x.h
> @@ -11,7 +11,7 @@
>  #include <poweroff.h>
>  
>  enum axp20x_variants {
> -	AXP152_ID = 0,
> +	AXP152_ID = 1,
>  	AXP202_ID,
>  	AXP209_ID,
>  	AXP221_ID,
> diff --git a/include/mfd/bd71837.h b/include/mfd/bd71837.h
> index 4e285a647a..ab4cc57c88 100644
> --- a/include/mfd/bd71837.h
> +++ b/include/mfd/bd71837.h
> @@ -7,7 +7,7 @@
>  #define BD718XX_REGULATOR_DRIVER "bd718x7_regulator"
>  
>  enum {
> -	ROHM_CHIP_TYPE_BD71837 = 0,
> +	ROHM_CHIP_TYPE_BD71837 = 1,
>  	ROHM_CHIP_TYPE_BD71847,
>  	ROHM_CHIP_TYPE_BD70528,
>  	ROHM_CHIP_TYPE_AMOUNT

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drivers: treewide: Do not use NULL as driver_data
  2025-09-16  9:38 ` Ahmad Fatoum
@ 2025-09-17 14:02   ` Sascha Hauer
  2025-09-17 14:10     ` Ahmad Fatoum
  0 siblings, 1 reply; 4+ messages in thread
From: Sascha Hauer @ 2025-09-17 14:02 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: Barebox List

On Tue, Sep 16, 2025 at 11:38:48AM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 9/16/25 11:32 AM, Sascha Hauer wrote:
> > Several drivers in the tree use an enum for the chip type casted to int
> > as driver_data. This comes with the problem that device_get_match_data()
> > returns NULL for these devices and a potential error check bails out
> > then.
> > 
> > This patch changes the enums used by the drivers to start from 1 instead
> > of 0 so that the return value of device_get_match_data() can safely
> > checked for being NULL.
> > 
> > Most drivers do not check the return value anyway, but fec_imx.c does
> > which causes the driver to no longer work on i.MX27. This issue is fixed
> > here.
> 
> If they don't check the return value, why bother changing it and risk a
> regression?

To not give others bad examples.

> > +++ b/drivers/mtd/nand/raw/nand_mxs.c
> > @@ -38,7 +38,7 @@
> >  #include "internals.h"
> >  
> >  enum gpmi_type {
> > -	GPMI_MXS,
> > +	GPMI_MXS = 1,
> >  	GPMI_IMX6,
> >  };

Anyway, this indeed introduces a regression. We have mxs_add_nand()
which instantiates a "mxs_nand" device without driver data, so
device_get_match_data() will return 0 aka GPMI_MXS which happens to
be the right thing here.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] drivers: treewide: Do not use NULL as driver_data
  2025-09-17 14:02   ` Sascha Hauer
@ 2025-09-17 14:10     ` Ahmad Fatoum
  0 siblings, 0 replies; 4+ messages in thread
From: Ahmad Fatoum @ 2025-09-17 14:10 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

On 9/17/25 4:02 PM, Sascha Hauer wrote:
> On Tue, Sep 16, 2025 at 11:38:48AM +0200, Ahmad Fatoum wrote:
>> Hello Sascha,
>>
>> On 9/16/25 11:32 AM, Sascha Hauer wrote:
>>> Several drivers in the tree use an enum for the chip type casted to int
>>> as driver_data. This comes with the problem that device_get_match_data()
>>> returns NULL for these devices and a potential error check bails out
>>> then.
>>>
>>> This patch changes the enums used by the drivers to start from 1 instead
>>> of 0 so that the return value of device_get_match_data() can safely
>>> checked for being NULL.
>>>
>>> Most drivers do not check the return value anyway, but fec_imx.c does
>>> which causes the driver to no longer work on i.MX27. This issue is fixed
>>> here.
>>
>> If they don't check the return value, why bother changing it and risk a
>> regression?
> 
> To not give others bad examples.

I am not convinced.

> 
>>> +++ b/drivers/mtd/nand/raw/nand_mxs.c
>>> @@ -38,7 +38,7 @@
>>>  #include "internals.h"
>>>  
>>>  enum gpmi_type {
>>> -	GPMI_MXS,
>>> +	GPMI_MXS = 1,
>>>  	GPMI_IMX6,
>>>  };
> 
> Anyway, this indeed introduces a regression. We have mxs_add_nand()
> which instantiates a "mxs_nand" device without driver data, so
> device_get_match_data() will return 0 aka GPMI_MXS which happens to
> be the right thing here.

Case in point.

Cheers,
Ahmad

> 
> Sascha
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-17 14:11 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-16  9:32 [PATCH] drivers: treewide: Do not use NULL as driver_data Sascha Hauer
2025-09-16  9:38 ` Ahmad Fatoum
2025-09-17 14:02   ` Sascha Hauer
2025-09-17 14:10     ` Ahmad Fatoum

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox