Re: [PATCH 4/6] usb: dwc3: gadget: Check ENBLSLPM before sending ep command

[Mattijs Korpershoek <mkorpershoek@baylibre.com>]

Hi Alexander,

Thank you for the patch.

On ven., avril 12, 2024 at 22:26, "A. Sverdlin" <alexander.sverdlin@siemens.com> wrote:

> From: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
>
> Upstream Linux commit 87dd96111b0b.
>
> When operating in USB 2.0 speeds (HS/FS), if GUSB2PHYCFG.ENBLSLPM or
> GUSB2PHYCFG.SUSPHY is set, it must be cleared before issuing an endpoint
> command.
>
> Current implementation only save and restore GUSB2PHYCFG.SUSPHY
> configuration. We must save and clear both GUSB2PHYCFG.ENBLSLPM and
> GUSB2PHYCFG.SUSPHY settings. Restore them after the command is
> completed.
>
> DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section 3.2.2
>
> Signed-off-by: Thinh Nguyen <thinhn@synopsys.com>
> Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@siemens.com>

Reviewed-by: Mattijs Korpershoek <mkorpershoek@baylibre.com>

> ---
>  drivers/usb/dwc3/gadget.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 00845dbadd27a..c14d7870b9461 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -301,26 +301,35 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
>  		unsigned cmd, struct dwc3_gadget_ep_cmd_params *params)
>  {
>  	u32			timeout = 500;
> +	u32			saved_config = 0;
>  	u32			reg;
>  
> -	int			susphy = false;
>  	int			ret = -EINVAL;
>  
>  	/*
> -	 * Synopsys Databook 2.60a states, on section 6.3.2.5.[1-8], that if
> -	 * we're issuing an endpoint command, we must check if
> -	 * GUSB2PHYCFG.SUSPHY bit is set. If it is, then we need to clear it.
> +	 * When operating in USB 2.0 speeds (HS/FS), if GUSB2PHYCFG.ENBLSLPM or
> +	 * GUSB2PHYCFG.SUSPHY is set, it must be cleared before issuing an
> +	 * endpoint command.
>  	 *
> -	 * We will also set SUSPHY bit to what it was before returning as stated
> -	 * by the same section on Synopsys databook.
> +	 * Save and clear both GUSB2PHYCFG.ENBLSLPM and GUSB2PHYCFG.SUSPHY
> +	 * settings. Restore them after the command is completed.
> +	 *
> +	 * DWC_usb3 3.30a and DWC_usb31 1.90a programming guide section 3.2.2
>  	 */
>  	if (dwc->gadget.speed <= USB_SPEED_HIGH) {
>  		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
>  		if (unlikely(reg & DWC3_GUSB2PHYCFG_SUSPHY)) {
> -			susphy = true;
> +			saved_config |= DWC3_GUSB2PHYCFG_SUSPHY;
>  			reg &= ~DWC3_GUSB2PHYCFG_SUSPHY;
> -			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>  		}
> +
> +		if (reg & DWC3_GUSB2PHYCFG_ENBLSLPM) {
> +			saved_config |= DWC3_GUSB2PHYCFG_ENBLSLPM;
> +			reg &= ~DWC3_GUSB2PHYCFG_ENBLSLPM;
> +		}
> +
> +		if (saved_config)
> +			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>  	}
>  
>  	dwc3_writel(dwc->regs, DWC3_DEPCMDPAR0(ep), params->param0);
> @@ -350,9 +359,9 @@ int dwc3_send_gadget_ep_cmd(struct dwc3 *dwc, unsigned ep,
>  		udelay(1);
>  	} while (1);
>  
> -	if (unlikely(susphy)) {
> +	if (saved_config) {
>  		reg = dwc3_readl(dwc->regs, DWC3_GUSB2PHYCFG(0));
> -		reg |= DWC3_GUSB2PHYCFG_SUSPHY;
> +		reg |= saved_config;
>  		dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
>  	}
>  
> -- 
> 2.44.0