[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

[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