| Message ID | 20240904-power-supply-extensions-v3-2-62efeb93f8ec@weissschuh.net (mailing list archive) |
|---|---|
| State | Handled Elsewhere, archived |
| Headers | show |
| Series | power: supply: extension API | expand |
Hi, On 9/4/24 9:25 PM, Thomas Weißschuh wrote: > power_supply_read_team() can also read the temperature from the battery. > But currently when registering the thermal zone, the battery is not > checked for POWER_SUPPLY_PROP_TEMP. > Introduce a helper which can check both the desc and the battery info > for property existence and use that. > Export the helper to the rest of the psy core because it will also be > used by different subcomponents. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> Thanks, patch looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > drivers/power/supply/power_supply.h | 3 +++ > drivers/power/supply/power_supply_core.c | 14 +++++++++++++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h > index 3cbafc58bdad..b01faeaf7827 100644 > --- a/drivers/power/supply/power_supply.h > +++ b/drivers/power/supply/power_supply.h > @@ -13,6 +13,9 @@ struct device; > struct device_type; > struct power_supply; > > +extern bool power_supply_has_property(struct power_supply *psy, > + enum power_supply_property psp); > + > #ifdef CONFIG_SYSFS > > extern void power_supply_init_attrs(void); > diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c > index cff68c4fd63c..dcb7e4853030 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -1199,6 +1199,18 @@ static bool psy_desc_has_property(const struct power_supply_desc *psy_desc, > return found; > } > > +bool power_supply_has_property(struct power_supply *psy, > + enum power_supply_property psp) > +{ > + if (psy_desc_has_property(psy->desc, psp)) > + return true; > + > + if (power_supply_battery_info_has_prop(psy->battery_info, psp)) > + return true; > + > + return false; > +} > + > int power_supply_get_property(struct power_supply *psy, > enum power_supply_property psp, > union power_supply_propval *val) > @@ -1308,7 +1320,7 @@ static int psy_register_thermal(struct power_supply *psy) > return 0; > > /* Register battery zone device psy reports temperature */ > - if (psy_desc_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) { > + if (power_supply_has_property(psy, POWER_SUPPLY_PROP_TEMP)) { > /* Prefer our hwmon device and avoid duplicates */ > struct thermal_zone_params tzp = { > .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON) >
Hi, On Wed, Sep 04, 2024 at 09:25:35PM GMT, Thomas Weißschuh wrote: > power_supply_read_team() can also read the temperature from the battery. power_supply_read_temp() > But currently when registering the thermal zone, the battery is > not checked for POWER_SUPPLY_PROP_TEMP. Introduce a helper which > can check both the desc and the battery info for property > existence and use that. Export the helper to the rest of the psy > core because it will also be used by different subcomponents. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- power_supply_battery_info contains constant battery information like design capacity or maximum voltage, which is e.g. supplied by the firmware and needed by fuel-gauges or chargers. The temperature is not constant and cannot be part of power_supply_battery_info, so this does not really make sense. Greetings, -- Sebastian > drivers/power/supply/power_supply.h | 3 +++ > drivers/power/supply/power_supply_core.c | 14 +++++++++++++- > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h > index 3cbafc58bdad..b01faeaf7827 100644 > --- a/drivers/power/supply/power_supply.h > +++ b/drivers/power/supply/power_supply.h > @@ -13,6 +13,9 @@ struct device; > struct device_type; > struct power_supply; > > +extern bool power_supply_has_property(struct power_supply *psy, > + enum power_supply_property psp); > + > #ifdef CONFIG_SYSFS > > extern void power_supply_init_attrs(void); > diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c > index cff68c4fd63c..dcb7e4853030 100644 > --- a/drivers/power/supply/power_supply_core.c > +++ b/drivers/power/supply/power_supply_core.c > @@ -1199,6 +1199,18 @@ static bool psy_desc_has_property(const struct power_supply_desc *psy_desc, > return found; > } > > +bool power_supply_has_property(struct power_supply *psy, > + enum power_supply_property psp) > +{ > + if (psy_desc_has_property(psy->desc, psp)) > + return true; > + > + if (power_supply_battery_info_has_prop(psy->battery_info, psp)) > + return true; > + > + return false; > +} > + > int power_supply_get_property(struct power_supply *psy, > enum power_supply_property psp, > union power_supply_propval *val) > @@ -1308,7 +1320,7 @@ static int psy_register_thermal(struct power_supply *psy) > return 0; > > /* Register battery zone device psy reports temperature */ > - if (psy_desc_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) { > + if (power_supply_has_property(psy, POWER_SUPPLY_PROP_TEMP)) { > /* Prefer our hwmon device and avoid duplicates */ > struct thermal_zone_params tzp = { > .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON) > > -- > 2.46.0 > >
Hi, On 2024-09-14 11:29:04+0000, Sebastian Reichel wrote: > On Wed, Sep 04, 2024 at 09:25:35PM GMT, Thomas Weißschuh wrote: > > power_supply_read_team() can also read the temperature from the battery. > power_supply_read_temp() > > > But currently when registering the thermal zone, the battery is > > not checked for POWER_SUPPLY_PROP_TEMP. Introduce a helper which > > can check both the desc and the battery info for property > > existence and use that. Export the helper to the rest of the psy > > core because it will also be used by different subcomponents. > > > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > > --- > > power_supply_battery_info contains constant battery information like > design capacity or maximum voltage, which is e.g. supplied by the > firmware and needed by fuel-gauges or chargers. The temperature is > not constant and cannot be part of power_supply_battery_info, so > this does not really make sense. Good point. Also this code will run before the extensions will be registered, so it doesn't make sense for that either. I'll change the patch to only introduce power_supply_has_property(). > > Greetings, > > -- Sebastian > > > drivers/power/supply/power_supply.h | 3 +++ > > drivers/power/supply/power_supply_core.c | 14 +++++++++++++- > > 2 files changed, 16 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h > > index 3cbafc58bdad..b01faeaf7827 100644 > > --- a/drivers/power/supply/power_supply.h > > +++ b/drivers/power/supply/power_supply.h > > @@ -13,6 +13,9 @@ struct device; > > struct device_type; > > struct power_supply; > > > > +extern bool power_supply_has_property(struct power_supply *psy, > > + enum power_supply_property psp); > > + > > #ifdef CONFIG_SYSFS > > > > extern void power_supply_init_attrs(void); > > diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c > > index cff68c4fd63c..dcb7e4853030 100644 > > --- a/drivers/power/supply/power_supply_core.c > > +++ b/drivers/power/supply/power_supply_core.c > > @@ -1199,6 +1199,18 @@ static bool psy_desc_has_property(const struct power_supply_desc *psy_desc, > > return found; > > } > > > > +bool power_supply_has_property(struct power_supply *psy, > > + enum power_supply_property psp) > > +{ > > + if (psy_desc_has_property(psy->desc, psp)) > > + return true; > > + > > + if (power_supply_battery_info_has_prop(psy->battery_info, psp)) > > + return true; > > + > > + return false; > > +} > > + > > int power_supply_get_property(struct power_supply *psy, > > enum power_supply_property psp, > > union power_supply_propval *val) > > @@ -1308,7 +1320,7 @@ static int psy_register_thermal(struct power_supply *psy) > > return 0; > > > > /* Register battery zone device psy reports temperature */ > > - if (psy_desc_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) { > > + if (power_supply_has_property(psy, POWER_SUPPLY_PROP_TEMP)) { > > /* Prefer our hwmon device and avoid duplicates */ > > struct thermal_zone_params tzp = { > > .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON) > > > > -- > > 2.46.0
diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h index 3cbafc58bdad..b01faeaf7827 100644 --- a/drivers/power/supply/power_supply.h +++ b/drivers/power/supply/power_supply.h @@ -13,6 +13,9 @@ struct device; struct device_type; struct power_supply; +extern bool power_supply_has_property(struct power_supply *psy, + enum power_supply_property psp); + #ifdef CONFIG_SYSFS extern void power_supply_init_attrs(void); diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c index cff68c4fd63c..dcb7e4853030 100644 --- a/drivers/power/supply/power_supply_core.c +++ b/drivers/power/supply/power_supply_core.c @@ -1199,6 +1199,18 @@ static bool psy_desc_has_property(const struct power_supply_desc *psy_desc, return found; } +bool power_supply_has_property(struct power_supply *psy, + enum power_supply_property psp) +{ + if (psy_desc_has_property(psy->desc, psp)) + return true; + + if (power_supply_battery_info_has_prop(psy->battery_info, psp)) + return true; + + return false; +} + int power_supply_get_property(struct power_supply *psy, enum power_supply_property psp, union power_supply_propval *val) @@ -1308,7 +1320,7 @@ static int psy_register_thermal(struct power_supply *psy) return 0; /* Register battery zone device psy reports temperature */ - if (psy_desc_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) { + if (power_supply_has_property(psy, POWER_SUPPLY_PROP_TEMP)) { /* Prefer our hwmon device and avoid duplicates */ struct thermal_zone_params tzp = { .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON)
power_supply_read_team() can also read the temperature from the battery. But currently when registering the thermal zone, the battery is not checked for POWER_SUPPLY_PROP_TEMP. Introduce a helper which can check both the desc and the battery info for property existence and use that. Export the helper to the rest of the psy core because it will also be used by different subcomponents. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- drivers/power/supply/power_supply.h | 3 +++ drivers/power/supply/power_supply_core.c | 14 +++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-)