GPIO state was inadvertently overwritten by the result of sip_sync,
reuniting in ti_ads7950_get() only returning 0 as gpio state (or error).

Fix this by introducing a separate variable to hold the state.

Reported-by: David Lechner <dlechner@baylibre.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/iio/adc/ti-ads7950.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
index b8cc39fc39fb..2a7d4a1d9fa9 100644
--- a/drivers/iio/adc/ti-ads7950.c
+++ b/drivers/iio/adc/ti-ads7950.c
@@ -427,13 +427,14 @@ static int ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
 static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 {
 	struct ti_ads7950_state *st = gpiochip_get_data(chip);
-	int ret;
+	int ret = 0;
+	bool state;
 
 	mutex_lock(&st->slock);
 
 	/* If set as output, return the output */
 	if (st->gpio_cmd_settings_bitmask & BIT(offset)) {
-		ret = (st->cmd_settings_bitmask & BIT(offset)) ? 1 : 0;
+		state = st->cmd_settings_bitmask & BIT(offset);
 		goto out;
 	}
 
@@ -444,7 +445,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 	if (ret)
 		goto out;
 
-	ret = ((st->single_rx >> 12) & BIT(offset)) ? 1 : 0;
+	state = (st->single_rx >> 12) & BIT(offset);
 
 	/* Revert back to original settings */
 	st->cmd_settings_bitmask &= ~TI_ADS7950_CR_GPIO_DATA;
@@ -456,7 +457,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
 out:
 	mutex_unlock(&st->slock);
 
-	return ret;
+	return ret ?: state;
 }
 
 static int ti_ads7950_get_direction(struct gpio_chip *chip,
-- 
2.53.0.335.g19a08e0c02-goog

On 2/18/26 8:29 PM, Dmitry Torokhov wrote:
> GPIO state was inadvertently overwritten by the result of sip_sync,
> reuniting in ti_ads7950_get() only returning 0 as gpio state (or error).
> 
> Fix this by introducing a separate variable to hold the state.
> 
> Reported-by: David Lechner <dlechner@baylibre.com>

This should have a Fixes: tag since it is fixing a real bug.

Also, fixes should come first in the series.

> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
>  drivers/iio/adc/ti-ads7950.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads7950.c b/drivers/iio/adc/ti-ads7950.c
> index b8cc39fc39fb..2a7d4a1d9fa9 100644
> --- a/drivers/iio/adc/ti-ads7950.c
> +++ b/drivers/iio/adc/ti-ads7950.c
> @@ -427,13 +427,14 @@ static int ti_ads7950_set(struct gpio_chip *chip, unsigned int offset,
>  static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
>  {
>  	struct ti_ads7950_state *st = gpiochip_get_data(chip);
> -	int ret;
> +	int ret = 0;
> +	bool state;
>  
>  	mutex_lock(&st->slock);
>  
>  	/* If set as output, return the output */
>  	if (st->gpio_cmd_settings_bitmask & BIT(offset)) {
> -		ret = (st->cmd_settings_bitmask & BIT(offset)) ? 1 : 0;
> +		state = st->cmd_settings_bitmask & BIT(offset);

I agree it would be better to put...

		ret = 0;

here.

>  		goto out;
>  	}
>  
> @@ -444,7 +445,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
>  	if (ret)
>  		goto out;
>  
> -	ret = ((st->single_rx >> 12) & BIT(offset)) ? 1 : 0;
> +	state = (st->single_rx >> 12) & BIT(offset);
>  
>  	/* Revert back to original settings */
>  	st->cmd_settings_bitmask &= ~TI_ADS7950_CR_GPIO_DATA;
> @@ -456,7 +457,7 @@ static int ti_ads7950_get(struct gpio_chip *chip, unsigned int offset)
>  out:
>  	mutex_unlock(&st->slock);
>  
> -	return ret;
> +	return ret ?: state;>  }
>  
>  static int ti_ads7950_get_direction(struct gpio_chip *chip,

On Wed, Feb 18, 2026 at 06:29:26PM -0800, Dmitry Torokhov wrote:
> GPIO state was inadvertently overwritten by the result of sip_sync,

sip?

> reuniting in ti_ads7950_get() only returning 0 as gpio state (or error).

GPIO

> Fix this by introducing a separate variable to hold the state.

...

> -	int ret;
> +	int ret = 0;

This kind of assignment is harder to maintain, because it leaves a room for
subtle mistakes (when it's reused by a newly injected code, quite similar,
actually, to what this change tries to address). Please, decouple definition
and assignment.

> +	bool state;

Not sure about this (yes, I know it was suggested). I would leave it still
the same type as one that is returned by the function or a compatible one
that is the same as st->* (if the used ones are all of the same type).

-- 
With Best Regards,
Andy Shevchenko