linux-kernel - Re: [PATCH 2/2] usb: dwc3: gadget: Preserve UDC max speed setting

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: <e6faade6-7c4c-5966-3afd-63be2deefa01@synopsys.com>
Date:   Thu, 29 Oct 2020 00:43:21 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Wesley Cheng <wcheng@...eaurora.org>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "jackp@...eaurora.org" <jackp@...eaurora.org>
Subject: Re: [PATCH 2/2] usb: dwc3: gadget: Preserve UDC max speed setting

Hi,

Wesley Cheng wrote:
> The USB gadget/UDC driver can restrict the DWC3 controller speed using
> dwc3_gadget_set_speed().  Store this setting into a variable, in order for
> this setting to persist across controller resets due to runtime PM.

Why do we need to do this? DCFG should persist unless we overwrite it.
The current PM shouldn't update the current speed setting.

BR,
Thinh

> Signed-off-by: Wesley Cheng <wcheng@...eaurora.org>
> ---
>  drivers/usb/dwc3/core.h   |   1 +
>  drivers/usb/dwc3/gadget.c | 108 ++++++++++++++++++++------------------
>  2 files changed, 58 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 2f04b3e42bf1..390d3deef0ba 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -1119,6 +1119,7 @@ struct dwc3 {
>  	u32			nr_scratch;
>  	u32			u1u2;
>  	u32			maximum_speed;
> +	u32			gadget_max_speed;
>  
>  	u32			ip;
>  
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 6364429b2122..1a93b92a5e6f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1941,6 +1941,61 @@ static void dwc3_stop_active_transfers(struct dwc3 *dwc)
>  	}
>  }
>  
> +static void __dwc3_gadget_set_speed(struct dwc3 *dwc)
> +{
> +	u32			reg;
> +
> +	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
> +	reg &= ~(DWC3_DCFG_SPEED_MASK);
> +
> +	/*
> +	 * WORKAROUND: DWC3 revision < 2.20a have an issue
> +	 * which would cause metastability state on Run/Stop
> +	 * bit if we try to force the IP to USB2-only mode.
> +	 *
> +	 * Because of that, we cannot configure the IP to any
> +	 * speed other than the SuperSpeed
> +	 *
> +	 * Refers to:
> +	 *
> +	 * STAR#9000525659: Clock Domain Crossing on DCTL in
> +	 * USB 2.0 Mode
> +	 */
> +	if (DWC3_VER_IS_PRIOR(DWC3, 220A) &&
> +	    !dwc->dis_metastability_quirk) {
> +		reg |= DWC3_DCFG_SUPERSPEED;
> +	} else {
> +		switch (dwc->gadget_max_speed) {
> +		case USB_SPEED_LOW:
> +			reg |= DWC3_DCFG_LOWSPEED;
> +			break;
> +		case USB_SPEED_FULL:
> +			reg |= DWC3_DCFG_FULLSPEED;
> +			break;
> +		case USB_SPEED_HIGH:
> +			reg |= DWC3_DCFG_HIGHSPEED;
> +			break;
> +		case USB_SPEED_SUPER:
> +			reg |= DWC3_DCFG_SUPERSPEED;
> +			break;
> +		case USB_SPEED_SUPER_PLUS:
> +			if (DWC3_IP_IS(DWC3))
> +				reg |= DWC3_DCFG_SUPERSPEED;
> +			else
> +				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
> +			break;
> +		default:
> +			dev_err(dwc->dev, "invalid speed (%d)\n", dwc->gadget_max_speed);
> +
> +			if (DWC3_IP_IS(DWC3))
> +				reg |= DWC3_DCFG_SUPERSPEED;
> +			else
> +				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
> +		}
> +	}
> +	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> +}
> +
>  static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>  {
>  	u32			reg;
> @@ -1963,6 +2018,7 @@ static int dwc3_gadget_run_stop(struct dwc3 *dwc, int is_on, int suspend)
>  		if (dwc->has_hibernation)
>  			reg |= DWC3_DCTL_KEEP_CONNECT;
>  
> +		__dwc3_gadget_set_speed(dwc);
>  		dwc->pullups_connected = true;
>  	} else {
>  		reg &= ~DWC3_DCTL_RUN_STOP;
> @@ -2318,59 +2374,9 @@ static void dwc3_gadget_set_speed(struct usb_gadget *g,
>  {
>  	struct dwc3		*dwc = gadget_to_dwc(g);
>  	unsigned long		flags;
> -	u32			reg;
>  
>  	spin_lock_irqsave(&dwc->lock, flags);
> -	reg = dwc3_readl(dwc->regs, DWC3_DCFG);
> -	reg &= ~(DWC3_DCFG_SPEED_MASK);
> -
> -	/*
> -	 * WORKAROUND: DWC3 revision < 2.20a have an issue
> -	 * which would cause metastability state on Run/Stop
> -	 * bit if we try to force the IP to USB2-only mode.
> -	 *
> -	 * Because of that, we cannot configure the IP to any
> -	 * speed other than the SuperSpeed
> -	 *
> -	 * Refers to:
> -	 *
> -	 * STAR#9000525659: Clock Domain Crossing on DCTL in
> -	 * USB 2.0 Mode
> -	 */
> -	if (DWC3_VER_IS_PRIOR(DWC3, 220A) &&
> -	    !dwc->dis_metastability_quirk) {
> -		reg |= DWC3_DCFG_SUPERSPEED;
> -	} else {
> -		switch (speed) {
> -		case USB_SPEED_LOW:
> -			reg |= DWC3_DCFG_LOWSPEED;
> -			break;
> -		case USB_SPEED_FULL:
> -			reg |= DWC3_DCFG_FULLSPEED;
> -			break;
> -		case USB_SPEED_HIGH:
> -			reg |= DWC3_DCFG_HIGHSPEED;
> -			break;
> -		case USB_SPEED_SUPER:
> -			reg |= DWC3_DCFG_SUPERSPEED;
> -			break;
> -		case USB_SPEED_SUPER_PLUS:
> -			if (DWC3_IP_IS(DWC3))
> -				reg |= DWC3_DCFG_SUPERSPEED;
> -			else
> -				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
> -			break;
> -		default:
> -			dev_err(dwc->dev, "invalid speed (%d)\n", speed);
> -
> -			if (DWC3_IP_IS(DWC3))
> -				reg |= DWC3_DCFG_SUPERSPEED;
> -			else
> -				reg |= DWC3_DCFG_SUPERSPEED_PLUS;
> -		}
> -	}
> -	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> -
> +	dwc->gadget_max_speed = speed;
>  	spin_unlock_irqrestore(&dwc->lock, flags);
>  }
>