[PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure

[PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure

[Chao Huang]

From: Chao Huang <huangchao@kylinos.cn>

If enabling VDDIO fails after VDD has been enabled, the power-up
path returns without disabling VDD. This leaves the regulator enabled
and its enable count unbalanced.

Disable VDD before returning the VDDIO error.

Fixes: eb16f59e8e58 ("HID: i2c-hid: goodix: Add mainboard-vddio-supply")
Signed-off-by: Chao Huang <huangchao@kylinos.cn>
---
 drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
index f1597ad67e7c..f4dbcd1d1d47 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
@@ -51,8 +51,10 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
 		return ret;
 
 	ret = regulator_enable(ihid_goodix->vddio);
-	if (ret)
+	if (ret) {
+		regulator_disable(ihid_goodix->vdd);
 		return ret;
+	}
 
 	if (ihid_goodix->timings->post_power_delay_ms)
 		msleep(ihid_goodix->timings->post_power_delay_ms);
-- 
2.25.1

Re: [PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing NULL check on device_get_match_data() return value leads to NULL pointer dereference if the driver is instantiated via sysfs.
--

--- Patch [1]: [PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure ---
commit 2325dfd5a42520477bfee927eabe09ddf852ba83
Author: Chao Huang <huangchao@kylinos.cn>
Subject: HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure

This commit prevents a regulator enable count imbalance during device power up.
It disables the VDD regulator if enabling the VDDIO regulator fails.

> diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> index f1597ad67e7c8..f4dbcd1d1d472 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
> @@ -51,8 +51,10 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
>  		return ret;
>  
>  	ret = regulator_enable(ihid_goodix->vddio);
> -	if (ret)
> +	if (ret) {
> +		regulator_disable(ihid_goodix->vdd);
>  		return ret;
> +	}
>  
>  	if (ihid_goodix->timings->post_power_delay_ms)

[Severity: High]
This is a pre-existing issue, but can this lead to a NULL pointer
dereference if the driver is instantiated via the I2C sysfs new_device
interface?

In i2c_hid_of_goodix_probe(), the driver unconditionally assigns the
result of device_get_match_data() to ihid_goodix->timings:

drivers/hid/i2c-hid/i2c-hid-of-goodix.c:i2c_hid_of_goodix_probe() {
    ...
    ihid_goodix->timings = device_get_match_data(&client->dev);
    ...
}

When created via sysfs (e.g., echo gt7375p 0x12 > new_device), the device
lacks an OF/fwnode, so device_get_match_data() returns NULL.

Does this cause a crash when ihid_goodix->timings is subsequently
dereferenced here in goodix_i2c_hid_power_up()?

>  		msleep(ihid_goodix->timings->post_power_delay_ms);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_FE1CA7ED99022E1734F8C9CAB538AB477405@qq.com?part=1

Re: [PATCH] HID: i2c-hid: goodix: Disable VDD on VDDIO enable failure

[Doug Anderson]

Hi,

On Wed, Jul 22, 2026 at 2:37 AM Chao Huang <958028483@qq.com> wrote:
>
> From: Chao Huang <huangchao@kylinos.cn>
>
> If enabling VDDIO fails after VDD has been enabled, the power-up
> path returns without disabling VDD. This leaves the regulator enabled
> and its enable count unbalanced.
>
> Disable VDD before returning the VDDIO error.
>
> Fixes: eb16f59e8e58 ("HID: i2c-hid: goodix: Add mainboard-vddio-supply")
> Signed-off-by: Chao Huang <huangchao@kylinos.cn>
> ---
>  drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Thanks for the fix.

Reviewed-by: Douglas Anderson <dianders@chromium.org>