Re: [PATCH v13 03/10] usb: dwc3: core: Refactor PHY logic to support Multiport Controller

[Johan Hovold]

On Sun, Oct 22, 2023 at 11:33:52PM +0530, Krishna Kurapati PSSNV wrote:
> On 10/20/2023 3:27 PM, Johan Hovold wrote:
> > On Sat, Oct 07, 2023 at 09:17:59PM +0530, Krishna Kurapati wrote:
> >> From: Harsh Agarwal <quic_harshq@xxxxxxxxxxx>
> >>
> >> Currently the DWC3 driver supports only single port controller
> >> which requires at most one HS and one SS PHY.
> > 
> > Should that not be "at least one HS PHY and at most one SS PHY"?
> >   
> 
> No, I think it would be appropriate to say "at least one phy (HS/SS)" as 
> even one phy is sufficient to get things working.

No, that would be a violation if the binding (even if the driver may not
currently enforce it for generic phys) as well as the USB spec.

Also note that your implementation (i.e. this very patch) assumes that
num_usb2_ports >= num_usb3_ports.
 
> >> Add support for detecting, obtaining and configuring phy's supported
> > 
> > "PHYs" for consistency, no apostrophe
> > 
> >> by a multiport controller and. Limit the max number of ports
> > 
> > "and." what? Looks like part of the sentence is missing? Or just drop
> > " and"?
> > 
> >> supported to 4 as only SC8280 which is a quad port controller supports
> > 
> > s/4/four/
> > 
> > Just change this to
> > 
> > 	Limit support to multiport controllers with up to four ports for
> > 	now (e.g. as needed for SC8280XP).
> > 
> >> Multiport currently.
> > 
> > You use capitalised "Multiport" in several places it seems. Is this an
> > established term for these controllers or should it just be "multiport"
> > or "multiple ports"?
> > 
> This is an established term AFAIK. So I've been using it here like this.

Do you have a pointer? A google search seems to mostly come up with
links to this patch series.

Johan