| 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
| ||
|
Date: Mon, 12 Jun 2023 06:45:18 -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: [RFC] sht3x code modifcation
On 6/12/23 02:58, JuenKit Yip wrote:
> The original code only support two mode: high-precision mode(high repeatability and
> 10Hz periodic measurement) and low-power mode(low repeatability and 0.5Hz measurement),
> but in fact this sensor support 5 periodic measurement duration(and single shot) and
> 3 repeatability which are not fully implemented.
> High-precision mode was defined manually so that I think that we should reserve the
> right to user to choose which one repeatability and periodic measurement is the best.
> I just put the patch for reference and hope you could give any comments.
> Medium-repeatability was not added into code and I hope put it later.
>
> If you have any other further question, kindly contact to me.
> Thanks
>
> Juen Kit Yip
>
> Signed-off-by: JuenKit Yip <JuenKit_Yip@...mail.com>
>
So this patch replaces the term "high precision" with "high repeatability",
"low power" with "low repeatability", and it removes the ability to select
high vs. low precision / repeatability from platform data.
I would accept a patch removing platform data entirely since there is
no in-tree driver supporting it. The ability to select precision
(or reliability) should still be supported, possibly with a non-standard
sysfs attribute. You might consider sending a series of patches.
1) Remove platform data support
2) (optional) Convert driver to with_info API
3) Replace "precision" with "repeatability" to match datasheet
terminology
4) Implement a non-standard sysfs attribute such as "repeatability"
to support selecting high/medium/low repeatability.
5) (optional) Implement devicetree support.
Guenter
> ---
> Documentation/hwmon/sht3x.rst | 6 ++----
> drivers/hwmon/sht3x.c | 27 +++++++++++++++++----------
> include/linux/platform_data/sht3x.h | 1 -
> 3 files changed, 19 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/hwmon/sht3x.rst b/Documentation/hwmon/sht3x.rst
> index 95a850d5b..c6b7a1aa5 100644
> --- a/Documentation/hwmon/sht3x.rst
> +++ b/Documentation/hwmon/sht3x.rst
> @@ -28,15 +28,13 @@ The device communicates with the I2C protocol. Sensors can have the I2C
> addresses 0x44 or 0x45, depending on the wiring. See
> Documentation/i2c/instantiating-devices.rst for methods to instantiate the device.
>
> -There are two options configurable by means of sht3x_platform_data:
> +There is only one option configurable by means of sht3x_platform_data:
>
> -1. blocking (pull the I2C clock line down while performing the measurement) or
> + blocking (pull the I2C clock line down while performing the measurement) or
> non-blocking mode. Blocking mode will guarantee the fastest result but
> the I2C bus will be busy during that time. By default, non-blocking mode
> is used. Make sure clock-stretching works properly on your device if you
> want to use blocking mode.
> -2. high or low accuracy. High accuracy is used by default and using it is
> - strongly recommended.
>
> The sht3x sensor supports a single shot mode as well as 5 periodic measure
> modes, which can be controlled with the update_interval sysfs interface.
> diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
> index 8305e44d9..6065312ae 100644
> --- a/drivers/hwmon/sht3x.c
> +++ b/drivers/hwmon/sht3x.c
> @@ -22,11 +22,11 @@
> #include <linux/jiffies.h>
> #include <linux/platform_data/sht3x.h>
>
> -/* commands (high precision mode) */
> +/* commands (high repeatability mode) */
> static const unsigned char sht3x_cmd_measure_blocking_hpm[] = { 0x2c, 0x06 };
> static const unsigned char sht3x_cmd_measure_nonblocking_hpm[] = { 0x24, 0x00 };
>
> -/* commands (low power mode) */
> +/* 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 };
>
> @@ -69,9 +69,14 @@ enum sht3x_limits {
> limit_min_hyst,
> };
>
> +enum sht3x_repeatability {
> + high_repeatability,
> + low_repeatability,
> +};
> +
> DECLARE_CRC8_TABLE(sht3x_crc8_table);
>
> -/* periodic measure commands (high precision mode) */
> +/* periodic measure commands (high repeatability mode) */
> static const char periodic_measure_commands_hpm[][SHT3X_CMD_LENGTH] = {
> /* 0.5 measurements per second */
> {0x20, 0x32},
> @@ -85,7 +90,7 @@ static const char periodic_measure_commands_hpm[][SHT3X_CMD_LENGTH] = {
> {0x27, 0x37},
> };
>
> -/* periodic measure commands (low power mode) */
> +/* periodic measure commands (low repeatability mode) */
> static const char periodic_measure_commands_lpm[][SHT3X_CMD_LENGTH] = {
> /* 0.5 measurements per second */
> {0x20, 0x2f},
> @@ -132,6 +137,7 @@ struct sht3x_data {
> struct mutex data_lock; /* lock for updating driver data */
>
> u8 mode;
> + enum sht3x_repeatability repeatability;
> const unsigned char *command;
> u32 wait_time; /* in us*/
> unsigned long last_update; /* last update in periodic mode*/
> @@ -442,12 +448,13 @@ static void sht3x_select_command(struct sht3x_data *data)
> data->command = sht3x_cmd_measure_periodic_mode;
> data->wait_time = 0;
> } else if (data->setup.blocking_io) {
> - data->command = data->setup.high_precision ?
> - sht3x_cmd_measure_blocking_hpm :
> - sht3x_cmd_measure_blocking_lpm;
> + if(data->repeatability == high_repeatability)
> + data->command = sht3x_cmd_measure_blocking_hpm;
> + else if(data->repeatability == low_repeatability)
> + data->command = sht3x_cmd_measure_blocking_lpm;
> data->wait_time = 0;
> } else {
> - if (data->setup.high_precision) {
> + if (data->repeatability == high_repeatability) {
> data->command = sht3x_cmd_measure_nonblocking_hpm;
> data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_HPM;
> } else {
> @@ -595,7 +602,7 @@ static ssize_t update_interval_store(struct device *dev,
> }
>
> if (mode > 0) {
> - if (data->setup.high_precision)
> + if (data->repeatability == high_repeatability)
> command = periodic_measure_commands_hpm[mode - 1];
> else
> command = periodic_measure_commands_lpm[mode - 1];
> @@ -691,7 +698,7 @@ static int sht3x_probe(struct i2c_client *client)
> return -ENOMEM;
>
> data->setup.blocking_io = false;
> - data->setup.high_precision = true;
> + data->repeatability = high_repeatability;
> data->mode = 0;
> data->last_update = jiffies - msecs_to_jiffies(3000);
> data->client = client;
> diff --git a/include/linux/platform_data/sht3x.h b/include/linux/platform_data/sht3x.h
> index 14680d2a9..626c1404a 100644
> --- a/include/linux/platform_data/sht3x.h
> +++ b/include/linux/platform_data/sht3x.h
> @@ -10,6 +10,5 @@
>
> struct sht3x_platform_data {
> bool blocking_io;
> - bool high_precision;
> };
> #endif /* __SHT3X_H_ */
Powered by blists - more mailing lists