Re: [PATCH] usb: cdns3: exit cdns3_gadget_udc_start if FAST_REG_ACCESS cannot be set

[Peter Chen <peter.chen@kernel.org>]

On 25-02-06 18:29:36, Siddharth Vadapalli wrote:
> When the device is in a low power state, access to the following
> registers takes a long time:
> - EP_CFG
> - EP_TRADDR
> - EP_CMD
> - EP_SEL
> - EP_STS
> - USB_CONF
> 
> To address this, the fast register access feature can be enabled by
> setting PUSB_PWR_FST_REG_ACCESS bit of the USB_PWR register, which
> allows quick access by software. Software is expected to poll on
> PUSB_PWR_FST_REG_ACCESS_STAT to ensure that fast register access has
> been enabled by the controller. Attempting to access any of the
> aforementioned registers after setting PUSB_PWR_FST_REG_ACCESS but
> before PUSB_PWR_FST_REG_ACCESS_STAT has been set will result in
> undefined behavior and potentially result in system hang.
> 
> Hence, poll on PUSB_PWR_FST_REG_ACCESS_STAT before proceeding with
> gadget configuration, and exit if it cannot be enabled.
> 
> Fixes: b5148d946f45 ("usb: cdns3: gadget: set fast access bit")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> Hello,
> 
> This patch is based on commit
> 92514ef226f5 Merge tag 'for-6.14-rc1-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux
> of Mainline Linux.
> 
> Regards,
> Siddharth.
> 
>  drivers/usb/cdns3/cdns3-gadget.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
> index fd1beb10bba7..b62691944272 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c
> @@ -2971,8 +2971,6 @@ static void cdns3_gadget_config(struct cdns3_device *priv_dev)
>  	/* enable generic interrupt*/
>  	writel(USB_IEN_INIT, &regs->usb_ien);
>  	writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, &regs->usb_conf);
> -	/*  keep Fast Access bit */
> -	writel(PUSB_PWR_FST_REG_ACCESS, &priv_dev->regs->usb_pwr);
>  
>  	cdns3_configure_dmult(priv_dev, NULL);
>  }
> @@ -2990,6 +2988,8 @@ static int cdns3_gadget_udc_start(struct usb_gadget *gadget,
>  	struct cdns3_device *priv_dev = gadget_to_cdns3_device(gadget);
>  	unsigned long flags;
>  	enum usb_device_speed max_speed = driver->max_speed;
> +	int ret;
> +	u32 reg;
>  
>  	spin_lock_irqsave(&priv_dev->lock, flags);
>  	priv_dev->gadget_driver = driver;
> @@ -2997,6 +2997,20 @@ static int cdns3_gadget_udc_start(struct usb_gadget *gadget,
>  	/* limit speed if necessary */
>  	max_speed = min(driver->max_speed, gadget->max_speed);
>  
> +	/*  keep Fast Access bit */
> +	writel(PUSB_PWR_FST_REG_ACCESS, &priv_dev->regs->usb_pwr);
> +	reg = readl(&priv_dev->regs->usb_pwr);
> +	if (!(reg & PUSB_PWR_FST_REG_ACCESS_STAT)) {
> +		ret = readl_poll_timeout_atomic(&priv_dev->regs->usb_pwr, reg,
> +						(reg & PUSB_PWR_FST_REG_ACCESS_STAT),
> +						10, 1000);
> +		if (ret) {
> +			dev_err(priv_dev->dev, "Failed to enable fast access\n");
> +			spin_unlock_irqrestore(&priv_dev->lock, flags);
> +			return ret;
> +		}
> +	}
> +
>  	switch (max_speed) {
>  	case USB_SPEED_FULL:
>  		writel(USB_CONF_SFORCE_FS, &priv_dev->regs->usb_conf);

Hi Siddharth,

Would you please keep this change at cdns3_gadget_config in case the
controller is power lost during the system suspend?

Peter