Re: [PATCH 1/3] hwmon: (sht3x)remove sht3x_platform_data
From: Guenter Roeck
Date: Wed Jun 14 2023 - 15:15:36 EST
On 6/14/23 08:02, JuenKit Yip wrote:
在 2023/6/14 20:57, Guenter Roeck 写道:
On 6/13/23 23:24, JuenKit Yip wrote:
Since no in-tree driver supports it, the sht3x_platform_data was
removed.
- "blocking_io" property and its related code have been removed, and
Single-Shot mode should be blocking in default.
- "high-precision" property has been replaced to "repeatability" for
matching datasheet.
That needs to be three patches.
Patch 1: remove sht3x_platform_data and its header file
Patch 2: move "blocking_io" to struct sht3x_data
Essentially merge it with update_interval==0 since (if I understand
correctly) blocking mode and update_interval==0 will be equivalent.
With that in mind, a separate "blocking_io" variable should no
longer be needed.
Patch 3: replace "high-precision" property to "repeatability"
precision -> reliability would apply to both high and low
precision. I think "low repeatability" is currently called
"low power mode", so you'll want to update that as well.
I also see "high accuracy" and "low accuracy" use in the
documentation, but I see you removed that already below.
Is it correct or I am misunderstand your statement?
Yes, that is what I meant.
Thanks,
Guenter
Thanks for your instruction
Signed-off-by: JuenKit Yip <JuenKit_Yip@xxxxxxxxxxx>
---
Documentation/hwmon/sht3x.rst | 12 +++++------
drivers/hwmon/sht3x.c | 32 ++++++++++++-----------------
include/linux/platform_data/sht3x.h | 15 --------------
3 files changed, 18 insertions(+), 41 deletions(-)
delete mode 100644 include/linux/platform_data/sht3x.h
diff --git a/Documentation/hwmon/sht3x.rst b/Documentation/hwmon/sht3x.rst
index 95a850d5b..2c87c8f58 100644
--- a/Documentation/hwmon/sht3x.rst
+++ b/Documentation/hwmon/sht3x.rst
@@ -28,28 +28,26 @@ 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:
+This driver supports block and non-block mode:
-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.
The allowed update_interval in milliseconds are as follows:
- ===== ======= ====================
- 0 single shot mode
+ ===== ======= ==========================
+ 0 single shot mode(blocking)
2000 0.5 Hz periodic measurement
1000 1 Hz periodic measurement
500 2 Hz periodic measurement
250 4 Hz periodic measurement
100 10 Hz periodic measurement
- ===== ======= ====================
+ ===== ======= ==========================
In the periodic measure mode, the sensor automatically triggers a measurement
with the configured update interval on the chip. When a temperature or humidity
diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
index 8305e44d9..5bc0001b1 100644
--- a/drivers/hwmon/sht3x.c
+++ b/drivers/hwmon/sht3x.c
@@ -20,13 +20,12 @@
#include <linux/module.h>
#include <linux/slab.h>
#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 +68,14 @@ enum sht3x_limits {
limit_min_hyst,
};
+enum sht3x_repeatability {
+ low_repeatability,
+ high_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 +89,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,12 +136,11 @@ 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*/
- struct sht3x_platform_data setup;
-
/*
* cached values for temperature and humidity and limits
* the limits arrays have the following order:
@@ -441,13 +444,8 @@ static void sht3x_select_command(struct sht3x_data *data)
if (data->mode > 0) {
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;
- data->wait_time = 0;
If update_interval is 0, those would presumably still be needed.
I don't know if the current code updating the interval is wrong
(that may well be), but removing this code entirely seems wrong.
update_interval "0" means Single-Shot mode and respectively data->command
should be in blocking mode.
Thanks for your correctness.
} 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 +593,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];
@@ -690,16 +688,12 @@ static int sht3x_probe(struct i2c_client *client)
if (!data)
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;
crc8_populate_msb(sht3x_crc8_table, SHT3X_CRC8_POLYNOMIAL);
- if (client->dev.platform_data)
- data->setup = *(struct sht3x_platform_data *)dev->platform_data;
-
sht3x_select_command(data);
mutex_init(&data->i2c_lock);
diff --git a/include/linux/platform_data/sht3x.h b/include/linux/platform_data/sht3x.h
deleted file mode 100644
index 14680d2a9..000000000
--- a/include/linux/platform_data/sht3x.h
+++ /dev/null
@@ -1,15 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * Copyright (C) 2016 Sensirion AG, Switzerland
- * Author: David Frey <david.frey@xxxxxxxxxxxxx>
- * Author: Pascal Sachs <pascal.sachs@xxxxxxxxxxxxx>
- */
-
-#ifndef __SHT3X_H_
-#define __SHT3X_H_
-
-struct sht3x_platform_data {
- bool blocking_io;
- bool high_precision;
-};
-#endif /* __SHT3X_H_ */