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

Hi

On Tue, Mar 1, 2022 at 9:23 PM Francois Berder fberder@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@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@amarulasolutions.com

Michael

-- 2.25.1

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

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