linux-kernel - Re: [PATCH v5 2/4] power: supply: core: implement extension API

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: <nf3knk45vithb2pxoezqh6aeuj544syy3hesdjl74jbklzoo5x@4rlwxt5dztpr>
Date: Fri, 6 Dec 2024 01:39:10 +0100
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Thomas Weißschuh <linux@...ssschuh.net>
Cc: Armin Wolf <W_Armin@....de>, Hans de Goede <hdegoede@...hat.com>, 
	Thomas Weißschuh <thomas@...ssschuh.net>, Benson Leung <bleung@...omium.org>, 
	Guenter Roeck <groeck@...omium.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Len Brown <lenb@...nel.org>, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	chrome-platform@...ts.linux.dev, linux-acpi@...r.kernel.org
Subject: Re: [PATCH v5 2/4] power: supply: core: implement extension API

Hi,

On Thu, Dec 05, 2024 at 09:46:36PM +0100, Thomas Weißschuh wrote:
> Various drivers, mostly in platform/x86 extend the ACPI battery driver
> with additional sysfs attributes to implement more UAPIs than are
> exposed through ACPI by using various side-channels, like WMI,
> nonstandard ACPI or EC communication.
> 
> While the created sysfs attributes look similar to the attributes
> provided by the powersupply core, there are various deficiencies:
> 
> * They don't show up in uevent payload.
> * They can't be queried with the standard in-kernel APIs.
> * They don't work with triggers.
> * The extending driver has to reimplement all of the parsing,
> formatting and sysfs display logic.
> * Writing a extension driver is completely different from writing a
> normal power supply driver.
> 
> This extension API avoids all of these issues.
> An extension is just a "struct power_supply_ext" with the same kind of
> callbacks as in a normal "struct power_supply_desc".
> 
> The API is meant to be used via battery_hook_register(), the same way as
> the current extensions.
> 
> Signed-off-by: Thomas Weißschuh <linux@...ssschuh.net>
> ---

[...]

> @@ -1241,7 +1297,22 @@ EXPORT_SYMBOL_GPL(power_supply_set_property);
>  int power_supply_property_is_writeable(struct power_supply *psy,
>  					enum power_supply_property psp)
>  {
> -	return psy->desc->property_is_writeable && psy->desc->property_is_writeable(psy, psp);
> +	struct power_supply_ext_registration *reg;
> +
> +	power_supply_for_each_extension(reg, psy) {
> +		if (power_supply_ext_has_property(reg->ext, psp)) {
> +			if (reg->ext->property_is_writeable)
> +				return reg->ext->property_is_writeable(psy, reg->ext,
> +								       reg->data, psp);
> +			else
> +				return -ENODEV;
> +		}
> +	}
> +
> +	if (!psy->desc->property_is_writeable)
> +		return -ENODEV;
> +
> +	return psy->desc->property_is_writeable(psy, psp);
>  }

I think the two 'return -ENODEV' should just be 'return 0'. This
function basically returns a bool. Otherwise the patch looks good to
me. Thanks!

-- Sebastian

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)