Received-SPF: pass (google.com: domain of linux-kernel+bounces-214420-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Fri, 14 Jun 2024 09:02:29 +0200
From: Thomas =?utf-8?Q?Wei=C3=9Fschuh?= <linux@weissschuh.net>
To: Armin Wolf <W_Armin@gmx.de>
Cc: Sebastian Reichel <sre@kernel.org>, linux-pm@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC v2 3/5] power: supply: core: implement extension API
Message-ID: <3491c641-bee2-4100-8eb6-8e90d63330b0@t-8ch.de>
References: <20240608-power-supply-extensions-v2-0-2dcd35b012ad@weissschuh.net>
 <20240608-power-supply-extensions-v2-3-2dcd35b012ad@weissschuh.net>
 <76dff03c-08f6-4edc-af57-c0b8dbf55293@gmx.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <76dff03c-08f6-4edc-af57-c0b8dbf55293@gmx.de>

Hi!

On 2024-06-14 01:11:29+0000, Armin Wolf wrote:
> Am 08.06.24 um 21:19 schrieb Thomas Weißschuh:
> 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >   drivers/power/supply/power_supply.h       |  13 ++-
> >   drivers/power/supply/power_supply_core.c  | 128 ++++++++++++++++++++++++++++--
> >   drivers/power/supply/power_supply_hwmon.c |   2 +-
> >   drivers/power/supply/power_supply_sysfs.c |  37 ++++++++-
> >   include/linux/power_supply.h              |  26 ++++++
> >   5 files changed, 192 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/power/supply/power_supply.h b/drivers/power/supply/power_supply.h
> > index 622be1f0a180..686b66161900 100644
> > --- a/drivers/power/supply/power_supply.h
> > +++ b/drivers/power/supply/power_supply.h
> > @@ -13,8 +13,17 @@ 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);
> > +struct psy_ext_registration {
> > +	struct list_head list_head;
> > +	const struct power_supply_ext *ext;
> > +};
> > +
> > +#define psy_for_each_extension(psy, pos) list_for_each_entry(pos, &(psy)->extensions, list_head)
> 
> sorry for taking so long to respond, the patch looks good to me except one single thing:
> 
> when removing a power supply extension, the driver has to be sure that no one is still using
> the removed extension. So you might want to add some sort of locking when using a power supply
> extension.

That was part of the v1 of the series [0].
I dropped it in v2 to focus on the external API and semantics first.
IMO now that we have the loop macro anyways that can also take care of
the locking: psy_for_each_extension_locked()

[0] https://lore.kernel.org/all/20240606-power-supply-extensions-v1-4-b45669290bdc@weissschuh.net/

Thomas

<snip>