Re: [PATCH] drivers/video: fix mb862xx_i2c depends issue build failure

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

[-- 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 --]