Skip to contentSkip to conversation

ACPI:add _STA evaluating at do_acpi_find_child()

PATCH Superseded [email protected] 3 participants 3 messages May 28, 2013 ACPI

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 ACPI:add _STA evaluating at do_acpi_find_child()

From: Jeff Wu <[email protected]>

v2:
1).If it's only one matching device, whether the deivce status is enabled or disabled,
do_acpi_find_child() return its' handle
2).If it's more than one matching devices, do_acpi_find_child() return the first
enabled device handle
The idea mail thread:
http://wwww.spinics.net/lists/linux-acpi/msg43481.html

v1:
Once do_acpi_find_child() found the first matching handle, it makes the acpi_get_child() loop stop and return the matching handle. But, at some of the platforms, a "_ADR" is with the multiple devices, and if one is enabled, the others will be disabled. Just like a case:
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV0
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV1
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV2

If DEV0 and DEV1 are disabled and DEV2 is enabled, the target is to get DEV2 handle, but actually it always return the first disabled device DEV0 handle. So add a _STA evaluating at do_acpi_find_child() to check the device status. If the object exists, but it is disabled, acpi_get_child() will continue looping to get the correct handle that the object exists and is enabled.

---
 drivers/acpi/glue.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/acpi/glue.c +4 -2
index 40a84cc..51ca764 100644
@@ -81,13 +81,15 @@ static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
 static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
 				      void *addr_p, void **ret_p)
 {
-	unsigned long long addr;
+	unsigned long long addr, sta;
 	acpi_status status;
 
 	status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
 	if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
 		*ret_p = handle;
-		return AE_CTRL_TERMINATE;
+		status = acpi_bus_get_status_handle(handle, &sta);
+		if (ACPI_SUCCESS(status) && (sta & ACPI_STA_DEVICE_ENABLED))
+			return AE_CTRL_TERMINATE;
 	}
 	return AE_OK;
 }
Signature · 1 line
1.8.1.2
#1 lore raw
Quoted 9 lines · On Tuesday, May 28, 2013 11:22:17 AM [email protected] wrote:

From: Jeff Wu <[email protected]>

v2:
1).If it's only one matching device, whether the deivce status is enabled or disabled,
do_acpi_find_child() return its' handle
2).If it's more than one matching devices, do_acpi_find_child() return the first
enabled device handle
The idea mail thread:
http://wwww.spinics.net/lists/linux-acpi/msg43481.html

The information what's changed in v2 with respect to v1 is rather not interesting to the future readers of the changelog, so can you please rearrange it as one comprehensive description of the patch?

The patch itself looks good to me.

Thanks,
Rafael

Quoted 44 lines

v1:
Once do_acpi_find_child() found the first matching handle, it makes the acpi_get_child() loop stop and return the matching handle. But, at some of the platforms, a "_ADR" is with the multiple devices, and if one is enabled, the others will be disabled. Just like a case:
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV0
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV1
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV2

If DEV0 and DEV1 are disabled and DEV2 is enabled, the target is to get DEV2 handle, but actually it always return the first disabled device DEV0 handle. So add a _STA evaluating at do_acpi_find_child() to check the device status. If the object exists, but it is disabled, acpi_get_child() will continue looping to get the correct handle that the object exists and is enabled.

---
 drivers/acpi/glue.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/acpi/glue.c +4 -2
index 40a84cc..51ca764 100644
@@ -81,13 +81,15 @@ static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
 static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
 				      void *addr_p, void **ret_p)
 {
-	unsigned long long addr;
+	unsigned long long addr, sta;
 	acpi_status status;
 
 	status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
 	if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
 		*ret_p = handle;
-		return AE_CTRL_TERMINATE;
+		status = acpi_bus_get_status_handle(handle, &sta);
+		if (ACPI_SUCCESS(status) && (sta & ACPI_STA_DEVICE_ENABLED))
+			return AE_CTRL_TERMINATE;
 	}
 	return AE_OK;
 }
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#2 lore raw

2013/5/28 Rafael J. Wysocki <[email protected]>:

Quoted 11 lines · On Tuesday, May 28, 2013 11:22:17 AM [email protected] wrote:
Quoted 9 lines

From: Jeff Wu <[email protected]>

v2:
1).If it's only one matching device, whether the deivce status is enabled or disabled,
do_acpi_find_child() return its' handle
2).If it's more than one matching devices, do_acpi_find_child() return the first
enabled device handle
The idea mail thread:
http://wwww.spinics.net/lists/linux-acpi/msg43481.html

Quoted 3 lines

The information what's changed in v2 with respect to v1 is rather not interesting to the future readers of the changelog, so can you please rearrange it as one comprehensive description of the patch?

Ok, i will rearrange it and send out the patch.
Thanks
Jeff wu

Quoted 2 lines

The patch itself looks good to me.

That‘s great.

Quoted 38 lines

Thanks,
Rafael

Quoted 33 lines

v1:
Once do_acpi_find_child() found the first matching handle, it makes the acpi_get_child() loop stop and return the matching handle. But, at some of the platforms, a "_ADR" is with the multiple devices, and if one is enabled, the others will be disabled. Just like a case:
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV0
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV1
Address : 0x1FFFF ; handle : SB_PCI0.SATA.DEV2

If DEV0 and DEV1 are disabled and DEV2 is enabled, the target is to get DEV2 handle, but actually it always return the first disabled device DEV0 handle. So add a _STA evaluating at do_acpi_find_child() to check the device status. If the object exists, but it is disabled, acpi_get_child() will continue looping to get the correct handle that the object exists and is enabled.

---
 drivers/acpi/glue.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..51ca764 100644
@@ -81,13 +81,15 @@ static struct acpi_bus_type *acpi_get_bus_type(struct device *dev)
 static acpi_status do_acpi_find_child(acpi_handle handle, u32 lvl_not_used,
                                    void *addr_p, void **ret_p)
 {
-     unsigned long long addr;
+     unsigned long long addr, sta;
      acpi_status status;
 
Quoted 14 lines

drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 95

Quoted 11 lines
drivers/acpi/glue.c +3 -1
      status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
      if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
              *ret_p = handle;
-             return AE_CTRL_TERMINATE;
+             status = acpi_bus_get_status_handle(handle, &sta);
+             if (ACPI_SUCCESS(status) && (sta & ACPI_STA_DEVICE_ENABLED))
+                     return AE_CTRL_TERMINATE;
      }
      return AE_OK;
 }

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

--
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