All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] iio: humidity: hdc100x: fix sensor data reads of temp and humidity
@ 2016-08-08 18:14 Alison Schofield
  2016-08-15 16:27 ` Jonathan Cameron
  0 siblings, 1 reply; 3+ messages in thread
From: Alison Schofield @ 2016-08-08 18:14 UTC (permalink / raw)
  To: jic23; +Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel

Replace the i2c_smbus_read_byte commmands used to retrieve the sensor
data with an i2c_master_recv command.

The smbus read byte method fails because the device does not expect a
stop condition after sending the first byte. When we issue the second
read, we are getting the first byte again. Net effect is that of the 14
bits used for the measurement, the 8 most significant bits are correct,
the lower 6 are not.

None of the smbus read protocols follow the pattern this device requires
(S Addr Rd [A] Data [A] Data NA P), hence the switch to an i2c receive
transaction.

Signed-off-by: Alison Schofield <amsfield22@gmail.com>
Cc: Daniel Baluta <daniel.baluta@gmail.com>
---
Changes in v2:
  update commit message to say what this fixes
  use endian type & macro to correctly manage byte order
  use sizeof for data length

 drivers/iio/humidity/hdc100x.c | 27 +++++++--------------------
 1 file changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
index a03832a..e0c9c70 100644
--- a/drivers/iio/humidity/hdc100x.c
+++ b/drivers/iio/humidity/hdc100x.c
@@ -142,7 +142,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
 	struct i2c_client *client = data->client;
 	int delay = data->adc_int_us[chan->address];
 	int ret;
-	int val;
+	__be16 val;
 
 	/* start measurement */
 	ret = i2c_smbus_write_byte(client, chan->address);
@@ -154,26 +154,13 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
 	/* wait for integration time to pass */
 	usleep_range(delay, delay + 1000);
 
-	/*
-	 * i2c_smbus_read_word_data cannot() be used here due to the command
-	 * value not being understood and causes NAKs preventing any reading
-	 * from being accessed.
-	 */
-	ret = i2c_smbus_read_byte(client);
+	/* read measurement */
+	ret = i2c_master_recv(data->client, (char *)&val, sizeof(val));
 	if (ret < 0) {
-		dev_err(&client->dev, "cannot read high byte measurement");
+		dev_err(&client->dev, "cannot read sensor data\n");
 		return ret;
 	}
-	val = ret << 8;
-
-	ret = i2c_smbus_read_byte(client);
-	if (ret < 0) {
-		dev_err(&client->dev, "cannot read low byte measurement");
-		return ret;
-	}
-	val |= ret;
-
-	return val;
+	return be16_to_cpu(val);
 }
 
 static int hdc100x_get_heater_status(struct hdc100x_data *data)
@@ -272,8 +259,8 @@ static int hdc100x_probe(struct i2c_client *client,
 	struct iio_dev *indio_dev;
 	struct hdc100x_data *data;
 
-	if (!i2c_check_functionality(client->adapter,
-				I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BYTE))
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
+				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))
 		return -EOPNOTSUPP;
 
 	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: humidity: hdc100x: fix sensor data reads of temp and humidity
  2016-08-08 18:14 [PATCH v2] iio: humidity: hdc100x: fix sensor data reads of temp and humidity Alison Schofield
@ 2016-08-15 16:27 ` Jonathan Cameron
  2016-08-17 17:45   ` Alison Schofield
  0 siblings, 1 reply; 3+ messages in thread
From: Jonathan Cameron @ 2016-08-15 16:27 UTC (permalink / raw)
  To: Alison Schofield
  Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel

On 08/08/16 19:14, Alison Schofield wrote:
> Replace the i2c_smbus_read_byte commmands used to retrieve the sensor
> data with an i2c_master_recv command.
> 
> The smbus read byte method fails because the device does not expect a
> stop condition after sending the first byte. When we issue the second
> read, we are getting the first byte again. Net effect is that of the 14
> bits used for the measurement, the 8 most significant bits are correct,
> the lower 6 are not.
> 
> None of the smbus read protocols follow the pattern this device requires
> (S Addr Rd [A] Data [A] Data NA P), hence the switch to an i2c receive
> transaction.
> 
> Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> Cc: Daniel Baluta <daniel.baluta@gmail.com>
Hi Alison,

Nice fix.  Note that I'd have liked a fixes: tag for this one...
It's a bit fiddly as will need backporting to older stable kernels.

I'll leave it marked for stable but with no version. As long as
it applies cleanly it should be fine.

Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan

> ---
> Changes in v2:
>   update commit message to say what this fixes
>   use endian type & macro to correctly manage byte order
>   use sizeof for data length
> 
>  drivers/iio/humidity/hdc100x.c | 27 +++++++--------------------
>  1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> index a03832a..e0c9c70 100644
> --- a/drivers/iio/humidity/hdc100x.c
> +++ b/drivers/iio/humidity/hdc100x.c
> @@ -142,7 +142,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
>  	struct i2c_client *client = data->client;
>  	int delay = data->adc_int_us[chan->address];
>  	int ret;
> -	int val;
> +	__be16 val;
>  
>  	/* start measurement */
>  	ret = i2c_smbus_write_byte(client, chan->address);
> @@ -154,26 +154,13 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
>  	/* wait for integration time to pass */
>  	usleep_range(delay, delay + 1000);
>  
> -	/*
> -	 * i2c_smbus_read_word_data cannot() be used here due to the command
> -	 * value not being understood and causes NAKs preventing any reading
> -	 * from being accessed.
> -	 */
> -	ret = i2c_smbus_read_byte(client);
> +	/* read measurement */
> +	ret = i2c_master_recv(data->client, (char *)&val, sizeof(val));
>  	if (ret < 0) {
> -		dev_err(&client->dev, "cannot read high byte measurement");
> +		dev_err(&client->dev, "cannot read sensor data\n");
>  		return ret;
>  	}
> -	val = ret << 8;
> -
> -	ret = i2c_smbus_read_byte(client);
> -	if (ret < 0) {
> -		dev_err(&client->dev, "cannot read low byte measurement");
> -		return ret;
> -	}
> -	val |= ret;
> -
> -	return val;
> +	return be16_to_cpu(val);
>  }
>  
>  static int hdc100x_get_heater_status(struct hdc100x_data *data)
> @@ -272,8 +259,8 @@ static int hdc100x_probe(struct i2c_client *client,
>  	struct iio_dev *indio_dev;
>  	struct hdc100x_data *data;
>  
> -	if (!i2c_check_functionality(client->adapter,
> -				I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BYTE))
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> +				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))
>  		return -EOPNOTSUPP;
>  
>  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH v2] iio: humidity: hdc100x: fix sensor data reads of temp and humidity
  2016-08-15 16:27 ` Jonathan Cameron
@ 2016-08-17 17:45   ` Alison Schofield
  0 siblings, 0 replies; 3+ messages in thread
From: Alison Schofield @ 2016-08-17 17:45 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: mranostay, knaack.h, lars, pmeerw, linux-iio, linux-kernel

On Mon, Aug 15, 2016 at 05:27:00PM +0100, Jonathan Cameron wrote:
> On 08/08/16 19:14, Alison Schofield wrote:
> > Replace the i2c_smbus_read_byte commmands used to retrieve the sensor
> > data with an i2c_master_recv command.
> > 
> > The smbus read byte method fails because the device does not expect a
> > stop condition after sending the first byte. When we issue the second
> > read, we are getting the first byte again. Net effect is that of the 14
> > bits used for the measurement, the 8 most significant bits are correct,
> > the lower 6 are not.
> > 
> > None of the smbus read protocols follow the pattern this device requires
> > (S Addr Rd [A] Data [A] Data NA P), hence the switch to an i2c receive
> > transaction.
> > 
> > Signed-off-by: Alison Schofield <amsfield22@gmail.com>
> > Cc: Daniel Baluta <daniel.baluta@gmail.com>
> Hi Alison,
> 
> Nice fix.  Note that I'd have liked a fixes: tag for this one...
> It's a bit fiddly as will need backporting to older stable kernels.
> 
> I'll leave it marked for stable but with no version. As long as
> it applies cleanly it should be fine.
> 
> Applied to the fixes-togreg branch of iio.git.
> 
> Thanks,
> 
> Jonathan

Here's the fixes tag, if you can still insert it...

	Fixes: 4839367d99e3 ("iio: humidity: add HDC100x support")

alisons

> 
> > ---
> > Changes in v2:
> >   update commit message to say what this fixes
> >   use endian type & macro to correctly manage byte order
> >   use sizeof for data length
> > 
> >  drivers/iio/humidity/hdc100x.c | 27 +++++++--------------------
> >  1 file changed, 7 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/iio/humidity/hdc100x.c b/drivers/iio/humidity/hdc100x.c
> > index a03832a..e0c9c70 100644
> > --- a/drivers/iio/humidity/hdc100x.c
> > +++ b/drivers/iio/humidity/hdc100x.c
> > @@ -142,7 +142,7 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
> >  	struct i2c_client *client = data->client;
> >  	int delay = data->adc_int_us[chan->address];
> >  	int ret;
> > -	int val;
> > +	__be16 val;
> >  
> >  	/* start measurement */
> >  	ret = i2c_smbus_write_byte(client, chan->address);
> > @@ -154,26 +154,13 @@ static int hdc100x_get_measurement(struct hdc100x_data *data,
> >  	/* wait for integration time to pass */
> >  	usleep_range(delay, delay + 1000);
> >  
> > -	/*
> > -	 * i2c_smbus_read_word_data cannot() be used here due to the command
> > -	 * value not being understood and causes NAKs preventing any reading
> > -	 * from being accessed.
> > -	 */
> > -	ret = i2c_smbus_read_byte(client);
> > +	/* read measurement */
> > +	ret = i2c_master_recv(data->client, (char *)&val, sizeof(val));
> >  	if (ret < 0) {
> > -		dev_err(&client->dev, "cannot read high byte measurement");
> > +		dev_err(&client->dev, "cannot read sensor data\n");
> >  		return ret;
> >  	}
> > -	val = ret << 8;
> > -
> > -	ret = i2c_smbus_read_byte(client);
> > -	if (ret < 0) {
> > -		dev_err(&client->dev, "cannot read low byte measurement");
> > -		return ret;
> > -	}
> > -	val |= ret;
> > -
> > -	return val;
> > +	return be16_to_cpu(val);
> >  }
> >  
> >  static int hdc100x_get_heater_status(struct hdc100x_data *data)
> > @@ -272,8 +259,8 @@ static int hdc100x_probe(struct i2c_client *client,
> >  	struct iio_dev *indio_dev;
> >  	struct hdc100x_data *data;
> >  
> > -	if (!i2c_check_functionality(client->adapter,
> > -				I2C_FUNC_SMBUS_WORD_DATA | I2C_FUNC_SMBUS_BYTE))
> > +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_WORD_DATA |
> > +				     I2C_FUNC_SMBUS_BYTE | I2C_FUNC_I2C))
> >  		return -EOPNOTSUPP;
> >  
> >  	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> > 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-08-17 17:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-08 18:14 [PATCH v2] iio: humidity: hdc100x: fix sensor data reads of temp and humidity Alison Schofield
2016-08-15 16:27 ` Jonathan Cameron
2016-08-17 17:45   ` Alison Schofield

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.