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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8e45875a-8af2-4409-8ea8-255281f97e68@roeck-us.net>
Date: Sun, 25 Aug 2024 17:15:41 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Antoni Pokusinski <apokusinski01@...il.com>, jdelvare@...e.com
Cc: linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: (sht4x): add heater support

On 8/25/24 03:09, Antoni Pokusinski wrote:
> Add support for manipulating the internal heater of sht4x devices.
> The heater may be used to remove condensed water on the sensor surface
> which disturbs the relative humidity measurements.
> 

Where is this seen in practice ? "may be" does not mean that the problem
is actively seen.

> The heater can operate at three heating levels (20, 110 or 200
> milliwatts). Also, two heating durations may be selected (0.1 or 1s).
> Once the heating time elapses the heater is automatically switched off.
> 
> Signed-off-by: Antoni Pokusinski <apokusinski01@...il.com>
> ---
> Changes since v1:
> * explain the use case of the new attributes set
> * heater_enable attr: make it write-only
> * heater_enable_store: define cmd as u8 instead of u8*
> * heater_enable_store: remove unreachable data path
> * heater_enable_store: remove unnecessary lock
> * heater_enable_store: call i2c_master_send only if status==true
> * define attributes as DEVICE_ATTR_* instead of SENSOR_DEVICE_ATTR_*
> ---
>   Documentation/hwmon/sht4x.rst |  11 +++
>   drivers/hwmon/sht4x.c         | 126 +++++++++++++++++++++++++++++++++-
>   2 files changed, 136 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/hwmon/sht4x.rst b/Documentation/hwmon/sht4x.rst
> index daf21e763425..3ff6c08d8267 100644
> --- a/Documentation/hwmon/sht4x.rst
> +++ b/Documentation/hwmon/sht4x.rst
> @@ -42,4 +42,15 @@ humidity1_input Measured humidity in %H
>   update_interval The minimum interval for polling the sensor,
>                   in milliseconds. Writable. Must be at least
>                   2000.
> +heater_power	The requested heater power, in milliwatts.
> +		Available values: 20, 110, 200 (default: 200).
> +heater_time	The requested operating time of the heater,
> +		in milliseconds.
> +		Available values: 100, 1000 (default 1000).
> +heater_enable	Enable the heater with the selected power
> +		and for the selected time in order to remove
> +		condensed water from the sensor surface. Write-only.
> +
> +			- 0: turn off

Not really. As implemented, this does not turn off the heater, it does nothing.
If that is what you want' you'd have to send a command "turn off heater immediately"
if such a command exists.

> +			- 1: turn on
>   =============== ============================================
> diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
> index b8916d2735b5..75e092f9b80e 100644
> --- a/drivers/hwmon/sht4x.c
> +++ b/drivers/hwmon/sht4x.c
> @@ -11,6 +11,7 @@
>   #include <linux/crc8.h>
>   #include <linux/delay.h>
>   #include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
>   #include <linux/i2c.h>
>   #include <linux/jiffies.h>
>   #include <linux/module.h>
> @@ -31,6 +32,12 @@
>    */
>   #define SHT4X_CMD_MEASURE_HPM	0b11111101
>   #define SHT4X_CMD_RESET		0b10010100
> +#define SHT4X_CMD_HEATER_20_1	0b00011110
> +#define SHT4X_CMD_HEATER_20_01	0b00010101
> +#define SHT4X_CMD_HEATER_110_1	0b00101111
> +#define SHT4X_CMD_HEATER_110_01	0b00100100
> +#define SHT4X_CMD_HEATER_200_1	0b00111001
> +#define SHT4X_CMD_HEATER_200_01 0b00110010
>   
>   #define SHT4X_CMD_LEN		1
>   #define SHT4X_CRC8_LEN		1
> @@ -54,6 +61,8 @@ DECLARE_CRC8_TABLE(sht4x_crc8_table);
>    * @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
> + * @heater_power: the power at which the heater will be started
> + * @heater_time: the time for which the heater will remain turned on
>    */
>   struct sht4x_data {
>   	struct i2c_client	*client;
> @@ -63,6 +72,8 @@ struct sht4x_data {
>   	long			last_updated;	/* in jiffies */
>   	s32			temperature;
>   	s32			humidity;
> +	u32			heater_power;	/* in milli-watts */
> +	u32			heater_time;	/* in milli-seconds */
>   };
>   
>   /**
> @@ -215,6 +226,117 @@ static int sht4x_hwmon_write(struct device *dev, enum hwmon_sensor_types type,
>   	}
>   }
>   
> +static ssize_t heater_enable_store(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf,
> +				   size_t count)
> +{
> +	struct sht4x_data *data = dev_get_drvdata(dev);
> +	bool status;
> +	ssize_t ret;
> +	u8 cmd;
> +
> +	ret = kstrtobool(buf, &status);
> +	if (ret)
> +		return ret;
> +
> +	if (status) {
> +		if (data->heater_power == 20) {
> +			if (data->heater_time == 100)
> +				cmd = SHT4X_CMD_HEATER_20_01;
> +			else /* data->heater_time == 1000 */
> +				cmd = SHT4X_CMD_HEATER_20_1;
> +		} else if (data->heater_power == 110) {
> +			if (data->heater_time == 100)
> +				cmd = SHT4X_CMD_HEATER_110_01;
> +			else /* data->heater_time == 1000 */
> +				cmd = SHT4X_CMD_HEATER_110_1;
> +		} else if (data->heater_power == 200) {

Unnecessary "if". Due to the limitations of static checkers, those
may wrongly conclude that "cmd" is not initialized when passed to
i2c_master_send().

> +			if (data->heater_time == 100)
> +				cmd = SHT4X_CMD_HEATER_200_01;
> +			else /* data->heater_time == 1000 */
> +				cmd = SHT4X_CMD_HEATER_200_1;
> +		}
> +
> +		ret = i2c_master_send(data->client, &cmd, SHT4X_CMD_LEN);
> +	}
> +
> +	return ret; > +}
> +
> +static ssize_t heater_power_show(struct device *dev,
> +				 struct device_attribute *attr,
> +				 char *buf)
> +{
> +	struct sht4x_data *data = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%u\n", data->heater_power);
> +}
> +
> +static ssize_t heater_power_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf,
> +				  size_t count)
> +{
> +	struct sht4x_data *data = dev_get_drvdata(dev);
> +	u32 power;
> +	ssize_t ret;
> +
> +	ret = kstrtou32(buf, 10, &power);
> +	if (ret)
> +		return ret;
> +
> +	if (power != 20 && power != 110 && power != 200)
> +		return -EINVAL;
> +
> +	data->heater_power = power;
> +
> +	return count;
> +}
> +
> +static ssize_t heater_time_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	struct sht4x_data *data = dev_get_drvdata(dev);
> +
> +	return sysfs_emit(buf, "%u\n", data->heater_time);
> +}
> +
> +static ssize_t heater_time_store(struct device *dev,
> +				 struct device_attribute *attr,
> +				 const char *buf,
> +				 size_t count)
> +{
> +	struct sht4x_data *data = dev_get_drvdata(dev);
> +	u32 time;
> +	ssize_t ret;
> +
> +	ret = kstrtou32(buf, 10, &time);
> +	if (ret)
> +		return ret;
> +
> +	if (time != 100 && time != 1000)
> +		return -EINVAL;
> +
> +	data->heater_time = time;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR_WO(heater_enable);
> +static DEVICE_ATTR_RW(heater_power);
> +static DEVICE_ATTR_RW(heater_time);
> +
> +static struct attribute *sht4x_attrs[] = {
> +	&dev_attr_heater_enable.attr,
> +	&dev_attr_heater_power.attr,
> +	&dev_attr_heater_time.attr,
> +	NULL
> +};
> +
> +ATTRIBUTE_GROUPS(sht4x);
> +
>   static const struct hwmon_channel_info * const sht4x_info[] = {
>   	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
>   	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> @@ -255,6 +377,8 @@ static int sht4x_probe(struct i2c_client *client)
>   
>   	data->update_interval = SHT4X_MIN_POLL_INTERVAL;
>   	data->client = client;
> +	data->heater_power = 200;
> +	data->heater_time = 1000;
>   
>   	mutex_init(&data->lock);
>   
> @@ -270,7 +394,7 @@ static int sht4x_probe(struct i2c_client *client)
>   							 client->name,
>   							 data,
>   							 &sht4x_chip_info,
> -							 NULL);
> +							 sht4x_groups);
>   
>   	return PTR_ERR_OR_ZERO(hwmon_dev);
>   }


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ