2015-02-27 13:29:55

by Valentin Rothberg

[permalink] [raw]
Subject: [PATCH] usb/isp1760: set IRQ flags properly

The IRQF_DISABLED is a NOOP and scheduled to be removed. According to
commit e58aa3d2d0cc (genirq: Run irq handlers with interrupts disabled)
running IRQ handlers with interrupts enabled can cause stack overflows
when the interrupt line of the issuing device is still active.

This patch removes using this deprecated flag and additionally removes
redundantly setting IRQF_SHARED.

Signed-off-by: Valentin Rothberg <[email protected]>
---
drivers/usb/isp1760/isp1760-core.c | 5 ++---
drivers/usb/isp1760/isp1760-udc.c | 4 ++--
2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/isp1760/isp1760-core.c b/drivers/usb/isp1760/isp1760-core.c
index b982755..9f4a0de 100644
--- a/drivers/usb/isp1760/isp1760-core.c
+++ b/drivers/usb/isp1760/isp1760-core.c
@@ -145,14 +145,13 @@ int isp1760_register(struct resource *mem, int irq, unsigned long irqflags,

if (IS_ENABLED(CONFIG_USB_ISP1760_HCD) && !usb_disabled()) {
ret = isp1760_hcd_register(&isp->hcd, isp->regs, mem, irq,
- irqflags | IRQF_SHARED, dev);
+ irqflags, dev);
if (ret < 0)
return ret;
}

if (IS_ENABLED(CONFIG_USB_ISP1761_UDC) && !udc_disabled) {
- ret = isp1760_udc_register(isp, irq, irqflags | IRQF_SHARED |
- IRQF_DISABLED);
+ ret = isp1760_udc_register(isp, irq, irqflags);
if (ret < 0) {
isp1760_hcd_unregister(&isp->hcd);
return ret;
diff --git a/drivers/usb/isp1760/isp1760-udc.c b/drivers/usb/isp1760/isp1760-udc.c
index 9612d79..0b46ff0 100644
--- a/drivers/usb/isp1760/isp1760-udc.c
+++ b/drivers/usb/isp1760/isp1760-udc.c
@@ -1451,8 +1451,8 @@ int isp1760_udc_register(struct isp1760_device *isp, int irq,

sprintf(udc->irqname, "%s (udc)", devname);

- ret = request_irq(irq, isp1760_udc_irq, IRQF_SHARED | IRQF_DISABLED |
- irqflags, udc->irqname, udc);
+ ret = request_irq(irq, isp1760_udc_irq, IRQF_SHARED | irqflags,
+ udc->irqname, udc);
if (ret < 0)
goto error;

--
1.9.1


2015-02-27 15:25:11

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] usb/isp1760: set IRQ flags properly

On Fri, Feb 27, 2015 at 02:29:40PM +0100, Valentin Rothberg wrote:
> The IRQF_DISABLED is a NOOP and scheduled to be removed. According to
> commit e58aa3d2d0cc (genirq: Run irq handlers with interrupts disabled)
> running IRQ handlers with interrupts enabled can cause stack overflows
> when the interrupt line of the issuing device is still active.
>
> This patch removes using this deprecated flag and additionally removes
> redundantly setting IRQF_SHARED.

why is it redundant ?

--
balbi


Attachments:
(No filename) (497.00 B)
signature.asc (819.00 B)
Digital signature
Download all attachments

2015-02-27 15:49:13

by Valentin Rothberg

[permalink] [raw]
Subject: Re: [PATCH] usb/isp1760: set IRQ flags properly

On Fri, Feb 27, 2015 at 4:24 PM, Felipe Balbi <[email protected]> wrote:
> On Fri, Feb 27, 2015 at 02:29:40PM +0100, Valentin Rothberg wrote:
>> The IRQF_DISABLED is a NOOP and scheduled to be removed. According to
>> commit e58aa3d2d0cc (genirq: Run irq handlers with interrupts disabled)
>> running IRQ handlers with interrupts enabled can cause stack overflows
>> when the interrupt line of the issuing device is still active.
>>
>> This patch removes using this deprecated flag and additionally removes
>> redundantly setting IRQF_SHARED.
>
> why is it redundant ?

It's redundant in the call of isp1760_udc_register() as this function
sets the flag by requesting the IRQ. I mistakenly removed it also in
the call of isp1760_hcd_register() which does not alter the passed
irqflags. I will fix this in a second version of this patch. I am
sorry for this mistake.

Valentin

> --
> balbi

2015-02-27 15:51:47

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] usb/isp1760: set IRQ flags properly

On Fri, Feb 27, 2015 at 04:48:39PM +0100, Valentin Rothberg wrote:
> On Fri, Feb 27, 2015 at 4:24 PM, Felipe Balbi <[email protected]> wrote:
> > On Fri, Feb 27, 2015 at 02:29:40PM +0100, Valentin Rothberg wrote:
> >> The IRQF_DISABLED is a NOOP and scheduled to be removed. According to
> >> commit e58aa3d2d0cc (genirq: Run irq handlers with interrupts disabled)
> >> running IRQ handlers with interrupts enabled can cause stack overflows
> >> when the interrupt line of the issuing device is still active.
> >>
> >> This patch removes using this deprecated flag and additionally removes
> >> redundantly setting IRQF_SHARED.
> >
> > why is it redundant ?
>
> It's redundant in the call of isp1760_udc_register() as this function
> sets the flag by requesting the IRQ. I mistakenly removed it also in
> the call of isp1760_hcd_register() which does not alter the passed
> irqflags. I will fix this in a second version of this patch. I am
> sorry for this mistake.

no problem, s**t happens ;)

--
balbi


Attachments:
(No filename) (0.98 kB)
signature.asc (819.00 B)
Digital signature
Download all attachments