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.

Reviewed-by: Pali Rohár <pali@kernel.org>
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

Hi,

On 1-Oct-24 11:28 PM, Armin Wolf wrote:
> 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.
> 
> Reviewed-by: Pali Rohár <pali@kernel.org>
> Fixes: ab58016c68cc ("platform/x86:dell-laptop: Add knobs to change battery charge settings")
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Thank you for your patch/series, I've applied this patch
(series) to my review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Note it will show up in the pdx86 review-hans branch once I've
pushed my local branch there, which might take a while.

I will include this patch in my next fixes pull-req to Linus
for the current kernel development cycle.

Regards,

Hans



> ---
>  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
> 
>