[PATCH] iio: humidity: hdc100x: Add margin to the conversion time

[PATCH] iio: humidity: hdc100x: Add margin to the conversion time

[Chris Lesiak]

The datasheets have the following note for the conversion time
specification: "This parameter is specified by design and/or
characterization and it is not tested in production."

Parts have been seen that require more time to do 14-bit conversions for
the relative humidity channel.  The result is ENXIO due to the address
phase of a transfer not getting an ACK.

Delay an additional 1 ms per conversion to allow for additional margin.

Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
---
 drivers/iio/humidity/hdc100x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 2a957f19048e..91790aa8beeb 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -166,7 +166,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
 				   struct iio_chan_spec const *chan)
 {
 	struct i2c_client *client = data->client;
-	int delay = data->adc_int_us[chan->address];
+	int delay = data->adc_int_us[chan->address] + 1000;
 	int ret;
 	__be16 val;
 
@@ -316,7 +316,7 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct hdc100x_data *data = iio_priv(indio_dev);
 	struct i2c_client *client = data->client;
-	int delay = data->adc_int_us[0] + data->adc_int_us[1];
+	int delay = data->adc_int_us[0] + data->adc_int_us[1] + 2000;
 	int ret;
 
 	/* dual read starts at temp register */
-- 
2.26.2

Re: [PATCH] iio: humidity: hdc100x: Add margin to the conversion time

[Jonathan Cameron]

On Wed,  9 Jun 2021 14:37:48 -0500
Chris Lesiak <chris.lesiak@licor.com> wrote:

> The datasheets have the following note for the conversion time
> specification: "This parameter is specified by design and/or
> characterization and it is not tested in production."
> 
> Parts have been seen that require more time to do 14-bit conversions for
> the relative humidity channel.  The result is ENXIO due to the address
> phase of a transfer not getting an ACK.
> 
> Delay an additional 1 ms per conversion to allow for additional margin.
> 
> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>

Hi Chris

Could you figure out a fixes tag for this one so we can get it backported
into stable?

If not I can probably guess when I catch up with applying patches.

Thanks,

Jonathan

> ---
>  drivers/iio/humidity/hdc100x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> index 2a957f19048e..91790aa8beeb 100644
> --- a/drivers/iio/humidity/hdc100x.c
> +++ b/drivers/iio/humidity/hdc100x.c
> @@ -166,7 +166,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
>  				   struct iio_chan_spec const *chan)
>  {
>  	struct i2c_client *client = data->client;
> -	int delay = data->adc_int_us[chan->address];
> +	int delay = data->adc_int_us[chan->address] + 1000;
>  	int ret;
>  	__be16 val;
>  
> @@ -316,7 +316,7 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p)
>  	struct iio_dev *indio_dev = pf->indio_dev;
>  	struct hdc100x_data *data = iio_priv(indio_dev);
>  	struct i2c_client *client = data->client;
> -	int delay = data->adc_int_us[0] + data->adc_int_us[1];
> +	int delay = data->adc_int_us[0] + data->adc_int_us[1] + 2000;
>  	int ret;
>  
>  	/* dual read starts at temp register */

[PATCH v2] iio: humidity: hdc100x: Add margin to the conversion time

[Chris Lesiak]

The datasheets have the following note for the conversion time
specification: "This parameter is specified by design and/or
characterization and it is not tested in production."

Parts have been seen that require more time to do 14-bit conversions for
the relative humidity channel.  The result is ENXIO due to the address
phase of a transfer not getting an ACK.

Delay an additional 1 ms per conversion to allow for additional margin.

Fixes: 4839367d99e3 ("iio: humidity: add HDC100x support")
Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
---
 drivers/iio/humidity/hdc100x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index 2a957f19048e..91790aa8beeb 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -166,7 +166,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
 				   struct iio_chan_spec const *chan)
 {
 	struct i2c_client *client = data->client;
-	int delay = data->adc_int_us[chan->address];
+	int delay = data->adc_int_us[chan->address] + 1000;
 	int ret;
 	__be16 val;
 
@@ -316,7 +316,7 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p)
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct hdc100x_data *data = iio_priv(indio_dev);
 	struct i2c_client *client = data->client;
-	int delay = data->adc_int_us[0] + data->adc_int_us[1];
+	int delay = data->adc_int_us[0] + data->adc_int_us[1] + 2000;
 	int ret;
 
 	/* dual read starts at temp register */
-- 
2.26.2

Re: [PATCH v2] iio: humidity: hdc100x: Add margin to the conversion time

[Andy Shevchenko]

On Thu, Jun 10, 2021 at 4:47 PM Chris Lesiak <chris.lesiak@licor.com> wrote:
>
> The datasheets have the following note for the conversion time
> specification: "This parameter is specified by design and/or
> characterization and it is not tested in production."
>
> Parts have been seen that require more time to do 14-bit conversions for
> the relative humidity channel.  The result is ENXIO due to the address
> phase of a transfer not getting an ACK.
>
> Delay an additional 1 ms per conversion to allow for additional margin.

This is crucial and not so visible in the code, see below.

> Fixes: 4839367d99e3 ("iio: humidity: add HDC100x support")
> Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
> ---
>  drivers/iio/humidity/hdc100x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> index 2a957f19048e..91790aa8beeb 100644
> --- a/drivers/iio/humidity/hdc100x.c
> +++ b/drivers/iio/humidity/hdc100x.c
> @@ -166,7 +166,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
>                                    struct iio_chan_spec const *chan)
>  {
>         struct i2c_client *client = data->client;
> -       int delay = data->adc_int_us[chan->address];
> +       int delay = data->adc_int_us[chan->address] + 1000;

+ 1 * USEC_PER_MSEC;

>         int ret;
>         __be16 val;
>
> @@ -316,7 +316,7 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p)
>         struct iio_dev *indio_dev = pf->indio_dev;
>         struct hdc100x_data *data = iio_priv(indio_dev);
>         struct i2c_client *client = data->client;
> -       int delay = data->adc_int_us[0] + data->adc_int_us[1];
> +       int delay = data->adc_int_us[0] + data->adc_int_us[1] + 2000;

+ 2 * USEC_PER_MSEC

>         int ret;
>
>         /* dual read starts at temp register */

Above might require to add #include <vdso/time64.h>.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2] iio: humidity: hdc100x: Add margin to the conversion time

[Jonathan Cameron]

On Thu, 10 Jun 2021 16:55:20 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Thu, Jun 10, 2021 at 4:47 PM Chris Lesiak <chris.lesiak@licor.com> wrote:
> >
> > The datasheets have the following note for the conversion time
> > specification: "This parameter is specified by design and/or
> > characterization and it is not tested in production."
> >
> > Parts have been seen that require more time to do 14-bit conversions for
> > the relative humidity channel.  The result is ENXIO due to the address
> > phase of a transfer not getting an ACK.
> >
> > Delay an additional 1 ms per conversion to allow for additional margin.  
> 
> This is crucial and not so visible in the code, see below.
> 
> > Fixes: 4839367d99e3 ("iio: humidity: add HDC100x support")
> > Signed-off-by: Chris Lesiak <chris.lesiak@licor.com>
Small process note.

For IIO at least, please don't send new versions of patches as replies
to earlier series.  Whilst it doesn't matter that much here, for large
series it can get very confusing and runs the risk of be picking up
the wrong patch versions.

A new thread is much easier to deal with.

Jonathan

> > ---
> >  drivers/iio/humidity/hdc100x.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> > index 2a957f19048e..91790aa8beeb 100644
> > --- a/drivers/iio/humidity/hdc100x.c
> > +++ b/drivers/iio/humidity/hdc100x.c
> > @@ -166,7 +166,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
> >                                    struct iio_chan_spec const *chan)
> >  {
> >         struct i2c_client *client = data->client;
> > -       int delay = data->adc_int_us[chan->address];
> > +       int delay = data->adc_int_us[chan->address] + 1000;  
> 
> + 1 * USEC_PER_MSEC;
> 
> >         int ret;
> >         __be16 val;
> >
> > @@ -316,7 +316,7 @@ static irqreturn_t hdc100x_trigger_handler(int irq, void *p)
> >         struct iio_dev *indio_dev = pf->indio_dev;
> >         struct hdc100x_data *data = iio_priv(indio_dev);
> >         struct i2c_client *client = data->client;
> > -       int delay = data->adc_int_us[0] + data->adc_int_us[1];
> > +       int delay = data->adc_int_us[0] + data->adc_int_us[1] + 2000;  
> 
> + 2 * USEC_PER_MSEC
> 
> >         int ret;
> >
> >         /* dual read starts at temp register */  
> 
> Above might require to add #include <vdso/time64.h>.
>