Re: [PATCH v7 1/5] usb: dwc3: omap: use request_threaded_irq()

From: Felipe Balbi
Date: Wed May 11 2016 - 08:42:20 EST



Hi,

Roger Quadros <rogerq@xxxxxx> 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

Attachment: signature.asc
Description: PGP signature