| Message ID | 20231129-th1520_mmc_dts-v7-2-c77fc19caa6f@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Conor Dooley |
| Headers | show |
| Series | RISC-V: Add MMC support for TH1520 boards | expand |
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@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@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@xry111.site/ 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>; ... }; sdio0: mmc@ffe7090000 { compatible = "thead,th1520-dwcmshc"; reg = <0xff 0xe7090000 0x0 0x10000>; ... }; sdio1: mmc@ffe70a0000 { compatible = "thead,th1520-dwcmshc"; reg = <0xff 0xe70a0000 0x0 0x10000>; ... }; /Emil
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@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@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@xry111.site/ > 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. And IIRC, the host for sd and sdio can support both, IOW, below sdio0/sdio1 may be used for sdcard. Thanks > > sdio0: mmc@ffe7090000 { > compatible = "thead,th1520-dwcmshc"; > reg = <0xff 0xe7090000 0x0 0x10000>; > ... > }; > > sdio1: mmc@ffe70a0000 { > compatible = "thead,th1520-dwcmshc"; > reg = <0xff 0xe70a0000 0x0 0x10000>; > ... > }; > > /Emil
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@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@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@xry111.site/ > > 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
On Tue, 05. Dec 06:48, Emil Renner Berthing wrote: > 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@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@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@xry111.site/ > > > 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. Hello together, yes i agree to this too. It is easier for troubleshooting etc. Damian > > > 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
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"; + }; + timer0: timer@ffefc32000 { compatible = "snps,dw-apb-timer"; reg = <0xff 0xefc32000 0x0 0x14>;
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@baylibre.com> --- arch/riscv/boot/dts/thead/th1520.dtsi | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+)