[PATCH v2 0/2] Input: drv260x: restore configuration across power loss

[PATCH v2 0/2] Input: drv260x: restore configuration across power loss

[Maurizio Casciano]

The DRV260x enable GPIO disables the I2C interface and resets volatile
configuration when lowered. Closing the input device or suspending the
system can therefore leave the controller unconfigured when it is used
again.

Restore the actuator configuration when the input device is reopened and
after system resume. Quiesce asynchronous force-feedback work during
suspend, restore a usable state if regulator shutdown fails, and keep the
firmware-selected actuator mode unchanged during playback.

Changes since v1:

- add a separate first patch restoring configuration after input close;
- stop overwriting the configured actuator mode during playback; and
- retain the suspend/resume sequencing fix while restoring all volatile
  configuration after the enable GPIO is asserted.

The previously posted optional-vbat change is withdrawn, as requested by
Dmitry. The Yoga Book enable-GPIO mapping has also been removed from the
generic Input driver and is provided by board software nodes in the
separately posted platform/x86 v2 series.

Both patches pass strict checkpatch, W=1 and sparse v0.6.5-rc1. They were
tested on a Lenovo Yoga Book YB1-X91L with both DRV2604 devices. An
RTC-timed suspend-to-RAM and resume completed without rebooting, and both
force-feedback devices played effects afterwards without drv260x, I2C or
configuration-restore errors.

With Best Regards,
Maurizio Casciano

Maurizio Casciano (2):
  Input: drv260x: Restore configuration after device close
  Input: drv260x: Fix suspend and resume sequencing

 drivers/input/misc/drv260x.c | 58 +++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 10 deletions(-)


base-commit: 9a29ee801f525bcad71fea021bfe2a030885c8df
-- 
2.53.0

[PATCH v2 1/2] Input: drv260x: Restore configuration after device close

[Maurizio Casciano]

The enable GPIO resets volatile device registers when it is lowered. The
input close callback lowers that GPIO, but the next open currently starts
playback without restoring the actuator configuration.

Raise the enable GPIO, observe the startup delay and reinitialize the
controller when the input device is opened. Keep the configured actuator
mode unchanged during playback so later initialization follows the mode
selected by firmware.

Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-input/20260827182855.DE8E91F000E9@smtp.kernel.org/
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
 drivers/input/misc/drv260x.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..f26a9052edd2 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -243,8 +243,6 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 {
 	struct drv260x_data *haptics = input_get_drvdata(input);
 
-	haptics->mode = DRV260X_LRA_NO_CAL_MODE;
-
 	/* Scale u16 magnitude into u8 register value */
 	if (effect->u.rumble.strong_magnitude > 0)
 		haptics->magnitude = effect->u.rumble.strong_magnitude >> 8;
@@ -426,6 +424,21 @@ static int drv260x_init(struct drv260x_data *haptics)
 	return 0;
 }
 
+static int drv260x_open(struct input_dev *input)
+{
+	struct drv260x_data *haptics = input_get_drvdata(input);
+	int error;
+
+	gpiod_set_value(haptics->enable_gpio, 1);
+	usleep_range(250, 500);
+
+	error = drv260x_init(haptics);
+	if (error)
+		gpiod_set_value(haptics->enable_gpio, 0);
+
+	return error;
+}
+
 static const struct regmap_config drv260x_regmap_config = {
 	.reg_bits = 8,
 	.val_bits = 8,
@@ -528,6 +541,7 @@ static int drv260x_probe(struct i2c_client *client)
 	}
 
 	haptics->input_dev->name = "drv260x:haptics";
+	haptics->input_dev->open = drv260x_open;
 	haptics->input_dev->close = drv260x_close;
 	input_set_drvdata(haptics->input_dev, haptics);
 	input_set_capability(haptics->input_dev, EV_FF, FF_RUMBLE);
-- 
2.53.0

Re: [PATCH v2 1/2] Input: drv260x: Restore configuration after device close

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Repeatedly opening the input device causes unbounded kernel memory growth via regmap_register_patch().
--

commit 7b2939fba808d6cd89b89de289b90816dad3fa8b
Author: Maurizio Casciano <mauriziocasciano7@gmail.com>

Input: drv260x: Restore configuration after device close

This commit restores the actuator configuration by reinitializing the
controller when the input device is opened.

Link: https://lore.kernel.org/linux-input/20260827182855.DE8E91F000E9@smtp.kernel.org/

> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> index 6c5c4c53753b1..f26a9052edd2d 100644
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c
[ ... ]
> @@ -426,6 +424,21 @@ static int drv260x_init(struct drv260x_data *haptics)
>  	return 0;
>  }
>  
> +static int drv260x_open(struct input_dev *input)
> +{
> +	struct drv260x_data *haptics = input_get_drvdata(input);
> +	int error;
> +
> +	gpiod_set_value(haptics->enable_gpio, 1);
> +	usleep_range(250, 500);
> +
> +	error = drv260x_init(haptics);

[Severity: High]
Does this code leak kernel memory?

By calling drv260x_init() on every open, it will repeatedly call
regmap_register_patch() inside drv260x_init().

Since regmap_register_patch() dynamically allocates and appends to the
internal map->patch list on every call, could this cause an unbounded memory
leak if a userspace process repeatedly opens and closes the device?

> +	if (error)
> +		gpiod_set_value(haptics->enable_gpio, 0);
> +
> +	return error;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.input-v2.1787872237.git.mauriziocasciano7@gmail.com?part=1

Re: [PATCH v2 1/2] Input: drv260x: Restore configuration after device close

[Dmitry Torokhov]

Hi Maurizio,

On Fri, Aug 28, 2026 at 01:16:36AM +0200, Maurizio Casciano wrote:
> The enable GPIO resets volatile device registers when it is lowered. The

I think this is Sahiko's hallucination and not a fact. I looked at the
data sheet and it says it is "enable" and not "reset" pin. Also, if it
was reset pin then what the driver is doing in suspend and resume makes
no sense.

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/2] Input: drv260x: Restore configuration after device close

[Maurizio Casciano]

Hi Dmitry,

Thank you for pointing this out in your review [8].  I reviewed the
DRV2604 Rev. C
datasheet and the regmap implementation again, and I am withdrawing this
patch.

The configuration-loss premise raised by the initial automated review
[6] was incorrect.  Section 7.4.1.3, "Operation
With EN Control", of the DRV2604 datasheet [1] explicitly states:

  "When the EN pin is logic low, the device enters the shutdown state,
  which is the lowest power state of the device.  The device registers
  are not reset."

The same section says that a complete reset to the power-up state
requires DEV_RESET in register 0x01.  Section 7.4.1.5 further describes
DEV_RESET as equivalent to power-cycling the device [1].  Therefore,
drv260x_close() deasserting the enable GPIO does not discard the
configuration established at probe, and there is no need for an open
callback to run drv260x_init() again.  The mainline close path can be
seen at [3].

This datasheet applies to the tested tablet: its firmware exposes two
present ACPI devices with HID and modalias DRV2604, at
\_SB_.PCI0.I2C1.VBR0 and \_SB_.PCI0.I2C4.VBR1.  They enumerate as
i2c-DRV2604:00 and i2c-DRV2604:01, and both are bound to the
drv260x-haptics driver.  The TI Linux support page also lists DRV2604
among the devices supported by this mainline driver [2].

Independently, Sashiko's follow-up warning [7] about the proposed
implementation is valid.  drv260x_init() calls regmap_register_patch()
for the selected actuator mode, as shown in the v7.2 source at [4].
As [5] shows, regmap_register_patch() grows map->patch with krealloc(),
copies the supplied sequence after the existing entries, and increments
map->patch_regs.  Calling
drv260x_init() on every input-device open would consequently append the
same register sequence repeatedly and retain that growing allocation
until the regmap is destroyed.  Repeated open/close cycles would thus
cause unbounded kernel-memory growth.  The relevant implementation is
quoted here and linked in full at [5]:

  p = krealloc(map->patch,
               sizeof(struct reg_sequence) *
                       (map->patch_regs + num_regs), GFP_KERNEL);
  ...
  memcpy(p + map->patch_regs, regs, num_regs * sizeof(*regs));
  map->patch_regs += num_regs;

The related concern about haptics->mode being changed by playback only
affected the proposed attempts to call drv260x_init() again during open
or resume.  Once those unnecessary reinitialization calls are removed,
the suspend/resume fix does not depend on re-running mode-specific
initialization.

I have therefore dropped v2 patch 1/2 completely, with no replacement.
The forthcoming v3 contains only the suspend/resume sequencing fix from
v2 patch 2/2.  It disables and drains the work item before suspend,
balances the work state on error paths, restores power and communication
before leaving standby, and does not call drv260x_init().

Technical references:

[1] Texas Instruments, DRV2604 Haptic Driver datasheet, Rev. C,
    sections 7.4.1.3 and 7.4.1.5:
    https://www.ti.com/lit/ds/symlink/drv2604.pdf
[2] Texas Instruments, Linux Driver for DRV260x; supported devices and
    mainline source information:
    https://www.ti.com/tool/DRV260XSW-LINUX
[3] Linux v7.2 drv260x_close(), including standby and EN deassertion:
    https://github.com/torvalds/linux/blob/v7.2/drivers/input/misc/drv260x.c#L260-L274
[4] Linux v7.2 drv260x_init(), including regmap_register_patch() calls:
    https://github.com/torvalds/linux/blob/v7.2/drivers/input/misc/drv260x.c#L315-L379
[5] Linux v7.2 regmap_register_patch() implementation:
    https://github.com/torvalds/linux/blob/v7.2/drivers/base/regmap/regmap.c#L3417-L3473
[6] Sashiko's initial review, including the configuration-loss premise:
    https://lore.kernel.org/linux-input/20260827182855.DE8E91F000E9@smtp.kernel.org/
[7] Sashiko's v2 follow-up identifying repeated patch registration:
    https://lore.kernel.org/linux-input/20260827233020.416711F000E9@smtp.kernel.org/
[8] Dmitry's review rejecting the EN-reset premise:
    https://lore.kernel.org/linux-input/apLAjoZPxp9JJTrl@google.com/

Thank you, and thanks to Sashiko for identifying the memory-growth issue
in the proposed implementation.

With Best Regards,
Maurizio Casciano

[PATCH v2 2/2] Input: drv260x: Fix suspend and resume sequencing

[Maurizio Casciano]

Force-feedback playback is queued asynchronously, but system suspend
can cut the enable GPIO and vbat supply while the worker is pending.
Quiesce the worker and reject new playback requests after suspend
begins.

The enable pin also disables I2C and resets volatile configuration.
Raise it before communicating on resume, observe the startup delay,
and rerun device initialization. Restore the same state if regulator
shutdown aborts suspend.

Assisted-by: Codex:gpt-5.6-sol sparse
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
 drivers/input/misc/drv260x.c | 42 ++++++++++++++++++++++++++++--------
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index f26a9052edd2..c64a86bc9bfd 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -181,6 +181,7 @@
  * @work: Work item used to off load the enable/disable of the vibration
  * @enable_gpio: Pointer to the gpio used for enable/disabling
  * @regulator: Pointer to the regulator for the IC
+ * @suspended: Whether force-feedback work must remain quiesced
  * @magnitude: Magnitude of the vibration event
  * @mode: The operating mode of the IC (LRA_NO_CAL, ERM or LRA)
  * @library: The vibration library to be used
@@ -194,6 +195,7 @@ struct drv260x_data {
 	struct work_struct work;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
+	bool suspended;
 	u8 magnitude;
 	u32 mode;
 	u32 library;
@@ -215,6 +217,13 @@ static int drv260x_calculate_voltage(unsigned int voltage)
 	return (voltage * 255 / 5600);
 }
 
+static void drv260x_set_suspended(struct drv260x_data *haptics,
+				  bool suspended)
+{
+	scoped_guard(spinlock_irqsave, &haptics->input_dev->event_lock)
+		haptics->suspended = suspended;
+}
+
 static void drv260x_worker(struct work_struct *work)
 {
 	struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
@@ -243,6 +252,9 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 {
 	struct drv260x_data *haptics = input_get_drvdata(input);
 
+	if (haptics->suspended)
+		return 0;
+
 	/* Scale u16 magnitude into u8 register value */
 	if (effect->u.rumble.strong_magnitude > 0)
 		haptics->magnitude = effect->u.rumble.strong_magnitude >> 8;
@@ -583,17 +595,21 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	int error, restore_error;
 
 	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		drv260x_set_suspended(haptics, true);
+		cancel_work_sync(&haptics->work);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK,
 					   DRV260X_STANDBY);
 		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
+			drv260x_set_suspended(haptics, false);
 			return error;
 		}
 
@@ -602,9 +618,15 @@ static int drv260x_suspend(struct device *dev)
 		error = regulator_disable(haptics->regulator);
 		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
-			regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
+
+			gpiod_set_value(haptics->enable_gpio, 1);
+			usleep_range(250, 500);
+			restore_error = drv260x_init(haptics);
+			if (restore_error)
+				dev_err(dev, "Failed to restore configuration: %d\n",
+					restore_error);
+
+			drv260x_set_suspended(haptics, false);
 			return error;
 		}
 	}
@@ -626,16 +648,18 @@ static int drv260x_resume(struct device *dev)
 			return error;
 		}
 
-		error = regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
+		gpiod_set_value(haptics->enable_gpio, 1);
+		usleep_range(250, 500);
+
+		error = drv260x_init(haptics);
 		if (error) {
-			dev_err(dev, "Failed to unset standby mode\n");
+			dev_err(dev, "Failed to restore configuration: %d\n", error);
+			gpiod_set_value(haptics->enable_gpio, 0);
 			regulator_disable(haptics->regulator);
 			return error;
 		}
 
-		gpiod_set_value(haptics->enable_gpio, 1);
+		drv260x_set_suspended(haptics, false);
 	}
 
 	return 0;
-- 
2.53.0

Re: [PATCH v2 2/2] Input: drv260x: Fix suspend and resume sequencing

[Dmitry Torokhov]

Hi Maurizio,

On Fri, Aug 28, 2026 at 01:16:37AM +0200, Maurizio Casciano wrote:
> Force-feedback playback is queued asynchronously, but system suspend
> can cut the enable GPIO and vbat supply while the worker is pending.
> Quiesce the worker and reject new playback requests after suspend
> begins.

Would it be easier to use disable_work_sync()/enable_work()?

> 
> The enable pin also disables I2C and resets volatile configuration.

Again, this is not true.

> Raise it before communicating on resume, observe the startup delay,
> and rerun device initialization. Restore the same state if regulator
> shutdown aborts suspend.
> 
> Assisted-by: Codex:gpt-5.6-sol sparse
> Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
> ---
>  drivers/input/misc/drv260x.c | 42 ++++++++++++++++++++++++++++--------
>  1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> index f26a9052edd2..c64a86bc9bfd 100644
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c
> @@ -181,6 +181,7 @@
>   * @work: Work item used to off load the enable/disable of the vibration
>   * @enable_gpio: Pointer to the gpio used for enable/disabling
>   * @regulator: Pointer to the regulator for the IC
> + * @suspended: Whether force-feedback work must remain quiesced
>   * @magnitude: Magnitude of the vibration event
>   * @mode: The operating mode of the IC (LRA_NO_CAL, ERM or LRA)
>   * @library: The vibration library to be used
> @@ -194,6 +195,7 @@ struct drv260x_data {
>  	struct work_struct work;
>  	struct gpio_desc *enable_gpio;
>  	struct regulator *regulator;
> +	bool suspended;
>  	u8 magnitude;
>  	u32 mode;
>  	u32 library;
> @@ -215,6 +217,13 @@ static int drv260x_calculate_voltage(unsigned int voltage)
>  	return (voltage * 255 / 5600);
>  }
>  
> +static void drv260x_set_suspended(struct drv260x_data *haptics,
> +				  bool suspended)
> +{
> +	scoped_guard(spinlock_irqsave, &haptics->input_dev->event_lock)
> +		haptics->suspended = suspended;
> +}
> +
>  static void drv260x_worker(struct work_struct *work)
>  {
>  	struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
> @@ -243,6 +252,9 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
>  {
>  	struct drv260x_data *haptics = input_get_drvdata(input);
>  
> +	if (haptics->suspended)
> +		return 0;
> +
>  	/* Scale u16 magnitude into u8 register value */
>  	if (effect->u.rumble.strong_magnitude > 0)
>  		haptics->magnitude = effect->u.rumble.strong_magnitude >> 8;
> @@ -583,17 +595,21 @@ static int drv260x_probe(struct i2c_client *client)
>  static int drv260x_suspend(struct device *dev)
>  {
>  	struct drv260x_data *haptics = dev_get_drvdata(dev);
> -	int error;
> +	int error, restore_error;
>  
>  	guard(mutex)(&haptics->input_dev->mutex);
>  
>  	if (input_device_enabled(haptics->input_dev)) {
> +		drv260x_set_suspended(haptics, true);
> +		cancel_work_sync(&haptics->work);
> +
>  		error = regmap_update_bits(haptics->regmap,
>  					   DRV260X_MODE,
>  					   DRV260X_STANDBY_MASK,
>  					   DRV260X_STANDBY);
>  		if (error) {
>  			dev_err(dev, "Failed to set standby mode\n");
> +			drv260x_set_suspended(haptics, false);
>  			return error;
>  		}
>  
> @@ -602,9 +618,15 @@ static int drv260x_suspend(struct device *dev)
>  		error = regulator_disable(haptics->regulator);
>  		if (error) {
>  			dev_err(dev, "Failed to disable regulator\n");
> -			regmap_update_bits(haptics->regmap,
> -					   DRV260X_MODE,
> -					   DRV260X_STANDBY_MASK, 0);
> +
> +			gpiod_set_value(haptics->enable_gpio, 1);
> +			usleep_range(250, 500);
> +			restore_error = drv260x_init(haptics);
> +			if (restore_error)
> +				dev_err(dev, "Failed to restore configuration: %d\n",
> +					restore_error);
> +
> +			drv260x_set_suspended(haptics, false);

If we need error unwinding please use goto style.

Thanks.

-- 
Dmitry

[PATCH v3] Input: drv260x: Fix suspend and resume sequencing

[Maurizio Casciano]

Force-feedback playback is queued asynchronously, but system suspend
can cut power while the worker is pending. Disable and drain the work
item before entering standby, and keep it disabled until resume has
restored communication.

The enable GPIO gates I2C access without resetting the device. Raise it
and observe the startup delay before leaving standby after resume or a
failed regulator shutdown. Use goto-based error unwinding to balance the
work state.

Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/
Assisted-by: Codex:gpt-5.6-sol [sparse]
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
 drivers/input/misc/drv260x.c | 38 ++++++++++++++++++++++++++----------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..d8208b3f7645 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -569,18 +569,20 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	int error, restore_error;
 
 	guard(mutex)(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		disable_work_sync(&haptics->work);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK,
 					   DRV260X_STANDBY);
 		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
-			return error;
+			goto err_enable_work;
 		}
 
 		gpiod_set_value(haptics->enable_gpio, 0);
@@ -588,14 +590,23 @@ static int drv260x_suspend(struct device *dev)
 		error = regulator_disable(haptics->regulator);
 		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
-			regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
-			return error;
+			goto err_leave_standby;
 		}
 	}
 
 	return 0;
+
+err_leave_standby:
+	gpiod_set_value(haptics->enable_gpio, 1);
+	fsleep(250);
+	restore_error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK, 0);
+	if (restore_error)
+		dev_err(dev, "Failed to leave standby mode: %d\n", restore_error);
+err_enable_work:
+	enable_work(&haptics->work);
+	return error;
 }
 
 static int drv260x_resume(struct device *dev)
@@ -612,19 +623,26 @@ static int drv260x_resume(struct device *dev)
 			return error;
 		}
 
+		gpiod_set_value(haptics->enable_gpio, 1);
+		fsleep(250);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK, 0);
 		if (error) {
-			dev_err(dev, "Failed to unset standby mode\n");
-			regulator_disable(haptics->regulator);
-			return error;
+			dev_err(dev, "Failed to leave standby mode: %d\n", error);
+			goto err_disable_regulator;
 		}
 
-		gpiod_set_value(haptics->enable_gpio, 1);
+		enable_work(&haptics->work);
 	}
 
 	return 0;
+
+err_disable_regulator:
+	gpiod_set_value(haptics->enable_gpio, 0);
+	regulator_disable(haptics->regulator);
+	return error;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
-- 
2.53.0

Re: [PATCH v3] Input: drv260x: Fix suspend and resume sequencing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The workqueue disable count becomes unbalanced if drv260x_resume() fails, permanently disabling haptic playback on subsequent successful resumes.
- [Medium] Mixing goto-based error handling with scope-based guard() cleanup in the same function.

Pre-existing issues:
- [High] The device loses its register configuration during system suspend because the regulator is disabled, but resume fails to reinitialize the registers.
--

commit 38e8103cb03d4a564c39fb41cb33305800090398
Author: Maurizio Casciano <mauriziocasciano7@gmail.com>

Input: drv260x: Fix suspend and resume sequencing

This commit updates the suspend and resume routines to correctly drain and
disable the haptics worker before entering standby. It also adds goto-based
error unwinding and uses the enable GPIO to gate I2C access while enforcing
a startup delay.

Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/

> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c
> @@ -569,18 +569,20 @@ static int drv260x_probe(struct i2c_client *client)
>  static int drv260x_suspend(struct device *dev)
>  {
>  	struct drv260x_data *haptics = dev_get_drvdata(dev);
> -	int error;
> +	int error, restore_error;
>  
>  	guard(mutex)(&haptics->input_dev->mutex);

[ ... ]

>  		if (error) {
>  			dev_err(dev, "Failed to set standby mode\n");
> -			return error;
> +			goto err_enable_work;
>  		}

[Severity: Medium]
Does this mix goto-based error handling with scope-based guard() cleanup in
the same function?

The kernel cleanup subsystem guidelines explicitly forbid mixing goto and
guard() because it creates confusing ownership semantics. While the lock is
correctly released at the function return in this specific code structure,
the pattern itself is prohibited by subsystem rules.

[ ... ]

>  		error = regulator_disable(haptics->regulator);

[Severity: High]
This is a pre-existing issue, but I noticed the regulator is disabled during
suspend, which completely cuts power to the chip and resets its internal
register state.

When the regulator is re-enabled during drv260x_resume(), the driver only
clears the standby bit and fails to reprogram the operational registers like
rated voltage and overdrive. Because the driver uses REGCACHE_NONE and does
not automatically restore register state, does this leave the hardware
unconfigured and functionally broken after a suspend and resume cycle?

[ ... ]

>  static int drv260x_resume(struct device *dev)
>  {
>  	struct drv260x_data *haptics = dev_get_drvdata(dev);
>  	int error;
>  
>  	guard(mutex)(&haptics->input_dev->mutex);
>  
>  	if (input_device_enabled(haptics->input_dev)) {
>  		error = regulator_enable(haptics->regulator);
>  		if (error) {
>  			dev_err(dev, "Failed to enable regulator\n");
>  			return error;
>  		}

[Severity: High]
If regulator_enable() fails here, we return directly without calling
enable_work(&haptics->work).

Since disable_work_sync() was called in drv260x_suspend(), doesn't this leave
the workqueue's disable count unbalanced? The PM core treats resume errors as
non-fatal to the system, so a subsequent suspend will increment the disable
count again to 2. A future successful resume will only decrement it to 1,
leaving the haptics playback permanently disabled.

[ ... ]

>  		error = regmap_update_bits(haptics->regmap,
>  					   DRV260X_MODE,
>  					   DRV260X_STANDBY_MASK, 0);
>  		if (error) {
> -			dev_err(dev, "Failed to unset standby mode\n");
> -			regulator_disable(haptics->regulator);
> -			return error;
> +			dev_err(dev, "Failed to leave standby mode: %d\n", error);
> +			goto err_disable_regulator;
>  		}

[Severity: High]
Similarly, if we take the err_disable_regulator goto path when leaving
standby fails, enable_work() is skipped.

Does this also permanently leak the workqueue disable count?

[Severity: Medium]
Also, just like in drv260x_suspend(), drv260x_resume() introduces a new
goto path while using guard(mutex) at the start of the function.

Could we refactor this to avoid mixing goto labels and scope-based cleanup?

> -		gpiod_set_value(haptics->enable_gpio, 1);
> +		enable_work(&haptics->work);
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260829225704.394707-1-mauriziocasciano7@gmail.com?part=1

[PATCH v4] Input: drv260x: Fix suspend and resume sequencing

[Maurizio Casciano]

Force-feedback playback is queued asynchronously, but system suspend
can cut power while the worker is pending. Disable and drain the work
item before entering standby, and keep it disabled until resume has
restored communication.

The enable GPIO gates I2C access without resetting the device. Raise it
and observe the startup delay before leaving standby after resume or a
failed regulator shutdown.

Use explicit mutex locking with goto-based error unwinding, and balance
the work disable count on every resume error path so a later successful
resume can restore playback.

Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/
Assisted-by: Codex:gpt-5.6-sol [sparse]
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
 drivers/input/misc/drv260x.c | 50 ++++++++++++++++++++++++++----------
 1 file changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..b6499b2fcd8d 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -569,18 +569,20 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	int error, restore_error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		disable_work_sync(&haptics->work);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK,
 					   DRV260X_STANDBY);
 		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
-			return error;
+			goto err_enable_work;
 		}
 
 		gpiod_set_value(haptics->enable_gpio, 0);
@@ -588,14 +590,25 @@ static int drv260x_suspend(struct device *dev)
 		error = regulator_disable(haptics->regulator);
 		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
-			regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
-			return error;
+			goto err_leave_standby;
 		}
 	}
 
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_leave_standby:
+	gpiod_set_value(haptics->enable_gpio, 1);
+	fsleep(250);
+	restore_error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK, 0);
+	if (restore_error)
+		dev_err(dev, "Failed to leave standby mode: %d\n", restore_error);
+err_enable_work:
+	enable_work(&haptics->work);
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static int drv260x_resume(struct device *dev)
@@ -603,28 +616,39 @@ static int drv260x_resume(struct device *dev)
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
 	int error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
 		error = regulator_enable(haptics->regulator);
 		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			return error;
+			goto err_enable_work;
 		}
 
+		gpiod_set_value(haptics->enable_gpio, 1);
+		fsleep(250);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK, 0);
 		if (error) {
-			dev_err(dev, "Failed to unset standby mode\n");
-			regulator_disable(haptics->regulator);
-			return error;
+			dev_err(dev, "Failed to leave standby mode: %d\n", error);
+			goto err_disable_regulator;
 		}
 
-		gpiod_set_value(haptics->enable_gpio, 1);
+		enable_work(&haptics->work);
 	}
 
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_disable_regulator:
+	gpiod_set_value(haptics->enable_gpio, 0);
+	regulator_disable(haptics->regulator);
+err_enable_work:
+	enable_work(&haptics->work);
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
-- 
2.53.0

Re: [PATCH v4] Input: drv260x: Fix suspend and resume sequencing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The resume error path incorrectly re-enables the haptics worker while the hardware regulator is disabled, exposing the unpowered device to I2C and GPIO writes.

Pre-existing issues:
- [High] The device's calibration and configuration registers are lost across a suspend/resume cycle because they are never restored after the regulator is re-enabled.
--

commit 6d265a379cd4795d6bb19ec724655bfda45835f4
Author: Maurizio Casciano <mauriziocasciano7@gmail.com>

Input: drv260x: Fix suspend and resume sequencing

This commit fixes system suspend handling for the drv260x haptics driver
by explicitly disabling and draining the force-feedback worker before
entering standby, and conditionally enabling it upon resume. It also
introduces explicit mutex locking and goto-based error unwinding.

Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/

> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> index 6c5c4c53753b1..b6499b2fcd8df 100644
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c
[ ... ]
>  static int drv260x_resume(struct device *dev)
>  {
>  	struct drv260x_data *haptics = dev_get_drvdata(dev);
>  	int error;
>  
> -	guard(mutex)(&haptics->input_dev->mutex);
> +	mutex_lock(&haptics->input_dev->mutex);
>  
>  	if (input_device_enabled(haptics->input_dev)) {
>  		error = regulator_enable(haptics->regulator);
>  		if (error) {
>  			dev_err(dev, "Failed to enable regulator\n");
> -			return error;
> +			goto err_enable_work;
>  		}

[Severity: High]
This is a pre-existing issue, but since the regulator was disabled during
suspend, the DRV260x chip loses all internal volatile registers, including
critical calibration data like rated voltage and overdrive clamp.

Because drv260x_regmap_config uses REGCACHE_NONE, regmap will not
automatically restore these values upon resume.

Does this device need a call to drv260x_init() here to restore its
calibration and configuration settings before attempting to operate?

>  
> +		gpiod_set_value(haptics->enable_gpio, 1);
> +		fsleep(250);
> +
>  		error = regmap_update_bits(haptics->regmap,
>  					   DRV260X_MODE,
>  					   DRV260X_STANDBY_MASK, 0);
>  		if (error) {
> -			dev_err(dev, "Failed to unset standby mode\n");
> -			regulator_disable(haptics->regulator);
> -			return error;
> +			dev_err(dev, "Failed to leave standby mode: %d\n", error);
> +			goto err_disable_regulator;
>  		}
>  
> -		gpiod_set_value(haptics->enable_gpio, 1);
> +		enable_work(&haptics->work);
>  	}
>  
> +	mutex_unlock(&haptics->input_dev->mutex);
>  	return 0;
> +
> +err_disable_regulator:
> +	gpiod_set_value(haptics->enable_gpio, 0);
> +	regulator_disable(haptics->regulator);
> +err_enable_work:
> +	enable_work(&haptics->work);

[Severity: High]
If the system resumes but the hardware initialization fails, does this
unconditionally re-enable the haptics worker while the regulator remains
disabled?

If userspace subsequently sends a force-feedback event, drv260x_worker()
will execute and attempt I2C communication with the unpowered chip.
Can this lead to I2C bus timeouts, lockups, or back-powering the chip
via GPIO/I2C lines?

Additionally, if the system later successfully retries and resumes the
device, the success path will call enable_work() again. Since the count
was already erroneously reset to 0 here, won't this underflow the disable
count and trigger a workqueue kernel warning?

> +	mutex_unlock(&haptics->input_dev->mutex);
> +	return error;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830141728.53320-1-mauriziocasciano7@gmail.com?part=1

[PATCH v5] Input: drv260x: Fix suspend and resume sequencing

[Maurizio Casciano]

Force-feedback playback is queued asynchronously, but system suspend can
cut power while the worker is pending. Disable and drain the work item
before entering standby, and keep force-feedback quiesced until resume has
restored communication.

The enable GPIO gates I2C access without resetting the device. Raise it and
observe the startup delay before leaving standby after resume or a failed
regulator shutdown.

Use explicit locking and goto-based error unwinding. Keep the work item
disabled after a resume failure. Track whether it is already disabled
under the input mutex, so another suspend or a direct resume retry does not
increment the disable count and a later successful resume balances it
exactly once. Skip close-time register access while recovery remains
pending.

Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260830143050.03E081F000E9@smtp.kernel.org/
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/
Assisted-by: Codex:gpt-5.6-sol [sparse]
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
Changes in v5:
- Keep the work item disabled after every resume failure, so force-feedback
  cannot reach hardware whose regulator is off.
- Track the disabled state to avoid incrementing the work disable count again
  on a later suspend or direct resume retry.
- Enable the work item only after resume has fully restored the regulator,
  enable GPIO, and standby state, balancing the disable count exactly once.
- Avoid close-time register access while PM recovery is pending.

 drivers/input/misc/drv260x.c | 80 ++++++++++++++++++++++++++++++------
 1 file changed, 67 insertions(+), 13 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..f261bb32fdc0 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -181,6 +181,7 @@
  * @work: Work item used to off load the enable/disable of the vibration
  * @enable_gpio: Pointer to the gpio used for enable/disabling
  * @regulator: Pointer to the regulator for the IC
+ * @work_disabled: Whether playback work is disabled pending PM recovery
  * @magnitude: Magnitude of the vibration event
  * @mode: The operating mode of the IC (LRA_NO_CAL, ERM or LRA)
  * @library: The vibration library to be used
@@ -194,6 +195,7 @@ struct drv260x_data {
 	struct work_struct work;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
+	bool work_disabled;
 	u8 magnitude;
 	u32 mode;
 	u32 library;
@@ -215,6 +217,24 @@ static int drv260x_calculate_voltage(unsigned int voltage)
 	return (voltage * 255 / 5600);
 }
 
+static void drv260x_disable_work(struct drv260x_data *haptics)
+{
+	if (haptics->work_disabled)
+		return;
+
+	disable_work_sync(&haptics->work);
+	haptics->work_disabled = true;
+}
+
+static void drv260x_enable_work(struct drv260x_data *haptics)
+{
+	if (!haptics->work_disabled)
+		return;
+
+	enable_work(&haptics->work);
+	haptics->work_disabled = false;
+}
+
 static void drv260x_worker(struct work_struct *work)
 {
 	struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
@@ -263,6 +283,10 @@ static void drv260x_close(struct input_dev *input)
 	struct drv260x_data *haptics = input_get_drvdata(input);
 	int error;
 
+	/* PM has not restored register access yet. */
+	if (haptics->work_disabled)
+		return;
+
 	cancel_work_sync(&haptics->work);
 
 	error = regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
@@ -569,18 +593,22 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	bool restore_work = false;
+	int error, restore_error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		restore_work = !haptics->work_disabled;
+		drv260x_disable_work(haptics);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK,
 					   DRV260X_STANDBY);
 		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
-			return error;
+			goto err_enable_work;
 		}
 
 		gpiod_set_value(haptics->enable_gpio, 0);
@@ -588,14 +616,28 @@ static int drv260x_suspend(struct device *dev)
 		error = regulator_disable(haptics->regulator);
 		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
-			regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
-			return error;
+			goto err_leave_standby;
 		}
 	}
 
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_leave_standby:
+	gpiod_set_value(haptics->enable_gpio, 1);
+	fsleep(250);
+	restore_error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK, 0);
+	if (restore_error) {
+		dev_err(dev, "Failed to leave standby mode: %d\n", restore_error);
+		restore_work = false;
+	}
+err_enable_work:
+	if (restore_work)
+		drv260x_enable_work(haptics);
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static int drv260x_resume(struct device *dev)
@@ -603,28 +645,40 @@ static int drv260x_resume(struct device *dev)
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
 	int error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		drv260x_disable_work(haptics);
+
 		error = regulator_enable(haptics->regulator);
 		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			return error;
+			goto err_unlock;
 		}
 
+		gpiod_set_value(haptics->enable_gpio, 1);
+		fsleep(250);
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK, 0);
 		if (error) {
-			dev_err(dev, "Failed to unset standby mode\n");
-			regulator_disable(haptics->regulator);
-			return error;
+			dev_err(dev, "Failed to leave standby mode: %d\n", error);
+			goto err_disable_regulator;
 		}
 
-		gpiod_set_value(haptics->enable_gpio, 1);
+		drv260x_enable_work(haptics);
 	}
 
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_disable_regulator:
+	gpiod_set_value(haptics->enable_gpio, 0);
+	regulator_disable(haptics->regulator);
+err_unlock:
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
-- 
2.53.0

Re: [PATCH v5] Input: drv260x: Fix suspend and resume sequencing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Device loses custom configuration and DT-provided calibration data after system suspend because power is cut and registers are not restored upon resume.
- [High] Unbalanced regulator disable when driver is unbound after a resume failure.
- [High] I2C register access during suspend and close fails because the enable GPIO is not asserted when no effects have been played, causing system suspend to abort.
--

commit bc29da04cf5ab833e43a374e3220aeca730bd6a9
Author: Maurizio Casciano <mauriziocasciano7@gmail.com>

    Input: drv260x: Fix suspend and resume sequencing
    
This commit addresses race conditions between the asynchronous force-feedback
playback worker and system suspend. It manually stops the worker and sequences
the regulator, GPIO, and standby mode transitions, attempting to gracefully
unwind on failure.

Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260830143050.03E081F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/

> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> index 6c5c4c53753b1..f261bb32fdc08 100644
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c

[ ... ]

> @@ -569,18 +593,22 @@ static int drv260x_probe(struct i2c_client *client)
>  static int drv260x_suspend(struct device *dev)
>  {
>  	struct drv260x_data *haptics = dev_get_drvdata(dev);
> -	int error;
> +	bool restore_work = false;
> +	int error, restore_error;
>  
> -	guard(mutex)(&haptics->input_dev->mutex);
> +	mutex_lock(&haptics->input_dev->mutex);
>  
>  	if (input_device_enabled(haptics->input_dev)) {
> +		restore_work = !haptics->work_disabled;
> +		drv260x_disable_work(haptics);
> +
>  		error = regmap_update_bits(haptics->regmap,
>  					   DRV260X_MODE,
>  					   DRV260X_STANDBY_MASK,
>  					   DRV260X_STANDBY);

[Severity: High]
This is a pre-existing issue, but will this I2C register access fail if the
enable GPIO is not asserted?

If a user opens and then closes the device, drv260x_close() sets the enable
GPIO to 0. Because there is no open() callback, reopening the device leaves
the GPIO at 0. If the system then suspends before any effects are played, the
chip is disabled.

Will the resulting I2C NACK cause regmap_update_bits() to return an error and
abort the entire system suspend?

>  		if (error) {
>  			dev_err(dev, "Failed to set standby mode\n");
> -			return error;
> +			goto err_enable_work;
>  		}

[ ... ]

> @@ -603,28 +645,40 @@ static int drv260x_resume(struct device *dev)
>  	struct drv260x_data *haptics = dev_get_drvdata(dev);
>  	int error;
>  
> -	guard(mutex)(&haptics->input_dev->mutex);
> +	mutex_lock(&haptics->input_dev->mutex);
>  
>  	if (input_device_enabled(haptics->input_dev)) {
> +		drv260x_disable_work(haptics);
> +
>  		error = regulator_enable(haptics->regulator);
>  		if (error) {
>  			dev_err(dev, "Failed to enable regulator\n");
> -			return error;
> +			goto err_unlock;
>  		}
>  
> +		gpiod_set_value(haptics->enable_gpio, 1);
> +		fsleep(250);
> +
>  		error = regmap_update_bits(haptics->regmap,
>  					   DRV260X_MODE,
>  					   DRV260X_STANDBY_MASK, 0);

[Severity: High]
This isn't a bug introduced by this patch, but does the device lose its custom
configuration here?

Because the regulator was disabled during suspend, power to the device was cut
and the registers were wiped. Since the driver uses REGCACHE_NONE for its
regmap configuration, clearing the standby mode won't automatically reapply the
device settings.

Are critical initialization settings like rated voltage, overdrive voltage, and
calibration data lost after a suspend/resume cycle?

>  		if (error) {
> -			dev_err(dev, "Failed to unset standby mode\n");
> -			regulator_disable(haptics->regulator);
> -			return error;
> +			dev_err(dev, "Failed to leave standby mode: %d\n", error);
> +			goto err_disable_regulator;
>  		}
>  
> -		gpiod_set_value(haptics->enable_gpio, 1);
> +		drv260x_enable_work(haptics);
>  	}
>  
> +	mutex_unlock(&haptics->input_dev->mutex);
>  	return 0;
> +
> +err_disable_regulator:
> +	gpiod_set_value(haptics->enable_gpio, 0);
> +	regulator_disable(haptics->regulator);
> +err_unlock:
> +	mutex_unlock(&haptics->input_dev->mutex);
> +	return error;
>  }

[Severity: High]
This is a pre-existing issue, but does this error unwinding break the invariant
expected by devm cleanup?

If regulator_enable() fails (goto err_unlock) or if regmap_update_bits() fails
(goto err_disable_regulator), drv260x_resume() exits with the regulator count
at 0.

If the device is subsequently unbound after this resume failure (for example,
the driver module is removed), the devm cleanup action unconditionally calls
regulator_disable() again.

Will this cause an unbalanced regulator disable, triggering a kernel WARN and
prematurely cutting power to other hardware sharing the same regulator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831010020.673316-1-mauriziocasciano7@gmail.com?part=1

[PATCH v6] Input: drv260x: Fix suspend and resume sequencing

[Maurizio Casciano]

Force-feedback playback is queued asynchronously, but system suspend can
cut power while the worker is pending. Disable and drain the work item
before entering standby, and keep force-feedback quiesced until resume has
restored communication.

An input device can be closed and reopened without playing an effect in
between. Since close lowers the enable GPIO, assert it and observe the
startup delay in the open callback so a following suspend can access the
registers.

Disabling the regulator may remove power and erase the device
configuration. Preserve the actuator mode selected by firmware, cache the
initial automatic-calibration results, and reapply the complete
configuration during resume. Use repeatable multi-register writes instead
of registering persistent regmap patches, which would append another copy
on every reinitialization.

Track whether this consumer has enabled the regulator so devres cleanup
does not issue an unbalanced disable after a resume failure. Keep the work
item disabled after failed recovery, and make regulator and work reference
counts idempotent across PM retries.

Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-input/20260831011526.24AB11F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260830143050.03E081F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/
Link: https://www.ti.com/lit/ds/symlink/drv2604.pdf
Assisted-by: Codex:gpt-5.6-sol [sparse]
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
Changes in v6:
- Assert EN from the input open callback, without reinitializing the device
  merely because EN was toggled.
- Reapply configuration after regulator power loss and cache the initial
  automatic-calibration results instead of rerunning calibration on resume.
- Replace regmap_register_patch() with repeatable multi-register writes so
  resume does not append persistent patches to the regmap.
- Keep the firmware-selected actuator mode immutable across playback.
- Track the regulator consumer state and make enable/disable operations
  idempotent across PM retries and devres cleanup.
- Retain the v5 work-disable accounting and explicit PM error unwinding.

Validation:
- Server build at integration commit f565dc5ad36e, containing the exact
  drv260x blob 71996f6d7d7745ebbc1a8c099d2e6c18657c822f:
  olddefconfig, focused W=1 C=2 CHECK=sparse build, and full Debian package
  build all passed.
- Lenovo Yoga Book YB1-X91L running
  7.2.0-yogabook-20260831-033805: both DRV2604 FF devices passed before and
  after an eight-second s2idle cycle. The test stopped the only userspace
  consumer to force the last close(), reopened both devices without playing
  an effect, and then suspended successfully. No drv260x, I2C, or unbalanced
  regulator error appeared in the test journal.
- This tablet uses dummy vbat regulators for both DRV2604 devices, so actual
  regulator power loss and injected resume-error cleanup could not be
  exercised on this hardware.

 drivers/input/misc/drv260x.c | 183 ++++++++++++++++++++++++++++-------
 1 file changed, 150 insertions(+), 33 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..71996f6d7d77 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -181,7 +181,11 @@
  * @work: Work item used to off load the enable/disable of the vibration
  * @enable_gpio: Pointer to the gpio used for enable/disabling
  * @regulator: Pointer to the regulator for the IC
+ * @regulator_enabled: Whether this consumer has enabled the regulator
+ * @work_disabled: Whether playback work is disabled pending PM recovery
+ * @calibration_valid: Whether calibration results have been cached
  * @magnitude: Magnitude of the vibration event
+ * @calibration_data: Cached automatic calibration results
  * @mode: The operating mode of the IC (LRA_NO_CAL, ERM or LRA)
  * @library: The vibration library to be used
  * @rated_voltage: The rated_voltage of the actuator
@@ -194,7 +198,11 @@ struct drv260x_data {
 	struct work_struct work;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
+	bool regulator_enabled;
+	bool work_disabled;
+	bool calibration_valid;
 	u8 magnitude;
+	u8 calibration_data[3];
 	u32 mode;
 	u32 library;
 	int rated_voltage;
@@ -215,6 +223,24 @@ static int drv260x_calculate_voltage(unsigned int voltage)
 	return (voltage * 255 / 5600);
 }
 
+static void drv260x_disable_work(struct drv260x_data *haptics)
+{
+	if (haptics->work_disabled)
+		return;
+
+	disable_work_sync(&haptics->work);
+	haptics->work_disabled = true;
+}
+
+static void drv260x_enable_work(struct drv260x_data *haptics)
+{
+	if (!haptics->work_disabled)
+		return;
+
+	enable_work(&haptics->work);
+	haptics->work_disabled = false;
+}
+
 static void drv260x_worker(struct work_struct *work)
 {
 	struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
@@ -243,8 +269,6 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 {
 	struct drv260x_data *haptics = input_get_drvdata(input);
 
-	haptics->mode = DRV260X_LRA_NO_CAL_MODE;
-
 	/* Scale u16 magnitude into u8 register value */
 	if (effect->u.rumble.strong_magnitude > 0)
 		haptics->magnitude = effect->u.rumble.strong_magnitude >> 8;
@@ -258,11 +282,29 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 	return 0;
 }
 
+static int drv260x_open(struct input_dev *input)
+{
+	struct drv260x_data *haptics = input_get_drvdata(input);
+
+	if (haptics->work_disabled)
+		return -EIO;
+
+	gpiod_set_value(haptics->enable_gpio, 1);
+	/* Data sheet says to wait 250us before trying to communicate */
+	fsleep(250);
+
+	return 0;
+}
+
 static void drv260x_close(struct input_dev *input)
 {
 	struct drv260x_data *haptics = input_get_drvdata(input);
 	int error;
 
+	/* PM has not restored register access yet. */
+	if (haptics->work_disabled)
+		return;
+
 	cancel_work_sync(&haptics->work);
 
 	error = regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
@@ -338,9 +380,9 @@ static int drv260x_init(struct drv260x_data *haptics)
 
 	switch (haptics->mode) {
 	case DRV260X_LRA_MODE:
-		error = regmap_register_patch(haptics->regmap,
-					      drv260x_lra_cal_regs,
-					      ARRAY_SIZE(drv260x_lra_cal_regs));
+		error = regmap_multi_reg_write(haptics->regmap,
+					       drv260x_lra_cal_regs,
+					       ARRAY_SIZE(drv260x_lra_cal_regs));
 		if (error) {
 			dev_err(&haptics->client->dev,
 				"Failed to write LRA calibration registers: %d\n",
@@ -351,9 +393,9 @@ static int drv260x_init(struct drv260x_data *haptics)
 		break;
 
 	case DRV260X_ERM_MODE:
-		error = regmap_register_patch(haptics->regmap,
-					      drv260x_erm_cal_regs,
-					      ARRAY_SIZE(drv260x_erm_cal_regs));
+		error = regmap_multi_reg_write(haptics->regmap,
+					       drv260x_erm_cal_regs,
+					       ARRAY_SIZE(drv260x_erm_cal_regs));
 		if (error) {
 			dev_err(&haptics->client->dev,
 				"Failed to write ERM calibration registers: %d\n",
@@ -374,9 +416,9 @@ static int drv260x_init(struct drv260x_data *haptics)
 		break;
 
 	default:
-		error = regmap_register_patch(haptics->regmap,
-					      drv260x_lra_init_regs,
-					      ARRAY_SIZE(drv260x_lra_init_regs));
+		error = regmap_multi_reg_write(haptics->regmap,
+					       drv260x_lra_init_regs,
+					       ARRAY_SIZE(drv260x_lra_init_regs));
 		if (error) {
 			dev_err(&haptics->client->dev,
 				"Failed to write LRA init registers: %d\n",
@@ -398,6 +440,11 @@ static int drv260x_init(struct drv260x_data *haptics)
 		return 0;
 	}
 
+	if (haptics->calibration_valid)
+		return regmap_bulk_write(haptics->regmap, DRV260X_CAL_COMP,
+					 haptics->calibration_data,
+					 ARRAY_SIZE(haptics->calibration_data));
+
 	error = regmap_write(haptics->regmap, DRV260X_GO, DRV260X_GO_BIT);
 	if (error) {
 		dev_err(&haptics->client->dev,
@@ -423,7 +470,13 @@ static int drv260x_init(struct drv260x_data *haptics)
 		}
 	} while (cal_buf == DRV260X_GO_BIT);
 
-	return 0;
+	error = regmap_bulk_read(haptics->regmap, DRV260X_CAL_COMP,
+				 haptics->calibration_data,
+				 ARRAY_SIZE(haptics->calibration_data));
+	if (!error)
+		haptics->calibration_valid = true;
+
+	return error;
 }
 
 static const struct regmap_config drv260x_regmap_config = {
@@ -434,11 +487,39 @@ static const struct regmap_config drv260x_regmap_config = {
 	.cache_type = REGCACHE_NONE,
 };
 
+static int drv260x_regulator_enable(struct drv260x_data *haptics)
+{
+	int error;
+
+	if (haptics->regulator_enabled)
+		return 0;
+
+	error = regulator_enable(haptics->regulator);
+	if (!error)
+		haptics->regulator_enabled = true;
+
+	return error;
+}
+
+static int drv260x_regulator_disable(struct drv260x_data *haptics)
+{
+	int error;
+
+	if (!haptics->regulator_enabled)
+		return 0;
+
+	error = regulator_disable(haptics->regulator);
+	if (!error)
+		haptics->regulator_enabled = false;
+
+	return error;
+}
+
 static void drv260x_power_off(void *data)
 {
 	struct drv260x_data *haptics = data;
 
-	regulator_disable(haptics->regulator);
+	drv260x_regulator_disable(haptics);
 }
 
 static int drv260x_probe(struct i2c_client *client)
@@ -506,7 +587,7 @@ static int drv260x_probe(struct i2c_client *client)
 		return error;
 	}
 
-	error = regulator_enable(haptics->regulator);
+	error = drv260x_regulator_enable(haptics);
 	if (error) {
 		dev_err(dev, "Failed to enable regulator: %d\n", error);
 		return error;
@@ -528,6 +609,7 @@ static int drv260x_probe(struct i2c_client *client)
 	}
 
 	haptics->input_dev->name = "drv260x:haptics";
+	haptics->input_dev->open = drv260x_open;
 	haptics->input_dev->close = drv260x_close;
 	input_set_drvdata(haptics->input_dev, haptics);
 	input_set_capability(haptics->input_dev, EV_FF, FF_RUMBLE);
@@ -569,62 +651,97 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	bool restore_work = false;
+	int error, restore_error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		restore_work = !haptics->work_disabled;
+		drv260x_disable_work(haptics);
+
+		/* A failed resume can leave the device already powered down. */
+		if (!haptics->regulator_enabled)
+			goto out_unlock;
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK,
 					   DRV260X_STANDBY);
 		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
-			return error;
+			goto err_enable_work;
 		}
 
 		gpiod_set_value(haptics->enable_gpio, 0);
 
-		error = regulator_disable(haptics->regulator);
+		error = drv260x_regulator_disable(haptics);
 		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
-			regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
-			return error;
+			goto err_leave_standby;
 		}
 	}
 
+out_unlock:
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_leave_standby:
+	gpiod_set_value(haptics->enable_gpio, 1);
+	fsleep(250);
+	restore_error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK, 0);
+	if (restore_error) {
+		dev_err(dev, "Failed to leave standby mode: %d\n", restore_error);
+		restore_work = false;
+	}
+err_enable_work:
+	if (restore_work)
+		drv260x_enable_work(haptics);
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static int drv260x_resume(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	int disable_error, error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		error = regulator_enable(haptics->regulator);
+		drv260x_disable_work(haptics);
+
+		error = drv260x_regulator_enable(haptics);
 		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			return error;
+			goto err_unlock;
 		}
 
-		error = regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
+		gpiod_set_value(haptics->enable_gpio, 1);
+		fsleep(250);
+
+		error = drv260x_init(haptics);
 		if (error) {
-			dev_err(dev, "Failed to unset standby mode\n");
-			regulator_disable(haptics->regulator);
-			return error;
+			dev_err(dev, "Failed to restore configuration: %d\n", error);
+			goto err_disable_regulator;
 		}
 
-		gpiod_set_value(haptics->enable_gpio, 1);
+		drv260x_enable_work(haptics);
 	}
 
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_disable_regulator:
+	gpiod_set_value(haptics->enable_gpio, 0);
+	disable_error = drv260x_regulator_disable(haptics);
+	if (disable_error)
+		dev_err(dev, "Failed to disable regulator: %d\n", disable_error);
+err_unlock:
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
-- 
2.53.0

Re: [PATCH v6] Input: drv260x: Fix suspend and resume sequencing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] The `work_disabled` flag becomes permanently wedged to `true` if the device is closed after a system resume failure, breaking force-feedback functionality on subsequent opens.
- [Low] Direct return of error code from regmap_bulk_write in a function with multiple failure points.

Pre-existing issues:
- [High] Device configuration is permanently lost if system power is cut during suspend while the input device is closed, leading to hardware misconfiguration upon the next open.
- [High] Uncancelled work item leads to Use-After-Free during device unbind due to asynchronous ff-memless timer.
--

commit 0cbbdf5958f6ed62525dabc219437a115afe42b3
Author: Maurizio Casciano <mauriziocasciano7@gmail.com>

Input: drv260x: Fix suspend and resume sequencing

Force-feedback playback is queued asynchronously, but system suspend can
cut power while the worker is pending. Disable and drain the work item
before entering standby, and keep force-feedback quiesced until resume has
restored communication.

Link: https://lore.kernel.org/linux-input/20260831011526.24AB11F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260830143050.03E081F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/
Link: https://www.ti.com/lit/ds/symlink/drv2604.pdf

> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c
[ ... ]
> @@ -258,11 +282,29 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
>  	return 0;
>  }
>  
> +static int drv260x_open(struct input_dev *input)
> +{
> +	struct drv260x_data *haptics = input_get_drvdata(input);
> +
> +	if (haptics->work_disabled)
> +		return -EIO;
> +
> +	gpiod_set_value(haptics->enable_gpio, 1);
> +	/* Data sheet says to wait 250us before trying to communicate */
> +	fsleep(250);
> +
> +	return 0;
> +}

[Severity: High]
This is a pre-existing issue, but will device configuration be permanently
lost if system power is cut during suspend while the input device is closed?

During system suspend, VDD power can be cut. Because drv260x_resume() checks
input_device_enabled(), it skips drv260x_init() if the device was closed.
When userspace later opens the device, drv260x_open() asserts the enable
GPIO but does not reinitialize the hardware registers, leaving the device
in an erratic default state.

>  static void drv260x_close(struct input_dev *input)
>  {
>  	struct drv260x_data *haptics = input_get_drvdata(input);
>  	int error;
>  
> +	/* PM has not restored register access yet. */
> +	if (haptics->work_disabled)
> +		return;

[Severity: High]
Could this early return leave the device permanently wedged if it is closed
after a system resume failure?

If drv260x_resume() fails, it leaves work_disabled = true. If userspace
then closes the device, this early return skips clearing the wedge state. On
the next suspend/resume cycle, drv260x_resume() skips recovery entirely
because input_device_enabled() evaluates to false (since users is 0). The
work_disabled flag would remain stuck to true, returning -EIO on all future
opens.

>  
>  	cancel_work_sync(&haptics->work);

[Severity: High]
This is a pre-existing issue, but does relying on close() for cancellation
risk a use-after-free during device unbind?

When the device is unbound, input_unregister_device() calls drv260x_close()
and then stops the background ff-memless timer. If the asynchronous timer
fires in that window, it can reschedule the work item after
cancel_work_sync() has already returned. Because the work item lacks a
registered devm cleanup action, it remains pending and could execute after
the memory has been freed.

[ ... ]
> @@ -398,6 +440,11 @@ static int drv260x_init(struct drv260x_data *haptics)
>  		return 0;
>  	}
>  
> +	if (haptics->calibration_valid)
> +		return regmap_bulk_write(haptics->regmap, DRV260X_CAL_COMP,
> +					 haptics->calibration_data,
> +					 ARRAY_SIZE(haptics->calibration_data));
> +

[Severity: Low]
Does returning the error code from regmap_bulk_write() directly violate the
explicit failure paths coding style?

The input subsystem maintainer guidelines prefer the expanded error checking
form instead of returning the action directly in functions with multiple
failure points.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831081227.1794986-1-mauriziocasciano7@gmail.com?part=1

[PATCH v7] Input: drv260x: Fix suspend and resume sequencing

[Maurizio Casciano]

Force-feedback playback is queued asynchronously, but system suspend can
cut power while the worker is pending. Disable and drain the work item
before entering standby, and keep force-feedback quiesced until resume has
restored communication.

An input device can be closed and reopened without playing an effect in
between. Since close lowers the enable GPIO, assert it and observe the
startup delay in the open callback so a following suspend can access the
registers.

Disabling the regulator may remove power and erase the device
configuration. Preserve the actuator mode selected by firmware, cache the
initial automatic-calibration results, and reapply the complete
configuration during resume. Use repeatable multi-register writes instead
of registering persistent regmap patches, which would append another copy
on every reinitialization.

Remember that configuration may have been lost across suspend even when
the input device was closed and resume therefore skipped hardware access.
Restore it on the next open. If resume recovery failed, let a later open
retry regulator enablement and initialization before re-enabling playback
work, instead of leaving the device permanently inaccessible.

Track whether this consumer has enabled the regulator so devres cleanup
does not issue an unbalanced disable after a resume failure. Make regulator
and work reference counts idempotent across PM retries. On removal,
unregister the input device to shut down the ff-memless timer before the
final work cancellation, preventing the timer from requeueing work during
teardown.

Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/linux-input/20260831082820.063D81F00A3D@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260831011526.24AB11F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260830143050.03E081F000E9@smtp.kernel.org/
Link: https://lore.kernel.org/linux-input/20260829230740.126461F000E9@smtp.kernel.org/
Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Link: https://lore.kernel.org/linux-input/apLD91vzHIrLOPWC@google.com/
Link: https://www.ti.com/lit/ds/symlink/drv2604.pdf
Assisted-by: Codex:gpt-5.6-sol [sparse]
Signed-off-by: Maurizio Casciano <mauriziocasciano7@gmail.com>
---
Changes in v7:
- Recover a failed-resume work-disable state from the next input open instead
  of returning -EIO permanently.
- Mark configuration for deferred restoration when suspend occurred while
  the input device was closed, and restore it on the next open.
- Add an explicit remove callback which unregisters the input device before
  cancelling the work item, closing the ff-memless timer requeue window.
- Expand cached-calibration write error handling and retain idempotent work
  and regulator state tracking across PM retries.

Validation:
- Integration commit 780663f0af3b, containing the exact drv260x blob
  1eb71e6adc53, passed olddefconfig, a focused W=1 C=2 CHECK=sparse build,
  and a full Debian package build on x86_64. The full build log contains no
  warning in drivers/input/misc/drv260x.c.
- Lenovo Yoga Book YB1-X91L running
  7.2.0-yogabook-20260831-153058 passed a 15-second s2idle cycle with RTC
  wake. Both DRV2604 devices then passed five open/close cycles and a
  force-feedback rumble upload/play/erase sequence.
- Camera streaming, SOF playback and capture, and idle runtime-PM state were
  rechecked after resume. No drv260x, I2C, regulator-balance, or severe kernel
  error was present in the boot log.
- The tablet uses dummy vbat regulators for both DRV2604 devices, so actual
  regulator power loss and injected resume-error cleanup could not be
  exercised on this hardware.

 drivers/input/misc/drv260x.c | 264 ++++++++++++++++++++++++++++-------
 1 file changed, 216 insertions(+), 48 deletions(-)

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..1eb71e6adc53 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -181,7 +181,12 @@
  * @work: Work item used to off load the enable/disable of the vibration
  * @enable_gpio: Pointer to the gpio used for enable/disabling
  * @regulator: Pointer to the regulator for the IC
+ * @regulator_enabled: Whether this consumer has enabled the regulator
+ * @work_disabled: Whether playback work is disabled pending PM recovery
+ * @needs_reinit: Whether device configuration must be restored on open
+ * @calibration_valid: Whether calibration results have been cached
  * @magnitude: Magnitude of the vibration event
+ * @calibration_data: Cached automatic calibration results
  * @mode: The operating mode of the IC (LRA_NO_CAL, ERM or LRA)
  * @library: The vibration library to be used
  * @rated_voltage: The rated_voltage of the actuator
@@ -194,7 +199,12 @@ struct drv260x_data {
 	struct work_struct work;
 	struct gpio_desc *enable_gpio;
 	struct regulator *regulator;
+	bool regulator_enabled;
+	bool work_disabled;
+	bool needs_reinit;
+	bool calibration_valid;
 	u8 magnitude;
+	u8 calibration_data[3];
 	u32 mode;
 	u32 library;
 	int rated_voltage;
@@ -215,6 +225,24 @@ static int drv260x_calculate_voltage(unsigned int voltage)
 	return (voltage * 255 / 5600);
 }
 
+static void drv260x_disable_work(struct drv260x_data *haptics)
+{
+	if (haptics->work_disabled)
+		return;
+
+	disable_work_sync(&haptics->work);
+	haptics->work_disabled = true;
+}
+
+static void drv260x_enable_work(struct drv260x_data *haptics)
+{
+	if (!haptics->work_disabled)
+		return;
+
+	enable_work(&haptics->work);
+	haptics->work_disabled = false;
+}
+
 static void drv260x_worker(struct work_struct *work)
 {
 	struct drv260x_data *haptics = container_of(work, struct drv260x_data, work);
@@ -243,8 +271,6 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 {
 	struct drv260x_data *haptics = input_get_drvdata(input);
 
-	haptics->mode = DRV260X_LRA_NO_CAL_MODE;
-
 	/* Scale u16 magnitude into u8 register value */
 	if (effect->u.rumble.strong_magnitude > 0)
 		haptics->magnitude = effect->u.rumble.strong_magnitude >> 8;
@@ -258,21 +284,6 @@ static int drv260x_haptics_play(struct input_dev *input, void *data,
 	return 0;
 }
 
-static void drv260x_close(struct input_dev *input)
-{
-	struct drv260x_data *haptics = input_get_drvdata(input);
-	int error;
-
-	cancel_work_sync(&haptics->work);
-
-	error = regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
-	if (error)
-		dev_err(&haptics->client->dev,
-			"Failed to enter standby mode: %d\n", error);
-
-	gpiod_set_value(haptics->enable_gpio, 0);
-}
-
 static const struct reg_sequence drv260x_lra_cal_regs[] = {
 	{ DRV260X_MODE, DRV260X_AUTO_CAL },
 	{ DRV260X_CTRL3, DRV260X_NG_THRESH_2 | DRV260X_RTP_UNSIGNED_DATA },
@@ -338,9 +349,9 @@ static int drv260x_init(struct drv260x_data *haptics)
 
 	switch (haptics->mode) {
 	case DRV260X_LRA_MODE:
-		error = regmap_register_patch(haptics->regmap,
-					      drv260x_lra_cal_regs,
-					      ARRAY_SIZE(drv260x_lra_cal_regs));
+		error = regmap_multi_reg_write(haptics->regmap,
+					       drv260x_lra_cal_regs,
+					       ARRAY_SIZE(drv260x_lra_cal_regs));
 		if (error) {
 			dev_err(&haptics->client->dev,
 				"Failed to write LRA calibration registers: %d\n",
@@ -351,9 +362,9 @@ static int drv260x_init(struct drv260x_data *haptics)
 		break;
 
 	case DRV260X_ERM_MODE:
-		error = regmap_register_patch(haptics->regmap,
-					      drv260x_erm_cal_regs,
-					      ARRAY_SIZE(drv260x_erm_cal_regs));
+		error = regmap_multi_reg_write(haptics->regmap,
+					       drv260x_erm_cal_regs,
+					       ARRAY_SIZE(drv260x_erm_cal_regs));
 		if (error) {
 			dev_err(&haptics->client->dev,
 				"Failed to write ERM calibration registers: %d\n",
@@ -374,9 +385,9 @@ static int drv260x_init(struct drv260x_data *haptics)
 		break;
 
 	default:
-		error = regmap_register_patch(haptics->regmap,
-					      drv260x_lra_init_regs,
-					      ARRAY_SIZE(drv260x_lra_init_regs));
+		error = regmap_multi_reg_write(haptics->regmap,
+					       drv260x_lra_init_regs,
+					       ARRAY_SIZE(drv260x_lra_init_regs));
 		if (error) {
 			dev_err(&haptics->client->dev,
 				"Failed to write LRA init registers: %d\n",
@@ -398,6 +409,19 @@ static int drv260x_init(struct drv260x_data *haptics)
 		return 0;
 	}
 
+	if (haptics->calibration_valid) {
+		error = regmap_bulk_write(haptics->regmap, DRV260X_CAL_COMP,
+					  haptics->calibration_data,
+					  ARRAY_SIZE(haptics->calibration_data));
+		if (error) {
+			dev_err(&haptics->client->dev,
+				"Failed to restore calibration registers: %d\n", error);
+			return error;
+		}
+
+		return 0;
+	}
+
 	error = regmap_write(haptics->regmap, DRV260X_GO, DRV260X_GO_BIT);
 	if (error) {
 		dev_err(&haptics->client->dev,
@@ -423,7 +447,13 @@ static int drv260x_init(struct drv260x_data *haptics)
 		}
 	} while (cal_buf == DRV260X_GO_BIT);
 
-	return 0;
+	error = regmap_bulk_read(haptics->regmap, DRV260X_CAL_COMP,
+				 haptics->calibration_data,
+				 ARRAY_SIZE(haptics->calibration_data));
+	if (!error)
+		haptics->calibration_valid = true;
+
+	return error;
 }
 
 static const struct regmap_config drv260x_regmap_config = {
@@ -434,11 +464,110 @@ static const struct regmap_config drv260x_regmap_config = {
 	.cache_type = REGCACHE_NONE,
 };
 
+static int drv260x_regulator_enable(struct drv260x_data *haptics)
+{
+	int error;
+
+	if (haptics->regulator_enabled)
+		return 0;
+
+	error = regulator_enable(haptics->regulator);
+	if (!error)
+		haptics->regulator_enabled = true;
+
+	return error;
+}
+
+static int drv260x_regulator_disable(struct drv260x_data *haptics)
+{
+	int error;
+
+	if (!haptics->regulator_enabled)
+		return 0;
+
+	error = regulator_disable(haptics->regulator);
+	if (!error)
+		haptics->regulator_enabled = false;
+
+	return error;
+}
+
 static void drv260x_power_off(void *data)
 {
 	struct drv260x_data *haptics = data;
 
-	regulator_disable(haptics->regulator);
+	drv260x_regulator_disable(haptics);
+}
+
+static int drv260x_open(struct input_dev *input)
+{
+	struct drv260x_data *haptics = input_get_drvdata(input);
+	bool recover_work = haptics->work_disabled;
+	int disable_error, error;
+
+	if (recover_work) {
+		error = drv260x_regulator_enable(haptics);
+		if (error) {
+			dev_err(&haptics->client->dev,
+				"Failed to enable regulator: %d\n", error);
+			return error;
+		}
+	}
+
+	gpiod_set_value(haptics->enable_gpio, 1);
+	/* Data sheet says to wait 250us before trying to communicate */
+	fsleep(250);
+
+	if (haptics->needs_reinit || recover_work) {
+		error = drv260x_init(haptics);
+		if (error) {
+			dev_err(&haptics->client->dev,
+				"Failed to restore configuration: %d\n", error);
+			gpiod_set_value(haptics->enable_gpio, 0);
+			if (recover_work) {
+				disable_error = drv260x_regulator_disable(haptics);
+				if (disable_error)
+					dev_err(&haptics->client->dev,
+						"Failed to disable regulator: %d\n",
+						disable_error);
+			}
+			return error;
+		}
+
+		haptics->needs_reinit = false;
+	}
+
+	if (recover_work)
+		drv260x_enable_work(haptics);
+
+	return 0;
+}
+
+static void drv260x_close(struct input_dev *input)
+{
+	struct drv260x_data *haptics = input_get_drvdata(input);
+	int error;
+
+	/* PM has not restored register access yet. */
+	if (haptics->work_disabled)
+		return;
+
+	cancel_work_sync(&haptics->work);
+
+	error = regmap_write(haptics->regmap, DRV260X_MODE, DRV260X_STANDBY);
+	if (error)
+		dev_err(&haptics->client->dev,
+			"Failed to enter standby mode: %d\n", error);
+
+	gpiod_set_value(haptics->enable_gpio, 0);
+}
+
+static void drv260x_remove(struct i2c_client *client)
+{
+	struct drv260x_data *haptics = i2c_get_clientdata(client);
+
+	input_unregister_device(haptics->input_dev);
+	cancel_work_sync(&haptics->work);
 }
 
 static int drv260x_probe(struct i2c_client *client)
@@ -506,7 +635,7 @@ static int drv260x_probe(struct i2c_client *client)
 		return error;
 	}
 
-	error = regulator_enable(haptics->regulator);
+	error = drv260x_regulator_enable(haptics);
 	if (error) {
 		dev_err(dev, "Failed to enable regulator: %d\n", error);
 		return error;
@@ -528,6 +657,7 @@ static int drv260x_probe(struct i2c_client *client)
 	}
 
 	haptics->input_dev->name = "drv260x:haptics";
+	haptics->input_dev->open = drv260x_open;
 	haptics->input_dev->close = drv260x_close;
 	input_set_drvdata(haptics->input_dev, haptics);
 	input_set_capability(haptics->input_dev, EV_FF, FF_RUMBLE);
@@ -569,62 +699,99 @@ static int drv260x_probe(struct i2c_client *client)
 static int drv260x_suspend(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	bool restore_work = false;
+	int error, restore_error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
+		restore_work = !haptics->work_disabled;
+		drv260x_disable_work(haptics);
+
+		/* A failed resume can leave the device already powered down. */
+		if (!haptics->regulator_enabled)
+			goto mark_reinit;
+
 		error = regmap_update_bits(haptics->regmap,
 					   DRV260X_MODE,
 					   DRV260X_STANDBY_MASK,
 					   DRV260X_STANDBY);
 		if (error) {
 			dev_err(dev, "Failed to set standby mode\n");
-			return error;
+			goto err_enable_work;
 		}
 
 		gpiod_set_value(haptics->enable_gpio, 0);
 
-		error = regulator_disable(haptics->regulator);
+		error = drv260x_regulator_disable(haptics);
 		if (error) {
 			dev_err(dev, "Failed to disable regulator\n");
-			regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
-			return error;
+			goto err_leave_standby;
 		}
 	}
 
+mark_reinit:
+	haptics->needs_reinit = true;
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_leave_standby:
+	gpiod_set_value(haptics->enable_gpio, 1);
+	fsleep(250);
+	restore_error = regmap_update_bits(haptics->regmap,
+					   DRV260X_MODE,
+					   DRV260X_STANDBY_MASK, 0);
+	if (restore_error) {
+		dev_err(dev, "Failed to leave standby mode: %d\n", restore_error);
+		restore_work = false;
+	}
+err_enable_work:
+	if (restore_work)
+		drv260x_enable_work(haptics);
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static int drv260x_resume(struct device *dev)
 {
 	struct drv260x_data *haptics = dev_get_drvdata(dev);
-	int error;
+	int disable_error, error;
 
-	guard(mutex)(&haptics->input_dev->mutex);
+	mutex_lock(&haptics->input_dev->mutex);
 
 	if (input_device_enabled(haptics->input_dev)) {
-		error = regulator_enable(haptics->regulator);
+		drv260x_disable_work(haptics);
+
+		error = drv260x_regulator_enable(haptics);
 		if (error) {
 			dev_err(dev, "Failed to enable regulator\n");
-			return error;
+			goto err_unlock;
 		}
 
-		error = regmap_update_bits(haptics->regmap,
-					   DRV260X_MODE,
-					   DRV260X_STANDBY_MASK, 0);
+		gpiod_set_value(haptics->enable_gpio, 1);
+		fsleep(250);
+
+		error = drv260x_init(haptics);
 		if (error) {
-			dev_err(dev, "Failed to unset standby mode\n");
-			regulator_disable(haptics->regulator);
-			return error;
+			dev_err(dev, "Failed to restore configuration: %d\n", error);
+			goto err_disable_regulator;
 		}
 
-		gpiod_set_value(haptics->enable_gpio, 1);
+		haptics->needs_reinit = false;
+		drv260x_enable_work(haptics);
 	}
 
+	mutex_unlock(&haptics->input_dev->mutex);
 	return 0;
+
+err_disable_regulator:
+	gpiod_set_value(haptics->enable_gpio, 0);
+	disable_error = drv260x_regulator_disable(haptics);
+	if (disable_error)
+		dev_err(dev, "Failed to disable regulator: %d\n", disable_error);
+err_unlock:
+	mutex_unlock(&haptics->input_dev->mutex);
+	return error;
 }
 
 static DEFINE_SIMPLE_DEV_PM_OPS(drv260x_pm_ops, drv260x_suspend, drv260x_resume);
@@ -657,6 +824,7 @@ MODULE_DEVICE_TABLE(of, drv260x_of_match);
 
 static struct i2c_driver drv260x_driver = {
 	.probe		= drv260x_probe,
+	.remove		= drv260x_remove,
 	.driver		= {
 		.name	= "drv260x-haptics",
 		.acpi_match_table = ACPI_PTR(drv260x_acpi_match),
-- 
With Best Regards,
Maurizio Casciano