Re: [PATCH] usb: cdnsp: Fix issue with resuming from L1

["gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>]

On Thu, Apr 10, 2025 at 07:34:16AM +0000, Pawel Laszczak wrote:
> Subject: [PATCH] usb: cdnsp: Fix issue with resuming from L1

Why is the subject line duplicated here?  Can you fix up your git
send-email process to not do that?

> In very rare cases after resuming controller from L1 to L0 it reads
> registers before the clock has been enabled and as the result driver
> reads incorrect value.
> To fix this issue driver increases APB timeout value.
> 
> Probably this issue occurs only on Cadence platform but fix
> should have no impact for other existing platforms.

If this is the case, shouldn't you just handle this for Cadence-specific
hardware and add the check for that to this change?

> 
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> cc: stable@vger.kernel.org
> Signed-off-by: Pawel Laszczak <pawell@cadence.com>
> ---
>  drivers/usb/cdns3/cdnsp-gadget.c | 22 ++++++++++++++++++++++
>  drivers/usb/cdns3/cdnsp-gadget.h |  4 ++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index 87f310841735..b12581b94567 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -139,6 +139,21 @@ static void cdnsp_clear_port_change_bit(struct cdnsp_device *pdev,
>  	       (portsc & PORT_CHANGE_BITS), port_regs);
>  }
>  
> +static void cdnsp_set_apb_timeout_value(struct cdnsp_device *pdev)
> +{
> +	__le32 __iomem *reg;
> +	void __iomem *base;
> +	u32 offset = 0;
> +	u32 val;
> +
> +	base = &pdev->cap_regs->hc_capbase;
> +	offset = cdnsp_find_next_ext_cap(base, offset, D_XEC_PRE_REGS_CAP);
> +	reg = base + offset + REG_CHICKEN_BITS_3_OFFSET;
> +
> +	val  = le32_to_cpu(readl(reg));
> +	writel(cpu_to_le32(CHICKEN_APB_TIMEOUT_SET(val)), reg);

Do you need to do a read to ensure that the write is flushed to the
device before continuing?

> +}
> +
>  static void cdnsp_set_chicken_bits_2(struct cdnsp_device *pdev, u32 bit)
>  {
>  	__le32 __iomem *reg;
> @@ -1798,6 +1813,13 @@ static int cdnsp_gen_setup(struct cdnsp_device *pdev)
>  	pdev->hci_version = HC_VERSION(pdev->hcc_params);
>  	pdev->hcc_params = readl(&pdev->cap_regs->hcc_params);
>  
> +	/* In very rare cases after resuming controller from L1 to L0 it reads
> +	 * registers before the clock has been enabled and as the result driver
> +	 * reads incorrect value.
> +	 * To fix this issue driver increases APB timeout value.
> +	 */

Nit, please use the "normal" kernel comment style.

thanks,

greg k-h