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.
> 

Do you mean it is better to check USBMODE register instead of using flag stopped?

> >> 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