On Thu, Apr 27, 2017 at 02:13:22PM -0700, Darren Hart wrote: > On Tue, Apr 25, 2017 at 12:19:39PM +0200, Jean Delvare wrote: > > On Mon, 24 Apr 2017 14:48:16 +0300, Andy Shevchenko wrote: > > > On Mon, 2017-04-24 at 13:23 +0200, Jean Delvare wrote: > > > > I'm looking at drivers/platform/x86/silead_dmi.c which is being added > > > > to kernel v4.11 and I do not like what I see. > > > > > > I don't like it either by some other reasons. > > > > > > > I have to say I don't understand the whole complexity of the design. > > > > As I understand it, the properties which are being added are only > > > > consumed by the "silead" touchscreen driver. I see no necessity to add > > > > the missing properties before that driver is even loaded. Can't you > > > > just look for the ACPI companion device at the time the silead driver > > > > tries to bind to the i2c device, and add the missing properties before > > > > performing the actual probe? This would be so much simpler. What am I > > > > missing? > > > > > > As far as I understand it would be as simple as adding a quirk in actual > > > driver (touchscreen), but there is strong objection of adding quirks to > > > the drivers/input/* from Dmitry as I noticed during discussion [1] about > > > GPIO ACPI library fixes I'm working on. > > > > > > [1] https://lkml.org/lkml/2017/4/4/593 > > > > Thanks for the pointer Andy. OK, I can understand the argument of > > Dmitry that platform-specific quirks do not belong to the device > > drivers, even though in practice I'm not sure the cost of having a > > separate platform driver for the purpose is always worth it - depends > > on how "popular" the device is, I suppose. > > > > But still, this can't justify non-modular platform code that will run > > on every X86 system out there. Any piece of platform-specific quirks, > > we should be able to build as a module, otherwise it simply doesn't > > scale. PCI quirks and such are enough pain already without inventing > > more flavors of bloat :-( > > > > Jean makes a good point. I would insist on this if the Kconfig default was y or > if distros were likely to enable this by default. However, this is for the still > very rare x86 tablet and is likely to only be enabled for those systems which > require it. > > To emphasize its role, perhaps this driver should have a depends on TOUCHSCREEN_SILEAD? Yeah, that makes sense to me. > > While the default for Kconfig is n if not explicitly set to m or y, perhaps > "default n" should be added to TOUCHSCREEN_SILEAD and SILEAD_DMI to make it more > explicit. I don't know if there is an official preference on "default n" > statements or not. > Thanks. -- Dmitry
