| Message ID | 20230912081402.51477-1-william.qiu@starfivetech.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/3] dt-bindings: mmc: Remove properties from required | expand |
William Qiu wrote: > Drop unused properties and limit cclk_in to 50M, thus cancelling the > internal frequency and adopting the by-pass mode. That's two unrelated changes which should really be in different patches. But again the hardware still has the relevant field in the syscon registers even if the driver doesn't use it, so maybe just leave them and just keep this patch adding the assigned-clock* properties. /Emil > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > --- > .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++ > arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 -- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi > index d79f94432b27..d1f2ec308bca 100644 > --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi > @@ -205,6 +205,8 @@ &i2c6 { > > &mmc0 { > max-frequency = <100000000>; > + assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>; > + assigned-clock-rates = <50000000>; > bus-width = <8>; > cap-mmc-highspeed; > mmc-ddr-1_8v; > @@ -221,6 +223,8 @@ &mmc0 { > > &mmc1 { > max-frequency = <100000000>; > + assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>; > + assigned-clock-rates = <50000000>; > bus-width = <4>; > no-sdio; > no-mmc; > diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi > index e85464c328d0..7b8e841aeef8 100644 > --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi > +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi > @@ -870,7 +870,6 @@ mmc0: mmc@16010000 { > fifo-depth = <32>; > fifo-watermark-aligned; > data-addr = <0>; > - starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>; > status = "disabled"; > }; > > @@ -886,7 +885,6 @@ mmc1: mmc@16020000 { > fifo-depth = <32>; > fifo-watermark-aligned; > data-addr = <0>; > - starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>; > status = "disabled"; > }; > > -- > 2.34.1 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Sep 12, 2023 at 02:12:44AM -0700, Emil Renner Berthing wrote: > William Qiu wrote: > > Due to the change of tuning implementation, it's no longer necessary to > > use the "starfive,sysreg" property in dts, so remove it from required. > > nit: again the device tree should be a description of the hardware, so the > justification here shouldn't be that the Linux driver doesn't use the field, > but that it turns out the registers aren't required for the peripheral to work > properly. Don't respin the series just for this though. > > Reviewed-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> The fact that I can't actually apply this without breaking bisection kinda hints at removing it in this patch is incorrect. > > > Signed-off-by: William Qiu <william.qiu@starfivetech.com> > > --- > > Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml > > index 51e1b04e799f..553a75195c2e 100644 > > --- a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml > > +++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml > > @@ -55,7 +55,6 @@ required: > > - clocks > > - clock-names > > - interrupts > > - - starfive,sysreg > > > > unevaluatedProperties: false > > > > @@ -73,5 +72,4 @@ examples: > > fifo-depth = <32>; > > fifo-watermark-aligned; > > data-addr = <0>; > > - starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>; > > }; > > -- > > 2.34.1 > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Sep 12, 2023 at 04:13:59PM +0800, William Qiu wrote: > Hi, > > This series of patches changes the tuning implementation, from the > previous way of reading and writing system controller registers to > reading and writing UHS_REG_EXT register, thus optimizing the tuning > of obtaining delay-chain. > > Changes v1->v2: Please don't send new versions as a reply to the prior version. > - Rebased to v6.6rc1. > - Keeped "starfive,sysreg" in dt-bindings but removed from required. > - Changed the function interface name. > - Maked the code implementation more concise. > > The patch series is based on v6.6rc1. > > William Qiu (3): > dt-bindings: mmc: Remove properties from required > mmc: starfive: Change tuning implementation > riscv: dts: starfive: Drop unused properties and limit frquency > > .../bindings/mmc/starfive,jh7110-mmc.yaml | 2 - > .../jh7110-starfive-visionfive-2.dtsi | 4 + > arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 - > drivers/mmc/host/dw_mmc-starfive.c | 137 +++++------------- > 4 files changed, 44 insertions(+), 101 deletions(-) > > -- > 2.34.1 >
On 2023/9/12 21:23, Emil Renner Berthing wrote: > William Qiu wrote: >> Drop unused properties and limit cclk_in to 50M, thus cancelling the >> internal frequency and adopting the by-pass mode. > > That's two unrelated changes which should really be in different patches. But > again the hardware still has the relevant field in the syscon registers even if > the driver doesn't use it, so maybe just leave them and just keep this patch > adding the assigned-clock* properties. > > /Emil > >> Will update. Best Regards, William >> Signed-off-by: William Qiu <william.qiu@starfivetech.com> >> --- >> .../riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi | 4 ++++ >> arch/riscv/boot/dts/starfive/jh7110.dtsi | 2 -- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi >> index d79f94432b27..d1f2ec308bca 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi >> @@ -205,6 +205,8 @@ &i2c6 { >> >> &mmc0 { >> max-frequency = <100000000>; >> + assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>; >> + assigned-clock-rates = <50000000>; >> bus-width = <8>; >> cap-mmc-highspeed; >> mmc-ddr-1_8v; >> @@ -221,6 +223,8 @@ &mmc0 { >> >> &mmc1 { >> max-frequency = <100000000>; >> + assigned-clocks = <&syscrg JH7110_SYSCLK_SDIO1_SDCARD>; >> + assigned-clock-rates = <50000000>; >> bus-width = <4>; >> no-sdio; >> no-mmc; >> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> index e85464c328d0..7b8e841aeef8 100644 >> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi >> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi >> @@ -870,7 +870,6 @@ mmc0: mmc@16010000 { >> fifo-depth = <32>; >> fifo-watermark-aligned; >> data-addr = <0>; >> - starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>; >> status = "disabled"; >> }; >> >> @@ -886,7 +885,6 @@ mmc1: mmc@16020000 { >> fifo-depth = <32>; >> fifo-watermark-aligned; >> data-addr = <0>; >> - starfive,sysreg = <&sys_syscon 0x9c 0x1 0x3e>; >> status = "disabled"; >> }; >> >> -- >> 2.34.1 >> >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv
diff --git a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml index 51e1b04e799f..553a75195c2e 100644 --- a/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml +++ b/Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml @@ -55,7 +55,6 @@ required: - clocks - clock-names - interrupts - - starfive,sysreg unevaluatedProperties: false @@ -73,5 +72,4 @@ examples: fifo-depth = <32>; fifo-watermark-aligned; data-addr = <0>; - starfive,sysreg = <&sys_syscon 0x14 0x1a 0x7c000000>; };
Due to the change of tuning implementation, it's no longer necessary to use the "starfive,sysreg" property in dts, so remove it from required. Signed-off-by: William Qiu <william.qiu@starfivetech.com> --- Documentation/devicetree/bindings/mmc/starfive,jh7110-mmc.yaml | 2 -- 1 file changed, 2 deletions(-)