| Message ID | DB4PR10MB6261D79FE16EC2BBD5316B91925AA@DB4PR10MB6261.EURPRD10.PROD.OUTLOOK.COM (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/3] hwmon: (sht3x)remove sht3x_platform_data | expand |
Hi JuenKit,
kernel test robot noticed the following build warnings:
[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.4-rc6 next-20230614]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/JuenKit-Yip/hwmon-sht3x-add-medium-repeatability-support/20230614-143100
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/DB4PR10MB6261D79FE16EC2BBD5316B91925AA%40DB4PR10MB6261.EURPRD10.PROD.OUTLOOK.COM
patch subject: [PATCH 1/3] hwmon: (sht3x)remove sht3x_platform_data
config: hexagon-randconfig-r045-20230612 (https://download.01.org/0day-ci/archive/20230614/202306141736.b9bPRO0Z-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add groeck-staging https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
git fetch groeck-staging hwmon-next
git checkout groeck-staging/hwmon-next
b4 shazam https://lore.kernel.org/r/DB4PR10MB6261D79FE16EC2BBD5316B91925AA@DB4PR10MB6261.EURPRD10.PROD.OUTLOOK.COM
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/hwmon/
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306141736.b9bPRO0Z-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/hwmon/sht3x.c:17:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/hwmon/sht3x.c:17:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/hwmon/sht3x.c:17:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:13:
In file included from include/linux/cgroup.h:26:
In file included from include/linux/kernel_stat.h:9:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
>> drivers/hwmon/sht3x.c:25:28: warning: unused variable 'sht3x_cmd_measure_blocking_hpm' [-Wunused-const-variable]
25 | static const unsigned char sht3x_cmd_measure_blocking_hpm[] = { 0x2c, 0x06 };
| ^
>> drivers/hwmon/sht3x.c:29:28: warning: unused variable 'sht3x_cmd_measure_blocking_lpm' [-Wunused-const-variable]
29 | static const unsigned char sht3x_cmd_measure_blocking_lpm[] = { 0x2c, 0x10 };
| ^
8 warnings generated.
vim +/sht3x_cmd_measure_blocking_hpm +25 drivers/hwmon/sht3x.c
7c84f7f80d6fcea David Frey 2016-06-02 23
cecbab8bdd40311 JuenKit Yip 2023-06-14 24 /* commands (high repeatability mode) */
7c84f7f80d6fcea David Frey 2016-06-02 @25 static const unsigned char sht3x_cmd_measure_blocking_hpm[] = { 0x2c, 0x06 };
7c84f7f80d6fcea David Frey 2016-06-02 26 static const unsigned char sht3x_cmd_measure_nonblocking_hpm[] = { 0x24, 0x00 };
7c84f7f80d6fcea David Frey 2016-06-02 27
cecbab8bdd40311 JuenKit Yip 2023-06-14 28 /* commands (low repeatability mode) */
7c84f7f80d6fcea David Frey 2016-06-02 @29 static const unsigned char sht3x_cmd_measure_blocking_lpm[] = { 0x2c, 0x10 };
7c84f7f80d6fcea David Frey 2016-06-02 30 static const unsigned char sht3x_cmd_measure_nonblocking_lpm[] = { 0x24, 0x16 };
7c84f7f80d6fcea David Frey 2016-06-02 31
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@hotmail.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@sensirion.com> > - * Author: Pascal Sachs <pascal.sachs@sensirion.com> > - */ > - > -#ifndef __SHT3X_H_ > -#define __SHT3X_H_ > - > -struct sht3x_platform_data { > - bool blocking_io; > - bool high_precision; > -}; > -#endif /* __SHT3X_H_ */
在 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 Patch 3: replace "high-precision" property to "repeatability" Is it correct or I am misunderstand your statement? Thanks for your instruction > >> Signed-off-by: JuenKit Yip <JuenKit_Yip@hotmail.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. 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@sensirion.com> >> - * Author: Pascal Sachs <pascal.sachs@sensirion.com> >> - */ >> - >> -#ifndef __SHT3X_H_ >> -#define __SHT3X_H_ >> - >> -struct sht3x_platform_data { >> - bool blocking_io; >> - bool high_precision; >> -}; >> -#endif /* __SHT3X_H_ */ >
在 2023/6/14 23:02, JuenKit Yip 写道: > > 在 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 > > Patch 3: replace "high-precision" property to "repeatability" > > Is it correct or I am misunderstand your statement? > > Thanks for your instruction > >> >>> Signed-off-by: JuenKit Yip <JuenKit_Yip@hotmail.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. > > update_interval "0" means Single-Shot mode and respectively data->command > > should be in blocking mode or non-blocking mode about clock strench. > > 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@sensirion.com> >>> - * Author: Pascal Sachs <pascal.sachs@sensirion.com> >>> - */ >>> - >>> -#ifndef __SHT3X_H_ >>> -#define __SHT3X_H_ >>> - >>> -struct sht3x_platform_data { >>> - bool blocking_io; >>> - bool high_precision; >>> -}; >>> -#endif /* __SHT3X_H_ */ >>
Hi JuenKit,
kernel test robot noticed the following build warnings:
[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on linus/master v6.4-rc6 next-20230614]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/JuenKit-Yip/hwmon-sht3x-add-medium-repeatability-support/20230614-143100
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link: https://lore.kernel.org/r/DB4PR10MB6261D79FE16EC2BBD5316B91925AA%40DB4PR10MB6261.EURPRD10.PROD.OUTLOOK.COM
patch subject: [PATCH 1/3] hwmon: (sht3x)remove sht3x_platform_data
config: i386-allyesconfig (https://download.01.org/0day-ci/archive/20230615/202306150115.i0k7ulfo-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build):
git remote add groeck-staging https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git
git fetch groeck-staging hwmon-next
git checkout groeck-staging/hwmon-next
b4 shazam https://lore.kernel.org/r/DB4PR10MB6261D79FE16EC2BBD5316B91925AA@DB4PR10MB6261.EURPRD10.PROD.OUTLOOK.COM
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 O=build_dir ARCH=i386 olddefconfig
make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hwmon/
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306150115.i0k7ulfo-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/hwmon/sht3x.c:29:28: warning: 'sht3x_cmd_measure_blocking_lpm' defined but not used [-Wunused-const-variable=]
29 | static const unsigned char sht3x_cmd_measure_blocking_lpm[] = { 0x2c, 0x10 };
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/hwmon/sht3x.c:25:28: warning: 'sht3x_cmd_measure_blocking_hpm' defined but not used [-Wunused-const-variable=]
25 | static const unsigned char sht3x_cmd_measure_blocking_hpm[] = { 0x2c, 0x06 };
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/sht3x_cmd_measure_blocking_lpm +29 drivers/hwmon/sht3x.c
7c84f7f80d6fcea David Frey 2016-06-02 23
cecbab8bdd40311 JuenKit Yip 2023-06-14 24 /* commands (high repeatability mode) */
7c84f7f80d6fcea David Frey 2016-06-02 @25 static const unsigned char sht3x_cmd_measure_blocking_hpm[] = { 0x2c, 0x06 };
7c84f7f80d6fcea David Frey 2016-06-02 26 static const unsigned char sht3x_cmd_measure_nonblocking_hpm[] = { 0x24, 0x00 };
7c84f7f80d6fcea David Frey 2016-06-02 27
cecbab8bdd40311 JuenKit Yip 2023-06-14 28 /* commands (low repeatability mode) */
7c84f7f80d6fcea David Frey 2016-06-02 @29 static const unsigned char sht3x_cmd_measure_blocking_lpm[] = { 0x2c, 0x10 };
7c84f7f80d6fcea David Frey 2016-06-02 30 static const unsigned char sht3x_cmd_measure_nonblocking_lpm[] = { 0x24, 0x16 };
7c84f7f80d6fcea David Frey 2016-06-02 31
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@hotmail.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. > > 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@sensirion.com> >>> - * Author: Pascal Sachs <pascal.sachs@sensirion.com> >>> - */ >>> - >>> -#ifndef __SHT3X_H_ >>> -#define __SHT3X_H_ >>> - >>> -struct sht3x_platform_data { >>> - bool blocking_io; >>> - bool high_precision; >>> -}; >>> -#endif /* __SHT3X_H_ */ >>
在 2023/6/15 3:15, Guenter Roeck 写道: > 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. > I reviewed the datasheet again, update_interval == 0 means Single-Shot mode which owns blocking(clock strench) and non-blocking(non-clock strench) options. If master supports clock-strench( I don't know how to detect it), the property may be reserved. >> 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@hotmail.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. >> >> 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@sensirion.com> >>>> - * Author: Pascal Sachs <pascal.sachs@sensirion.com> >>>> - */ >>>> - >>>> -#ifndef __SHT3X_H_ >>>> -#define __SHT3X_H_ >>>> - >>>> -struct sht3x_platform_data { >>>> - bool blocking_io; >>>> - bool high_precision; >>>> -}; >>>> -#endif /* __SHT3X_H_ */ >>> >
On 6/14/23 16:54, JuenKit Yip wrote: > > 在 2023/6/15 3:15, Guenter Roeck 写道: >> 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. >> > I reviewed the datasheet again, update_interval == 0 means Single-Shot > > mode which owns blocking(clock strench) and non-blocking(non-clock strench) > > options. If master supports clock-strench( I don't know how to detect it), > > the property may be reserved. > I see. In practice, blocking mode was never really used, at least not in the upstream kernel, since platform data was not set from any in-kernel code. Given that it is pretty much untested, I would suggest to always use non-blocking mode. This is only relevant if the chip is in single-shot mode, so worst case the user would have to wait a bit longer for results in that mode. I find that acceptable over the risk involved when trying to use/support blocking mode. Guenter
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; } 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@sensirion.com> - * Author: Pascal Sachs <pascal.sachs@sensirion.com> - */ - -#ifndef __SHT3X_H_ -#define __SHT3X_H_ - -struct sht3x_platform_data { - bool blocking_io; - bool high_precision; -}; -#endif /* __SHT3X_H_ */
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. Signed-off-by: JuenKit Yip <JuenKit_Yip@hotmail.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