[PATCH 0/2] Input: Add ACPI support needed by Yoga Book DRV2604

[PATCH 0/2] Input: Add ACPI support needed by Yoga Book DRV2604

[Maurizio Casciano]

The Lenovo Yoga Book YB1-X91F/L contains two ACPI-enumerated DRV2604
haptic controllers.  ACPI does not describe a controllable vbat supply
or provide a _DSD function mapping for the enable GPIO, so the existing
driver cannot initialize the devices reliably.

Make the vbat regulator optional while preserving the existing behavior
when a supply is described, then map the first ACPI GPIO resource as the
enable line before requesting it.

Tested on a Lenovo Yoga Book YB1-X91L with both controllers probing and
direct per-device force-feedback playback.  A separately posted
platform/x86 series supplies the device mode and waveform properties.

Maurizio Casciano (2):
  Input: drv260x: Make vbat supply optional
  Input: drv260x: Map ACPI enable GPIO

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


base-commit: 9a29ee801f525bcad71fea021bfe2a030885c8df
-- 
2.53.0

[PATCH 1/2] Input: drv260x: Make vbat supply optional

[Maurizio Casciano]

The Lenovo Yoga Book YB1-X91L firmware instantiates two DRV2604
ACPI devices, but does not describe a software-controllable vbat supply
for either device. The generic regulator lookup therefore creates a
dummy supply.

Use the optional regulator lookup and skip regulator operations when no
supply is described. Systems which provide vbat retain the existing
enable, managed-disable, suspend, resume, and error-unwind behavior.

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

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index 6c5c4c53753b..c4fbb10b254e 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -499,22 +499,26 @@ static int drv260x_probe(struct i2c_client *client)
 	haptics->overdrive_voltage = error ? DRV260X_DEF_OD_CLAMP_VOLT :
 					     drv260x_calculate_voltage(voltage);
 
-	haptics->regulator = devm_regulator_get(dev, "vbat");
+	haptics->regulator = devm_regulator_get_optional(dev, "vbat");
 	if (IS_ERR(haptics->regulator)) {
-		error = PTR_ERR(haptics->regulator);
-		dev_err(dev, "unable to get regulator, error: %d\n", error);
-		return error;
+		if (PTR_ERR(haptics->regulator) == -ENODEV) {
+			haptics->regulator = NULL;
+			dev_dbg(dev, "No vbat regulator found\n");
+		} else {
+			error = PTR_ERR(haptics->regulator);
+			return dev_err_probe(dev, error, "Unable to get vbat regulator\n");
+		}
 	}
 
-	error = regulator_enable(haptics->regulator);
-	if (error) {
-		dev_err(dev, "Failed to enable regulator: %d\n", error);
-		return error;
-	}
+	if (haptics->regulator) {
+		error = regulator_enable(haptics->regulator);
+		if (error)
+			return dev_err_probe(dev, error, "Failed to enable regulator\n");
 
-	error = devm_add_action_or_reset(dev, drv260x_power_off, haptics);
-	if (error)
-		return error;
+		error = devm_add_action_or_reset(dev, drv260x_power_off, haptics);
+		if (error)
+			return error;
+	}
 
 	haptics->enable_gpio = devm_gpiod_get_optional(dev, "enable",
 						       GPIOD_OUT_HIGH);
@@ -585,13 +589,15 @@ static int drv260x_suspend(struct device *dev)
 
 		gpiod_set_value(haptics->enable_gpio, 0);
 
-		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;
+		if (haptics->regulator) {
+			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;
+			}
 		}
 	}
 
@@ -606,10 +612,12 @@ static int drv260x_resume(struct device *dev)
 	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;
+		if (haptics->regulator) {
+			error = regulator_enable(haptics->regulator);
+			if (error) {
+				dev_err(dev, "Failed to enable regulator\n");
+				return error;
+			}
 		}
 
 		error = regmap_update_bits(haptics->regmap,
@@ -617,7 +625,8 @@ static int drv260x_resume(struct device *dev)
 					   DRV260X_STANDBY_MASK, 0);
 		if (error) {
 			dev_err(dev, "Failed to unset standby mode\n");
-			regulator_disable(haptics->regulator);
+			if (haptics->regulator)
+				regulator_disable(haptics->regulator);
 			return error;
 		}
 
-- 
2.53.0

Re: [PATCH 1/2] Input: drv260x: Make vbat supply optional

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the resume callback attempts I2C writes before powering on the device and fails to reinitialize volatile registers after power loss. In `drv260x_resume()`, `regmap_update_bits()` is called to clear the standby bit over I2C before asserting the enable GPIO. Because the DRV260X entirely disables its I2C interface when the EN pin is low, the I2C write will invariably fail with `-EREMOTEIO`. This failure causes the resume function to abort prematurely without ever asserting the enable GPIO, leaving the device permanently unpowered and broken after a single suspend/resume cycle. Additionally, because power is cut during suspend, the device loses all volatile configurations (e.g., calibration, rated/overdrive voltages), which are never restored upon resume.
- [High] This problem wasn't introduced by this patch, but there is concurrent access and a lack of synchronization between the async force-feedback worker and device suspend paths. The force-feedback worker (`drv260x_worker`) executes asynchronously and does not hold a lock to serialize it against system suspend. During suspend, `drv260x_suspend` shuts down the device by asserting the EN pin low and disabling the vbat regulator. Because the worker is not canceled or flushed during suspend, it can execute while the device is powered down, driving the EN GPIO high and attempting I2C writes while the vbat supply is disabled. This can cause I2C timeouts, corrupt device state, and physically stress the chip by back-powering it via the GPIO.
--

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

Input: drv260x: Make vbat supply optional

This patch makes the vbat regulator lookup optional to support systems that
do not describe a software-controllable supply. Systems with a vbat supply
described will retain the existing enable, disable, and power management
behaviors.

> 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

[ ... ]

> @@ -585,13 +589,15 @@ static int drv260x_suspend(struct device *dev)
>  
>  		gpiod_set_value(haptics->enable_gpio, 0);
>  
> -		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;
> +		if (haptics->regulator) {
> +			error = regulator_disable(haptics->regulator);

[Severity: High]
This is a pre-existing issue, but is there a lack of synchronization between
the async force-feedback worker and device suspend paths?

The force-feedback worker (drv260x_worker) executes asynchronously and does
not hold a lock to serialize it against system suspend. During suspend, the
device is shut down here by asserting the EN pin low and disabling the vbat
regulator.

Because the worker is not canceled or flushed during suspend, couldn't it
execute while the device is powered down, driving the EN GPIO high and
attempting I2C writes while the vbat supply is disabled? This might cause I2C
timeouts, corrupt device state, and physically stress the chip by
back-powering it via the GPIO.

> +			if (error) {
> +				dev_err(dev, "Failed to disable regulator\n");
> +				regmap_update_bits(haptics->regmap,
> +						   DRV260X_MODE,
> +						   DRV260X_STANDBY_MASK, 0);
> +				return error;
> +			}
>  		}
>  	}

[ ... ]

> @@ -606,10 +612,12 @@ static int drv260x_resume(struct device *dev)
>  	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;
> +		if (haptics->regulator) {
> +			error = regulator_enable(haptics->regulator);
> +			if (error) {
> +				dev_err(dev, "Failed to enable regulator\n");
> +				return error;
> +			}
>  		}
>  
>  		error = regmap_update_bits(haptics->regmap,
> @@ -617,7 +625,8 @@ static int drv260x_resume(struct device *dev)
>  					   DRV260X_STANDBY_MASK, 0);
>  		if (error) {
>  			dev_err(dev, "Failed to unset standby mode\n");
> -			regulator_disable(haptics->regulator);
> +			if (haptics->regulator)
> +				regulator_disable(haptics->regulator);
>  			return error;
>  		}
>  

[Severity: High]
This isn't a bug introduced by this patch, but does the resume callback
attempt I2C writes before powering on the device?

Here in drv260x_resume(), regmap_update_bits() is called to clear the standby
bit over I2C before asserting the enable GPIO. Because the DRV260X entirely
disables its I2C interface when the EN pin is low, won't this I2C write
invariably fail with -EREMOTEIO?

This failure would cause the resume function to abort prematurely without ever
asserting the enable GPIO, leaving the device permanently unpowered and broken
after a single suspend/resume cycle.

Additionally, because power is cut during suspend, does the device lose all
volatile configurations (e.g., calibration, rated/overdrive voltages) that
should be restored upon resume?

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

Re: [PATCH 1/2] Input: drv260x: Make vbat supply optional

[Dmitry Torokhov]

Hi Maurizio,

On Wed, Aug 26, 2026 at 03:22:10PM +0200, Maurizio Casciano wrote:
> The Lenovo Yoga Book YB1-X91L firmware instantiates two DRV2604
> ACPI devices, but does not describe a software-controllable vbat supply
> for either device. The generic regulator lookup therefore creates a
> dummy supply.

Yes, that is how most of the drivers handle supplies on ACPI systems.

> 
> Use the optional regulator lookup and skip regulator operations when no
> supply is described. Systems which provide vbat retain the existing
> enable, managed-disable, suspend, resume, and error-unwind behavior.

regulator_get_optional() should only be used in cases where supply can
be left unconnected in normal use. This is different from this case
where supply *is connected* but it is managed by ACPI/board firmware.

IOW this patch is not needed.

Thanks.

-- 
Dmitry

Re: [PATCH 1/2] Input: drv260x: Make vbat supply optional

[Maurizio Casciano]

Agreed, thank you. The supply is connected and managed by ACPI/board
firmware, so the dummy regulator is the expected representation here.

I am withdrawing this patch; vbat remains mandatory in drv260x.

Maurizio

[PATCH 2/2] Input: drv260x: Map ACPI enable GPIO

[Maurizio Casciano]

Some ACPI DRV2604 devices describe the enable line as the first GPIO
resource in _CRS but provide no _DSD mapping for its function. The GPIO
consumer lookup then returns no descriptor, leaving Yoga Book haptics
unable to complete calibration.

Add a managed ACPI GPIO mapping before requesting the optional enable
line so these devices can power their haptic path.

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

diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
index c4fbb10b254e..b6243ce8157d 100644
--- a/drivers/input/misc/drv260x.c
+++ b/drivers/input/misc/drv260x.c
@@ -434,6 +434,13 @@ static const struct regmap_config drv260x_regmap_config = {
 	.cache_type = REGCACHE_NONE,
 };
 
+/* ACPI DRV2604 devices describe enable as their first GPIO resource. */
+static const struct acpi_gpio_params drv260x_enable_gpio = { 0, 0, false };
+static const struct acpi_gpio_mapping drv260x_acpi_gpios[] = {
+	{ "enable-gpios", &drv260x_enable_gpio, 1 },
+	{ }
+};
+
 static void drv260x_power_off(void *data)
 {
 	struct drv260x_data *haptics = data;
@@ -448,6 +455,14 @@ static int drv260x_probe(struct i2c_client *client)
 	u32 voltage;
 	int error;
 
+	if (has_acpi_companion(dev)) {
+		error = devm_acpi_dev_add_driver_gpios(dev,
+						       drv260x_acpi_gpios);
+		if (error)
+			return dev_err_probe(dev, error,
+					     "Failed to add ACPI GPIO mapping\n");
+	}
+
 	haptics = devm_kzalloc(dev, sizeof(*haptics), GFP_KERNEL);
 	if (!haptics)
 		return -ENOMEM;
-- 
2.53.0

Re: [PATCH 2/2] Input: drv260x: Map ACPI enable GPIO

[sashiko-bot]

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

Pre-existing issues:
- [High] I2C writes in `drv260x_resume()` and error paths of `drv260x_suspend()` will fail because the enable GPIO is not asserted.
--

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

Input: drv260x: Map ACPI enable GPIO
    
Some ACPI DRV2604 devices describe the enable line as the first GPIO
resource in _CRS but provide no _DSD mapping for its function. Add a managed
ACPI GPIO mapping before requesting the optional enable line so these devices
can power their haptic path.

> diff --git a/drivers/input/misc/drv260x.c b/drivers/input/misc/drv260x.c
> index c4fbb10b254e9..b6243ce8157d6 100644
> --- a/drivers/input/misc/drv260x.c
> +++ b/drivers/input/misc/drv260x.c

[ ... ]

> @@ -448,6 +455,14 @@ static int drv260x_probe(struct i2c_client *client)
>  	u32 voltage;
>  	int error;
>  
> +	if (has_acpi_companion(dev)) {
> +		error = devm_acpi_dev_add_driver_gpios(dev,
> +						       drv260x_acpi_gpios);
> +		if (error)
> +			return dev_err_probe(dev, error,
> +					     "Failed to add ACPI GPIO mapping\n");
> +	}
> +
>  	haptics = devm_kzalloc(dev, sizeof(*haptics), GFP_KERNEL);
>  	if (!haptics)
>  		return -ENOMEM;

[Severity: High]
This isn't a bug introduced by this patch, but it appears there is a
pre-existing issue in the suspend and resume paths related to the enable GPIO.

Looking at drv260x_resume():

drivers/input/misc/drv260x.c:drv260x_resume() {
...
		error = regmap_update_bits(haptics->regmap,
					   DRV260X_MODE,
					   DRV260X_STANDBY_MASK, 0);
		if (error) {
			dev_err(dev, "Failed to unset standby mode\n");
			if (haptics->regulator)
				regulator_disable(haptics->regulator);
			return error;
		}

		gpiod_set_value(haptics->enable_gpio, 1);
...
}

Will this I2C transaction fail because it occurs while the chip is still held
in reset/shutdown by the GPIO? Since the enable GPIO is not asserted until
after the I2C write, and the driver aborts the resume on error, does this
leave the chip permanently disabled?

Additionally, because pulling the enable GPIO low resets all chip registers and
REGCACHE_NONE is used, is the device configuration (calibration, over-drive
voltage, library selection) lost and never restored during resume?

A similar issue appears to exist in the error path of drv260x_suspend():

drivers/input/misc/drv260x.c:drv260x_suspend() {
...
		gpiod_set_value(haptics->enable_gpio, 0);

		if (haptics->regulator) {
			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;
			}
		}
...
}

Does the error path attempt to communicate with the chip via I2C without
restoring the enable GPIO to 1?

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

Re: [PATCH 2/2] Input: drv260x: Map ACPI enable GPIO

[Dmitry Torokhov]

Hi Maurizio,

On Wed, Aug 26, 2026 at 03:22:11PM +0200, Maurizio Casciano wrote:
> Some ACPI DRV2604 devices describe the enable line as the first GPIO
> resource in _CRS but provide no _DSD mapping for its function. The GPIO
> consumer lookup then returns no descriptor, leaving Yoga Book haptics
> unable to complete calibration.
> 
> Add a managed ACPI GPIO mapping before requesting the optional enable
> line so these devices can power their haptic path.

I would prefer to avoid encoding board behaviors in a generic driver.
Can we try using a software node to provide GPIO mapping for the device?
Maybe in drivers/platform/x86/lenovo/yogabook.c?

Thanks.

-- 
Dmitry

Re: [PATCH 2/2] Input: drv260x: Map ACPI enable GPIO

[Maurizio Casciano]

Agreed, thank you. I am withdrawing this generic-driver patch.

The enable-GPIO mappings are now supplied by Yoga Book board software nodes
in the platform/x86 v2 series. That series also carries the supporting fix
which attaches GPIO-provider secondary software nodes to the matching
gpio_device.

Maurizio