[PATCH v6 0/3] Add a property to turn off the max touch controller if not used

[PATCH v6 0/3] Add a property to turn off the max touch controller if not used

[Stefan Eichenberger]

Our hardware has a shared regulator that powers various peripherals such
as the display, touch, USB hub, etc. Since the Maxtouch controller
doesn't currently allow it to be turned off, this regulator has to stay
on when not used. This increases the overall power consumption. In order
to turn off the controller when the system does not use it, this series
adds a device tree property to the Maxtouch driver that allows the
controller to be turned off completely and ensures that it can resume
from the power off state.

Changes since v5:

- Keep reset pin untouched in mxt_power_off (Dmitry)
- Generate proper reset signal in mxt_power_on (Dmitry)
- Drop introduction of mxt_device_register (not necessary)

Changes since v4:

- Load configuration firmware during probe and not after resume (Dmitry)
- Do some improvements on error handling (Dmitry)
- Add Reviewed-by tag from Joao

Changes since v3:

- Move the power on part to mxt_start and the power off part to
  mxt_stop. This allows to turn the touch controller off even when not
  in use and not only when being suspended (Dmitry)

Changes since v2:

- Add Reviewed-by tags from Linus and Krzysztof to the dt-bindings patch

Changes since v1:

- Rename the property and change the description (Krzysztof, Linus,
  Dmitry, Conor)

Stefan Eichenberger (3):
  Input: atmel_mxt_ts - add power off and power on functions
  dt-bindings: input: atmel,maxtouch: add poweroff-sleep property
  Input: atmel_mxt_ts - add support for poweroff-sleep

 .../bindings/input/atmel,maxtouch.yaml        |   6 +
 drivers/input/touchscreen/atmel_mxt_ts.c      | 134 +++++++++++++-----
 2 files changed, 108 insertions(+), 32 deletions(-)

-- 
2.43.0

[PATCH v6 1/3] Input: atmel_mxt_ts - add power off and power on functions

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Add a separate function for power off and power on instead of calling
regulator_bulk_enable and regulator_bulk_disable directly.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Joao Paulo Goncalves <joao.goncalves@toradex.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 56 ++++++++++++++----------
 1 file changed, 34 insertions(+), 22 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 8a606bd441ae6..9416de53bf9af 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1307,6 +1307,35 @@ static int mxt_soft_reset(struct mxt_data *data)
 	return 0;
 }
 
+static int mxt_power_on(struct mxt_data *data)
+{
+	int error;
+
+	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
+				      data->regulators);
+	if (error) {
+		dev_err(&data->client->dev, "failed to enable regulators: %d\n",
+			error);
+		return error;
+	}
+
+	msleep(MXT_BACKUP_TIME);
+
+	if (data->reset_gpio) {
+		/* Wait a while and then de-assert the RESET GPIO line */
+		msleep(MXT_RESET_GPIO_TIME);
+		gpiod_set_value(data->reset_gpio, 0);
+		msleep(MXT_RESET_INVALID_CHG);
+	}
+
+	return 0;
+}
+
+static void mxt_power_off(struct mxt_data *data)
+{
+	regulator_bulk_disable(ARRAY_SIZE(data->regulators), data->regulators);
+}
+
 static void mxt_update_crc(struct mxt_data *data, u8 cmd, u8 value)
 {
 	/*
@@ -3305,25 +3334,9 @@ static int mxt_probe(struct i2c_client *client)
 		return error;
 	}
 
-	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
-				      data->regulators);
-	if (error) {
-		dev_err(&client->dev, "failed to enable regulators: %d\n",
-			error);
+	error = mxt_power_on(data);
+	if (error)
 		return error;
-	}
-	/*
-	 * The device takes 40ms to come up after power-on according
-	 * to the mXT224 datasheet, page 13.
-	 */
-	msleep(MXT_BACKUP_TIME);
-
-	if (data->reset_gpio) {
-		/* Wait a while and then de-assert the RESET GPIO line */
-		msleep(MXT_RESET_GPIO_TIME);
-		gpiod_set_value(data->reset_gpio, 0);
-		msleep(MXT_RESET_INVALID_CHG);
-	}
 
 	/*
 	 * Controllers like mXT1386 have a dedicated WAKE line that could be
@@ -3361,8 +3374,8 @@ static int mxt_probe(struct i2c_client *client)
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
 err_disable_regulators:
-	regulator_bulk_disable(ARRAY_SIZE(data->regulators),
-			       data->regulators);
+	mxt_power_off(data);
+
 	return error;
 }
 
@@ -3374,8 +3387,7 @@ static void mxt_remove(struct i2c_client *client)
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
-	regulator_bulk_disable(ARRAY_SIZE(data->regulators),
-			       data->regulators);
+	mxt_power_off(data);
 }
 
 static int mxt_suspend(struct device *dev)
-- 
2.43.0

[PATCH v6 2/3] dt-bindings: input: atmel,maxtouch: add poweroff-sleep property

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Add a new property to indicate that the device should power off rather
than use deep sleep. Deep sleep is a feature of the controller that
expects the controller to remain powered in suspend. However, if a
display shares its regulator with the touch controller, we may want to
do a power off so that the display and touch controller do not use any
power.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Joao Paulo Goncalves <joao.goncalves@toradex.com>
---
 Documentation/devicetree/bindings/input/atmel,maxtouch.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
index c40799355ed75..8de5f539b30e3 100644
--- a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
+++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
@@ -87,6 +87,12 @@ properties:
       - 2 # ATMEL_MXT_WAKEUP_GPIO
     default: 0
 
+  atmel,poweroff-sleep:
+    description: |
+      Instead of using the deep sleep feature of the maXTouch controller,
+      poweroff the regulators.
+    type: boolean
+
   wakeup-source:
     type: boolean
 
-- 
2.43.0

Re: [PATCH v6 2/3] dt-bindings: input: atmel,maxtouch: add poweroff-sleep property

[Krzysztof Kozlowski]

On 15/07/2024 17:31, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> Add a new property to indicate that the device should power off rather
> than use deep sleep. Deep sleep is a feature of the controller that
> expects the controller to remain powered in suspend. However, if a
> display shares its regulator with the touch controller, we may want to
> do a power off so that the display and touch controller do not use any
> power.
> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Joao Paulo Goncalves <joao.goncalves@toradex.com>
> ---
>  Documentation/devicetree/bindings/input/atmel,maxtouch.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
> index c40799355ed75..8de5f539b30e3 100644
> --- a/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
> +++ b/Documentation/devicetree/bindings/input/atmel,maxtouch.yaml
> @@ -87,6 +87,12 @@ properties:
>        - 2 # ATMEL_MXT_WAKEUP_GPIO
>      default: 0
>  
> +  atmel,poweroff-sleep:
> +    description: |

Since you keep sending new versions (fourth? fifth?):
Do not need '|' unless you need to preserve formatting.

Best regards,
Krzysztof

[PATCH v6 3/3] Input: atmel_mxt_ts - add support for poweroff-sleep

[Stefan Eichenberger]

From: Stefan Eichenberger <stefan.eichenberger@toradex.com>

Add support for poweroff-sleep to the Atmel maXTouch driver. This allows
us to power off the input device entirely and only power it on when it
is opened. This will also automatically power it off when we suspend the
system.

Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
---
 drivers/input/touchscreen/atmel_mxt_ts.c | 82 ++++++++++++++++++++----
 1 file changed, 70 insertions(+), 12 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 9416de53bf9af..46ed3dbf0c5ed 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -265,6 +265,7 @@ enum v4l_dbg_inputs {
 enum mxt_suspend_mode {
 	MXT_SUSPEND_DEEP_SLEEP	= 0,
 	MXT_SUSPEND_T9_CTRL	= 1,
+	MXT_SUSPEND_POWEROFF	= 2,
 };
 
 /* Config update context */
@@ -1311,6 +1312,10 @@ static int mxt_power_on(struct mxt_data *data)
 {
 	int error;
 
+	/* Make sure the device is in reset before enabling power */
+	if (data->reset_gpio)
+		gpiod_set_value_cansleep(data->reset_gpio, 1);
+
 	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
 				      data->regulators);
 	if (error) {
@@ -2270,8 +2275,38 @@ static int mxt_configure_objects(struct mxt_data *data,
 
 static void mxt_config_cb(const struct firmware *cfg, void *ctx)
 {
+	struct mxt_data *data = ctx;
+
 	mxt_configure_objects(ctx, cfg);
 	release_firmware(cfg);
+
+	if ((data->suspend_mode == MXT_SUSPEND_POWEROFF) && !data->in_bootloader) {
+		disable_irq(data->irq);
+		mxt_power_off(data);
+	}
+}
+
+static void mxt_initialize_after_resume(struct mxt_data *data)
+{
+	int error;
+
+	error = mxt_power_on(data);
+	if (error) {
+		dev_err(&data->client->dev, "Failed to power on device\n");
+		return;
+	}
+
+	error = mxt_acquire_irq(data);
+	if (error) {
+		dev_err(&data->client->dev, "Failed to acquire IRQ\n");
+		return;
+	}
+
+	error = mxt_configure_objects(data, NULL);
+	if (error) {
+		dev_err(&data->client->dev, "Failed to configure objects\n");
+		return;
+	}
 }
 
 static int mxt_initialize(struct mxt_data *data)
@@ -2828,15 +2863,18 @@ static int mxt_configure_objects(struct mxt_data *data,
 			dev_warn(dev, "Error %d updating config\n", error);
 	}
 
-	if (data->multitouch) {
-		error = mxt_initialize_input_device(data);
-		if (error)
-			return error;
-	} else {
-		dev_warn(dev, "No touch object detected\n");
-	}
+	/* Do not initialize and register input device twice */
+	if (!data->input_dev) {
+		if (data->multitouch) {
+			error = mxt_initialize_input_device(data);
+			if (error)
+				return error;
+		} else {
+			dev_warn(dev, "No touch object detected\n");
+		}
 
-	mxt_debug_init(data);
+		mxt_debug_init(data);
+	}
 
 	return 0;
 }
@@ -3070,6 +3108,12 @@ static ssize_t mxt_update_fw_store(struct device *dev,
 	struct mxt_data *data = dev_get_drvdata(dev);
 	int error;
 
+	if ((data->suspend_mode == MXT_SUSPEND_POWEROFF) && !data->in_bootloader) {
+		error = mxt_power_on(data);
+		if (error)
+			return error;
+	}
+
 	error = mxt_load_fw(dev, MXT_FW_NAME);
 	if (error) {
 		dev_err(dev, "The firmware update failed(%d)\n", error);
@@ -3104,7 +3148,10 @@ static const struct attribute_group mxt_attr_group = {
 
 static void mxt_start(struct mxt_data *data)
 {
-	mxt_wakeup_toggle(data->client, true, false);
+	if (data->suspend_mode == MXT_SUSPEND_POWEROFF)
+		mxt_initialize_after_resume(data);
+	else
+		mxt_wakeup_toggle(data->client, true, false);
 
 	switch (data->suspend_mode) {
 	case MXT_SUSPEND_T9_CTRL:
@@ -3116,6 +3163,7 @@ static void mxt_start(struct mxt_data *data)
 				MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83);
 		break;
 
+	case MXT_SUSPEND_POWEROFF:
 	case MXT_SUSPEND_DEEP_SLEEP:
 	default:
 		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
@@ -3141,7 +3189,12 @@ static void mxt_stop(struct mxt_data *data)
 		break;
 	}
 
-	mxt_wakeup_toggle(data->client, false, false);
+	if (data->suspend_mode == MXT_SUSPEND_POWEROFF) {
+		disable_irq(data->irq);
+		mxt_power_off(data);
+	} else {
+		mxt_wakeup_toggle(data->client, false, false);
+	}
 }
 
 static int mxt_input_open(struct input_dev *dev)
@@ -3338,6 +3391,9 @@ static int mxt_probe(struct i2c_client *client)
 	if (error)
 		return error;
 
+	if (device_property_read_bool(&client->dev, "atmel,poweroff-sleep"))
+		data->suspend_mode = MXT_SUSPEND_POWEROFF;
+
 	/*
 	 * Controllers like mXT1386 have a dedicated WAKE line that could be
 	 * connected to a GPIO or to I2C SCL pin, or permanently asserted low.
@@ -3387,7 +3443,8 @@ static void mxt_remove(struct i2c_client *client)
 	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
 	mxt_free_input_device(data);
 	mxt_free_object_table(data);
-	mxt_power_off(data);
+	if (!(data->suspend_mode == MXT_SUSPEND_POWEROFF))
+		mxt_power_off(data);
 }
 
 static int mxt_suspend(struct device *dev)
@@ -3420,7 +3477,8 @@ static int mxt_resume(struct device *dev)
 	if (!input_dev)
 		return 0;
 
-	enable_irq(data->irq);
+	if (!(data->suspend_mode == MXT_SUSPEND_POWEROFF))
+		enable_irq(data->irq);
 
 	mutex_lock(&input_dev->mutex);
 
-- 
2.43.0

Re: [PATCH v6 3/3] Input: atmel_mxt_ts - add support for poweroff-sleep

[Dmitry Torokhov]

Hi Stefan,

On Mon, Jul 15, 2024 at 05:31:23PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> 
> Add support for poweroff-sleep to the Atmel maXTouch driver. This allows
> us to power off the input device entirely and only power it on when it
> is opened. This will also automatically power it off when we suspend the
> system.

I have been looking at the patch closely and I have a few comments.

> 
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@toradex.com>
> ---
>  drivers/input/touchscreen/atmel_mxt_ts.c | 82 ++++++++++++++++++++----
>  1 file changed, 70 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 9416de53bf9af..46ed3dbf0c5ed 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -265,6 +265,7 @@ enum v4l_dbg_inputs {
>  enum mxt_suspend_mode {
>  	MXT_SUSPEND_DEEP_SLEEP	= 0,
>  	MXT_SUSPEND_T9_CTRL	= 1,
> +	MXT_SUSPEND_POWEROFF	= 2,
>  };
>  
>  /* Config update context */
> @@ -1311,6 +1312,10 @@ static int mxt_power_on(struct mxt_data *data)
>  {
>  	int error;
>  
> +	/* Make sure the device is in reset before enabling power */
> +	if (data->reset_gpio)
> +		gpiod_set_value_cansleep(data->reset_gpio, 1);
> +
>  	error = regulator_bulk_enable(ARRAY_SIZE(data->regulators),
>  				      data->regulators);
>  	if (error) {
> @@ -2270,8 +2275,38 @@ static int mxt_configure_objects(struct mxt_data *data,
>  
>  static void mxt_config_cb(const struct firmware *cfg, void *ctx)
>  {
> +	struct mxt_data *data = ctx;
> +
>  	mxt_configure_objects(ctx, cfg);
>  	release_firmware(cfg);
> +
> +	if ((data->suspend_mode == MXT_SUSPEND_POWEROFF) && !data->in_bootloader) {
> +		disable_irq(data->irq);
> +		mxt_power_off(data);
> +	}

I do not think you can do it like that here. When you register an input
device it goes through the list of registered handlers and attaches
matching ones. Some of them may be in-kernel (for example on Chrome OS
ARM we have a handler that momentarily boosts CPU frequency on user
activity) and may open the input device immediately. So when you get to
this spot the device might be powered up and being used.

You should probably check result of input_device_enabled() when deciding
whether to power it off.

Also I think this would be valid for other suspend modes. Why don't we
power off unused device?

> +}
> +
> +static void mxt_initialize_after_resume(struct mxt_data *data)
> +{
> +	int error;
> +
> +	error = mxt_power_on(data);
> +	if (error) {
> +		dev_err(&data->client->dev, "Failed to power on device\n");
> +		return;
> +	}
> +
> +	error = mxt_acquire_irq(data);
> +	if (error) {
> +		dev_err(&data->client->dev, "Failed to acquire IRQ\n");
> +		return;
> +	}
> +
> +	error = mxt_configure_objects(data, NULL);

I do not think you need to call mxt_configure_objects() here. You are
not going to apply the config (you do not have it) and you are not going
to create the input device (it should already be created or we do not
have right fw/config for it to be created). You also do not need to call
mxt_init_t7_power_cfg() because it is supposed to be run already. We
just need to call mxt_set_t7_power_cfg() to set the right T7 power
config, which happens later in mxt_start() anyways.

I think you just need to power on and re-enable interrupts here, and I
would do it directly in mxt_start.


> +	if (error) {
> +		dev_err(&data->client->dev, "Failed to configure objects\n");
> +		return;
> +	}
>  }
>  
>  static int mxt_initialize(struct mxt_data *data)
> @@ -2828,15 +2863,18 @@ static int mxt_configure_objects(struct mxt_data *data,
>  			dev_warn(dev, "Error %d updating config\n", error);
>  	}
>  
> -	if (data->multitouch) {
> -		error = mxt_initialize_input_device(data);
> -		if (error)
> -			return error;
> -	} else {
> -		dev_warn(dev, "No touch object detected\n");
> -	}
> +	/* Do not initialize and register input device twice */
> +	if (!data->input_dev) {
> +		if (data->multitouch) {
> +			error = mxt_initialize_input_device(data);
> +			if (error)
> +				return error;
> +		} else {
> +			dev_warn(dev, "No touch object detected\n");
> +		}
>  
> -	mxt_debug_init(data);
> +		mxt_debug_init(data);
> +	}
>  
>  	return 0;
>  }
> @@ -3070,6 +3108,12 @@ static ssize_t mxt_update_fw_store(struct device *dev,
>  	struct mxt_data *data = dev_get_drvdata(dev);
>  	int error;
>  
> +	if ((data->suspend_mode == MXT_SUSPEND_POWEROFF) && !data->in_bootloader) {
> +		error = mxt_power_on(data);

Can we not make it dependent on data->suspend_mode? Maybe keep track of
the power state the device is in and just call mxt_power_on() if we
believe that the device is off? Or even have this check (counter?) in
mxt_power_on()/mxt_power_off()?

> +		if (error)
> +			return error;
> +	}
> +
>  	error = mxt_load_fw(dev, MXT_FW_NAME);
>  	if (error) {
>  		dev_err(dev, "The firmware update failed(%d)\n", error);
> @@ -3104,7 +3148,10 @@ static const struct attribute_group mxt_attr_group = {
>  
>  static void mxt_start(struct mxt_data *data)
>  {
> -	mxt_wakeup_toggle(data->client, true, false);
> +	if (data->suspend_mode == MXT_SUSPEND_POWEROFF)
> +		mxt_initialize_after_resume(data);
> +	else
> +		mxt_wakeup_toggle(data->client, true, false);
>  
>  	switch (data->suspend_mode) {
>  	case MXT_SUSPEND_T9_CTRL:
> @@ -3116,6 +3163,7 @@ static void mxt_start(struct mxt_data *data)
>  				MXT_TOUCH_MULTI_T9, MXT_T9_CTRL, 0x83);
>  		break;
>  
> +	case MXT_SUSPEND_POWEROFF:

I would do like this:

		error = mxt_power_on(...);
		if (error) {
			dev_err(...);
			return;
		}

  		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);

		/*
		 * I am not sure if explicit calibration is needed
		 * after full power up.
		 */
		mxt_t6_command(data, MXT_COMMAND_CALIBRATE, 1, false);
		mxt_acquire_irq(...);
		break;

>  	case MXT_SUSPEND_DEEP_SLEEP:
>  	default:
>  		mxt_set_t7_power_cfg(data, MXT_POWER_CFG_RUN);
> @@ -3141,7 +3189,12 @@ static void mxt_stop(struct mxt_data *data)
>  		break;
>  	}
>  
> -	mxt_wakeup_toggle(data->client, false, false);
> +	if (data->suspend_mode == MXT_SUSPEND_POWEROFF) {
> +		disable_irq(data->irq);
> +		mxt_power_off(data);

Work it into switch() as well. If you need to move mxt_wakeup_toggle()
into individual cases that is fine.

> +	} else {
> +		mxt_wakeup_toggle(data->client, false, false);
> +	}
>  }
>  
>  static int mxt_input_open(struct input_dev *dev)
> @@ -3338,6 +3391,9 @@ static int mxt_probe(struct i2c_client *client)
>  	if (error)
>  		return error;
>  
> +	if (device_property_read_bool(&client->dev, "atmel,poweroff-sleep"))
> +		data->suspend_mode = MXT_SUSPEND_POWEROFF;
> +
>  	/*
>  	 * Controllers like mXT1386 have a dedicated WAKE line that could be
>  	 * connected to a GPIO or to I2C SCL pin, or permanently asserted low.
> @@ -3387,7 +3443,8 @@ static void mxt_remove(struct i2c_client *client)
>  	sysfs_remove_group(&client->dev.kobj, &mxt_attr_group);
>  	mxt_free_input_device(data);
>  	mxt_free_object_table(data);
> -	mxt_power_off(data);
> +	if (!(data->suspend_mode == MXT_SUSPEND_POWEROFF))
> +		mxt_power_off(data);

Please make decision based on the state, not suspend mode.

>  }
>  
>  static int mxt_suspend(struct device *dev)
> @@ -3420,7 +3477,8 @@ static int mxt_resume(struct device *dev)
>  	if (!input_dev)
>  		return 0;
>  
> -	enable_irq(data->irq);
> +	if (!(data->suspend_mode == MXT_SUSPEND_POWEROFF))
> +		enable_irq(data->irq);

It would be good to have consistent IRQ management regardless of the
suspend mode.

>  
>  	mutex_lock(&input_dev->mutex);
>  
> -- 
> 2.43.0
> 

Thanks.

-- 
Dmitry