RE: [PATCH v1 2/2] usb:cdns3 Add Cadence USB3 DRD Driver

[Peter Chen]

> >> >> +                                           USB_CMD_STMODE |
> >> >> +
> >> >> + USB_STS_TMODE_SEL(tmode - 1));
> >> >
> >> >I'm 90% sure this won't work. There's a reason why we only enter the
> >> >requested test mode from status stage. How have you tested this?
> >>
> >
> > What's the reason?
> > It can work although the code is a little different with above, I
> > tested it using USBxHSETT tool at Windows.
> 
> put a sniffer. Status stage won't complete
> 

Ok, you mean sending test packet during the status completion handler?
I will see whether the status stage can complete next time when doing test.

> >> >> +    irqreturn_t ret = IRQ_NONE;
> >> >> +    unsigned long flags;
> >> >> +    u32 reg;
> >> >> +
> >> >> +    priv_dev = cdns->gadget_dev;
> >> >> +    spin_lock_irqsave(&priv_dev->lock, flags);
> >> >
> >> >you're already running in hardirq context. Why do you need this lock
> >> >at all? I would be better to use the hardirq handler to mask your
> >> >interrupts, so they don't fire again, then used the top-half
> >> >(softirq) handler to actually handle the interrupts.
> >>
> >
> > This controller may be ran at SMP environment, register and flag
> > access needs to be protected among CPUs running.
> 
> in hardirq context? When interrupts are already disabled?

Interrupt handler (hardirq context) at CPU0, and process at CPU1, eg role switch, unload module, etc.

Peter