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

public inbox for devicetree@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
@ 2015-03-04  6:37 Chen Gang
  2015-03-04 15:04 ` Geert Uytterhoeven
  0 siblings, 1 reply; 9+ messages in thread
From: Chen Gang @ 2015-03-04  6:37 UTC (permalink / raw)
  To: grant.likely, robh+dt; +Cc: devicetree, linux-kernel@vger.kernel.org

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.chen.5i5j@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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-04  6:37 [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y" Chen Gang
@ 2015-03-04 15:04 ` Geert Uytterhoeven
  2015-03-04 19:49   ` Arnd Bergmann
  0 siblings, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2015-03-04 15:04 UTC (permalink / raw)
  To: Chen Gang, Pantelis Antoniou
  Cc: Grant Likely, Rob Herring, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Wed, Mar 4, 2015 at 7:37 AM, Chen Gang <xili_gchen_5257@hotmail.com> wrote:
> 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.chen.5i5j@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"

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 -- geert@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-04 15:04 ` Geert Uytterhoeven
@ 2015-03-04 19:49   ` Arnd Bergmann
  2015-03-04 19:58     ` Pantelis Antoniou
                       ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Arnd Bergmann @ 2015-03-04 19:49 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Chen Gang, Pantelis Antoniou, Grant Likely, Rob Herring,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org

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.chen.5i5j@gmail.com>
Signed-off-by: Arnd Bergmann <arnd@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;
 

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-04 19:49   ` Arnd Bergmann
@ 2015-03-04 19:58     ` Pantelis Antoniou
  2015-03-05 20:01       ` Rob Herring
  2015-03-05  8:06     ` Geert Uytterhoeven
  2015-03-13 15:23     ` Rob Herring
  2 siblings, 1 reply; 9+ messages in thread
From: Pantelis Antoniou @ 2015-03-04 19:58 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Geert Uytterhoeven, Chen Gang, Grant Likely, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

Hi Arnd,

> On Mar 4, 2015, at 21:49 , Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> 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.chen.5i5j-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> 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;
> 
> 

Patch is fine. I’ll pick it up for the next batch of overlay patches.

Regards

— Pantelis

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-04 19:58     ` Pantelis Antoniou
@ 2015-03-05 20:01       ` Rob Herring
  0 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2015-03-05 20:01 UTC (permalink / raw)
  To: Pantelis Antoniou
  Cc: Arnd Bergmann, Geert Uytterhoeven, Chen Gang, Grant Likely,
	Rob Herring, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Wed, Mar 4, 2015 at 1:58 PM, Pantelis Antoniou
<pantelis.antoniou@konsulko.com> wrote:
> Hi Arnd,
>
>> On Mar 4, 2015, at 21:49 , Arnd Bergmann <arnd@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-04 19:49   ` Arnd Bergmann
  2015-03-04 19:58     ` Pantelis Antoniou
@ 2015-03-05  8:06     ` Geert Uytterhoeven
  2015-03-05 19:51       ` Chen Gang
  2015-03-09 21:28       ` Arnd Bergmann
  2015-03-13 15:23     ` Rob Herring
  2 siblings, 2 replies; 9+ messages in thread
From: Geert Uytterhoeven @ 2015-03-05  8:06 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Chen Gang, Pantelis Antoniou, Grant Likely, Rob Herring,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Wed, Mar 4, 2015 at 8:49 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> 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 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-05  8:06     ` Geert Uytterhoeven
@ 2015-03-05 19:51       ` Chen Gang
  2015-03-09 21:28       ` Arnd Bergmann
  1 sibling, 0 replies; 9+ messages in thread
From: Chen Gang @ 2015-03-05 19:51 UTC (permalink / raw)
  To: Geert Uytterhoeven, Arnd Bergmann
  Cc: Pantelis Antoniou, Grant Likely, Rob Herring,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org

On 3/5/15 16:06, Geert Uytterhoeven wrote:
> On Wed, Mar 4, 2015 at 8:49 PM, Arnd Bergmann <arnd@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-05  8:06     ` Geert Uytterhoeven
  2015-03-05 19:51       ` Chen Gang
@ 2015-03-09 21:28       ` Arnd Bergmann
  1 sibling, 0 replies; 9+ messages in thread
From: Arnd Bergmann @ 2015-03-09 21:28 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Chen Gang, Pantelis Antoniou, Grant Likely, Rob Herring,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org

On Thursday 05 March 2015 09:06:54 Geert Uytterhoeven wrote:
> On Wed, Mar 4, 2015 at 8:49 PM, Arnd Bergmann <arnd@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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y"
  2015-03-04 19:49   ` Arnd Bergmann
  2015-03-04 19:58     ` Pantelis Antoniou
  2015-03-05  8:06     ` Geert Uytterhoeven
@ 2015-03-13 15:23     ` Rob Herring
  2 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2015-03-13 15:23 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Geert Uytterhoeven, Chen Gang, Pantelis Antoniou, Grant Likely,
	Rob Herring, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org

On Wed, Mar 4, 2015 at 1:49 PM, Arnd Bergmann <arnd@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.chen.5i5j@gmail.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Fixes: d5e75500ca401 ("of: unitest: Add I2C overlay unit tests.")

Applied for 4.0. Thanks.

Rob

>
> 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;
>
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-03-13 15:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-04  6:37 [PATCH] of: Kconfig: Let OF_UNITTEST depend on "I2C=y" and "I2C_MUX=y" Chen Gang
2015-03-04 15:04 ` Geert Uytterhoeven
2015-03-04 19:49   ` Arnd Bergmann
2015-03-04 19:58     ` Pantelis Antoniou
2015-03-05 20:01       ` Rob Herring
2015-03-05  8:06     ` Geert Uytterhoeven
2015-03-05 19:51       ` Chen Gang
2015-03-09 21:28       ` Arnd Bergmann
2015-03-13 15:23     ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox