Re: [PATCH v3 10/20] clk: samsung: clk-gs101: Add cmu_top registers, plls, mux and gates

[Peter Griffin <peter.griffin@xxxxxxxxxx>]

Hi Krzysztof,

On Thu, 12 Oct 2023 at 13:24, Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 12/10/2023 14:06, Peter Griffin wrote:
> > Hi Sam,
> >
> > Thanks for the review.
> >
> > On Thu, 12 Oct 2023 at 01:07, Sam Protsenko <semen.protsenko@xxxxxxxxxx> wrote:
> >>
> >> On Wed, Oct 11, 2023 at 1:49 PM Peter Griffin <peter.griffin@xxxxxxxxxx> wrote:
> >>>
> >>> CMU_TOP is the top level clock management unit which contains PLLs, muxes
> >>> and gates that feed the other clock management units.
> >>>
> >>> Signed-off-by: Peter Griffin <peter.griffin@xxxxxxxxxx>
> >>> ---
> >>>  drivers/clk/samsung/Kconfig     |    9 +
> >>>  drivers/clk/samsung/Makefile    |    2 +
> >>>  drivers/clk/samsung/clk-gs101.c | 1551 +++++++++++++++++++++++++++++++
> >>>  3 files changed, 1562 insertions(+)
> >>>  create mode 100644 drivers/clk/samsung/clk-gs101.c
> >>>
> >>> diff --git a/drivers/clk/samsung/Kconfig b/drivers/clk/samsung/Kconfig
> >>> index 76a494e95027..14362ec9c543 100644
> >>> --- a/drivers/clk/samsung/Kconfig
> >>> +++ b/drivers/clk/samsung/Kconfig
> >>> @@ -12,6 +12,7 @@ config COMMON_CLK_SAMSUNG
> >>>         select EXYNOS_5410_COMMON_CLK if ARM && SOC_EXYNOS5410
> >>>         select EXYNOS_5420_COMMON_CLK if ARM && SOC_EXYNOS5420
> >>>         select EXYNOS_ARM64_COMMON_CLK if ARM64 && ARCH_EXYNOS
> >>> +       select GOOGLE_GS101_COMMON_CLK if ARM64 && ARCH_GOOGLE_TENSOR
> >>>         select TESLA_FSD_COMMON_CLK if ARM64 && ARCH_TESLA_FSD
> >>>
> >>>  config S3C64XX_COMMON_CLK
> >>> @@ -95,6 +96,14 @@ config EXYNOS_CLKOUT
> >>>           status of the certains clocks from SoC, but it could also be tied to
> >>>           other devices as an input clock.
> >>>
> >>> +config GOOGLE_GS101_COMMON_CLK
> >>> +       bool "Google gs101 clock controller support" if COMPILE_TEST
> >>> +       depends on COMMON_CLK_SAMSUNG
> >>> +       depends on EXYNOS_ARM64_COMMON_CLK
> >>> +       help
> >>> +         Support for the clock controller present on the Google gs101 SoC.
> >>> +         Choose Y here only if you build for this SoC.
> >>> +
> >>
> >> Why is that new option needed? From the look of it, it could be just a
> >> part of EXYNOS_ARM64_COMMON_CLK. Like clk-exynos850 or
> >> clk-exynosautov9. Is there any particular feature that makes it SoC
> >> special?
> >
> > No, it could also be added to EXYNOS_ARM64_COMMON_CLK. I was following
> > the example set by TESLA_FSD which is another custom Exynos based chipset
> > that added its own config option.
> >
> > Krzysztof do you have any preference on this?
>
> Usually there is only one image for several boards so long time ago we
> stopped adding per-SoC Kconfig entries. This has its own ARCH_xxx, just
> like Tesla, thus having separate Kconfig for all Google Tensor clock
> drivers makes sense. Maybe it should be just called a bit differently,
> e.g. GOOGLE_TENSOR_COMMON_CLK

Thanks, I will update the name in v4.

Peter.