Hello. On 28-11-2011 15:40, Neil Zhang wrote: >>>>> When device is stopped, there is no need to handle ISR. >>>>> Especially when otg switch to HOST mode. [...] >>>>> Signed-off-by: Neil Zhang<zhangwm@xxxxxxxxxxx> >>>>> --- >>>>> drivers/usb/gadget/mv_udc.h | 3 ++- >>>>> drivers/usb/gadget/mv_udc_core.c | 8 ++++++++ >>>>> 2 files changed, 10 insertions(+), 1 deletions(-) >> >>>>> diff --git a/drivers/usb/gadget/mv_udc_core.c >>>> b/drivers/usb/gadget/mv_udc_core.c >>>>> index c7c0bfa..e852f9d 100644 >>>>> --- a/drivers/usb/gadget/mv_udc_core.c >>>>> +++ b/drivers/usb/gadget/mv_udc_core.c >>>>> @@ -1056,6 +1056,8 @@ static void udc_stop(struct mv_udc *udc) >>>>> USBINTR_PORT_CHANGE_DETECT_EN | USBINTR_RESET_EN); >>>>> writel(tmp,&udc->op_regs->usbintr); >>>>> >>>>> + udc->stopped = 1; >>>>> + >>>>> /* Reset the Run the bit in the command register to stop VUSB */ >>>>> tmp = readl(&udc->op_regs->usbcmd); >>>>> tmp&= ~USBCMD_RUN_STOP; >>>>> @@ -1072,6 +1074,8 @@ static void udc_start(struct mv_udc *udc) >>>>> /* Enable interrupts */ >>>>> writel(usbintr,&udc->op_regs->usbintr); >>>>> >>>>> + udc->stopped = 0; >>>>> + >>>>> /* Set the Run bit in the command register */ >>>>> writel(USBCMD_RUN_STOP,&udc->op_regs->usbcmd); >>>>> } >>>>> @@ -2040,6 +2044,10 @@ static irqreturn_t mv_udc_irq(int irq, void *dev) >>>>> struct mv_udc *udc = (struct mv_udc *)dev; >>>>> u32 status, intr; >>>>> >>>>> + /* Disable ISR when stopped bit is set */ >>>>> + if (udc->stopped) >>>>> + return IRQ_NONE; >>>> Can't this lead to the interrupt storm and masking the >>>> interrupt as a result? >>>> You'd better mask off interrupts via your hardware's interrupt mask >>>> register. >>> Yes, we will mask the interrupt when stop the gadget. But this irq line >>> is shared between gadget and host driver, if the host is not active, there >>> will be no interrupt here, so I think it Should be OK here to return IRQ_NONE. >> And if the host is active? Aren't you checking the gadget interrupt >> status >> register in order to know if it's gadget's interrupt? You should if not, >> and >> so your new code seems to have no sense at all. > If we don't add this code, when otg works in host mode, the udc_irq is still called > because of PORT CHANGE event when we plug in an usb key. Is that event mapped to the gadget's interrupt register too? If not, the mask register will be zero, and the code will just return IRQ_NONE. > But it should not be called when otg is in host mode. Why not, if gadget interrupts are masked? You return IRQ_NONE in that case anyway. > The code if (udc->stopped) here means that the controller exits the gadget mode, > And udc_irq should not be called. Not sure why your gadget driver is located separately from the host driver if this is OTG controller... shouldn't it be in the same place, like drivers/usb/musb/? > Best Regards, > Neil Zhang WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
