On Tue, Sep 21, 2021 at 1:35 AM Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxxxxx> wrote: > > On 21/09/2021 09:50, Geert Uytterhoeven wrote: > > On Tue, Sep 21, 2021 at 9:31 AM Krzysztof Kozlowski <krzk@xxxxxxxxxx> wrote: > >> On 20/09/2021 21:03, Will McVicker wrote: > >>> COMMON_CLK_SAMSUNG is selected by ARCH_EXYNOS which forces this config > >>> to be built-in when ARCH_EXYNOS is enabled. Switch the logic to use a > >>> "default y if ARCH_EXYNOS" to provide flexibilty for vendors to disable > >>> or modularize this driver. > >> > >> The clock drivers are essential, you cannot disable them for a generic > >> kernel supporting ARCH_EXYNOS. Such kernel won't work properly on platforms. > > > > Obviously it's not gonna work if the clock driver is not enabled > > at all. But does it work if you make the clock driver modular, and > > put it with all other essential driver modules in initramfs? Debugging > > would be hard, as the serial console driver also relies on clocks > > and PM Domains etc. > > The kernel could boot without clock drivers (default settings from > bootloader), probe clocks from initramfs and proceed with rootfs from > eMMC/SD/net. > > In theory. > > However I have no reports that it ever worked. If there is such working > upstream configuration, I don't mind here. Just please explain this in > the commit msg. > > > > > If not, this patch should be NAKed, until it works with a modular > > clock driver. > > > > If yes, perhaps another line should be added (_before_ the other line)? > > > > + default m if ARCH_EXYNOS && MODULES > > default y if ARCH_EXYNOS > > > > However, many developers may want MODULES=y, but not want to bother > > with an initramfs. So perhaps we need a new symbol > > MINIMUM_GENERIC_KERNEL or so, protected by EXPERT, and make the > > driver default to m if that is enabled? > > Yeah, that's indeed a problem to solve. For most users (and distros) > building kernel for Exynos this should be built-in by default. > > Anyway, the option is non-selectable so it cannot be converted to "m" or > disabled. And this is claimed in the commit msg: > "provide flexibilty for vendors to disable or modularize this driver." > > The commit does not achieve it. > > Best regards, > Krzysztof Thanks for the reviews! As Lee has explained in his replies, the intent of this series is to provide config flexibility to create a defconfig that allows us to move out SoC specific drivers in order to create a generic kernel that can be used across multiple devices with different SoCs. I'm sorry I added confusion by mentioning modularization. All of these drivers that I am modifying in this series can be modularized which is an ongoing effort, but is not addressed here and I don't believe that modularizing them should be a requirement before supporting enabling/disabling them. I will update the series with my patch that refactors the Samsung SoC drivers menuconfig to make these visible as well. Thanks, Will
