On 21-08-27 10:35:05, Jeaho Hwang wrote:

2021년 8월 27일 (금) 오전 8:08, Peter Chen [off-list ref]님이 작성:

quoted

On 21-08-24 17:31:44, Jeaho Hwang wrote:

quoted

2021년 8월 17일 (화) 오후 3:44, Jeaho Hwang [off-list ref]님이 작성:

quoted

If ctrl EP priming is failed (very rare case in standard linux),
hw_ep_set_halt goes infinite loop. waiting 100 times was enough
for zynq7000.

Hi Peter.
I found from zynq7000 TRM that the hardware clears Stall bit if a
setup packet is received on a control endpoint.
I think hw_ep_set_halt goes infinite loop since:

1. hw_ep_prime for control EP which is called from
isr_tr_complete_handler -> isr_setup_status_phase is failed due to a
setup packet received.

How do you know that? Do you observe the new setup packet on the bus
before the current status stage? Usually, the host doesn't begin new setup
transfer before current setup transfer has finished.

Peter

I found an error return from the second ENDPTSETUPSTAT checking
routine, then setting the stall bit(TXS) kept failing. So I guessed it
is due to a setup packet received.
I didn't observe the setup packet by e.g. HW probes. Any other reason
to produce that symptom?

I guess two possible reasons for that:
- The new setup is coming after priming
- The interrupt occurs after prime, and when the back from interrupt,
other thread for USB transfer is scheduled, eg, usb_ep_queue from RNDIS 

From your experiments and observation, it seems the first reason is not possible.
Did your get failure with UP system?

Peter

For reminder, only reproduced on preemp_rt kernel and with Windows(10)
RNDIS host.

thanks.

 191 static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
 192 {
 193     int n = hw_ep_bit(num, dir);
 194
 195     /* Synchronize before ep prime */
 196     wmb();
 197
 198     if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
 199         return -EAGAIN;
 200
 201     hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
 202
 203     while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
 204         cpu_relax();
 205     if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
 206         return -EAGAIN;
             ~~~~~~~~~~~~~~~~
 207
 208     /* status shoult be tested according with manual but it doesn't work */
 209     return 0;
 210 }

quoted

quoted

2. in isr_tr_complete_handler it tries to call _ep_set_halt if either
isr_tr_complete_low or isr_setup_status_phase returns error.
3. Since the control EP got a setup packet, HW resets TXS bit just as
the driver sets inside hw_ep_set_halt so it goes infinite loop.

Does it make sense? If it is right, we shouldn't call _ep_set_halt if
the err is -EAGAIN, which could be returned ONLY due to the setup
packet issue described above.
And the loop timeout is not required anymore.

Can I ask your opinion on this, Peter and USB experts?

Thanks.

-- 

Thanks,
Peter Chen