From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932129AbcDKMx2 (ORCPT ); Mon, 11 Apr 2016 08:53:28 -0400 Received: from devils.ext.ti.com ([198.47.26.153]:46704 "EHLO devils.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754879AbcDKMvU (ORCPT ); Mon, 11 Apr 2016 08:51:20 -0400 Subject: Re: [PATCH v6 02/10] usb: dwc3: omap: Make the wrapper interrupt shared To: Felipe Balbi References: <1460374506-9779-1-git-send-email-rogerq@ti.com> <1460374506-9779-3-git-send-email-rogerq@ti.com> <87h9f8ny14.fsf@intel.com> CC: , , , , , , , , , , From: Roger Quadros Message-ID: <570B9DB9.8050807@ti.com> Date: Mon, 11 Apr 2016 15:51:05 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <87h9f8ny14.fsf@intel.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/04/16 15:13, Felipe Balbi wrote: > > Hi, > > Roger Quadros 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 >> --- >> 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