2017-04-16 23:27:22

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 0/2] ACPI: Device enumeration udates

Hi,

These two patches change a couple of things related to the ACPI enumeration of
devices.

[1/2] causes the default enumeration to also be used for device objects with
ACPI drivers bound for consistency.

[2/2] makes acpi_bus_attach() look at the "visited" flag of device objects as
it should really.

Please let me know if you see any potential problems with these patches.

Thanks,
Rafael


2017-04-16 23:27:07

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 1/2] ACPI / scan: Apply default enumeration to devices with ACPI drivers

From: Rafael J. Wysocki <[email protected]>

The current code in acpi_bus_attach() is inconsistent with respect
to device objects with ACPI drivers bound to them, as it allows
ACPI drivers to bind to device objects with existing "physical"
device companions, but it doesn't allow "physical" device objects
to be created for ACPI device objects with ACPI drivers bound to
them. Thus, in some cases, the outcome depends on the ordering
of events which is confusing at best.

For this reason, modify acpi_bus_attach() to call
acpi_default_enumeration() for device objects with the
pnp.type.platform_id flag set regardless of whether or not
any ACPI drivers are bound to them.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/scan.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1866,10 +1866,10 @@ static void acpi_bus_attach(struct acpi_
if (ret < 0)
return;

- if (ret > 0 || !device->pnp.type.platform_id)
- acpi_device_set_enumerated(device);
- else
+ if (device->pnp.type.platform_id)
acpi_default_enumeration(device);
+ else
+ acpi_device_set_enumerated(device);

ok:
list_for_each_entry(child, &device->children, node)

2017-04-16 23:27:04

by Rafael J. Wysocki

[permalink] [raw]
Subject: [PATCH 2/2] ACPI / scan: Avoid enumerating devices more than once

From: Rafael J. Wysocki <[email protected]>

acpi_bus_attach() does not check the visited flag for devices that
have been enumerated already and some of them may be enumerated
for multiple times as a result, because some callers of
acpi_bus_scan() don't check the visited flag either.

For this reason, modify acpi_bus_attach() to check the visited flag
and avoid enumerating devices that have already been enumerated.

Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/scan.c | 2 ++
1 file changed, 2 insertions(+)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1850,6 +1850,8 @@ static void acpi_bus_attach(struct acpi_
device->flags.power_manageable = 0;

device->flags.initialized = true;
+ } else if (device->flags.visited) {
+ goto ok;
}

ret = acpi_scan_attach_handler(device);

2017-04-18 10:21:40

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH 1/2] ACPI / scan: Apply default enumeration to devices with ACPI drivers

On Mon, Apr 17, 2017 at 01:19:50AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The current code in acpi_bus_attach() is inconsistent with respect
> to device objects with ACPI drivers bound to them, as it allows
> ACPI drivers to bind to device objects with existing "physical"
> device companions, but it doesn't allow "physical" device objects
> to be created for ACPI device objects with ACPI drivers bound to
> them. Thus, in some cases, the outcome depends on the ordering
> of events which is confusing at best.
>
> For this reason, modify acpi_bus_attach() to call
> acpi_default_enumeration() for device objects with the
> pnp.type.platform_id flag set regardless of whether or not
> any ACPI drivers are bound to them.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2017-04-18 10:22:05

by Mika Westerberg

[permalink] [raw]
Subject: Re: [PATCH 2/2] ACPI / scan: Avoid enumerating devices more than once

On Mon, Apr 17, 2017 at 01:20:48AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> acpi_bus_attach() does not check the visited flag for devices that
> have been enumerated already and some of them may be enumerated
> for multiple times as a result, because some callers of
> acpi_bus_scan() don't check the visited flag either.
>
> For this reason, modify acpi_bus_attach() to check the visited flag
> and avoid enumerating devices that have already been enumerated.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Reviewed-by: Mika Westerberg <[email protected]>

2017-04-19 16:39:16

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 1/2] ACPI / scan: Apply default enumeration to devices with ACPI drivers

On Mon, Apr 17, 2017 at 01:19:50AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> The current code in acpi_bus_attach() is inconsistent with respect
> to device objects with ACPI drivers bound to them, as it allows
> ACPI drivers to bind to device objects with existing "physical"
> device companions, but it doesn't allow "physical" device objects
> to be created for ACPI device objects with ACPI drivers bound to
> them. Thus, in some cases, the outcome depends on the ordering
> of events which is confusing at best.
>
> For this reason, modify acpi_bus_attach() to call
> acpi_default_enumeration() for device objects with the
> pnp.type.platform_id flag set regardless of whether or not
> any ACPI drivers are bound to them.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

I did my best to review the context of codes in this patchset.

Reviewed-by: Joey Lee <[email protected]>

Thanka a lot!
Joey Lee

> ---
> drivers/acpi/scan.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -1866,10 +1866,10 @@ static void acpi_bus_attach(struct acpi_
> if (ret < 0)
> return;
>
> - if (ret > 0 || !device->pnp.type.platform_id)
> - acpi_device_set_enumerated(device);
> - else
> + if (device->pnp.type.platform_id)
> acpi_default_enumeration(device);
> + else
> + acpi_device_set_enumerated(device);
>
> ok:
> list_for_each_entry(child, &device->children, node)
>

2017-04-19 16:40:13

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 2/2] ACPI / scan: Avoid enumerating devices more than once

On Mon, Apr 17, 2017 at 01:20:48AM +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> acpi_bus_attach() does not check the visited flag for devices that
> have been enumerated already and some of them may be enumerated
> for multiple times as a result, because some callers of
> acpi_bus_scan() don't check the visited flag either.
>
> For this reason, modify acpi_bus_attach() to check the visited flag
> and avoid enumerating devices that have already been enumerated.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Reviewed-by: Joey Lee <[email protected]>

Thanka a lot!
Joey Lee

> ---
> drivers/acpi/scan.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -1850,6 +1850,8 @@ static void acpi_bus_attach(struct acpi_
> device->flags.power_manageable = 0;
>
> device->flags.initialized = true;
> + } else if (device->flags.visited) {
> + goto ok;
> }
>
> ret = acpi_scan_attach_handler(device);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html