Re: [PATCH] iio: afe: rescale: Fix logic bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi!

2022-05-24 at 09:54, Linus Walleij wrote:
> When introducing support for processed channels I needed
> to invert the expression:
> 
>   if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
>       !iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE))
>         dev_err(dev, "source channel does not support raw/scale\n");
> 
> To the inverse, meaning detect when we can usse raw+scale

Nit: use

> rather than when we can not. This was the result:
> 
>   if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
>       iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE))
>        dev_info(dev, "using raw+scale source channel\n");
> 
> Ooops. Spot the error. Yep old George Boole came up and bit me.
> That should be an &&.

Good catch!

> The current code "mostly works" because we have not run into
> systems supporting only raw but not scale or only scale but not
> raw, and I doubt there are few using the rescaler on anything
> such, but let's fix the logic.

Scaling something that provides raw but not scale *could* have
been implemented by assuming an upstream scale of 1, but it is
not. Instead you get an error in that case and thus no scale at
all. While that is perhaps not wrong, it is also a pretty
useless situation.

Scaling something as if there are raw values when that something
only provides scale but not raw also seems pretty useless.

So, you have my

Acked-by: Peter Rosin <peda@xxxxxxxxxx>

Cheers,
Peter

> 
> Cc: Liam Beguin <liambeguin@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 53ebee949980 ("iio: afe: iio-rescale: Support processed channels")
> Signed-off-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> ---
>  drivers/iio/afe/iio-rescale.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index 7e511293d6d1..dc426e1484f0 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -278,7 +278,7 @@ static int rescale_configure_channel(struct device *dev,
>  	chan->ext_info = rescale->ext_info;
>  	chan->type = rescale->cfg->type;
>  
> -	if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
> +	if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) &&
>  	    iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) {
>  		dev_info(dev, "using raw+scale source channel\n");
>  	} else if (iio_channel_has_info(schan, IIO_CHAN_INFO_PROCESSED)) {



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux