mbox series

[v6,0/4] initial clock support for exynosauto v920 SoC

Message ID 20240819052416.2258976-1-sunyeal.hong@samsung.com
Headers show
Series initial clock support for exynosauto v920 SoC | expand

Message

Sunyeal Hong Aug. 19, 2024, 5:24 a.m. UTC 
This patchset adds initial clock driver support for Exynos Auto v920 SoC.
This driver uses HW Auto Clock gating. So all gate clocks did not register.

Below CMU blocks are supported in this patchset and remains will be
implemented later.

- CMU_TOP
- CMU_PERIC0

Changes in v6:
 - Add peric1, mis and hsi0/1 in the bindings document

Changes in v5:
 - Change CMU_TOP odd numbering
 - Move the descriptions and names common clocks properties

Changes in v4:
 - Change PLL_531x fdiv type and mask bit
 - Change PLL_531x mdiv type

Changes in v3:
 - Change SoC name from Exynos Auto to ExynosAuto
 - Change the makefile order to the bottom of exynosautov9
 - Add PLL_531x formula for integer PLL

Changes in v2:
 - Fix typo from v209 to v920
 - Change USI clock to appropriate
 - Merge headers into binding patches
 - Change clock-name to the recommended name

Sunyeal Hong (4):
  dt-bindings: clock: add ExynosAuto v920 SoC CMU bindings
  arm64: dts: exynos: add initial CMU clock nodes in ExynosAuto v920
  clk: samsung: clk-pll: Add support for pll_531x
  clk: samsung: add top clock support for ExynosAuto v920 SoC

 .../clock/samsung,exynosautov920-clock.yaml   |  197 +++
 .../arm64/boot/dts/exynos/exynosautov920.dtsi |   40 +-
 drivers/clk/samsung/Makefile                  |    1 +
 drivers/clk/samsung/clk-exynosautov920.c      | 1173 +++++++++++++++++
 drivers/clk/samsung/clk-pll.c                 |   44 +
 drivers/clk/samsung/clk-pll.h                 |    1 +
 .../clock/samsung,exynosautov920.h            |  191 +++
 7 files changed, 1634 insertions(+), 13 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/samsung,exynosautov920-clock.yaml
 create mode 100644 drivers/clk/samsung/clk-exynosautov920.c
 create mode 100644 include/dt-bindings/clock/samsung,exynosautov920.h

Comments

Kwanghoon Son Aug. 19, 2024, 9:32 a.m. UTC | #1 
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.
Sunyeal Hong Aug. 20, 2024, 1:50 a.m. UTC | #2 
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.

Best Regards,
sunyeal
Kwanghoon Son Aug. 20, 2024, 9:54 a.m. UTC | #3 
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.

But if you tested your patch and working fine, ignore my comment.

Kwang.

> 
> Best Regards,
> sunyeal
>
Sunyeal Hong Aug. 21, 2024, 2:23 a.m. UTC | #4 
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.

Best Regards,
Sunyeal

> But if you tested your patch and working fine, ignore my comment.
> 
> Kwang.
> 
> >
> > Best Regards,
> > sunyeal
> >
Krzysztof Kozlowski Aug. 21, 2024, 6:22 a.m. UTC | #5 
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
Sunyeal Hong Aug. 21, 2024, 7:06 a.m. UTC | #6 
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
Krzysztof Kozlowski Aug. 21, 2024, 8:02 a.m. UTC | #7 
On 21/08/2024 09:06, sunyeal.hong wrote:
>>>>>>> +	.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.

No. The question was about clk_name entry in cmu info used directly for
clk_get.


Best regards,
Krzysztof
Sunyeal Hong Aug. 21, 2024, 9:30 a.m. UTC | #8 
Hello Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, August 21, 2024 5:02 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 09:06, sunyeal.hong wrote:
> >>>>>>> +	.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.
> 
> No. The question was about clk_name entry in cmu info used directly for
> clk_get.
> 

I have verified that peric0 has a dev parameter and that it should use a clk_name that matches the clock-names declared in the device tree. I will update the patch with a fix.

Best Regards,
sunyeal

> 
> Best regards,
> Krzysztof
>