[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <sl55t3ds2lak53pe6omee25vt7zdtnzl2j24ec54lui42w47jt@urbvvqmns5nv>
Date: Thu, 4 Jul 2024 12:42:57 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Armin Wolf <W_Armin@....de>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v2 2/5] power: supply: core: avoid iterating
properties directly
Hi,
On Sat, Jun 08, 2024 at 09:19:38PM GMT, Thomas Weißschuh wrote:
> With the introduction of power supply extension, it will not be enough
> to iterate the properties on the struct power_supply directly.
> Instead introduce a helper power_supply_has_property() which will handle
> properties added by extensions.
power_supply_has_property() needs to include properties from
battery_info, which are exposed in addition to the driver's
own properties.
Greetings,
-- Sebastian
>
> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> ---
> drivers/power/supply/power_supply.h | 3 ++
> drivers/power/supply/power_supply_core.c | 10 +++----
> drivers/power/supply/power_supply_hwmon.c | 48 +++++++++++++++----------------
> drivers/power/supply/power_supply_sysfs.c | 14 +++------
> 4 files changed, 36 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h
> index 3cbafc58bdad..622be1f0a180 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(const struct power_supply_desc *psy_desc,
> + 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 fefe938c9342..d57ecdd966e0 100644
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1183,8 +1183,8 @@ bool power_supply_battery_bti_in_range(struct power_supply_battery_info *info,
> }
> EXPORT_SYMBOL_GPL(power_supply_battery_bti_in_range);
>
> -static bool psy_has_property(const struct power_supply_desc *psy_desc,
> - enum power_supply_property psp)
> +bool power_supply_has_property(const struct power_supply_desc *psy_desc,
> + enum power_supply_property psp)
> {
> bool found = false;
> int i;
> @@ -1209,7 +1209,7 @@ int power_supply_get_property(struct power_supply *psy,
> return -ENODEV;
> }
>
> - if (psy_has_property(psy->desc, psp))
> + if (power_supply_has_property(psy->desc, psp))
> return psy->desc->get_property(psy, psp, val);
> else if (power_supply_battery_info_has_prop(psy->battery_info, psp))
> return power_supply_battery_info_get_prop(psy->battery_info, psp, val);
> @@ -1308,7 +1308,7 @@ static int psy_register_thermal(struct power_supply *psy)
> return 0;
>
> /* Register battery zone device psy reports temperature */
> - if (psy_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) {
> + if (power_supply_has_property(psy->desc, POWER_SUPPLY_PROP_TEMP)) {
> /* Prefer our hwmon device and avoid duplicates */
> struct thermal_zone_params tzp = {
> .no_hwmon = IS_ENABLED(CONFIG_POWER_SUPPLY_HWMON)
> @@ -1361,7 +1361,7 @@ __power_supply_register(struct device *parent,
> pr_warn("%s: Expected proper parent device for '%s'\n",
> __func__, desc->name);
>
> - if (psy_has_property(desc, POWER_SUPPLY_PROP_USB_TYPE) &&
> + if (power_supply_has_property(desc, POWER_SUPPLY_PROP_USB_TYPE) &&
> (!desc->usb_types || !desc->num_usb_types))
> return ERR_PTR(-EINVAL);
>
> diff --git a/drivers/power/supply/power_supply_hwmon.c b/drivers/power/supply/power_supply_hwmon.c
> index c97893d4c25e..2ecbe4a74c25 100644
> --- a/drivers/power/supply/power_supply_hwmon.c
> +++ b/drivers/power/supply/power_supply_hwmon.c
> @@ -8,6 +8,8 @@
> #include <linux/power_supply.h>
> #include <linux/slab.h>
>
> +#include "power_supply.h"
> +
> struct power_supply_hwmon {
> struct power_supply *psy;
> unsigned long *props;
> @@ -324,9 +326,26 @@ static const struct hwmon_chip_info power_supply_hwmon_chip_info = {
> .info = power_supply_hwmon_info,
> };
>
> +static const enum power_supply_property power_supply_hwmon_props[] = {
> + POWER_SUPPLY_PROP_CURRENT_AVG,
> + POWER_SUPPLY_PROP_CURRENT_MAX,
> + POWER_SUPPLY_PROP_CURRENT_NOW,
> + POWER_SUPPLY_PROP_TEMP,
> + POWER_SUPPLY_PROP_TEMP_MAX,
> + POWER_SUPPLY_PROP_TEMP_MIN,
> + POWER_SUPPLY_PROP_TEMP_ALERT_MIN,
> + POWER_SUPPLY_PROP_TEMP_ALERT_MAX,
> + POWER_SUPPLY_PROP_TEMP_AMBIENT,
> + POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN,
> + POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX,
> + POWER_SUPPLY_PROP_VOLTAGE_AVG,
> + POWER_SUPPLY_PROP_VOLTAGE_MIN,
> + POWER_SUPPLY_PROP_VOLTAGE_MAX,
> + POWER_SUPPLY_PROP_VOLTAGE_NOW,
> +};
> +
> int power_supply_add_hwmon_sysfs(struct power_supply *psy)
> {
> - const struct power_supply_desc *desc = psy->desc;
> struct power_supply_hwmon *psyhw;
> struct device *dev = &psy->dev;
> struct device *hwmon;
> @@ -352,30 +371,11 @@ int power_supply_add_hwmon_sysfs(struct power_supply *psy)
> goto error;
> }
>
> - for (i = 0; i < desc->num_properties; i++) {
> - const enum power_supply_property prop = desc->properties[i];
> -
> - switch (prop) {
> - case POWER_SUPPLY_PROP_CURRENT_AVG:
> - case POWER_SUPPLY_PROP_CURRENT_MAX:
> - case POWER_SUPPLY_PROP_CURRENT_NOW:
> - case POWER_SUPPLY_PROP_TEMP:
> - case POWER_SUPPLY_PROP_TEMP_MAX:
> - case POWER_SUPPLY_PROP_TEMP_MIN:
> - case POWER_SUPPLY_PROP_TEMP_ALERT_MIN:
> - case POWER_SUPPLY_PROP_TEMP_ALERT_MAX:
> - case POWER_SUPPLY_PROP_TEMP_AMBIENT:
> - case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MIN:
> - case POWER_SUPPLY_PROP_TEMP_AMBIENT_ALERT_MAX:
> - case POWER_SUPPLY_PROP_VOLTAGE_AVG:
> - case POWER_SUPPLY_PROP_VOLTAGE_MIN:
> - case POWER_SUPPLY_PROP_VOLTAGE_MAX:
> - case POWER_SUPPLY_PROP_VOLTAGE_NOW:
> + for (i = 0; i < ARRAY_SIZE(power_supply_hwmon_props); i++) {
> + const enum power_supply_property prop = power_supply_hwmon_props[i];
> +
> + if (power_supply_has_property(psy->desc, prop))
> set_bit(prop, psyhw->props);
> - break;
> - default:
> - break;
> - }
> }
>
> name = psy->desc->name;
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 3e63d165b2f7..abd44ebfe6fe 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -367,7 +367,6 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj,
> struct device *dev = kobj_to_dev(kobj);
> struct power_supply *psy = dev_get_drvdata(dev);
> umode_t mode = S_IRUSR | S_IRGRP | S_IROTH;
> - int i;
>
> if (!power_supply_attrs[attrno].prop_name)
> return 0;
> @@ -375,15 +374,10 @@ static umode_t power_supply_attr_is_visible(struct kobject *kobj,
> if (attrno == POWER_SUPPLY_PROP_TYPE)
> return mode;
>
> - for (i = 0; i < psy->desc->num_properties; i++) {
> - int property = psy->desc->properties[i];
> -
> - if (property == attrno) {
> - if (power_supply_property_is_writeable(psy, property) > 0)
> - mode |= S_IWUSR;
> -
> - return mode;
> - }
> + if (power_supply_has_property(psy->desc, attrno)) {
> + if (power_supply_property_is_writeable(psy, attrno) > 0)
> + mode |= S_IWUSR;
> + return mode;
> }
>
> if (power_supply_battery_info_has_prop(psy->battery_info, attrno))
>
> --
> 2.45.2
>
>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists