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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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