[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230120010226.wjwtisj4id6frirl@synopsys.com>
Date: Fri, 20 Jan 2023 01:02:38 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Felipe Balbi <balbi@...nel.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"quic_pkondeti@...cinc.com" <quic_pkondeti@...cinc.com>,
"quic_ppratap@...cinc.com" <quic_ppratap@...cinc.com>,
"quic_wcheng@...cinc.com" <quic_wcheng@...cinc.com>,
"quic_jackp@...cinc.com" <quic_jackp@...cinc.com>,
"quic_harshq@...cinc.com" <quic_harshq@...cinc.com>
Subject: Re: [RFC v4 2/5] usb: dwc3: core: Refactor PHY logic to support
Multiport Controller
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@...cinc.com>
> > > Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
> > > ---
> > > 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.
> I believe this covers all cases and I can read this in get_properties
> function. Let me know your opinion if this design is good to proceed
> further.
>
Thanks,
Thinh
Powered by blists - more mailing lists