RE: [PATCH v2 2/4] arm64: dts: exynos: add initial CMU clock nodes in Exynos Auto v920

[sunyeal.hong]

Hello Alim,

> -----Original Message-----
> From: Alim Akhtar <alim.akhtar@xxxxxxxxxxx>
> Sent: Monday, July 8, 2024 8:05 PM
> To: 'Sunyeal Hong' <sunyeal.hong@xxxxxxxxxxx>; 'Krzysztof Kozlowski'
> <krzk@xxxxxxxxxx>; 'Sylwester Nawrocki' <s.nawrocki@xxxxxxxxxxx>; 'Chanwoo
> Choi' <cw00.choi@xxxxxxxxxxx>; 'Michael Turquette'
> <mturquette@xxxxxxxxxxxx>; 'Stephen Boyd' <sboyd@xxxxxxxxxx>; 'Rob
> Herring' <robh@xxxxxxxxxx>; 'Conor Dooley' <conor+dt@xxxxxxxxxx>
> Cc: linux-samsung-soc@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: RE: [PATCH v2 2/4] arm64: dts: exynos: add initial CMU clock
> nodes in Exynos Auto v920
> 
> 
> 
> > -----Original Message-----
> > From: Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > Sent: Monday, July 8, 2024 4:43 AM
> > To: Krzysztof Kozlowski <krzk@xxxxxxxxxx>; Sylwester Nawrocki
> > <s.nawrocki@xxxxxxxxxxx>; Chanwoo Choi <cw00.choi@xxxxxxxxxxx>; Alim
> > Akhtar <alim.akhtar@xxxxxxxxxxx>; Michael Turquette
> > <mturquette@xxxxxxxxxxxx>; Stephen Boyd <sboyd@xxxxxxxxxx>; Rob
> > Herring <robh@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>
> > Cc: linux-samsung-soc@xxxxxxxxxxxxxxx; linux-clk@xxxxxxxxxxxxxxx;
> > devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> > linux- kernel@xxxxxxxxxxxxxxx; Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > Subject: [PATCH v2 2/4] arm64: dts: exynos: add initial CMU clock
> > nodes in Exynos Auto v920
> >
> > Add cmu_top, cmu_peric0 clock nodes and switch USI clocks instead of
> > dummy fixed-rate-clock.
> >
> > Signed-off-by: Sunyeal Hong <sunyeal.hong@xxxxxxxxxxx>
> > ---
> >  .../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
> > @@ -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>;
> Please cross check the size of the register range, this looks to be more
> then what is needed.
> 
In the case of preic0, the size is up to 0x7088. The CMU block SFR area of ​​ExynosAuto v920 is generally specified up to 0x8000. There are differences for each block, but the settings are the same.
Do you think it is necessary to change the actual size of each block?
> > +			#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";
> > --
> > 2.45.2
> 
> 

Please review my answer again.

Thanks,
Sunyeal Hong