RE: [PATCH 2/4] Input: adp5588-keys - switch to using threaded interrupt

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




> -----Original Message-----
> From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Sent: Samstag, 28. Mai 2022 06:56
> To: Hennerich, Michael <Michael.Hennerich@xxxxxxxxxx>
> Cc: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>; linux-
> input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH 2/4] Input: adp5588-keys - switch to using threaded interrupt
> 
> [External]
> 
> Instead of using hard interrupt handler and manually scheduling work item to
> handle I2C communications, let's switch to threaded interrupt handling.
> 
> While at that enforce the readout delay required on pre- revision 4 silicon.
> 

Looks good to me. I'll test this altogether with the devicetree and matrix_keypad helper updates.

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>

Acked-by: Michael Hennerich <michael.hennerich@xxxxxxxxxx>

> ---
>  drivers/input/keyboard/adp5588-keys.c | 81 +++++++++++++++------------
>  1 file changed, 45 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/input/keyboard/adp5588-keys.c
> b/drivers/input/keyboard/adp5588-keys.c
> index ea67d0834be1..ac21873ba1d7 100644
> --- a/drivers/input/keyboard/adp5588-keys.c
> +++ b/drivers/input/keyboard/adp5588-keys.c
> @@ -8,17 +8,19 @@
>   * Copyright (C) 2008-2010 Analog Devices Inc.
>   */
> 
> -#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/i2c.h>
> +#include <linux/input.h>
>  #include <linux/interrupt.h>
>  #include <linux/irq.h>
> -#include <linux/workqueue.h>
> -#include <linux/errno.h>
> -#include <linux/pm.h>
> +#include <linux/ktime.h>
> +#include <linux/module.h>
>  #include <linux/platform_device.h>
> -#include <linux/input.h>
> -#include <linux/i2c.h>
> -#include <linux/gpio/driver.h>
> +#include <linux/pm.h>
>  #include <linux/slab.h>
> +#include <linux/timekeeping.h>
> 
>  #include <linux/platform_data/adp5588.h>
> 
> @@ -36,11 +38,12 @@
>   * asserted.
>   */
>  #define WA_DELAYED_READOUT_REVID(rev)		((rev) < 4)
> +#define WA_DELAYED_READOUT_TIME			25
> 
>  struct adp5588_kpad {
>  	struct i2c_client *client;
>  	struct input_dev *input;
> -	struct delayed_work work;
> +	ktime_t irq_time;
>  	unsigned long delay;
>  	unsigned short keycode[ADP5588_KEYMAPSIZE];
>  	const struct adp5588_gpi_map *gpimap;
> @@ -289,13 +292,36 @@ static void adp5588_report_events(struct
> adp5588_kpad *kpad, int ev_cnt)
>  	}
>  }
> 
> -static void adp5588_work(struct work_struct *work)
> +static irqreturn_t adp5588_hard_irq(int irq, void *handle) {
> +	struct adp5588_kpad *kpad = handle;
> +
> +	kpad->irq_time = ktime_get();
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t adp5588_thread_irq(int irq, void *handle)
>  {
> -	struct adp5588_kpad *kpad = container_of(work,
> -						struct adp5588_kpad,
> work.work);
> +	struct adp5588_kpad *kpad = handle;
>  	struct i2c_client *client = kpad->client;
> +	ktime_t target_time, now;
> +	unsigned long delay;
>  	int status, ev_cnt;
> 
> +	/*
> +	 * Readout needs to wait for at least 25ms after the notification
> +	 * for REVID < 4.
> +	 */
> +	if (kpad->delay) {
> +		target_time = ktime_add_ms(kpad->irq_time, kpad->delay);
> +		now = ktime_get();
> +		if (ktime_before(now, target_time)) {
> +			delay = ktime_to_us(ktime_sub(target_time, now));
> +			usleep_range(delay, delay + 1000);
> +		}
> +	}
> +
>  	status = adp5588_read(client, INT_STAT);
> 
>  	if (status & ADP5588_OVR_FLOW_INT)	/* Unlikely and should never
> happen */
> @@ -308,20 +334,8 @@ static void adp5588_work(struct work_struct *work)
>  			input_sync(kpad->input);
>  		}
>  	}
> -	adp5588_write(client, INT_STAT, status); /* Status is W1C */
> -}
> 
> -static irqreturn_t adp5588_irq(int irq, void *handle) -{
> -	struct adp5588_kpad *kpad = handle;
> -
> -	/*
> -	 * use keventd context to read the event fifo registers
> -	 * Schedule readout at least 25ms after notification for
> -	 * REVID < 4
> -	 */
> -
> -	schedule_delayed_work(&kpad->work, kpad->delay);
> +	adp5588_write(client, INT_STAT, status); /* Status is W1C */
> 
>  	return IRQ_HANDLED;
>  }
> @@ -505,7 +519,6 @@ static int adp5588_probe(struct i2c_client *client,
> 
>  	kpad->client = client;
>  	kpad->input = input;
> -	INIT_DELAYED_WORK(&kpad->work, adp5588_work);
> 
>  	ret = adp5588_read(client, DEV_ID);
>  	if (ret < 0) {
> @@ -515,7 +528,7 @@ static int adp5588_probe(struct i2c_client *client,
> 
>  	revid = (u8) ret & ADP5588_DEVICE_ID_MASK;
>  	if (WA_DELAYED_READOUT_REVID(revid))
> -		kpad->delay = msecs_to_jiffies(30);
> +		kpad->delay =
> msecs_to_jiffies(WA_DELAYED_READOUT_TIME);
> 
>  	input->name = client->name;
>  	input->phys = "adp5588-keys/input0";
> @@ -560,9 +573,10 @@ static int adp5588_probe(struct i2c_client *client,
>  		goto err_free_mem;
>  	}
> 
> -	error = request_irq(client->irq, adp5588_irq,
> -			    IRQF_TRIGGER_FALLING,
> -			    client->dev.driver->name, kpad);
> +	error = request_threaded_irq(client->irq,
> +				     adp5588_hard_irq, adp5588_thread_irq,
> +				     IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
> +				     client->dev.driver->name, kpad);
>  	if (error) {
>  		dev_err(&client->dev, "irq %d busy?\n", client->irq);
>  		goto err_unreg_dev;
> @@ -587,7 +601,6 @@ static int adp5588_probe(struct i2c_client *client,
> 
>   err_free_irq:
>  	free_irq(client->irq, kpad);
> -	cancel_delayed_work_sync(&kpad->work);
>   err_unreg_dev:
>  	input_unregister_device(input);
>  	input = NULL;
> @@ -604,7 +617,6 @@ static int adp5588_remove(struct i2c_client *client)
> 
>  	adp5588_write(client, CFG, 0);
>  	free_irq(client->irq, kpad);
> -	cancel_delayed_work_sync(&kpad->work);
>  	input_unregister_device(kpad->input);
>  	adp5588_gpio_remove(kpad);
>  	kfree(kpad);
> @@ -614,11 +626,9 @@ static int adp5588_remove(struct i2c_client *client)
> 
>  static int __maybe_unused adp5588_suspend(struct device *dev)  {
> -	struct adp5588_kpad *kpad = dev_get_drvdata(dev);
> -	struct i2c_client *client = kpad->client;
> +	struct i2c_client *client = to_i2c_client(dev);
> 
>  	disable_irq(client->irq);
> -	cancel_delayed_work_sync(&kpad->work);
> 
>  	if (device_may_wakeup(&client->dev))
>  		enable_irq_wake(client->irq);
> @@ -628,8 +638,7 @@ static int __maybe_unused adp5588_suspend(struct
> device *dev)
> 
>  static int __maybe_unused adp5588_resume(struct device *dev)  {
> -	struct adp5588_kpad *kpad = dev_get_drvdata(dev);
> -	struct i2c_client *client = kpad->client;
> +	struct i2c_client *client = to_i2c_client(dev);
> 
>  	if (device_may_wakeup(&client->dev))
>  		disable_irq_wake(client->irq);
> --
> 2.36.1.124.g0e6072fb45-goog





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux