| Message ID | 20241204111424.263055-1-bigunclemax@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] riscv: dts: thead: Fix TH1520 emmc and shdci clock rate | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/vmtest-fixes-PR | fail | merge-conflict |
bigunclemax@ wrote: > From: Maksim Kiselev <bigunclemax@gmail.com> > > In accordance with LicheePi 4A BSP the clock that comes to emmc/sdhci > is 198Mhz. > > But changing from fixed-clock to CLK_EMMC_SDIO leads to increasing input > clock from 198Mhz to 792Mhz. Because the CLK_EMMC_SDIO is actually 792Mhz. > > Therefore calculation of output SDCLK is incorrect now. > The mmc driver sets the divisor to 4 times larger than it should be > and emmc/sd works 4 times slower. > > This can be confirmed with fio test: > Sequential read of emmc with fixed 198Mz clock: > READ: bw=289MiB/s (303MB/s) > > Sequential read with CLK_EMMC_SDIO clock: > READ: bw=82.6MiB/s (86.6MB/s) > > Let's fix this issue by providing fixed-factor-clock that divides > CLK_EMMC_SDIO by 4 for emmc/sd nodes. Thanks for finding this bug! However, this feels like a work-around for a bug in the clock driver, and even if there is a fixed factor divider somewhere this should probably be modelled by the clock driver. Did you look into the documentation[1] and try to figure out where eMMC clock comes from and where the /4 is missing? There is also a vendor tree somewhere with a much more complete clock driver. Drew do you remember where it is? Maybe it's worth looking at how that driver models the eMMC clocks. [1]: https://openbeagle.org/beaglev-ahead/beaglev-ahead/-/blob/main/docs/TH1520%20System%20User%20Manual.pdf /Emil > > Fixes: 03a20182e1e0 ("riscv: dts: thead: change TH1520 mmc nodes to use clock controller") > Signed-off-by: Maksim Kiselev <bigunclemax@gmail.com> > --- > arch/riscv/boot/dts/thead/th1520.dtsi | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi > index acfe030e803a..6c20965cd10c 100644 > --- a/arch/riscv/boot/dts/thead/th1520.dtsi > +++ b/arch/riscv/boot/dts/thead/th1520.dtsi > @@ -229,6 +229,14 @@ stmmac_axi_config: stmmac-axi-config { > snps,blen = <0 0 64 32 0 0 0>; > }; > > + sdhci_clk: sdhci-clock { > + compatible = "fixed-factor-clock"; > + clocks = <&clk CLK_EMMC_SDIO>; > + #clock-cells = <0>; > + clock-div = <4>; > + clock-mult = <1>; > + }; > + > soc { > compatible = "simple-bus"; > interrupt-parent = <&plic>; > @@ -328,7 +336,7 @@ emmc: mmc@ffe7080000 { > compatible = "thead,th1520-dwcmshc"; > reg = <0xff 0xe7080000 0x0 0x10000>; > interrupts = <62 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&clk CLK_EMMC_SDIO>; > + clocks = <&sdhci_clk>; > clock-names = "core"; > status = "disabled"; > }; > @@ -337,7 +345,7 @@ sdio0: mmc@ffe7090000 { > compatible = "thead,th1520-dwcmshc"; > reg = <0xff 0xe7090000 0x0 0x10000>; > interrupts = <64 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&clk CLK_EMMC_SDIO>; > + clocks = <&sdhci_clk>; > clock-names = "core"; > status = "disabled"; > }; > @@ -346,7 +354,7 @@ sdio1: mmc@ffe70a0000 { > compatible = "thead,th1520-dwcmshc"; > reg = <0xff 0xe70a0000 0x0 0x10000>; > interrupts = <71 IRQ_TYPE_LEVEL_HIGH>; > - clocks = <&clk CLK_EMMC_SDIO>; > + clocks = <&sdhci_clk>; > clock-names = "core"; > status = "disabled"; > }; > -- > 2.45.2 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi index acfe030e803a..6c20965cd10c 100644 --- a/arch/riscv/boot/dts/thead/th1520.dtsi +++ b/arch/riscv/boot/dts/thead/th1520.dtsi @@ -229,6 +229,14 @@ stmmac_axi_config: stmmac-axi-config { snps,blen = <0 0 64 32 0 0 0>; }; + sdhci_clk: sdhci-clock { + compatible = "fixed-factor-clock"; + clocks = <&clk CLK_EMMC_SDIO>; + #clock-cells = <0>; + clock-div = <4>; + clock-mult = <1>; + }; + soc { compatible = "simple-bus"; interrupt-parent = <&plic>; @@ -328,7 +336,7 @@ emmc: mmc@ffe7080000 { compatible = "thead,th1520-dwcmshc"; reg = <0xff 0xe7080000 0x0 0x10000>; interrupts = <62 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clk CLK_EMMC_SDIO>; + clocks = <&sdhci_clk>; clock-names = "core"; status = "disabled"; }; @@ -337,7 +345,7 @@ sdio0: mmc@ffe7090000 { compatible = "thead,th1520-dwcmshc"; reg = <0xff 0xe7090000 0x0 0x10000>; interrupts = <64 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clk CLK_EMMC_SDIO>; + clocks = <&sdhci_clk>; clock-names = "core"; status = "disabled"; }; @@ -346,7 +354,7 @@ sdio1: mmc@ffe70a0000 { compatible = "thead,th1520-dwcmshc"; reg = <0xff 0xe70a0000 0x0 0x10000>; interrupts = <71 IRQ_TYPE_LEVEL_HIGH>; - clocks = <&clk CLK_EMMC_SDIO>; + clocks = <&sdhci_clk>; clock-names = "core"; status = "disabled"; };