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: <DB4PR10MB626134E4213F378A430E5EED925AA@DB4PR10MB6261.EURPRD10.PROD.OUTLOOK.COM>
Date:   Wed, 14 Jun 2023 23:02:46 +0800
From:   JuenKit Yip <JuenKit_Yip@...mail.com>
To:     Guenter Roeck <linux@...ck-us.net>
Cc:     linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
        jdelvare@...e.com
Subject: Re: [PATCH 1/3] hwmon: (sht3x)remove sht3x_platform_data


在 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@...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.

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@...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

Powered by Openwall GNU/*/Linux Powered by OpenVZ