* [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure @ 2014-03-20 15:16 Paul Gortmaker 2014-03-21 13:32 ` Tomi Valkeinen 0 siblings, 1 reply; 5+ messages in thread From: Paul Gortmaker @ 2014-03-20 15:16 UTC (permalink / raw) To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen Cc: linux-fbdev, linux-kernel, Jim Davis, Fengguang Wu, Paul Gortmaker Any randconfig that sets I2C=m and FB_MB862XX_I2C=y will encounter a final link failure that looks like this: drivers/built-in.o: In function `mb862xx_i2c_init': drivers/video/mb862xx/mb862xx-i2c.c:165: undefined reference to `i2c_add_adapter' drivers/built-in.o: In function `mb862xx_i2c_exit': drivers/video/mb862xx/mb862xx-i2c.c:176: undefined reference to `i2c_del_adapter' Since FB_MB862XX_I2C is a bool and not tristate, simply don't offer it at all if core I2C support is not built in. Reported-by: Jim Davis <jim.epost@gmail.com> Reported-by: Fengguang Wu <fengguang.wu@intel.com> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig index dade5b7699bc..aefd1b9a3cbd 100644 --- a/drivers/video/Kconfig +++ b/drivers/video/Kconfig @@ -2338,7 +2338,7 @@ endchoice config FB_MB862XX_I2C bool "Support I2C bus on MB862XX GDC" - depends on FB_MB862XX && I2C + depends on FB_MB862XX && I2C=y default y help Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter -- 1.9.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure 2014-03-20 15:16 [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure Paul Gortmaker @ 2014-03-21 13:32 ` Tomi Valkeinen 2014-03-21 13:53 ` Paul Gortmaker 0 siblings, 1 reply; 5+ messages in thread From: Tomi Valkeinen @ 2014-03-21 13:32 UTC (permalink / raw) To: Paul Gortmaker Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel, Jim Davis, Fengguang Wu [-- Attachment #1: Type: text/plain, Size: 1592 bytes --] Hi, On 20/03/14 17:16, Paul Gortmaker wrote: > Any randconfig that sets I2C=m and FB_MB862XX_I2C=y will > encounter a final link failure that looks like this: It compiles fine with I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y. > drivers/built-in.o: In function `mb862xx_i2c_init': > drivers/video/mb862xx/mb862xx-i2c.c:165: undefined reference to `i2c_add_adapter' > drivers/built-in.o: In function `mb862xx_i2c_exit': > drivers/video/mb862xx/mb862xx-i2c.c:176: undefined reference to `i2c_del_adapter' > > Since FB_MB862XX_I2C is a bool and not tristate, simply > don't offer it at all if core I2C support is not built in. FB_MB862XX_I2C is not a driver, it just adds the i2c support to FB_MB862XX. The relevant thing is whether FB_MB862XX is m or y, so compiling with: I2C=m, FB_MB862XX=y and FB_MB862XX_I2C=y will fail. > Reported-by: Jim Davis <jim.epost@gmail.com> > Reported-by: Fengguang Wu <fengguang.wu@intel.com> > Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> > > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig > index dade5b7699bc..aefd1b9a3cbd 100644 > --- a/drivers/video/Kconfig > +++ b/drivers/video/Kconfig > @@ -2338,7 +2338,7 @@ endchoice > > config FB_MB862XX_I2C > bool "Support I2C bus on MB862XX GDC" > - depends on FB_MB862XX && I2C > + depends on FB_MB862XX && I2C=y > default y > help > Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter This fix is not correct, as it prevents the following, valid, config: I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y Tomi [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 901 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure 2014-03-21 13:32 ` Tomi Valkeinen @ 2014-03-21 13:53 ` Paul Gortmaker 2014-03-21 14:07 ` Tomi Valkeinen 2014-03-21 23:11 ` Randy Dunlap 0 siblings, 2 replies; 5+ messages in thread From: Paul Gortmaker @ 2014-03-21 13:53 UTC (permalink / raw) To: Tomi Valkeinen Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel, Jim Davis, Fengguang Wu On 14-03-21 09:32 AM, Tomi Valkeinen wrote: > Hi, > > On 20/03/14 17:16, Paul Gortmaker wrote: >> Any randconfig that sets I2C=m and FB_MB862XX_I2C=y will >> encounter a final link failure that looks like this: > > It compiles fine with I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y. > >> drivers/built-in.o: In function `mb862xx_i2c_init': >> drivers/video/mb862xx/mb862xx-i2c.c:165: undefined reference to `i2c_add_adapter' >> drivers/built-in.o: In function `mb862xx_i2c_exit': >> drivers/video/mb862xx/mb862xx-i2c.c:176: undefined reference to `i2c_del_adapter' >> >> Since FB_MB862XX_I2C is a bool and not tristate, simply >> don't offer it at all if core I2C support is not built in. > > FB_MB862XX_I2C is not a driver, it just adds the i2c support to > FB_MB862XX. The relevant thing is whether FB_MB862XX is m or y, so > compiling with: > > I2C=m, FB_MB862XX=y and FB_MB862XX_I2C=y > > will fail. How would you suggest we fix it then? Perhaps we could simplify the Kconfig space and just get rid of FB_MB862XX_I2C entirely? Is there ever a reason why someone would want it turned off when I2C is present? Paul. -- > >> Reported-by: Jim Davis <jim.epost@gmail.com> >> Reported-by: Fengguang Wu <fengguang.wu@intel.com> >> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> >> >> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig >> index dade5b7699bc..aefd1b9a3cbd 100644 >> --- a/drivers/video/Kconfig >> +++ b/drivers/video/Kconfig >> @@ -2338,7 +2338,7 @@ endchoice >> >> config FB_MB862XX_I2C >> bool "Support I2C bus on MB862XX GDC" >> - depends on FB_MB862XX && I2C >> + depends on FB_MB862XX && I2C=y >> default y >> help >> Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter > > This fix is not correct, as it prevents the following, valid, config: > > I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y > > Tomi > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure 2014-03-21 13:53 ` Paul Gortmaker @ 2014-03-21 14:07 ` Tomi Valkeinen 2014-03-21 23:11 ` Randy Dunlap 1 sibling, 0 replies; 5+ messages in thread From: Tomi Valkeinen @ 2014-03-21 14:07 UTC (permalink / raw) To: Paul Gortmaker Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel, Jim Davis, Fengguang Wu [-- Attachment #1: Type: text/plain, Size: 2241 bytes --] On 21/03/14 15:53, Paul Gortmaker wrote: > On 14-03-21 09:32 AM, Tomi Valkeinen wrote: >> Hi, >> >> On 20/03/14 17:16, Paul Gortmaker wrote: >>> Any randconfig that sets I2C=m and FB_MB862XX_I2C=y will >>> encounter a final link failure that looks like this: >> >> It compiles fine with I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y. >> >>> drivers/built-in.o: In function `mb862xx_i2c_init': >>> drivers/video/mb862xx/mb862xx-i2c.c:165: undefined reference to `i2c_add_adapter' >>> drivers/built-in.o: In function `mb862xx_i2c_exit': >>> drivers/video/mb862xx/mb862xx-i2c.c:176: undefined reference to `i2c_del_adapter' >>> >>> Since FB_MB862XX_I2C is a bool and not tristate, simply >>> don't offer it at all if core I2C support is not built in. >> >> FB_MB862XX_I2C is not a driver, it just adds the i2c support to >> FB_MB862XX. The relevant thing is whether FB_MB862XX is m or y, so >> compiling with: >> >> I2C=m, FB_MB862XX=y and FB_MB862XX_I2C=y >> >> will fail. > > How would you suggest we fix it then? Perhaps we could simplify the > Kconfig space and just get rid of FB_MB862XX_I2C entirely? Is there > ever a reason why someone would want it turned off when I2C is present? I'm not familiar with the driver and devices that use it, so I can't really say. But you could probably have a board with the FB_MB862XX, without i2c displays, while still you'd have I2C for other uses. So there you could minimally reduce the kernel size by leaving out the FB_MB862XX_I2C. I'm fine with that solution, though. But how would it work in practice? Did you mean that FB_MB862XX would depend on I2C? That's not a good option, but how would you otherwise make the i2c dependency correct? Actually, I'm fine with the original patch also, as I believe the case where I2C=m is somewhat theoretical (correct me if I'm wrong). But the patch description was totally wrong, and if that solution is preferred, it should also clearly state that the patch prevents the I2C support when I2C is built as a module. But I think your proposal in this mail is better. Maybe there are even better ways to handle it in the Kconfig, as such "add feature X to the driver" sounds quite common to me. Tomi [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 901 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure 2014-03-21 13:53 ` Paul Gortmaker 2014-03-21 14:07 ` Tomi Valkeinen @ 2014-03-21 23:11 ` Randy Dunlap 1 sibling, 0 replies; 5+ messages in thread From: Randy Dunlap @ 2014-03-21 23:11 UTC (permalink / raw) To: Paul Gortmaker, Tomi Valkeinen Cc: Jean-Christophe Plagniol-Villard, linux-fbdev, linux-kernel, Jim Davis, Fengguang Wu On 03/21/2014 06:53 AM, Paul Gortmaker wrote: > On 14-03-21 09:32 AM, Tomi Valkeinen wrote: >> Hi, >> >> On 20/03/14 17:16, Paul Gortmaker wrote: >>> Any randconfig that sets I2C=m and FB_MB862XX_I2C=y will >>> encounter a final link failure that looks like this: >> >> It compiles fine with I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y. >> >>> drivers/built-in.o: In function `mb862xx_i2c_init': >>> drivers/video/mb862xx/mb862xx-i2c.c:165: undefined reference to `i2c_add_adapter' >>> drivers/built-in.o: In function `mb862xx_i2c_exit': >>> drivers/video/mb862xx/mb862xx-i2c.c:176: undefined reference to `i2c_del_adapter' >>> >>> Since FB_MB862XX_I2C is a bool and not tristate, simply >>> don't offer it at all if core I2C support is not built in. >> >> FB_MB862XX_I2C is not a driver, it just adds the i2c support to >> FB_MB862XX. The relevant thing is whether FB_MB862XX is m or y, so >> compiling with: >> >> I2C=m, FB_MB862XX=y and FB_MB862XX_I2C=y >> >> will fail. > > How would you suggest we fix it then? Perhaps we could simplify the > Kconfig space and just get rid of FB_MB862XX_I2C entirely? Is there > ever a reason why someone would want it turned off when I2C is present? > > Paul. > -- > >> >>> Reported-by: Jim Davis <jim.epost@gmail.com> >>> Reported-by: Fengguang Wu <fengguang.wu@intel.com> >>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> >>> >>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig >>> index dade5b7699bc..aefd1b9a3cbd 100644 >>> --- a/drivers/video/Kconfig >>> +++ b/drivers/video/Kconfig >>> @@ -2338,7 +2338,7 @@ endchoice >>> >>> config FB_MB862XX_I2C >>> bool "Support I2C bus on MB862XX GDC" >>> - depends on FB_MB862XX && I2C >>> + depends on FB_MB862XX && I2C=y >>> default y >>> help >>> Selecting this option adds Coral-P(A)/Lime GDC I2C bus adapter >> >> This fix is not correct, as it prevents the following, valid, config: >> >> I2C=m, FB_MB862XX=m and FB_MB862XX_I2C=y If I am following this correctly, this kconfig situation is often handled by something like depends on I2C=y || I2Cû_MB862XX -- ~Randy ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-21 23:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-03-20 15:16 [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure Paul Gortmaker 2014-03-21 13:32 ` Tomi Valkeinen 2014-03-21 13:53 ` Paul Gortmaker 2014-03-21 14:07 ` Tomi Valkeinen 2014-03-21 23:11 ` Randy Dunlap
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).