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

[Maurizio Casciano <mauriziocasciano7@gmail.com>]

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