Re: [PATCH 5/8] Input: cap11xx - add reset gpio support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Inefficient delay using usleep_range() for a large duration (400-500ms).
- [Low] Missing explicit inclusion of <linux/delay.h>.
--

commit 83de47341df3454fdb8216dc0a95948c3f158a40
Author: Jun Yan <jerrysteve1101@gmail.com>

Input: cap11xx - add reset gpio support

This commit adds hardware reset support for CAP11xx devices by toggling a
dedicated reset GPIO during the i2c probe phase. It incorporates specific
timing requirements for the reset filter and active reset duration.

> diff --git a/drivers/input/keyboard/cap11xx.c b/drivers/input/keyboard/cap11xx.c
> index 686174722204c..7015aa7654ce7 100644
> --- a/drivers/input/keyboard/cap11xx.c
> +++ b/drivers/input/keyboard/cap11xx.c

[ ... ]

> @@ -452,6 +457,17 @@ static int cap11xx_i2c_probe(struct i2c_client *i2c_client)
>  	if (IS_ERR(priv->regmap))
>  		return PTR_ERR(priv->regmap);
>  
> +	priv->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(priv->reset_gpio))
> +		return dev_err_probe(dev, PTR_ERR(priv->reset_gpio),
> +							"Failed to get 'reset' GPIO\n");
> +	if (priv->reset_gpio) {
> +		gpiod_set_value_cansleep(priv->reset_gpio, 1);
> +		usleep_range(CAP11XX_T_RST_FILT_MIN, CAP11XX_T_RST_FILT_MIN * 2);
> +		gpiod_set_value_cansleep(priv->reset_gpio, 0);
> +		usleep_range(CAP11XX_T_RST_ON_MIN, CAP11XX_T_RST_ON_MAX);

[Severity: Low]
Does this regression introduce unnecessary overhead by using usleep_range()
for a 400-500ms delay in cap11xx_i2c_probe()?

According to kernel timer guidelines, usleep_range() is designed for short
delays (10us to 20ms) and sets up high-resolution timers. For delays longer
than 20ms, using msleep() or fsleep() avoids unnecessary overhead and
expensive CPU wakeups.

[Severity: Low]
Could this regression in header dependencies be avoided by explicitly
including <linux/delay.h> since we are now calling usleep_range() here?

While the code might compile due to implicit inclusion via other headers,
relying on implicit includes can sometimes lead to build failures on other
architectures or if intermediate headers are refactored in the future.

> +	}
> +
>  	error = regmap_read(priv->regmap, CAP11XX_REG_PRODUCT_ID, &val);
>  	if (error)
>  		return dev_err_probe(dev, error, "Failed to read product ID\n");

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