Thank You Guenter.
Regards,
-Ninad.
-----Original Message-----
From: Guenter Roeck <groeck7@xxxxxxxxx> On Behalf Of Guenter Roeck
Sent: Friday, June 4, 2021 7:11 PM
To: Ninad Malwade <nmalwade@xxxxxxxxxx>
Cc: Jean Delvare <jdelvare@xxxxxxxx>; Bibek Basu <bbasu@xxxxxxxxxx>; Nicolin Chen <nicolinc@xxxxxxxxxx>; Rajkumar Kasirajan <rkasirajan@xxxxxxxxxx>; linux-hwmon@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
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@xxxxxxxxxx>
> ---
> 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, ®val);
> 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:
