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@xxxxxxxxxxxx> > > > --- > > > 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@ffefc00000 { > > > status = "disabled"; > > > }; > > > > > > + mmc0: mmc@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@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@xxxxxxxxxxx/ > > From the TH1520 System User Manual.pdf in there, I'd expect something like > > > > > emmc: mmc@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