[PATCH v7 2/4] riscv: dts: thead: Add TH1520 mmc controllers and sdhci clock

[Emil Renner Berthing]

Jisheng Zhang wrote:
> On Mon, Dec 04, 2023 at 01:47:45AM -0800, Emil Renner Berthing wrote:
> > Drew Fustini wrote:
> > > Add node for the SDHCI fixed clock. Add mmc0 node for the first mmc
> > > controller instance which is typically connected to the eMMC device.
> > > Add mmc1 node for the second mmc controller instance which is typically
> > > connected to microSD slot.
> > >
> > > Signed-off-by: Drew Fustini <dfustini at baylibre.com>
> > > ---
> > >  arch/riscv/boot/dts/thead/th1520.dtsi | 25 +++++++++++++++++++++++++
> > >  1 file changed, 25 insertions(+)
> > >
> > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > index ba4d2c673ac8..af4fdcd82e0b 100644
> > > --- a/arch/riscv/boot/dts/thead/th1520.dtsi
> > > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi
> > > @@ -146,6 +146,13 @@ uart_sclk: uart-sclk-clock {
> > >  		#clock-cells = <0>;
> > >  	};
> > >
> > > +	sdhci_clk: sdhci-clock {
> > > +		compatible = "fixed-clock";
> > > +		clock-frequency = <198000000>;
> > > +		clock-output-names = "sdhci_clk";
> > > +		#clock-cells = <0>;
> > > +	};
> > > +
> > >  	soc {
> > >  		compatible = "simple-bus";
> > >  		interrupt-parent = <&plic>;
> > > @@ -304,6 +311,24 @@ dmac0: dma-controller at ffefc00000 {
> > >  			status = "disabled";
> > >  		};
> > >
> > > +		mmc0: mmc at ffe7080000 {
> > > +			compatible = "thead,th1520-dwcmshc";
> > > +			reg = <0xff 0xe7080000 0x0 0x10000>;
> > > +			interrupts = <62 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&sdhci_clk>;
> > > +			clock-names = "core";
> > > +			status = "disabled";
> > > +		};
> > > +
> > > +		mmc1: mmc at ffe7090000 {
> > > +			compatible = "thead,th1520-dwcmshc";
> > > +			reg = <0xff 0xe7090000 0x0 0x10000>;
> > > +			interrupts = <64 IRQ_TYPE_LEVEL_HIGH>;
> > > +			clocks = <&sdhci_clk>;
> > > +			clock-names = "core";
> > > +			status = "disabled";
> > > +		};
> > > +
> >
> > Hi Drew,
> >
> > This doesn't seem to match the documentation shared here:
> > https://lore.kernel.org/linux-riscv/5f437109d2be2b8843f549a661054a2e3ec0d66e.camel@xry111.site/
> > From the TH1520 System User Manual.pdf in there, I'd expect something like
>
> >
> > 	emmc: mmc at ffe7080000 {
> > 		compatible = "thead,th1520-dwcmshc";
> > 		reg = <0xff 0xe7080000 0x0 0x10000>;
> > 		...
> > 	};
>
> Hi Emil,
>
> I think this isn't necessary. From other soc dts files, I see such
> naming, but lots socs just use mmc0, mmc1, and so on.

No it certainly isn't necessary. Those labels are purely for us humans to read
and are not present in the dtb. But that's exactly why I think it'd be a good
idea match the labels with the documentation, so it will easier for us humans
to match up the device tree source to documentation.

> And IIRC, the host for sd and sdio can support both, IOW, below
> sdio0/sdio1 may be used for sdcard.

Yes, all of the EMMC, SDIO0 and SDIO1 seem to be instances of the same IP.

/Emil