Re: [PATCH 6/7] arm64: dts: renesas: r8a779f0: Add UFS node

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 12/04/2022 09:36, Yoshihiro Shimoda wrote:
> Add UFS node for R-Car S4-8 (r8a779f0).
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  arch/arm64/boot/dts/renesas/r8a779f0.dtsi | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git

Thank you for your patch. There is something to discuss/improve.

 a/arch/arm64/boot/dts/renesas/r8a779f0.dtsi
b/arch/arm64/boot/dts/renesas/r8a779f0.dtsi
> index b0241aa29fc8..8bf418a4232f 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779f0.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779f0.dtsi
> @@ -40,6 +40,13 @@ extalr_clk: extalr {
>  		clock-frequency = <0>;
>  	};
>  
> +	ufs30_clk: ufs30_refclk_v {

No underscores in node names. Node names should be generic, so if you
insist on prefix, it could be "ufs30-clk".

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		/* This value must be overridden by the board */
> +		clock-frequency = <0>;
> +	};
> +
>  	pmu_a55 {
>  		compatible = "arm,cortex-a55-pmu";
>  		interrupts-extended = <&gic GIC_PPI 7 IRQ_TYPE_LEVEL_LOW>;
> @@ -258,6 +265,18 @@ i2c5: i2c@e66e0000 {
>  			status = "disabled";
>  		};
>  
> +		ufs: scsi@e6860000 {

Node name: ufs (it is not a SCSI device, AFAIK).

> +			compatible = "renesas,r8a779f0-ufs";
> +			reg = <0 0xe6860000 0 0x100>;
> +			interrupts = <GIC_SPI 235 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cpg CPG_MOD 1514>, <&ufs30_clk>;
> +			clock-names = "fck", "ref_clk";


Best regards,
Krzysztof