Re: Chipidea USB device goes infinite loop due to interrupt while hw_ep_prime
From: Peter Chen <peter.chen@kernel.org>
Date: 2021-08-09 01:40:56
On 21-08-04 12:11:31, Jeaho Hwang wrote:
Thanks for the answer Peter. I still have two questions.
1) There's a busy loop in hw_ep_prime to wait for endpoint priming. Is
it safe without timeout?
192 static int hw_ep_prime(struct ci_hdrc *ci, int num, int dir, int is_ctrl)
193 {
194 int n = hw_ep_bit(num, dir);
195
196 /* Synchronize before ep prime */
197 wmb();
198
199 if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
200 return -EAGAIN;
201
202 hw_write(ci, OP_ENDPTPRIME, ~0, BIT(n));
203
204 while (hw_read(ci, OP_ENDPTPRIME, BIT(n)))
205 cpu_relax();
206 if (is_ctrl && dir == RX && hw_read(ci, OP_ENDPTSETUPSTAT, BIT(num)))
207 return -EAGAIN;
208
209 /* status shoult be tested according with manual but it doesn't work */
210 return 0;
211 }You could add loop-timeout for it. Without timeout, it may lead issue, I am not so sure.
2) We experienced an infinite loop in hw_ep_set_halt, which is called
at isr_tr_complete_handler due to an error encountered that we
reported in the previous mail. It seems that hw_write to set halt
fails. Is it related to the interruption while hw_ep_prime is running?
If we make a timeout for the loop and error return, are there any
considerations for the caller functions?
223 static int hw_ep_set_halt(struct ci_hdrc *ci, int num, int dir, int value)
224 {
225 if (value != 0 && value != 1)
226 return -EINVAL;
227
228 do {
229 enum ci_hw_regs reg = OP_ENDPTCTRL + num;
230 u32 mask_xs = (dir == TX) ? ENDPTCTRL_TXS : ENDPTCTRL_RXS;
231 u32 mask_xr = (dir == TX) ? ENDPTCTRL_TXR : ENDPTCTRL_RXR;
232
233 /* data toggle - reserved for EP0 but it's in ESS */
234 hw_write(ci, reg, mask_xs|mask_xr,
235 value ? mask_xs : mask_xr);
236 } while (value != hw_ep_get_halt(ci, num, dir));
237
238 return 0;
239 }If the error occurs for set halt, I think the transfer may stop, and it may can't be recovered unless re-plug the cable. To not affect the whole system, it is better to add loop-time mechanism. -- Thanks, Peter Chen