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

From: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>,
	"avri.altman@wdc.com" <avri.altman@wdc.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzk+dt@kernel.org>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	scsi <linux-scsi@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH v4 6/7] arm64: dts: renesas: r8a779f0: Add UFS node
Date: Thu, 28 Apr 2022 06:54:04 +0000	[thread overview]
Message-ID: <TYBPR01MB5341FD0D3C9B69EAF0CED3CDD8FD9@TYBPR01MB5341.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdU7_nqwT8J_OKLez9GDU8sCiP_PB1ioZBK5YvevToOsXg@mail.gmail.com>

Hi Geert-san,

Thank you for your review!

> From: Geert Uytterhoeven, Sent: Wednesday, April 27, 2022 10:14 PM
> 
> On Wed, Apr 20, 2022 at 10:31 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@renesas.com> wrote:
> > Add UFS node for R-Car S4-8 (r8a779f0).
> >
> > Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> 
> > --- 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-clk {
> > +               compatible = "fixed-clock";
> > +               #clock-cells = <0>;
> > +               /* This value must be overridden by the board */
> > +               clock-frequency = <0>;
> > +       };
> 
> This matches what we typically do for the PCIe reference clock,
> but relies on the currently-missing clock generator initialization
> in the boot loader stack.

Yes, that's true.

On UFS, the ufshcd.c requires a ref_clk and this is for it.
The driver will get "ref_clk" rate and check it by ufs_ref_clk_freqs[].

> > +
> >         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: ufs@e6860000 {
> > +                       compatible = "renesas,r8a779f0-ufs";
> > +                       reg = <0 0xe6860000 0 0x100>;
> > +                       interrupts = <GIC_SPI 235 IRQ_TYPE_LEVEL_HIGH>;
> 
> The R-Car S4-8 interrupt mapping spreadsheet lists two interrupts.

Oops. I'll add it.

> > +                       clocks = <&cpg CPG_MOD 1514>, <&ufs30_clk>;
> > +                       clock-names = "fck", "ref_clk";
> > +                       freq-table-hz = <200000000 200000000>, <38400000 38400000>;
> > +                       power-domains = <&sysc R8A779F0_PD_ALWAYS_ON>;
> > +                       resets = <&cpg 1514>;
> > +                       status = "disabled";
> > +               };
> > +
> >                 scif3: serial@e6c50000 {
> >                         compatible = "renesas,scif-r8a779f0",
> >                                      "renesas,rcar-gen4-scif", "renesas,scif";
> 
> The rest LGTM, so with the above resolved:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thanks!

Best regards,
Yoshihiro Shimoda