Re: [PATCH v13 1/4] battery: Add the battery hooking API

From: Sebastian Reichel <sre@kernel.org>
To: Kristian Klausen <kristian@klausen.dk>
Cc: "Ognjen Galic" <smclt30p@gmail.com>,
	"Andy Shevchenko" <andy.shevchenko@gmail.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	"Len Brown" <lenb@kernel.org>,
	"Robert Moore" <robert.moore@intel.com>,
	"Lv Zheng" <lv.zheng@intel.com>,
	"ACPI Devel Maling List" <linux-acpi@vger.kernel.org>,
	devel@acpica.org, "Darren Hart" <dvhart@infradead.org>,
	"Andy Shevchenko" <andy@infradead.org>,
	"Henrique de Moraes Holschuh" <ibm-acpi@hmh.eng.br>,
	"Platform Driver" <platform-driver-x86@vger.kernel.org>,
	ibm-acpi-devel@lists.sourceforge.net,
	"Linux PM" <linux-pm@vger.kernel.org>,
	"Christoph Böhmwalder" <christoph@boehmwalder.at>,
	"Kevin Locke" <kevin@kevinlocke.name>
Subject: Re: [PATCH v13 1/4] battery: Add the battery hooking API
Date: Sat, 25 Jul 2020 21:00:50 +0200	[thread overview]
Message-ID: <20200725190050.3ssslnkrauq6zdrd@earth.universe> (raw)
In-Reply-To: <2270b8c7-beec-5ab6-f8b7-5ec41751c699@klausen.dk>

[-- Attachment #1: Type: text/plain, Size: 2336 bytes --]

Hi,

On Wed, Jun 24, 2020 at 01:42:26AM +0200, Kristian Klausen wrote:
> On 07.02.2018 15.58, Ognjen Galic wrote:
> > This is a patch that implements a generic hooking API for the
> > generic ACPI battery driver.
> > 
> > With this new generic API, drivers can expose platform specific
> > behaviour via sysfs attributes in /sys/class/power_supply/BATn/
> > in a generic way.
> > 
> > A perfect example of the need for this API are Lenovo ThinkPads.
> > 
> > Lenovo ThinkPads have a ACPI extension that allows the setting of
> > start and stop charge thresholds in the EC and battery firmware
> > via ACPI. The thinkpad_acpi module can use this API to expose
> > sysfs attributes that it controls inside the ACPI battery driver
> > sysfs tree, under /sys/class/power_supply/BATN/.
> > 
> > The file drivers/acpi/battery.h has been moved to
> > include/acpi/battery.h and the includes inside ac.c, sbs.c, and
> > battery.c have been adjusted to reflect that.
> > 
> > When drivers hooks into the API, the API calls add_battery() for
> > each battery in the system that passes it a acpi_battery
> > struct. Then, the drivers can use device_create_file() to create
> > new sysfs attributes with that struct and identify the batteries
> > for per-battery attributes.
> 
> Hi
> 
> I did that, when I implemented charge threshold support for ASUS
> laptops[1][2].
> 
> It works very well but I can't control the threshold with udev (also
> reported by another user here[3]). So I did a bit of digging and the doc[4]
> states: "If attributes are added after the device is registered, then
> userspace won’t get notified and userspace will not know about the new
> attributes.", which seems to be the way the current code works:
> power_supply_register_no_ws is called[5] and if it success all the hooks are
> run.
>
> Looking at the code I'm not sure there is a easy way to fix it, do you have
> any good ideas?

That problem is described by Greg in his blog post from 2013:

http://kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/

The power-supply subsystem offers registering extra attributes
at registration time by filling in the .attr_grp field in the
struct power_supply_config supplied as last parameter to
power_supply_register_*() since 4.21.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]