[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e2ed8a03-7973-4ab9-9b9d-d1c23616de86@redhat.com>
Date: Wed, 4 Sep 2024 21:58:59 +0200
From: Hans de Goede <hdegoede@...hat.com>
To: Thomas Weißschuh <linux@...ssschuh.net>,
Sebastian Reichel <sre@...nel.org>, Armin Wolf <W_Armin@....de>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC v3 5/9] power: supply: sysfs: rework uevent property
loop
Hi,
On 9/4/24 9:25 PM, Thomas Weißschuh wrote:
> Instead of looping through all properties known to be supported by the
> psy, loop over all known properties and decide based on the return value
> of power_supply_get_property() whether the property existed.
>
> This makes the code shorter now and even more so when power supply
> extensions are added.
> It also simplifies the locking, as it can all happen inside
> power_supply_get_property().
>
> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
Thanks, patch looks good to me:
Reviewed-by: Hans de Goede <hdegoede@...hat.com>
Regards,
Hans
p.s.
Last review for me in this set. I'm afraid I don't have the bandwidth
atm to also review the actual extension API.
> ---
> drivers/power/supply/power_supply_sysfs.c | 26 +++++---------------------
> 1 file changed, 5 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/power/supply/power_supply_sysfs.c b/drivers/power/supply/power_supply_sysfs.c
> index 4ab08386bcb7..915a4ba62258 100644
> --- a/drivers/power/supply/power_supply_sysfs.c
> +++ b/drivers/power/supply/power_supply_sysfs.c
> @@ -290,6 +290,8 @@ static ssize_t power_supply_show_property(struct device *dev,
> dev_dbg_ratelimited(dev,
> "driver has no data for `%s' property\n",
> attr->attr.name);
> + else if (ret == -EINVAL) /* property is not supported */
> + return -ENODATA;
> else if (ret != -ENODEV && ret != -EAGAIN)
> dev_err_ratelimited(dev,
> "driver failed to report `%s' property: %zd\n",
> @@ -451,11 +453,7 @@ static int add_prop_uevent(const struct device *dev, struct kobj_uevent_env *env
>
> int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env)
> {
> - const struct power_supply *psy = dev_get_drvdata(dev);
> - const enum power_supply_property *battery_props =
> - power_supply_battery_info_properties;
> - unsigned long psy_drv_properties[POWER_SUPPLY_ATTR_CNT /
> - sizeof(unsigned long) + 1] = {0};
> + struct power_supply *psy = dev_get_drvdata(dev);
> int ret = 0, j;
> char *prop_buf;
>
> @@ -483,22 +481,8 @@ int power_supply_uevent(const struct device *dev, struct kobj_uevent_env *env)
> if (ret)
> goto out;
>
> - for (j = 0; j < psy->desc->num_properties; j++) {
> - set_bit(psy->desc->properties[j], psy_drv_properties);
> - ret = add_prop_uevent(dev, env, psy->desc->properties[j],
> - prop_buf);
> - if (ret)
> - goto out;
> - }
> -
> - for (j = 0; j < power_supply_battery_info_properties_size; j++) {
> - if (test_bit(battery_props[j], psy_drv_properties))
> - continue;
> - if (!power_supply_battery_info_has_prop(psy->battery_info,
> - battery_props[j]))
> - continue;
> - ret = add_prop_uevent(dev, env, battery_props[j],
> - prop_buf);
> + for (j = 0; j < POWER_SUPPLY_ATTR_CNT; j++) {
> + ret = add_prop_uevent(dev, env, j, prop_buf);
> if (ret)
> goto out;
> }
>
Powered by blists - more mailing lists