Re: [PATCH v2] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value

[WeitaoWang-oc@xxxxxxxxxxx]

On 2023/4/21 22:59, Alan Stern wrote:
> On Sat, Apr 22, 2023 at 01:41:42AM +0800, Weitao Wang wrote:
> > OverCurrent condition is not standardized in the UHCI spec.
> > Zhaoxin UHCI controllers report OverCurrent bit active off.
> > In order to handle OverCurrent condition correctly, the uhci-hcd
> > driver needs to be told to expect the active-off behavior.
> >
> > Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Weitao Wang <WeitaoWang-oc@xxxxxxxxxxx>
> > ---
> > v1->v2
> >   - Modify the description of this patch.
> >   - Let Zhaoxin and VIA share a common oc_low flag
> >
> >   drivers/usb/host/uhci-pci.c | 9 +++++----
> >   1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
> > index 3592f757fe05..034586911bb5 100644
> > --- a/drivers/usb/host/uhci-pci.c
> > +++ b/drivers/usb/host/uhci-pci.c
> > @@ -119,11 +119,12 @@ static int uhci_pci_init(struct usb_hcd *hcd)
> >   
> >   	uhci->rh_numports = uhci_count_ports(hcd);
> >   
> > -	/* Intel controllers report the OverCurrent bit active on.
> > -	 * VIA controllers report it active off, so we'll adjust the
> > -	 * bit value.  (It's not standardized in the UHCI spec.)
> > +	/* Intel controllers report the OverCurrent bit active on.  VIA
> > +	 * and ZHAOXIN controllers report it active off, so we'll adjust
> > +	 * the bit value.  (It's not standardized in the UHCI spec.)
> >   	 */
>
> The style we use now for multi-line comments is:
>
> 	/*
> 	 * Blah blah blah
> 	 * blah blah blah
> 	 */
>
> > -	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
> > +	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA ||
> > +		to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
>
> The indentation level of the continuation line should be different from
> the indentation of the statement below.  Otherwise it looks like the
> continuation line is part of the conditional block.
I see, Thanks for your careful examination. I'll change it later.

Best Regards,
Weitao

> Alan Stern
>
> >   		uhci->oc_low = 1;
> >   
> >   	/* HP's server management chip requires a longer port reset delay. */
> > --
> > 2.32.0
> .