| Message ID | 249d7ad42b02bfeb8c31c49a64ee92b3e745086d.1588345420.git.mirq-linux@rere.qmqm.pl (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | power: supply: core: extend with new properties | expand |
Hi, On Fri, May 01, 2020 at 05:11:18PM +0200, Michał Mirosław wrote: > Introduce input voltage and current limits and measurements. > This makes room for e.g. VBUS measurements in USB chargers. We already have properties for charger input voltage/current. Unfortunately the naming is not as straight forward, as it could be. Basically the properties have been added over time and are ABI now. Things are documented in Documentation/ABI/testing/sysfs-class-power I provided the relevant properties below. > Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> [...] > --- a/drivers/power/supply/power_supply_sysfs.c > +++ b/drivers/power/supply/power_supply_sysfs.c > @@ -273,7 +273,9 @@ static struct device_attribute power_supply_attrs[] = { > POWER_SUPPLY_ATTR(charge_control_limit_max), > POWER_SUPPLY_ATTR(charge_control_start_threshold), > POWER_SUPPLY_ATTR(charge_control_end_threshold), > + POWER_SUPPLY_ATTR(input_current_now), > POWER_SUPPLY_ATTR(input_current_limit), > + POWER_SUPPLY_ATTR(input_voltage_now), > POWER_SUPPLY_ATTR(input_voltage_limit), > POWER_SUPPLY_ATTR(input_power_limit), > POWER_SUPPLY_ATTR(energy_full_design), > diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h > index 6a34df65d4d1..5313d1284aad 100644 > --- a/include/linux/power_supply.h > +++ b/include/linux/power_supply.h > @@ -127,7 +127,9 @@ enum power_supply_property { > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX, > POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD, /* in percents! */ > POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, /* in percents! */ > + POWER_SUPPLY_PROP_INPUT_CURRENT_NOW, What: /sys/class/power_supply/<supply_name>/current_avg Date: May 2007 Contact: linux-pm@vger.kernel.org Description: Reports an average IBUS current reading over a fixed period. Normally devices will provide a fixed interval in which they average readings to smooth out the reported value. Access: Read Valid values: Represented in microamps > POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, > + POWER_SUPPLY_PROP_INPUT_VOLTAGE_NOW, What: /sys/class/power_supply/<supply_name>/voltage_now Date: May 2007 Contact: linux-pm@vger.kernel.org Description: Reports the VBUS voltage supplied now. This value is generally read-only reporting, unless the 'online' state of the supply is set to be programmable, in which case this value can be set within the reported min/max range. Access: Read, Write Valid values: Represented in microvolts > POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT, > POWER_SUPPLY_PROP_INPUT_POWER_LIMIT, > POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN, -- Sebastian
On Sun, May 03, 2020 at 12:23:49AM +0200, Sebastian Reichel wrote: > Hi, > > On Fri, May 01, 2020 at 05:11:18PM +0200, Michał Mirosław wrote: > > Introduce input voltage and current limits and measurements. > > This makes room for e.g. VBUS measurements in USB chargers. > We already have properties for charger input voltage/current. > Unfortunately the naming is not as straight forward, as it > could be. Basically the properties have been added over time > and are ABI now. Things are documented in > > Documentation/ABI/testing/sysfs-class-power > > I provided the relevant properties below. Hmm. Looks like there is no battery current/voltage properties then? This is different from IBUS (input current), as IBUS = charging current + system load. Documentation/power/power_supply_class.rst is missing descriptions for the properties you mention. [...] > > --- a/include/linux/power_supply.h > > +++ b/include/linux/power_supply.h > > @@ -127,7 +127,9 @@ enum power_supply_property { > > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX, > > POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD, /* in percents! */ > > POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, /* in percents! */ > > + POWER_SUPPLY_PROP_INPUT_CURRENT_NOW, > > What: /sys/class/power_supply/<supply_name>/current_avg > Date: May 2007 > Contact: linux-pm@vger.kernel.org > Description: > Reports an average IBUS current reading over a fixed period. > Normally devices will provide a fixed interval in which they > average readings to smooth out the reported value. > > Access: Read > Valid values: Represented in microamps > There are two entries for /sys/class/power_supply/<supply_name>/current_avg in the file, the other one mentions IBAT instead. "voltage_now" has the same problem. There seems to be a split-personality disorder present in the kernel ABI. ;-) Best Regards, Michał Mirosław
On Sun, May 03, 2020 at 12:45:26AM +0200, Michał Mirosław wrote: > On Sun, May 03, 2020 at 12:23:49AM +0200, Sebastian Reichel wrote: > > On Fri, May 01, 2020 at 05:11:18PM +0200, Michał Mirosław wrote: [...] > > > --- a/include/linux/power_supply.h > > > +++ b/include/linux/power_supply.h > > > @@ -127,7 +127,9 @@ enum power_supply_property { > > > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX, > > > POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD, /* in percents! */ > > > POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, /* in percents! */ > > > + POWER_SUPPLY_PROP_INPUT_CURRENT_NOW, > > > > What: /sys/class/power_supply/<supply_name>/current_avg > > Date: May 2007 > > Contact: linux-pm@vger.kernel.org > > Description: > > Reports an average IBUS current reading over a fixed period. > > Normally devices will provide a fixed interval in which they > > average readings to smooth out the reported value. > > > > Access: Read > > Valid values: Represented in microamps > > > > There are two entries for /sys/class/power_supply/<supply_name>/current_avg > in the file, the other one mentions IBAT instead. "voltage_now" has the > same problem. [...] So the general idea of the sysfs API seems to require separate devices for the input (charger) and battery elements. Since what I'm looking at is an integrated battery controller (bq25896) which has three connections: an USB power (VBUS), a battery and the system load, but it creates only a single power-class device. This is complicated by the fact that this is an OTG device and so it can sink or source VBUS power. Best Regards, Michał Mirosław
Hi, On Sun, May 03, 2020 at 01:11:58AM +0200, Michał Mirosław wrote: > On Sun, May 03, 2020 at 12:45:26AM +0200, Michał Mirosław wrote: > > On Sun, May 03, 2020 at 12:23:49AM +0200, Sebastian Reichel wrote: > > > On Fri, May 01, 2020 at 05:11:18PM +0200, Michał Mirosław wrote: > [...] > > > > --- a/include/linux/power_supply.h > > > > +++ b/include/linux/power_supply.h > > > > @@ -127,7 +127,9 @@ enum power_supply_property { > > > > POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX, > > > > POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD, /* in percents! */ > > > > POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, /* in percents! */ > > > > + POWER_SUPPLY_PROP_INPUT_CURRENT_NOW, > > > > > > What: /sys/class/power_supply/<supply_name>/current_avg > > > Date: May 2007 > > > Contact: linux-pm@vger.kernel.org > > > Description: > > > Reports an average IBUS current reading over a fixed period. > > > Normally devices will provide a fixed interval in which they > > > average readings to smooth out the reported value. > > > > > > Access: Read > > > Valid values: Represented in microamps > > > > > > > There are two entries for /sys/class/power_supply/<supply_name>/current_avg > > in the file, the other one mentions IBAT instead. "voltage_now" has the > > same problem. > [...] > > So the general idea of the sysfs API seems to require separate devices for the > input (charger) and battery elements. > > Since what I'm looking at is an integrated battery controller > (bq25896) which has three connections: an USB power (VBUS), a > battery and the system load, but it creates only a single > power-class device. power-supply exposes either TYPE_MAINS/TYPE_USB or TYPE_BATTERY. If a device is combined function, then it should register two power-supply devices. > This is complicated by the fact that this is an OTG device and so > it can sink or source VBUS power. Ok. -- Sebastian
diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c index f5d538485aaa..3863b2a73ecf 100644 --- a/drivers/power/supply/power_supply_hwmon.c +++ b/drivers/power/supply/power_supply_hwmon.c @@ -13,12 +13,17 @@ struct power_supply_hwmon { unsigned long *props; }; +static const char *const ps_input_label[] = { + "battery", + "external source", +}; + static const char *const ps_temp_label[] = { "temp", "ambient temp", }; -static int power_supply_hwmon_in_to_property(u32 attr) +static int power_supply_hwmon_in0_to_property(u32 attr) { switch (attr) { case hwmon_in_average: @@ -34,7 +39,31 @@ static int power_supply_hwmon_in_to_property(u32 attr) } } -static int power_supply_hwmon_curr_to_property(u32 attr) +static int power_supply_hwmon_in1_to_property(u32 attr) +{ + switch (attr) { + case hwmon_in_max: + return POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT; + case hwmon_in_input: + return POWER_SUPPLY_PROP_INPUT_VOLTAGE_NOW; + default: + return -EINVAL; + } +} + +static int power_supply_hwmon_in_to_property(u32 attr, int channel) +{ + switch (channel) { + case 0: + return power_supply_hwmon_in0_to_property(attr); + case 1: + return power_supply_hwmon_in1_to_property(attr); + default: + return -EINVAL; + } +} + +static int power_supply_hwmon_curr0_to_property(u32 attr) { switch (attr) { case hwmon_curr_average: @@ -48,6 +77,30 @@ static int power_supply_hwmon_curr_to_property(u32 attr) } } +static int power_supply_hwmon_curr1_to_property(u32 attr) +{ + switch (attr) { + case hwmon_curr_max: + return POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT; + case hwmon_curr_input: + return POWER_SUPPLY_PROP_INPUT_CURRENT_NOW; + default: + return -EINVAL; + } +} + +static int power_supply_hwmon_curr_to_property(u32 attr, int channel) +{ + switch (channel) { + case 0: + return power_supply_hwmon_curr0_to_property(attr); + case 1: + return power_supply_hwmon_curr1_to_property(attr); + default: + return -EINVAL; + } +} + static int power_supply_hwmon_temp_to_property(u32 attr, int channel) { if (channel) { @@ -87,9 +140,9 @@ power_supply_hwmon_to_property(enum hwmon_sensor_types type, { switch (type) { case hwmon_in: - return power_supply_hwmon_in_to_property(attr); + return power_supply_hwmon_in_to_property(attr, channel); case hwmon_curr: - return power_supply_hwmon_curr_to_property(attr); + return power_supply_hwmon_curr_to_property(attr, channel); case hwmon_temp: return power_supply_hwmon_temp_to_property(attr, channel); default: @@ -100,7 +153,9 @@ power_supply_hwmon_to_property(enum hwmon_sensor_types type, static bool power_supply_hwmon_is_a_label(enum hwmon_sensor_types type, u32 attr) { - return type == hwmon_temp && attr == hwmon_temp_label; + return (type == hwmon_temp && attr == hwmon_temp_label) || + (type == hwmon_in && attr == hwmon_in_label) || + (type == hwmon_curr && attr == hwmon_curr_label); } struct hwmon_type_attr_list { @@ -114,7 +169,19 @@ static const u32 ps_temp_attrs[] = { hwmon_temp_min_alarm, hwmon_temp_max_alarm, }; +static const u32 ps_in_attrs[] = { + hwmon_in_input, hwmon_in_average, + hwmon_in_min, hwmon_in_max, +}; + +static const u32 ps_curr_attrs[] = { + hwmon_curr_input, hwmon_curr_average, + hwmon_curr_max, +}; + static const struct hwmon_type_attr_list ps_type_attrs[hwmon_max] = { + [hwmon_in] = { ps_in_attrs, ARRAY_SIZE(ps_in_attrs) }, + [hwmon_curr] = { ps_curr_attrs, ARRAY_SIZE(ps_curr_attrs) }, [hwmon_temp] = { ps_temp_attrs, ARRAY_SIZE(ps_temp_attrs) }, }; @@ -186,6 +253,11 @@ static int power_supply_hwmon_read_string(struct device *dev, const char **str) { switch (type) { + case hwmon_in: + case hwmon_curr: + *str = ps_input_label[channel]; + break; + case hwmon_temp: *str = ps_temp_label[channel]; break; @@ -309,15 +381,26 @@ static const struct hwmon_channel_info *power_supply_hwmon_info[] = { HWMON_T_MAX_ALARM), HWMON_CHANNEL_INFO(curr, + HWMON_C_LABEL | HWMON_C_AVERAGE | HWMON_C_MAX | + HWMON_C_INPUT, + + HWMON_C_LABEL | + HWMON_C_MAX | HWMON_C_INPUT), HWMON_CHANNEL_INFO(in, + HWMON_I_LABEL | HWMON_I_AVERAGE | HWMON_I_MIN | HWMON_I_MAX | + HWMON_I_INPUT, + + HWMON_I_LABEL | + HWMON_I_MAX | HWMON_I_INPUT), + NULL }; @@ -382,6 +465,10 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy) case POWER_SUPPLY_PROP_VOLTAGE_MIN: case POWER_SUPPLY_PROP_VOLTAGE_MAX: case POWER_SUPPLY_PROP_VOLTAGE_NOW: + case POWER_SUPPLY_PROP_INPUT_CURRENT_NOW: + case POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT: + case POWER_SUPPLY_PROP_INPUT_VOLTAGE_NOW: + case POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT: set_bit(prop, psyhw->props); break; default: diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c index 51de3f47b25d..1d1fb69516a8 100644 --- a/drivers/power/supply/power_supply_sysfs.c +++ b/drivers/power/supply/power_supply_sysfs.c @@ -273,7 +273,9 @@ static struct device_attribute power_supply_attrs[] = { POWER_SUPPLY_ATTR(charge_control_limit_max), POWER_SUPPLY_ATTR(charge_control_start_threshold), POWER_SUPPLY_ATTR(charge_control_end_threshold), + POWER_SUPPLY_ATTR(input_current_now), POWER_SUPPLY_ATTR(input_current_limit), + POWER_SUPPLY_ATTR(input_voltage_now), POWER_SUPPLY_ATTR(input_voltage_limit), POWER_SUPPLY_ATTR(input_power_limit), POWER_SUPPLY_ATTR(energy_full_design), diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h index 6a34df65d4d1..5313d1284aad 100644 --- a/include/linux/power_supply.h +++ b/include/linux/power_supply.h @@ -127,7 +127,9 @@ enum power_supply_property { POWER_SUPPLY_PROP_CHARGE_CONTROL_LIMIT_MAX, POWER_SUPPLY_PROP_CHARGE_CONTROL_START_THRESHOLD, /* in percents! */ POWER_SUPPLY_PROP_CHARGE_CONTROL_END_THRESHOLD, /* in percents! */ + POWER_SUPPLY_PROP_INPUT_CURRENT_NOW, POWER_SUPPLY_PROP_INPUT_CURRENT_LIMIT, + POWER_SUPPLY_PROP_INPUT_VOLTAGE_NOW, POWER_SUPPLY_PROP_INPUT_VOLTAGE_LIMIT, POWER_SUPPLY_PROP_INPUT_POWER_LIMIT, POWER_SUPPLY_PROP_ENERGY_FULL_DESIGN,
Introduce input voltage and current limits and measurements. This makes room for e.g. VBUS measurements in USB chargers. Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> --- v2: add parameter checking in power_supply_hwmon_read_string() v3: remove power_supply_hwmon_read_string() parameter checks as it is internal API (suggested by Guenter Roeck) --- drivers/power/supply/power_supply_hwmon.c | 97 +++++++++++++++++++++-- drivers/power/supply/power_supply_sysfs.c | 2 + include/linux/power_supply.h | 2 + 3 files changed, 96 insertions(+), 5 deletions(-)