linux-kernel - RE: [PATCH] ina3221: use CVRF only for single-shot conversion

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] [day] [month] [year] [list]

Message-ID: <BN9PR12MB5129C8D9DE7A35BBC3B6C4E3B13B9@BN9PR12MB5129.namprd12.prod.outlook.com>
Date:   Fri, 4 Jun 2021 15:27:30 +0000
From:   Ninad Malwade <nmalwade@...dia.com>
To:     Guenter Roeck <linux@...ck-us.net>
CC:     Jean Delvare <jdelvare@...e.com>, Bibek Basu <bbasu@...dia.com>,
        Nicolin Chen <nicolinc@...dia.com>,
        Rajkumar Kasirajan <rkasirajan@...dia.com>,
        "linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] ina3221: use CVRF only for single-shot conversion

Thank You Guenter.

Regards,
-Ninad.

-----Original Message-----
From: Guenter Roeck <groeck7@...il.com> On Behalf Of Guenter Roeck
Sent: Friday, June 4, 2021 7:11 PM
To: Ninad Malwade <nmalwade@...dia.com>
Cc: Jean Delvare <jdelvare@...e.com>; Bibek Basu <bbasu@...dia.com>; Nicolin Chen <nicolinc@...dia.com>; Rajkumar Kasirajan <rkasirajan@...dia.com>; linux-hwmon@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ina3221: use CVRF only for single-shot conversion

External email: Use caution opening links or attachments


On Fri, Jun 04, 2021 at 02:54:43PM +0800, Ninad Malwade wrote:
> As per current logic the wait time per conversion is arouns 430ms for 
> 512 samples and around 860ms for 1024 samples for 3 channels 
> considering 140us as the bus voltage and shunt voltage sampling 
> conversion time.
>
> This waiting time is a lot for the continuous mode and even for the 
> single shot mode. For continuous mode when moving average is 
> considered the waiting for CVRF bit is not required and the data from 
> the previous conversion is sufficuent. As mentioned in the datasheet 
> the conversion ready bit is provided to help coordinate single-shot 
> conversions, we can restrict the use to single-shot mode only.
>
> Also, the conversion time is for the averaged samples, the wait time 
> for the polling can omit the number of samples consideration.
>
Makes sense. Applied.

Thanks,
Guenter

> Signed-off-by: Ninad Malwade <nmalwade@...dia.com>
> ---
>  drivers/hwmon/ina3221.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c index 
> c602583..58d3828 100644
> --- a/drivers/hwmon/ina3221.c
> +++ b/drivers/hwmon/ina3221.c
> @@ -196,13 +196,11 @@ static inline u32 ina3221_reg_to_interval_us(u16 config)
>       u32 channels = hweight16(config & INA3221_CONFIG_CHs_EN_MASK);
>       u32 vbus_ct_idx = INA3221_CONFIG_VBUS_CT(config);
>       u32 vsh_ct_idx = INA3221_CONFIG_VSH_CT(config);
> -     u32 samples_idx = INA3221_CONFIG_AVG(config);
> -     u32 samples = ina3221_avg_samples[samples_idx];
>       u32 vbus_ct = ina3221_conv_time[vbus_ct_idx];
>       u32 vsh_ct = ina3221_conv_time[vsh_ct_idx];
>
>       /* Calculate total conversion time */
> -     return channels * (vbus_ct + vsh_ct) * samples;
> +     return channels * (vbus_ct + vsh_ct);
>  }
>
>  static inline int ina3221_wait_for_data(struct ina3221_data *ina) @@ 
> -288,13 +286,14 @@ static int ina3221_read_in(struct device *dev, u32 attr, int channel, long *val)
>                       return -ENODATA;
>
>               /* Write CONFIG register to trigger a single-shot measurement */
> -             if (ina->single_shot)
> +             if (ina->single_shot) {
>                       regmap_write(ina->regmap, INA3221_CONFIG,
>                                    ina->reg_config);
>
> -             ret = ina3221_wait_for_data(ina);
> -             if (ret)
> -                     return ret;
> +                     ret = ina3221_wait_for_data(ina);
> +                     if (ret)
> +                             return ret;
> +             }
>
>               ret = ina3221_read_value(ina, reg, &regval);
>               if (ret)
> @@ -344,13 +343,14 @@ static int ina3221_read_curr(struct device *dev, u32 attr,
>                       return -ENODATA;
>
>               /* Write CONFIG register to trigger a single-shot measurement */
> -             if (ina->single_shot)
> +             if (ina->single_shot) {
>                       regmap_write(ina->regmap, INA3221_CONFIG,
>                                    ina->reg_config);
>
> -             ret = ina3221_wait_for_data(ina);
> -             if (ret)
> -                     return ret;
> +                     ret = ina3221_wait_for_data(ina);
> +                     if (ret)
> +                             return ret;
> +             }
>
>               fallthrough;
>       case hwmon_curr_crit: