Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"

36 views
Skip to first unread message

Chen Gang

unread,
Mar 4, 2015, 1:40:06 AM3/4/15
to
They need several symbols which are in I2C and I2C_MUX, the related
error:

drivers/built-in.o: In function `selftest_i2c_mux_remove':
unittest.c:(.text+0xb0ce4): undefined reference to `i2c_del_mux_adapter'
unittest.c:(.text+0xb0ce8): undefined reference to `i2c_del_mux_adapter'
drivers/built-in.o: In function `selftest_i2c_mux_probe':
unittest.c:(.text+0xb0f20): undefined reference to `i2c_add_mux_adapter'
unittest.c:(.text+0xb0f24): undefined reference to `i2c_add_mux_adapter'
unittest.c:(.text+0xb0f94): undefined reference to `i2c_del_mux_adapter'
unittest.c:(.text+0xb0f9c): undefined reference to `i2c_del_mux_adapter'
drivers/built-in.o: In function `selftest_i2c_bus_remove':
unittest.c:(.text+0xb10cc): undefined reference to `i2c_del_adapter'
unittest.c:(.text+0xb10d4): undefined reference to `i2c_del_adapter'
drivers/built-in.o: In function `selftest_i2c_bus_probe':
unittest.c:(.text+0xb1298): undefined reference to `i2c_add_numbered_adapter'
unittest.c:(.text+0xb12a0): undefined reference to `i2c_add_numbered_adapter'
drivers/built-in.o: In function `of_selftest_overlay':
unittest.c:(.init.text+0xc9d0): undefined reference to `i2c_register_driver'
unittest.c:(.init.text+0xc9dc): undefined reference to `i2c_register_driver'
unittest.c:(.init.text+0xcdb4): undefined reference to `i2c_del_driver'
unittest.c:(.init.text+0xcdb8): undefined reference to `i2c_del_driver'
drivers/built-in.o: In function `of_selftest_device_exists':
unittest.c:(.text.unlikely+0xd70): undefined reference to `of_find_i2c_device_by_node'
unittest.c:(.text.unlikely+0xd7c): undefined reference to `of_find_i2c_device_by_node'
make: *** [vmlinux] Error 1

Signed-off-by: Chen Gang <gang.ch...@gmail.com>
---
drivers/of/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 7bcaeec..b60fc66 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -9,7 +9,7 @@ menu "Device Tree and Open Firmware support"

config OF_UNITTEST
bool "Device Tree runtime unit tests"
- depends on OF_IRQ && OF_EARLY_FLATTREE
+ depends on OF_IRQ && OF_EARLY_FLATTREE && I2C=y && I2C_MUX=y
select OF_RESOLVE
help
This option builds in test cases for the device tree infrastructure
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Geert Uytterhoeven

unread,
Mar 4, 2015, 10:10:06 AM3/4/15
to
As this is "bool"...

> - depends on OF_IRQ && OF_EARLY_FLATTREE
> + depends on OF_IRQ && OF_EARLY_FLATTREE && I2C=y && I2C_MUX=y

... I think it would be better to replace "#if IS_ENABLED(CONFIG_XXX)" by
"#ifdef CONFIG_XXX" in drivers/of/unittest.c instead.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Arnd Bergmann

unread,
Mar 4, 2015, 3:00:07 PM3/4/15
to
On Wednesday 04 March 2015 16:04:23 Geert Uytterhoeven wrote:
> > - depends on OF_IRQ && OF_EARLY_FLATTREE
> > + depends on OF_IRQ && OF_EARLY_FLATTREE && I2C=y && I2C_MUX=y
>
> ... I think it would be better to replace "#if IS_ENABLED(CONFIG_XXX)" by
> "#ifdef CONFIG_XXX" in drivers/of/unittest.c instead.
>

Agreed. I came across the same bug and came to the same conclusion as you.

How about this:

8<----
Subject: of: unittest: fix I2C dependency

The unittest fails to link if I2C or I2C_MUX is a loadable module:

drivers/built-in.o: In function `selftest_i2c_mux_remove':
unittest.c:(.text+0xb0ce4): undefined reference to `i2c_del_mux_adapter'

This changes the newly added IS_ENABLED() checks to use IS_BUILTIN()
instead, which evaluates to false if the other driver is a module.

Reported-by: Chen Gang <gang.ch...@gmail.com>
Signed-off-by: Arnd Bergmann <ar...@arndb.de>
Fixes: d5e75500ca401 ("of: unitest: Add I2C overlay unit tests.")

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 0cf9a236d438..8daa49206c36 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -979,7 +979,7 @@ static int of_path_platform_device_exists(const char *path)
return pdev != NULL;
}

-#if IS_ENABLED(CONFIG_I2C)
+#if IS_BUILTIN(CONFIG_I2C)

/* get the i2c client device instantiated at the path */
static struct i2c_client *of_path_to_i2c_client(const char *path)
@@ -1445,7 +1445,7 @@ static void of_selftest_overlay_11(void)
return;
}

-#if IS_ENABLED(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY)
+#if IS_BUILTIN(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY)

struct selftest_i2c_bus_data {
struct platform_device *pdev;
@@ -1584,7 +1584,7 @@ static struct i2c_driver selftest_i2c_dev_driver = {
.id_table = selftest_i2c_dev_id,
};

-#if IS_ENABLED(CONFIG_I2C_MUX)
+#if IS_BUILTIN(CONFIG_I2C_MUX)

struct selftest_i2c_mux_data {
int nchans;
@@ -1695,7 +1695,7 @@ static int of_selftest_overlay_i2c_init(void)
"could not register selftest i2c bus driver\n"))
return ret;

-#if IS_ENABLED(CONFIG_I2C_MUX)
+#if IS_BUILTIN(CONFIG_I2C_MUX)
ret = i2c_add_driver(&selftest_i2c_mux_driver);
if (selftest(ret == 0,
"could not register selftest i2c mux driver\n"))
@@ -1707,7 +1707,7 @@ static int of_selftest_overlay_i2c_init(void)

static void of_selftest_overlay_i2c_cleanup(void)
{
-#if IS_ENABLED(CONFIG_I2C_MUX)
+#if IS_BUILTIN(CONFIG_I2C_MUX)
i2c_del_driver(&selftest_i2c_mux_driver);
#endif
platform_driver_unregister(&selftest_i2c_bus_driver);
@@ -1814,7 +1814,7 @@ static void __init of_selftest_overlay(void)
of_selftest_overlay_10();
of_selftest_overlay_11();

-#if IS_ENABLED(CONFIG_I2C)
+#if IS_BUILTIN(CONFIG_I2C)
if (selftest(of_selftest_overlay_i2c_init() == 0, "i2c init failed\n"))
goto out;

Pantelis Antoniou

unread,
Mar 4, 2015, 3:00:13 PM3/4/15
to
Hi Arnd,
Patch is fine. I’ll pick it up for the next batch of overlay patches.

Regards

— Pantelis

Geert Uytterhoeven

unread,
Mar 5, 2015, 3:10:06 AM3/5/15
to
On Wed, Mar 4, 2015 at 8:49 PM, Arnd Bergmann <ar...@arndb.de> wrote:
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -979,7 +979,7 @@ static int of_path_platform_device_exists(const char *path)
> return pdev != NULL;
> }
>
> -#if IS_ENABLED(CONFIG_I2C)
> +#if IS_BUILTIN(CONFIG_I2C)

Wondering: is there any advantage in using "#if IS_BUILTIN(CONFIG_XXX)"
instead of "#ifdef CONFIG_XXX"?

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

Chen Gang

unread,
Mar 5, 2015, 2:50:06 PM3/5/15
to
On 3/5/15 16:06, Geert Uytterhoeven wrote:
> On Wed, Mar 4, 2015 at 8:49 PM, Arnd Bergmann <ar...@arndb.de> wrote:
>> --- a/drivers/of/unittest.c
>> +++ b/drivers/of/unittest.c
>> @@ -979,7 +979,7 @@ static int of_path_platform_device_exists(const char *path)
>> return pdev != NULL;
>> }
>>
>> -#if IS_ENABLED(CONFIG_I2C)
>> +#if IS_BUILTIN(CONFIG_I2C)
>
> Wondering: is there any advantage in using "#if IS_BUILTIN(CONFIG_XXX)"
> instead of "#ifdef CONFIG_XXX"?
>

For me, "#if IS_BUILTIN(CONFIG_XXX)" is better than "#ifdef CONFIG_XXX":

- "#if IS_BUILTIN(CONFIG_XXX)" is only for CONFIG_XXX=y, and its name
is also obvious.

- "#ifdef CONFIG_XXX" may be for others, e.g. CONFIG_DEFCONFIG_LIST:

.config:13:CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
include/generated/autoconf.h:1174:#define CONFIG_DEFCONFIG_LIST "/lib/modules/$UNAME_RELEASE/.config"


And this time, arnd's patch is OK to me.

Thanks.
--
Chen Gang

Open, share, and attitude like air, water, and life which God blessed

Rob Herring

unread,
Mar 5, 2015, 3:10:09 PM3/5/15
to
On Wed, Mar 4, 2015 at 1:58 PM, Pantelis Antoniou
<pantelis...@konsulko.com> wrote:
> Hi Arnd,
>
>> On Mar 4, 2015, at 21:49 , Arnd Bergmann <ar...@arndb.de> wrote:
>>
>> On Wednesday 04 March 2015 16:04:23 Geert Uytterhoeven wrote:
>>>> - depends on OF_IRQ && OF_EARLY_FLATTREE
>>>> + depends on OF_IRQ && OF_EARLY_FLATTREE && I2C=y && I2C_MUX=y
>>>
>>> ... I think it would be better to replace "#if IS_ENABLED(CONFIG_XXX)" by
>>> "#ifdef CONFIG_XXX" in drivers/of/unittest.c instead.
>>>
>>
>> Agreed. I came across the same bug and came to the same conclusion as you.
>>
>> How about this:
>>
>> 8<----
>> Subject: of: unittest: fix I2C dependency

[...]

>> -#if IS_ENABLED(CONFIG_I2C)
>> +#if IS_BUILTIN(CONFIG_I2C)
>> if (selftest(of_selftest_overlay_i2c_init() == 0, "i2c init failed\n"))
>> goto out;
>>
>>
>
> Patch is fine. I’ll pick it up for the next batch of overlay patches.

Do you have other things that need to go into 4.0? If not I'll pick this up.

Rob

Arnd Bergmann

unread,
Mar 9, 2015, 5:30:07 PM3/9/15
to
On Thursday 05 March 2015 09:06:54 Geert Uytterhoeven wrote:
> On Wed, Mar 4, 2015 at 8:49 PM, Arnd Bergmann <ar...@arndb.de> wrote:
> > --- a/drivers/of/unittest.c
> > +++ b/drivers/of/unittest.c
> > @@ -979,7 +979,7 @@ static int of_path_platform_device_exists(const char *path)
> > return pdev != NULL;
> > }
> >
> > -#if IS_ENABLED(CONFIG_I2C)
> > +#if IS_BUILTIN(CONFIG_I2C)
>
> Wondering: is there any advantage in using "#if IS_BUILTIN(CONFIG_XXX)"
> instead of "#ifdef CONFIG_XXX"?

Mostly consistency within the file.

There are also lines like

#if IS_BUILTIN(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY)

which I find more readable than mixing the two styles as in

#if defined(CONFIG_I2C) && IS_ENABLED(CONFIG_OF_OVERLAY)

Arnd

Rob Herring

unread,
Mar 13, 2015, 11:30:06 AM3/13/15
to
On Wed, Mar 4, 2015 at 1:49 PM, Arnd Bergmann <ar...@arndb.de> wrote:
> On Wednesday 04 March 2015 16:04:23 Geert Uytterhoeven wrote:
>> > - depends on OF_IRQ && OF_EARLY_FLATTREE
>> > + depends on OF_IRQ && OF_EARLY_FLATTREE && I2C=y && I2C_MUX=y
>>
>> ... I think it would be better to replace "#if IS_ENABLED(CONFIG_XXX)" by
>> "#ifdef CONFIG_XXX" in drivers/of/unittest.c instead.
>>
>
> Agreed. I came across the same bug and came to the same conclusion as you.
>
> How about this:
>
> 8<----
> Subject: of: unittest: fix I2C dependency
>
> The unittest fails to link if I2C or I2C_MUX is a loadable module:
>
> drivers/built-in.o: In function `selftest_i2c_mux_remove':
> unittest.c:(.text+0xb0ce4): undefined reference to `i2c_del_mux_adapter'
>
> This changes the newly added IS_ENABLED() checks to use IS_BUILTIN()
> instead, which evaluates to false if the other driver is a module.
>
> Reported-by: Chen Gang <gang.ch...@gmail.com>
> Signed-off-by: Arnd Bergmann <ar...@arndb.de>
> Fixes: d5e75500ca401 ("of: unitest: Add I2C overlay unit tests.")

Applied for 4.0. Thanks.

Rob
0 new messages