[PATCH] mtd: Fix misuses of of_match_ptr()

Paul Cercueil paul at crapouillou.net
Thu Jan 27 03:18:27 PST 2022


Hi Miquel,

Le jeu., janv. 27 2022 at 12:06:31 +0100, Miquel Raynal 
<miquel.raynal at bootlin.com> a écrit :
> of_match_ptr() either expands to NULL if !CONFIG_OF, or is transparent
> otherwise. There are several drivers using this macro which keep their
> of_device_id array enclosed within an #ifdef CONFIG_OF check, these 
> are
> considered fine. However, When misused, the of_device_id array pointed
> by this macro will produce a warning because it is finally unused when
> compiled without OF support.
> 
> A number of fixes are possible:
> - Always depend on CONFIG_OF, but this will not always work and may
>   break boards.
> - Enclose the compatible array by #ifdef's, this may save a bit of
>   memory but will reduce build coverage.
> - Tell the compiler the array may be unused, if this can be avoided,
>   let's not do this.
> - Just drop the macro, setting the of_device_id array for a non OF
>   enabled platform is not an issue, it will just be unused.
> 
> The latter solution seems the more appropriate, so let's use it.

I disagree. The proper solution would be to not have of_match_ptr() 
conditionally defined.

Right now it's defined basically like this:
#ifdef CONFIG_OF
#define of_match_ptr(_ptr) (_ptr)
#else
#define of_match_ptr(_ptr) NULL
#endif

This is bad, because in the !CONFIG_OF case, the pointer is never 
referenced, and the compiler complains about it, as you can notice.

Instead, it should be defined like this:
#define of_match_ptr(_ptr) PTR_IF(IS_ENABLED(CONFIG_OF), (_ptr))

Then in the !CONFIG_OF case the compiler will see the array as 
effectively unused, and drop it as needed.

We are doing the exact same work with the PM callbacks, with the new 
pm_ptr() macro.

Note that I don't know the behaviour of MODULE_DEVICE_TABLE(of, ...), 
it might be a good idea to make it a NOP if !CONFIG_OF so that the 
array is removed by the compiler as dead code (if it's not the case 
already).

Cheers,
-Paul

> 
> Reported-by: kernel test robot <lkp at intel.com>
> Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>
> ---
>  drivers/mtd/devices/mchp23k256.c                | 2 +-
>  drivers/mtd/devices/mchp48l640.c                | 2 +-
>  drivers/mtd/nand/raw/atmel/nand-controller.c    | 2 +-
>  drivers/mtd/nand/raw/atmel/pmecc.c              | 2 +-
>  drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c | 2 +-
>  drivers/mtd/nand/raw/ingenic/jz4780_bch.c       | 2 +-
>  drivers/mtd/nand/raw/mtk_ecc.c                  | 2 +-
>  drivers/mtd/nand/raw/omap2.c                    | 2 +-
>  drivers/mtd/nand/raw/renesas-nand-controller.c  | 2 +-
>  drivers/mtd/nand/raw/sh_flctl.c                 | 2 +-
>  10 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/devices/mchp23k256.c 
> b/drivers/mtd/devices/mchp23k256.c
> index a8b31bddf14b..bf51eebf052c 100644
> --- a/drivers/mtd/devices/mchp23k256.c
> +++ b/drivers/mtd/devices/mchp23k256.c
> @@ -234,7 +234,7 @@ MODULE_DEVICE_TABLE(of, mchp23k256_of_table);
>  static struct spi_driver mchp23k256_driver = {
>  	.driver = {
>  		.name	= "mchp23k256",
> -		.of_match_table = of_match_ptr(mchp23k256_of_table),
> +		.of_match_table = mchp23k256_of_table,
>  	},
>  	.probe		= mchp23k256_probe,
>  	.remove		= mchp23k256_remove,
> diff --git a/drivers/mtd/devices/mchp48l640.c 
> b/drivers/mtd/devices/mchp48l640.c
> index 231a10790196..4ec505b3f4a6 100644
> --- a/drivers/mtd/devices/mchp48l640.c
> +++ b/drivers/mtd/devices/mchp48l640.c
> @@ -362,7 +362,7 @@ MODULE_DEVICE_TABLE(of, mchp48l640_of_table);
>  static struct spi_driver mchp48l640_driver = {
>  	.driver = {
>  		.name	= "mchp48l640",
> -		.of_match_table = of_match_ptr(mchp48l640_of_table),
> +		.of_match_table = mchp48l640_of_table,
>  	},
>  	.probe		= mchp48l640_probe,
>  	.remove		= mchp48l640_remove,
> diff --git a/drivers/mtd/nand/raw/atmel/nand-controller.c 
> b/drivers/mtd/nand/raw/atmel/nand-controller.c
> index f3276ee9e4fe..4ecbaccf5526 100644
> --- a/drivers/mtd/nand/raw/atmel/nand-controller.c
> +++ b/drivers/mtd/nand/raw/atmel/nand-controller.c
> @@ -2648,7 +2648,7 @@ static 
> SIMPLE_DEV_PM_OPS(atmel_nand_controller_pm_ops, NULL,
>  static struct platform_driver atmel_nand_controller_driver = {
>  	.driver = {
>  		.name = "atmel-nand-controller",
> -		.of_match_table = of_match_ptr(atmel_nand_controller_of_ids),
> +		.of_match_table = atmel_nand_controller_of_ids,
>  		.pm = &atmel_nand_controller_pm_ops,
>  	},
>  	.probe = atmel_nand_controller_probe,
> diff --git a/drivers/mtd/nand/raw/atmel/pmecc.c 
> b/drivers/mtd/nand/raw/atmel/pmecc.c
> index 498e41ccabbd..43ebd78816c0 100644
> --- a/drivers/mtd/nand/raw/atmel/pmecc.c
> +++ b/drivers/mtd/nand/raw/atmel/pmecc.c
> @@ -1003,7 +1003,7 @@ static int atmel_pmecc_probe(struct 
> platform_device *pdev)
>  static struct platform_driver atmel_pmecc_driver = {
>  	.driver = {
>  		.name = "atmel-pmecc",
> -		.of_match_table = of_match_ptr(atmel_pmecc_match),
> +		.of_match_table = atmel_pmecc_match,
>  	},
>  	.probe = atmel_pmecc_probe,
>  };
> diff --git a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c 
> b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> index b18861bdcdc8..ff26c10f295d 100644
> --- a/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> +++ b/drivers/mtd/nand/raw/ingenic/ingenic_nand_drv.c
> @@ -567,7 +567,7 @@ static struct platform_driver ingenic_nand_driver 
> = {
>  	.remove		= ingenic_nand_remove,
>  	.driver	= {
>  		.name	= DRV_NAME,
> -		.of_match_table = of_match_ptr(ingenic_nand_dt_match),
> +		.of_match_table = ingenic_nand_dt_match,
>  	},
>  };
>  module_platform_driver(ingenic_nand_driver);
> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c 
> b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
> index d67dbfff76cc..12b5b0484fe9 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
> @@ -260,7 +260,7 @@ static struct platform_driver jz4780_bch_driver = 
> {
>  	.probe		= jz4780_bch_probe,
>  	.driver	= {
>  		.name	= "jz4780-bch",
> -		.of_match_table = of_match_ptr(jz4780_bch_dt_match),
> +		.of_match_table = jz4780_bch_dt_match,
>  	},
>  };
>  module_platform_driver(jz4780_bch_driver);
> diff --git a/drivers/mtd/nand/raw/mtk_ecc.c 
> b/drivers/mtd/nand/raw/mtk_ecc.c
> index 1b47964cb6da..e7df3dac705e 100644
> --- a/drivers/mtd/nand/raw/mtk_ecc.c
> +++ b/drivers/mtd/nand/raw/mtk_ecc.c
> @@ -579,7 +579,7 @@ static struct platform_driver mtk_ecc_driver = {
>  	.probe  = mtk_ecc_probe,
>  	.driver = {
>  		.name  = "mtk-ecc",
> -		.of_match_table = of_match_ptr(mtk_ecc_dt_match),
> +		.of_match_table = mtk_ecc_dt_match,
>  #ifdef CONFIG_PM_SLEEP
>  		.pm = &mtk_ecc_pm_ops,
>  #endif
> diff --git a/drivers/mtd/nand/raw/omap2.c 
> b/drivers/mtd/nand/raw/omap2.c
> index f0bbbe401e76..58c32a11792e 100644
> --- a/drivers/mtd/nand/raw/omap2.c
> +++ b/drivers/mtd/nand/raw/omap2.c
> @@ -2298,7 +2298,7 @@ static struct platform_driver omap_nand_driver 
> = {
>  	.remove		= omap_nand_remove,
>  	.driver		= {
>  		.name	= DRIVER_NAME,
> -		.of_match_table = of_match_ptr(omap_nand_ids),
> +		.of_match_table = omap_nand_ids,
>  	},
>  };
> 
> diff --git a/drivers/mtd/nand/raw/renesas-nand-controller.c 
> b/drivers/mtd/nand/raw/renesas-nand-controller.c
> index 428e08362956..6db063b230a9 100644
> --- a/drivers/mtd/nand/raw/renesas-nand-controller.c
> +++ b/drivers/mtd/nand/raw/renesas-nand-controller.c
> @@ -1412,7 +1412,7 @@ MODULE_DEVICE_TABLE(of, rnandc_id_table);
>  static struct platform_driver rnandc_driver = {
>  	.driver = {
>  		.name = "renesas-nandc",
> -		.of_match_table = of_match_ptr(rnandc_id_table),
> +		.of_match_table = rnandc_id_table,
>  	},
>  	.probe = rnandc_probe,
>  	.remove = rnandc_remove,
> diff --git a/drivers/mtd/nand/raw/sh_flctl.c 
> b/drivers/mtd/nand/raw/sh_flctl.c
> index 13df4bdf792a..b85b9c6fcc42 100644
> --- a/drivers/mtd/nand/raw/sh_flctl.c
> +++ b/drivers/mtd/nand/raw/sh_flctl.c
> @@ -1220,7 +1220,7 @@ static struct platform_driver flctl_driver = {
>  	.remove		= flctl_remove,
>  	.driver = {
>  		.name	= "sh_flctl",
> -		.of_match_table = of_match_ptr(of_flctl_match),
> +		.of_match_table = of_flctl_match,
>  	},
>  };
> 
> --
> 2.27.0
> 





More information about the linux-mtd mailing list