| Message ID | 1395328560-48497-1-git-send-email-paul.gortmaker@windriver.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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
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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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=FB_MB862XX
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
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>