diff mbox series

[v6,3/3] arm64: dts: renesas: r9a07g044: Add USB2.0 device support

Message ID 20210812151808.7916-4-biju.das.jz@bp.renesas.com
State Accepted
Commit f86e17d6e8be33083d20ab802840ce68a9fff40f
Headers show
Series Add USB2.0 support | expand

Commit Message

Biju Das Aug. 12, 2021, 3:18 p.m. UTC 
Add USB2.0 device support to RZ/G2L SoC DT.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v5->v6:
 * Added Rb tag from Geert.
v4->v5:
 * No change.
v3->v4:
 * No change.
 V3:
  * Updated reset entries.
---
 arch/arm64/boot/dts/renesas/r9a07g044.dtsi | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Biju Das Aug. 17, 2021, 11:12 a.m. UTC | #1 
Hi All,

> Subject: RE: [PATCH v6 3/3] arm64: dts: renesas: r9a07g044: Add USB2.0

> device support

> 

> Hi Sergei,

> 

> Thanks for the feedback.

> 

> > Subject: Re: [PATCH v6 3/3] arm64: dts: renesas: r9a07g044: Add USB2.0

> > device support

> >

> > On 8/12/21 6:18 PM, Biju Das wrote:

> >

> > > Add USB2.0 device support to RZ/G2L SoC DT.

> > >

> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > [...]

> > > diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> > b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> > > index de78c921af22..2f313c2a81c7 100644

> > > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> > > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> > > @@ -391,6 +391,25 @@

> > >  			power-domains = <&cpg>;

> > >  			status = "disabled";

> > >  		};

> > > +

> > > +		hsusb: usb@11c60000 {

> > > +			compatible = "renesas,usbhs-r9a07g044",

> > > +				     "renesas,rza2-usbhs";

> > > +			reg = <0 0x11c60000 0 0x10000>;

> > > +			interrupts = <GIC_SPI 100 IRQ_TYPE_EDGE_RISING>,

> > > +				     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,

> > > +				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,

> > > +				     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;

> >

> >    Don't we need to specify "interrupt-names" when there a more than 1

> > interrupts?

> 

> This dtsi changes, as per binding documentation [1]. As you see,

> "interrupt-names" is optional.


For now I will go with current dt changes.

Later  I will create incremental patches for dt-binding with optional "interrupt-names",
"clock-names" and "reset names" for all the SoC's supported by this binding doc. 

After that, will send an incremental patch with adding optional properties in all SoC dtsi.

Does it make sense?

Regards,
Biju

> 

> [1] :- https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-

> next.git/tree/Documentation/devicetree/bindings/usb/renesas,usbhs.yaml?h=n

> ext-20210812

> 

> >

> > > +			clocks = <&cpg CPG_MOD R9A07G044_USB_PCLK>,

> > > +				 <&cpg CPG_MOD R9A07G044_USB_U2P_EXR_CPUCLK>;

> >

> >    And "clock-names" too?

> 

> Same here. It is optional.

> 

> >

> > > +			resets = <&phyrst 0>,

> > > +				 <&cpg R9A07G044_USB_U2P_EXL_SYSRST>;

> >

> >    And "reset-names"?

> 

> Same here. It is optional.

> 

> Regards,

> Biju

> >

> > [...]

> >

> > MBR, Sergei
Sergey Shtylyov Aug. 17, 2021, 4:41 p.m. UTC | #2 
On 8/17/21 2:12 PM, Biju Das wrote:

[...]
>>>> Add USB2.0 device support to RZ/G2L SoC DT.

>>>>

>>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

>>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

>>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

>>> [...]

>>>> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

>>> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

>>>> index de78c921af22..2f313c2a81c7 100644

>>>> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

>>>> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

>>>> @@ -391,6 +391,25 @@

>>>>  			power-domains = <&cpg>;

>>>>  			status = "disabled";

>>>>  		};

>>>> +

>>>> +		hsusb: usb@11c60000 {

>>>> +			compatible = "renesas,usbhs-r9a07g044",

>>>> +				     "renesas,rza2-usbhs";

>>>> +			reg = <0 0x11c60000 0 0x10000>;

>>>> +			interrupts = <GIC_SPI 100 IRQ_TYPE_EDGE_RISING>,

>>>> +				     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,

>>>> +				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,

>>>> +				     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;

>>>

>>>    Don't we need to specify "interrupt-names" when there a more than 1

>>> interrupts?

>>

>> This dtsi changes, as per binding documentation [1]. As you see,

>> "interrupt-names" is optional.

> 

> For now I will go with current dt changes.

> 

> Later  I will create incremental patches for dt-binding with optional "interrupt-names",

> "clock-names" and "reset names" for all the SoC's supported by this binding doc. 

> 

> After that, will send an incremental patch with adding optional properties in all SoC dtsi.

> 

> Does it make sense?


   I had the impression that the "*-names" prop was mandatory for a "*" prop having 2 values or mores.
If it's now allowed to be optional, don't bother with that at all.

> Regards,

> Biju



MBR, Sergei
Geert Uytterhoeven Aug. 23, 2021, 9:22 a.m. UTC | #3 
Hi Sergey,

On Tue, Aug 17, 2021 at 6:41 PM Sergey Shtylyov <s.shtylyov@omp.ru> wrote:
> On 8/17/21 2:12 PM, Biju Das wrote:

> [...]

> >>>> Add USB2.0 device support to RZ/G2L SoC DT.

> >>>>

> >>>> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>

> >>>> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> >>>> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> >>> [...]

> >>>> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> >>> b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> >>>> index de78c921af22..2f313c2a81c7 100644

> >>>> --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> >>>> +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi

> >>>> @@ -391,6 +391,25 @@

> >>>>                    power-domains = <&cpg>;

> >>>>                    status = "disabled";

> >>>>            };

> >>>> +

> >>>> +          hsusb: usb@11c60000 {

> >>>> +                  compatible = "renesas,usbhs-r9a07g044",

> >>>> +                               "renesas,rza2-usbhs";

> >>>> +                  reg = <0 0x11c60000 0 0x10000>;

> >>>> +                  interrupts = <GIC_SPI 100 IRQ_TYPE_EDGE_RISING>,

> >>>> +                               <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,

> >>>> +                               <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,

> >>>> +                               <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;

> >>>

> >>>    Don't we need to specify "interrupt-names" when there a more than 1

> >>> interrupts?

> >>

> >> This dtsi changes, as per binding documentation [1]. As you see,

> >> "interrupt-names" is optional.

> >

> > For now I will go with current dt changes.

> >

> > Later  I will create incremental patches for dt-binding with optional "interrupt-names",

> > "clock-names" and "reset names" for all the SoC's supported by this binding doc.

> >

> > After that, will send an incremental patch with adding optional properties in all SoC dtsi.

> >

> > Does it make sense?

>

>    I had the impression that the "*-names" prop was mandatory for a "*" prop having 2 values or mores.

> If it's now allowed to be optional, don't bother with that at all.


There's a difference between "mandatory according to good DT
binding design" and "mandatory according to the actual json-schema
DT bindings".  For now the tools only enforce the latter...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
index de78c921af22..2f313c2a81c7 100644
--- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
@@ -391,6 +391,25 @@ 
 			power-domains = <&cpg>;
 			status = "disabled";
 		};
+
+		hsusb: usb@11c60000 {
+			compatible = "renesas,usbhs-r9a07g044",
+				     "renesas,rza2-usbhs";
+			reg = <0 0x11c60000 0 0x10000>;
+			interrupts = <GIC_SPI 100 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cpg CPG_MOD R9A07G044_USB_PCLK>,
+				 <&cpg CPG_MOD R9A07G044_USB_U2P_EXR_CPUCLK>;
+			resets = <&phyrst 0>,
+				 <&cpg R9A07G044_USB_U2P_EXL_SYSRST>;
+			renesas,buswait = <7>;
+			phys = <&usb2_phy0 3>;
+			phy-names = "usb";
+			power-domains = <&cpg>;
+			status = "disabled";
+		};
 	};
 
 	timer {