| Message ID | 1622789683-30931-1-git-send-email-nmalwade@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | ina3221: use CVRF only for single-shot conversion | expand |
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@nvidia.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, ®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:
Thank You Guenter. Regards, -Ninad. -----Original Message----- From: Guenter Roeck <groeck7@gmail.com> On Behalf Of Guenter Roeck Sent: Friday, June 4, 2021 7:11 PM To: Ninad Malwade <nmalwade@nvidia.com> Cc: Jean Delvare <jdelvare@suse.com>; Bibek Basu <bbasu@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; Rajkumar Kasirajan <rkasirajan@nvidia.com>; linux-hwmon@vger.kernel.org; linux-kernel@vger.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@nvidia.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, ®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:
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:
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. Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> --- drivers/hwmon/ina3221.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)