* [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
@ 2022-07-11 8:30 Sumit Garg
2022-07-11 15:46 ` Daniel Thompson
2022-07-16 15:19 ` (subset) " Bjorn Andersson
0 siblings, 2 replies; 6+ messages in thread
From: Sumit Garg @ 2022-07-11 8:30 UTC (permalink / raw)
To: linux-arm-msm, devicetree
Cc: agross, bjorn.andersson, robh+dt, krzysztof.kozlowski+dt, vkoul,
shawn.guo, bryan.odonoghue, nicolas.dechesne, mworsfold,
daniel.thompson, andrey.konovalov, Sumit Garg
Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
assigned to USB3 controller and USB2 controller. This assignment is
incorrect which only works by luck: as when each USB HCI comes up it
configures the *other* controllers PHY which is enough to make them
happy. If, for any reason, we were to disable one of the controllers then
both would stop working.
This was a difficult inconsistency to be caught which was found while
trying to enable USB support in u-boot. So with all the required drivers
ported to u-boot, I couldn't get the same USB storage device enumerated
in u-boot which was being enumerated fine by the kernel.
The root cause of the problem came out to be that I wasn't enabling USB2
PHY: "usb2_phy_prim" in u-boot. Then I realised that via simply disabling
the same USB2 PHY currently assigned to USB2 host controller in the
kernel disabled enumeration for USB3 host controller as well.
So fix this inconsistency by correctly assigning USB2 PHYs.
Fixes: 9375e7d719b3 ("arm64: dts: qcom: qcs404: Add USB devices and PHYs")
Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
---
Changes in v2:
- Update commit message description.
arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
index 513bf7343b2c..50edc11a5bb5 100644
--- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
@@ -557,7 +557,7 @@ usb3_dwc3: usb@7580000 {
compatible = "snps,dwc3";
reg = <0x07580000 0xcd00>;
interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
- phys = <&usb2_phy_sec>, <&usb3_phy>;
+ phys = <&usb2_phy_prim>, <&usb3_phy>;
phy-names = "usb2-phy", "usb3-phy";
snps,has-lpm-erratum;
snps,hird-threshold = /bits/ 8 <0x10>;
@@ -586,7 +586,7 @@ usb@78c0000 {
compatible = "snps,dwc3";
reg = <0x078c0000 0xcc00>;
interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
- phys = <&usb2_phy_prim>;
+ phys = <&usb2_phy_sec>;
phy-names = "usb2-phy";
snps,has-lpm-erratum;
snps,hird-threshold = /bits/ 8 <0x10>;
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
2022-07-11 8:30 [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment Sumit Garg
@ 2022-07-11 15:46 ` Daniel Thompson
2022-07-12 12:32 ` Sumit Garg
2022-07-16 15:19 ` (subset) " Bjorn Andersson
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Thompson @ 2022-07-11 15:46 UTC (permalink / raw)
To: Sumit Garg
Cc: linux-arm-msm, devicetree, agross, bjorn.andersson, robh+dt,
krzysztof.kozlowski+dt, vkoul, shawn.guo, bryan.odonoghue,
nicolas.dechesne, mworsfold, andrey.konovalov
On Mon, Jul 11, 2022 at 02:00:38PM +0530, Sumit Garg wrote:
> Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
> assigned to USB3 controller and USB2 controller. This assignment is
> incorrect which only works by luck: as when each USB HCI comes up it
> configures the *other* controllers PHY which is enough to make them
> happy. If, for any reason, we were to disable one of the controllers then
> both would stop working.
>
> This was a difficult inconsistency to be caught which was found while
> trying to enable USB support in u-boot. So with all the required drivers
> ported to u-boot, I couldn't get the same USB storage device enumerated
> in u-boot which was being enumerated fine by the kernel.
>
> The root cause of the problem came out to be that I wasn't enabling USB2
> PHY: "usb2_phy_prim" in u-boot. Then I realised that via simply disabling
> the same USB2 PHY currently assigned to USB2 host controller in the
> kernel disabled enumeration for USB3 host controller as well.
>
> So fix this inconsistency by correctly assigning USB2 PHYs.
>
> Fixes: 9375e7d719b3 ("arm64: dts: qcom: qcs404: Add USB devices and PHYs")
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
I've not got one of these board (nor any documentation for them) but the
description and change look OK. Thus FWIW:
Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
Daniel.
> ---
>
> Changes in v2:
> - Update commit message description.
>
> arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> index 513bf7343b2c..50edc11a5bb5 100644
> --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> @@ -557,7 +557,7 @@ usb3_dwc3: usb@7580000 {
> compatible = "snps,dwc3";
> reg = <0x07580000 0xcd00>;
> interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> - phys = <&usb2_phy_sec>, <&usb3_phy>;
> + phys = <&usb2_phy_prim>, <&usb3_phy>;
> phy-names = "usb2-phy", "usb3-phy";
> snps,has-lpm-erratum;
> snps,hird-threshold = /bits/ 8 <0x10>;
> @@ -586,7 +586,7 @@ usb@78c0000 {
> compatible = "snps,dwc3";
> reg = <0x078c0000 0xcc00>;
> interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> - phys = <&usb2_phy_prim>;
> + phys = <&usb2_phy_sec>;
> phy-names = "usb2-phy";
> snps,has-lpm-erratum;
> snps,hird-threshold = /bits/ 8 <0x10>;
> --
> 2.25.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
2022-07-11 15:46 ` Daniel Thompson
@ 2022-07-12 12:32 ` Sumit Garg
2022-07-12 14:05 ` Daniel Thompson
0 siblings, 1 reply; 6+ messages in thread
From: Sumit Garg @ 2022-07-12 12:32 UTC (permalink / raw)
To: Daniel Thompson
Cc: linux-arm-msm, devicetree, agross, bjorn.andersson, robh+dt,
krzysztof.kozlowski+dt, vkoul, shawn.guo, bryan.odonoghue,
nicolas.dechesne, mworsfold, andrey.konovalov
On Mon, 11 Jul 2022 at 21:16, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Mon, Jul 11, 2022 at 02:00:38PM +0530, Sumit Garg wrote:
> > Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
> > assigned to USB3 controller and USB2 controller. This assignment is
> > incorrect which only works by luck: as when each USB HCI comes up it
> > configures the *other* controllers PHY which is enough to make them
> > happy. If, for any reason, we were to disable one of the controllers then
> > both would stop working.
> >
> > This was a difficult inconsistency to be caught which was found while
> > trying to enable USB support in u-boot. So with all the required drivers
> > ported to u-boot, I couldn't get the same USB storage device enumerated
> > in u-boot which was being enumerated fine by the kernel.
> >
> > The root cause of the problem came out to be that I wasn't enabling USB2
> > PHY: "usb2_phy_prim" in u-boot. Then I realised that via simply disabling
> > the same USB2 PHY currently assigned to USB2 host controller in the
> > kernel disabled enumeration for USB3 host controller as well.
> >
> > So fix this inconsistency by correctly assigning USB2 PHYs.
> >
> > Fixes: 9375e7d719b3 ("arm64: dts: qcom: qcs404: Add USB devices and PHYs")
> > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
>
> I've not got one of these board (nor any documentation for them) but the
> description and change look OK. Thus FWIW:
>
> Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
>
Thanks Daniel for the review.
BTW, I did confirmed that this fix is correct with respect to
documentation (SA2150P LINUX USB TECHNICAL OVERVIEW) as well:
2.1 USB memory addresses
■ USB3.0 core address starts with 0x7580000. USB3.0 is connected to:
□ SS PHY with start address as 0x78000
□ HS PHY with start address as 0x7a000.
■ USB2.0 core address starts with 0x78c0000; it is connected only to
HS PHY with the start address as 0x7c000.
-Sumit
>
> Daniel.
>
>
> > ---
> >
> > Changes in v2:
> > - Update commit message description.
> >
> > arch/arm64/boot/dts/qcom/qcs404.dtsi | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/qcs404.dtsi b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > index 513bf7343b2c..50edc11a5bb5 100644
> > --- a/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/qcs404.dtsi
> > @@ -557,7 +557,7 @@ usb3_dwc3: usb@7580000 {
> > compatible = "snps,dwc3";
> > reg = <0x07580000 0xcd00>;
> > interrupts = <GIC_SPI 26 IRQ_TYPE_LEVEL_HIGH>;
> > - phys = <&usb2_phy_sec>, <&usb3_phy>;
> > + phys = <&usb2_phy_prim>, <&usb3_phy>;
> > phy-names = "usb2-phy", "usb3-phy";
> > snps,has-lpm-erratum;
> > snps,hird-threshold = /bits/ 8 <0x10>;
> > @@ -586,7 +586,7 @@ usb@78c0000 {
> > compatible = "snps,dwc3";
> > reg = <0x078c0000 0xcc00>;
> > interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
> > - phys = <&usb2_phy_prim>;
> > + phys = <&usb2_phy_sec>;
> > phy-names = "usb2-phy";
> > snps,has-lpm-erratum;
> > snps,hird-threshold = /bits/ 8 <0x10>;
> > --
> > 2.25.1
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
2022-07-12 12:32 ` Sumit Garg
@ 2022-07-12 14:05 ` Daniel Thompson
2022-07-13 5:36 ` Sumit Garg
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Thompson @ 2022-07-12 14:05 UTC (permalink / raw)
To: Sumit Garg
Cc: linux-arm-msm, devicetree, agross, bjorn.andersson, robh+dt,
krzysztof.kozlowski+dt, vkoul, shawn.guo, bryan.odonoghue,
nicolas.dechesne, mworsfold, andrey.konovalov
On Tue, Jul 12, 2022 at 06:02:22PM +0530, Sumit Garg wrote:
> On Mon, 11 Jul 2022 at 21:16, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
> >
> > On Mon, Jul 11, 2022 at 02:00:38PM +0530, Sumit Garg wrote:
> > > Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
> > > assigned to USB3 controller and USB2 controller. This assignment is
> > > incorrect which only works by luck: as when each USB HCI comes up it
> > > configures the *other* controllers PHY which is enough to make them
> > > happy. If, for any reason, we were to disable one of the controllers then
> > > both would stop working.
> > >
> > > This was a difficult inconsistency to be caught which was found while
> > > trying to enable USB support in u-boot. So with all the required drivers
> > > ported to u-boot, I couldn't get the same USB storage device enumerated
> > > in u-boot which was being enumerated fine by the kernel.
> > >
> > > The root cause of the problem came out to be that I wasn't enabling USB2
> > > PHY: "usb2_phy_prim" in u-boot. Then I realised that via simply disabling
> > > the same USB2 PHY currently assigned to USB2 host controller in the
> > > kernel disabled enumeration for USB3 host controller as well.
> > >
> > > So fix this inconsistency by correctly assigning USB2 PHYs.
> > >
> > > Fixes: 9375e7d719b3 ("arm64: dts: qcom: qcs404: Add USB devices and PHYs")
> > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> >
> > I've not got one of these board (nor any documentation for them) but the
> > description and change look OK. Thus FWIW:
> >
> > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> >
>
> Thanks Daniel for the review.
No worries.
> BTW, I did confirmed that this fix is correct with respect to
> documentation (SA2150P LINUX USB TECHNICAL OVERVIEW) as well:
>
> 2.1 USB memory addresses
> ■ USB3.0 core address starts with 0x7580000. USB3.0 is connected to:
> □ SS PHY with start address as 0x78000
> □ HS PHY with start address as 0x7a000.
> ■ USB2.0 core address starts with 0x78c0000; it is connected only to
> HS PHY with the start address as 0x7c000.
I didn't mean to imply the patch was in any way deficient (the patch
description showed your experimental method pretty clearly). I just
wanted to be clear that I hadn't double checked anything outside of the
patch itself!
Daniel.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
2022-07-12 14:05 ` Daniel Thompson
@ 2022-07-13 5:36 ` Sumit Garg
0 siblings, 0 replies; 6+ messages in thread
From: Sumit Garg @ 2022-07-13 5:36 UTC (permalink / raw)
To: Daniel Thompson
Cc: linux-arm-msm, devicetree, agross, bjorn.andersson, robh+dt,
krzysztof.kozlowski+dt, vkoul, shawn.guo, bryan.odonoghue,
nicolas.dechesne, mworsfold, andrey.konovalov
On Tue, 12 Jul 2022 at 19:35, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Tue, Jul 12, 2022 at 06:02:22PM +0530, Sumit Garg wrote:
> > On Mon, 11 Jul 2022 at 21:16, Daniel Thompson
> > <daniel.thompson@linaro.org> wrote:
> > >
> > > On Mon, Jul 11, 2022 at 02:00:38PM +0530, Sumit Garg wrote:
> > > > Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
> > > > assigned to USB3 controller and USB2 controller. This assignment is
> > > > incorrect which only works by luck: as when each USB HCI comes up it
> > > > configures the *other* controllers PHY which is enough to make them
> > > > happy. If, for any reason, we were to disable one of the controllers then
> > > > both would stop working.
> > > >
> > > > This was a difficult inconsistency to be caught which was found while
> > > > trying to enable USB support in u-boot. So with all the required drivers
> > > > ported to u-boot, I couldn't get the same USB storage device enumerated
> > > > in u-boot which was being enumerated fine by the kernel.
> > > >
> > > > The root cause of the problem came out to be that I wasn't enabling USB2
> > > > PHY: "usb2_phy_prim" in u-boot. Then I realised that via simply disabling
> > > > the same USB2 PHY currently assigned to USB2 host controller in the
> > > > kernel disabled enumeration for USB3 host controller as well.
> > > >
> > > > So fix this inconsistency by correctly assigning USB2 PHYs.
> > > >
> > > > Fixes: 9375e7d719b3 ("arm64: dts: qcom: qcs404: Add USB devices and PHYs")
> > > > Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> > >
> > > I've not got one of these board (nor any documentation for them) but the
> > > description and change look OK. Thus FWIW:
> > >
> > > Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>
> > >
> >
> > Thanks Daniel for the review.
>
> No worries.
>
>
> > BTW, I did confirmed that this fix is correct with respect to
> > documentation (SA2150P LINUX USB TECHNICAL OVERVIEW) as well:
> >
> > 2.1 USB memory addresses
> > ■ USB3.0 core address starts with 0x7580000. USB3.0 is connected to:
> > □ SS PHY with start address as 0x78000
> > □ HS PHY with start address as 0x7a000.
> > ■ USB2.0 core address starts with 0x78c0000; it is connected only to
> > HS PHY with the start address as 0x7c000.
>
> I didn't mean to imply the patch was in any way deficient (the patch
> description showed your experimental method pretty clearly). I just
> wanted to be clear that I hadn't double checked anything outside of the
> patch itself!
>
No worries, I see your point. I just wanted to put out this
information for maintainers which I was able to find yesterday.
-Sumit
>
> Daniel.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: (subset) [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
2022-07-11 8:30 [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment Sumit Garg
2022-07-11 15:46 ` Daniel Thompson
@ 2022-07-16 15:19 ` Bjorn Andersson
1 sibling, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2022-07-16 15:19 UTC (permalink / raw)
To: linux-arm-msm, Sumit Garg, devicetree
Cc: shawn.guo, mworsfold, bryan.odonoghue, robh+dt, vkoul,
andrey.konovalov, krzysztof.kozlowski+dt, nicolas.dechesne,
daniel.thompson, agross
On Mon, 11 Jul 2022 14:00:38 +0530, Sumit Garg wrote:
> Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
> assigned to USB3 controller and USB2 controller. This assignment is
> incorrect which only works by luck: as when each USB HCI comes up it
> configures the *other* controllers PHY which is enough to make them
> happy. If, for any reason, we were to disable one of the controllers then
> both would stop working.
>
> [...]
Applied, thanks!
[1/1] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment
commit: 58577966a42fc0b660b5e2c7c9e5a2241363ea83
Best regards,
--
Bjorn Andersson <bjorn.andersson@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-07-16 15:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-07-11 8:30 [PATCH v2] arm64: dts: qcom: qcs404: Fix incorrect USB2 PHYs assignment Sumit Garg
2022-07-11 15:46 ` Daniel Thompson
2022-07-12 12:32 ` Sumit Garg
2022-07-12 14:05 ` Daniel Thompson
2022-07-13 5:36 ` Sumit Garg
2022-07-16 15:19 ` (subset) " Bjorn Andersson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).