| Message ID | 1460374506-9779-3-git-send-email-rogerq@ti.com |
|---|---|
| State | New |
| Headers | show |
On 11/04/16 15:13, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: >> The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED. >> >> Use request_threaded_irq() to ensure that irqflags match for the >> shared interrupt handlers. If we don't use request_treaded_irq() then >> forced threaded irq will set IRQF_ONESHOT and this won't match with >> the OTG irq handler. > > then it seems you need to _first_ fix the OTG IRQ handler. Why does it There is no OTG irq handler yet. > defer ? At a minimum, first switch to threaded IRQ handler, then (in > another patch) switch to IRQF_SHARED OK. > >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> drivers/usb/dwc3/dwc3-omap.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >> index 22e9606..51ca098 100644 >> --- a/drivers/usb/dwc3/dwc3-omap.c >> +++ b/drivers/usb/dwc3/dwc3-omap.c >> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) >> { >> struct dwc3_omap *omap = _omap; >> u32 reg; >> + int ret = IRQ_NONE; >> >> reg = dwc3_omap_read_irqmisc_status(omap); >> >> + if (reg) >> + ret = IRQ_HANDLED; > > you can avoid the local variable by returning early here. How can we return early? we need to check irq0_status as well right? > > if (!reg) > return IRQ_NONE; > >> if (reg & USBOTGSS_IRQMISC_DMADISABLECLR) >> omap->dma_status = false; >> >> dwc3_omap_write_irqmisc_status(omap, reg); >> >> reg = dwc3_omap_read_irq0_status(omap); >> + if (reg) >> + ret = IRQ_HANDLED; > > and this check is probably unnecessary. > >> @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev) >> reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG); >> omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE); >> >> - ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0, >> - "dwc3-omap", omap); >> + ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt, > > this switch to threaded IRQ is not part of $subject. > OK. cheers, -roger
On 11/04/16 15:58, Felipe Balbi wrote: > > Hi, > > Roger Quadros <rogerq@ti.com> writes: >>>> diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c >>>> index 22e9606..51ca098 100644 >>>> --- a/drivers/usb/dwc3/dwc3-omap.c >>>> +++ b/drivers/usb/dwc3/dwc3-omap.c >>>> @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) >>>> { >>>> struct dwc3_omap *omap = _omap; >>>> u32 reg; >>>> + int ret = IRQ_NONE; >>>> >>>> reg = dwc3_omap_read_irqmisc_status(omap); >>>> >>>> + if (reg) >>>> + ret = IRQ_HANDLED; >>> >>> you can avoid the local variable by returning early here. >> >> How can we return early? we need to check irq0_status as well right? > > Oh, that's true. > > There's one thing that I noticed though. dma_status is only written to, > never read, so you should be able to remove it completely (a bit > off-topic, sorry). > I'll send a patch for that. Can it go in -rc as well along with the other 2? cheers, -roger
diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c index 22e9606..51ca098 100644 --- a/drivers/usb/dwc3/dwc3-omap.c +++ b/drivers/usb/dwc3/dwc3-omap.c @@ -274,19 +274,25 @@ static irqreturn_t dwc3_omap_interrupt(int irq, void *_omap) { struct dwc3_omap *omap = _omap; u32 reg; + int ret = IRQ_NONE; reg = dwc3_omap_read_irqmisc_status(omap); + if (reg) + ret = IRQ_HANDLED; + if (reg & USBOTGSS_IRQMISC_DMADISABLECLR) omap->dma_status = false; dwc3_omap_write_irqmisc_status(omap, reg); reg = dwc3_omap_read_irq0_status(omap); + if (reg) + ret = IRQ_HANDLED; dwc3_omap_write_irq0_status(omap, reg); - return IRQ_HANDLED; + return ret; } static void dwc3_omap_enable_irqs(struct dwc3_omap *omap) @@ -506,8 +512,8 @@ static int dwc3_omap_probe(struct platform_device *pdev) reg = dwc3_omap_readl(omap->base, USBOTGSS_SYSCONFIG); omap->dma_status = !!(reg & USBOTGSS_SYSCONFIG_DMADISABLE); - ret = devm_request_irq(dev, omap->irq, dwc3_omap_interrupt, 0, - "dwc3-omap", omap); + ret = devm_request_threaded_irq(dev, omap->irq, dwc3_omap_interrupt, + NULL, IRQF_SHARED, "dwc3-omap", omap); if (ret) { dev_err(dev, "failed to request IRQ #%d --> %d\n", omap->irq, ret);
The wrapper interrupt is shared with OTG core so mark it IRQF_SHARED. Use request_threaded_irq() to ensure that irqflags match for the shared interrupt handlers. If we don't use request_treaded_irq() then forced threaded irq will set IRQF_ONESHOT and this won't match with the OTG irq handler. Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/usb/dwc3/dwc3-omap.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) -- 2.5.0