| Message ID | 20240722223333.1137947-3-sunyeal.hong@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/4] dt-bindings: clock: add ExynosAuto v920 SoC CMU bindings | expand |
On 23/07/2024 00:33, Sunyeal Hong wrote: > Add cmu_top, cmu_peric0 clock nodes and > switch USI clocks instead of dummy fixed-rate-clock. > > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com> Thank you for your patch. There is something to discuss/improve. > cpus: cpus { > #address-cells = <2>; > #size-cells = <0>; > @@ -182,6 +172,28 @@ chipid@10000000 { > reg = <0x10000000 0x24>; > }; > > + cmu_peric0: clock-controller@10800000 { > + compatible = "samsung,exynosautov920-cmu-peric0"; > + reg = <0x10800000 0x8000>; > + #clock-cells = <1>; > + > + clocks = <&xtcxo>, > + <&cmu_top DOUT_CLKCMU_PERIC0_NOC>, > + <&cmu_top DOUT_CLKCMU_PERIC0_IP>; > + clock-names = "oscclk", > + "noc", > + "ip"; > + }; > + > + cmu_top: clock-controller@11000000 { This does not look ordered. Please read DTS coding style. Best regards, Krzysztof
Hi, Sunyeal, I quickly skimmed over the series and I fail to see where/how the HW auto clock gating is enabled/configured. Would you please add more details on how this works? On 7/22/24 11:33 PM, Sunyeal Hong wrote: > Add cmu_top, cmu_peric0 clock nodes and > switch USI clocks instead of dummy fixed-rate-clock. > > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com> > --- > .../arm64/boot/dts/exynos/exynosautov920.dtsi | 40 +++++++++++++------ > 1 file changed, 27 insertions(+), 13 deletions(-) > > diff --git a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi > index c1c8566d74f5..54fc32074379 100644 > --- a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi > +++ b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi cut > @@ -224,7 +237,8 @@ serial_0: serial@10880000 { > interrupts = <GIC_SPI 764 IRQ_TYPE_LEVEL_HIGH>; > pinctrl-names = "default"; > pinctrl-0 = <&uart0_bus>; > - clocks = <&clock_usi>, <&clock_usi>; > + clocks = <&cmu_peric0 CLK_MOUT_PERIC0_NOC_USER>, isn't this MUX common to multiple GATEs? Wouldn't turning it off affect other users than the serial? Thanks, ta > + <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>; > clock-names = "uart", "clk_uart_baud0"; > samsung,uart-fifosize = <256>; > status = "disabled";
Hello Krzysztof, > -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Wednesday, July 24, 2024 7:14 PM > To: Sunyeal Hong <sunyeal.hong@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 v4 2/4] arm64: dts: exynos: add initial CMU clock > nodes in ExynosAuto v920 > > On 23/07/2024 00:33, Sunyeal Hong wrote: > > Add cmu_top, cmu_peric0 clock nodes and switch USI clocks instead of > > dummy fixed-rate-clock. > > > > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com> > > Thank you for your patch. There is something to discuss/improve. > > > cpus: cpus { > > #address-cells = <2>; > > #size-cells = <0>; > > @@ -182,6 +172,28 @@ chipid@10000000 { > > reg = <0x10000000 0x24>; > > }; > > > > + cmu_peric0: clock-controller@10800000 { > > + compatible = "samsung,exynosautov920-cmu-peric0"; > > + reg = <0x10800000 0x8000>; > > + #clock-cells = <1>; > > + > > + clocks = <&xtcxo>, > > + <&cmu_top DOUT_CLKCMU_PERIC0_NOC>, > > + <&cmu_top DOUT_CLKCMU_PERIC0_IP>; > > + clock-names = "oscclk", > > + "noc", > > + "ip"; > > + }; > > + > > + cmu_top: clock-controller@11000000 { > > This does not look ordered. Please read DTS coding style. > I will modify it by referring to the Order of Nodes item in the dts coding style guide document. > Best regards, > Krzysztof Thanks, Sunyeal Hong.
Hello Tudor, > -----Original Message----- > From: Tudor Ambarus <tudor.ambarus@linaro.org> > Sent: Wednesday, July 24, 2024 8:18 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 v4 2/4] arm64: dts: exynos: add initial CMU clock > nodes in ExynosAuto v920 > > Hi, Sunyeal, > > I quickly skimmed over the series and I fail to see where/how the HW auto > clock gating is enabled/configured. Would you please add more details on > how this works? > The HW auto clock gating function is activated in the bootloader stage. So we don't have to control it in the kernel. > On 7/22/24 11:33 PM, Sunyeal Hong wrote: > > Add cmu_top, cmu_peric0 clock nodes and switch USI clocks instead of > > dummy fixed-rate-clock. > > > > Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com> > > --- > > .../arm64/boot/dts/exynos/exynosautov920.dtsi | 40 > > +++++++++++++------ > > 1 file changed, 27 insertions(+), 13 deletions(-) > > > > diff --git a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi > > b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi > > index c1c8566d74f5..54fc32074379 100644 > > --- a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi > > +++ b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi > > > cut > > > @@ -224,7 +237,8 @@ serial_0: serial@10880000 { > > interrupts = <GIC_SPI 764 IRQ_TYPE_LEVEL_HIGH>; > > pinctrl-names = "default"; > > pinctrl-0 = <&uart0_bus>; > > - clocks = <&clock_usi>, <&clock_usi>; > > + clocks = <&cmu_peric0 CLK_MOUT_PERIC0_NOC_USER>, > > isn't this MUX common to multiple GATEs? Wouldn't turning it off affect > other users than the serial? > > Thanks, > ta > I don’t think there will be any problems you are worried about through the enable count of CCF. Thanks, Sunyeal Hong. > > + <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>; > > clock-names = "uart", "clk_uart_baud0"; > > samsung,uart-fifosize = <256>; > > status = "disabled";
diff --git a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi index c1c8566d74f5..54fc32074379 100644 --- a/arch/arm64/boot/dts/exynos/exynosautov920.dtsi +++ b/arch/arm64/boot/dts/exynos/exynosautov920.dtsi @@ -6,6 +6,7 @@ * */ +#include <dt-bindings/clock/samsung,exynosautov920.h> #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/soc/samsung,exynos-usi.h> @@ -38,17 +39,6 @@ xtcxo: clock { clock-output-names = "oscclk"; }; - /* - * FIXME: Keep the stub clock for serial driver, until proper clock - * driver is implemented. - */ - clock_usi: clock-usi { - compatible = "fixed-clock"; - #clock-cells = <0>; - clock-frequency = <200000000>; - clock-output-names = "usi"; - }; - cpus: cpus { #address-cells = <2>; #size-cells = <0>; @@ -182,6 +172,28 @@ chipid@10000000 { reg = <0x10000000 0x24>; }; + cmu_peric0: clock-controller@10800000 { + compatible = "samsung,exynosautov920-cmu-peric0"; + reg = <0x10800000 0x8000>; + #clock-cells = <1>; + + clocks = <&xtcxo>, + <&cmu_top DOUT_CLKCMU_PERIC0_NOC>, + <&cmu_top DOUT_CLKCMU_PERIC0_IP>; + clock-names = "oscclk", + "noc", + "ip"; + }; + + cmu_top: clock-controller@11000000 { + compatible = "samsung,exynosautov920-cmu-top"; + reg = <0x11000000 0x8000>; + #clock-cells = <1>; + + clocks = <&xtcxo>; + clock-names = "oscclk"; + }; + gic: interrupt-controller@10400000 { compatible = "arm,gic-v3"; #interrupt-cells = <3>; @@ -213,7 +225,8 @@ usi_0: usi@108800c0 { #address-cells = <1>; #size-cells = <1>; ranges; - clocks = <&clock_usi>, <&clock_usi>; + clocks = <&cmu_peric0 CLK_MOUT_PERIC0_NOC_USER>, + <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>; clock-names = "pclk", "ipclk"; status = "disabled"; @@ -224,7 +237,8 @@ serial_0: serial@10880000 { interrupts = <GIC_SPI 764 IRQ_TYPE_LEVEL_HIGH>; pinctrl-names = "default"; pinctrl-0 = <&uart0_bus>; - clocks = <&clock_usi>, <&clock_usi>; + clocks = <&cmu_peric0 CLK_MOUT_PERIC0_NOC_USER>, + <&cmu_peric0 CLK_DOUT_PERIC0_USI00_USI>; clock-names = "uart", "clk_uart_baud0"; samsung,uart-fifosize = <256>; status = "disabled";
Add cmu_top, cmu_peric0 clock nodes and switch USI clocks instead of dummy fixed-rate-clock. Signed-off-by: Sunyeal Hong <sunyeal.hong@samsung.com> --- .../arm64/boot/dts/exynos/exynosautov920.dtsi | 40 +++++++++++++------ 1 file changed, 27 insertions(+), 13 deletions(-)