2017-04-12 04:42:03

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 0/3] of: Make of_match_node() an inline stub for CONFIG_OF=n

Hi all,

This patch series makes of_match_node() an inline stub for CONFIG_OF=n. kbuild
reported two build errors which are fixed as preriquisite patches.

This is based on Linus' master, not sure which tree would merge this, Frank's?

Thanks!

Florian Fainelli (3):
mfd: max8998: Remove CONFIG_OF around max8998_dt_match
net: macb: Remove CONFIG_OF around DT match table
of: Make of_match_node() an inline stub for CONFIG_OF=n

drivers/mfd/max8998.c | 2 --
drivers/net/ethernet/cadence/macb.c | 2 --
include/linux/of.h | 6 +++++-
3 files changed, 5 insertions(+), 5 deletions(-)

--
2.9.3


2017-04-12 04:42:08

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 2/3] net: macb: Remove CONFIG_OF around DT match table

A subsequent patch is going to make of_match_node() an inline stub when
CONFIG_OF is disabled, which will let the compiler eliminate unused variables.
In order not to clutter the code more, remove the CONFIG_OF #ifdef such that
macb_dt_ids and what it references are always defined.

Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/net/ethernet/cadence/macb.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
index 30606b11b128..01016e9525ee 100644
--- a/drivers/net/ethernet/cadence/macb.c
+++ b/drivers/net/ethernet/cadence/macb.c
@@ -2811,7 +2811,6 @@ static int macb_init(struct platform_device *pdev)
return 0;
}

-#if defined(CONFIG_OF)
/* 1518 rounded up */
#define AT91ETHER_MAX_RBUFF_SZ 0x600
/* max number of receive buffers */
@@ -3215,7 +3214,6 @@ static const struct of_device_id macb_dt_ids[] = {
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, macb_dt_ids);
-#endif /* CONFIG_OF */

static const struct macb_config default_gem_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_JUMBO,
--
2.9.3

2017-04-12 04:42:24

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 3/3] of: Make of_match_node() an inline stub for CONFIG_OF=n

Make of_match_node() an inline function when CONFIG_OF=n which allows the
compiler to eliminate warnings about unused variables.

Signed-off-by: Florian Fainelli <[email protected]>
---
include/linux/of.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/of.h b/include/linux/of.h
index 21e6323de0f3..2803a85e81ec 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -839,7 +839,11 @@ static inline void of_property_clear_flag(struct property *p, unsigned long flag
}

#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.9.3

2017-04-12 04:42:47

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH v2 1/3] mfd: max8998: Remove CONFIG_OF around max8998_dt_match

A subsequent patch is going to make of_match_node() an inline stub when
CONFIG_OF is disabled which will properly take care of having the compiler
eliminate the variable. To avoid more #ifdef/#else, just always make the match
table available.

Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/mfd/max8998.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 4c33b8063bc3..372f681ec1bb 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -129,14 +129,12 @@ int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
}
EXPORT_SYMBOL(max8998_update_reg);

-#ifdef CONFIG_OF
static const struct of_device_id max8998_dt_match[] = {
{ .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
{ .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
{ .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
{},
};
-#endif

/*
* Only the common platform data elements for max8998 are parsed here from the
--
2.9.3

2017-04-12 07:59:27

by Lee Jones

[permalink] [raw]
Subject: Re: [PATCH v2 1/3] mfd: max8998: Remove CONFIG_OF around max8998_dt_match

On Tue, 11 Apr 2017, Florian Fainelli wrote:

> A subsequent patch is going to make of_match_node() an inline stub when
> CONFIG_OF is disabled which will properly take care of having the compiler
> eliminate the variable. To avoid more #ifdef/#else, just always make the match
> table available.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/mfd/max8998.c | 2 --
> 1 file changed, 2 deletions(-)

If it works, great!

For my own reference:
Acked-for-MFD-by: Lee Jones <[email protected]>

> diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
> index 4c33b8063bc3..372f681ec1bb 100644
> --- a/drivers/mfd/max8998.c
> +++ b/drivers/mfd/max8998.c
> @@ -129,14 +129,12 @@ int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
> }
> EXPORT_SYMBOL(max8998_update_reg);
>
> -#ifdef CONFIG_OF
> static const struct of_device_id max8998_dt_match[] = {
> { .compatible = "maxim,max8998", .data = (void *)TYPE_MAX8998 },
> { .compatible = "national,lp3974", .data = (void *)TYPE_LP3974 },
> { .compatible = "ti,lp3974", .data = (void *)TYPE_LP3974 },
> {},
> };
> -#endif
>
> /*
> * Only the common platform data elements for max8998 are parsed here from the

--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

2017-04-12 14:22:18

by David Miller

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] of: Make of_match_node() an inline stub for CONFIG_OF=n

From: Florian Fainelli <[email protected]>
Date: Tue, 11 Apr 2017 21:41:53 -0700

> Hi all,
>
> This patch series makes of_match_node() an inline stub for CONFIG_OF=n. kbuild
> reported two build errors which are fixed as preriquisite patches.
>
> This is based on Linus' master, not sure which tree would merge this, Frank's?

I think Frank's would be best. For the series:

Acked-by: David S. Miller <[email protected]>

2017-04-12 18:58:45

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] of: Make of_match_node() an inline stub for CONFIG_OF=n

Hi Florian,

[auto build test ERROR on ljones-mfd/for-mfd-next]
[also build test ERROR on v4.11-rc6 next-20170412]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Florian-Fainelli/of-Make-of_match_node-an-inline-stub-for-CONFIG_OF-n/20170412-160916
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: arm-s3c2410_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm

All errors (new ones prefixed by >>):

drivers/i2c/busses/i2c-s3c2410.c: In function 's3c24xx_get_device_quirks':
>> drivers/i2c/busses/i2c-s3c2410.c:174:25: error: 's3c24xx_i2c_match' undeclared (first use in this function)
match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
^~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-s3c2410.c:174:25: note: each undeclared identifier is reported only once for each function it appears in
--
drivers/watchdog/s3c2410_wdt.c: In function 's3c2410_get_wdt_drv_data':
>> drivers/watchdog/s3c2410_wdt.c:515:25: error: 's3c2410_wdt_match' undeclared (first use in this function)
match = of_match_node(s3c2410_wdt_match, pdev->dev.of_node);
^~~~~~~~~~~~~~~~~
drivers/watchdog/s3c2410_wdt.c:515:25: note: each undeclared identifier is reported only once for each function it appears in

vim +/s3c24xx_i2c_match +174 drivers/i2c/busses/i2c-s3c2410.c

faf93ff6 Giridhar Maruthy 2013-01-24 158 .data = (void *)(QUIRK_S3C2440 | QUIRK_NO_GPIO) },
117053f7 Vasanth Ananthan 2013-11-11 159 { .compatible = "samsung,exynos5-sata-phy-i2c",
117053f7 Vasanth Ananthan 2013-11-11 160 .data = (void *)(QUIRK_S3C2440 | QUIRK_POLL | QUIRK_NO_GPIO) },
27452498 Karol Lewandowski 2012-04-23 161 {},
27452498 Karol Lewandowski 2012-04-23 162 };
27452498 Karol Lewandowski 2012-04-23 163 MODULE_DEVICE_TABLE(of, s3c24xx_i2c_match);
27452498 Karol Lewandowski 2012-04-23 164 #endif
^1da177e Linus Torvalds 2005-04-16 165
ec7c34a4 Krzysztof Kozlowski 2016-04-21 166 /*
27452498 Karol Lewandowski 2012-04-23 167 * Get controller type either from device tree or platform device variant.
^1da177e Linus Torvalds 2005-04-16 168 */
5f1b1155 Pankaj Dubey 2014-01-15 169 static inline kernel_ulong_t s3c24xx_get_device_quirks(struct platform_device *pdev)
^1da177e Linus Torvalds 2005-04-16 170 {
27452498 Karol Lewandowski 2012-04-23 171 if (pdev->dev.of_node) {
27452498 Karol Lewandowski 2012-04-23 172 const struct of_device_id *match;
0915833b Krzysztof Kozlowski 2016-04-21 173
b900ba4c Karol Lewandowski 2012-05-30 @174 match = of_match_node(s3c24xx_i2c_match, pdev->dev.of_node);
5f1b1155 Pankaj Dubey 2014-01-15 175 return (kernel_ulong_t)match->data;
27452498 Karol Lewandowski 2012-04-23 176 }
5a5f5080 Thomas Abraham 2011-09-13 177
27452498 Karol Lewandowski 2012-04-23 178 return platform_get_device_id(pdev)->driver_data;
^1da177e Linus Torvalds 2005-04-16 179 }
^1da177e Linus Torvalds 2005-04-16 180
ec7c34a4 Krzysztof Kozlowski 2016-04-21 181 /*
ec7c34a4 Krzysztof Kozlowski 2016-04-21 182 * Complete the message and wake up the caller, using the given return code,

:::::: The code at line 174 was first introduced by commit
:::::: b900ba4c1513a8c9a2fab8dca4cc6f50b17d6861 i2c: s3c2410: Fix pointer type passed to of_match_node()

:::::: TO: Karol Lewandowski <[email protected]>
:::::: CC: Wolfram Sang <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.94 kB)
.config.gz (22.12 kB)
Download all attachments

2017-04-12 20:52:01

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] net: macb: Remove CONFIG_OF around DT match table

On Tue, Apr 11, 2017 at 11:41 PM, Florian Fainelli <[email protected]> wrote:
> A subsequent patch is going to make of_match_node() an inline stub when
> CONFIG_OF is disabled, which will let the compiler eliminate unused variables.
> In order not to clutter the code more, remove the CONFIG_OF #ifdef such that
> macb_dt_ids and what it references are always defined.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 30606b11b128..01016e9525ee 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2811,7 +2811,6 @@ static int macb_init(struct platform_device *pdev)
> return 0;
> }
>
> -#if defined(CONFIG_OF)
> /* 1518 rounded up */
> #define AT91ETHER_MAX_RBUFF_SZ 0x600
> /* max number of receive buffers */
> @@ -3215,7 +3214,6 @@ static const struct of_device_id macb_dt_ids[] = {
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, macb_dt_ids);

Did you actually check that the compiler can eliminate things when
modules are enabled? Because it's not going to be able to eliminate
MODULE_DEVICE_TABLE entry AFAICT.

Rob

2017-04-13 03:47:08

by Frank Rowand

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] of: Make of_match_node() an inline stub for CONFIG_OF=n

On 04/11/17 21:41, Florian Fainelli wrote:
> Hi all,
>
> This patch series makes of_match_node() an inline stub for CONFIG_OF=n. kbuild
> reported two build errors which are fixed as preriquisite patches.
>
> This is based on Linus' master, not sure which tree would merge this, Frank's?

It would come in via Rob.

I am not comfortable with patch 3/3 at this moment.

Version 1 of the patch resulted in two errors from the kbuild test robot. This
version results in another error from the kbuild test robot. I know it is a
lot of work, but please look at all of the callers of of_match_node() and
check whether any of the other callers will have the same type of error that
the kbuild test robot is catching.

-Frank

>
> Thanks!
>
> Florian Fainelli (3):
> mfd: max8998: Remove CONFIG_OF around max8998_dt_match
> net: macb: Remove CONFIG_OF around DT match table
> of: Make of_match_node() an inline stub for CONFIG_OF=n
>
> drivers/mfd/max8998.c | 2 --
> drivers/net/ethernet/cadence/macb.c | 2 --
> include/linux/of.h | 6 +++++-
> 3 files changed, 5 insertions(+), 5 deletions(-)
>