linux-kernel - Re: [PATCH 2/3] hwmon: (sht3x) add medium repeatability support

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <9e9a7066-b841-db97-f8a5-44b2bdac124c@roeck-us.net>
Date:   Wed, 14 Jun 2023 06:00:42 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     JuenKit Yip <JuenKit_Yip@...mail.com>, jdelvare@...e.com
Cc:     linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] hwmon: (sht3x) add medium repeatability support

On 6/13/23 23:27, JuenKit Yip wrote:
> add medium repeatability support for matching datasheet
> 
> Signed-off-by: JuenKit Yip <JuenKit_Yip@...mail.com>
> ---
>   drivers/hwmon/sht3x.c | 29 +++++++++++++++++++++++++++--
>   1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
> index 5bc0001b1..eb968b9d3 100644
> --- a/drivers/hwmon/sht3x.c
> +++ b/drivers/hwmon/sht3x.c
> @@ -25,6 +25,10 @@
>   static const unsigned char sht3x_cmd_measure_blocking_hpm[]    = { 0x2c, 0x06 };
>   static const unsigned char sht3x_cmd_measure_nonblocking_hpm[] = { 0x24, 0x00 };
>   
> +/* commands (medium repeatability mode) */
> +static const unsigned char sht3x_cmd_measure_blocking_mpm[]    = { 0x2c, 0x0d };
> +static const unsigned char sht3x_cmd_measure_nonblocking_mpm[] = { 0x24, 0x0b };
> +
>   /* commands (low repeatability mode) */
>   static const unsigned char sht3x_cmd_measure_blocking_lpm[]    = { 0x2c, 0x10 };
>   static const unsigned char sht3x_cmd_measure_nonblocking_lpm[] = { 0x24, 0x16 };
> @@ -43,6 +47,7 @@ static const unsigned char sht3x_cmd_clear_status_reg[]        = { 0x30, 0x41 };
>   
>   /* delays for non-blocking i2c commands, both in us */
>   #define SHT3X_NONBLOCKING_WAIT_TIME_HPM  15000
> +#define SHT3X_NONBLOCKING_WAIT_TIME_MPM   6000
>   #define SHT3X_NONBLOCKING_WAIT_TIME_LPM   4000
>   
>   #define SHT3X_WORD_LEN         2
> @@ -69,8 +74,9 @@ enum sht3x_limits {
>   };
>   
>   enum sht3x_repeatability {
> -	low_repeatability,
> -	high_repeatability,
> +	low_repeatability    = 0,
> +	medium_repeatability = 1,
> +	high_repeatability   = 2,

Why the assignments ?

>   };
>   
>   DECLARE_CRC8_TABLE(sht3x_crc8_table);
> @@ -89,6 +95,20 @@ static const char periodic_measure_commands_hpm[][SHT3X_CMD_LENGTH] = {
>   	{0x27, 0x37},
>   };
>   
> +/* periodic measure commands (medium repeatability) */
> +static const char periodic_measure_commands_mpm[][SHT3X_CMD_LENGTH] = {

FWIW, it might make sense to add the command for blocking mode
(0 measurements per second) into this table to simplify the code.

> +	/* 0.5 measurements per second */
> +	{0x20, 0x24},
> +	/* 1 measurements per second */
> +	{0x21, 0x26},
> +	/* 2 measurements per second */
> +	{0x22, 0x20},
> +	/* 4 measurements per second */
> +	{0x23, 0x22},
> +	/* 10 measurements per second */
> +	{0x27, 0x21},
> +};
> +
>   /* periodic measure commands (low repeatability mode) */
>   static const char periodic_measure_commands_lpm[][SHT3X_CMD_LENGTH] = {
>   	/* 0.5 measurements per second */
> @@ -448,6 +468,9 @@ static void sht3x_select_command(struct sht3x_data *data)
>   		if (data->repeatability == high_repeatability) {
>   			data->command = sht3x_cmd_measure_nonblocking_hpm;
>   			data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_HPM;
> +		} else if (data->repeatability ==  medium_repeatability) {
> +			data->command = sht3x_cmd_measure_nonblocking_mpm;
> +			data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_MPM;
>   		} else {
>   			data->command = sht3x_cmd_measure_nonblocking_lpm;
>   			data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_LPM;
> @@ -595,6 +618,8 @@ static ssize_t update_interval_store(struct device *dev,
>   	if (mode > 0) {
>   		if (data->repeatability == high_repeatability)
>   			command = periodic_measure_commands_hpm[mode - 1];
> +		else if (data->repeatability == medium_repeatability)
> +			command = periodic_measure_commands_mpm[mode - 1];
>   		else
>   			command = periodic_measure_commands_lpm[mode - 1];
>