On Tue, 31 Jan 2017 12:54:37 -0800
Dmitry Torokhov <dmitry.torok...@gmail.com> wrote:
...
> +
> +static irqreturn_t ht16k33_keypad_irq_thread(int irq, void *dev)
> +{
> + struct ht16k33_keypad *keypad = dev;
> +
> + do {
> + wait_event_timeout(keypad->wait, keypad->stopped,
> + msecs_to_jiffies(keypad->debounce_ms));
> + if (keypad->stopped)
> + break;
> + } while (ht16k33_keypad_scan(keypad));
I like how you worked this one out! Its way cleaner and simpler.
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ht16k33_keypad_start(struct input_dev *dev)
> +{
> + struct ht16k33_keypad *keypad = input_get_drvdata(dev);
> +
> + keypad->stopped = false;
> + mb();
> + enable_irq(keypad->client->irq);
> +
> + return 0;
> +}
> +
> +static void ht16k33_keypad_stop(struct input_dev *dev)
> +{
> + struct ht16k33_keypad *keypad = input_get_drvdata(dev);
> +
> + keypad->stopped = true;
> + mb();
> + wake_up(&keypad->wait);
> + disable_irq(keypad->client->irq);
> +}
Nice!
> +
> +static int ht16k33_keypad_probe(struct i2c_client *client,
> + struct ht16k33_keypad *keypad)
> +{
...
> +
> + err = devm_request_threaded_irq(&client->dev, client->irq,
> + NULL, ht16k33_keypad_irq_thread,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + DRIVER_NAME, keypad);
I believe the interrupt trigger should be switched to IRQF_TRIGGER_HIGH
combined with IRQF_ONESHOT, for this new setup. This is causing a race
condition.
In the previous situation a 'key data' read was triggered by
ht16k33_keypad_start(). This cleared the IRQ and gave a (somewhat racy)
known state. Now, when the IRQ line is HIGH during probe keyscan
wont work.
Also, when ht16k33_keypad_stop() is run while the irq thread is waiting
for debounce, the scan will be skipped and irq will never be cleared.
This also breaks keyscan.
Regards,
Robin
