[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6d1209e9-0d40-c2ba-b94c-6590e98070d7@roeck-us.net>
Date: Wed, 14 Jun 2023 05:57:21 -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 1/3] hwmon: (sht3x)remove sht3x_platform_data
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.
> Signed-off-by: JuenKit Yip <JuenKit_Yip@...mail.com>
> ---
> 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.
> } 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@...sirion.com>
> - * Author: Pascal Sachs <pascal.sachs@...sirion.com>
> - */
> -
> -#ifndef __SHT3X_H_
> -#define __SHT3X_H_
> -
> -struct sht3x_platform_data {
> - bool blocking_io;
> - bool high_precision;
> -};
> -#endif /* __SHT3X_H_ */
Powered by blists - more mailing lists