On 4/9/2024 6:41 AM, Thinh Nguyen wrote:
On Mon, Apr 08, 2024, Krishna Kurapati wrote:
Currently the DWC3 driver supports only single port controller
which requires at least one HS PHY and at most 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 PHYs supported
by a multiport controller. Limit support to multiport controllers
with up to four ports for now (e.g. as needed for SC8280XP).
Signed-off-by: Krishna Kurapati <quic_kriskura@xxxxxxxxxxx>
Reviewed-by: Johan Hovold <johan+linaro@xxxxxxxxxx>
---
drivers/usb/dwc3/core.c | 251 ++++++++++++++++++++++++++++------------
drivers/usb/dwc3/core.h | 14 ++-
drivers/usb/dwc3/drd.c | 15 ++-
3 files changed, 193 insertions(+), 87 deletions(-)
<snip>
@@ -1937,6 +2020,10 @@ static int dwc3_get_num_ports(struct dwc3 *dwc)
iounmap(base);
+ if (dwc->num_usb2_ports > DWC3_MAX_PORTS ||
+ dwc->num_usb3_ports > DWC3_MAX_PORTS)
+ return -ENOMEM;
This should be -EINVAL.
+
return 0;
}
<snip>
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 341e4c73cb2e..df2e111aa848 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -33,6 +33,12 @@
#include <linux/power_supply.h>
+/*
+ * Maximum number of ports currently supported for multiport
+ * controllers.
This macro here is being used per USB2 vs USB3 ports rather than USB2 +
USB3, unlike the xHCI MAXPORTS. You can clarify in the comment and
rename the macro to avoid any confusion. You can also create 2 separate
macros for number of USB2 and USB3 ports even if they share the same
value.
As noted[*], we support have different max number of usb2 ports vs usb3
ports. I would suggest splitting the macros.
Hi Thinh,
This macro was intended only to identify how many USB2 (or USB3) Phy's
were serviced/operated by this driver, not how many logical ports
present (like in xHCI). I don't think it would be confusing currently
given that it is only used to identify number of generic phy instances
to allocate and not used for any other purpose. Once the num_usb2_ports
and num_usb3_ports are read by get_num_ports(...) call, they directly
indicate how many ports are HS and SS respectively. Keeping the same in
mind, I returned ENOMEM above (as you mentioned) because we don't
allocate more than DWC3_MAX_PORTS and if the number of hs or ss ports is
more than that, we simply return ENOMEM saying the driver doesn't
support operating those many phy's.
[*] https://lore.kernel.org/linux-usb/20230801013031.ft3zpoatiyfegmh6@xxxxxxxxxxxx/
+ */
+#define DWC3_MAX_PORTS 4
+
But it's not a big issue whether you decided to push a new version or a
create a separate patch for the comments above. Here's my Ack:
Since this is not a bug, I would prefer to make a separate patch to
rename the macros. (If that is fine).
Acked-by: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx>
Thanks,
Thinh
