| Message ID | 20241001212835.341788-4-W_Armin@gmx.de |
|---|---|
| State | Accepted |
| Commit | b6c57b70a343da512e0fdcae9a097b3aa506b9bb |
| Headers | show |
| Series | platform/x86: dell-laptop: Battery hook fixes | expand |
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 > >
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; }