[PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[Armin Wolf]

This patch series fixes some issues around the battery hook handling
inside the ACPI battery driver and the dell-laptop driver.

The first patch simplifies the locking during battery hook removal as
a preparation for the second patch which fixes a possible crash when
unregistering a battery hook.

The third patch allows the dell-laptop driver to handle systems with
multiple batteries.

All patches where tested on a Dell Inspiron 3505 and appear to work.

Changes since v1:
- fix the underlying issue inside the ACPI battery driver
- reword patch for dell-laptop

Armin Wolf (3):
  ACPI: battery: Simplify battery hook locking
  ACPI: battery: Fix possible crash when unregistering a battery hook
  platform/x86: dell-laptop: Do not fail when encountering unsupported
    batteries

 drivers/acpi/battery.c                  | 27 ++++++++++++++++---------
 drivers/platform/x86/dell/dell-laptop.c | 15 +++++++++++---
 include/acpi/battery.h                  |  1 +
 3 files changed, 31 insertions(+), 12 deletions(-)

--
2.39.5

[PATCH v2 1/3] ACPI: battery: Simplify battery hook locking

[Armin Wolf]

Move the conditional locking from __battery_hook_unregister()
into battery_hook_unregister() and rename the low-level function
to simplify the locking during battery hook removal.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/battery.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index da3a879d638a..10e9136897a7 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -706,28 +706,28 @@ static LIST_HEAD(acpi_battery_list);
 static LIST_HEAD(battery_hook_list);
 static DEFINE_MUTEX(hook_mutex);

-static void __battery_hook_unregister(struct acpi_battery_hook *hook, int lock)
+static void battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
 {
 	struct acpi_battery *battery;
+
 	/*
 	 * In order to remove a hook, we first need to
 	 * de-register all the batteries that are registered.
 	 */
-	if (lock)
-		mutex_lock(&hook_mutex);
 	list_for_each_entry(battery, &acpi_battery_list, list) {
 		if (!hook->remove_battery(battery->bat, hook))
 			power_supply_changed(battery->bat);
 	}
 	list_del(&hook->list);
-	if (lock)
-		mutex_unlock(&hook_mutex);
+
 	pr_info("extension unregistered: %s\n", hook->name);
 }

 void battery_hook_unregister(struct acpi_battery_hook *hook)
 {
-	__battery_hook_unregister(hook, 1);
+	mutex_lock(&hook_mutex);
+	battery_hook_unregister_unlocked(hook);
+	mutex_unlock(&hook_mutex);
 }
 EXPORT_SYMBOL_GPL(battery_hook_unregister);

@@ -753,7 +753,7 @@ void battery_hook_register(struct acpi_battery_hook *hook)
 			 * hooks.
 			 */
 			pr_err("extension failed to load: %s", hook->name);
-			__battery_hook_unregister(hook, 0);
+			battery_hook_unregister_unlocked(hook);
 			goto end;
 		}

@@ -807,7 +807,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
 			 */
 			pr_err("error in extension, unloading: %s",
 					hook_node->name);
-			__battery_hook_unregister(hook_node, 0);
+			battery_hook_unregister_unlocked(hook_node);
 		}
 	}
 	mutex_unlock(&hook_mutex);
@@ -840,7 +840,7 @@ static void __exit battery_hook_exit(void)
 	 * need to remove the hooks.
 	 */
 	list_for_each_entry_safe(hook, ptr, &battery_hook_list, list) {
-		__battery_hook_unregister(hook, 1);
+		battery_hook_unregister(hook);
 	}
 	mutex_destroy(&hook_mutex);
 }
--
2.39.5

Re: [PATCH v2 1/3] ACPI: battery: Simplify battery hook locking

[Ilpo Järvinen]

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

On Sun, 22 Sep 2024, Armin Wolf wrote:

> Move the conditional locking from __battery_hook_unregister()
> into battery_hook_unregister() and rename the low-level function
> to simplify the locking during battery hook removal.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

> ---
>  drivers/acpi/battery.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index da3a879d638a..10e9136897a7 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -706,28 +706,28 @@ static LIST_HEAD(acpi_battery_list);
>  static LIST_HEAD(battery_hook_list);
>  static DEFINE_MUTEX(hook_mutex);
> 
> -static void __battery_hook_unregister(struct acpi_battery_hook *hook, int lock)
> +static void battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
>  {
>  	struct acpi_battery *battery;
> +
>  	/*
>  	 * In order to remove a hook, we first need to
>  	 * de-register all the batteries that are registered.
>  	 */
> -	if (lock)
> -		mutex_lock(&hook_mutex);
>  	list_for_each_entry(battery, &acpi_battery_list, list) {
>  		if (!hook->remove_battery(battery->bat, hook))
>  			power_supply_changed(battery->bat);
>  	}
>  	list_del(&hook->list);
> -	if (lock)
> -		mutex_unlock(&hook_mutex);
> +
>  	pr_info("extension unregistered: %s\n", hook->name);
>  }
> 
>  void battery_hook_unregister(struct acpi_battery_hook *hook)
>  {
> -	__battery_hook_unregister(hook, 1);
> +	mutex_lock(&hook_mutex);
> +	battery_hook_unregister_unlocked(hook);
> +	mutex_unlock(&hook_mutex);
>  }
>  EXPORT_SYMBOL_GPL(battery_hook_unregister);
> 
> @@ -753,7 +753,7 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>  			 * hooks.
>  			 */
>  			pr_err("extension failed to load: %s", hook->name);
> -			__battery_hook_unregister(hook, 0);
> +			battery_hook_unregister_unlocked(hook);
>  			goto end;
>  		}
> 
> @@ -807,7 +807,7 @@ static void battery_hook_add_battery(struct acpi_battery *battery)
>  			 */
>  			pr_err("error in extension, unloading: %s",
>  					hook_node->name);
> -			__battery_hook_unregister(hook_node, 0);
> +			battery_hook_unregister_unlocked(hook_node);
>  		}
>  	}
>  	mutex_unlock(&hook_mutex);
> @@ -840,7 +840,7 @@ static void __exit battery_hook_exit(void)
>  	 * need to remove the hooks.
>  	 */
>  	list_for_each_entry_safe(hook, ptr, &battery_hook_list, list) {
> -		__battery_hook_unregister(hook, 1);
> +		battery_hook_unregister(hook);
>  	}
>  	mutex_destroy(&hook_mutex);
>  }
> --
> 2.39.5
> 

[PATCH v2 2/3] ACPI: battery: Fix possible crash when unregistering a battery hook

[Armin Wolf]

When a battery hook returns an error when adding a new battery, then
the battery hook is automatically unregistered.
However the battery hook provider cannot know that, so it will later
call battery_hook_unregister() on the already unregistered battery
hook, resulting in a crash.

Fix this by using a boolean flag to mark already unregistered battery
hooks as "dead" so that they can be ignored by
battery_hook_unregister().

Fixes: fa93854f7a7e ("battery: Add the battery hooking API")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/acpi/battery.c | 11 ++++++++++-
 include/acpi/battery.h |  1 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index 10e9136897a7..b31a6183a082 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -719,6 +719,7 @@ static void battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
 			power_supply_changed(battery->bat);
 	}
 	list_del(&hook->list);
+	hook->dead = true;

 	pr_info("extension unregistered: %s\n", hook->name);
 }
@@ -726,7 +727,14 @@ static void battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
 void battery_hook_unregister(struct acpi_battery_hook *hook)
 {
 	mutex_lock(&hook_mutex);
-	battery_hook_unregister_unlocked(hook);
+	/*
+	 * Ignore already unregistered battery hooks. This might happen
+	 * if a battery hook was previously unloaded due to an error when
+	 * adding a new battery.
+	 */
+	if (!hook->dead)
+		battery_hook_unregister_unlocked(hook);
+
 	mutex_unlock(&hook_mutex);
 }
 EXPORT_SYMBOL_GPL(battery_hook_unregister);
@@ -737,6 +745,7 @@ void battery_hook_register(struct acpi_battery_hook *hook)

 	mutex_lock(&hook_mutex);
 	INIT_LIST_HEAD(&hook->list);
+	hook->dead = false;
 	list_add(&hook->list, &battery_hook_list);
 	/*
 	 * Now that the driver is registered, we need
diff --git a/include/acpi/battery.h b/include/acpi/battery.h
index c93f16dfb944..5cfe132bb7f5 100644
--- a/include/acpi/battery.h
+++ b/include/acpi/battery.h
@@ -16,6 +16,7 @@ struct acpi_battery_hook {
 	int (*add_battery)(struct power_supply *battery, struct acpi_battery_hook *hook);
 	int (*remove_battery)(struct power_supply *battery, struct acpi_battery_hook *hook);
 	struct list_head list;
+	bool dead;
 };

 void battery_hook_register(struct acpi_battery_hook *hook);
--
2.39.5

Re: [PATCH v2 2/3] ACPI: battery: Fix possible crash when unregistering a battery hook

[Rafael J. Wysocki]

On Sun, Sep 22, 2024 at 8:40 AM Armin Wolf <W_Armin@gmx.de> wrote:
>
> When a battery hook returns an error when adding a new battery, then
> the battery hook is automatically unregistered.
> However the battery hook provider cannot know that, so it will later
> call battery_hook_unregister() on the already unregistered battery
> hook, resulting in a crash.
>
> Fix this by using a boolean flag to mark already unregistered battery
> hooks as "dead" so that they can be ignored by
> battery_hook_unregister().
>
> Fixes: fa93854f7a7e ("battery: Add the battery hooking API")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>
> ---
>  drivers/acpi/battery.c | 11 ++++++++++-
>  include/acpi/battery.h |  1 +
>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
> index 10e9136897a7..b31a6183a082 100644
> --- a/drivers/acpi/battery.c
> +++ b/drivers/acpi/battery.c
> @@ -719,6 +719,7 @@ static void battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
>                         power_supply_changed(battery->bat);
>         }
>         list_del(&hook->list);
> +       hook->dead = true;

It looks like you could do

list_del_init((&hook->list);

here and then do a list_emtpy() check below.

>
>         pr_info("extension unregistered: %s\n", hook->name);
>  }
> @@ -726,7 +727,14 @@ static void battery_hook_unregister_unlocked(struct acpi_battery_hook *hook)
>  void battery_hook_unregister(struct acpi_battery_hook *hook)
>  {
>         mutex_lock(&hook_mutex);
> -       battery_hook_unregister_unlocked(hook);
> +       /*
> +        * Ignore already unregistered battery hooks. This might happen
> +        * if a battery hook was previously unloaded due to an error when
> +        * adding a new battery.
> +        */
> +       if (!hook->dead)

if (!list_empty(&hook->list))

> +               battery_hook_unregister_unlocked(hook);
> +
>         mutex_unlock(&hook_mutex);

and the new struct field would not be necessary if I'm not mistaken.

>  }
>  EXPORT_SYMBOL_GPL(battery_hook_unregister);
> @@ -737,6 +745,7 @@ void battery_hook_register(struct acpi_battery_hook *hook)
>
>         mutex_lock(&hook_mutex);
>         INIT_LIST_HEAD(&hook->list);

Also the above statement is redundant, so maybe drop it while you're at it?

> +       hook->dead = false;
>         list_add(&hook->list, &battery_hook_list);
>         /*
>          * Now that the driver is registered, we need
> diff --git a/include/acpi/battery.h b/include/acpi/battery.h
> index c93f16dfb944..5cfe132bb7f5 100644
> --- a/include/acpi/battery.h
> +++ b/include/acpi/battery.h
> @@ -16,6 +16,7 @@ struct acpi_battery_hook {
>         int (*add_battery)(struct power_supply *battery, struct acpi_battery_hook *hook);
>         int (*remove_battery)(struct power_supply *battery, struct acpi_battery_hook *hook);
>         struct list_head list;
> +       bool dead;
>  };
>
>  void battery_hook_register(struct acpi_battery_hook *hook);
> --

[PATCH v2 3/3] platform/x86: dell-laptop: Do not fail when encountering unsupported batteries

[Armin Wolf]

If the battery hook encounters a unsupported battery, it will
return an error. This in turn will cause the battery driver to
automatically unregister the battery hook.

On machines with multiple batteries however, this will prevent
the battery hook from handling the primary battery, since it will
always get unregistered upon encountering one of the unsupported
batteries.

Fix this by simply ignoring unsupported batteries.

Fixes: ab58016c68cc ("platform/x86:dell-laptop: Add knobs to change battery charge settings")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-laptop.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-laptop.c b/drivers/platform/x86/dell/dell-laptop.c
index a3cd0505f282..5671bd0deee7 100644
--- a/drivers/platform/x86/dell/dell-laptop.c
+++ b/drivers/platform/x86/dell/dell-laptop.c
@@ -2391,12 +2391,18 @@ static struct attribute *dell_battery_attrs[] = {
 };
 ATTRIBUTE_GROUPS(dell_battery);

+static bool dell_battery_supported(struct power_supply *battery)
+{
+	/* We currently only support the primary battery */
+	return strcmp(battery->desc->name, "BAT0") == 0;
+}
+
 static int dell_battery_add(struct power_supply *battery,
 		struct acpi_battery_hook *hook)
 {
-	/* this currently only supports the primary battery */
-	if (strcmp(battery->desc->name, "BAT0") != 0)
-		return -ENODEV;
+	/* Return 0 instead of an error to avoid being unloaded */
+	if (!dell_battery_supported(battery))
+		return 0;

 	return device_add_groups(&battery->dev, dell_battery_groups);
 }
@@ -2404,6 +2410,9 @@ static int dell_battery_add(struct power_supply *battery,
 static int dell_battery_remove(struct power_supply *battery,
 		struct acpi_battery_hook *hook)
 {
+	if (!dell_battery_supported(battery))
+		return 0;
+
 	device_remove_groups(&battery->dev, dell_battery_groups);
 	return 0;
 }
--
2.39.5

Re: [PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[Andres Salomon]

On Sun, 22 Sep 2024 08:40:23 +0200
Armin Wolf <W_Armin@gmx.de> wrote:

> This patch series fixes some issues around the battery hook handling
> inside the ACPI battery driver and the dell-laptop driver.
> 
> The first patch simplifies the locking during battery hook removal as
> a preparation for the second patch which fixes a possible crash when
> unregistering a battery hook.
> 
> The third patch allows the dell-laptop driver to handle systems with
> multiple batteries.
> 
> All patches where tested on a Dell Inspiron 3505 and appear to work.

Can you tell me more about the system? What type of battery is the second
battery, and how is it attached? What do the kernel logs look like when the
two batteries are registered? I'm still confused as to how the same
battery->dev ends up being reused for multiple physical batteries.

The patches look good to me, btw; feel free to add my Reviewed-by
if that's helpful.

Also, with the caveat that I'm not quite understanding the aforementioned
battery->dev conflict - worth noting that dell-laptop isn't the only driver
that could have this problem with multiple batteries. A quick glance
through some other drivers:

 - asus-wmi.c does basically the same thing in checking for just the first
   battery, and the comment implies that there may be multiple batteries.

 - system76.c claims that the EC only supports one battery, so maybe that
   one is okay? But to be on the safe side, it should probably do the same
   thing.

 - thinkpad_acpi.c actually supports multiple batteries, so maybe it
   doesn't have the problem. But if tpacpi_battery_probe() fails for one
   of the batteries and the battery->dev is shared between the two
   batteries, then same issue?


> 
> Changes since v1:
> - fix the underlying issue inside the ACPI battery driver
> - reword patch for dell-laptop
> 
> Armin Wolf (3):
>   ACPI: battery: Simplify battery hook locking
>   ACPI: battery: Fix possible crash when unregistering a battery hook
>   platform/x86: dell-laptop: Do not fail when encountering unsupported
>     batteries
> 
>  drivers/acpi/battery.c                  | 27 ++++++++++++++++---------
>  drivers/platform/x86/dell/dell-laptop.c | 15 +++++++++++---
>  include/acpi/battery.h                  |  1 +
>  3 files changed, 31 insertions(+), 12 deletions(-)
> 
> --
> 2.39.5
> 



-- 
I'm available for contract & employment work, please contact me if
interested.

Re: [PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[Pali Rohár]

On Sunday 22 September 2024 03:45:13 Andres Salomon wrote:
> On Sun, 22 Sep 2024 08:40:23 +0200
> Armin Wolf <W_Armin@gmx.de> wrote:
> 
> > This patch series fixes some issues around the battery hook handling
> > inside the ACPI battery driver and the dell-laptop driver.
> > 
> > The first patch simplifies the locking during battery hook removal as
> > a preparation for the second patch which fixes a possible crash when
> > unregistering a battery hook.
> > 
> > The third patch allows the dell-laptop driver to handle systems with
> > multiple batteries.
> > 
> > All patches where tested on a Dell Inspiron 3505 and appear to work.
> 
> Can you tell me more about the system? What type of battery is the second
> battery, and how is it attached? What do the kernel logs look like when the
> two batteries are registered? I'm still confused as to how the same
> battery->dev ends up being reused for multiple physical batteries.
> 
> The patches look good to me, btw; feel free to add my Reviewed-by
> if that's helpful.

For me these patches also looks good, so you can add also my
Reviewed-by. But please wait for the review from ACPI people.

> Also, with the caveat that I'm not quite understanding the aforementioned
> battery->dev conflict - worth noting that dell-laptop isn't the only driver
> that could have this problem with multiple batteries. A quick glance
> through some other drivers:
> 
>  - asus-wmi.c does basically the same thing in checking for just the first
>    battery, and the comment implies that there may be multiple batteries.
> 
>  - system76.c claims that the EC only supports one battery, so maybe that
>    one is okay? But to be on the safe side, it should probably do the same
>    thing.
> 
>  - thinkpad_acpi.c actually supports multiple batteries, so maybe it
>    doesn't have the problem. But if tpacpi_battery_probe() fails for one
>    of the batteries and the battery->dev is shared between the two
>    batteries, then same issue?

It is possible that there is same issue.

> 
> > 
> > Changes since v1:
> > - fix the underlying issue inside the ACPI battery driver
> > - reword patch for dell-laptop
> > 
> > Armin Wolf (3):
> >   ACPI: battery: Simplify battery hook locking
> >   ACPI: battery: Fix possible crash when unregistering a battery hook
> >   platform/x86: dell-laptop: Do not fail when encountering unsupported
> >     batteries
> > 
> >  drivers/acpi/battery.c                  | 27 ++++++++++++++++---------
> >  drivers/platform/x86/dell/dell-laptop.c | 15 +++++++++++---
> >  include/acpi/battery.h                  |  1 +
> >  3 files changed, 31 insertions(+), 12 deletions(-)
> > 
> > --
> > 2.39.5
> > 
> 
> 
> 
> -- 
> I'm available for contract & employment work, please contact me if
> interested.

Re: [PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[Armin Wolf]

Am 22.09.24 um 09:45 schrieb Andres Salomon:

> On Sun, 22 Sep 2024 08:40:23 +0200
> Armin Wolf <W_Armin@gmx.de> wrote:
>
>> This patch series fixes some issues around the battery hook handling
>> inside the ACPI battery driver and the dell-laptop driver.
>>
>> The first patch simplifies the locking during battery hook removal as
>> a preparation for the second patch which fixes a possible crash when
>> unregistering a battery hook.
>>
>> The third patch allows the dell-laptop driver to handle systems with
>> multiple batteries.
>>
>> All patches where tested on a Dell Inspiron 3505 and appear to work.
> Can you tell me more about the system? What type of battery is the second
> battery, and how is it attached? What do the kernel logs look like when the
> two batteries are registered? I'm still confused as to how the same
> battery->dev ends up being reused for multiple physical batteries.

Hi,

i think there is a misunderstanding here, each battery->dev is associated with a separate
battery. The issue is that the ACPI battery driver responsible for managing battery hooks
does not correctly handle battery hook errors during runtime. For example:

1. Driver registers a batter hook.
2. New ACPI battery is discovered, add_battery callback of the battery hook is called.
3. The callback returns an error for some reason.
4. ACPI battery driver automatically unregisters the battery hook.
5. Driver unregisters battery hook during removal.
6. Crash since battery hook was already unregistered by the ACPI battery driver.

This patch series fixes this by adding a boolean flag to signal that a battery hook was
already unregistered, so the ACPI battery driver can ignore those battery hooks in
battery_hook_unregister().

> The patches look good to me, btw; feel free to add my Reviewed-by
> if that's helpful.
>
> Also, with the caveat that I'm not quite understanding the aforementioned
> battery->dev conflict - worth noting that dell-laptop isn't the only driver
> that could have this problem with multiple batteries. A quick glance
> through some other drivers:
>
>   - asus-wmi.c does basically the same thing in checking for just the first
>     battery, and the comment implies that there may be multiple batteries.
>
>   - system76.c claims that the EC only supports one battery, so maybe that
>     one is okay? But to be on the safe side, it should probably do the same
>     thing.
>
>   - thinkpad_acpi.c actually supports multiple batteries, so maybe it
>     doesn't have the problem. But if tpacpi_battery_probe() fails for one
>     of the batteries and the battery->dev is shared between the two
>     batteries, then same issue?
>
Good point regarding asus-wmi and system76, maybe we should provide some documentation
for writing battery hook providers.

Thanks,
Armin Wolf

>> Changes since v1:
>> - fix the underlying issue inside the ACPI battery driver
>> - reword patch for dell-laptop
>>
>> Armin Wolf (3):
>>    ACPI: battery: Simplify battery hook locking
>>    ACPI: battery: Fix possible crash when unregistering a battery hook
>>    platform/x86: dell-laptop: Do not fail when encountering unsupported
>>      batteries
>>
>>   drivers/acpi/battery.c                  | 27 ++++++++++++++++---------
>>   drivers/platform/x86/dell/dell-laptop.c | 15 +++++++++++---
>>   include/acpi/battery.h                  |  1 +
>>   3 files changed, 31 insertions(+), 12 deletions(-)
>>
>> --
>> 2.39.5
>>
>
>

Re: [PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[Armin Wolf]

Am 22.09.24 um 08:40 schrieb Armin Wolf:

> This patch series fixes some issues around the battery hook handling
> inside the ACPI battery driver and the dell-laptop driver.
>
> The first patch simplifies the locking during battery hook removal as
> a preparation for the second patch which fixes a possible crash when
> unregistering a battery hook.
>
> The third patch allows the dell-laptop driver to handle systems with
> multiple batteries.
>
> All patches where tested on a Dell Inspiron 3505 and appear to work.

Any thoughts from the ACPI maintainers?

Thanks,
Armin Wolf

> Changes since v1:
> - fix the underlying issue inside the ACPI battery driver
> - reword patch for dell-laptop
>
> Armin Wolf (3):
>    ACPI: battery: Simplify battery hook locking
>    ACPI: battery: Fix possible crash when unregistering a battery hook
>    platform/x86: dell-laptop: Do not fail when encountering unsupported
>      batteries
>
>   drivers/acpi/battery.c                  | 27 ++++++++++++++++---------
>   drivers/platform/x86/dell/dell-laptop.c | 15 +++++++++++---
>   include/acpi/battery.h                  |  1 +
>   3 files changed, 31 insertions(+), 12 deletions(-)
>
> --
> 2.39.5
>
>

Re: [PATCH v2 0/3] platform/x86: dell-laptop: Battery hook fixes

[Rafael J. Wysocki]

On Mon, Sep 30, 2024 at 8:53 PM Armin Wolf <W_Armin@gmx.de> wrote:
>
> Am 22.09.24 um 08:40 schrieb Armin Wolf:
>
> > This patch series fixes some issues around the battery hook handling
> > inside the ACPI battery driver and the dell-laptop driver.
> >
> > The first patch simplifies the locking during battery hook removal as
> > a preparation for the second patch which fixes a possible crash when
> > unregistering a battery hook.
> >
> > The third patch allows the dell-laptop driver to handle systems with
> > multiple batteries.
> >
> > All patches where tested on a Dell Inspiron 3505 and appear to work.
>
> Any thoughts from the ACPI maintainers?

The first patch looks good to me, but I have a comment regarding the second one.

I'll get to this tomorrow.

> > Changes since v1:
> > - fix the underlying issue inside the ACPI battery driver
> > - reword patch for dell-laptop
> >
> > Armin Wolf (3):
> >    ACPI: battery: Simplify battery hook locking
> >    ACPI: battery: Fix possible crash when unregistering a battery hook
> >    platform/x86: dell-laptop: Do not fail when encountering unsupported
> >      batteries
> >
> >   drivers/acpi/battery.c                  | 27 ++++++++++++++++---------
> >   drivers/platform/x86/dell/dell-laptop.c | 15 +++++++++++---
> >   include/acpi/battery.h                  |  1 +
> >   3 files changed, 31 insertions(+), 12 deletions(-)
> >
> > --