| Message ID | 20240908192303.151562-5-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | power: supply: Add new "charge_types" property | expand |
Hi, On Sun, Sep 08, 2024 at 09:23:03PM GMT, Hans de Goede wrote: > Make battery_modes a map between tokens and enum power_supply_charge_type > values instead of between tokens and strings and use the new > power_supply_charge_types_show/_parse() helpers for show()/store() > to ensure that things are handled in the same way as in other drivers. > > This also changes battery_supported_modes to be a bitmap of charge-types > (enum power_supply_charge_type values) rather then a bitmap of indices > into battery_modes[]. > > FIXME: not tested yet > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- I quickly skipped over this and it LGTM. Ideally the Extension API will get fully rid of the custom show()/store() :) -- Sebastian > drivers/platform/x86/dell/dell-laptop.c | 54 ++++++++++++------------- > 1 file changed, 25 insertions(+), 29 deletions(-) > > diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c > index a3cd0505f282..d2ef18547c41 100644 > --- a/drivers/platform/x86/dell/dell-laptop.c > +++ b/drivers/platform/x86/dell/dell-laptop.c > @@ -103,15 +103,15 @@ static bool mute_led_registered; > > struct battery_mode_info { > int token; > - const char *label; > + enum power_supply_charge_type charge_type; > }; > > static const struct battery_mode_info battery_modes[] = { > - { BAT_PRI_AC_MODE_TOKEN, "Trickle" }, > - { BAT_EXPRESS_MODE_TOKEN, "Fast" }, > - { BAT_STANDARD_MODE_TOKEN, "Standard" }, > - { BAT_ADAPTIVE_MODE_TOKEN, "Adaptive" }, > - { BAT_CUSTOM_MODE_TOKEN, "Custom" }, > + { BAT_PRI_AC_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_TRICKLE }, > + { BAT_EXPRESS_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_FAST }, > + { BAT_STANDARD_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_STANDARD }, > + { BAT_ADAPTIVE_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE }, > + { BAT_CUSTOM_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_CUSTOM }, > }; > static u32 battery_supported_modes; > > @@ -2261,46 +2261,42 @@ static ssize_t charge_types_show(struct device *dev, > struct device_attribute *attr, > char *buf) > { > - ssize_t count = 0; > + enum power_supply_charge_type charge_type; > int i; > > for (i = 0; i < ARRAY_SIZE(battery_modes); i++) { > - bool active; > + charge_type = battery_modes[i].charge_type; > > - if (!(battery_supported_modes & BIT(i))) > + if (!(battery_supported_modes & BIT(charge_type))) > continue; > > - active = dell_battery_mode_is_active(battery_modes[i].token); > - count += sysfs_emit_at(buf, count, active ? "[%s] " : "%s ", > - battery_modes[i].label); > + if (!dell_battery_mode_is_active(battery_modes[i].token)) > + continue; > + > + return power_supply_charge_types_show(dev, battery_supported_modes, > + charge_type, buf); > } > > - /* convert the last space to a newline */ > - if (count > 0) > - count--; > - count += sysfs_emit_at(buf, count, "\n"); > - > - return count; > + /* No active mode found */ > + return -EIO; > } > > static ssize_t charge_types_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t size) > { > - bool matched = false; > - int err, i; > + int charge_type, err, i; > + > + charge_type = power_supply_charge_types_parse(battery_supported_modes, buf); > + if (charge_type < 0) > + return charge_type; > > for (i = 0; i < ARRAY_SIZE(battery_modes); i++) { > - if (!(battery_supported_modes & BIT(i))) > - continue; > - > - if (sysfs_streq(battery_modes[i].label, buf)) { > - matched = true; > + if (battery_modes[i].charge_type == charge_type) > break; > - } > } > - if (!matched) > - return -EINVAL; > + if (i == ARRAY_SIZE(battery_modes)) > + return -EIO; > > err = dell_battery_set_mode(battery_modes[i].token); > if (err) > @@ -2421,7 +2417,7 @@ static u32 __init battery_get_supported_modes(void) > > for (i = 0; i < ARRAY_SIZE(battery_modes); i++) { > if (dell_smbios_find_token(battery_modes[i].token)) > - modes |= BIT(i); > + modes |= BIT(battery_modes[i].charge_type); > } > > return modes; > -- > 2.46.0 >
diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c index a3cd0505f282..d2ef18547c41 100644 --- a/drivers/platform/x86/dell/dell-laptop.c +++ b/drivers/platform/x86/dell/dell-laptop.c @@ -103,15 +103,15 @@ static bool mute_led_registered; struct battery_mode_info { int token; - const char *label; + enum power_supply_charge_type charge_type; }; static const struct battery_mode_info battery_modes[] = { - { BAT_PRI_AC_MODE_TOKEN, "Trickle" }, - { BAT_EXPRESS_MODE_TOKEN, "Fast" }, - { BAT_STANDARD_MODE_TOKEN, "Standard" }, - { BAT_ADAPTIVE_MODE_TOKEN, "Adaptive" }, - { BAT_CUSTOM_MODE_TOKEN, "Custom" }, + { BAT_PRI_AC_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_TRICKLE }, + { BAT_EXPRESS_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_FAST }, + { BAT_STANDARD_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_STANDARD }, + { BAT_ADAPTIVE_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_ADAPTIVE }, + { BAT_CUSTOM_MODE_TOKEN, POWER_SUPPLY_CHARGE_TYPE_CUSTOM }, }; static u32 battery_supported_modes; @@ -2261,46 +2261,42 @@ static ssize_t charge_types_show(struct device *dev, struct device_attribute *attr, char *buf) { - ssize_t count = 0; + enum power_supply_charge_type charge_type; int i; for (i = 0; i < ARRAY_SIZE(battery_modes); i++) { - bool active; + charge_type = battery_modes[i].charge_type; - if (!(battery_supported_modes & BIT(i))) + if (!(battery_supported_modes & BIT(charge_type))) continue; - active = dell_battery_mode_is_active(battery_modes[i].token); - count += sysfs_emit_at(buf, count, active ? "[%s] " : "%s ", - battery_modes[i].label); + if (!dell_battery_mode_is_active(battery_modes[i].token)) + continue; + + return power_supply_charge_types_show(dev, battery_supported_modes, + charge_type, buf); } - /* convert the last space to a newline */ - if (count > 0) - count--; - count += sysfs_emit_at(buf, count, "\n"); - - return count; + /* No active mode found */ + return -EIO; } static ssize_t charge_types_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t size) { - bool matched = false; - int err, i; + int charge_type, err, i; + + charge_type = power_supply_charge_types_parse(battery_supported_modes, buf); + if (charge_type < 0) + return charge_type; for (i = 0; i < ARRAY_SIZE(battery_modes); i++) { - if (!(battery_supported_modes & BIT(i))) - continue; - - if (sysfs_streq(battery_modes[i].label, buf)) { - matched = true; + if (battery_modes[i].charge_type == charge_type) break; - } } - if (!matched) - return -EINVAL; + if (i == ARRAY_SIZE(battery_modes)) + return -EIO; err = dell_battery_set_mode(battery_modes[i].token); if (err) @@ -2421,7 +2417,7 @@ static u32 __init battery_get_supported_modes(void) for (i = 0; i < ARRAY_SIZE(battery_modes); i++) { if (dell_smbios_find_token(battery_modes[i].token)) - modes |= BIT(i); + modes |= BIT(battery_modes[i].charge_type); } return modes;
Make battery_modes a map between tokens and enum power_supply_charge_type values instead of between tokens and strings and use the new power_supply_charge_types_show/_parse() helpers for show()/store() to ensure that things are handled in the same way as in other drivers. This also changes battery_supported_modes to be a bitmap of charge-types (enum power_supply_charge_type values) rather then a bitmap of indices into battery_modes[]. FIXME: not tested yet Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/platform/x86/dell/dell-laptop.c | 54 ++++++++++++------------- 1 file changed, 25 insertions(+), 29 deletions(-)