* [PATCH] hwmon: sht4x: Fix sht4x_read_values return value
@ 2021-05-29 9:13 Joe Perches
2021-06-02 11:57 ` Guenter Roeck
0 siblings, 1 reply; 2+ messages in thread
From: Joe Perches @ 2021-05-29 9:13 UTC (permalink / raw)
To: Jean Delvare, Guenter Roeck; +Cc: Navin Sankar Velliangiri, linux-hwmon, LKML
Kernel doc for sht4x_read_values() shows 0 on success, 1 on failure but
the return value on success is actually always positive as it is set to
SHT4X_RESPONSE_LENGTH by a successful call to i2c_master_recv().
Miscellanea:
o Update the kernel doc for sht4x_read_values to 0 for success or -ERRNO
o Remove incorrectly used kernel doc /** header for other _read functions
o Typo fix succesfull->successful
o Reverse a test to unindent a block and use goto unlock
o Declare cmd[SHT4X_CMD_LEN] rather than cmd[]
At least for gcc 10.2, object size is reduced a tiny bit.
$ size drivers/hwmon/sht4x.o*
text data bss dec hex filename
1752 404 256 2412 96c drivers/hwmon/sht4x.o.new
1825 404 256 2485 9b5 drivers/hwmon/sht4x.o.old
Signed-off-by: Joe Perches <joe@perches.com>
---
compiled, untested, no hardware
drivers/hwmon/sht4x.c | 95 ++++++++++++++++++++++++---------------------------
1 file changed, 45 insertions(+), 50 deletions(-)
diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
index 1dc51ee2a72ba..09c2a0b064444 100644
--- a/drivers/hwmon/sht4x.c
+++ b/drivers/hwmon/sht4x.c
@@ -67,7 +67,7 @@ struct sht4x_data {
/**
* sht4x_read_values() - read and parse the raw data from the SHT4X
* @sht4x_data: the struct sht4x_data to use for the lock
- * Return: 0 if succesfull, 1 if not
+ * Return: 0 if successful, -ERRNO if not
*/
static int sht4x_read_values(struct sht4x_data *data)
{
@@ -75,51 +75,53 @@ static int sht4x_read_values(struct sht4x_data *data)
u16 t_ticks, rh_ticks;
unsigned long next_update;
struct i2c_client *client = data->client;
- u8 crc, raw_data[SHT4X_RESPONSE_LENGTH],
- cmd[] = {SHT4X_CMD_MEASURE_HPM};
+ u8 crc;
+ u8 cmd[SHT4X_CMD_LEN] = {SHT4X_CMD_MEASURE_HPM};
+ u8 raw_data[SHT4X_RESPONSE_LENGTH];
mutex_lock(&data->lock);
next_update = data->last_updated +
msecs_to_jiffies(data->update_interval);
- if (!data->valid || time_after(jiffies, next_update)) {
- ret = i2c_master_send(client, cmd, SHT4X_CMD_LEN);
- if (ret < 0)
- goto unlock;
-
- usleep_range(SHT4X_MEAS_DELAY,
- SHT4X_MEAS_DELAY + SHT4X_DELAY_EXTRA);
-
- ret = i2c_master_recv(client, raw_data, SHT4X_RESPONSE_LENGTH);
- if (ret != SHT4X_RESPONSE_LENGTH) {
- if (ret >= 0)
- ret = -ENODATA;
-
- goto unlock;
- }
-
- t_ticks = raw_data[0] << 8 | raw_data[1];
- rh_ticks = raw_data[3] << 8 | raw_data[4];
-
- crc = crc8(sht4x_crc8_table, &raw_data[0], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
- if (crc != raw_data[2]) {
- dev_err(&client->dev, "data integrity check failed\n");
- ret = -EIO;
- goto unlock;
- }
-
- crc = crc8(sht4x_crc8_table, &raw_data[3], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
- if (crc != raw_data[5]) {
- dev_err(&client->dev, "data integrity check failed\n");
- ret = -EIO;
- goto unlock;
- }
-
- data->temperature = ((21875 * (int32_t)t_ticks) >> 13) - 45000;
- data->humidity = ((15625 * (int32_t)rh_ticks) >> 13) - 6000;
- data->last_updated = jiffies;
- data->valid = true;
+
+ if (data->valid && time_before_eq(jiffies, next_update))
+ goto unlock;
+
+ ret = i2c_master_send(client, cmd, SHT4X_CMD_LEN);
+ if (ret < 0)
+ goto unlock;
+
+ usleep_range(SHT4X_MEAS_DELAY, SHT4X_MEAS_DELAY + SHT4X_DELAY_EXTRA);
+
+ ret = i2c_master_recv(client, raw_data, SHT4X_RESPONSE_LENGTH);
+ if (ret != SHT4X_RESPONSE_LENGTH) {
+ if (ret >= 0)
+ ret = -ENODATA;
+ goto unlock;
+ }
+
+ t_ticks = raw_data[0] << 8 | raw_data[1];
+ rh_ticks = raw_data[3] << 8 | raw_data[4];
+
+ crc = crc8(sht4x_crc8_table, &raw_data[0], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
+ if (crc != raw_data[2]) {
+ dev_err(&client->dev, "data integrity check failed\n");
+ ret = -EIO;
+ goto unlock;
}
+ crc = crc8(sht4x_crc8_table, &raw_data[3], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
+ if (crc != raw_data[5]) {
+ dev_err(&client->dev, "data integrity check failed\n");
+ ret = -EIO;
+ goto unlock;
+ }
+
+ data->temperature = ((21875 * (int32_t)t_ticks) >> 13) - 45000;
+ data->humidity = ((15625 * (int32_t)rh_ticks) >> 13) - 6000;
+ data->last_updated = jiffies;
+ data->valid = true;
+ ret = 0;
+
unlock:
mutex_unlock(&data->lock);
return ret;
@@ -132,19 +134,14 @@ static ssize_t sht4x_interval_write(struct sht4x_data *data, long val)
return 0;
}
-/**
- * sht4x_interval_read() - read the minimum poll interval
- * in milliseconds
- */
+/* sht4x_interval_read() - read the minimum poll interval in milliseconds */
static size_t sht4x_interval_read(struct sht4x_data *data, long *val)
{
*val = data->update_interval;
return 0;
}
-/**
- * sht4x_temperature1_read() - read the temperature in millidegrees
- */
+/* sht4x_temperature1_read() - read the temperature in millidegrees */
static int sht4x_temperature1_read(struct sht4x_data *data, long *val)
{
int ret;
@@ -158,9 +155,7 @@ static int sht4x_temperature1_read(struct sht4x_data *data, long *val)
return 0;
}
-/**
- * sht4x_humidity1_read() - read a relative humidity in millipercent
- */
+/* sht4x_humidity1_read() - read a relative humidity in millipercent */
static int sht4x_humidity1_read(struct sht4x_data *data, long *val)
{
int ret;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] hwmon: sht4x: Fix sht4x_read_values return value
2021-05-29 9:13 [PATCH] hwmon: sht4x: Fix sht4x_read_values return value Joe Perches
@ 2021-06-02 11:57 ` Guenter Roeck
0 siblings, 0 replies; 2+ messages in thread
From: Guenter Roeck @ 2021-06-02 11:57 UTC (permalink / raw)
To: Joe Perches; +Cc: Jean Delvare, Navin Sankar Velliangiri, linux-hwmon, LKML
On Sat, May 29, 2021 at 02:13:52AM -0700, Joe Perches wrote:
> Kernel doc for sht4x_read_values() shows 0 on success, 1 on failure but
> the return value on success is actually always positive as it is set to
> SHT4X_RESPONSE_LENGTH by a successful call to i2c_master_recv().
>
> Miscellanea:
>
> o Update the kernel doc for sht4x_read_values to 0 for success or -ERRNO
> o Remove incorrectly used kernel doc /** header for other _read functions
> o Typo fix succesfull->successful
> o Reverse a test to unindent a block and use goto unlock
> o Declare cmd[SHT4X_CMD_LEN] rather than cmd[]
>
> At least for gcc 10.2, object size is reduced a tiny bit.
>
> $ size drivers/hwmon/sht4x.o*
> text data bss dec hex filename
> 1752 404 256 2412 96c drivers/hwmon/sht4x.o.new
> 1825 404 256 2485 9b5 drivers/hwmon/sht4x.o.old
>
> Signed-off-by: Joe Perches <joe@perches.com>
Thanks, applied.
Guenter
> ---
>
> compiled, untested, no hardware
>
> drivers/hwmon/sht4x.c | 95 ++++++++++++++++++++++++---------------------------
> 1 file changed, 45 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
> index 1dc51ee2a72ba..09c2a0b064444 100644
> --- a/drivers/hwmon/sht4x.c
> +++ b/drivers/hwmon/sht4x.c
> @@ -67,7 +67,7 @@ struct sht4x_data {
> /**
> * sht4x_read_values() - read and parse the raw data from the SHT4X
> * @sht4x_data: the struct sht4x_data to use for the lock
> - * Return: 0 if succesfull, 1 if not
> + * Return: 0 if successful, -ERRNO if not
> */
> static int sht4x_read_values(struct sht4x_data *data)
> {
> @@ -75,51 +75,53 @@ static int sht4x_read_values(struct sht4x_data *data)
> u16 t_ticks, rh_ticks;
> unsigned long next_update;
> struct i2c_client *client = data->client;
> - u8 crc, raw_data[SHT4X_RESPONSE_LENGTH],
> - cmd[] = {SHT4X_CMD_MEASURE_HPM};
> + u8 crc;
> + u8 cmd[SHT4X_CMD_LEN] = {SHT4X_CMD_MEASURE_HPM};
> + u8 raw_data[SHT4X_RESPONSE_LENGTH];
>
> mutex_lock(&data->lock);
> next_update = data->last_updated +
> msecs_to_jiffies(data->update_interval);
> - if (!data->valid || time_after(jiffies, next_update)) {
> - ret = i2c_master_send(client, cmd, SHT4X_CMD_LEN);
> - if (ret < 0)
> - goto unlock;
> -
> - usleep_range(SHT4X_MEAS_DELAY,
> - SHT4X_MEAS_DELAY + SHT4X_DELAY_EXTRA);
> -
> - ret = i2c_master_recv(client, raw_data, SHT4X_RESPONSE_LENGTH);
> - if (ret != SHT4X_RESPONSE_LENGTH) {
> - if (ret >= 0)
> - ret = -ENODATA;
> -
> - goto unlock;
> - }
> -
> - t_ticks = raw_data[0] << 8 | raw_data[1];
> - rh_ticks = raw_data[3] << 8 | raw_data[4];
> -
> - crc = crc8(sht4x_crc8_table, &raw_data[0], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
> - if (crc != raw_data[2]) {
> - dev_err(&client->dev, "data integrity check failed\n");
> - ret = -EIO;
> - goto unlock;
> - }
> -
> - crc = crc8(sht4x_crc8_table, &raw_data[3], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
> - if (crc != raw_data[5]) {
> - dev_err(&client->dev, "data integrity check failed\n");
> - ret = -EIO;
> - goto unlock;
> - }
> -
> - data->temperature = ((21875 * (int32_t)t_ticks) >> 13) - 45000;
> - data->humidity = ((15625 * (int32_t)rh_ticks) >> 13) - 6000;
> - data->last_updated = jiffies;
> - data->valid = true;
> +
> + if (data->valid && time_before_eq(jiffies, next_update))
> + goto unlock;
> +
> + ret = i2c_master_send(client, cmd, SHT4X_CMD_LEN);
> + if (ret < 0)
> + goto unlock;
> +
> + usleep_range(SHT4X_MEAS_DELAY, SHT4X_MEAS_DELAY + SHT4X_DELAY_EXTRA);
> +
> + ret = i2c_master_recv(client, raw_data, SHT4X_RESPONSE_LENGTH);
> + if (ret != SHT4X_RESPONSE_LENGTH) {
> + if (ret >= 0)
> + ret = -ENODATA;
> + goto unlock;
> + }
> +
> + t_ticks = raw_data[0] << 8 | raw_data[1];
> + rh_ticks = raw_data[3] << 8 | raw_data[4];
> +
> + crc = crc8(sht4x_crc8_table, &raw_data[0], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
> + if (crc != raw_data[2]) {
> + dev_err(&client->dev, "data integrity check failed\n");
> + ret = -EIO;
> + goto unlock;
> }
>
> + crc = crc8(sht4x_crc8_table, &raw_data[3], SHT4X_WORD_LEN, CRC8_INIT_VALUE);
> + if (crc != raw_data[5]) {
> + dev_err(&client->dev, "data integrity check failed\n");
> + ret = -EIO;
> + goto unlock;
> + }
> +
> + data->temperature = ((21875 * (int32_t)t_ticks) >> 13) - 45000;
> + data->humidity = ((15625 * (int32_t)rh_ticks) >> 13) - 6000;
> + data->last_updated = jiffies;
> + data->valid = true;
> + ret = 0;
> +
> unlock:
> mutex_unlock(&data->lock);
> return ret;
> @@ -132,19 +134,14 @@ static ssize_t sht4x_interval_write(struct sht4x_data *data, long val)
> return 0;
> }
>
> -/**
> - * sht4x_interval_read() - read the minimum poll interval
> - * in milliseconds
> - */
> +/* sht4x_interval_read() - read the minimum poll interval in milliseconds */
> static size_t sht4x_interval_read(struct sht4x_data *data, long *val)
> {
> *val = data->update_interval;
> return 0;
> }
>
> -/**
> - * sht4x_temperature1_read() - read the temperature in millidegrees
> - */
> +/* sht4x_temperature1_read() - read the temperature in millidegrees */
> static int sht4x_temperature1_read(struct sht4x_data *data, long *val)
> {
> int ret;
> @@ -158,9 +155,7 @@ static int sht4x_temperature1_read(struct sht4x_data *data, long *val)
> return 0;
> }
>
> -/**
> - * sht4x_humidity1_read() - read a relative humidity in millipercent
> - */
> +/* sht4x_humidity1_read() - read a relative humidity in millipercent */
> static int sht4x_humidity1_read(struct sht4x_data *data, long *val)
> {
> int ret;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-06-02 11:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-29 9:13 [PATCH] hwmon: sht4x: Fix sht4x_read_values return value Joe Perches
2021-06-02 11:57 ` Guenter Roeck
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.