* [PATCH 5.11 regression-fix 0/1] ACPI: scan: Fix Battery devices sometimes never binding
@ 2021-02-01 16:34 Hans de Goede
2021-02-01 16:34 ` [PATCH 1/1] " Hans de Goede
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2021-02-01 16:34 UTC (permalink / raw)
To: Rafael J . Wysocki; +Cc: Hans de Goede, linux-acpi
Hi Rafael,
Sorry for the last minute 5.11 fix, but I only noticed this yesterday
(and wrote the fix today). I believe that this was covered up by the
broken acpi_bus_get_device() error handling which we debugged earlier :|
For details see the actual patch.
It would be nice if you can get this to Linus for 5.11. But I understand
this is cutting it pretty close to the release, so if it misses 5.11 then
this can be picked up by the linux-stable releases.
Regards,
Hans
Hans de Goede (1):
ACPI: scan: Fix Battery devices sometimes never binding
drivers/acpi/scan.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
--
2.29.2
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] ACPI: scan: Fix Battery devices sometimes never binding
2021-02-01 16:34 [PATCH 5.11 regression-fix 0/1] ACPI: scan: Fix Battery devices sometimes never binding Hans de Goede
@ 2021-02-01 16:34 ` Hans de Goede
2021-02-01 17:56 ` Rafael J. Wysocki
0 siblings, 1 reply; 3+ messages in thread
From: Hans de Goede @ 2021-02-01 16:34 UTC (permalink / raw)
To: Rafael J . Wysocki; +Cc: Hans de Goede, linux-acpi
With the new 2 step scanning process, which defers instantiating some
ACPI-devices based on their _DEP to the second step, the following may
happen:
1. During the first acpi_walk_namespace(acpi_bus_check_add) call
acpi_scan_check_dep() gets called on the Battery ACPI dev handle and
adds one or more deps for this handle to the acpi_dep_list
2. During the first acpi_bus_attach() call one or more of the suppliers of
these deps get their driver attached and
acpi_walk_dep_device_list(supplier_handle) gets called.
At this point acpi_bus_get_device(dep->consumer) get called,
but since the battery has DEPs it has not been instantiated during the
first acpi_walk_namespace(acpi_bus_check_add), so the
acpi_bus_get_device(dep->consumer) call fails.
Before this commit, acpi_walk_dep_device_list() would now continue
*without* removing the acpi_dep_data entry for this supplier,consumer
pair from the acpi_dep_list.
3. During the second acpi_walk_namespace(acpi_bus_check_add) call
an acpi_device gets instantiated for the battery and
acpi_scan_dep_init() gets called to initialize its dep_unmet val.
Before this commit, the dep_unmet count would include DEPs for
suppliers for which acpi_walk_dep_device_list(supplier_handle)
has already been called, so it will never become 0 and the
ACPI battery driver will never get attached / bind.
Fix the ACPI battery driver never binding in this scenario by making
acpi_walk_dep_device_list() always remove matching acpi_dep_data
entries independent of the acpi_bus_get_device(dep->consumer) call
succeeding or not.
Fixes: 71da201f38df ("ACPI: scan: Defer enumeration of devices with _DEP lists")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/scan.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 5d7b2fcecf06..4ce54115e981 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2123,12 +2123,12 @@ void acpi_walk_dep_device_list(acpi_handle handle)
list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) {
if (dep->supplier == handle) {
acpi_bus_get_device(dep->consumer, &adev);
- if (!adev)
- continue;
- adev->dep_unmet--;
- if (!adev->dep_unmet)
- acpi_bus_attach(adev, true);
+ if (adev) {
+ adev->dep_unmet--;
+ if (!adev->dep_unmet)
+ acpi_bus_attach(adev, true);
+ }
list_del(&dep->node);
kfree(dep);
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] ACPI: scan: Fix Battery devices sometimes never binding
2021-02-01 16:34 ` [PATCH 1/1] " Hans de Goede
@ 2021-02-01 17:56 ` Rafael J. Wysocki
0 siblings, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2021-02-01 17:56 UTC (permalink / raw)
To: Hans de Goede; +Cc: Rafael J . Wysocki, ACPI Devel Maling List
On Mon, Feb 1, 2021 at 5:35 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> With the new 2 step scanning process, which defers instantiating some
> ACPI-devices based on their _DEP to the second step, the following may
> happen:
>
> 1. During the first acpi_walk_namespace(acpi_bus_check_add) call
> acpi_scan_check_dep() gets called on the Battery ACPI dev handle and
> adds one or more deps for this handle to the acpi_dep_list
>
> 2. During the first acpi_bus_attach() call one or more of the suppliers of
> these deps get their driver attached and
> acpi_walk_dep_device_list(supplier_handle) gets called.
>
> At this point acpi_bus_get_device(dep->consumer) get called,
> but since the battery has DEPs it has not been instantiated during the
> first acpi_walk_namespace(acpi_bus_check_add), so the
> acpi_bus_get_device(dep->consumer) call fails.
>
> Before this commit, acpi_walk_dep_device_list() would now continue
> *without* removing the acpi_dep_data entry for this supplier,consumer
> pair from the acpi_dep_list.
Yeah, I've overlooked the fact that the consumer needs to have a
struct acpi_device in order for the entry to be dropped from the list.
Sorry.
> 3. During the second acpi_walk_namespace(acpi_bus_check_add) call
> an acpi_device gets instantiated for the battery and
> acpi_scan_dep_init() gets called to initialize its dep_unmet val.
>
> Before this commit, the dep_unmet count would include DEPs for
> suppliers for which acpi_walk_dep_device_list(supplier_handle)
> has already been called, so it will never become 0 and the
> ACPI battery driver will never get attached / bind.
>
> Fix the ACPI battery driver never binding in this scenario by making
> acpi_walk_dep_device_list() always remove matching acpi_dep_data
> entries independent of the acpi_bus_get_device(dep->consumer) call
> succeeding or not.
>
> Fixes: 71da201f38df ("ACPI: scan: Defer enumeration of devices with _DEP lists")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied and thanks for fixing this!
> ---
> drivers/acpi/scan.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 5d7b2fcecf06..4ce54115e981 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2123,12 +2123,12 @@ void acpi_walk_dep_device_list(acpi_handle handle)
> list_for_each_entry_safe(dep, tmp, &acpi_dep_list, node) {
> if (dep->supplier == handle) {
> acpi_bus_get_device(dep->consumer, &adev);
> - if (!adev)
> - continue;
>
> - adev->dep_unmet--;
> - if (!adev->dep_unmet)
> - acpi_bus_attach(adev, true);
> + if (adev) {
> + adev->dep_unmet--;
> + if (!adev->dep_unmet)
> + acpi_bus_attach(adev, true);
> + }
>
> list_del(&dep->node);
> kfree(dep);
> --
> 2.29.2
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-01 17:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-01 16:34 [PATCH 5.11 regression-fix 0/1] ACPI: scan: Fix Battery devices sometimes never binding Hans de Goede
2021-02-01 16:34 ` [PATCH 1/1] " Hans de Goede
2021-02-01 17:56 ` Rafael J. Wysocki
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.