diff mbox series

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

Message ID 20230421174142.382602-1-WeitaoWang-oc@zhaoxin.com (mailing list archive)
State Superseded
Headers show
Series [v2] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value | expand

Commit Message

Weitao Wang April 21, 2023, 5:41 p.m. UTC
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@rowland.harvard.edu>
Cc: stable@vger.kernel.org
Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
---
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(-)

Comments

Alan Stern April 21, 2023, 2:59 p.m. UTC | #1
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@rowland.harvard.edu>
> Cc: stable@vger.kernel.org
> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
> ---
> 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
>
Weitao Wang April 22, 2023, 11:11 p.m. UTC | #2
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@rowland.harvard.edu>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
>> ---
>> 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
>>
> .
diff mbox series

Patch

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. */