| Message ID | 20240204-power_supply-charge_behaviour_prop-v1-3-06a20c958f96@weissschuh.net |
|---|---|
| State | New |
| Headers | show |
| Series | power: supply: core: align charge_behaviour format with docs | expand |
On 4.02.2024 18:26, Thomas Weißschuh wrote: > The sysfs is documented to report both the current and all available > behaviours. For this POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR_AVAILABLE needs > to be implemented. > > Note that this changes the format of the sysfs file > (to the documented format): > > Before: "auto" > After: "[auto] inhibit-charge" > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- LGTM, thanks Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On 5.02.2024 12:21, Thomas Weißschuh wrote: > On 2024-02-05 11:00:01+0100, Hans de Goede wrote: >> Hi, >> >> On 2/4/24 18:26, Thomas Weißschuh wrote: >>> The sysfs is documented to report both the current and all available >>> behaviours. For this POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR_AVAILABLE needs >>> to be implemented. >>> >>> Note that this changes the format of the sysfs file >>> (to the documented format): >>> >>> Before: "auto" >>> After: "[auto] inhibit-charge" >>> >>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> >> >> Changing userspace API like this is never ideal, but given how >> new the mm8013 driver is and that this brings things inline >> with the docs I think that this should be fine: > > I agree that it's unfortunate. > > However looking at the datasheet [0] it seems to me the driver is > not correctly using the API. > > Page 23 documents the flag CHG_INH as follows: > > CHG_INH : Charge Inhibit When the current is more than or equal to charge > threshold current, > charge inhibit temperature (upper/lower limit) ：1 > charge permission temperature or the current is > less than charge threshold current ：0 > > This is only diagnostic information and not a control-knob, which the API > was meant for. > So POWER_SUPPLY_STATUS_NOT_CHARGING seems like the better match. Oh, that's definitely something I glossed over, thanks for taking a look! I'll send a patch untangling this shortly. Konrad > >> [..] > > Thomas > > > [0] https://product.minebeamitsumi.com/en/product/category/ics/battery/fuel_gauge/parts/download/__icsFiles/afieldfile/2023/07/12/1_download_01_12.pdf
diff --git a/drivers/power/supply/mm8013.c b/drivers/power/supply/mm8013.c index caa272b03564..695df8bd6cb0 100644 --- a/drivers/power/supply/mm8013.c +++ b/drivers/power/supply/mm8013.c @@ -72,6 +72,7 @@ static int mm8013_checkdevice(struct mm8013_chip *chip) static enum power_supply_property mm8013_battery_props[] = { POWER_SUPPLY_PROP_CAPACITY, POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR, + POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR_AVAILABLE, POWER_SUPPLY_PROP_CHARGE_FULL, POWER_SUPPLY_PROP_CHARGE_FULL_DESIGN, POWER_SUPPLY_PROP_CHARGE_NOW, @@ -113,6 +114,10 @@ static int mm8013_get_property(struct power_supply *psy, else val->intval = POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO; break; + case POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR_AVAILABLE: + val->intval = BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_AUTO) + | BIT(POWER_SUPPLY_CHARGE_BEHAVIOUR_INHIBIT_CHARGE); + break; case POWER_SUPPLY_PROP_CHARGE_FULL: ret = regmap_read(chip->regmap, REG_FULL_CHARGE_CAPACITY, ®val); if (ret < 0)
The sysfs is documented to report both the current and all available behaviours. For this POWER_SUPPLY_PROP_CHARGE_BEHAVIOUR_AVAILABLE needs to be implemented. Note that this changes the format of the sysfs file (to the documented format): Before: "auto" After: "[auto] inhibit-charge" Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- drivers/power/supply/mm8013.c | 5 +++++ 1 file changed, 5 insertions(+)