Re: [PATCH v3 6/7] riscv: dts: starfive: Add initial StarFive JH7110 device tree

[Hal Feng]

On Tue, 20 Dec 2022 21:31:43 +0000, Conor Dooley wrote:
> On Tue, Dec 20, 2022 at 09:12:46AM +0800, Hal Feng wrote:
> > From: Emil Renner Berthing <kernel@xxxxxxxx>
> > 
> > Add initial device tree for the JH7110 RISC-V SoC by StarFive
> > Technology Ltd.
> > 
> > Signed-off-by: Emil Renner Berthing <kernel@xxxxxxxx>
> > Co-developed-by: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Jianlong Huang <jianlong.huang@xxxxxxxxxxxxxxxx>
> > Co-developed-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Hal Feng <hal.feng@xxxxxxxxxxxxxxxx>
> > ---
> 
> FWIW, this cpu-map is now the default in linux, so you no longer *need*
> to add it for that purpose - but there's obviously no harm in being
> explicit for other operating systems etc. (IOW, don't remove it!)
> 
> > +		cpu-map {
> > +			cluster0 {
> > +				core0 {
> > +					cpu = <&S76_0>;
> > +				};
> > +
> > +				core1 {
> > +					cpu = <&U74_1>;
> > +				};
> > +
> > +				core2 {
> > +					cpu = <&U74_2>;
> > +				};
> > +
> > +				core3 {
> > +					cpu = <&U74_3>;
> > +				};
> > +
> > +				core4 {
> > +					cpu = <&U74_4>;
> > +				};
> > +			};
> > +		};
> > +	};
> 
> > +		syscrg: clock-controller@13020000 {
> 
> For obvious reasons, I cannot apply this until both the clock & pinctrl
> bindings are in my tree - but you know that already.
> 
> > +			compatible = "starfive,jh7110-syscrg";
> > +			reg = <0x0 0x13020000 0x0 0x10000>;
> > +			clocks = <&osc>, <&gmac1_rmii_refin>,
> > +				 <&gmac1_rgmii_rxin>,
> > +				 <&i2stx_bclk_ext>, <&i2stx_lrck_ext>,
> > +				 <&i2srx_bclk_ext>, <&i2srx_lrck_ext>,
> > +				 <&tdm_ext>, <&mclk_ext>;
> 
> As Krzk asked - are these clocks really all inputs to the SoC?

Yes, they are all external clock sources inputted to the SoC. They are
used as root clocks or optional parent clocks in clock tree.

> 
> > +			clock-names = "osc", "gmac1_rmii_refin",
> > +				      "gmac1_rgmii_rxin",
> > +				      "i2stx_bclk_ext", "i2stx_lrck_ext",
> > +				      "i2srx_bclk_ext", "i2srx_lrck_ext",
> > +				      "tdm_ext", "mclk_ext";
> > +			#clock-cells = <1>;
> > +			#reset-cells = <1>;
> > +		};
> > +
> > +		gpio: gpio@13040000 {
> 
> > +		gpioa: gpio@17020000 {
> 
> Out of curiousity, why gpio & gpioa?

Oh, is it easier to read if I change "gpio" and "gpioa"
to "sysgpio" and "aongpio"? Thanks.

Best regards,
Hal