Re: [PATCH v2] lcd: Provide dummy functions if CONFIG_LCD_CLASS_DEVICE is not set

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

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

On 20/03/14 08:41, Alexander Shiyan wrote:
> Hello.
> 
> Wed, 19 Mar 2014 15:31:49 +0200 от Tomi Valkeinen <tomi.valkeinen@ti.com>:
>> Hi,
>>
>> On 14/03/14 13:09, Alexander Shiyan wrote:
>>> Provide dummy functions for LCD register()/unregister() if
>>> CONFIG_LCD_CLASS_DEVICE is not set.
>>> This allows us to use the LCD class as an optional.
>>>
>>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
>>> ---
>>>  include/linux/lcd.h | 37 ++++++++++++++++++++++++++++++-------
>>>  1 file changed, 30 insertions(+), 7 deletions(-)
>>
>> I'm still not convinced about this. Isn't it simpler to just
>> select/depend on LCD_CLASS_DEVICE? The lcd.c is a bit less than 9 kB, so
>> it's not like you're adding a huge amount of code into the kernel even
>> if the driver doesn't happen to use it for a particular display.
> 
> Ie we prefer to increase the size of the kernel, even for functions which are optional?

How often can LCD_CLASS_DEVICE be turned off when using imxfb? How often
will it be turned off in practice? I fear this is a micro optimization
for cases that never happen in real life.

I'm fine with micro optimizing the kernel when it comes for free. In
this case I see drawback: usually (correct me if I'm wrong) if a fb
driver uses LCD_CLASS_DEVICE funcs, it requires it. Currently it's easy
to catch missing LCD_CLASS_DEVICE as the kernel won't compile. This
patch hides it until you try to run the driver. And compile time
checking is much, much better than runtime checking.

We have components that use the method you use in this patch, like
gpio.h. However, the difference with gpio and this one is that the
selection of gpio comes from the platform code, while the
LCD_CLASS_DEVICE is selected by the fb driver in question. So the fb
driver can easily select LCD_CLASS_DEVICE, while it cannot select GPIO.
My point here is that for gpios we more or less need such functionality,
whereas for LCD_CLASS_DEVICE we don't.

I'm not strongly against this patch. But, as I said, I see a small
drawback with it, so I'd like to either hear "yes, this is good" from
other people, or see information showing that this optimization will
actually help in lots of cases in real life.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]