Re: [PATCH 2/3] hwmon: (sht3x) add medium repeatability support

[JuenKit Yip]

在 2023/6/14 21:00, Guenter Roeck 写道:
> On 6/13/23 23:27, JuenKit Yip wrote:
> > add medium repeatability support for matching datasheet
> >
> > Signed-off-by: JuenKit Yip <JuenKit_Yip@xxxxxxxxxxx>
> > ---
> >   drivers/hwmon/sht3x.c | 29 +++++++++++++++++++++++++++--
> >   1 file changed, 27 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/hwmon/sht3x.c b/drivers/hwmon/sht3x.c
> > index 5bc0001b1..eb968b9d3 100644
> > --- a/drivers/hwmon/sht3x.c
> > +++ b/drivers/hwmon/sht3x.c
> > @@ -25,6 +25,10 @@
> >   static const unsigned char sht3x_cmd_measure_blocking_hpm[] = { 
> > 0x2c, 0x06 };
> >   static const unsigned char sht3x_cmd_measure_nonblocking_hpm[] = { 
> > 0x24, 0x00 };
> >   +/* commands (medium repeatability mode) */
> > +static const unsigned char sht3x_cmd_measure_blocking_mpm[] = { 
> > 0x2c, 0x0d };
> > +static const unsigned char sht3x_cmd_measure_nonblocking_mpm[] = { 
> > 0x24, 0x0b };
> > +
> >   /* 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 };
> > @@ -43,6 +47,7 @@ static const unsigned char 
> > sht3x_cmd_clear_status_reg[]        = { 0x30, 0x41 };
> >     /* delays for non-blocking i2c commands, both in us */
> >   #define SHT3X_NONBLOCKING_WAIT_TIME_HPM  15000
> > +#define SHT3X_NONBLOCKING_WAIT_TIME_MPM   6000
> >   #define SHT3X_NONBLOCKING_WAIT_TIME_LPM   4000
> >     #define SHT3X_WORD_LEN         2
> > @@ -69,8 +74,9 @@ enum sht3x_limits {
> >   };
> >     enum sht3x_repeatability {
> > -    low_repeatability,
> > -    high_repeatability,
> > +    low_repeatability    = 0,
> > +    medium_repeatability = 1,
> > +    high_repeatability   = 2,
>
> Why the assignments ?
Maybe it won't be needed.
> >   };
> >     DECLARE_CRC8_TABLE(sht3x_crc8_table);
> > @@ -89,6 +95,20 @@ static const char 
> > periodic_measure_commands_hpm[][SHT3X_CMD_LENGTH] = {
> >       {0x27, 0x37},
> >   };
> >   +/* periodic measure commands (medium repeatability) */
> > +static const char periodic_measure_commands_mpm[][SHT3X_CMD_LENGTH] = {
>
> FWIW, it might make sense to add the command for blocking mode
> (0 measurements per second) into this table to simplify the code.
I am trying to do it and review the effects.
> > +    /* 0.5 measurements per second */
> > +    {0x20, 0x24},
> > +    /* 1 measurements per second */
> > +    {0x21, 0x26},
> > +    /* 2 measurements per second */
> > +    {0x22, 0x20},
> > +    /* 4 measurements per second */
> > +    {0x23, 0x22},
> > +    /* 10 measurements per second */
> > +    {0x27, 0x21},
> > +};
> > +
> >   /* periodic measure commands (low repeatability mode) */
> >   static const char periodic_measure_commands_lpm[][SHT3X_CMD_LENGTH] 
> > = {
> >       /* 0.5 measurements per second */
> > @@ -448,6 +468,9 @@ static void sht3x_select_command(struct 
> > sht3x_data *data)
> >           if (data->repeatability == high_repeatability) {
> >               data->command = sht3x_cmd_measure_nonblocking_hpm;
> >               data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_HPM;
> > +        } else if (data->repeatability == medium_repeatability) {
> > +            data->command = sht3x_cmd_measure_nonblocking_mpm;
> > +            data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_MPM;
> >           } else {
> >               data->command = sht3x_cmd_measure_nonblocking_lpm;
> >               data->wait_time = SHT3X_NONBLOCKING_WAIT_TIME_LPM;
> > @@ -595,6 +618,8 @@ static ssize_t update_interval_store(struct 
> > device *dev,
> >       if (mode > 0) {
> >           if (data->repeatability == high_repeatability)
> >               command = periodic_measure_commands_hpm[mode - 1];
> > +        else if (data->repeatability == medium_repeatability)
> > +            command = periodic_measure_commands_mpm[mode - 1];
> >           else
> >               command = periodic_measure_commands_lpm[mode - 1];