RE: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
Hi Simon-san, > From: Simon Horman, Sent: Friday, September 21, 2018 4:37 PM > > On Thu, Sep 20, 2018 at 05:55:00AM +, Yoshihiro Shimoda wrote: > > Hi Simon-san, > > > > > From: Simon Horman, Sent: Wednesday, September 5, 2018 7:33 PM > > > > > > On Fri, Aug 31, 2018 at 05:20:51PM +0900, Yoshihiro Shimoda wrote: > > > > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > > > > host (included phy) and peripheral. Otherwise, other side device > > > > cannot work correctly. So, this patch revises properties of clocks > > > > and resets. After that, each device driver can enable/deassert > > > > clocks/resets by its self. > > > > > > > > Notes: > > > > - To work the renesas_usbhs driver correctly when host side drivers > > > >are disabled and the renesas_usbhs driver doesn't have multiple > > > >clock management, this patch doesn't change the order of the clocks > > > >property in each hsusb node. > > > > - This patch doesn't have any side-effects even if the renesas_usbhs > > > >driver doesn't have reset_control and multiple clock management. > > > > > > > > Signed-off-by: Yoshihiro Shimoda > > > > > > Thanks Shimoda-san, > > > > > > This looks fine to me but I will wait to see if there are other reviews > > > before applying. > > > > > > Reviewed-by: Simon Horman > > > > Thank you for your review! > > However, since clock-names will be not used by renesas_usbhs driver, > > I'll submit v2 patch to remove the properties. > > Thanks, I'll wait for v2. I submitted v2 patch: https://patchwork.kernel.org/patch/10609245/ Best regards, Yoshihiro Shimoda
Re: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
On Thu, Sep 20, 2018 at 05:55:00AM +, Yoshihiro Shimoda wrote: > Hi Simon-san, > > > From: Simon Horman, Sent: Wednesday, September 5, 2018 7:33 PM > > > > On Fri, Aug 31, 2018 at 05:20:51PM +0900, Yoshihiro Shimoda wrote: > > > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > > > host (included phy) and peripheral. Otherwise, other side device > > > cannot work correctly. So, this patch revises properties of clocks > > > and resets. After that, each device driver can enable/deassert > > > clocks/resets by its self. > > > > > > Notes: > > > - To work the renesas_usbhs driver correctly when host side drivers > > >are disabled and the renesas_usbhs driver doesn't have multiple > > >clock management, this patch doesn't change the order of the clocks > > >property in each hsusb node. > > > - This patch doesn't have any side-effects even if the renesas_usbhs > > >driver doesn't have reset_control and multiple clock management. > > > > > > Signed-off-by: Yoshihiro Shimoda > > > > Thanks Shimoda-san, > > > > This looks fine to me but I will wait to see if there are other reviews > > before applying. > > > > Reviewed-by: Simon Horman > > Thank you for your review! > However, since clock-names will be not used by renesas_usbhs driver, > I'll submit v2 patch to remove the properties. Thanks, I'll wait for v2.
RE: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
Hi Simon-san, > From: Simon Horman, Sent: Wednesday, September 5, 2018 7:33 PM > > On Fri, Aug 31, 2018 at 05:20:51PM +0900, Yoshihiro Shimoda wrote: > > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > > host (included phy) and peripheral. Otherwise, other side device > > cannot work correctly. So, this patch revises properties of clocks > > and resets. After that, each device driver can enable/deassert > > clocks/resets by its self. > > > > Notes: > > - To work the renesas_usbhs driver correctly when host side drivers > >are disabled and the renesas_usbhs driver doesn't have multiple > >clock management, this patch doesn't change the order of the clocks > >property in each hsusb node. > > - This patch doesn't have any side-effects even if the renesas_usbhs > >driver doesn't have reset_control and multiple clock management. > > > > Signed-off-by: Yoshihiro Shimoda > > Thanks Shimoda-san, > > This looks fine to me but I will wait to see if there are other reviews > before applying. > > Reviewed-by: Simon Horman Thank you for your review! However, since clock-names will be not used by renesas_usbhs driver, I'll submit v2 patch to remove the properties. Best regards, Yoshihiro Shimoda
RE: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, September 6, 2018 7:19 PM > > Hi Shimoda-san, > > On Thu, Sep 6, 2018 at 12:09 PM Yoshihiro Shimoda > wrote: > > > From: Geert Uytterhoeven, Sent: Wednesday, September 5, 2018 9:59 PM > > > On Fri, Aug 31, 2018 at 10:22 AM Yoshihiro Shimoda > > > wrote: > > > > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > > > > host (included phy) and peripheral. Otherwise, other side device > > > > cannot work correctly. So, this patch revises properties of clocks > > > > and resets. After that, each device driver can enable/deassert > > > > clocks/resets by its self. > > > > > > > > Notes: > > > > - To work the renesas_usbhs driver correctly when host side drivers > > > >are disabled and the renesas_usbhs driver doesn't have multiple > > > >clock management, this patch doesn't change the order of the clocks > > > >property in each hsusb node. > > > > - This patch doesn't have any side-effects even if the renesas_usbhs > > > >driver doesn't have reset_control and multiple clock management. > > > > > > > > Signed-off-by: Yoshihiro Shimoda > > > > > > Thanks for your patch! > > > > > > I'm a bit confused about the HS-USB <-> EHCI/OHCI topology. > > > Can you please explain? > > > > > > Thanks! > > > > HS-USB <-> EHCI/OHCI topology on R-Car H3 is: > > > > EHCI/OHCI ch0/3 ---+--- PHY (is included on the EHCI/OHCI) --- External > > pins > > HS-USB ch0/3---+ > > > > EHCI ch1/2 --- PHY (is included on the EHCI/OHCI) --- External > > pins > > # These channels don't have HS-USB. > > Thanks, that's the part is missed, and couldn't find immediately in the > HW manual. That's true... > So HS-USB is the usb device ("gadget") part, and EHCI/OHCI is the > usb host part? That's right. > > > > And module stops and resets of HS-USB and EHCI/OHCI topology on R-Car H3 is: > > > > MSTP/RST703 ---+(OR)---+--- EHCI/OHCI ch0 > > MSTP/RST704 ---+ +--- HS-USB ch0 > > > > MSTP/RST702 --- EHCI/OHCI ch1 > > MSTP/RST701 --- EHCI/OHCI ch2 > > > > MSTP/RST700 ---+(OR)---+--- EHCI/OHCI ch3 > > MSTP/RST705 ---+ +--- HS-USB ch3 > > > > Should I describe these topology on the commit log or somewhere? > > Yes, I think that would be helpful. I got it. I'll submit v2 patch tomorrow or later. Best regards, Yoshihiro Shimoda > Thanks! > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@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
Re: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
Hi Shimoda-san, On Thu, Sep 6, 2018 at 12:09 PM Yoshihiro Shimoda wrote: > > From: Geert Uytterhoeven, Sent: Wednesday, September 5, 2018 9:59 PM > > On Fri, Aug 31, 2018 at 10:22 AM Yoshihiro Shimoda > > wrote: > > > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > > > host (included phy) and peripheral. Otherwise, other side device > > > cannot work correctly. So, this patch revises properties of clocks > > > and resets. After that, each device driver can enable/deassert > > > clocks/resets by its self. > > > > > > Notes: > > > - To work the renesas_usbhs driver correctly when host side drivers > > >are disabled and the renesas_usbhs driver doesn't have multiple > > >clock management, this patch doesn't change the order of the clocks > > >property in each hsusb node. > > > - This patch doesn't have any side-effects even if the renesas_usbhs > > >driver doesn't have reset_control and multiple clock management. > > > > > > Signed-off-by: Yoshihiro Shimoda > > > > Thanks for your patch! > > > > I'm a bit confused about the HS-USB <-> EHCI/OHCI topology. > > Can you please explain? > > > > Thanks! > > HS-USB <-> EHCI/OHCI topology on R-Car H3 is: > > EHCI/OHCI ch0/3 ---+--- PHY (is included on the EHCI/OHCI) --- External pins > HS-USB ch0/3---+ > > EHCI ch1/2 --- PHY (is included on the EHCI/OHCI) --- External pins > # These channels don't have HS-USB. Thanks, that's the part is missed, and couldn't find immediately in the HW manual. So HS-USB is the usb device ("gadget") part, and EHCI/OHCI is the usb host part? > > And module stops and resets of HS-USB and EHCI/OHCI topology on R-Car H3 is: > > MSTP/RST703 ---+(OR)---+--- EHCI/OHCI ch0 > MSTP/RST704 ---+ +--- HS-USB ch0 > > MSTP/RST702 --- EHCI/OHCI ch1 > MSTP/RST701 --- EHCI/OHCI ch2 > > MSTP/RST700 ---+(OR)---+--- EHCI/OHCI ch3 > MSTP/RST705 ---+ +--- HS-USB ch3 > > Should I describe these topology on the commit log or somewhere? Yes, I think that would be helpful. Thanks! Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
RE: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Wednesday, September 5, 2018 9:59 PM > > Hi Shimoda-san, > > On Fri, Aug 31, 2018 at 10:22 AM Yoshihiro Shimoda > wrote: > > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > > host (included phy) and peripheral. Otherwise, other side device > > cannot work correctly. So, this patch revises properties of clocks > > and resets. After that, each device driver can enable/deassert > > clocks/resets by its self. > > > > Notes: > > - To work the renesas_usbhs driver correctly when host side drivers > >are disabled and the renesas_usbhs driver doesn't have multiple > >clock management, this patch doesn't change the order of the clocks > >property in each hsusb node. > > - This patch doesn't have any side-effects even if the renesas_usbhs > >driver doesn't have reset_control and multiple clock management. > > > > Signed-off-by: Yoshihiro Shimoda > > Thanks for your patch! > > I'm a bit confused about the HS-USB <-> EHCI/OHCI topology. > Can you please explain? > > Thanks! HS-USB <-> EHCI/OHCI topology on R-Car H3 is: EHCI/OHCI ch0/3 ---+--- PHY (is included on the EHCI/OHCI) --- External pins HS-USB ch0/3---+ EHCI ch1/2 --- PHY (is included on the EHCI/OHCI) --- External pins # These channels don't have HS-USB. And module stops and resets of HS-USB and EHCI/OHCI topology on R-Car H3 is: MSTP/RST703 ---+(OR)---+--- EHCI/OHCI ch0 MSTP/RST704 ---+ +--- HS-USB ch0 MSTP/RST702 --- EHCI/OHCI ch1 MSTP/RST701 --- EHCI/OHCI ch2 MSTP/RST700 ---+(OR)---+--- EHCI/OHCI ch3 MSTP/RST705 ---+ +--- HS-USB ch3 Should I describe these topology on the commit log or somewhere? > > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > > @@ -707,7 +707,8 @@ > > "renesas,rcar-gen3-usbhs"; > > reg = <0 0xe659 0 0x100>; > > interrupts = ; > > - clocks = < CPG_MOD 704>; > > + clocks = < CPG_MOD 704>, < CPG_MOD 703>; > > + clock-names = "hsusb", "ehci/ohci"; > > 703 is EHCI/OHCI0. What about EHCI/OHCI1 and EHCI/OHCI2? EHCI/OHCI1 and EHCI/OHCI2 don't need because they are not related to the HS-USB0. > > dmas = <_dmac0 0>, <_dmac0 1>, > ><_dmac1 0>, <_dmac1 1>; > > dma-names = "ch0", "ch1", "ch2", "ch3"; > > @@ -715,7 +716,7 @@ > > phys = <_phy0>; > > phy-names = "usb"; > > power-domains = < R8A7795_PD_ALWAYS_ON>; > > - resets = < 704>; > > + resets = < 704>, < 703>; > > status = "disabled"; > > }; > > > > @@ -724,7 +725,8 @@ > > "renesas,rcar-gen3-usbhs"; > > reg = <0 0xe659c000 0 0x100>; > > interrupts = ; > > - clocks = < CPG_MOD 705>; > > + clocks = < CPG_MOD 705>, < CPG_MOD 700>; > > + clock-names = "hsusb", "ehci/ohci"; > > So this one is linked to EHCI/OHCI3? Yes. > > dmas = <_dmac2 0>, <_dmac2 1>, > ><_dmac3 0>, <_dmac3 1>; > > dma-names = "ch0", "ch1", "ch2", "ch3"; > > @@ -732,7 +734,7 @@ > > phys = <_phy3>; > > phy-names = "usb"; > > power-domains = < R8A7795_PD_ALWAYS_ON>; > > - resets = < 705>; > > + resets = < 705>, < 700>; > > status = "disabled"; > > }; > > > > @@ -2098,11 +2100,11 @@ > > compatible = "generic-ohci"; > > reg = <0 0xee08 0 0x100>; > > interrupts = ; > > - clocks = < CPG_MOD 703>; > > + clocks = < CPG_MOD 703>, < CPG_MOD 704>; > > phys = <_phy0>; > > phy-names = "usb"; > > power-domains = < R8A7795_PD_ALWAYS_ON>; > > - resets = < 703>; > > + resets = < 703>, < 704>; > > status = "disabled"; > > }; > > The above is for the ohci0 node. > What about ohci1 and ohci2? > > Same for ehci below. [oe]hci[12] don't have HS-USB. So, we don't need to revise them. Best regards, Yoshihiro Shimoda > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- > ge...@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say
Re: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
Hi Shimoda-san, On Fri, Aug 31, 2018 at 10:22 AM Yoshihiro Shimoda wrote: > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > host (included phy) and peripheral. Otherwise, other side device > cannot work correctly. So, this patch revises properties of clocks > and resets. After that, each device driver can enable/deassert > clocks/resets by its self. > > Notes: > - To work the renesas_usbhs driver correctly when host side drivers >are disabled and the renesas_usbhs driver doesn't have multiple >clock management, this patch doesn't change the order of the clocks >property in each hsusb node. > - This patch doesn't have any side-effects even if the renesas_usbhs >driver doesn't have reset_control and multiple clock management. > > Signed-off-by: Yoshihiro Shimoda Thanks for your patch! I'm a bit confused about the HS-USB <-> EHCI/OHCI topology. Can you please explain? Thanks! > --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi > @@ -707,7 +707,8 @@ > "renesas,rcar-gen3-usbhs"; > reg = <0 0xe659 0 0x100>; > interrupts = ; > - clocks = < CPG_MOD 704>; > + clocks = < CPG_MOD 704>, < CPG_MOD 703>; > + clock-names = "hsusb", "ehci/ohci"; 703 is EHCI/OHCI0. What about EHCI/OHCI1 and EHCI/OHCI2? > dmas = <_dmac0 0>, <_dmac0 1>, ><_dmac1 0>, <_dmac1 1>; > dma-names = "ch0", "ch1", "ch2", "ch3"; > @@ -715,7 +716,7 @@ > phys = <_phy0>; > phy-names = "usb"; > power-domains = < R8A7795_PD_ALWAYS_ON>; > - resets = < 704>; > + resets = < 704>, < 703>; > status = "disabled"; > }; > > @@ -724,7 +725,8 @@ > "renesas,rcar-gen3-usbhs"; > reg = <0 0xe659c000 0 0x100>; > interrupts = ; > - clocks = < CPG_MOD 705>; > + clocks = < CPG_MOD 705>, < CPG_MOD 700>; > + clock-names = "hsusb", "ehci/ohci"; So this one is linked to EHCI/OHCI3? > dmas = <_dmac2 0>, <_dmac2 1>, ><_dmac3 0>, <_dmac3 1>; > dma-names = "ch0", "ch1", "ch2", "ch3"; > @@ -732,7 +734,7 @@ > phys = <_phy3>; > phy-names = "usb"; > power-domains = < R8A7795_PD_ALWAYS_ON>; > - resets = < 705>; > + resets = < 705>, < 700>; > status = "disabled"; > }; > > @@ -2098,11 +2100,11 @@ > compatible = "generic-ohci"; > reg = <0 0xee08 0 0x100>; > interrupts = ; > - clocks = < CPG_MOD 703>; > + clocks = < CPG_MOD 703>, < CPG_MOD 704>; > phys = <_phy0>; > phy-names = "usb"; > power-domains = < R8A7795_PD_ALWAYS_ON>; > - resets = < 703>; > + resets = < 703>, < 704>; > status = "disabled"; > }; The above is for the ohci0 node. What about ohci1 and ohci2? Same for ehci below. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@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
Re: [PATCH] arm64: dts: renesas: revise properties for usb 2.0
On Fri, Aug 31, 2018 at 05:20:51PM +0900, Yoshihiro Shimoda wrote: > R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 > host (included phy) and peripheral. Otherwise, other side device > cannot work correctly. So, this patch revises properties of clocks > and resets. After that, each device driver can enable/deassert > clocks/resets by its self. > > Notes: > - To work the renesas_usbhs driver correctly when host side drivers >are disabled and the renesas_usbhs driver doesn't have multiple >clock management, this patch doesn't change the order of the clocks >property in each hsusb node. > - This patch doesn't have any side-effects even if the renesas_usbhs >driver doesn't have reset_control and multiple clock management. > > Signed-off-by: Yoshihiro Shimoda Thanks Shimoda-san, This looks fine to me but I will wait to see if there are other reviews before applying. Reviewed-by: Simon Horman
[PATCH] arm64: dts: renesas: revise properties for usb 2.0
R-Car Gen3 needs to enable/deassert clocks/resets of both usb 2.0 host (included phy) and peripheral. Otherwise, other side device cannot work correctly. So, this patch revises properties of clocks and resets. After that, each device driver can enable/deassert clocks/resets by its self. Notes: - To work the renesas_usbhs driver correctly when host side drivers are disabled and the renesas_usbhs driver doesn't have multiple clock management, this patch doesn't change the order of the clocks property in each hsusb node. - This patch doesn't have any side-effects even if the renesas_usbhs driver doesn't have reset_control and multiple clock management. Signed-off-by: Yoshihiro Shimoda --- This patch is based on the renesas.git / renesas-devel-20180830-v4.19-rc1 tag. This patch is related to the following patch series: https://patchwork.kernel.org/project/linux-renesas-soc/list/?series=13587 arch/arm64/boot/dts/renesas/r8a7795.dtsi | 34 --- arch/arm64/boot/dts/renesas/r8a7796.dtsi | 17 arch/arm64/boot/dts/renesas/r8a77965.dtsi | 17 arch/arm64/boot/dts/renesas/r8a77990.dtsi | 12 +-- arch/arm64/boot/dts/renesas/r8a77995.dtsi | 12 +-- 5 files changed, 48 insertions(+), 44 deletions(-) diff --git a/arch/arm64/boot/dts/renesas/r8a7795.dtsi b/arch/arm64/boot/dts/renesas/r8a7795.dtsi index c417d4a..9e9aead 100644 --- a/arch/arm64/boot/dts/renesas/r8a7795.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a7795.dtsi @@ -707,7 +707,8 @@ "renesas,rcar-gen3-usbhs"; reg = <0 0xe659 0 0x100>; interrupts = ; - clocks = < CPG_MOD 704>; + clocks = < CPG_MOD 704>, < CPG_MOD 703>; + clock-names = "hsusb", "ehci/ohci"; dmas = <_dmac0 0>, <_dmac0 1>, <_dmac1 0>, <_dmac1 1>; dma-names = "ch0", "ch1", "ch2", "ch3"; @@ -715,7 +716,7 @@ phys = <_phy0>; phy-names = "usb"; power-domains = < R8A7795_PD_ALWAYS_ON>; - resets = < 704>; + resets = < 704>, < 703>; status = "disabled"; }; @@ -724,7 +725,8 @@ "renesas,rcar-gen3-usbhs"; reg = <0 0xe659c000 0 0x100>; interrupts = ; - clocks = < CPG_MOD 705>; + clocks = < CPG_MOD 705>, < CPG_MOD 700>; + clock-names = "hsusb", "ehci/ohci"; dmas = <_dmac2 0>, <_dmac2 1>, <_dmac3 0>, <_dmac3 1>; dma-names = "ch0", "ch1", "ch2", "ch3"; @@ -732,7 +734,7 @@ phys = <_phy3>; phy-names = "usb"; power-domains = < R8A7795_PD_ALWAYS_ON>; - resets = < 705>; + resets = < 705>, < 700>; status = "disabled"; }; @@ -2098,11 +2100,11 @@ compatible = "generic-ohci"; reg = <0 0xee08 0 0x100>; interrupts = ; - clocks = < CPG_MOD 703>; + clocks = < CPG_MOD 703>, < CPG_MOD 704>; phys = <_phy0>; phy-names = "usb"; power-domains = < R8A7795_PD_ALWAYS_ON>; - resets = < 703>; + resets = < 703>, < 704>; status = "disabled"; }; @@ -2134,11 +2136,11 @@ compatible = "generic-ohci"; reg = <0 0xee0e 0 0x100>; interrupts = ; - clocks = < CPG_MOD 700>; + clocks = < CPG_MOD 700>, < CPG_MOD 705>; phys = <_phy3>; phy-names = "usb"; power-domains = < R8A7795_PD_ALWAYS_ON>; - resets = < 700>; + resets = < 700>, < 705>; status = "disabled"; }; @@ -2146,12 +2148,12 @@ compatible = "generic-ehci"; reg = <0 0xee080100 0 0x100>; interrupts = ; - clocks = < CPG_MOD 703>; + clocks = < CPG_MOD 703>, < CPG_MOD 704>; phys = <_phy0>; phy-names = "usb"; companion = <>; power-domains = < R8A7795_PD_ALWAYS_ON>; - resets = < 703>; + resets = < 703>, < 704>; status