davidker / unisys

Master repository for new changes to drivers/staging/unisys and drivers/visorbus
Other
2 stars 1 forks source link

visorbus_enable_channel_interrupts() should fail if no channel_interrupt function provided #65

Closed selltc closed 7 years ago

selltc commented 8 years ago

KanBoard-1339

David K and I noticed this while discussing whether or not commit 35c78e2fdae63d67572a151a61cd3b0ee7298efb (staging: unisys: Re-enable interrupts after we have done the work) should be tweaked as follows:

--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -536,8 +536,6 @@ dev_periodic_work(unsigned long __opaque)

        if (drv->channel_interrupt)
                drv->channel_interrupt(dev);
-       else
-               mod_timer(&dev->timer, jiffies + POLLJIFFIES_NORMALCHANNEL);
}

static void

That's logically how it originally was prior to my periodic_work --> kernel timer patch (which introduced the mod_timer() calls.)

After discussing this more, we actually came to the conclusion that something like this would make the most sense:

--- a/drivers/staging/unisys/visorbus/visorbus_main.c
+++ b/drivers/staging/unisys/visorbus/visorbus_main.c
@@ -534,10 +534,7 @@ dev_periodic_work(unsigned long __opaque)
        struct visor_device *dev = (struct visor_device *) __opaque;
        struct visor_driver *drv = to_visor_driver(dev->device.driver);

-       if (drv->channel_interrupt)
-               drv->channel_interrupt(dev);
-       else
-               mod_timer(&dev->timer, jiffies + POLLJIFFIES_NORMALCHANNEL);
+       drv->channel_interrupt(dev);
}

static void
@@ -703,6 +700,11 @@ EXPORT_SYMBOL_GPL(visorbus_write_channel);
void
visorbus_enable_channel_interrupts(struct visor_device *dev)
{
+       struct visor_driver *drv = to_visor_driver(dev->device.driver);
+
+       if (!drv->channel_interrupt)
+               return;
+
        if (dev->irq)
                visorchannel_set_sig_features(dev->visorchannel,
                                              dev->recv_queue,
@@ -750,12 +752,8 @@ visorbus_isr(int irq, void *dev_id)
        visorchannel_clear_sig_features(dev->visorchannel,
                                        dev->recv_queue,
                                        ULTRA_CHANNEL_ENABLE_INTS);
-       if (drv->channel_interrupt) {
-               drv->channel_interrupt(dev);
-               return IRQ_HANDLED;
-       }
-
-       return IRQ_NONE;
+       drv->channel_interrupt(dev);
+       return IRQ_HANDLED;
}

int visorbus_set_channel_features(struct visor_device *dev, u64 feature_bits)
selltc commented 8 years ago

I have a branch for-later-proposed-selltc-20160526 that is a re-written version of for-later, including only the least-risky/controversial visorinput fixes. Refer to github #58 for more details about this branch. commit 903529fd476d508174b60f8fd7216616e4b0f4a6 is on that branch, and addresses this github issue. It is checkpatch-clean and tested clean.

selltc commented 8 years ago

Commit b030539e2a092ef8e4e629e20538c31aff3f720c takes care of this issue in the latest version of for-later.

davidker commented 7 years ago

This is closed in staging-next with 396e36c