| Message ID | 20220412073647.3808493-7-yoshihiro.shimoda.uh@renesas.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | treewide: scsi: ufs: Add support for Renesas R-Car UFS controller | expand |
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
Hi Krzysztof, Thank you for your review! > From: Krzysztof Kozlowski, Sent: Tuesday, April 12, 2022 6:04 PM > > 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". I got it. I'll fix it. > > + 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). I got it. I'll fix it. (I had assumed that we should choose a node name from "2.2.2 Generic Names Recommendation" of the ePAPR v1.1 [1]. But, that's wrong. ) [1] https://elinux.org/images/c/cf/Power_ePAPR_APPROVED_v1.1.pdf Best regards, Yoshihiro Shimoda > > + 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
On 13/04/2022 02:09, Yoshihiro Shimoda wrote: >>> + ufs: scsi@e6860000 { >> >> Node name: ufs (it is not a SCSI device, AFAIK). > > I got it. I'll fix it. > (I had assumed that we should choose a node name from "2.2.2 Generic Names > Recommendation" of the ePAPR v1.1 [1]. But, that's wrong. ) > > [1] > https://elinux.org/images/c/cf/Power_ePAPR_APPROVED_v1.1.pdf These are examples and at the time written, I am not sure if UFS was considered. :) "ufs" is name of a standard, so it is a generic name. Best regards, Krzysztof
Hi Krzysztof, > From: Krzysztof Kozlowski, Sent: Wednesday, April 13, 2022 3:27 PM > > On 13/04/2022 02:09, Yoshihiro Shimoda wrote: > >>> + ufs: scsi@e6860000 { > >> > >> Node name: ufs (it is not a SCSI device, AFAIK). > > > > I got it. I'll fix it. > > (I had assumed that we should choose a node name from "2.2.2 Generic Names > > Recommendation" of the ePAPR v1.1 [1]. But, that's wrong. ) > > > > [1] > > > https://elinux.org/images/c/cf/Power_ePAPR_APPROVED_v1.1.pdf > > These are examples and at the time written, I am not sure if UFS was > considered. :) "ufs" is name of a standard, so it is a generic name. I see :) I understand "ufs" is a generic name. Best regards, Yoshihiro Shimoda
diff --git 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 { + 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 { + 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"; + 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";
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(+)