| Message ID | 20250414140901.460719-1-gshahrouzi@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: Correct conditional logic for store mode | expand |
On 4/14/25 9:09 AM, Gabriel Shahrouzi wrote: > The mode setting logic in ad7816_store_mode was reversed due to > incorrect handling of the strcmp return value. strcmp returns 0 on > match, so the `if (strcmp(buf, "full"))` block executed when the > input was not "full". > > This resulted in "full" setting the mode to AD7816_PD (power-down) and > other inputs setting it to AD7816_FULL. > > Fix this by checking it against 0 to correctly check for "full" and > "power-down", mapping them to AD7816_FULL and AD7816_PD respectively. > Sounds like we need a Fixes: tag here that reference the commit that introduced the bug. > Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com> > ---
On 4/14/25 9:59 AM, David Lechner wrote: > On 4/14/25 9:09 AM, Gabriel Shahrouzi wrote: >> The mode setting logic in ad7816_store_mode was reversed due to >> incorrect handling of the strcmp return value. strcmp returns 0 on >> match, so the `if (strcmp(buf, "full"))` block executed when the >> input was not "full". >> >> This resulted in "full" setting the mode to AD7816_PD (power-down) and >> other inputs setting it to AD7816_FULL. >> >> Fix this by checking it against 0 to correctly check for "full" and >> "power-down", mapping them to AD7816_FULL and AD7816_PD respectively. >> > > Sounds like we need a Fixes: tag here that reference the commit > that introduced the bug. > >> Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com> >> --- There is also a typo in your email address in the cc: gshahrozui@gmail.com
On Mon, Apr 14, 2025 at 11:02 AM David Lechner <dlechner@baylibre.com> wrote: > > On 4/14/25 9:59 AM, David Lechner wrote: > > On 4/14/25 9:09 AM, Gabriel Shahrouzi wrote: > >> The mode setting logic in ad7816_store_mode was reversed due to > >> incorrect handling of the strcmp return value. strcmp returns 0 on > >> match, so the `if (strcmp(buf, "full"))` block executed when the > >> input was not "full". > >> > >> This resulted in "full" setting the mode to AD7816_PD (power-down) and > >> other inputs setting it to AD7816_FULL. > >> > >> Fix this by checking it against 0 to correctly check for "full" and > >> "power-down", mapping them to AD7816_FULL and AD7816_PD respectively. > >> > > > > Sounds like we need a Fixes: tag here that reference the commit > > that introduced the bug. Sounds good. Included the fixes tag and CC'd stable@vger.kernel.org. > > > >> Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com> > >> --- > > There is also a typo in your email address in the cc: gshahrozui@gmail.com Whoops - I'll update that. I just realized I didn't update the version for the second patch correctly so I'll send in a new one.
diff --git a/drivers/staging/iio/adc/ad7816.c b/drivers/staging/iio/adc/ad7816.c index 6c14d7bcdd675..6b545547660dd 100644 --- a/drivers/staging/iio/adc/ad7816.c +++ b/drivers/staging/iio/adc/ad7816.c @@ -136,7 +136,7 @@ static ssize_t ad7816_store_mode(struct device *dev, struct iio_dev *indio_dev = dev_to_iio_dev(dev); struct ad7816_chip_info *chip = iio_priv(indio_dev); - if (strcmp(buf, "full")) { + if (sysfs_streq(buf, "full")) { gpiod_set_value(chip->rdwr_pin, 1); chip->mode = AD7816_FULL; } else {
The mode setting logic in ad7816_store_mode was reversed due to incorrect handling of the strcmp return value. strcmp returns 0 on match, so the `if (strcmp(buf, "full"))` block executed when the input was not "full". This resulted in "full" setting the mode to AD7816_PD (power-down) and other inputs setting it to AD7816_FULL. Fix this by checking it against 0 to correctly check for "full" and "power-down", mapping them to AD7816_FULL and AD7816_PD respectively. Signed-off-by: Gabriel Shahrouzi <gshahrouzi@gmail.com> --- drivers/staging/iio/adc/ad7816.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)