RE: [PATCH 4/7 v1] usb: gadget: mv_udc: disable ISR when stopped

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> -----Original Message-----
> From: Sergei Shtylyov [mailto:sshtylyov@xxxxxxxxxx]
> Sent: 2011年11月28日 18:56
> To: Neil Zhang
> Cc: balbi@xxxxxx; stern@xxxxxxxxxxxxxxxxxxx; gregkh@xxxxxxx; linux-
> usb@xxxxxxxxxxxxxxx; Chao Xie
> Subject: Re: [PATCH 4/7 v1] usb: gadget: mv_udc: disable ISR when
> stopped
> 
> Hello.
> 
> On 28-11-2011 6:09, Neil Zhang wrote:
> 
> >>> When device is stopped, there is no need to handle ISR.
> >>> Especially when otg switch to HOST mode.
> 
> >>> Change-Id: I17ccb8eee5162c52ba1761d2d471f77c14d89da9
> 
> >>      This line has no place in the upstream patch -- remove it
> please.
> 
> 
> > 	Thanks for your remind.
> 
> >>> 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. But it should not be called 
when otg is in host mode. 
The code if (udc->stopped) here means that the controller exits the gadget mode, 
And udc_irq should not be called.

> >> WBR, Sergei
> 
> > Best Regards,
> > Neil Zhang
> 
> WBR, Sergei

Best Regards,
Neil Zhang
?韬{.n?????%??檩??w?{.n???{炳???骅w*jg????????G??⒏⒎?:+v????????????"??????


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux