Re: Chipidea USB device goes infinite loop due to interrupt while hw_ep_prime

[Peter Chen <peter.chen@kernel.org>]

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