linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value
  2023-04-20 11:14 [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value Weitao Wang
@ 2023-04-20  8:05 ` Greg KH
  2023-04-20 17:14   ` WeitaoWang-oc
  2023-04-20 14:49 ` Alan Stern
  1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2023-04-20  8:05 UTC (permalink / raw)
  To: Weitao Wang; +Cc: stern, linux-usb, linux-kernel, tonywwang, weitaowang

On Thu, Apr 20, 2023 at 07:14:45PM +0800, Weitao Wang wrote:
> Over Current condition is not standardized in the UHCI spec.
> Zhaoxin UHCI controllers report OverCurrent bit active off.
> Intel controllers report it active on, so we'll adjust the bit value.
> 
> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>

Note, your Subject: line has odd characters in it, can you rewrite it to
just use ascii?  I think it's the ':' character which should be ':',
right?

> ---
>  drivers/usb/host/uhci-pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
> index 3592f757fe05..177e3c2aa287 100644
> --- a/drivers/usb/host/uhci-pci.c
> +++ b/drivers/usb/host/uhci-pci.c
> @@ -126,6 +126,10 @@ static int uhci_pci_init(struct usb_hcd *hcd)
>  	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
>  		uhci->oc_low = 1;
>  
> +	/* ZHAOXIN controllers report OverCurrent bit active off. */
> +	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
> +		uhci->oc_low = 1;
> +

This should also go to the stable kernels, right?

But a new UHCI controller?  And this affects all of them?  And why is
Intel an issue here (as you mention in the changelog text)?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value
  2023-04-20 17:14   ` WeitaoWang-oc
@ 2023-04-20  9:23     ` Greg KH
  0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2023-04-20  9:23 UTC (permalink / raw)
  To: WeitaoWang-oc@zhaoxin.com
  Cc: stern, linux-usb, linux-kernel, tonywwang, weitaowang

On Fri, Apr 21, 2023 at 01:14:46AM +0800, WeitaoWang-oc@zhaoxin.com wrote:
> On 2023/4/20 16:05, Greg KH wrote:
> > On Thu, Apr 20, 2023 at 07:14:45PM +0800, Weitao Wang wrote:
> > > Over Current condition is not standardized in the UHCI spec.
> > > Zhaoxin UHCI controllers report OverCurrent bit active off.
> > > Intel controllers report it active on, so we'll adjust the bit value.
> > > 
> > > Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
> > 
> > Note, your Subject: line has odd characters in it, can you rewrite it to
> > just use ascii?  I think it's the ':' character which should be ':',
> > right?Yes,you are right. it's my typo mistake. I'll rewrite this patch. Thanks!
> 
> > > ---
> > >   drivers/usb/host/uhci-pci.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
> > > index 3592f757fe05..177e3c2aa287 100644
> > > --- a/drivers/usb/host/uhci-pci.c
> > > +++ b/drivers/usb/host/uhci-pci.c
> > > @@ -126,6 +126,10 @@ static int uhci_pci_init(struct usb_hcd *hcd)
> > >   	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
> > >   		uhci->oc_low = 1;
> > > +	/* ZHAOXIN controllers report OverCurrent bit active off. */
> > > +	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
> > > +		uhci->oc_low = 1;
> > > +
> > 
> > This should also go to the stable kernels, right?
> > 
> > But a new UHCI controller?  And this affects all of them?
> All UHCI controller of zhaoxin have this issue, And Hope this
> patch go to stable kernel.Could I resubmit this patch with
> modify subject typo mistake?

Yes, please do so and add the needed cc: stable as is documented
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value
@ 2023-04-20 11:14 Weitao Wang
  2023-04-20  8:05 ` Greg KH
  2023-04-20 14:49 ` Alan Stern
  0 siblings, 2 replies; 6+ messages in thread
From: Weitao Wang @ 2023-04-20 11:14 UTC (permalink / raw)
  To: stern, gregkh, linux-usb, linux-kernel; +Cc: tonywwang, weitaowang

Over Current condition is not standardized in the UHCI spec.
Zhaoxin UHCI controllers report OverCurrent bit active off.
Intel controllers report it active on, so we'll adjust the bit value.

Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
---
 drivers/usb/host/uhci-pci.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
index 3592f757fe05..177e3c2aa287 100644
--- a/drivers/usb/host/uhci-pci.c
+++ b/drivers/usb/host/uhci-pci.c
@@ -126,6 +126,10 @@ static int uhci_pci_init(struct usb_hcd *hcd)
 	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
 		uhci->oc_low = 1;
 
+	/* ZHAOXIN controllers report OverCurrent bit active off. */
+	if (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. */
 	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_HP)
 		uhci->wait_for_hp = 1;
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value
  2023-04-20 11:14 [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value Weitao Wang
  2023-04-20  8:05 ` Greg KH
@ 2023-04-20 14:49 ` Alan Stern
  2023-04-21 10:59   ` WeitaoWang-oc
  1 sibling, 1 reply; 6+ messages in thread
From: Alan Stern @ 2023-04-20 14:49 UTC (permalink / raw)
  To: Weitao Wang; +Cc: gregkh, linux-usb, linux-kernel, tonywwang, weitaowang

On Thu, Apr 20, 2023 at 07:14:45PM +0800, Weitao Wang wrote:
> Over Current condition is not standardized in the UHCI spec.
> Zhaoxin UHCI controllers report OverCurrent bit active off.
> Intel controllers report it active on, so we'll adjust the bit value.

The last sentence is irrelevant.  You should say instead that the 
uhci-hcd driver needs to be told to expect the active-off behavior.

> 
> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
> ---
>  drivers/usb/host/uhci-pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
> index 3592f757fe05..177e3c2aa287 100644
> --- a/drivers/usb/host/uhci-pci.c
> +++ b/drivers/usb/host/uhci-pci.c
> @@ -126,6 +126,10 @@ static int uhci_pci_init(struct usb_hcd *hcd)
>  	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
>  		uhci->oc_low = 1;
>  
> +	/* ZHAOXIN controllers report OverCurrent bit active off. */
> +	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
> +		uhci->oc_low = 1;

This is really just a matter of taste, but IMO it would be better to 
combine this comment and test with the preceding one.  Something like:

	/*
	 * 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 ||
	    to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
		uhci->oc_low = 1;

Alan Stern

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value
  2023-04-20  8:05 ` Greg KH
@ 2023-04-20 17:14   ` WeitaoWang-oc
  2023-04-20  9:23     ` Greg KH
  0 siblings, 1 reply; 6+ messages in thread
From: WeitaoWang-oc @ 2023-04-20 17:14 UTC (permalink / raw)
  To: Greg KH; +Cc: stern, linux-usb, linux-kernel, tonywwang, weitaowang

On 2023/4/20 16:05, Greg KH wrote:
> On Thu, Apr 20, 2023 at 07:14:45PM +0800, Weitao Wang wrote:
>> Over Current condition is not standardized in the UHCI spec.
>> Zhaoxin UHCI controllers report OverCurrent bit active off.
>> Intel controllers report it active on, so we'll adjust the bit value.
>>
>> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
> 
> Note, your Subject: line has odd characters in it, can you rewrite it to
> just use ascii?  I think it's the ':' character which should be ':',
> right?Yes,you are right. it's my typo mistake. I'll rewrite this patch. Thanks!

>> ---
>>   drivers/usb/host/uhci-pci.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
>> index 3592f757fe05..177e3c2aa287 100644
>> --- a/drivers/usb/host/uhci-pci.c
>> +++ b/drivers/usb/host/uhci-pci.c
>> @@ -126,6 +126,10 @@ static int uhci_pci_init(struct usb_hcd *hcd)
>>   	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
>>   		uhci->oc_low = 1;
>>   
>> +	/* ZHAOXIN controllers report OverCurrent bit active off. */
>> +	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
>> +		uhci->oc_low = 1;
>> +
> 
> This should also go to the stable kernels, right?
> 
> But a new UHCI controller?  And this affects all of them? 
All UHCI controller of zhaoxin have this issue, And Hope this
patch go to stable kernel.Could I resubmit this patch with
modify subject typo mistake?

> Intel an issue here (as you mention in the changelog text)?
> 
I got this info from uhci driver comments in function uhci_pci_init.

"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.)"

Best Regards,
weitao

> thanks,
> 
> greg k-h
> .

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value
  2023-04-20 14:49 ` Alan Stern
@ 2023-04-21 10:59   ` WeitaoWang-oc
  0 siblings, 0 replies; 6+ messages in thread
From: WeitaoWang-oc @ 2023-04-21 10:59 UTC (permalink / raw)
  To: Alan Stern; +Cc: gregkh, linux-usb, linux-kernel, tonywwang, weitaowang

On 2023/4/20 22:49, Alan Stern wrote:
> On Thu, Apr 20, 2023 at 07:14:45PM +0800, Weitao Wang wrote:
>> Over Current condition is not standardized in the UHCI spec.
>> Zhaoxin UHCI controllers report OverCurrent bit active off.
>> Intel controllers report it active on, so we'll adjust the bit value.
> 
> The last sentence is irrelevant.  You should say instead that the
> uhci-hcd driver needs to be told to expect the active-off behavior.

Okay, I'll change this description.Thanks!

>>
>> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
>> ---
>>   drivers/usb/host/uhci-pci.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/usb/host/uhci-pci.c b/drivers/usb/host/uhci-pci.c
>> index 3592f757fe05..177e3c2aa287 100644
>> --- a/drivers/usb/host/uhci-pci.c
>> +++ b/drivers/usb/host/uhci-pci.c
>> @@ -126,6 +126,10 @@ static int uhci_pci_init(struct usb_hcd *hcd)
>>   	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_VIA)
>>   		uhci->oc_low = 1;
>>   
>> +	/* ZHAOXIN controllers report OverCurrent bit active off. */
>> +	if (to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
>> +		uhci->oc_low = 1;
> 
> This is really just a matter of taste, but IMO it would be better to
> combine this comment and test with the preceding one.  Something like:

This advice is good and the code looks more concise.
I'll change these code and description in next patch version.

Thanks,
Weitao
> 	/*
> 	 * 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 ||
> 	    to_pci_dev(uhci_dev(uhci))->vendor == PCI_VENDOR_ID_ZHAOXIN)
> 		uhci->oc_low = 1;
> 
> Alan Stern
> .

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-04-21  3:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-20 11:14 [PATCH] UHCI:adjust zhaoxin UHCI controllers OverCurrent bit value Weitao Wang
2023-04-20  8:05 ` Greg KH
2023-04-20 17:14   ` WeitaoWang-oc
2023-04-20  9:23     ` Greg KH
2023-04-20 14:49 ` Alan Stern
2023-04-21 10:59   ` WeitaoWang-oc

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).