| Message ID | 20210812151808.7916-4-biju.das.jz@bp.renesas.com |
|---|---|
| State | Accepted |
| Commit | f86e17d6e8be33083d20ab802840ce68a9fff40f |
| Headers | show |
| Series | Add USB2.0 support | expand |
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
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
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 --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 {