diff mbox series

[V2] usb: chipidea: udc: Add revision check of 2.20[CI_REVISION_22]

Message ID 20231102070603.777313-1-piyush.mehta@amd.com
State New
Headers show
Series [V2] usb: chipidea: udc: Add revision check of 2.20[CI_REVISION_22] | expand

Commit Message

Mehta, Piyush Nov. 2, 2023, 7:06 a.m. UTC
Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
revision 2.20a.

There is an issue with the add dTD tripwire semaphore (ATDTW bit in
USBCMD register) that can cause the controller to ignore a dTD that is
added to a primed endpoint. When this happens, the software can read
the tripwire bit and the status bit at '1' even though the endpoint is
unprimed.
This issue observed with the Windows host machine.

Workaround:
The software must implement a periodic cycle, and check for each dTD
pending on execution (Active = 1), if the endpoint is primed. It can do
this by reading the corresponding bits in the ENDPTPRIME and ENDPTSTAT
registers. If these bits are read at 0, the software needs to re-prime
the endpoint by writing 1 to the corresponding bit in the ENDPTPRIME
register.

Added conditional revision check of 2.20[CI_REVISION_22] along with 2.40.

Signed-off-by: Piyush Mehta <piyush.mehta@amd.com>
Acked-by: Peter Chen <peter.chen@kernel.org>
---
Change in V2:
- Addressed the Peter review comment - Update the subject line.
- Added Peter Ack in patch as Acked-by.
- Switch to new @amd.com to AMD/Xilinx acquisition.

Link: https://lore.kernel.org/all/1629825378-8089-7-git-send-email-manish.narani@xilinx.com
---
 drivers/usb/chipidea/udc.c | 3 ++-
 drivers/usb/chipidea/udc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Junzhong Pan March 6, 2025, 2:12 p.m. UTC | #1
Hi Piyush,

> Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
> revision 2.20a.
> 
> There is an issue with the add dTD tripwire semaphore (ATDTW bit in
> USBCMD register) that can cause the controller to ignore a dTD that is
> added to a primed endpoint. When this happens, the software can read
> the tripwire bit and the status bit at '1' even though the endpoint is
> unprimed.
> This issue observed with the Windows host machine.

Sorry to dig through this old patch email.

May I ask what does **the scene look like (bus condition/dtd status)**
when this issue observed with the Windows host machine occurs?

I want to know if it's relevant to an issue I encountered recently. 

I met this a tricky issue with an soc have a marvel udc (driven by
mv_udc_core.c, it's a same chipidea IP accordingly) when running rndis
gadget with the Windows host machine.

When the rndis gadget running for a long time (randomly), the udc would
suddenly stopped and no longger response to IN token sent from the host.

The bus trace looks like this:

BULK IN:
	IN
	DATA0 120 Bytes
	ACK
BULK OUT:
	OUT
	DATA1 98 Bytes
	NYET
BULK IN	??
	IN NAK
	IN NAK
	IN NAK
	.....
	IN !!Propagated Error!!(Turnaround/Timeout Error)



The driver doesn't receive any further interrupts when it stopped,
though the IN 102 Bytes transaction is finished on the bus, but the 
TD_TOTAL_BYTES of that dTd is never updated.

 ep num: 1 dir: 1(in)
  qh of current ep: 1 dir: 1(in)
    qh maxpacklen: 0x22000000, token: 0x668080
    qh td_dma curr_dtd_ptr: 0x658403c0, next_dtd_ptr: 0x1
    qh buf p0: 0x672dcde, p1: 0x672e378, p2: 0x672f000, p3: 0x6730000, p4: 0x6731000
  req 0xd81e245540 len: **102**, first_dtd: 0x658403c0, last_dtd: 0x658403c0, dtd_count: 1, actual: 102
     [qh->curr_dtd_ptr]dtd dma 0x658403c0, token: 0x668080, err: 0x0, dtd_nxt: 0x65840540
         buf p0:0x672dcde p1:0x672e000 p2:0x672f000 p3:0x6730000 p4:0x6731000 scr: 0x0
  req 0xd81e245b40 len: 1558, first_dtd: 0x65840540, last_dtd: 0x65840540, dtd_count: 1, actual: 1558
     [---]dtd dma 0x65840540, token: 0x6168080, err: 0x0, dtd_nxt: 0x65840080
         buf p0:0x20ce50de p1:0x20ce6000 p2:0x20ce7000 p3:0x20ce8000 p4:0x20ce9000 scr: 0x0
......

epnak: 0x00070003
epsetupstat: 0x00000000
epprime: 0x00000000
epflush: 0x00000000
epstatus: 0x00020002


Thanks!
Michal Simek March 6, 2025, 2:22 p.m. UTC | #2
+ Radhey.

On 3/6/25 15:12, Junzhong Pan wrote:
> Hi Piyush,
> 
>> Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
>> revision 2.20a.
>>
>> There is an issue with the add dTD tripwire semaphore (ATDTW bit in
>> USBCMD register) that can cause the controller to ignore a dTD that is
>> added to a primed endpoint. When this happens, the software can read
>> the tripwire bit and the status bit at '1' even though the endpoint is
>> unprimed.
>> This issue observed with the Windows host machine.
> 
> Sorry to dig through this old patch email.
> 
> May I ask what does **the scene look like (bus condition/dtd status)**
> when this issue observed with the Windows host machine occurs?
> 
> I want to know if it's relevant to an issue I encountered recently.
> 
> I met this a tricky issue with an soc have a marvel udc (driven by
> mv_udc_core.c, it's a same chipidea IP accordingly) when running rndis
> gadget with the Windows host machine.
> 
> When the rndis gadget running for a long time (randomly), the udc would
> suddenly stopped and no longger response to IN token sent from the host.
> 
> The bus trace looks like this:
> 
> BULK IN:
> 	IN
> 	DATA0 120 Bytes
> 	ACK
> BULK OUT:
> 	OUT
> 	DATA1 98 Bytes
> 	NYET
> BULK IN	??
> 	IN NAK
> 	IN NAK
> 	IN NAK
> 	.....
> 	IN !!Propagated Error!!(Turnaround/Timeout Error)
> 
> 
> 
> The driver doesn't receive any further interrupts when it stopped,
> though the IN 102 Bytes transaction is finished on the bus, but the
> TD_TOTAL_BYTES of that dTd is never updated.
> 
>   ep num: 1 dir: 1(in)
>    qh of current ep: 1 dir: 1(in)
>      qh maxpacklen: 0x22000000, token: 0x668080
>      qh td_dma curr_dtd_ptr: 0x658403c0, next_dtd_ptr: 0x1
>      qh buf p0: 0x672dcde, p1: 0x672e378, p2: 0x672f000, p3: 0x6730000, p4: 0x6731000
>    req 0xd81e245540 len: **102**, first_dtd: 0x658403c0, last_dtd: 0x658403c0, dtd_count: 1, actual: 102
>       [qh->curr_dtd_ptr]dtd dma 0x658403c0, token: 0x668080, err: 0x0, dtd_nxt: 0x65840540
>           buf p0:0x672dcde p1:0x672e000 p2:0x672f000 p3:0x6730000 p4:0x6731000 scr: 0x0
>    req 0xd81e245b40 len: 1558, first_dtd: 0x65840540, last_dtd: 0x65840540, dtd_count: 1, actual: 1558
>       [---]dtd dma 0x65840540, token: 0x6168080, err: 0x0, dtd_nxt: 0x65840080
>           buf p0:0x20ce50de p1:0x20ce6000 p2:0x20ce7000 p3:0x20ce8000 p4:0x20ce9000 scr: 0x0
> ......
> 
> epnak: 0x00070003
> epsetupstat: 0x00000000
> epprime: 0x00000000
> epflush: 0x00000000
> epstatus: 0x00020002
> 

Piyush is no longer with us but Radhey will take care about this.

Thanks,
Michal
Pandey, Radhey Shyam March 9, 2025, 2:55 p.m. UTC | #3
[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Junzhong Pan <panjunzhong@outlook.com>
> Sent: Thursday, March 6, 2025 7:43 PM
> To: piyush.mehta@amd.com
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org; linux-
> usb@vger.kernel.org; Simek, Michal <michal.simek@amd.com>;
> peter.chen@kernel.org; Pandey, Radhey Shyam
> <radhey.shyam.pandey@amd.com>; Paladugu, Siva Durga Prasad
> <siva.durga.prasad.paladugu@amd.com>
> Subject: Re: [PATCH V2] usb: chipidea: udc: Add revision check of
> 2.20[CI_REVISION_22]
>
> Hi Piyush,
>
> > Issue: Adding a dTD to a Primed Endpoint May Not Get Recognized with
> > revision 2.20a.
> >
> > There is an issue with the add dTD tripwire semaphore (ATDTW bit in
> > USBCMD register) that can cause the controller to ignore a dTD that is
> > added to a primed endpoint. When this happens, the software can read
> > the tripwire bit and the status bit at '1' even though the endpoint is
> > unprimed.
> > This issue observed with the Windows host machine.
>
> Sorry to dig through this old patch email.
>
> May I ask what does **the scene look like (bus condition/dtd status)** when this
> issue observed with the Windows host machine occurs?
>
> I want to know if it's relevant to an issue I encountered recently.
>
> I met this a tricky issue with an soc have a marvel udc (driven by mv_udc_core.c, it's
> a same chipidea IP accordingly) when running rndis gadget with the Windows host
> machine.

What is IP version ? If not 2.20, can you try with extending current check?

>
> When the rndis gadget running for a long time (randomly), the udc would suddenly
> stopped and no longger response to IN token sent from the host.
>
> The bus trace looks like this:
>
> BULK IN:
>       IN
>       DATA0 120 Bytes
>       ACK
> BULK OUT:
>       OUT
>       DATA1 98 Bytes
>       NYET
> BULK IN       ??
>       IN NAK
>       IN NAK
>       IN NAK
>       .....
>       IN !!Propagated Error!!(Turnaround/Timeout Error)
>
>
>
> The driver doesn't receive any further interrupts when it stopped, though the IN 102
> Bytes transaction is finished on the bus, but the TD_TOTAL_BYTES of that dTd is
> never updated.
>
>  ep num: 1 dir: 1(in)
>   qh of current ep: 1 dir: 1(in)
>     qh maxpacklen: 0x22000000, token: 0x668080
>     qh td_dma curr_dtd_ptr: 0x658403c0, next_dtd_ptr: 0x1
>     qh buf p0: 0x672dcde, p1: 0x672e378, p2: 0x672f000, p3: 0x6730000, p4:
> 0x6731000
>   req 0xd81e245540 len: **102**, first_dtd: 0x658403c0, last_dtd: 0x658403c0,
> dtd_count: 1, actual: 102
>      [qh->curr_dtd_ptr]dtd dma 0x658403c0, token: 0x668080, err: 0x0, dtd_nxt:
> 0x65840540
>          buf p0:0x672dcde p1:0x672e000 p2:0x672f000 p3:0x6730000 p4:0x6731000
> scr: 0x0
>   req 0xd81e245b40 len: 1558, first_dtd: 0x65840540, last_dtd: 0x65840540,
> dtd_count: 1, actual: 1558
>      [---]dtd dma 0x65840540, token: 0x6168080, err: 0x0, dtd_nxt: 0x65840080
>          buf p0:0x20ce50de p1:0x20ce6000 p2:0x20ce7000 p3:0x20ce8000
> p4:0x20ce9000 scr: 0x0 ......
>
> epnak: 0x00070003
> epsetupstat: 0x00000000
> epprime: 0x00000000
> epflush: 0x00000000
> epstatus: 0x00020002
>
>
> Thanks!
diff mbox series

Patch

diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 0b7bd3c643c3..2d7f616270c1 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -688,7 +688,8 @@  static int _hardware_dequeue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq)
 		if ((TD_STATUS_ACTIVE & tmptoken) != 0) {
 			int n = hw_ep_bit(hwep->num, hwep->dir);
 
-			if (ci->rev == CI_REVISION_24)
+			if (ci->rev == CI_REVISION_24 ||
+			    ci->rev == CI_REVISION_22)
 				if (!hw_read(ci, OP_ENDPTSTAT, BIT(n)))
 					reprime_dtd(ci, hwep, node);
 			hwreq->req.status = -EALREADY;