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

[Krzysztof Kozlowski]

On 03/02/2023 09:19, William Qiu wrote:
> This adds the mmc node for the StarFive JH7110 SoC.

Do not use "This xxx". Use imperative mode.
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> Set mmco node to emmc and set mmc1 node to sd.
> 
> Signed-off-by: William Qiu <william.qiu@xxxxxxxxxxxxxxxx>


> +
>  &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 64d260ea1f29..ae1a664e7af5 100644
> --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
> +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
> @@ -370,6 +370,11 @@ syscrg: clock-controller@13020000 {
>  			#reset-cells = <1>;
>  		};
>  
> +		sysreg: syscon@13030000 {
> +			compatible = "starfive,sysreg", "syscon";

No:
1. Undocumented.
2. A bit too generic. You should have here SoC specific compatible as
well (either as second or third compatible, if all your SoCs share
register layout).

> +			reg = <0x0 0x13030000 0x0 0x1000>;
> +		};
> +

Best regards,
Krzysztof