linux-kernel - RE: [PATCH] usb: cdnsp: Fix issue with resuming from L1

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:
 <PH7PR07MB9538B41AA7F38F515DD402F4DDB62@PH7PR07MB9538.namprd07.prod.outlook.com>
Date: Fri, 11 Apr 2025 09:48:40 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC: "peter.chen@...nel.org" <peter.chen@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] usb: cdnsp: Fix issue with resuming from L1



>-----Original Message-----
>From: Pawel Laszczak
>Sent: Friday, April 11, 2025 11:40 AM
>To: gregkh@...uxfoundation.org
>Cc: peter.chen@...nel.org; linux-usb@...r.kernel.org; linux-
>kernel@...r.kernel.org; stable@...r.kernel.org
>Subject: RE: [PATCH] usb: cdnsp: Fix issue with resuming from L1
>
>>
>>
>>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?
>
>This fix will not have negative impact for other platforms, but I'm not sure
>whether other platforms are free from this issue.
>It is very hard to recreate and debug this issue.

Sorry, you have right should be platform specific. Other platform with Cadence
USB controller may require this time even longer.

>
>Thanks,
>Pawel
>>
>>>
>>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
>>> USBSSP DRD Driver")
>>> cc: stable@...r.kernel.org
>>> Signed-off-by: Pawel Laszczak <pawell@...ence.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