lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ