linux-kernel - Re: [PATCH v7 1/5] usb: dwc3: omap: use request_threaded

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87r3d8g44i.fsf@linux.intel.com>
Date:	Wed, 11 May 2016 15:39:57 +0300
From:	Felipe Balbi <balbi@...nel.org>
To:	Roger Quadros <rogerq@...com>
Cc:	tony@...mide.com, Joao.Pinto@...opsys.com,
	sergei.shtylyov@...entembedded.com, peter.chen@...escale.com,
	jun.li@...escale.com, grygorii.strashko@...com,
	yoshihiro.shimoda.uh@...esas.com, nsekhar@...com, b-liu@...com,
	linux-usb@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 1/5] usb: dwc3: omap: use request_threaded_irq()


Hi,

Roger Quadros <rogerq@...com> writes:
>>>> static irqreturn_t dwc3_omap_threaded_interrupt(int irq, void *_omap)
>>>> {
>>>>         struct dwc3_omap *omap = _omap;
>>>>         u32 reg;
>>>>
>>>>         spin_lock(&omap->lock);
>>>
>>> Do we really need a spin_lock for the dwc3-omap driver?
>>> Currently we won't be doing anything other than just
>>> clearing the irqstatus and re-enabling the interrupts.
>> 
>> well, if there's no possibility of races, then no. But only testing will
>> say for sure, I guess. I didn't really go through the entire thing just
>> to a write a quick little template :-p
>> 
> OK. Another hurdle I have is that how do I mask/unmask the interrupts?
> I do not see any masking bits, only enable/disable bits.
>
> I don't think we can use the enable/disable bits to mask as we'd loose
> events while the event is disabled.

I'm pretty sure your TRM discusses the usage of IRQSTATUS_RAW*
registers, doesn't it ? :-)

Those registers should be modified by HW even when interrupts are
disabled/masked. Note that, the IRQSTATUS_SET* and IRQSTATUS_CLR*
registers act more like mask/unmask than enable/disable considering we
can still read IRQ status using *RAW* registers.

See if below works fine for OMAP5, AM4 and AM5 SoCs:

diff --git a/drivers/usb/dwc3/dwc3-omap.c b/drivers/usb/dwc3/dwc3-omap.c
index af264493bbae..ece2f25ad2c3 100644
--- a/drivers/usb/dwc3/dwc3-omap.c
+++ b/drivers/usb/dwc3/dwc3-omap.c
@@ -165,20 +165,20 @@ static void dwc3_omap_write_utmi_ctrl(struct dwc3_omap *omap, u32 value)
 
 static u32 dwc3_omap_read_irq0_status(struct dwc3_omap *omap)
 {
-	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_0 -
+	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_RAW_0 -
 						omap->irq0_offset);
 }
 
 static void dwc3_omap_write_irq0_status(struct dwc3_omap *omap, u32 value)
 {
-	dwc3_omap_writel(omap->base, USBOTGSS_IRQSTATUS_0 -
+	dwc3_omap_writel(omap->base, USBOTGSS_IRQSTATUS_RAW_0 -
 						omap->irq0_offset, value);
 
 }
 
 static u32 dwc3_omap_read_irqmisc_status(struct dwc3_omap *omap)
 {
-	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_MISC +
+	return dwc3_omap_readl(omap->base, USBOTGSS_IRQSTATUS_RAW_MISC +
 						omap->irqmisc_offset);
 }
 
-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (819 bytes)