2023-04-21 09:45:01

by Weitao Wang

[permalink] [raw]
Subject: [PATCH v2] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value

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 <[email protected]>
Cc: [email protected]
Signed-off-by: Weitao Wang <[email protected]>
---
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.)
*/
- 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)
uhci->oc_low = 1;

/* HP's server management chip requires a longer port reset delay. */
--
2.32.0


2023-04-21 15:12:46

by Alan Stern

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

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 <[email protected]>
> Cc: [email protected]
> Signed-off-by: Weitao Wang <[email protected]>
> ---
> 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.

Alan Stern

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

2023-04-22 15:16:39

by Weitao Wang

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

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 <[email protected]>
>> Cc: [email protected]
>> Signed-off-by: Weitao Wang <[email protected]>
>> ---
>> 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
>>
> .