[PATCH] staging: iio: adc: ad7192: disable burnout currents on misconfig

[PATCH] staging: iio: adc: ad7192: disable burnout currents on misconfig

[alexandru.ardelean]

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

According to the datasheet for all ad719x ADCs,
the burnout currents can be enabled only if buffer is enabled
and CHOP is disabled.

So, if neither of these conditions are met, then
we should disable the burnout currents in the driver as well,
and warn the user.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---
 drivers/staging/iio/adc/ad7192.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d11c6de9c777..7f204013d6d4 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -266,6 +266,12 @@ static int ad7192_setup(struct ad7192_state *st,
 
 	st->conf = AD7192_CONF_GAIN(0);
 
+	if (pdata->burnout_curr_en && (!pdata->buf_en || pdata->chop_en)) {
+		pdata->burnout_curr_en = false;
+		dev_warn(dev,
+			 "Can't enable burnout currents: see CHOP or buffer\n");
+	}
+
 	if (pdata->rej60_en)
 		st->mode |= AD7192_MODE_REJ60;
 
-- 
2.14.1

Re: [PATCH] staging: iio: adc: ad7192: disable burnout currents on misconfig

[Jonathan Cameron]

On Wed, 10 Jan 2018 13:14:11 +0200
<alexandru.ardelean@analog.com> wrote:

> From: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> According to the datasheet for all ad719x ADCs,
> the burnout currents can be enabled only if buffer is enabled
> and CHOP is disabled.
> 
> So, if neither of these conditions are met, then
> we should disable the burnout currents in the driver as well,
> and warn the user.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>

What this patch is missing is a description of what the result of
getting this wrong is.  Does this break the device, cause fires
or just not work as expected?

We need this information to decided whether backporting to stable
is required or not.

Patch content is fine.

Jonathan

> ---
>  drivers/staging/iio/adc/ad7192.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index d11c6de9c777..7f204013d6d4 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -266,6 +266,12 @@ static int ad7192_setup(struct ad7192_state *st,
>  
>  	st->conf = AD7192_CONF_GAIN(0);
>  
> +	if (pdata->burnout_curr_en && (!pdata->buf_en || pdata->chop_en)) {
> +		pdata->burnout_curr_en = false;
> +		dev_warn(dev,
> +			 "Can't enable burnout currents: see CHOP or buffer\n");
> +	}
> +
>  	if (pdata->rej60_en)
>  		st->mode |= AD7192_MODE_REJ60;
>  

[PATCH v3] staging: iio: adc: ad7192: disable burnout currents on misconfig

[alexandru.ardelean]

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

The burnout currents can be enabled only if buffer is enabled
and CHOP is disabled.

So, if neither of these conditions are met, then
we should disable the burnout currents in the driver as well,
and warn the user.

This change doesn't fix anything.
The burnout currents simply won't work if CHOP is enabled
or buffer is disabled.
The intent is to provide the user with some feedback
instead of silently not working inside the chip.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changes v2 -> v3:
  * fixed "dev_warn(dev," ==> "dev_warn(&st->sd.spi->dev,"
    `dev` does not exist
  * moved the warning as part of an else case ; patch is
    simpler and `pdata` is const which fails

 drivers/staging/iio/adc/ad7192.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d11c6de9c777..aebbc3b58194 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -291,8 +291,12 @@ static int ad7192_setup(struct ad7192_state *st,
 	if (pdata->unipolar_en)
 		st->conf |= AD7192_CONF_UNIPOLAR;
 
-	if (pdata->burnout_curr_en)
+	if (pdata->burnout_curr_en && pdata->buf_en && !pdata->chop_en) {
 		st->conf |= AD7192_CONF_BURN;
+	} else {
+		dev_warn(&st->sd.spi->dev,
+			 "Can't enable burnout currents: see CHOP or buffer\n");
+	}
 
 	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
 	if (ret)
-- 
2.14.1

[PATCH v4] staging: iio: adc: ad7192: disable burnout currents on misconfig

[alexandru.ardelean]

From: Alexandru Ardelean <alexandru.ardelean@analog.com>

The burnout currents can be enabled only if buffer is enabled
and CHOP is disabled.

So, if neither of these conditions are met, then
we should disable the burnout currents in the driver as well,
and warn the user.

This change doesn't fix anything.
The burnout currents simply won't work if CHOP is enabled
or buffer is disabled.
The intent is to provide the user with some feedback
instead of silently not working inside the chip.

Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
---

Changes v3 -> v4:
 * need to check if `pdata->burnout_curr_en` is positive on the
   else path, otherwise a false warning may be thrown

 drivers/staging/iio/adc/ad7192.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
index d11c6de9c777..9287b50b7870 100644
--- a/drivers/staging/iio/adc/ad7192.c
+++ b/drivers/staging/iio/adc/ad7192.c
@@ -291,8 +291,12 @@ static int ad7192_setup(struct ad7192_state *st,
 	if (pdata->unipolar_en)
 		st->conf |= AD7192_CONF_UNIPOLAR;
 
-	if (pdata->burnout_curr_en)
+	if (pdata->burnout_curr_en && pdata->buf_en && !pdata->chop_en) {
 		st->conf |= AD7192_CONF_BURN;
+	} else if (pdata->burnout_curr_en) {
+		dev_warn(&st->sd.spi->dev,
+			 "Can't enable burnout currents: see CHOP or buffer\n");
+	}
 
 	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
 	if (ret)
-- 
2.14.1

Re: [PATCH v4] staging: iio: adc: ad7192: disable burnout currents on misconfig

[Jonathan Cameron]

On Thu, 18 Jan 2018 16:57:40 +0200
<alexandru.ardelean@analog.com> wrote:

> From: Alexandru Ardelean <alexandru.ardelean@analog.com>
> 
> The burnout currents can be enabled only if buffer is enabled
> and CHOP is disabled.
> 
> So, if neither of these conditions are met, then
> we should disable the burnout currents in the driver as well,
> and warn the user.
> 
> This change doesn't fix anything.
> The burnout currents simply won't work if CHOP is enabled
> or buffer is disabled.
> The intent is to provide the user with some feedback
> instead of silently not working inside the chip.
> 
> Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com>
Applied to the togreg branch of iio.git and pushed out as testing
for the autobuilders to play with it.

Thanks,

Jonathan

> ---
> 
> Changes v3 -> v4:
>  * need to check if `pdata->burnout_curr_en` is positive on the
>    else path, otherwise a false warning may be thrown
> 
>  drivers/staging/iio/adc/ad7192.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/iio/adc/ad7192.c b/drivers/staging/iio/adc/ad7192.c
> index d11c6de9c777..9287b50b7870 100644
> --- a/drivers/staging/iio/adc/ad7192.c
> +++ b/drivers/staging/iio/adc/ad7192.c
> @@ -291,8 +291,12 @@ static int ad7192_setup(struct ad7192_state *st,
>  	if (pdata->unipolar_en)
>  		st->conf |= AD7192_CONF_UNIPOLAR;
>  
> -	if (pdata->burnout_curr_en)
> +	if (pdata->burnout_curr_en && pdata->buf_en && !pdata->chop_en) {
>  		st->conf |= AD7192_CONF_BURN;
> +	} else if (pdata->burnout_curr_en) {
> +		dev_warn(&st->sd.spi->dev,
> +			 "Can't enable burnout currents: see CHOP or buffer\n");
> +	}
>  
>  	ret = ad_sd_write_reg(&st->sd, AD7192_REG_MODE, 3, st->mode);
>  	if (ret)