usb: dwc3: core: Set GUCTL1[29] to ignore single SE0 glitch for FS/LS operation

[PATCH] usb: dwc3: core: Set GUCTL1[29] to ignore single SE0 glitch for FS/LS operation

Posted by Uttkarsh Aggarwal 1 year, 5 months ago

Currently in few of Qualcomm chips USB LS mouse not detected
showing following errors:

[ 81.585361] usb 1-1: Device not responding to setup address.
[ 81.798676] usb 1-1: device not accepting address 2, error -71
[ 81.928378] usb 1-1: new low-speed USB device number 3 using xhci-hcd
[ 82.076248] usb 1-1: Device not responding to setup address.
[ 82.290630] usb 1-1: Device not responding to setup address.
[ 82.503877] usb 1-1: device not accepting address 3, error -71
[ 82.512548] usb usb1-port1: attempt power cycle

Based on the Logic analyzer waveforms, It has been identified that
there is skew of about 8nS b/w DP & DM linestate signals
(o/p of PHY & i/p to controller) at the UTMI interface,
Due to this controller is seeing SE0 glitch condition,
this is causing controller to pre-maturely assume that PHY
has sent all the data & is initiating next packet much early,
though in reality PHY is still busy sending previous packets.

Enabling the GUCTL1.FILTER_SE0_FSLS_EOP bit29 allows the controller
to ignore single SE0 glitches on the linestate during transmission.
Only two or more SE0 signals are recognized as a valid EOP.

When this feature is activated, SE0 signals on the linestate
are validated over two consecutive UTMI/ULPI clock edges for EOP detection.

Device mode (FS): If GUCTL1.FILTER_SE0_FSLS_EOP is set,
then for device LPM handshake, the controller ignores single
SE0 glitch on the linestate during transmit. Only two or more
SE0 is considered as a valid EOP on FS port.

Host mode (FS/LS): If GUCTL1.FILTER_SE0_FSLS_EOP is set, then
the controller ignores single SE0 glitch on the linestate during transmit.
Only two or more SE0 is considered as a valid EOP on FS/LS port.

Signed-off-by: Uttkarsh Aggarwal <quic_uaggarwa@quicinc.com>
---
 drivers/usb/dwc3/core.c | 10 ++++++++++
 drivers/usb/dwc3/core.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 3e55838c0001..1edb16d2940c 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -194,6 +194,16 @@ static void __dwc3_set_mode(struct work_struct *work)
 
 	switch (desired_dr_role) {
 	case DWC3_GCTL_PRTCAP_HOST:
+
+		/*
+		 * Setting GUCTL1 bit 29 so that controller
+		 * will ignore single SE0 glitch on the linestate
+		 * during transmit.
+		 */
+		reg = dwc3_readl(dwc->regs, DWC3_GUCTL1);
+		reg |= DWC3_GUCTL1_FILTER_SE0_FSLS_EOP;
+		dwc3_writel(dwc->regs, DWC3_GUCTL1, reg);
+
 		ret = dwc3_host_init(dwc);
 		if (ret) {
 			dev_err(dwc->dev, "failed to initialize host\n");
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index e3eea965e57b..73e90daafecf 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -271,6 +271,7 @@
 #define DWC3_GUCTL1_PARKMODE_DISABLE_SS		BIT(17)
 #define DWC3_GUCTL1_PARKMODE_DISABLE_HS		BIT(16)
 #define DWC3_GUCTL1_RESUME_OPMODE_HS_HOST	BIT(10)
+#define DWC3_GUCTL1_FILTER_SE0_FSLS_EOP		BIT(29)
 
 /* Global Status Register */
 #define DWC3_GSTS_OTG_IP	BIT(10)
-- 
2.17.1

Re: [PATCH] usb: dwc3: core: Set GUCTL1[29] to ignore single SE0 glitch for FS/LS operation

Posted by Thinh Nguyen 1 year, 5 months ago

On Fri, Aug 23, 2024, Uttkarsh Aggarwal wrote:
> Currently in few of Qualcomm chips USB LS mouse not detected
> showing following errors:
> 
> [ 81.585361] usb 1-1: Device not responding to setup address.
> [ 81.798676] usb 1-1: device not accepting address 2, error -71
> [ 81.928378] usb 1-1: new low-speed USB device number 3 using xhci-hcd
> [ 82.076248] usb 1-1: Device not responding to setup address.
> [ 82.290630] usb 1-1: Device not responding to setup address.
> [ 82.503877] usb 1-1: device not accepting address 3, error -71
> [ 82.512548] usb usb1-port1: attempt power cycle
> 
> Based on the Logic analyzer waveforms, It has been identified that
> there is skew of about 8nS b/w DP & DM linestate signals
> (o/p of PHY & i/p to controller) at the UTMI interface,
> Due to this controller is seeing SE0 glitch condition,
> this is causing controller to pre-maturely assume that PHY
> has sent all the data & is initiating next packet much early,
> though in reality PHY is still busy sending previous packets.
> 
> Enabling the GUCTL1.FILTER_SE0_FSLS_EOP bit29 allows the controller
> to ignore single SE0 glitches on the linestate during transmission.
> Only two or more SE0 signals are recognized as a valid EOP.
> 
> When this feature is activated, SE0 signals on the linestate
> are validated over two consecutive UTMI/ULPI clock edges for EOP detection.
> 
> Device mode (FS): If GUCTL1.FILTER_SE0_FSLS_EOP is set,
> then for device LPM handshake, the controller ignores single
> SE0 glitch on the linestate during transmit. Only two or more
> SE0 is considered as a valid EOP on FS port.
> 
> Host mode (FS/LS): If GUCTL1.FILTER_SE0_FSLS_EOP is set, then
> the controller ignores single SE0 glitch on the linestate during transmit.
> Only two or more SE0 is considered as a valid EOP on FS/LS port.
> 
> Signed-off-by: Uttkarsh Aggarwal <quic_uaggarwa@quicinc.com>
> ---
>  drivers/usb/dwc3/core.c | 10 ++++++++++
>  drivers/usb/dwc3/core.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 3e55838c0001..1edb16d2940c 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -194,6 +194,16 @@ static void __dwc3_set_mode(struct work_struct *work)
>  
>  	switch (desired_dr_role) {
>  	case DWC3_GCTL_PRTCAP_HOST:
> +
> +		/*
> +		 * Setting GUCTL1 bit 29 so that controller
> +		 * will ignore single SE0 glitch on the linestate
> +		 * during transmit.
> +		 */
> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL1);
> +		reg |= DWC3_GUCTL1_FILTER_SE0_FSLS_EOP;
> +		dwc3_writel(dwc->regs, DWC3_GUCTL1, reg);
> +
>  		ret = dwc3_host_init(dwc);
>  		if (ret) {
>  			dev_err(dwc->dev, "failed to initialize host\n");
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index e3eea965e57b..73e90daafecf 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -271,6 +271,7 @@
>  #define DWC3_GUCTL1_PARKMODE_DISABLE_SS		BIT(17)
>  #define DWC3_GUCTL1_PARKMODE_DISABLE_HS		BIT(16)
>  #define DWC3_GUCTL1_RESUME_OPMODE_HS_HOST	BIT(10)
> +#define DWC3_GUCTL1_FILTER_SE0_FSLS_EOP		BIT(29)
>  
>  /* Global Status Register */
>  #define DWC3_GSTS_OTG_IP	BIT(10)
> -- 
> 2.17.1
> 

This is your platform specific quirk. Please add proper checks to target
your platform only.

Thanks,
Thinh

Re: [PATCH] usb: dwc3: core: Set GUCTL1[29] to ignore single SE0 glitch for FS/LS operation

Posted by UTTKARSH AGGARWAL 1 year, 5 months ago

On 8/28/2024 7:41 AM, Thinh Nguyen wrote:
> On Fri, Aug 23, 2024, Uttkarsh Aggarwal wrote:
>> Currently in few of Qualcomm chips USB LS mouse not detected
>> showing following errors:
>>
>> [ 81.585361] usb 1-1: Device not responding to setup address.
>> [ 81.798676] usb 1-1: device not accepting address 2, error -71
>> [ 81.928378] usb 1-1: new low-speed USB device number 3 using xhci-hcd
>> [ 82.076248] usb 1-1: Device not responding to setup address.
>> [ 82.290630] usb 1-1: Device not responding to setup address.
>> [ 82.503877] usb 1-1: device not accepting address 3, error -71
>> [ 82.512548] usb usb1-port1: attempt power cycle
>>
>> Based on the Logic analyzer waveforms, It has been identified that
>> there is skew of about 8nS b/w DP & DM linestate signals
>> (o/p of PHY & i/p to controller) at the UTMI interface,
>> Due to this controller is seeing SE0 glitch condition,
>> this is causing controller to pre-maturely assume that PHY
>> has sent all the data & is initiating next packet much early,
>> though in reality PHY is still busy sending previous packets.
>>
>> Enabling the GUCTL1.FILTER_SE0_FSLS_EOP bit29 allows the controller
>> to ignore single SE0 glitches on the linestate during transmission.
>> Only two or more SE0 signals are recognized as a valid EOP.
>>
>> When this feature is activated, SE0 signals on the linestate
>> are validated over two consecutive UTMI/ULPI clock edges for EOP detection.
>>
>> Device mode (FS): If GUCTL1.FILTER_SE0_FSLS_EOP is set,
>> then for device LPM handshake, the controller ignores single
>> SE0 glitch on the linestate during transmit. Only two or more
>> SE0 is considered as a valid EOP on FS port.
>>
>> Host mode (FS/LS): If GUCTL1.FILTER_SE0_FSLS_EOP is set, then
>> the controller ignores single SE0 glitch on the linestate during transmit.
>> Only two or more SE0 is considered as a valid EOP on FS/LS port.
>>
>> Signed-off-by: Uttkarsh Aggarwal <quic_uaggarwa@quicinc.com>
>> ---
>>   drivers/usb/dwc3/core.c | 10 ++++++++++
>>   drivers/usb/dwc3/core.h |  1 +
>>   2 files changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 3e55838c0001..1edb16d2940c 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -194,6 +194,16 @@ static void __dwc3_set_mode(struct work_struct *work)
>>   
>>   	switch (desired_dr_role) {
>>   	case DWC3_GCTL_PRTCAP_HOST:
>> +
>> +		/*
>> +		 * Setting GUCTL1 bit 29 so that controller
>> +		 * will ignore single SE0 glitch on the linestate
>> +		 * during transmit.
>> +		 */
>> +		reg = dwc3_readl(dwc->regs, DWC3_GUCTL1);
>> +		reg |= DWC3_GUCTL1_FILTER_SE0_FSLS_EOP;
>> +		dwc3_writel(dwc->regs, DWC3_GUCTL1, reg);
>> +
>>   		ret = dwc3_host_init(dwc);
>>   		if (ret) {
>>   			dev_err(dwc->dev, "failed to initialize host\n");
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
>> index e3eea965e57b..73e90daafecf 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -271,6 +271,7 @@
>>   #define DWC3_GUCTL1_PARKMODE_DISABLE_SS		BIT(17)
>>   #define DWC3_GUCTL1_PARKMODE_DISABLE_HS		BIT(16)
>>   #define DWC3_GUCTL1_RESUME_OPMODE_HS_HOST	BIT(10)
>> +#define DWC3_GUCTL1_FILTER_SE0_FSLS_EOP		BIT(29)
>>   
>>   /* Global Status Register */
>>   #define DWC3_GSTS_OTG_IP	BIT(10)
>> -- 
>> 2.17.1
>>
> This is your platform specific quirk. Please add proper checks to target
> your platform only.
>
> Thanks,
> Thinh

Thanks Thinh for the comments. I will push v2 with a quirk check.

Thanks,

Uttkarsh