[PATCH v2] drivers: adc: fix return value if timeout occurs

[Michael Nazzareno Trimarchi]

Hi

On Tue, Mar 1, 2022 at 9:23 PM Francois Berder <fberder at outlook.fr> wrote:
>
> Because unsigned integers cannot be negative,
> timeout variable is never less than zero. Hence, checks
> in Amlogic Meson ADC driver to detect timeouts always
> evaluated to false. Fix that.
>
> Signed-off-by: Francois Berder <fberder at outlook.fr>
> ---
> Changes for v2:
>    - Replace timeout handling logic by regmap_read_poll_timeout
>
>  drivers/adc/meson-saradc.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/adc/meson-saradc.c b/drivers/adc/meson-saradc.c
> index 1a45a3a265..b36f09cdb1 100644
> --- a/drivers/adc/meson-saradc.c
> +++ b/drivers/adc/meson-saradc.c
> @@ -192,7 +192,8 @@ meson_saradc_get_fifo_count(struct meson_saradc_priv *priv)
>
>  static int meson_saradc_lock(struct meson_saradc_priv *priv)
>  {
> -       uint val, timeout = 10000;
> +       uint val;
> +       int ret;
>
>         /* prevent BL30 from using the SAR ADC while we are using it */
>         regmap_update_bits(priv->regmap, MESON_SAR_ADC_DELAY,
> @@ -202,17 +203,14 @@ static int meson_saradc_lock(struct meson_saradc_priv *priv)
>         /*
>          * wait until BL30 releases it's lock (so we can use the SAR ADC)
>          */
> -       do {
> -               udelay(1);
> -               regmap_read(priv->regmap, MESON_SAR_ADC_DELAY, &val);
> -       } while (val & MESON_SAR_ADC_DELAY_BL30_BUSY && timeout--);
> -
> -       if (timeout < 0) {
> +       ret =
> +           regmap_read_poll_timeout(priv->regmap, MESON_SAR_ADC_DELAY, val,
> +                                    val & MESON_SAR_ADC_DELAY_BL30_BUSY, 1,
> +                                    10);
> +       if (ret)
>                 printf("Timeout while waiting for BL30 unlock\n");
> -               return -ETIMEDOUT;
> -       }
>
> -       return 0;
> +       return ret;
>  }
>
>  static void meson_saradc_unlock(struct meson_saradc_priv *priv)
> @@ -246,22 +244,19 @@ static int meson_saradc_calib_val(struct meson_saradc_priv *priv, int val)
>
>  static int meson_saradc_wait_busy_clear(struct meson_saradc_priv *priv)
>  {
> -       uint regval, timeout = 10000;
> +       uint regval;
>
>         /*
>          * NOTE: we need a small delay before reading the status, otherwise
>          * the sample engine may not have started internally (which would
>          * seem to us that sampling is already finished).
>          */
> -       do {
> -               udelay(1);
> -               regmap_read(priv->regmap, MESON_SAR_ADC_REG0, &regval);
> -       } while (FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK, regval) && timeout--);
> +       udelay(1);
>
> -       if (timeout < 0)
> -               return -ETIMEDOUT;
> -
> -       return 0;
> +       return regmap_read_poll_timeout(priv->regmap, MESON_SAR_ADC_REG0,
> +                                       regval,
> +                                       FIELD_GET(MESON_SAR_ADC_REG0_BUSY_MASK,
> +                                                 regval), 1, 10);
>  }
>
>  static int meson_saradc_read_raw_sample(struct meson_saradc_priv *priv,

Please next time, include the author of this driver and CC who already
review it or suggest how improve it

Reviewed-by: Michael Trimarchi <michael at amarulasolutions.com>

Michael

> --
> 2.25.1
>


--
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael at amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info at amarulasolutions.com
www.amarulasolutions.com