Thread (2 messages) 2 messages, 1 author, 2014-09-28

Re: [PATCH RFC] drivers: parport: Ask user for irqreturn_t value

From: Matwey V. Kornilov <hidden>
Date: 2014-09-28 14:31:01
Also in: linux-hams, linux-i2c, linux-media, lkml, netdev

Forget it, I invented something strange.

2014-09-28 12:02 GMT+04:00 Matwey V. Kornilov [off-list ref]:
quoted hunk ↗ jump to hunk
Current parport_irq_handler behaviour is not correct when IRQ is shared.
LDDv3 on page 279 requires us:

"Be sure to return IRQ_NONE whenever your handler is called and finds
the device is not interrupting."

This is not the case of parport_irq_handler.
Current implementation of IRQ handling use this to optimize shared IRQ processing.
When an action returns IRQ_HANDLED no other actions are processed.

Modify callback function void (*irq_func)(void *) to irqreturn_t (*irq_func)(void *) and return the value in parport_irq_handler

This also fixes https://bugzilla.kernel.org/show_bug.cgi?id=85221

Signed-off-by: Matwey V. Kornilov <redacted>
---
 drivers/char/ppdev.c                       |  4 +++-
 drivers/i2c/busses/i2c-parport.c           |  5 ++++-
 drivers/input/joystick/walkera0701.c       |  6 ++++--
 drivers/net/hamradio/baycom_par.c          |  4 +++-
 drivers/net/plip/plip.c                    |  8 +++++---
 drivers/parport/share.c                    |  6 ++----
 drivers/pps/clients/pps_parport.c          |  6 +++---
 drivers/staging/media/lirc/lirc_parallel.c | 12 +++++++-----
 include/linux/parport.h                    | 12 ++++++++----
 9 files changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
index ae0b42b..61fec51 100644
--- a/drivers/char/ppdev.c
+++ b/drivers/char/ppdev.c
@@ -264,7 +264,7 @@ static ssize_t pp_write (struct file * file, const char __user * buf,
        return bytes_written;
 }

-static void pp_irq (void *private)
+static irqreturn_t pp_irq (void *private)
 {
        struct pp_struct *pp = private;
@@ -275,6 +275,8 @@ static void pp_irq (void *private)

        atomic_inc (&pp->irqc);
        wake_up_interruptible (&pp->irq_wait);
+
+       return IRQ_HANDLED;
 }

 static int register_device (int minor, struct pp_struct *pp)
diff --git a/drivers/i2c/busses/i2c-parport.c b/drivers/i2c/busses/i2c-parport.c
index a27aae2..7549e10 100644
--- a/drivers/i2c/busses/i2c-parport.c
+++ b/drivers/i2c/busses/i2c-parport.c
@@ -151,7 +151,7 @@ static const struct i2c_algo_bit_data parport_algo_data = {

 /* ----- I2c and parallel port call-back functions and structures --------- */

-static void i2c_parport_irq(void *data)
+static irqreturn_t i2c_parport_irq(void *data)
 {
        struct i2c_par *adapter = data;
        struct i2c_client *ara = adapter->ara;
@@ -159,9 +159,12 @@ static void i2c_parport_irq(void *data)
        if (ara) {
                dev_dbg(&ara->dev, "SMBus alert received\n");
                i2c_handle_smbus_alert(ara);
+               return IRQ_HANDLED;
        } else
                dev_dbg(&adapter->adapter.dev,
                        "SMBus alert received but no ARA client!\n");
+
+       return IRQ_NONE;
 }

 static void i2c_parport_attach(struct parport *port)
diff --git a/drivers/input/joystick/walkera0701.c b/drivers/input/joystick/walkera0701.c
index b76ac58..f3903a6 100644
--- a/drivers/input/joystick/walkera0701.c
+++ b/drivers/input/joystick/walkera0701.c
@@ -124,7 +124,7 @@ static inline int read_ack(struct pardevice *p)
 }

 /* falling edge, prepare to BIN value calculation */
-static void walkera0701_irq_handler(void *handler_data)
+static irqreturn_t walkera0701_irq_handler(void *handler_data)
 {
        u64 pulse_time;
        struct walkera_dev *w = handler_data;
@@ -136,7 +136,7 @@ static void walkera0701_irq_handler(void *handler_data)
        /* cancel timer, if in handler or active do resync */
        if (unlikely(0 != hrtimer_try_to_cancel(&w->timer))) {
                w->counter = NO_SYNC;
-               return;
+               return IRQ_HANDLED;
        }

        if (w->counter < NO_SYNC) {
@@ -166,6 +166,8 @@ static void walkera0701_irq_handler(void *handler_data)
                w->counter = 0;

        hrtimer_start(&w->timer, ktime_set(0, BIN_SAMPLE), HRTIMER_MODE_REL);
+
+       return IRQ_HANDLED;
 }

 static enum hrtimer_restart timer_handler(struct hrtimer
diff --git a/drivers/net/hamradio/baycom_par.c b/drivers/net/hamradio/baycom_par.c
index acb6369..1fd9a7b 100644
--- a/drivers/net/hamradio/baycom_par.c
+++ b/drivers/net/hamradio/baycom_par.c
@@ -269,7 +269,7 @@ static __inline__ void par96_rx(struct net_device *dev, struct baycom_state *bc)

 /* --------------------------------------------------------------------- */

-static void par96_interrupt(void *dev_id)
+static irqreturn_t par96_interrupt(void *dev_id)
 {
        struct net_device *dev = dev_id;
        struct baycom_state *bc = netdev_priv(dev);
@@ -292,6 +292,8 @@ static void par96_interrupt(void *dev_id)
        hdlcdrv_transmitter(dev, &bc->hdrv);
        hdlcdrv_receiver(dev, &bc->hdrv);
         local_irq_disable();
+
+       return IRQ_HANDLED;
 }

 /* --------------------------------------------------------------------- */
diff --git a/drivers/net/plip/plip.c b/drivers/net/plip/plip.c
index 040b897..00a8b3a 100644
--- a/drivers/net/plip/plip.c
+++ b/drivers/net/plip/plip.c
@@ -143,7 +143,7 @@ static void plip_bh(struct work_struct *work);
 static void plip_timer_bh(struct work_struct *work);

 /* Interrupt handler */
-static void plip_interrupt(void *dev_id);
+static irqreturn_t plip_interrupt(void *dev_id);

 /* Functions for DEV methods */
 static int plip_tx_packet(struct sk_buff *skb, struct net_device *dev);
@@ -900,7 +900,7 @@ plip_error(struct net_device *dev, struct net_local *nl,
 }

 /* Handle the parallel port interrupts. */
-static void
+static irqreturn_t
 plip_interrupt(void *dev_id)
 {
        struct net_device *dev = dev_id;
@@ -919,7 +919,7 @@ plip_interrupt(void *dev_id)
                if ((dev->irq != -1) && (net_debug > 1))
                        printk(KERN_DEBUG "%s: spurious interrupt\n", dev->name);
                spin_unlock_irqrestore (&nl->lock, flags);
-               return;
+               return IRQ_NONE;
        }

        if (net_debug > 3)
@@ -948,6 +948,8 @@ plip_interrupt(void *dev_id)
        }

        spin_unlock_irqrestore(&nl->lock, flags);
+
+       return IRQ_HANDLED;
 }

 static int
diff --git a/drivers/parport/share.c b/drivers/parport/share.c
index 3fa6624..1fd8dc2 100644
--- a/drivers/parport/share.c
+++ b/drivers/parport/share.c
@@ -523,7 +523,7 @@ void parport_remove_port(struct parport *port)
 struct pardevice *
 parport_register_device(struct parport *port, const char *name,
                        int (*pf)(void *), void (*kf)(void *),
-                       void (*irq_func)(void *),
+                       irqreturn_t (*irq_func)(void *),
                        int flags, void *handle)
 {
        struct pardevice *tmp;
@@ -1006,9 +1006,7 @@ irqreturn_t parport_irq_handler(int irq, void *dev_id)
 {
        struct parport *port = dev_id;

-       parport_generic_irq(port);
-
-       return IRQ_HANDLED;
+       return parport_generic_irq(port);
 }

 /* Exported symbols for modules. */
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
index 38a8bbe..9acf376 100644
--- a/drivers/pps/clients/pps_parport.c
+++ b/drivers/pps/clients/pps_parport.c
@@ -64,7 +64,7 @@ static inline int signal_is_set(struct parport *port)
 }

 /* parport interrupt handler */
-static void parport_irq(void *handle)
+static irqreturn_t parport_irq(void *handle)
 {
        struct pps_event_time ts_assert, ts_clear;
        struct pps_client_pp *dev = handle;
@@ -122,7 +122,7 @@ out_assert:
        /* fire assert event */
        pps_event(dev->pps, &ts_assert,
                        PPS_CAPTUREASSERT, NULL);
-       return;
+       return IRQ_HANDLED;

 out_both:
        /* fire assert event */
@@ -131,7 +131,7 @@ out_both:
        /* fire clear event */
        pps_event(dev->pps, &ts_clear,
                        PPS_CAPTURECLEAR, NULL);
-       return;
+       return IRQ_HANDLED;
 }

 static void parport_attach(struct parport *port)
diff --git a/drivers/staging/media/lirc/lirc_parallel.c b/drivers/staging/media/lirc/lirc_parallel.c
index 672858a..63fdc27 100644
--- a/drivers/staging/media/lirc/lirc_parallel.c
+++ b/drivers/staging/media/lirc/lirc_parallel.c
@@ -219,7 +219,7 @@ static void rbuf_write(int signal)
        wptr = nwptr;
 }

-static void lirc_lirc_irq_handler(void *blah)
+static irqreturn_t lirc_lirc_irq_handler(void *blah)
 {
        struct timeval tv;
        static struct timeval lasttv;
@@ -230,10 +230,10 @@ static void lirc_lirc_irq_handler(void *blah)
        unsigned int timeout;

        if (!is_open)
-               return;
+               return IRQ_NONE;

        if (!is_claimed)
-               return;
+               return IRQ_NONE;

 #if 0
        /* disable interrupt */
@@ -241,7 +241,7 @@ static void lirc_lirc_irq_handler(void *blah)
          out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ) & (~LP_PINTEN));
 #endif
        if (check_pselecd && (in(1) & LP_PSELECD))
-               return;
+               return IRQ_NONE;

 #ifdef LIRC_TIMER
        if (init) {
@@ -265,7 +265,7 @@ static void lirc_lirc_irq_handler(void *blah)
                         */
                        timer = init_lirc_timer();
                        /* enable_irq(irq); */
-                       return;
+                       return IRQ_HANDLED;
                }
                init = 1;
        }
@@ -314,6 +314,8 @@ static void lirc_lirc_irq_handler(void *blah)
          enable_irq(irq);
          out(LIRC_PORT_IRQ, in(LIRC_PORT_IRQ)|LP_PINTEN);
        */
+
+       return IRQ_HANDLED;
 }

 /*** file operations ***/
diff --git a/include/linux/parport.h b/include/linux/parport.h
index c22f125..2ab774c 100644
--- a/include/linux/parport.h
+++ b/include/linux/parport.h
@@ -141,7 +141,7 @@ struct pardevice {
        int (*preempt)(void *);
        void (*wakeup)(void *);
        void *private;
-       void (*irq_func)(void *);
+       irqreturn_t (*irq_func)(void *);
        unsigned int flags;
        struct pardevice *next;
        struct pardevice *prev;
@@ -298,7 +298,7 @@ extern void parport_put_port (struct parport *);
 struct pardevice *parport_register_device(struct parport *port,
                          const char *name,
                          int (*pf)(void *), void (*kf)(void *),
-                         void (*irq_func)(void *),
+                         irqreturn_t (*irq_func)(void *),
                          int flags, void *handle);

 /* parport_unregister unlinks a device from the chain. */
@@ -429,13 +429,17 @@ extern void parport_daisy_deselect_all (struct parport *port);
 extern int parport_daisy_select (struct parport *port, int daisy, int mode);

 /* Lowlevel drivers _can_ call this support function to handle irqs.  */
-static inline void parport_generic_irq(struct parport *port)
+static inline irqreturn_t parport_generic_irq(struct parport *port)
 {
+       irqreturn_t ret = IRQ_NONE;
+
        parport_ieee1284_interrupt (port);
        read_lock(&port->cad_lock);
        if (port->cad && port->cad->irq_func)
-               port->cad->irq_func(port->cad->private);
+               ret = port->cad->irq_func(port->cad->private);
        read_unlock(&port->cad_lock);
+
+       return ret;
 }

 /* Prototypes from parport_procfs */
--
2.1.0


-- 
With best regards,
Matwey V. Kornilov.
Sternberg Astronomical Institute, Lomonosov Moscow State University, Russia
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help