On 1/20/2023 3:39 AM, Andrew Halaney wrote:
On Sun, Jan 15, 2023 at 05:11:43PM +0530, Krishna Kurapati wrote:
Currently the DWC3 driver supports only single port controller
which requires at most one HS and one SS PHY.
But the DWC3 USB controller can be connected to multiple ports and
each port can have their own PHYs. Each port of the multiport
controller can either be HS+SS capable or HS only capable
Proper quantification of them is required to modify GUSB2PHYCFG
and GUSB3PIPECTL registers appropriately.
Add support for detecting, obtaining and configuring phy's supported
by a multiport controller and limit the max number of ports
supported to 4.
Signed-off-by: Harsh Agarwal <quic_harshq@xxxxxxxxxxx>
Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
---
drivers/usb/dwc3/core.c | 304 +++++++++++++++++++++++++++++-----------
drivers/usb/dwc3/core.h | 15 +-
drivers/usb/dwc3/drd.c | 14 +-
3 files changed, 244 insertions(+), 89 deletions(-)
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 476b63618511..7e0a9a598dfd 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
<snip>
@@ -1575,6 +1690,21 @@ static void dwc3_get_properties(struct dwc3 *dwc)
dwc->dis_split_quirk = device_property_read_bool(dev,
"snps,dis-split-quirk");
+
+ /*
+ * If no mulitport properties are defined, default
+ * the port count to '1'.
+ */
+ ret = device_property_read_u32(dev, "num-ports",
+ &dwc->num_ports);
+ if (ret)
+ dwc->num_ports = 1;
+
+ ret = device_property_read_u32(dev, "num-ss-ports",
+ &dwc->num_ss_ports);
+ if (ret)
+ dwc->num_ss_ports = 1;
By using this DT property instead of using the number of each phy type you
find you can get into situations where you're writing DWC3_GUSB2PHYCFG, etc,
when there's no phy to go along with it.
Hi Andrew,
Thanks for the review. Yes, this decoupling is still there and its
fine I believe.
I ran into this when testing on sa8540p-ride, which only uses one of the
ports on the multiport controller. I didn't enable the other phys (not
sure if that was smart or not) and overrode phy-names/phys, but did not
override num-ports/num-ss-ports, which resulted in that. Nothing bad
happened on a quick test.. but I thought I'd highlight that as another
downside of decoupling this value from the number of phys you grab.
If we do not override phy-names or num-ports/num-ss-ports info in DT,
they are just defaulted to '1' and as per the current logic only port-1
registers must be configured. Isn't that the case happening ?
Here's a patch enabling sa8540p-ride, I'd love if you'd add it to the
series (probably needs clean up after review, and will definitely need
alteration after you update the dt-binding again). If not I'll continue
to test/review so please CC me!:
Sure, I can add this patch (probably will add the other phy's too)
during the final submission.
From dcb27d07f079194ebd7efe1c9bec64da78beb290 Mon Sep 17 00:00:00 2001
From: Andrew Halaney <ahalaney@xxxxxxxxxx>
Date: Thu, 19 Jan 2023 14:53:38 -0600
Subject: [PATCH] arm64: dts: qcom: sa8540p-ride: Enable usb_2
Content-type: text/plain
There is now support for the multiport USB controller this uses
so enable it.
The board only has a single port hooked up (despite it being wired up to
the multiport IP on the SoC). There's also a USB 2.0 mux hooked up,
which by default on boot is selected to mux properly. Grab the gpio
controlling that and ensure it stays in the right position so USB 2.0
continues to be routed from the external port to the SoC.
Signed-off-by: Andrew Halaney <ahalaney@xxxxxxxxxx>
---
arch/arm64/boot/dts/qcom/sa8540p-ride.dts | 24 +++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
index 97957f3baa64..56d4f43faa1e 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
+++ b/arch/arm64/boot/dts/qcom/sa8540p-ride.dts
@@ -246,6 +246,21 @@ &usb_0_qmpphy {
status = "okay";
};
+&usb_2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb2_en_state>;
+
+ status = "okay";
+};
+
+&usb_2_dwc3 {
+ dr_mode = "host";
+ num-ports = <1>;
+ num-ss-ports = <1>;
More over, if this is a multiport controller and you are using only
port-1, it is as good as a single port controller I believe and the
normal DT convention must work. Adding these properties as "1" is not
required as the driver logic defaults them to "1" if they are not found.
Just to add a point here (as I was not clear in DT Binding description,
My bad), the num-ports and num-ss-ports must indicate the HS/SS Phys
present on HW whether they are used in DT or not. Just to cover all
cases which user can use [1].
[]1:
https://lore.kernel.org/all/4eb26a54-148b-942f-01c6-64e66541de8b@xxxxxxxxxxx/
Regards,
Krishna,
+ phy-names = "usb2-phy", "usb3-phy";
+ phys = <&usb_2_hsphy0>, <&usb_2_qmpphy0>;
+};
+
&usb_2_hsphy0 {
vdda-pll-supply = <&vreg_l5a>;
vdda18-supply = <&vreg_l7g>;
@@ -313,4 +328,13 @@ wake-pins {
bias-pull-up;
};
};
+
+ usb2_en_state: usb2-en-state {
+ /* TS3USB221A USB2.0 mux select */
+ pins = "gpio24";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ output-low;
+ };
};
