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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a55aa38d-0fbf-4a95-a2b2-40821815275f@rowland.harvard.edu>
Date:   Fri, 21 Apr 2023 10:59:41 -0400
From:   Alan Stern <stern@...land.harvard.edu>
To:     Weitao Wang <WeitaoWang-oc@...oxin.com>
Cc:     gregkh@...uxfoundation.org, linux-usb@...r.kernel.org,
        linux-kernel@...r.kernel.org, tonywwang@...oxin.com,
        weitaowang@...oxin.com, stable@...r.kernel.org
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 <stern@...land.harvard.edu>
> Cc: stable@...r.kernel.org
> Signed-off-by: Weitao Wang <WeitaoWang-oc@...oxin.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
>