[PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[Andrew Lunn]

When building without CONFIG_OF and W=1, errors are given about unused
arrays of match data, because of_match_node is stubbed as a macro. The
compile does not see it takes parameters when not astub, so it
generates warnings about unused variables. Replace the stub with an
inline function to avoid these false warnings.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
 include/linux/of.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index 5cf7ae0465d1..e9838387e7d9 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -991,7 +991,12 @@ static inline int of_map_id(struct device_node *np, u32 id,
 }
 
 #define of_match_ptr(_ptr)	NULL
-#define of_match_node(_matches, _node)	NULL
+
+static inline const struct of_device_id *of_match_node(
+	const struct of_device_id *matches, const struct device_node *node)
+{
+	return NULL;
+}
 #endif /* CONFIG_OF */
 
 /* Default string compare functions, Allow arch asm/prom.h to override */
-- 
2.28.0

Re: [PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3670 bytes --]

Hi Andrew,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linux/master linus/master v5.9-rc2 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Andrew-Lunn/of-of_match_node-Make-stub-an-inline-function-to-avoid-W-1-warnings/20200828-102035
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-a006-20200828 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/misc/atmel-ssc.c: In function 'atmel_ssc_get_driver_data':
>> drivers/misc/atmel-ssc.c:137:25: error: 'atmel_ssc_dt_ids' undeclared (first use in this function); did you mean 'atmel_ssc_devtypes'?
     137 |   match = of_match_node(atmel_ssc_dt_ids, pdev->dev.of_node);
         |                         ^~~~~~~~~~~~~~~~
         |                         atmel_ssc_devtypes
   drivers/misc/atmel-ssc.c:137:25: note: each undeclared identifier is reported only once for each function it appears in
--
   drivers/i2c/busses/i2c-s3c2410.c: In function 's3c24xx_get_device_quirks':
>> drivers/i2c/busses/i2c-s3c2410.c:162:25: error: 's3c24xx_i2c_match' undeclared (first use in this function); did you mean 's3c24xx_i2c_state'?
     162 |   match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
         |                         ^~~~~~~~~~~~~~~~~
         |                         s3c24xx_i2c_state
   drivers/i2c/busses/i2c-s3c2410.c:162:25: note: each undeclared identifier is reported only once for each function it appears in

# https://github.com/0day-ci/linux/commit/3213d0cc82c7a4e0e85e0db3da3da279460c833b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Lunn/of-of_match_node-Make-stub-an-inline-function-to-avoid-W-1-warnings/20200828-102035
git checkout 3213d0cc82c7a4e0e85e0db3da3da279460c833b
vim +137 drivers/misc/atmel-ssc.c

099343c64e1615 Bo Shen           2012-11-07  131  
7c97301285b62a Nathan Chancellor 2018-10-17  132  static inline const struct atmel_ssc_platform_data *
099343c64e1615 Bo Shen           2012-11-07  133  	atmel_ssc_get_driver_data(struct platform_device *pdev)
099343c64e1615 Bo Shen           2012-11-07  134  {
099343c64e1615 Bo Shen           2012-11-07  135  	if (pdev->dev.of_node) {
099343c64e1615 Bo Shen           2012-11-07  136  		const struct of_device_id *match;
099343c64e1615 Bo Shen           2012-11-07 @137  		match = of_match_node(atmel_ssc_dt_ids, pdev->dev.of_node);
099343c64e1615 Bo Shen           2012-11-07  138  		if (match == NULL)
099343c64e1615 Bo Shen           2012-11-07  139  			return NULL;
099343c64e1615 Bo Shen           2012-11-07  140  		return match->data;
099343c64e1615 Bo Shen           2012-11-07  141  	}
099343c64e1615 Bo Shen           2012-11-07  142  
099343c64e1615 Bo Shen           2012-11-07  143  	return (struct atmel_ssc_platform_data *)
099343c64e1615 Bo Shen           2012-11-07  144  		platform_get_device_id(pdev)->driver_data;
099343c64e1615 Bo Shen           2012-11-07  145  }
099343c64e1615 Bo Shen           2012-11-07  146  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36873 bytes --]

Re: [PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]

Hi Andrew,

I love your patch! Yet something to improve:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linux/master linus/master v5.9-rc2 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Andrew-Lunn/of-of_match_node-Make-stub-an-inline-function-to-avoid-W-1-warnings/20200828-102035
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: i386-randconfig-r032-20200828 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
        # save the attached .config to linux build tree
        make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/mfd/max8998.c: In function 'max8998_i2c_get_driver_data':
>> drivers/mfd/max8998.c:160:25: error: 'max8998_dt_match' undeclared (first use in this function)
     160 |   match = of_match_node(max8998_dt_match, i2c->dev.of_node);
         |                         ^~~~~~~~~~~~~~~~
   drivers/mfd/max8998.c:160:25: note: each undeclared identifier is reported only once for each function it appears in

# https://github.com/0day-ci/linux/commit/3213d0cc82c7a4e0e85e0db3da3da279460c833b
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Andrew-Lunn/of-of_match_node-Make-stub-an-inline-function-to-avoid-W-1-warnings/20200828-102035
git checkout 3213d0cc82c7a4e0e85e0db3da3da279460c833b
vim +/max8998_dt_match +160 drivers/mfd/max8998.c

ee999fb3f17faa Tomasz Figa 2013-06-25  154  
8bace2d5b4baa0 Lee Jones   2014-02-03  155  static inline unsigned long max8998_i2c_get_driver_data(struct i2c_client *i2c,
ee999fb3f17faa Tomasz Figa 2013-06-25  156  						const struct i2c_device_id *id)
ee999fb3f17faa Tomasz Figa 2013-06-25  157  {
ee999fb3f17faa Tomasz Figa 2013-06-25  158  	if (IS_ENABLED(CONFIG_OF) && i2c->dev.of_node) {
ee999fb3f17faa Tomasz Figa 2013-06-25  159  		const struct of_device_id *match;
ee999fb3f17faa Tomasz Figa 2013-06-25 @160  		match = of_match_node(max8998_dt_match, i2c->dev.of_node);
8bace2d5b4baa0 Lee Jones   2014-02-03  161  		return (unsigned long)match->data;
ee999fb3f17faa Tomasz Figa 2013-06-25  162  	}
ee999fb3f17faa Tomasz Figa 2013-06-25  163  
8bace2d5b4baa0 Lee Jones   2014-02-03  164  	return id->driver_data;
ee999fb3f17faa Tomasz Figa 2013-06-25  165  }
ee999fb3f17faa Tomasz Figa 2013-06-25  166  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28590 bytes --]

Re: [PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[Andrew Lunn]

On Fri, Aug 28, 2020 at 04:19:39AM +0200, Andrew Lunn wrote:
> When building without CONFIG_OF and W=1, errors are given about unused
> arrays of match data, because of_match_node is stubbed as a macro. The
> compile does not see it takes parameters when not astub, so it
> generates warnings about unused variables. Replace the stub with an
> inline function to avoid these false warnings.

Hi Rob

So 0-day shows some people have worked around this with #ifdef
CONFIG_OF around the match table.

I checked the object code for the file i'm interested in.  The
optimiser has correctly throw away the match table and all code around
it with the inline stub.

Which do you prefer? This patch and i remove the #ifdef, or the old
stub and if add #ifdef around the driver i'm getting warnings from?

     Andrew

Re: [PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[Rob Herring]

On Fri, Aug 28, 2020 at 7:00 AM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Aug 28, 2020 at 04:19:39AM +0200, Andrew Lunn wrote:
> > When building without CONFIG_OF and W=1, errors are given about unused
> > arrays of match data, because of_match_node is stubbed as a macro. The
> > compile does not see it takes parameters when not astub, so it
> > generates warnings about unused variables. Replace the stub with an
> > inline function to avoid these false warnings.
>
> Hi Rob
>
> So 0-day shows some people have worked around this with #ifdef
> CONFIG_OF around the match table.
>
> I checked the object code for the file i'm interested in.  The
> optimiser has correctly throw away the match table and all code around
> it with the inline stub.
>
> Which do you prefer? This patch and i remove the #ifdef, or the old
> stub and if add #ifdef around the driver i'm getting warnings from?

Use of_device_get_match_data instead of of_match_node.

Rob

Re: [PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[Andrew Lunn]

On Fri, Aug 28, 2020 at 05:09:52PM -0600, Rob Herring wrote:
> On Fri, Aug 28, 2020 at 7:00 AM Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Fri, Aug 28, 2020 at 04:19:39AM +0200, Andrew Lunn wrote:
> > > When building without CONFIG_OF and W=1, errors are given about unused
> > > arrays of match data, because of_match_node is stubbed as a macro. The
> > > compile does not see it takes parameters when not astub, so it
> > > generates warnings about unused variables. Replace the stub with an
> > > inline function to avoid these false warnings.
> >
> > Hi Rob
> >
> > So 0-day shows some people have worked around this with #ifdef
> > CONFIG_OF around the match table.
> >
> > I checked the object code for the file i'm interested in.  The
> > optimiser has correctly throw away the match table and all code around
> > it with the inline stub.
> >
> > Which do you prefer? This patch and i remove the #ifdef, or the old
> > stub and if add #ifdef around the driver i'm getting warnings from?
> 
> Use of_device_get_match_data instead of of_match_node.
> 

Hi Rob

That does not work in the use case i'm interested in, which is giving
a W=1 warning. Take a look at the last example in
Documentation/devicetree/bindings/net/dsa/marvell.txt

We have an Ethernet switch, using the compatible string
"marvell,mv88e6390". Embedded within the hardware, and within the same
driver, we have two MDIO busses. One is internal, for the PHYs
integrated into the switch, and one is external, of discrete PHY
connected to the switch. The external MDIO bus has its own compatible
string. However, there is no struct driver for it, the switch driver
is driving the MDIO bus. So of_device_get_match_data() will use the
wrong match table.

      Andrew

Re: [PATCH] of: of_match_node: Make stub an inline function to avoid W=1 warnings

[Andy Shevchenko]

On Sun, Aug 30, 2020 at 6:25 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Fri, Aug 28, 2020 at 05:09:52PM -0600, Rob Herring wrote:
> > On Fri, Aug 28, 2020 at 7:00 AM Andrew Lunn <andrew@lunn.ch> wrote:
> > >
> > > On Fri, Aug 28, 2020 at 04:19:39AM +0200, Andrew Lunn wrote:
> > > > When building without CONFIG_OF and W=1, errors are given about unused
> > > > arrays of match data, because of_match_node is stubbed as a macro. The
> > > > compile does not see it takes parameters when not astub, so it
> > > > generates warnings about unused variables. Replace the stub with an
> > > > inline function to avoid these false warnings.
> > >
> > > Hi Rob
> > >
> > > So 0-day shows some people have worked around this with #ifdef
> > > CONFIG_OF around the match table.
> > >
> > > I checked the object code for the file i'm interested in.  The
> > > optimiser has correctly throw away the match table and all code around
> > > it with the inline stub.
> > >
> > > Which do you prefer? This patch and i remove the #ifdef, or the old
> > > stub and if add #ifdef around the driver i'm getting warnings from?
> >
> > Use of_device_get_match_data instead of of_match_node.
> >
>
> Hi Rob
>
> That does not work in the use case i'm interested in, which is giving
> a W=1 warning. Take a look at the last example in
> Documentation/devicetree/bindings/net/dsa/marvell.txt
>
> We have an Ethernet switch, using the compatible string
> "marvell,mv88e6390". Embedded within the hardware, and within the same
> driver, we have two MDIO busses. One is internal, for the PHYs
> integrated into the switch, and one is external, of discrete PHY
> connected to the switch. The external MDIO bus has its own compatible
> string. However, there is no struct driver for it, the switch driver
> is driving the MDIO bus. So of_device_get_match_data() will use the
> wrong match table.

Looks like in that code you may use of_device_is_compatible().

-- 
With Best Regards,
Andy Shevchenko