* [PATCH v2 0/2] hwmon: (ina3221) Two changes for INA3221 @ 2019-04-17 23:12 Nicolin Chen 2019-04-17 23:12 ` [PATCH v2 1/2] hwmon: (ina3221) Do not read-back to cache reg_config Nicolin Chen 2019-04-17 23:12 ` [PATCH v2 2/2] hwmon: (ina3221) Add voltage conversion time settings Nicolin Chen 0 siblings, 2 replies; 5+ messages in thread From: Nicolin Chen @ 2019-04-17 23:12 UTC (permalink / raw) To: jdelvare, linux; +Cc: linux-hwmon, linux-kernel, corbet, linux-doc Adding two changes for INA3221 HWMON driver. Changlog v1->v2: * Added PATCH-1 * Rebased PATCH-2 (see detail of changelog in it) Nicolin Chen (2): hwmon: (ina3221) Do not read-back to cache reg_config hwmon: (ina3221) Add voltage conversion time settings Documentation/hwmon/ina3221 | 9 +++++ drivers/hwmon/ina3221.c | 77 +++++++++++++++++++++++++++++-------- 2 files changed, 69 insertions(+), 17 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] hwmon: (ina3221) Do not read-back to cache reg_config 2019-04-17 23:12 [PATCH v2 0/2] hwmon: (ina3221) Two changes for INA3221 Nicolin Chen @ 2019-04-17 23:12 ` Nicolin Chen 2019-04-18 13:37 ` Guenter Roeck 2019-04-17 23:12 ` [PATCH v2 2/2] hwmon: (ina3221) Add voltage conversion time settings Nicolin Chen 1 sibling, 1 reply; 5+ messages in thread From: Nicolin Chen @ 2019-04-17 23:12 UTC (permalink / raw) To: jdelvare, linux; +Cc: linux-hwmon, linux-kernel, corbet, linux-doc Reading back the CONFIG register increases an extra I2C transaction. This's not necessary and could be replaced with a local variable caching the register settings. So this patch replaces two readback regmap_read() calls with a tmp variable. Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- drivers/hwmon/ina3221.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c index 74d39d212931..62040aac653c 100644 --- a/drivers/hwmon/ina3221.c +++ b/drivers/hwmon/ina3221.c @@ -309,21 +309,22 @@ static int ina3221_write_chip(struct device *dev, u32 attr, long val) { struct ina3221_data *ina = dev_get_drvdata(dev); int ret, idx; + u32 tmp; switch (attr) { case hwmon_chip_samples: idx = find_closest(val, ina3221_avg_samples, ARRAY_SIZE(ina3221_avg_samples)); - ret = regmap_update_bits(ina->regmap, INA3221_CONFIG, - INA3221_CONFIG_AVG_MASK, - idx << INA3221_CONFIG_AVG_SHIFT); + tmp = (ina->reg_config & ~INA3221_CONFIG_AVG_MASK) | + (idx << INA3221_CONFIG_AVG_SHIFT); + ret = regmap_write(ina->regmap, INA3221_CONFIG, tmp); if (ret) return ret; /* Update reg_config accordingly */ - return regmap_read(ina->regmap, INA3221_CONFIG, - &ina->reg_config); + ina->reg_config = tmp; + return 0; default: return -EOPNOTSUPP; } @@ -359,6 +360,7 @@ static int ina3221_write_enable(struct device *dev, int channel, bool enable) struct ina3221_data *ina = dev_get_drvdata(dev); u16 config, mask = INA3221_CONFIG_CHx_EN(channel); u16 config_old = ina->reg_config & mask; + u32 tmp; int ret; config = enable ? mask : 0; @@ -377,14 +379,13 @@ static int ina3221_write_enable(struct device *dev, int channel, bool enable) } /* Enable or disable the channel */ - ret = regmap_update_bits(ina->regmap, INA3221_CONFIG, mask, config); + tmp = (ina->reg_config & ~mask) | (config & mask); + ret = regmap_write(ina->regmap, INA3221_CONFIG, tmp); if (ret) goto fail; /* Cache the latest config register value */ - ret = regmap_read(ina->regmap, INA3221_CONFIG, &ina->reg_config); - if (ret) - goto fail; + ina->reg_config = tmp; /* For disabling routine, decrease refcount or suspend() at last */ if (!enable) -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] hwmon: (ina3221) Do not read-back to cache reg_config 2019-04-17 23:12 ` [PATCH v2 1/2] hwmon: (ina3221) Do not read-back to cache reg_config Nicolin Chen @ 2019-04-18 13:37 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2019-04-18 13:37 UTC (permalink / raw) To: Nicolin Chen; +Cc: jdelvare, linux-hwmon, linux-kernel, corbet, linux-doc On Wed, Apr 17, 2019 at 04:12:09PM -0700, Nicolin Chen wrote: > Reading back the CONFIG register increases an extra I2C > transaction. This's not necessary and could be replaced > with a local variable caching the register settings. > > So this patch replaces two readback regmap_read() calls > with a tmp variable. > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> Applied to hwmon-next. Thanks, Guenter ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] hwmon: (ina3221) Add voltage conversion time settings 2019-04-17 23:12 [PATCH v2 0/2] hwmon: (ina3221) Two changes for INA3221 Nicolin Chen 2019-04-17 23:12 ` [PATCH v2 1/2] hwmon: (ina3221) Do not read-back to cache reg_config Nicolin Chen @ 2019-04-17 23:12 ` Nicolin Chen 2019-04-18 13:45 ` Guenter Roeck 1 sibling, 1 reply; 5+ messages in thread From: Nicolin Chen @ 2019-04-17 23:12 UTC (permalink / raw) To: jdelvare, linux; +Cc: linux-hwmon, linux-kernel, corbet, linux-doc The CONFIG register has two 3-bit fields for conversion time settings of Bus-voltage and Shunt-voltage, respectively. The conversion settings, along with averaging mode, allow users to optimize available timing requirement. This patch adds an 'update_interval' sysfs node through the hwmon_chip_info of hwmon core. It reflects a total hardware conversion time: samples * channels * (Bus + Shunt conversion times) Though INA3221 supports different conversion time setups for Bus and Shunt voltages, this patch only adds the support of a unified setting for both conversion times, by dividing the conversion time into two equal values. Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> --- Changelog v1->v2: * Merged two adjacent calls of regmap_update_bits(). * Replaced CONFIG register read-back with tmp variable Documentation/hwmon/ina3221 | 9 ++++++ drivers/hwmon/ina3221.c | 58 ++++++++++++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 8 deletions(-) diff --git a/Documentation/hwmon/ina3221 b/Documentation/hwmon/ina3221 index ed3f22769d4b..3b05170112f0 100644 --- a/Documentation/hwmon/ina3221 +++ b/Documentation/hwmon/ina3221 @@ -38,3 +38,12 @@ in[456]_input Shunt voltage(uV) for channels 1, 2, and 3 respectively samples Number of samples using in the averaging mode. Supports the list of number of samples: 1, 4, 16, 64, 128, 256, 512, 1024 +update_interval Data conversion time in millisecond, following: + update_interval = C x S x (BC + SC) + C: number of enabled channels + S: number of samples + BC: bus-voltage conversion time in millisecond + SC: shunt-voltage conversion time in millisecond + Affects both Bus- and Shunt-voltage conversion time. + Note that setting update_interval to 0ms sets both BC + and SC to 140 us (minimum conversion time). diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c index 62040aac653c..e0637fed9585 100644 --- a/drivers/hwmon/ina3221.c +++ b/drivers/hwmon/ina3221.c @@ -144,19 +144,37 @@ static const int ina3221_avg_samples[] = { 1, 4, 16, 64, 128, 256, 512, 1024, }; -static inline int ina3221_wait_for_data(struct ina3221_data *ina) +/* Converting update_interval in msec to conversion time in usec */ +static inline u32 ina3221_interval_ms_to_conv_time(u16 config, int interval) +{ + u32 channels = hweight16(config & INA3221_CONFIG_CHs_EN_MASK); + u32 samples_idx = INA3221_CONFIG_AVG(config); + u32 samples = ina3221_avg_samples[samples_idx]; + + /* Bisect the result to Bus and Shunt conversion times */ + return DIV_ROUND_CLOSEST(interval * 1000 / 2, channels * samples); +} + +/* Converting CONFIG register value to update_interval in usec */ +static inline u32 ina3221_reg_to_interval_us(u16 config) { - u32 channels = hweight16(ina->reg_config & INA3221_CONFIG_CHs_EN_MASK); - u32 vbus_ct_idx = INA3221_CONFIG_VBUS_CT(ina->reg_config); - u32 vsh_ct_idx = INA3221_CONFIG_VSH_CT(ina->reg_config); - u32 samples_idx = INA3221_CONFIG_AVG(ina->reg_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]; - u32 wait, cvrf; /* Calculate total conversion time */ - wait = channels * (vbus_ct + vsh_ct) * samples; + return channels * (vbus_ct + vsh_ct) * samples; +} + +static inline int ina3221_wait_for_data(struct ina3221_data *ina) +{ + u32 wait, cvrf; + + wait = ina3221_reg_to_interval_us(ina->reg_config); /* Polling the CVRF bit to make sure read data is ready */ return regmap_field_read_poll_timeout(ina->fields[F_CVRF], @@ -197,6 +215,11 @@ static int ina3221_read_chip(struct device *dev, u32 attr, long *val) regval = INA3221_CONFIG_AVG(ina->reg_config); *val = ina3221_avg_samples[regval]; return 0; + case hwmon_chip_update_interval: + /* Return in msec */ + *val = ina3221_reg_to_interval_us(ina->reg_config); + *val = DIV_ROUND_CLOSEST(*val, 1000); + return 0; default: return -EOPNOTSUPP; } @@ -322,6 +345,23 @@ static int ina3221_write_chip(struct device *dev, u32 attr, long val) if (ret) return ret; + /* Update reg_config accordingly */ + ina->reg_config = tmp; + return 0; + case hwmon_chip_update_interval: + tmp = ina3221_interval_ms_to_conv_time(ina->reg_config, val); + idx = find_closest(tmp, ina3221_conv_time, + ARRAY_SIZE(ina3221_conv_time)); + + /* Update Bus and Shunt voltage conversion times */ + tmp = INA3221_CONFIG_VBUS_CT_MASK | INA3221_CONFIG_VSH_CT_MASK; + tmp = (ina->reg_config & ~tmp) | + (idx << INA3221_CONFIG_VBUS_CT_SHIFT) | + (idx << INA3221_CONFIG_VSH_CT_SHIFT); + ret = regmap_write(ina->regmap, INA3221_CONFIG, tmp); + if (ret) + return ret; + /* Update reg_config accordingly */ ina->reg_config = tmp; return 0; @@ -483,6 +523,7 @@ static umode_t ina3221_is_visible(const void *drvdata, case hwmon_chip: switch (attr) { case hwmon_chip_samples: + case hwmon_chip_update_interval: return 0644; default: return 0; @@ -528,7 +569,8 @@ static umode_t ina3221_is_visible(const void *drvdata, static const struct hwmon_channel_info *ina3221_info[] = { HWMON_CHANNEL_INFO(chip, - HWMON_C_SAMPLES), + HWMON_C_SAMPLES, + HWMON_C_UPDATE_INTERVAL), HWMON_CHANNEL_INFO(in, /* 0: dummy, skipped in is_visible */ HWMON_I_INPUT, -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] hwmon: (ina3221) Add voltage conversion time settings 2019-04-17 23:12 ` [PATCH v2 2/2] hwmon: (ina3221) Add voltage conversion time settings Nicolin Chen @ 2019-04-18 13:45 ` Guenter Roeck 0 siblings, 0 replies; 5+ messages in thread From: Guenter Roeck @ 2019-04-18 13:45 UTC (permalink / raw) To: Nicolin Chen; +Cc: jdelvare, linux-hwmon, linux-kernel, corbet, linux-doc On Wed, Apr 17, 2019 at 04:12:10PM -0700, Nicolin Chen wrote: > The CONFIG register has two 3-bit fields for conversion time > settings of Bus-voltage and Shunt-voltage, respectively. The > conversion settings, along with averaging mode, allow users > to optimize available timing requirement. > > This patch adds an 'update_interval' sysfs node through the > hwmon_chip_info of hwmon core. It reflects a total hardware > conversion time: > samples * channels * (Bus + Shunt conversion times) > > Though INA3221 supports different conversion time setups for > Bus and Shunt voltages, this patch only adds the support of > a unified setting for both conversion times, by dividing the > conversion time into two equal values. > > Signed-off-by: Nicolin Chen <nicoleotsuka@gmail.com> Applied to -next. I had to update the documentation for the .rst formatting changes. Hope I got it right. Thanks, Guenter > --- > Changelog > v1->v2: > * Merged two adjacent calls of regmap_update_bits(). > * Replaced CONFIG register read-back with tmp variable > > Documentation/hwmon/ina3221 | 9 ++++++ > drivers/hwmon/ina3221.c | 58 ++++++++++++++++++++++++++++++++----- > 2 files changed, 59 insertions(+), 8 deletions(-) > > diff --git a/Documentation/hwmon/ina3221 b/Documentation/hwmon/ina3221 > index ed3f22769d4b..3b05170112f0 100644 > --- a/Documentation/hwmon/ina3221 > +++ b/Documentation/hwmon/ina3221 > @@ -38,3 +38,12 @@ in[456]_input Shunt voltage(uV) for channels 1, 2, and 3 respectively > samples Number of samples using in the averaging mode. > Supports the list of number of samples: > 1, 4, 16, 64, 128, 256, 512, 1024 > +update_interval Data conversion time in millisecond, following: > + update_interval = C x S x (BC + SC) > + C: number of enabled channels > + S: number of samples > + BC: bus-voltage conversion time in millisecond > + SC: shunt-voltage conversion time in millisecond > + Affects both Bus- and Shunt-voltage conversion time. > + Note that setting update_interval to 0ms sets both BC > + and SC to 140 us (minimum conversion time). > diff --git a/drivers/hwmon/ina3221.c b/drivers/hwmon/ina3221.c > index 62040aac653c..e0637fed9585 100644 > --- a/drivers/hwmon/ina3221.c > +++ b/drivers/hwmon/ina3221.c > @@ -144,19 +144,37 @@ static const int ina3221_avg_samples[] = { > 1, 4, 16, 64, 128, 256, 512, 1024, > }; > > -static inline int ina3221_wait_for_data(struct ina3221_data *ina) > +/* Converting update_interval in msec to conversion time in usec */ > +static inline u32 ina3221_interval_ms_to_conv_time(u16 config, int interval) > +{ > + u32 channels = hweight16(config & INA3221_CONFIG_CHs_EN_MASK); > + u32 samples_idx = INA3221_CONFIG_AVG(config); > + u32 samples = ina3221_avg_samples[samples_idx]; > + > + /* Bisect the result to Bus and Shunt conversion times */ > + return DIV_ROUND_CLOSEST(interval * 1000 / 2, channels * samples); > +} > + > +/* Converting CONFIG register value to update_interval in usec */ > +static inline u32 ina3221_reg_to_interval_us(u16 config) > { > - u32 channels = hweight16(ina->reg_config & INA3221_CONFIG_CHs_EN_MASK); > - u32 vbus_ct_idx = INA3221_CONFIG_VBUS_CT(ina->reg_config); > - u32 vsh_ct_idx = INA3221_CONFIG_VSH_CT(ina->reg_config); > - u32 samples_idx = INA3221_CONFIG_AVG(ina->reg_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]; > - u32 wait, cvrf; > > /* Calculate total conversion time */ > - wait = channels * (vbus_ct + vsh_ct) * samples; > + return channels * (vbus_ct + vsh_ct) * samples; > +} > + > +static inline int ina3221_wait_for_data(struct ina3221_data *ina) > +{ > + u32 wait, cvrf; > + > + wait = ina3221_reg_to_interval_us(ina->reg_config); > > /* Polling the CVRF bit to make sure read data is ready */ > return regmap_field_read_poll_timeout(ina->fields[F_CVRF], > @@ -197,6 +215,11 @@ static int ina3221_read_chip(struct device *dev, u32 attr, long *val) > regval = INA3221_CONFIG_AVG(ina->reg_config); > *val = ina3221_avg_samples[regval]; > return 0; > + case hwmon_chip_update_interval: > + /* Return in msec */ > + *val = ina3221_reg_to_interval_us(ina->reg_config); > + *val = DIV_ROUND_CLOSEST(*val, 1000); > + return 0; > default: > return -EOPNOTSUPP; > } > @@ -322,6 +345,23 @@ static int ina3221_write_chip(struct device *dev, u32 attr, long val) > if (ret) > return ret; > > + /* Update reg_config accordingly */ > + ina->reg_config = tmp; > + return 0; > + case hwmon_chip_update_interval: > + tmp = ina3221_interval_ms_to_conv_time(ina->reg_config, val); > + idx = find_closest(tmp, ina3221_conv_time, > + ARRAY_SIZE(ina3221_conv_time)); > + > + /* Update Bus and Shunt voltage conversion times */ > + tmp = INA3221_CONFIG_VBUS_CT_MASK | INA3221_CONFIG_VSH_CT_MASK; > + tmp = (ina->reg_config & ~tmp) | > + (idx << INA3221_CONFIG_VBUS_CT_SHIFT) | > + (idx << INA3221_CONFIG_VSH_CT_SHIFT); > + ret = regmap_write(ina->regmap, INA3221_CONFIG, tmp); > + if (ret) > + return ret; > + > /* Update reg_config accordingly */ > ina->reg_config = tmp; > return 0; > @@ -483,6 +523,7 @@ static umode_t ina3221_is_visible(const void *drvdata, > case hwmon_chip: > switch (attr) { > case hwmon_chip_samples: > + case hwmon_chip_update_interval: > return 0644; > default: > return 0; > @@ -528,7 +569,8 @@ static umode_t ina3221_is_visible(const void *drvdata, > > static const struct hwmon_channel_info *ina3221_info[] = { > HWMON_CHANNEL_INFO(chip, > - HWMON_C_SAMPLES), > + HWMON_C_SAMPLES, > + HWMON_C_UPDATE_INTERVAL), > HWMON_CHANNEL_INFO(in, > /* 0: dummy, skipped in is_visible */ > HWMON_I_INPUT, ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-18 13:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-17 23:12 [PATCH v2 0/2] hwmon: (ina3221) Two changes for INA3221 Nicolin Chen 2019-04-17 23:12 ` [PATCH v2 1/2] hwmon: (ina3221) Do not read-back to cache reg_config Nicolin Chen 2019-04-18 13:37 ` Guenter Roeck 2019-04-17 23:12 ` [PATCH v2 2/2] hwmon: (ina3221) Add voltage conversion time settings Nicolin Chen 2019-04-18 13:45 ` Guenter Roeck
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).