| Message ID | 20200828021939.2912798-1-andrew@lunn.ch |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | of: of_match_node: Make stub an inline function to avoid W=1 warnings | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success |
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
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
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
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
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
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().
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 */
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(-)