On Sunday 22 September 2024 03:45:13 Andres Salomon wrote: > On Sun, 22 Sep 2024 08:40:23 +0200 > Armin Wolf <W_Armin@xxxxxx> 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.
