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

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, ®val); - } 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,

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, ®val);
} 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
participants (2)
-
Francois Berder
-
Michael Nazzareno Trimarchi