diff mbox series

xhci: tegra: Delay for disabling LFPS detector

Message ID 20201218164234.128762-1-jckuo@nvidia.com (mailing list archive)
State Superseded
Headers show
Series xhci: tegra: Delay for disabling LFPS detector | expand

Commit Message

JC Kuo Dec. 18, 2020, 4:42 p.m. UTC
Occasionally, we are seeing some SuperSpeed devices resumes right after
being directed to U3. This commits add 500us delay to ensure LFPS
detector is disabled before sending ACK to firmware.

[   16.099363] tegra-xusb 70090000.usb: entering ELPG
[   16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
[   16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
[   16.120789] tegra-xusb 70090000.usb: entering ELPG failed

Signed-off-by: JC Kuo <jckuo@nvidia.com>
---
 drivers/usb/host/xhci-tegra.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Greg Kroah-Hartman Dec. 18, 2020, 4:53 p.m. UTC | #1
On Sat, Dec 19, 2020 at 12:42:34AM +0800, JC Kuo wrote:
> Occasionally, we are seeing some SuperSpeed devices resumes right after
> being directed to U3. This commits add 500us delay to ensure LFPS
> detector is disabled before sending ACK to firmware.
> 
> [   16.099363] tegra-xusb 70090000.usb: entering ELPG
> [   16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
> [   16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
> [   16.120789] tegra-xusb 70090000.usb: entering ELPG failed
> 
> Signed-off-by: JC Kuo <jckuo@nvidia.com>
> ---
>  drivers/usb/host/xhci-tegra.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
> index 934be1686352..20cdc11f7dc6 100644
> --- a/drivers/usb/host/xhci-tegra.c
> +++ b/drivers/usb/host/xhci-tegra.c
> @@ -623,6 +623,12 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
>  								     enable);
>  			if (err < 0)
>  				break;
> +
> +			/*
> +			 * wait 500us for LFPS detector to be disabled before sending ACK
> +			 */
> +			if (!enable)
> +				usleep_range(500, 1000);

Where does the magic 500us come from?  How can we "know" this is long
enough?

thanks,

greg k-h
JC Kuo Jan. 6, 2021, 6:05 a.m. UTC | #2
On 12/19/20 12:53 AM, Greg KH wrote:
> On Sat, Dec 19, 2020 at 12:42:34AM +0800, JC Kuo wrote:
>> Occasionally, we are seeing some SuperSpeed devices resumes right after
>> being directed to U3. This commits add 500us delay to ensure LFPS
>> detector is disabled before sending ACK to firmware.
>>
>> [   16.099363] tegra-xusb 70090000.usb: entering ELPG
>> [   16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
>> [   16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
>> [   16.120789] tegra-xusb 70090000.usb: entering ELPG failed
>>
>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>> ---
>>  drivers/usb/host/xhci-tegra.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>> index 934be1686352..20cdc11f7dc6 100644
>> --- a/drivers/usb/host/xhci-tegra.c
>> +++ b/drivers/usb/host/xhci-tegra.c
>> @@ -623,6 +623,12 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
>>  								     enable);
>>  			if (err < 0)
>>  				break;
>> +
>> +			/*
>> +			 * wait 500us for LFPS detector to be disabled before sending ACK
>> +			 */
>> +			if (!enable)
>> +				usleep_range(500, 1000);
> 
> Where does the magic 500us come from?  How can we "know" this is long
> enough?

Hi Greg,
The register write passes through a few flop stages of 32KHz clock domain. Our
ASIC designer reviewed RTL and suggests 500us delay. It has also been verified
thoroughly.

Thanks,
JC

> 
> thanks,
> 
> greg k-h
>
Mathias Nyman Jan. 7, 2021, 9:53 a.m. UTC | #3
On 6.1.2021 8.05, JC Kuo wrote:
> On 12/19/20 12:53 AM, Greg KH wrote:
>> On Sat, Dec 19, 2020 at 12:42:34AM +0800, JC Kuo wrote:
>>> Occasionally, we are seeing some SuperSpeed devices resumes right after
>>> being directed to U3. This commits add 500us delay to ensure LFPS
>>> detector is disabled before sending ACK to firmware.
>>>
>>> [   16.099363] tegra-xusb 70090000.usb: entering ELPG
>>> [   16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
>>> [   16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
>>> [   16.120789] tegra-xusb 70090000.usb: entering ELPG failed
>>>
>>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>>> ---
>>>  drivers/usb/host/xhci-tegra.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>>> index 934be1686352..20cdc11f7dc6 100644
>>> --- a/drivers/usb/host/xhci-tegra.c
>>> +++ b/drivers/usb/host/xhci-tegra.c
>>> @@ -623,6 +623,12 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
>>>  								     enable);
>>>  			if (err < 0)
>>>  				break;
>>> +
>>> +			/*
>>> +			 * wait 500us for LFPS detector to be disabled before sending ACK
>>> +			 */
>>> +			if (!enable)
>>> +				usleep_range(500, 1000);
>>
>> Where does the magic 500us come from?  How can we "know" this is long
>> enough?
> 
> Hi Greg,
> The register write passes through a few flop stages of 32KHz clock domain. Our
> ASIC designer reviewed RTL and suggests 500us delay. It has also been verified
> thoroughly.
> 

Could you add that comment to the commit message? I can pick it up then.
Should this go to stable as well?

Thanks
-Mathias
JC Kuo Jan. 8, 2021, 6:32 a.m. UTC | #4
On 1/7/21 5:53 PM, Mathias Nyman wrote:
> On 6.1.2021 8.05, JC Kuo wrote:
>> On 12/19/20 12:53 AM, Greg KH wrote:
>>> On Sat, Dec 19, 2020 at 12:42:34AM +0800, JC Kuo wrote:
>>>> Occasionally, we are seeing some SuperSpeed devices resumes right after
>>>> being directed to U3. This commits add 500us delay to ensure LFPS
>>>> detector is disabled before sending ACK to firmware.
>>>>
>>>> [   16.099363] tegra-xusb 70090000.usb: entering ELPG
>>>> [   16.104343] tegra-xusb 70090000.usb: 2-1 isn't suspended: 0x0c001203
>>>> [   16.114576] tegra-xusb 70090000.usb: not all ports suspended: -16
>>>> [   16.120789] tegra-xusb 70090000.usb: entering ELPG failed
>>>>
>>>> Signed-off-by: JC Kuo <jckuo@nvidia.com>
>>>> ---
>>>>  drivers/usb/host/xhci-tegra.c | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
>>>> index 934be1686352..20cdc11f7dc6 100644
>>>> --- a/drivers/usb/host/xhci-tegra.c
>>>> +++ b/drivers/usb/host/xhci-tegra.c
>>>> @@ -623,6 +623,12 @@ static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
>>>>  								     enable);
>>>>  			if (err < 0)
>>>>  				break;
>>>> +
>>>> +			/*
>>>> +			 * wait 500us for LFPS detector to be disabled before sending ACK
>>>> +			 */
>>>> +			if (!enable)
>>>> +				usleep_range(500, 1000);
>>>
>>> Where does the magic 500us come from?  How can we "know" this is long
>>> enough?
>>
>> Hi Greg,
>> The register write passes through a few flop stages of 32KHz clock domain. Our
>> ASIC designer reviewed RTL and suggests 500us delay. It has also been verified
>> thoroughly.
>>
> 
> Could you add that comment to the commit message? I can pick it up then.
> Should this go to stable as well?
> 
Hi Mathias,
Yes, I will send v2 with the information in commit message.
Yes, this should go to stable as well.

Thanks,
JC

> Thanks
> -Mathias
>
diff mbox series

Patch

diff --git a/drivers/usb/host/xhci-tegra.c b/drivers/usb/host/xhci-tegra.c
index 934be1686352..20cdc11f7dc6 100644
--- a/drivers/usb/host/xhci-tegra.c
+++ b/drivers/usb/host/xhci-tegra.c
@@ -623,6 +623,12 @@  static void tegra_xusb_mbox_handle(struct tegra_xusb *tegra,
 								     enable);
 			if (err < 0)
 				break;
+
+			/*
+			 * wait 500us for LFPS detector to be disabled before sending ACK
+			 */
+			if (!enable)
+				usleep_range(500, 1000);
 		}
 
 		if (err < 0) {