RE: [PATCH v6 4/4] clk: samsung: add top clock support for ExynosAuto v920 SoC

["sunyeal.hong" <sunyeal.hong@samsung.com>]

Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, August 21, 2024 3:23 PM
> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Kwanghoon Son'
> <k.son@samsung.com>; 'Sylwester Nawrocki' <s.nawrocki@samsung.com>;
> 'Chanwoo Choi' <cw00.choi@samsung.com>; 'Alim Akhtar'
> <alim.akhtar@samsung.com>; 'Michael Turquette' <mturquette@baylibre.com>;
> 'Stephen Boyd' <sboyd@kernel.org>; 'Rob Herring' <robh@kernel.org>; 'Conor
> Dooley' <conor+dt@kernel.org>
> Cc: linux-samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v6 4/4] clk: samsung: add top clock support for
> ExynosAuto v920 SoC
> 
> On 21/08/2024 04:23, sunyeal.hong wrote:
> > Hello Kwanghoon,
> >
> >> -----Original Message-----
> >> From: Kwanghoon Son <k.son@samsung.com>
> >> Sent: Tuesday, August 20, 2024 6:54 PM
> >> To: sunyeal.hong <sunyeal.hong@samsung.com>; 'Krzysztof Kozlowski'
> >> <krzk@kernel.org>; 'Sylwester Nawrocki' <s.nawrocki@samsung.com>;
> >> 'Chanwoo Choi' <cw00.choi@samsung.com>; 'Alim Akhtar'
> >> <alim.akhtar@samsung.com>; 'Michael Turquette'
> <mturquette@baylibre.com>; 'Stephen Boyd'
> >> <sboyd@kernel.org>; 'Rob Herring' <robh@kernel.org>; 'Conor Dooley'
> >> <conor+dt@kernel.org>
> >> Cc: linux-samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org;
> >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux- kernel@vger.kernel.org
> >> Subject: Re: [PATCH v6 4/4] clk: samsung: add top clock support for
> >> ExynosAuto v920 SoC
> >>
> >> On Tue, 2024-08-20 at 10:50 +0900, sunyeal.hong wrote:
> >>> Hello Kwanghoon,
> >>>
> >>>> -----Original Message-----
> >>>> From: Kwanghoon Son <k.son@samsung.com>
> >>>> Sent: Monday, August 19, 2024 6:32 PM
> >>>> To: Sunyeal Hong <sunyeal.hong@samsung.com>; Krzysztof Kozlowski
> >>>> <krzk@kernel.org>; Sylwester Nawrocki <s.nawrocki@samsung.com>;
> >>>> Chanwoo Choi <cw00.choi@samsung.com>; Alim Akhtar
> >>>> <alim.akhtar@samsung.com>; Michael Turquette
> >>>> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Rob
> >>>> Herring <robh@kernel.org>; Conor Dooley <conor+dt@kernel.org>
> >>>> Cc: linux-samsung-soc@vger.kernel.org; linux-clk@vger.kernel.org;
> >>>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >>>> linux- kernel@vger.kernel.org
> >>>> Subject: Re: [PATCH v6 4/4] clk: samsung: add top clock support for
> >>>> ExynosAuto v920 SoC
> >>>>
> >>>> On Mon, 2024-08-19 at 14:24 +0900, Sunyeal Hong wrote:
> >>>>> This adds support for CMU_TOP which generates clocks for all the
> >>>>> function blocks such as CORE, HSI0/1/2, PERIC0/1 and so on. For
> >>>>> CMU_TOP, PLL_SHARED0,1,2,3,4 and 5 will be the sources of this
> >>>>> block and they will generate bus clocks.
> >>>>>
> >>>>> Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com>
> >>>>> ---
> >>>>>  drivers/clk/samsung/Makefile             |    1 +
> >>>>>  drivers/clk/samsung/clk-exynosautov920.c | 1173
> >>>>> ++++++++++++++++++++++
> >>>>>  2 files changed, 1174 insertions(+)  create mode 100644
> >>>>> drivers/clk/samsung/clk-exynosautov920.c
> >>>>>
> >>>>> diff --git a/drivers/clk/samsung/Makefile
> >>>>> b/drivers/clk/samsung/Makefile index 3056944a5a54..f1ba48758c78
> >>>>> 100644
> >>>>> --- a/drivers/clk/samsung/Makefile
> >>>>> +++ b/drivers/clk/samsung/Makefile
> >>>>> @@ -21,6 +21,7 @@ obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-
> >>>> exynos7.o
> >>>>>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos7885.o
> >>>>>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynos850.o
> >>>>>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynosautov9.o
> >>>>> +obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-exynosautov920.o
> >>>>>  obj-$(CONFIG_EXYNOS_ARM64_COMMON_CLK)	+= clk-gs101.o
> >>>>>  obj-$(CONFIG_S3C64XX_COMMON_CLK)	+= clk-s3c64xx.o
> >>>>>  obj-$(CONFIG_S5PV210_COMMON_CLK)	+= clk-s5pv210.o clk-s5pv210-
> >>>> audss.o
> >>>>> diff --git a/drivers/clk/samsung/clk-exynosautov920.c
> >>>>> b/drivers/clk/samsung/clk-exynosautov920.c
> >>>>> new file mode 100644
> >>>>> index 000000000000..c17d25e3c9a0
> >>>>> --- /dev/null
> >>>>> +++ b/drivers/clk/samsung/clk-exynosautov920.c
> >>>>
> >>>> [snip]
> >>>>
> >>>>> +};
> >>>>> +
> >>>>> +static const struct samsung_cmu_info peric0_cmu_info __initconst
> >>>>> +=
> >> {
> >>>>> +	.mux_clks		= peric0_mux_clks,
> >>>>> +	.nr_mux_clks		= ARRAY_SIZE(peric0_mux_clks),
> >>>>> +	.div_clks		= peric0_div_clks,
> >>>>> +	.nr_div_clks		= ARRAY_SIZE(peric0_div_clks),
> >>>>> +	.nr_clk_ids		= CLKS_NR_PERIC0,
> >>>>> +	.clk_regs		= peric0_clk_regs,
> >>>>> +	.nr_clk_regs		= ARRAY_SIZE(peric0_clk_regs),
> >>>>> +	.clk_name		= "dout_clkcmu_peric0_noc",
> >>>>
> >>>> same question.
> >>>> Isn't it "noc"?
> >>>> https://lore.kernel.org/linux-samsung-
> >>>> soc/58dfae564a4a624e464c7803a309f1f07b5ae83d.camel@samsung.com/
> >>>>
> >>>> In my case(autov9), if put wrong clk_name dmesg will show that,
> >>>> exynos_arm64_register_cmu: could not enable bus clock ...; err = -2
> >>>>
> >>>> Kwang.
> >>>>
> >>>>
> >>>
> >>> clk_name follows the guide document provided by hw. v9 is bus, but
> >>> v920
> >> uses noc.
> >>
> >> What I mean,
> >>
> >> .clk_name		= "dout_clkcmu_peric0_noc", // wrong
> >> .clk_name		= "noc", // correct
> >>
> >> Because there is no clock-names "dout_clkcmu_peric0_noc" in
> >> exynos/exynosautov920.dtsi.
> >>
> >
> > The clk_name written here has nothing to do with the device tree. Please
> look at the code carefully.
> 
> Hm? I see in the code clearly:
> 
> 	clk_get(dev, cmu->clk_name);
> 
> Where cmu is the discussed struct.
> 
> If you claim it does not have anything to do with DT, then what is it for?
> 
> Best regards,
> Krzysztof

In general, clk_get is used via the clk_name declared in the DT.

However, the question asked here is the parent clock name of peric0_noc, so it is unrelated to the device tree.

	PNAME(mout_peric0_noc_user_p) = { "oscclk", "dout_clkcmu_peric0_noc" };

So we are using clk_name as "dout_clkcmu_peric0_noc".

Best Regards,
sunyeal