* [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c
@ 2021-03-28 11:19 Hans de Goede
2021-03-28 11:20 ` [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies Hans de Goede
2021-03-29 12:52 ` [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Rafael J. Wysocki
0 siblings, 2 replies; 7+ messages in thread
From: Hans de Goede @ 2021-03-28 11:19 UTC (permalink / raw)
To: Rafael J . Wysocki, Len Brown; +Cc: Hans de Goede, linux-acpi
Move acpi_scan_dep_init() higher up in scan.c to avoid needing a forward
declaration in the next patch in this set.
Fixes: 71da201f38df ("ACPI: scan: Defer enumeration of devices with _DEP lists")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/scan.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1584c9e463bd..19f8fd6ea17a 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1655,6 +1655,20 @@ void acpi_device_add_finalize(struct acpi_device *device)
kobject_uevent(&device->dev.kobj, KOBJ_ADD);
}
+static void acpi_scan_dep_init(struct acpi_device *adev)
+{
+ struct acpi_dep_data *dep;
+
+ mutex_lock(&acpi_dep_list_lock);
+
+ list_for_each_entry(dep, &acpi_dep_list, node) {
+ if (dep->consumer == adev->handle)
+ adev->dep_unmet++;
+ }
+
+ mutex_unlock(&acpi_dep_list_lock);
+}
+
static int acpi_add_single_object(struct acpi_device **child,
acpi_handle handle, int type,
unsigned long long sta)
@@ -1906,20 +1920,6 @@ static u32 acpi_scan_check_dep(acpi_handle handle)
return count;
}
-static void acpi_scan_dep_init(struct acpi_device *adev)
-{
- struct acpi_dep_data *dep;
-
- mutex_lock(&acpi_dep_list_lock);
-
- list_for_each_entry(dep, &acpi_dep_list, node) {
- if (dep->consumer == adev->handle)
- adev->dep_unmet++;
- }
-
- mutex_unlock(&acpi_dep_list_lock);
-}
-
static bool acpi_bus_scan_second_pass;
static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep,
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies
2021-03-28 11:19 [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Hans de Goede
@ 2021-03-28 11:20 ` Hans de Goede
2021-03-29 13:39 ` Rafael J. Wysocki
2021-03-29 12:52 ` [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Rafael J. Wysocki
1 sibling, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-03-28 11:20 UTC (permalink / raw)
To: Rafael J . Wysocki, Len Brown; +Cc: Hans de Goede, linux-acpi
Commit 71da201f38df ("ACPI: scan: Defer enumeration of devices with
_DEP lists") dropped the following 2 lines from acpi_init_device_object():
/* Assume there are unmet deps until acpi_device_dep_initialize() runs */
device->dep_unmet = 1;
Leaving the initial value of dep_unmet at the 0 from the kzalloc(). This
causes the acpi_bus_get_status() call in acpi_add_single_object() to
actually call _STA, even though there maybe unmet deps, leading to errors
like these:
[ 0.123579] ACPI Error: No handler for Region [ECRM] (00000000ba9edc4c)
[GenericSerialBus] (20170831/evregion-166)
[ 0.123601] ACPI Error: Region GenericSerialBus (ID=9) has no handler
(20170831/exfldio-299)
[ 0.123618] ACPI Error: Method parse/execution failed
\_SB.I2C1.BAT1._STA, AE_NOT_EXIST (20170831/psparse-550)
Fix this by moving the acpi_scan_dep_init() call done for devices added
during the second pass done by acpi_bus_scan() to inside
acpi_add_single_object(), so that dep_unmet is properly initialized
before the acpi_bus_get_status() call.
This re-fixes the issue initially fixed by
commit 63347db0affa ("ACPI / scan: Use acpi_bus_get_status() to initialize
ACPI_TYPE_DEVICE devs"), which introduced the removed
"device->dep_unmet = 1;" statement.
This issue was noticed; and the fix tested on a Dell Venue 10 Pro 5055.
Fixes: 71da201f38df ("ACPI: scan: Defer enumeration of devices with _DEP lists")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/acpi/scan.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 19f8fd6ea17a..0d0176559bc2 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1671,7 +1671,8 @@ static void acpi_scan_dep_init(struct acpi_device *adev)
static int acpi_add_single_object(struct acpi_device **child,
acpi_handle handle, int type,
- unsigned long long sta)
+ unsigned long long sta,
+ bool init_dep)
{
struct acpi_device_info *info = NULL;
struct acpi_device *device;
@@ -1693,9 +1694,13 @@ static int acpi_add_single_object(struct acpi_device **child,
* that we can call acpi_bus_get_status() and use its quirk handling.
* Note this must be done before the get power-/wakeup_dev-flags calls.
*/
- if (type == ACPI_BUS_TYPE_DEVICE)
+ if (type == ACPI_BUS_TYPE_DEVICE) {
+ if (init_dep)
+ acpi_scan_dep_init(device);
+
if (acpi_bus_get_status(device) < 0)
acpi_set_device_status(device, 0);
+ }
acpi_bus_get_power_flags(device);
acpi_bus_get_wakeup_device_flags(device);
@@ -1952,13 +1957,11 @@ static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep,
}
}
- acpi_add_single_object(&device, handle, type, sta);
+ acpi_add_single_object(&device, handle, type, sta, !check_dep);
if (!device)
return AE_CTRL_DEPTH;
acpi_scan_init_hotplug(device);
- if (!check_dep)
- acpi_scan_dep_init(device);
out:
if (!*adev_p)
@@ -2221,7 +2224,7 @@ int acpi_bus_register_early_device(int type)
int result;
result = acpi_add_single_object(&device, NULL,
- type, ACPI_STA_DEFAULT);
+ type, ACPI_STA_DEFAULT, false);
if (result)
return result;
@@ -2242,7 +2245,7 @@ static int acpi_bus_scan_fixed(void)
result = acpi_add_single_object(&device, NULL,
ACPI_BUS_TYPE_POWER_BUTTON,
- ACPI_STA_DEFAULT);
+ ACPI_STA_DEFAULT, false);
if (result)
return result;
@@ -2259,7 +2262,7 @@ static int acpi_bus_scan_fixed(void)
result = acpi_add_single_object(&device, NULL,
ACPI_BUS_TYPE_SLEEP_BUTTON,
- ACPI_STA_DEFAULT);
+ ACPI_STA_DEFAULT, false);
if (result)
return result;
--
2.30.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c
2021-03-28 11:19 [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Hans de Goede
2021-03-28 11:20 ` [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies Hans de Goede
@ 2021-03-29 12:52 ` Rafael J. Wysocki
2021-03-29 14:29 ` Hans de Goede
1 sibling, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-03-29 12:52 UTC (permalink / raw)
To: Hans de Goede; +Cc: Rafael J . Wysocki, Len Brown, ACPI Devel Maling List
On Sun, Mar 28, 2021 at 1:20 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Move acpi_scan_dep_init() higher up in scan.c to avoid needing a forward
> declaration in the next patch in this set.
>
> Fixes: 71da201f38df ("ACPI: scan: Defer enumeration of devices with _DEP lists")
Well, this doesn't seem applicable here, as the patch shouldn't make a
practical difference.
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
> drivers/acpi/scan.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1584c9e463bd..19f8fd6ea17a 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1655,6 +1655,20 @@ void acpi_device_add_finalize(struct acpi_device *device)
> kobject_uevent(&device->dev.kobj, KOBJ_ADD);
> }
>
> +static void acpi_scan_dep_init(struct acpi_device *adev)
> +{
> + struct acpi_dep_data *dep;
> +
> + mutex_lock(&acpi_dep_list_lock);
> +
> + list_for_each_entry(dep, &acpi_dep_list, node) {
> + if (dep->consumer == adev->handle)
> + adev->dep_unmet++;
> + }
> +
> + mutex_unlock(&acpi_dep_list_lock);
> +}
> +
> static int acpi_add_single_object(struct acpi_device **child,
> acpi_handle handle, int type,
> unsigned long long sta)
> @@ -1906,20 +1920,6 @@ static u32 acpi_scan_check_dep(acpi_handle handle)
> return count;
> }
>
> -static void acpi_scan_dep_init(struct acpi_device *adev)
> -{
> - struct acpi_dep_data *dep;
> -
> - mutex_lock(&acpi_dep_list_lock);
> -
> - list_for_each_entry(dep, &acpi_dep_list, node) {
> - if (dep->consumer == adev->handle)
> - adev->dep_unmet++;
> - }
> -
> - mutex_unlock(&acpi_dep_list_lock);
> -}
> -
> static bool acpi_bus_scan_second_pass;
>
> static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep,
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies
2021-03-28 11:20 ` [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies Hans de Goede
@ 2021-03-29 13:39 ` Rafael J. Wysocki
2021-03-29 14:45 ` Hans de Goede
0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-03-29 13:39 UTC (permalink / raw)
To: Hans de Goede; +Cc: Len Brown, linux-acpi, Rafael J. Wysocki
On Sunday, March 28, 2021 1:20:00 PM CEST Hans de Goede wrote:
> Commit 71da201f38df ("ACPI: scan: Defer enumeration of devices with
> _DEP lists") dropped the following 2 lines from acpi_init_device_object():
>
> /* Assume there are unmet deps until acpi_device_dep_initialize() runs */
> device->dep_unmet = 1;
>
> Leaving the initial value of dep_unmet at the 0 from the kzalloc(). This
> causes the acpi_bus_get_status() call in acpi_add_single_object() to
> actually call _STA, even though there maybe unmet deps, leading to errors
> like these:
>
> [ 0.123579] ACPI Error: No handler for Region [ECRM] (00000000ba9edc4c)
> [GenericSerialBus] (20170831/evregion-166)
> [ 0.123601] ACPI Error: Region GenericSerialBus (ID=9) has no handler
> (20170831/exfldio-299)
> [ 0.123618] ACPI Error: Method parse/execution failed
> \_SB.I2C1.BAT1._STA, AE_NOT_EXIST (20170831/psparse-550)
>
> Fix this by moving the acpi_scan_dep_init() call done for devices added
> during the second pass done by acpi_bus_scan() to inside
> acpi_add_single_object(), so that dep_unmet is properly initialized
> before the acpi_bus_get_status() call.
I wonder why the change below can't be made instead.
The behavior would be closer to the original then AFAICS.
---
drivers/acpi/scan.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1647,6 +1647,8 @@ void acpi_init_device_object(struct acpi
device_initialize(&device->dev);
dev_set_uevent_suppress(&device->dev, true);
acpi_init_coherency(device);
+ /* Assume there are unmet deps to start with. */
+ device->dep_unmet = 1;
}
void acpi_device_add_finalize(struct acpi_device *device)
@@ -1957,7 +1959,13 @@ static acpi_status acpi_bus_check_add(ac
return AE_CTRL_DEPTH;
acpi_scan_init_hotplug(device);
- if (!check_dep)
+ /*
+ * If check_dep is true at this point, the device has no dependencies,
+ * or the creation of the device object would have been postponed above.
+ */
+ if (check_dep)
+ device->dep_unmet = 0;
+ else
acpi_scan_dep_init(device);
out:
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c
2021-03-29 12:52 ` [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Rafael J. Wysocki
@ 2021-03-29 14:29 ` Hans de Goede
0 siblings, 0 replies; 7+ messages in thread
From: Hans de Goede @ 2021-03-29 14:29 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Rafael J . Wysocki, Len Brown, ACPI Devel Maling List
Hi,
On 3/29/21 2:52 PM, Rafael J. Wysocki wrote:
> On Sun, Mar 28, 2021 at 1:20 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>
>> Move acpi_scan_dep_init() higher up in scan.c to avoid needing a forward
>> declaration in the next patch in this set.
>>
>> Fixes: 71da201f38df ("ACPI: scan: Defer enumeration of devices with _DEP lists")
>
> Well, this doesn't seem applicable here, as the patch shouldn't make a
> practical difference.
Its a pre-req for the actual fix, hence the fixes tag so that it will get
cherry-pick into the same stable-series as the actual fix (assuming the
actual fix gets merged as is).
Regards,
Hans
>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/acpi/scan.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 1584c9e463bd..19f8fd6ea17a 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -1655,6 +1655,20 @@ void acpi_device_add_finalize(struct acpi_device *device)
>> kobject_uevent(&device->dev.kobj, KOBJ_ADD);
>> }
>>
>> +static void acpi_scan_dep_init(struct acpi_device *adev)
>> +{
>> + struct acpi_dep_data *dep;
>> +
>> + mutex_lock(&acpi_dep_list_lock);
>> +
>> + list_for_each_entry(dep, &acpi_dep_list, node) {
>> + if (dep->consumer == adev->handle)
>> + adev->dep_unmet++;
>> + }
>> +
>> + mutex_unlock(&acpi_dep_list_lock);
>> +}
>> +
>> static int acpi_add_single_object(struct acpi_device **child,
>> acpi_handle handle, int type,
>> unsigned long long sta)
>> @@ -1906,20 +1920,6 @@ static u32 acpi_scan_check_dep(acpi_handle handle)
>> return count;
>> }
>>
>> -static void acpi_scan_dep_init(struct acpi_device *adev)
>> -{
>> - struct acpi_dep_data *dep;
>> -
>> - mutex_lock(&acpi_dep_list_lock);
>> -
>> - list_for_each_entry(dep, &acpi_dep_list, node) {
>> - if (dep->consumer == adev->handle)
>> - adev->dep_unmet++;
>> - }
>> -
>> - mutex_unlock(&acpi_dep_list_lock);
>> -}
>> -
>> static bool acpi_bus_scan_second_pass;
>>
>> static acpi_status acpi_bus_check_add(acpi_handle handle, bool check_dep,
>> --
>> 2.30.2
>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies
2021-03-29 13:39 ` Rafael J. Wysocki
@ 2021-03-29 14:45 ` Hans de Goede
2021-03-29 14:56 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: Hans de Goede @ 2021-03-29 14:45 UTC (permalink / raw)
To: Rafael J. Wysocki; +Cc: Len Brown, linux-acpi, Rafael J. Wysocki
Hi,
On 3/29/21 3:39 PM, Rafael J. Wysocki wrote:
> On Sunday, March 28, 2021 1:20:00 PM CEST Hans de Goede wrote:
>> Commit 71da201f38df ("ACPI: scan: Defer enumeration of devices with
>> _DEP lists") dropped the following 2 lines from acpi_init_device_object():
>>
>> /* Assume there are unmet deps until acpi_device_dep_initialize() runs */
>> device->dep_unmet = 1;
>>
>> Leaving the initial value of dep_unmet at the 0 from the kzalloc(). This
>> causes the acpi_bus_get_status() call in acpi_add_single_object() to
>> actually call _STA, even though there maybe unmet deps, leading to errors
>> like these:
>>
>> [ 0.123579] ACPI Error: No handler for Region [ECRM] (00000000ba9edc4c)
>> [GenericSerialBus] (20170831/evregion-166)
>> [ 0.123601] ACPI Error: Region GenericSerialBus (ID=9) has no handler
>> (20170831/exfldio-299)
>> [ 0.123618] ACPI Error: Method parse/execution failed
>> \_SB.I2C1.BAT1._STA, AE_NOT_EXIST (20170831/psparse-550)
>>
>> Fix this by moving the acpi_scan_dep_init() call done for devices added
>> during the second pass done by acpi_bus_scan() to inside
>> acpi_add_single_object(), so that dep_unmet is properly initialized
>> before the acpi_bus_get_status() call.
>
> I wonder why the change below can't be made instead.
>
> The behavior would be closer to the original then AFAICS.
Right the behavior would be closer to the code before the 2 fase scan
rework. But just actually making sure we have the right value in unmet_dep
a tiny bit earlier seems cleaner to me.
Note that the one acpi_add_single_object() call which actually sets the
new init_dep parameter to true and the previous place of calling
acpi_scan_dep_init() are very close together, here is the code before
this patch:
acpi_add_single_object(&device, handle, type, sta, !check_dep);
if (!device)
return AE_CTRL_DEPTH;
acpi_scan_init_hotplug(device);
if (!check_dep)
acpi_scan_dep_init();
So we are only doing the acpi_scan_dep_init() call a tiny bit earlier
and wrt which locks are being held when it gets called no changes are
made since it is still called as part of the call-graph below
acpi_bus_check_add(), I explicitly checked the locking situation because
that was my one worry with this patch.
And this new approach also has the advantage of not setting dep_unmet=1
(and never clearing it) for devices instantiated through:
acpi_bus_register_early_device()
acpi_bus_scan_fixed()
acpi_add_power_resource()
IOW while looking into fixing the regression of the errors being logged
again I tried to do a cleaner fix then last time.
With that said if you prefer the version you suggest let me know and I'll
post a single v2 patch doing things that way.
If you want to go with your suggestion, shall I then add a dep_unmet=0
statement to the 3 mentioned functions which leave it at 1 when going back
to the old way of handling this ?
Regards,
Hans
>
> ---
> drivers/acpi/scan.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -1647,6 +1647,8 @@ void acpi_init_device_object(struct acpi
> device_initialize(&device->dev);
> dev_set_uevent_suppress(&device->dev, true);
> acpi_init_coherency(device);
> + /* Assume there are unmet deps to start with. */
> + device->dep_unmet = 1;
> }
>
> void acpi_device_add_finalize(struct acpi_device *device)
> @@ -1957,7 +1959,13 @@ static acpi_status acpi_bus_check_add(ac
> return AE_CTRL_DEPTH;
>
> acpi_scan_init_hotplug(device);
> - if (!check_dep)
> + /*
> + * If check_dep is true at this point, the device has no dependencies,
> + * or the creation of the device object would have been postponed above.
> + */
> + if (check_dep)
> + device->dep_unmet = 0;
> + else
> acpi_scan_dep_init(device);
>
> out:
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies
2021-03-29 14:45 ` Hans de Goede
@ 2021-03-29 14:56 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2021-03-29 14:56 UTC (permalink / raw)
To: Hans de Goede
Cc: Rafael J. Wysocki, Len Brown, ACPI Devel Maling List, Rafael J. Wysocki
On Mon, Mar 29, 2021 at 4:46 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi,
>
> On 3/29/21 3:39 PM, Rafael J. Wysocki wrote:
> > On Sunday, March 28, 2021 1:20:00 PM CEST Hans de Goede wrote:
> >> Commit 71da201f38df ("ACPI: scan: Defer enumeration of devices with
> >> _DEP lists") dropped the following 2 lines from acpi_init_device_object():
> >>
> >> /* Assume there are unmet deps until acpi_device_dep_initialize() runs */
> >> device->dep_unmet = 1;
> >>
> >> Leaving the initial value of dep_unmet at the 0 from the kzalloc(). This
> >> causes the acpi_bus_get_status() call in acpi_add_single_object() to
> >> actually call _STA, even though there maybe unmet deps, leading to errors
> >> like these:
> >>
> >> [ 0.123579] ACPI Error: No handler for Region [ECRM] (00000000ba9edc4c)
> >> [GenericSerialBus] (20170831/evregion-166)
> >> [ 0.123601] ACPI Error: Region GenericSerialBus (ID=9) has no handler
> >> (20170831/exfldio-299)
> >> [ 0.123618] ACPI Error: Method parse/execution failed
> >> \_SB.I2C1.BAT1._STA, AE_NOT_EXIST (20170831/psparse-550)
> >>
> >> Fix this by moving the acpi_scan_dep_init() call done for devices added
> >> during the second pass done by acpi_bus_scan() to inside
> >> acpi_add_single_object(), so that dep_unmet is properly initialized
> >> before the acpi_bus_get_status() call.
> >
> > I wonder why the change below can't be made instead.
> >
> > The behavior would be closer to the original then AFAICS.
>
> Right the behavior would be closer to the code before the 2 fase scan
> rework. But just actually making sure we have the right value in unmet_dep
> a tiny bit earlier seems cleaner to me.
>
> Note that the one acpi_add_single_object() call which actually sets the
> new init_dep parameter to true and the previous place of calling
> acpi_scan_dep_init() are very close together, here is the code before
> this patch:
>
> acpi_add_single_object(&device, handle, type, sta, !check_dep);
> if (!device)
> return AE_CTRL_DEPTH;
>
> acpi_scan_init_hotplug(device);
> if (!check_dep)
> acpi_scan_dep_init();
>
> So we are only doing the acpi_scan_dep_init() call a tiny bit earlier
> and wrt which locks are being held when it gets called no changes are
> made since it is still called as part of the call-graph below
> acpi_bus_check_add(), I explicitly checked the locking situation because
> that was my one worry with this patch.
>
> And this new approach also has the advantage of not setting dep_unmet=1
> (and never clearing it) for devices instantiated through:
>
> acpi_bus_register_early_device()
> acpi_bus_scan_fixed()
> acpi_add_power_resource()
>
> IOW while looking into fixing the regression of the errors being logged
> again I tried to do a cleaner fix then last time.
>
> With that said if you prefer the version you suggest let me know and I'll
> post a single v2 patch doing things that way.
I'd prefer to do the simple fix at this stage of the development
cycle, so yes, please.
I agree that it would be better to initialize dep_unmet properly in
acpi_add_single_object(), but I'd do that a bit differently.
> If you want to go with your suggestion, shall I then add a dep_unmet=0
> statement to the 3 mentioned functions which leave it at 1 when going back
> to the old way of handling this ?
No, I'll take care of this separately.
Cheers!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-03-29 14:56 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-28 11:19 [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Hans de Goede
2021-03-28 11:20 ` [PATCH 2/2] ACPI: scan: Fix _STA getting called on devices with unmet dependencies Hans de Goede
2021-03-29 13:39 ` Rafael J. Wysocki
2021-03-29 14:45 ` Hans de Goede
2021-03-29 14:56 ` Rafael J. Wysocki
2021-03-29 12:52 ` [PATCH 1/2] ACPI: scan: Move acpi_scan_dep_init() higher up in scan.c Rafael J. Wysocki
2021-03-29 14:29 ` Hans de Goede
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).