[PATCH] regulator: sgm3804: handle gpiod_get_value_cansleep() error returns

[PATCH] regulator: sgm3804: handle gpiod_get_value_cansleep() error returns

[kr494167]

From: Surendra Singh Chouhan <kr494167@gmail.com>

sgm3804_sync_regcache_state() evaluated !gpiod_get_value_cansleep(...) to
check if the enable GPIOs were down.

gpiod_get_value_cansleep() returns 1 if active, 0 if inactive, and a
negative error code (e.g. -EIO or -EINVAL) on failure. Evaluating
!gpiod_get_value_cansleep(...) treats a negative error code as boolean
false, swallowing GPIO read failures and misinterpreting a failed read as
an active GPIO. This improperly attempts to sync regcache registers
when the IC may be powered down.

Fix this by capturing the return values of gpiod_get_value_cansleep() for
both positive and negative rails and propagating any error immediately.
In addition, replace mutex_init() with devm_mutex_init() in probe().

Fixes: 0c47e1a8cf5d ("regulator: add SGM3804 Dual Output driver")
Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
---
 drivers/regulator/sgm3804-regulator.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/regulator/sgm3804-regulator.c b/drivers/regulator/sgm3804-regulator.c
index c3406cfb73d0..a430c89b4d5d 100644
--- a/drivers/regulator/sgm3804-regulator.c
+++ b/drivers/regulator/sgm3804-regulator.c
@@ -97,11 +97,20 @@ static const struct regmap_config sgm3804_regmap_config = {
 
 static int sgm3804_sync_regcache_state(struct sgm3804_data *ctx)
 {
+	int pos, neg;
+
 	guard(mutex)(&ctx->lock);
 
+	pos = gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]);
+	if (pos < 0)
+		return pos;
+
+	neg = gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL]);
+	if (neg < 0)
+		return neg;
+
 	/* If both GPIOs are down, IC is powered down and I2C writes will fail */
-	if (!gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]) &&
-	    !gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL])) {
+	if (!pos && !neg) {
 		regcache_cache_only(ctx->regmap, true);
 		regcache_mark_dirty(ctx->regmap);
 	} else {
@@ -235,7 +244,9 @@ static int sgm3804_probe(struct i2c_client *i2c)
 	if (!ctx)
 		return -ENOMEM;
 
-	mutex_init(&ctx->lock);
+	ret = devm_mutex_init(dev, &ctx->lock);
+	if (ret)
+		return ret;
 
 	ctx->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
 	if (IS_ERR(ctx->regmap))
-- 
2.55.0

Re: [PATCH] regulator: sgm3804: handle gpiod_get_value_cansleep() error returns

[neil.armstrong]

On 7/24/26 04:46, kr494167@gmail.com wrote:
> From: Surendra Singh Chouhan <kr494167@gmail.com>
> 
> sgm3804_sync_regcache_state() evaluated !gpiod_get_value_cansleep(...) to
> check if the enable GPIOs were down.
> 
> gpiod_get_value_cansleep() returns 1 if active, 0 if inactive, and a
> negative error code (e.g. -EIO or -EINVAL) on failure. Evaluating
> !gpiod_get_value_cansleep(...) treats a negative error code as boolean
> false, swallowing GPIO read failures and misinterpreting a failed read as
> an active GPIO. This improperly attempts to sync regcache registers
> when the IC may be powered down.
> 
> Fix this by capturing the return values of gpiod_get_value_cansleep() for
> both positive and negative rails and propagating any error immediately.
> In addition, replace mutex_init() with devm_mutex_init() in probe().
> 
> Fixes: 0c47e1a8cf5d ("regulator: add SGM3804 Dual Output driver")
> Signed-off-by: Surendra Singh Chouhan <kr494167@gmail.com>
> ---
>   drivers/regulator/sgm3804-regulator.c | 17 ++++++++++++++---
>   1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/regulator/sgm3804-regulator.c b/drivers/regulator/sgm3804-regulator.c
> index c3406cfb73d0..a430c89b4d5d 100644
> --- a/drivers/regulator/sgm3804-regulator.c
> +++ b/drivers/regulator/sgm3804-regulator.c
> @@ -97,11 +97,20 @@ static const struct regmap_config sgm3804_regmap_config = {
>   
>   static int sgm3804_sync_regcache_state(struct sgm3804_data *ctx)
>   {
> +	int pos, neg;
> +
>   	guard(mutex)(&ctx->lock);
>   
> +	pos = gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]);
> +	if (pos < 0)
> +		return pos;
> +
> +	neg = gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL]);
> +	if (neg < 0)
> +		return neg;
> +
>   	/* If both GPIOs are down, IC is powered down and I2C writes will fail */
> -	if (!gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]) &&
> -	    !gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL])) {
> +	if (!pos && !neg) {
>   		regcache_cache_only(ctx->regmap, true);
>   		regcache_mark_dirty(ctx->regmap);
>   	} else {
> @@ -235,7 +244,9 @@ static int sgm3804_probe(struct i2c_client *i2c)
>   	if (!ctx)
>   		return -ENOMEM;
>   
> -	mutex_init(&ctx->lock);
> +	ret = devm_mutex_init(dev, &ctx->lock);
> +	if (ret)
> +		return ret;
>   
>   	ctx->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
>   	if (IS_ERR(ctx->regmap))

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH] regulator: sgm3804: handle gpiod_get_value_cansleep() error returns

[Mark Brown]

On Fri, 24 Jul 2026 08:16:25 +0530, kr494167@gmail.com wrote:
> regulator: sgm3804: handle gpiod_get_value_cansleep() error returns

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-7.5

Thanks!

[1/1] regulator: sgm3804: handle gpiod_get_value_cansleep() error returns
      https://git.kernel.org/broonie/regulator/c/cdfbeace852d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark