On 1/20/2023 6:32 AM, Thinh Nguyen wrote:
Hi,
On Thu, Jan 19, 2023, Krishna Kurapati PSSNV wrote:
On 1/19/2023 6:06 AM, Thinh Nguyen wrote:
Hi,
On Sun, Jan 15, 2023, Krishna Kurapati wrote:
Currently the DWC3 driver supports only single port controller
which requires at most one HS and one SS PHY.
Add note here that multi-port is for host mode for clarity.
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(-)
<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
multi*
+ * the port count to '1'.
+ */
Can we initialize num_ports and num_ss_ports to 1 instead of setting it
on error (similar to how we handle other properties).
Hi Thinh,
Thanks for the review. On the bindings, Rob and Krzysztof have suggested
to get the num-ports and num-ss-ports by counting the Phy-names in DT.
This may be a bit problematic for non-DT device. Currently pci devices
pass fake DT properties to send these kinds of info. But that's fine,
we can enhance dwc3 for non-DT devices later.
Since there may be many cases where the user might skip giving any Phy's or
even skip different ports in the DT if he doesn't want to use them, can we
design/refactor the below logic as follows while mandating the fact that
user must give the SS Phy's if any starting from Port-0.:
num-ss-ports = max_port_index (usb3-portX) + 1
num-ports = max (max_port_index(usb2-portX), num-ss-ports) + 1
Ex: If there are 3 ports and only 1 is SS capable and user decides to skip
port-2 HS Phy.
case-1: phy-names = "usb2-port0", "usb3-port0", "usb2-port-1"
case-2: phy-names = "usb2-port0", "usb2-port-1", "usb3-port1"
In both cases, only one SS is present, just the order is changed. (Not sure
if last few ports can be made SS Capable instead of the first ports on any
HW) ?
But according to the above formula:
In case-1 : (num-ports = 2, num-ss-ports = 1) - This is correct
In case-2: (num-ports = 2, num-ss-ports = 2) - This is wrong
Can't we just walk through all the phy names to figure that out? Let's
not require the user to specify Port-0 is SS capable if they can skip
it.
Hi Thinh,
Thanks for the review.
May be I wasn't able to convey my intention properly in my previous
thread. The above suggested method doesn't tell that user must not skip
any phy.
Let us take the below case for 2 Port (HS+SS) capable controller.
If the user skips ss-phy 2, then:
phy-names = "usb2-port0", "usb3-port0", "usb2-port-1"
We don't need to configure GUSB3PIPECTL2 (for port-2) register ere. If
we parse phy-names and find it out, we see there are 2 hs-phy's and
1-ssphy and num-ports = 2 and num-ss-ports = 1.
If the user skips ss-phy-1, then phy-names would be something like the
below:
phy-names = "usb2-port0", "usb2-port-1", "usb3-port1";
We need to handle two types of interpretations here while parsing the
phy-names:
a) There are 2 SS Phy's and we just skipped the first one. In this
scenario, if we consider (num-ss-ports = 2) and (num-ports = 2) by using
the above formula as reference, we configure both GUSB3PIPECTL registers
present in the address map although we never use SS Phy-1 but nothing
must break. All ports would still work as the user intends with the
exception of GUSB3PIPECTL1 (for-port1) still being modified.
b) The second interpretation is something like, port-1 is only HS
capable and only Port-2 is SS Capable, and so in the phy-names only
port-2 has been provided a SS Phy. Just by parsing through the
phy-names, it would not be possible to get that info. So if we consider
num-ss-ports as 2 in this scenario, we end up meddling with wrong
registers (as there is only 1 GUSB3PIPECTL reg and we are assuing there
are 2). I wanted to make sure that this scenario was not possible.
num-ss-ports = max_port_index (usb3-portX) + 1
num-ports = max (max_port_index(usb2-portX), max_port_index(usb2-portX)) + 1
Taking case of a quad port controller with all ports SS Capable, if the
user wants to completely skip port-4. Then with above formula, we get
(num-ports = 3) and (num-ss-ports = 3) by parsing the phy-names and we
will configure exactly those dwc3-phy registers and not touch the port-4
registers which is still fine.
Please let me know if the above idea helps us in this scenario.
Regards,
Krishna,
