On 20/03/14 08:41, Alexander Shiyan wrote: > Hello. > > Wed, 19 Mar 2014 15:31:49 +0200 от Tomi Valkeinen <tomi.valkeinen@xxxxxx>: >> 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@xxxxxxx> >>> --- >>> 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:
signature.asc
Description: OpenPGP digital signature
