On 06/10/2023 18:33, William McVicker wrote: > On 10/06/2023, Arnd Bergmann wrote: >> On Fri, Oct 6, 2023, at 08:06, Krzysztof Kozlowski wrote: >>> On 06/10/2023 01:19, William McVicker wrote: >>>> On 10/05/2023, Krzysztof Kozlowski wrote: >>>>> On 05/10/2023 21:23, Greg KH wrote: >>>> >>>> Being able to include SERIAL_SAMSUNG and SERIAL_MSM without all the vendor> specific drivers that ARCH_EXYNOS and ARCH_QCOM select is very >>> valuable for >>>> debugging early boot issues. >>> >>> Really? How related? The drivers are independent. You describe some >>> out-of-tree development process which we never needed for upstream work. >>> And we did here quite a lot of upstream, specially if you look at ARCH_QCOM. >> >> Right: in general, all drivers are independent of the platform >> besides the typical 'depends on ARCH_FOO || COMPILE_TEST' dependency, >> but I think it's worth mentioning the known exceptions, so Greg and >> Will can take that fight to the respective places rather than >> discussing it in the platform submission: >> >> - Some subsystems are considered 'special' and the maintainers >> prefer the drivers to be automatically selected based on the >> ARCH_* settings instead of having user-visible options. This is >> traditionally true for large chunks of drivers/irqchip, >> drivers/clocksource and drivers/pinctrl, though it has gotten >> better over time on all of them. >> >> - Some older 32-bit platforms are still not as modular as we'd >> like them to be, especially the StrongARM (ARMv4) platforms that >> require a custom kernel build, and some of ARMv4T and ARMv5 >> boards that are still missing DT support. These tend to require >> drivers they directly link to from board code, so disabling >> the drivers would cause a link failure until this gets >> cleaned up. >> >> - A couple of drivers are force-enabled based on the ARCH_* >> options because booting without these drivers would risk >> permanent damage to hardware, e.g. in overtemp or overcurrent >> scenarios. >> >> - ACPI based platforms require the PCI host bridge driver to >> be built-in rather than a loadable module because ACPI >> needs to probe PCI devices during early boot. >> >> - Some subsystems (notably drivers/gpu/, but others as well) >> have an excessive number of 'select' statements, so you >> end up surprise-enabling a number of additional drivers >> and subsystems by enabling certain less important platform >> specific drivers. >> >> Arnd > > So if the argument is that the existing upstream Exynos platforms are required > to have these drivers built-in to the kernel to boot: > COMMON_CLK_SAMSUNG > CLKSRC_EXYNOS_MCT > EXYNOS_PM_DOMAINS if PM_GENERIC_DOMAINS > EXYNOS_PMU > PINCTRL > PINCTRL_EXYNOS > PM_GENERIC_DOMAINS if PM > SOC_SAMSUNG > > ...then that is understandable and we can work to fix that. > > My last question then is -- why do we need a new ARCH_GOOGLE_TENSOR config in > the platform Kconfig? For example, I don't really like this: > > diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig > index 76a494e95027..4c8f173c4dec 100644 > --- a/drivers/clk/samsung/Kconfig > +++ b/drivers/clk/samsung/Kconfig > @@ -13,6 +13,7 @@ config COMMON_CLK_SAMSUNG > select EXYNOS_5420_COMMON_CLK if ARM && SOC_EXYNOS5420 > select EXYNOS_ARM64_COMMON_CLK if ARM64 && ARCH_EXYNOS > select TESLA_FSD_COMMON_CLK if ARM64 && ARCH_TESLA_FSD > + select GOOGLE_GS101_COMMON_CLK if ARM64 && ARCH_GOOGLE_TENSOR > > What happens when we have GOOGLE_GS101_COMMON_CLK, GOOGLE_GS201_COMMON_CLK, and > so on? Nothing happens... or happens anything you wish. Did you read the motivation why this was created like this? > How are we going to pick the right driver when e have a generic > ARCH_GOOGLE_TENSOR config? You do not have to pick. You select ARCH_GOOGLE_TENSOR and proper pick is done by you. Nothing to do more. > Ideally, we should have one Exynos clock driver that > can detect what hardware is running (using the DT) to determine what it needs It's already like this. We're done. > to do. If you really want to compile out the other vendor's clock drivers using > some configs, then we should do that with SOC_GS101, SOC_GS201, SOC_TESLA_FSD Whether you call it SOC or ARCH it is the same. We organized it as ARCH. > configs (not ideal though). With that approach, we could drop the platform > ARCH_GOOGLE_TENSOR config and create an SOC_GS101 config that can be used for > things like the COMMON_CLK_SAMSUNG driver (for now) and building the GS101 dtb. There is no need for this. ARCH does exactly the same. Best regards, Krzysztof
