| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0fdb63a8-f353-cbcb-b97b-b54ab2da9d7f@roeck-us.net>
Date: Sun, 23 May 2021 07:01:35 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Navin Sankar Velliangiri <navin@...umiz.com>
Cc: Jean Delvare <jdelvare@...e.com>, Jonathan Corbet <corbet@....net>,
linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6] hwmon: Add sht4x Temperature and Humidity Sensor
Driver
On 5/21/21 11:09 PM, Navin Sankar Velliangiri wrote:
> This patch adds a hwmon driver for the SHT4x Temperature and
> Humidity sensor.
>
> Signed-off-by: Navin Sankar Velliangiri <navin@...umiz.com>
>
> Changes in v2:
>
> * Removed unused macro SHT4X_MIN_POLL_INTERVAL
> * Replaced time_after instead of ktime_after
> * Used goto statements for error handling
> * Hardcoded the interval_time instead of clamp_val().
>
> Changes in v3:
>
> * Accept the poll interval if it is greater than SHT4X_MIN_POLL_INTERVAL and
> return -EINVAL for negative values & less than SHT4X_MIN_POLL_INTERVAL
> * Changed the data type of update_interval and last_updated to long.
>
> Changes in v4:
>
> * "update_interval" is long but msecs_to_jiffies() accepts only unsigned int.
> clamp_val() api is used to assign the update_interval stays within UINT_MAX.
>
> Changes in v5:
>
> * Added error handling when master unable to send the data.
>
> Changes in v6:
>
> * clamp_val() alone is used to set the update interval. since the update
> interval is a continuous setting.
> ---
> Documentation/hwmon/index.rst | 1 +
> Documentation/hwmon/sht4x.rst | 45 +++++
> drivers/hwmon/Kconfig | 11 ++
> drivers/hwmon/Makefile | 1 +
> drivers/hwmon/sht4x.c | 305 ++++++++++++++++++++++++++++++++++
> 5 files changed, 363 insertions(+)
> create mode 100644 Documentation/hwmon/sht4x.rst
> create mode 100644 drivers/hwmon/sht4x.c
>
> diff --git a/Documentation/hwmon/index.rst b/Documentation/hwmon/index.rst
> index 9ed60fa84cbe..b6fcae40258c 100644
> --- a/Documentation/hwmon/index.rst
> +++ b/Documentation/hwmon/index.rst
> @@ -164,6 +164,7 @@ Hardware Monitoring Kernel Drivers
> sht15
> sht21
> sht3x
> + sht4x
> shtc1
> sis5595
> sl28cpld
> diff --git a/Documentation/hwmon/sht4x.rst b/Documentation/hwmon/sht4x.rst
> new file mode 100644
> index 000000000000..3b37abcd4a46
> --- /dev/null
> +++ b/Documentation/hwmon/sht4x.rst
> @@ -0,0 +1,45 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +Kernel driver sht4x
> +===================
> +
> +Supported Chips:
> +
> + * Sensirion SHT4X
> +
> + Prefix: 'sht4x'
> +
> + Addresses scanned: None
> +
> + Datasheet:
> +
> + English: https://www.sensirion.com/fileadmin/user_upload/customers/sensirion/Dokumente/2_Humidity_Sensors/Datasheets/Sensirion_Humidity_Sensors_SHT4x_Datasheet.pdf
> +
> +Author: Navin Sankar Velliangiri <navin@...umiz.com>
> +
> +
> +Description
> +-----------
> +
> +This driver implements support for the Sensirion SHT4x chip, a humidity
> +and temperature sensor. Temperature is measured in degree celsius, relative
> +humidity is expressed as a percentage. In sysfs interface, all values are
> +scaled by 1000, i.e. the value for 31.5 degrees celsius is 31500.
> +
> +Usage Notes
> +-----------
> +
> +The device communicates with the I2C protocol. Sensors can have the I2C
> +address 0x44. See Documentation/i2c/instantiating-devices.rst for methods
> +to instantiate the device.
> +
> +Sysfs entries
> +-------------
> +
> +=============== ============================================
> +temp1_input Measured temperature in millidegrees Celcius
> +humidity1_input Measured humidity in %H
> +update_interval The minimum interval for polling the sensor,
> + in milliseconds. Writable. Must be at least
> + 2000.
> +============== =============================================
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 87624902ea80..e3675377bc5d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1583,6 +1583,17 @@ config SENSORS_SHT3x
> This driver can also be built as a module. If so, the module
> will be called sht3x.
>
> +config SENSORS_SHT4x
> + tristate "Sensiron humidity and temperature sensors. SHT4x and compat."
> + depends on I2C
> + select CRC8
> + help
> + If you say yes here you get support for the Sensiron SHT40, SHT41 and
> + SHT45 humidity and temperature sensors.
> +
> + This driver can also be built as a module. If so, the module
> + will be called sht4x.
> +
> config SENSORS_SHTC1
> tristate "Sensiron humidity and temperature sensors. SHTC1 and compat."
> depends on I2C
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 59e78bc212cf..d712c61c1f5e 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -171,6 +171,7 @@ obj-$(CONFIG_SENSORS_SL28CPLD) += sl28cpld-hwmon.o
> obj-$(CONFIG_SENSORS_SHT15) += sht15.o
> obj-$(CONFIG_SENSORS_SHT21) += sht21.o
> obj-$(CONFIG_SENSORS_SHT3x) += sht3x.o
> +obj-$(CONFIG_SENSORS_SHT4x) += sht4x.o
> obj-$(CONFIG_SENSORS_SHTC1) += shtc1.o
> obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o
> obj-$(CONFIG_SENSORS_SMM665) += smm665.o
> diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
> new file mode 100644
> index 000000000000..39e1b4a123fa
> --- /dev/null
> +++ b/drivers/hwmon/sht4x.c
> @@ -0,0 +1,305 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (c) Linumiz 2021
> + *
> + * sht4x.c - Linux hwmon driver for SHT4x Temperature and Humidity sensor
> + *
> + * Author: Navin Sankar Velliangiri <navin@...umiz.com>
> + */
> +
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/hwmon.h>
> +#include <linux/i2c.h>
> +#include <linux/jiffies.h>
> +#include <linux/module.h>
> +
> +/*
> + * Poll intervals (in milliseconds)
> + */
> +#define SHT4X_MIN_POLL_INTERVAL 2000
> +
> +/*
> + * I2C command delays (in microseconds)
> + */
> +#define SHT4X_MEAS_DELAY 1000
> +#define SHT4X_DELAY_EXTRA 10000
> +
> +/*
> + * Command Bytes
> + */
> +#define SHT4X_CMD_MEASURE_HPM 0b11111101
> +#define SHT4X_CMD_RESET 0b10010100
> +
> +#define SHT4X_CMD_LEN 1
> +#define SHT4X_CRC8_LEN 1
> +#define SHT4X_WORD_LEN 2
> +#define SHT4X_RESPONSE_LENGTH 6
> +#define SHT4X_CRC8_POLYNOMIAL 0x31
> +#define SHT4X_CRC8_INIT 0xff
> +#define SHT4X_MIN_TEMPERATURE -45000
> +#define SHT4X_MAX_TEMPERATURE 125000
> +#define SHT4X_MIN_HUMIDITY 0
> +#define SHT4X_MAX_HUMIDITY 100000
> +
> +DECLARE_CRC8_TABLE(sht4x_crc8_table);
> +
> +/**
> + * struct sht4x_data - All the data required to operate an SHT4X chip
> + * @client: the i2c client associated with the SHT4X
> + * @lock: a mutex that is used to prevent parallel access to the i2c client
> + * @update_interval: the minimum poll interval
> + * @last_updated: the previous time that the SHT4X was polled
> + * @temperature: the latest temperature value received from the SHT4X
> + * @humidity: the latest humidity value received from the SHT4X
> + */
> +struct sht4x_data {
> + struct i2c_client *client;
> + struct mutex lock; /* atomic read data updates */
> + bool valid; /* validity of fields below */
> + long update_interval; /* in milli-seconds */
> + long last_updated; /* in jiffies */
> + s32 temperature;
> + s32 humidity;
> +};
> +
> +/**
> + * 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
> + */
> +static int sht4x_read_values(struct sht4x_data *data)
> +{
> + int ret;
Ah yes, the value of ret will be uninitialized, and the function will return
a random value if data->valid is false or if two measurements follow each other.
Please fix and resubmit. The driver looks good otherwise.
Guenter
Powered by blists - more mailing lists