Hi Andy, On Fri, 16 Dec 2016 16:02:27 +0200, Andy Shevchenko wrote: > On Fri, 2016-12-16 at 14:28 +0100, Jean Delvare wrote: > > On Fri, 16 Dec 2016 13:31:19 +0200, Andy Shevchenko wrote: > > > On Fri, 2016-12-16 at 11:50 +0100, Jean Delvare wrote: > > > > Hiding tristate options with "if EXPERT" is usually not a good > > > > idea. You can decide that the driver should be included by > > > > default, but you don't know if the user wants it built-in or as > > > > a module. Hiding the option prevents the user from making that > > > > decision. > > > > > > Then please start from 8250_PCI. > > > > 8250_PCI is indeed a candidate for a similar change. However it is > > less specific and also doesn't select other drivers, so fixing it > > had lower priority for me. I can look into it later. > > Okay, just to make sure you know that drivers we are talking about used > to be a part of 8250_pci I wasn't aware of that, I'll look into it, thanks. > > > (...) > > > That's why I would like to keep this for EXPERTs. > > > > > > CONFIG_EXPERT=y > > > CONFIG_SERIAL_8250_LPSS=m > > > > > > will not work? > > > > It would "work" in that it would allow us to modularize the drivers. > > However enabling CONFIG_EXPERT would make more than 100 extra kernel > > options visible, making our kernel configuration work even more > > tedious and error-prone. > > It worked for now, no one complained about having 8250_pci compiled in > until now. Maybe because 8250_pci doesn't draw a bunch of dependencies as 8250_lpss and 8250_mid do, and also because it looks less hardware-specific. Nevertheless, 8250_pci isn't small (53 kiB on my x86_64 build) and most users don't actually need that driver (think of laptops, although it seems many of them have a serial port device enabled despite not offering any physical connector to it, *sigh*), so let me be the first one to complain about it being build into the kernel. I'll send a patch. > > > (...) > > > No. You forget Intel Quark. It has nothing to do with > > > X86_INTEL_LPSS. > > > > OK, it wasn't obvious to me. No big deal, we can omit this part of the > > patch, it was only an "optimization" which I can easily live without. > > Yeah, please, remove those parts from your patches and re-submit. For > "if EXPERT" part I'm not the one who is supporting it (I remember > shouting Linus when we had enabled two HSU_DMA options instead of one). > I leave this to more experienced developers / maintainers. I reverted these changes and updated the patch descriptions. I'll resubmit in a minute, thanks for the review. -- Jean Delvare SUSE L3 Support -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html