Re: [PATCH v1 3/3] riscv: dts: starfive: Add mmc node

William Qiu
2022-12-08T09:51:14Z

On 2022/12/7 23:14, Krzysztof Kozlowski wrote:
> On 07/12/2022 14:17, William Qiu wrote:
>> This adds the mmc node for the StarFive JH7110 SoC.
>> Set sdioo node to emmc and set sdio1 node to sd.
>>
>> Signed-off-by: William Qiu <william.qiu@starfivetech.com>
>> ---
>> .../jh7110-starfive-visionfive-v2.dts | 25 ++++++++++++
>> arch/riscv/boot/dts/starfive/jh7110.dtsi | 38 +++++++++++++++++++
>> 2 files changed, 63 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> index c8946cf3a268..6ef8e303c2e6 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> +++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
>> @@ -47,6 +47,31 @@ &clk_rtc {
>> clock-frequency = <32768>;
>> };
>>
>> +&sdio0 {
>> + max-frequency = <100000000>;
>> + card-detect-delay = <300>;
>> + bus-width = <8>;
>> + cap-mmc-highspeed;
>> + mmc-ddr-1_8v;
>> + mmc-hs200-1_8v;
>> + non-removable;
>> + cap-mmc-hw-reset;
>> + post-power-on-delay-ms = <200>;
>> + status = "okay";
>> +};
>> +
>> +&sdio1 {
>> + max-frequency = <100000000>;
>> + card-detect-delay = <300>;
>> + bus-width = <4>;
>> + no-sdio;
>> + no-mmc;
>> + broken-cd;
>> + cap-sd-highspeed;
>> + post-power-on-delay-ms = <200>;
>> + status = "okay";
>> +};
>> +
>> &gmac0_rmii_refin {
>> clock-frequency = <50000000>;
>> };
>> diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> index c22e8f1d2640..e90b085d7e41 100644
>> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
>> @@ -331,6 +331,11 @@ aoncrg: clock-controller@17000000 {
>> #reset-cells = <1>;
>> };
>>
>> + sys_syscon: sys_syscon@13030000 {
>
> No underscores in node names, generic node names (syscon or
> system-controller)

Will fix.

>
>> + compatible = "syscon";
>
> This is not allowed alone.

Will fix.

>
>> + reg = <0x0 0x13030000 0x0 0x1000>;
>> + };
>> +
>> gpio: gpio@13040000 {
>> compatible = "starfive,jh7110-sys-pinctrl";
>> reg = <0x0 0x13040000 0x0 0x10000>;
>> @@ -433,5 +438,38 @@ uart5: serial@12020000 {
>> reg-shift = <2>;
>> status = "disabled";
>> };
>> +
>> + /* unremovable emmc as mmcblk0 */
>> + sdio0: mmc@16010000 {
>> + compatible = "starfive,jh7110-sdio";
>> + reg = <0x0 0x16010000 0x0 0x10000>;
>> + clocks = <&syscrg JH7110_SYSCLK_SDIO0_AHB>,
>> + <&syscrg JH7110_SYSCLK_SDIO0_SDCARD>;
>> + clock-names = "biu","ciu";
>> + resets = <&syscrg JH7110_SYSRST_SDIO0_AHB>;
>> + reset-names = "reset";
>> + interrupts = <74>;
>> + fifo-depth = <32>;
>> + fifo-watermark-aligned;
>> + data-addr = <0>;
>> + starfive,sys-syscon = <&sys_syscon 0x14 0x1a 0x7c000000>;
>
> This does not match your bindings at all. "&sys_syscon" is a phandle,
> not a number of tuning retries, as you expect in your bindings.
>

Hi Krzysztof,

The sys_syscon is one of system controller registers, and it is used to be the data strobe
delay chain select. I will update its type and description in next version.

Thanks,
William Qiu

>
> Best regards,
> Krzysztof