From: "Chen, Jie" <[email protected]>
During resume, a startup will request_irq again, meantime resume function's
enable_irq will cause unbalanced IRQ issue.
Fix this issue by moving request_irq to probe function.
Signed-off-by: David Cohen <[email protected]>
Signed-off-by: Chen, Jie <[email protected]>
---
drivers/tty/serial/mrst_max3110.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/tty/serial/mrst_max3110.c b/drivers/tty/serial/mrst_max3110.c
index a67e708..6f618dd 100644
--- a/drivers/tty/serial/mrst_max3110.c
+++ b/drivers/tty/serial/mrst_max3110.c
@@ -43,6 +43,7 @@
#include <linux/kthread.h>
#include <linux/spi/spi.h>
+#include <linux/pm.h>
#include "mrst_max3110.h"
@@ -491,19 +492,9 @@ static int serial_m3110_startup(struct uart_port *port)
port->state->port.low_latency = 1;
if (max->irq) {
- max->read_thread = NULL;
- ret = request_irq(max->irq, serial_m3110_irq,
- IRQ_TYPE_EDGE_FALLING, "max3110", max);
- if (ret) {
- max->irq = 0;
- pr_err(PR_FMT "unable to allocate IRQ, polling\n");
- } else {
- /* Enable RX IRQ only */
- config |= WC_RXA_IRQ_ENABLE;
- }
- }
-
- if (max->irq == 0) {
+ /* Enable RX IRQ only */
+ config |= WC_RXA_IRQ_ENABLE;
+ } else {
/* If IRQ is disabled, start a read thread for input data */
max->read_thread =
kthread_run(max3110_read_thread, max, "max3110_read");
@@ -517,8 +508,6 @@ static int serial_m3110_startup(struct uart_port *port)
ret = max3110_out(max, config);
if (ret) {
- if (max->irq)
- free_irq(max->irq, max);
if (max->read_thread)
kthread_stop(max->read_thread);
max->read_thread = NULL;
@@ -540,9 +529,6 @@ static void serial_m3110_shutdown(struct uart_port *port)
max->read_thread = NULL;
}
- if (max->irq)
- free_irq(max->irq, max);
-
/* Disable interrupts from this port */
config = WC_TAG | WC_SW_SHDI;
max3110_out(max, config);
@@ -840,6 +826,16 @@ static int serial_m3110_probe(struct spi_device *spi)
goto err_kthread;
}
+ if (max->irq) {
+ ret = request_irq(max->irq, serial_m3110_irq,
+ IRQ_TYPE_EDGE_FALLING, "max3110", max);
+ if (ret) {
+ max->irq = 0;
+ dev_warn(&spi->dev,
+ "unable to allocate IRQ, will use polling method\n");
+ }
+ }
+
spi_set_drvdata(spi, max);
pmax = max;
@@ -867,6 +863,9 @@ static int serial_m3110_remove(struct spi_device *dev)
free_page((unsigned long)max->con_xmit.buf);
+ if (max->irq)
+ free_irq(max->irq, max);
+
if (max->main_thread)
kthread_stop(max->main_thread);
--
1.8.4.rc3
From: Ivan Gorinov <[email protected]>
The change in the max3110 driver makes the IRQ handling threaded, now
the handler is called only once per received character. Without that
change, we had many (more than 100) interrupts per one received
character.
Unfortunately, SFI interface does not support IRQ polarity and
triggering modes, so we have to keep the hacks as hard-coded device
names and IRQ numbers until we switch to ACPI.
Edge-triggered IRQ still supported to keep old platforms working.
Use platform data to pass the irq mode argument.
Signed-off-by: Ivan Gorinov <[email protected]>
Signed-off-by: Li Ning <[email protected]>
Signed-off-by: David Cohen <[email protected]>
---
drivers/tty/serial/mrst_max3110.c | 35 +++++++++++++++++++++++++++--------
include/linux/serial_max3110.h | 16 ++++++++++++++++
2 files changed, 43 insertions(+), 8 deletions(-)
create mode 100644 include/linux/serial_max3110.h
diff --git a/drivers/tty/serial/mrst_max3110.c b/drivers/tty/serial/mrst_max3110.c
index 6f618dd..3778dab 100644
--- a/drivers/tty/serial/mrst_max3110.c
+++ b/drivers/tty/serial/mrst_max3110.c
@@ -40,6 +40,7 @@
#include <linux/tty_flip.h>
#include <linux/serial_core.h>
#include <linux/serial_reg.h>
+#include <linux/serial_max3110.h>
#include <linux/kthread.h>
#include <linux/spi/spi.h>
@@ -68,6 +69,7 @@ struct uart_max3110 {
u8 clock;
u8 parity, word_7bits;
u16 irq;
+ u16 irq_edge_triggered;
unsigned long uart_flags;
@@ -415,8 +417,8 @@ static int max3110_main_thread(void *_max)
max->uart_flags || kthread_should_stop());
mutex_lock(&max->thread_mutex);
-
- if (test_and_clear_bit(BIT_IRQ_PENDING, &max->uart_flags))
+ if (max->irq_edge_triggered &&
+ test_and_clear_bit(BIT_IRQ_PENDING, &max->uart_flags))
max3110_con_receive(max);
/* first handle console output */
@@ -438,11 +440,15 @@ static irqreturn_t serial_m3110_irq(int irq, void *dev_id)
{
struct uart_max3110 *max = dev_id;
- /* max3110's irq is a falling edge, not level triggered,
- * so no need to disable the irq */
+ if (max->irq_edge_triggered) {
+ /* max3110's irq is a falling edge, not level triggered,
+ * so no need to disable the irq */
- if (!test_and_set_bit(BIT_IRQ_PENDING, &max->uart_flags))
- wake_up(&max->wq);
+ if (!test_and_set_bit(BIT_IRQ_PENDING, &max->uart_flags))
+ wake_up(&max->wq);
+ } else {
+ max3110_con_receive(max);
+ }
return IRQ_HANDLED;
}
@@ -766,6 +772,10 @@ static int serial_m3110_probe(struct spi_device *spi)
void *buffer;
u16 res;
int ret = 0;
+ struct plat_max3110 *pdata = spi->dev.platform_data;
+
+ if (!pdata)
+ return -EINVAL;
max = kzalloc(sizeof(*max), GFP_KERNEL);
if (!max)
@@ -826,9 +836,18 @@ static int serial_m3110_probe(struct spi_device *spi)
goto err_kthread;
}
+ max->irq_edge_triggered = pdata->irq_edge_triggered;
+
if (max->irq) {
- ret = request_irq(max->irq, serial_m3110_irq,
- IRQ_TYPE_EDGE_FALLING, "max3110", max);
+ if (max->irq_edge_triggered) {
+ ret = request_irq(max->irq, serial_m3110_irq,
+ IRQ_TYPE_EDGE_FALLING, "max3110", max);
+ } else {
+ ret = request_threaded_irq(max->irq, NULL,
+ serial_m3110_irq,
+ IRQF_ONESHOT, "max3110", max);
+ }
+
if (ret) {
max->irq = 0;
dev_warn(&spi->dev,
diff --git a/include/linux/serial_max3110.h b/include/linux/serial_max3110.h
new file mode 100644
index 0000000..5470556
--- /dev/null
+++ b/include/linux/serial_max3110.h
@@ -0,0 +1,16 @@
+#ifndef _LINUX_SERIAL_MAX3110_H
+#define _LINUX_SERIAL_MAX3110_H
+
+/**
+ * struct plat_max3110 - MAX3110 SPI UART platform data
+ * @irq_edge_trigger: if IRQ is edge triggered
+ *
+ * You should use this structure in your machine description to specify
+ * how the MAX3110 is connected.
+ *
+ */
+struct plat_max3110 {
+ int irq_edge_triggered;
+};
+
+#endif
--
1.8.4.rc3
> The change in the max3110 driver makes the IRQ handling threaded, now
> the handler is called only once per received character. Without that
> change, we had many (more than 100) interrupts per one received
> character.
>
> Unfortunately, SFI interface does not support IRQ polarity and
> triggering modes, so we have to keep the hacks as hard-coded device
> names and IRQ numbers until we switch to ACPI.
>
> Edge-triggered IRQ still supported to keep old platforms working.
> Use platform data to pass the irq mode argument.
>
> Signed-off-by: Ivan Gorinov <[email protected]>
> Signed-off-by: Li Ning <[email protected]>
> Signed-off-by: David Cohen <[email protected]>
...
> +++ b/include/linux/serial_max3110.h
> @@ -0,0 +1,16 @@
> +#ifndef _LINUX_SERIAL_MAX3110_H
> +#define _LINUX_SERIAL_MAX3110_H
> +
> +/**
> + * struct plat_max3110 - MAX3110 SPI UART platform data
> + * @irq_edge_trigger: if IRQ is edge triggered
> + *
> + * You should use this structure in your machine description to specify
> + * how the MAX3110 is connected.
> + *
> + */
> +struct plat_max3110 {
> + int irq_edge_triggered;
> +};
> +
> +#endif
> --
Is just resource->flags for IRQ can be reused for handle such case?
---
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 10/22/2013 12:46 PM, Alexander Shiyan wrote:
>> The change in the max3110 driver makes the IRQ handling threaded, now
>> the handler is called only once per received character. Without that
>> change, we had many (more than 100) interrupts per one received
>> character.
>>
>> Unfortunately, SFI interface does not support IRQ polarity and
>> triggering modes, so we have to keep the hacks as hard-coded device
>> names and IRQ numbers until we switch to ACPI.
>>
>> Edge-triggered IRQ still supported to keep old platforms working.
>> Use platform data to pass the irq mode argument.
>>
>> Signed-off-by: Ivan Gorinov <[email protected]>
>> Signed-off-by: Li Ning <[email protected]>
>> Signed-off-by: David Cohen <[email protected]>
> ...
>> +++ b/include/linux/serial_max3110.h
>> @@ -0,0 +1,16 @@
>> +#ifndef _LINUX_SERIAL_MAX3110_H
>> +#define _LINUX_SERIAL_MAX3110_H
>> +
>> +/**
>> + * struct plat_max3110 - MAX3110 SPI UART platform data
>> + * @irq_edge_trigger: if IRQ is edge triggered
>> + *
>> + * You should use this structure in your machine description to specify
>> + * how the MAX3110 is connected.
>> + *
>> + */
>> +struct plat_max3110 {
>> + int irq_edge_triggered;
>> +};
>> +
>> +#endif
>> --
>
> Is just resource->flags for IRQ can be reused for handle such case?
I believe your suggestion makes perfect sense. I'll rework it.
Br, David Cohen
On 10/22/2013 01:30 PM, David Cohen wrote:
> On 10/22/2013 12:46 PM, Alexander Shiyan wrote:
>>> The change in the max3110 driver makes the IRQ handling threaded, now
>>> the handler is called only once per received character. Without that
>>> change, we had many (more than 100) interrupts per one received
>>> character.
>>>
>>> Unfortunately, SFI interface does not support IRQ polarity and
>>> triggering modes, so we have to keep the hacks as hard-coded device
>>> names and IRQ numbers until we switch to ACPI.
>>>
>>> Edge-triggered IRQ still supported to keep old platforms working.
>>> Use platform data to pass the irq mode argument.
>>>
>>> Signed-off-by: Ivan Gorinov <[email protected]>
>>> Signed-off-by: Li Ning <[email protected]>
>>> Signed-off-by: David Cohen <[email protected]>
>> ...
>>> +++ b/include/linux/serial_max3110.h
>>> @@ -0,0 +1,16 @@
>>> +#ifndef _LINUX_SERIAL_MAX3110_H
>>> +#define _LINUX_SERIAL_MAX3110_H
>>> +
>>> +/**
>>> + * struct plat_max3110 - MAX3110 SPI UART platform data
>>> + * @irq_edge_trigger: if IRQ is edge triggered
>>> + *
>>> + * You should use this structure in your machine description to specify
>>> + * how the MAX3110 is connected.
>>> + *
>>> + */
>>> +struct plat_max3110 {
>>> + int irq_edge_triggered;
>>> +};
>>> +
>>> +#endif
>>> --
>>
>> Is just resource->flags for IRQ can be reused for handle such case?
>
> I believe your suggestion makes perfect sense. I'll rework it.
Looks like isp_device has no place for 'resource'. In this case pdata
seems to be the way to go here.
Or maybe there's a better way to recommend?
Br, David Cohen
> On 10/22/2013 01:30 PM, David Cohen wrote:
> > On 10/22/2013 12:46 PM, Alexander Shiyan wrote:
> >>> The change in the max3110 driver makes the IRQ handling threaded, now
> >>> the handler is called only once per received character. Without that
> >>> change, we had many (more than 100) interrupts per one received
> >>> character.
> >>>
> >>> Unfortunately, SFI interface does not support IRQ polarity and
> >>> triggering modes, so we have to keep the hacks as hard-coded device
> >>> names and IRQ numbers until we switch to ACPI.
> >>>
> >>> Edge-triggered IRQ still supported to keep old platforms working.
> >>> Use platform data to pass the irq mode argument.
> >>>
> >>> Signed-off-by: Ivan Gorinov <[email protected]>
> >>> Signed-off-by: Li Ning <[email protected]>
> >>> Signed-off-by: David Cohen <[email protected]>
> >> ...
> >>> +++ b/include/linux/serial_max3110.h
> >>> @@ -0,0 +1,16 @@
> >>> +#ifndef _LINUX_SERIAL_MAX3110_H
> >>> +#define _LINUX_SERIAL_MAX3110_H
> >>> +
> >>> +/**
> >>> + * struct plat_max3110 - MAX3110 SPI UART platform data
> >>> + * @irq_edge_trigger: if IRQ is edge triggered
> >>> + *
> >>> + * You should use this structure in your machine description to specify
> >>> + * how the MAX3110 is connected.
> >>> + *
> >>> + */
> >>> +struct plat_max3110 {
> >>> + int irq_edge_triggered;
> >>> +};
> >>> +
> >>> +#endif
> >>> --
> >>
> >> Is just resource->flags for IRQ can be reused for handle such case?
> >
> > I believe your suggestion makes perfect sense. I'll rework it.
>
> Looks like isp_device has no place for 'resource'. In this case pdata
> seems to be the way to go here.
> Or maybe there's a better way to recommend?
What do you mean by the isp_device?
My idea is always use threaded irq and passing flags into request.
Like as:
unsigned long flags = res->flags & IORESOURCE_BITS;
...
request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
---
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
On 10/22/2013 11:27 PM, Alexander Shiyan wrote:
>> On 10/22/2013 01:30 PM, David Cohen wrote:
>>> On 10/22/2013 12:46 PM, Alexander Shiyan wrote:
>>>>> The change in the max3110 driver makes the IRQ handling threaded, now
>>>>> the handler is called only once per received character. Without that
>>>>> change, we had many (more than 100) interrupts per one received
>>>>> character.
>>>>>
>>>>> Unfortunately, SFI interface does not support IRQ polarity and
>>>>> triggering modes, so we have to keep the hacks as hard-coded device
>>>>> names and IRQ numbers until we switch to ACPI.
>>>>>
>>>>> Edge-triggered IRQ still supported to keep old platforms working.
>>>>> Use platform data to pass the irq mode argument.
>>>>>
>>>>> Signed-off-by: Ivan Gorinov <[email protected]>
>>>>> Signed-off-by: Li Ning <[email protected]>
>>>>> Signed-off-by: David Cohen <[email protected]>
>>>> ...
>>>>> +++ b/include/linux/serial_max3110.h
>>>>> @@ -0,0 +1,16 @@
>>>>> +#ifndef _LINUX_SERIAL_MAX3110_H
>>>>> +#define _LINUX_SERIAL_MAX3110_H
>>>>> +
>>>>> +/**
>>>>> + * struct plat_max3110 - MAX3110 SPI UART platform data
>>>>> + * @irq_edge_trigger: if IRQ is edge triggered
>>>>> + *
>>>>> + * You should use this structure in your machine description to specify
>>>>> + * how the MAX3110 is connected.
>>>>> + *
>>>>> + */
>>>>> +struct plat_max3110 {
>>>>> + int irq_edge_triggered;
>>>>> +};
>>>>> +
>>>>> +#endif
>>>>> --
>>>>
>>>> Is just resource->flags for IRQ can be reused for handle such case?
>>>
>>> I believe your suggestion makes perfect sense. I'll rework it.
>>
>> Looks like isp_device has no place for 'resource'. In this case pdata
>> seems to be the way to go here.
>> Or maybe there's a better way to recommend?
>
> What do you mean by the isp_device?
Allow me to correct my typo: I meant spi_device (this is a spi
device/driver).
>
> My idea is always use threaded irq and passing flags into request.
> Like as:
> unsigned long flags = res->flags & IORESOURCE_BITS;
> ...
> request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
Oh, maybe we were talking about different things afterall :)
The reason this struct plat_max3110 was created is to allow platform
code (located under arch/x86/platform/intel-mid/device_libs/) to define
the irq edge type.
When I saw your comment I though you were referring to struct resource
(which has IORESOURCE_IRQ_* flags). But unlike platform_device,
spi_device has no struct resource * to replace the need of struct
plat_max3110.
OTOH your suggestion can replace this piece of code:
@@ -68,6 +69,7 @@ struct uart_max3110 {
u8 clock;
u8 parity, word_7bits;
u16 irq;
+ u16 irq_edge_triggered;
unsigned long uart_flags;
I'll check again the patch and possibly make the changes.
Thanks,
David Cohen
Hi,
On Wed, Oct 23, 2013 at 11:10:48AM -0700, David Cohen wrote:
> >My idea is always use threaded irq and passing flags into request.
> >Like as:
> >unsigned long flags = res->flags & IORESOURCE_BITS;
> >...
> >request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
>
>
> Oh, maybe we were talking about different things afterall :)
> The reason this struct plat_max3110 was created is to allow platform
> code (located under arch/x86/platform/intel-mid/device_libs/) to define
> the irq edge type.
> When I saw your comment I though you were referring to struct resource
> (which has IORESOURCE_IRQ_* flags). But unlike platform_device,
> spi_device has no struct resource * to replace the need of struct
> plat_max3110.
>
> OTOH your suggestion can replace this piece of code:
>
> @@ -68,6 +69,7 @@ struct uart_max3110 {
> u8 clock;
> u8 parity, word_7bits;
> u16 irq;
> + u16 irq_edge_triggered;
max3110 is already edge triggered:
495 ret = request_irq(max->irq, serial_m3110_irq,
496 IRQ_TYPE_EDGE_FALLING, "max3110", max);
it would be nice a threaded IRQ instead of using a singlethread
workqueue, though.
--
balbi
Hi again,
On Wed, Oct 23, 2013 at 01:21:43PM -0500, Felipe Balbi wrote:
> On Wed, Oct 23, 2013 at 11:10:48AM -0700, David Cohen wrote:
> > >My idea is always use threaded irq and passing flags into request.
> > >Like as:
> > >unsigned long flags = res->flags & IORESOURCE_BITS;
> > >...
> > >request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
> >
> >
> > Oh, maybe we were talking about different things afterall :)
> > The reason this struct plat_max3110 was created is to allow platform
> > code (located under arch/x86/platform/intel-mid/device_libs/) to define
> > the irq edge type.
> > When I saw your comment I though you were referring to struct resource
> > (which has IORESOURCE_IRQ_* flags). But unlike platform_device,
> > spi_device has no struct resource * to replace the need of struct
> > plat_max3110.
> >
> > OTOH your suggestion can replace this piece of code:
> >
> > @@ -68,6 +69,7 @@ struct uart_max3110 {
> > u8 clock;
> > u8 parity, word_7bits;
> > u16 irq;
> > + u16 irq_edge_triggered;
>
> max3110 is already edge triggered:
>
> 495 ret = request_irq(max->irq, serial_m3110_irq,
> 496 IRQ_TYPE_EDGE_FALLING, "max3110", max);
>
> it would be nice a threaded IRQ instead of using a singlethread
> workqueue, though.
making it clearer, you only converted to threaded IRQ when
edge_triggered isn't set, why don't you *always* use threaded IRQs ?
--
balbi
On 10/23/2013 11:21 AM, Felipe Balbi wrote:
> Hi,
>
> On Wed, Oct 23, 2013 at 11:10:48AM -0700, David Cohen wrote:
>>> My idea is always use threaded irq and passing flags into request.
>>> Like as:
>>> unsigned long flags = res->flags & IORESOURCE_BITS;
>>> ...
>>> request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
>>
>>
>> Oh, maybe we were talking about different things afterall :)
>> The reason this struct plat_max3110 was created is to allow platform
>> code (located under arch/x86/platform/intel-mid/device_libs/) to define
>> the irq edge type.
>> When I saw your comment I though you were referring to struct resource
>> (which has IORESOURCE_IRQ_* flags). But unlike platform_device,
>> spi_device has no struct resource * to replace the need of struct
>> plat_max3110.
>>
>> OTOH your suggestion can replace this piece of code:
>>
>> @@ -68,6 +69,7 @@ struct uart_max3110 {
>> u8 clock;
>> u8 parity, word_7bits;
>> u16 irq;
>> + u16 irq_edge_triggered;
>
> max3110 is already edge triggered:
>
> 495 ret = request_irq(max->irq, serial_m3110_irq,
> 496 IRQ_TYPE_EDGE_FALLING, "max3110", max);
Yeah. But in Merrifield case (at least the reference board used now)
it's not edge triggered. I need this driver to support this situation
prior to send mrst_max3110 platform code for it.
>
> it would be nice a threaded IRQ instead of using a singlethread
> workqueue, though.
>
That sounds reasonable. I'll add this to my TODO list too. Thanks.
Br, David Cohen
Hi,
On Wed, Oct 23, 2013 at 11:33:45AM -0700, David Cohen wrote:
> On 10/23/2013 11:21 AM, Felipe Balbi wrote:
> >Hi,
> >
> >On Wed, Oct 23, 2013 at 11:10:48AM -0700, David Cohen wrote:
> >>>My idea is always use threaded irq and passing flags into request.
> >>>Like as:
> >>>unsigned long flags = res->flags & IORESOURCE_BITS;
> >>>...
> >>>request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
> >>
> >>
> >>Oh, maybe we were talking about different things afterall :)
> >>The reason this struct plat_max3110 was created is to allow platform
> >>code (located under arch/x86/platform/intel-mid/device_libs/) to define
> >>the irq edge type.
> >>When I saw your comment I though you were referring to struct resource
> >>(which has IORESOURCE_IRQ_* flags). But unlike platform_device,
> >>spi_device has no struct resource * to replace the need of struct
> >>plat_max3110.
> >>
> >>OTOH your suggestion can replace this piece of code:
> >>
> >>@@ -68,6 +69,7 @@ struct uart_max3110 {
> >> u8 clock;
> >> u8 parity, word_7bits;
> >> u16 irq;
> >>+ u16 irq_edge_triggered;
> >
> >max3110 is already edge triggered:
> >
> >495 ret = request_irq(max->irq, serial_m3110_irq,
> >496 IRQ_TYPE_EDGE_FALLING, "max3110", max);
>
> Yeah. But in Merrifield case (at least the reference board used now)
> it's not edge triggered. I need this driver to support this situation
> prior to send mrst_max3110 platform code for it.
got it. Then why don't you use irq_set_irq_type() from platform code and
change that request_irq() into:
ret = request_threaded_irq(max->irq, NULL, serial_m3110_irq,
IRQF_ONESHOT, "max3110", max);
if you call irq_set_irq_type() correctly, driver wouldn't need to know
the IRQ type.
--
balbi
On 10/23/2013 11:23 AM, Felipe Balbi wrote:
> Hi again,
>
> On Wed, Oct 23, 2013 at 01:21:43PM -0500, Felipe Balbi wrote:
>> On Wed, Oct 23, 2013 at 11:10:48AM -0700, David Cohen wrote:
>>>> My idea is always use threaded irq and passing flags into request.
>>>> Like as:
>>>> unsigned long flags = res->flags & IORESOURCE_BITS;
>>>> ...
>>>> request_threaded_irq(max->irq, serial_m3110_irq, IRQF_ONESHOT | flags, "max3110", max);
>>>
>>>
>>> Oh, maybe we were talking about different things afterall :)
>>> The reason this struct plat_max3110 was created is to allow platform
>>> code (located under arch/x86/platform/intel-mid/device_libs/) to define
>>> the irq edge type.
>>> When I saw your comment I though you were referring to struct resource
>>> (which has IORESOURCE_IRQ_* flags). But unlike platform_device,
>>> spi_device has no struct resource * to replace the need of struct
>>> plat_max3110.
>>>
>>> OTOH your suggestion can replace this piece of code:
>>>
>>> @@ -68,6 +69,7 @@ struct uart_max3110 {
>>> u8 clock;
>>> u8 parity, word_7bits;
>>> u16 irq;
>>> + u16 irq_edge_triggered;
>>
>> max3110 is already edge triggered:
>>
>> 495 ret = request_irq(max->irq, serial_m3110_irq,
>> 496 IRQ_TYPE_EDGE_FALLING, "max3110", max);
>>
>> it would be nice a threaded IRQ instead of using a singlethread
>> workqueue, though.
>
> making it clearer, you only converted to threaded IRQ when
> edge_triggered isn't set, why don't you *always* use threaded IRQs ?
This is a very difficult question to answer, considering the
non-threaded irq is used just to wake up a wq :)
I wish this driver was better maintained.
I'll rework this whole IRQ scheme here.
Br, David Cohen