Hi Andy, Thanks for the review. 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. > > This is even more problematic when said option selects other options. > > You end up with several device drivers forcibly built into the kernel. > > > > In this specific case, drivers 8250_lpss, dw_dmac_core and > > dw_dmac_pci end up being built-in as soon as SERIAL_8250=y. It is > > very common for distribution kernels to build the subsystem core code > > into the kernel, because almost everybody will need it, but build all > > the drivers as modules. This should be made possible. > > ...and make new problems occur. We have no driver dependency between > them, so, depending of load ordering the user will or will not have DMA > resource available. OK, I see. How is the problem solved when all drivers are built-in? When modular, what is the order in which 8250_lpss, dw_dmac_core and dw_dmac_pci (and maybe others?) should be loaded? Users setting CONFIG_EXPERT=y can already build these drivers as modules. It's even the default if CONFIG_SERIAL_8250=m, even when CONFIG_EXPERT isn't enabled. So the problem you mention already exists today, it has little to do with my proposed patch. I think there are ways to solve that kind inter-module "soft" dependencies. Either in kernel-space, using try_then_request_module(), or in user-space, with modprobe install statements. This should be done. > 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. If even general-purpose distribution kernels need to enable CONFIG_EXPERT to get a sane configuration, it pretty much voids the point of this option. So I do not consider this possibility as satisfactory, sorry. > > > > So drop the "if EXPERT" and make SERIAL_8250_LPSS visible. The right > > way to avoid annoying people who couldn't care less about this option > > is to make it depend on X86_INTEL_LPSS. There may be no technical > > dependency between these two options, but in practice, either people > > want support for a platform, or they don't. > > 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. Thanks, -- 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
