ACPI:add _STA evaluating at do_acpi_find_child()

PATCH Superseded [email protected] 4 participants 13 messages May 21, 2013 – May 24, 2013 ACPI

lore

Share & tools

lore

[email protected]

author

2013-05-21 03:34 UTC #0 lore raw

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

From: Jeff Wu <[email protected]>

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.

Signed-off-by Jeff Wu <[email protected]>

---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/acpi/glue.c +4 -1

index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
 	acpi_status status;
+	unsigned long long addr = 0, sta = 0;
 
 	status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
 	if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+		status = acpi_bus_get_status_handle(handle, &sta);
+		if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+			return AE_CTRL_DEPTH;
 		*ret_p = handle;
 		return AE_CTRL_TERMINATE;
 	}

Signature · 1 line

1.8.1.2

Jeff Wu

2013-05-21 04:13 UTC #1 lore raw

I also found the similar topic at mail list :
1.
[PATCH] ACPI: return first _ADR match for acpi_get_child
http://www.spinics.net/lists/linux-acpi/msg39739.html

2.ACPI: Rework acpi_get_child() to be more efficient" has been added to the 3.8-stable tree
http://www.spinics.net/lists/stable/msg03621.html

2013/5/21 <[email protected]>:

Quoted 46 lines

From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +4 -2
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2

Lan Tianyu

2013-05-21 13:34 UTC #2 lore raw

In reply to [email protected]

2013/5/21 <[email protected]>:

Quoted 29 lines

From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +0 -0
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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)

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

Quoted 10 lines

drivers/acpi/glue.c +4 -1

 {
-       unsigned long long addr;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;

Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.

Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.

Quoted 11 lines

drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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

--
Best regards
Tianyu Lan

Lan Tianyu

2013-05-21 14:21 UTC #3 lore raw

2013/5/21 Lan Tianyu <[email protected]>:

Quoted 44 lines

2013/5/21 <[email protected]>:
Quoted 29 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +0 -0
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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)
drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 10 lines
drivers/acpi/glue.c +4 -1
 {
-       unsigned long long addr;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.

Quoted 5 lines

Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific

Sorry for broken line. Just update.

Will this affect other devices? Since the device maybe disabled during do binding with ACPI. After some operations, the device would be enabled.

Hi Rafael:
How about add a new function "acpi_get_children_with_
same_addr" to return a list which contains all these child devices with same _ADR.And then do some special check in the specific bus glue callback. Since these cases are no common.

Quoted 19 lines

bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
--
Best regards
Tianyu Lan

Signature · 2 lines

Best regards
Tianyu Lan

Jeff Wu

2013-05-22 03:28 UTC #4 lore raw

In reply to Lan Tianyu

2013/5/21 Lan Tianyu <[email protected]>:

Quoted 35 lines

2013/5/21 <[email protected]>:
Quoted 34 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 

Quoted 8 lines

drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 5 lines
drivers/acpi/glue.c +3 -0
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.

At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.

At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?

Quoted 25 lines

Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
--
Best regards
Tianyu Lan

Lan Tianyu

2013-05-22 06:41 UTC #5 lore raw

2013/5/22 Jeff Wu <[email protected]>:

Quoted 44 lines

2013/5/21 Lan Tianyu <[email protected]>:
Quoted 35 lines
2013/5/21 <[email protected]>:
Quoted 34 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
Quoted 8 lines
drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 5 lines
drivers/acpi/glue.c +3 -0
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.

Quoted 6 lines

At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?

Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.

Quoted 26 lines

Quoted 26 lines
Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
--
Best regards
Tianyu Lan

Signature · 2 lines

Best regards
Tianyu Lan

Jeff Wu

2013-05-22 07:42 UTC #6 lore raw

2013/5/22 Lan Tianyu <[email protected]>:

Quoted 48 lines

2013/5/22 Jeff Wu <[email protected]>:
Quoted 47 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 34 lines
Quoted 34 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
Quoted 8 lines
drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 5 lines
drivers/acpi/glue.c +3 -0
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.

Quoted 5 lines

Quoted 2 lines
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.

Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,
waiting for Rafael's suggestion.

Quoted 32 lines

Quoted 23 lines
Quoted 23 lines
Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
Quoted 3 lines
Quoted 3 lines
--
Best regards
Tianyu Lan
--
Best regards
Tianyu Lan

Rafael J. Wysocki

2013-05-22 23:58 UTC #7 lore raw

Quoted 49 lines · Jeff Wu

2013/5/22 Lan Tianyu <[email protected]>:
Quoted 48 lines
2013/5/22 Jeff Wu <[email protected]>:
Quoted 47 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 34 lines
Quoted 34 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
Quoted 8 lines
drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 5 lines
drivers/acpi/glue.c +3 -0
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.
Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,

I don't really think this is a good idea, because then it won't be clear when to use which version.

Your patch kind of makes sense (although we don't need to initialize both local variables to 0), but the Tianyu's concern is valid in principle either.

Perhaps it would be better to make do_acpi_find_child() return the disabled device if its the only one matching or the first enabled matching device otherwise?

Rafael

Quoted 34 lines

Quoted 27 lines
Quoted 21 lines
Quoted 21 lines
Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
Quoted 3 lines
Quoted 3 lines
--
Best regards
Tianyu Lan
Quoted 3 lines
--
Best regards
Tianyu Lan
--
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

Signature · 2 lines

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

Lan Tianyu

2013-05-23 12:56 UTC #8 lore raw

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

Quoted 17 lines · Jeff Wu

Quoted 16 lines
2013/5/22 Lan Tianyu <[email protected]>:
Quoted 15 lines
2013/5/22 Jeff Wu <[email protected]>:
Quoted 11 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 4 lines
Quoted 1 line
From: Jeff Wu <[email protected]>
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.

Quoted 6 lines

Quoted 3 lines
Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,
I don't really think this is a good idea, because then it won't be clear when to use which version.

Ok. How about make them as one function?.
New function will return all the children's handle with same _ADR and the number of children. The caller should check the number. If the number is 1 and then do eveything that now have done. If the number is larger than 1. It should select one according their strategy.

There is another such issue. In this case, the first child should be selected.
http://www.spinics.net/lists/linux-acpi/msg39739.html

Quoted 11 lines

Your patch kind of makes sense (although we don't need to initialize both local variables to 0), but the Tianyu's concern is valid in principle either.
Perhaps it would be better to make do_acpi_find_child() return the disabled device if its the only one matching or the first enabled matching device otherwise?
Rafael

Signature · 2 lines

Best regards
Tianyu Lan

Rafael J. Wysocki

2013-05-23 21:51 UTC #9 lore raw

Quoted 22 lines · Lan Tianyu

2013/5/23 Rafael J. Wysocki <[email protected]>:
Quoted 21 lines · Jeff Wu
Quoted 19 lines
2013/5/22 Lan Tianyu <[email protected]>:
Quoted 10 lines
2013/5/22 Jeff Wu <[email protected]>:
Quoted 9 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 4 lines
Quoted 1 line
From: Jeff Wu <[email protected]>
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.
Quoted 5 lines
Quoted 2 lines
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.
Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,

Quoted 7 lines

Quoted 2 lines
I don't really think this is a good idea, because then it won't be clear when to use which version.
Ok. How about make them as one function?.
New function will return all the children's handle with same _ADR and the number of children. The caller should check the number. If the number is 1 and then do eveything that now have done. If the number is larger than 1. It should select one according their strategy.

Why do you think that the callers may have different strategies? They are subsystems, not drivers.

Quoted 2 lines

There is another such issue. In this case, the first child should be selected.
http://www.spinics.net/lists/linux-acpi/msg39739.html

No, that's about something different.

Thanks,
Rafael

Quoted 12 lines

Quoted 10 lines
Your patch kind of makes sense (although we don't need to initialize both local variables to 0), but the Tianyu's concern is valid in principle either.
Perhaps it would be better to make do_acpi_find_child() return the disabled device if its the only one matching or the first enabled matching device otherwise?
Rafael

Signature · 2 lines

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

Jeff Wu

2013-05-24 08:22 UTC #10 lore raw

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

Quoted 31 lines · Lan Tianyu

Quoted 22 lines
2013/5/23 Rafael J. Wysocki <[email protected]>:
Quoted 21 lines · Jeff Wu
Quoted 19 lines
2013/5/22 Lan Tianyu <[email protected]>:
Quoted 10 lines
2013/5/22 Jeff Wu <[email protected]>:
Quoted 9 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 4 lines
Quoted 1 line
From: Jeff Wu <[email protected]>
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.
Quoted 5 lines
Quoted 2 lines
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.
Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,
Quoted 7 lines
Quoted 2 lines
I don't really think this is a good idea, because then it won't be clear when to use which version.
Ok. How about make them as one function?.
New function will return all the children's handle with same _ADR and the number of children. The caller should check the number. If the number is 1 and then do eveything that now have done. If the number is larger than 1. It should select one according their strategy.

Quoted 7 lines

Why do you think that the callers may have different strategies? They are subsystems, not drivers.
Quoted 2 lines
There is another such issue. In this case, the first child should be selected.
http://www.spinics.net/lists/linux-acpi/msg39739.html
No, that's about something different.

Thanks, Rafel and Lan,TianYu

Quoted 20 lines

Thanks,
Rafael
Quoted 12 lines
Quoted 10 lines
Your patch kind of makes sense (although we don't need to initialize both local variables to 0), but the Tianyu's concern is valid in principle either.
Perhaps it would be better to make do_acpi_find_child() return the disabled device if its the only one matching or the first enabled matching device otherwise?
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Jeff Wu

2013-05-24 08:21 UTC #11 lore raw

In reply to Rafael J. Wysocki

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

Quoted 62 lines · Jeff Wu

Quoted 49 lines
2013/5/22 Lan Tianyu <[email protected]>:
Quoted 48 lines
2013/5/22 Jeff Wu <[email protected]>:
Quoted 47 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 34 lines
Quoted 34 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
Quoted 8 lines
drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 5 lines
drivers/acpi/glue.c +3 -0
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.
Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,
I don't really think this is a good idea, because then it won't be clear when to use which version.

Quoted 6 lines

Your patch kind of makes sense (although we don't need to initialize both local variables to 0), but the Tianyu's concern is valid in principle either.
Perhaps it would be better to make do_acpi_find_child() return the disabled device if its the only one matching or the first enabled matching device otherwise?

Hi Rafael,
Thanks, i will try it , if it is ok, i will send out the patch.

Quoted 41 lines

Rafael
Quoted 27 lines
Quoted 27 lines
Quoted 24 lines
Quoted 24 lines
Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
--
Best regards
Tianyu Lan
Quoted 7 lines
Quoted 3 lines
--
Best regards
Tianyu Lan
--
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
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Rafael J. Wysocki

2013-05-24 21:37 UTC #12 lore raw

Quoted 66 lines · Jeff Wu

2013/5/23 Rafael J. Wysocki <[email protected]>:
Quoted 65 lines · Jeff Wu
Quoted 49 lines
2013/5/22 Lan Tianyu <[email protected]>:
Quoted 48 lines
2013/5/22 Jeff Wu <[email protected]>:
Quoted 47 lines
2013/5/21 Lan Tianyu <[email protected]>:
Quoted 34 lines
Quoted 34 lines
From: Jeff Wu <[email protected]>
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.
Signed-off-by Jeff Wu <[email protected]>
---
 drivers/acpi/glue.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
drivers/acpi/glue.c +1 -1
index 40a84cc..ace6c2e 100644
@@ -81,11 +81,14 @@ 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;
        acpi_status status;
+       unsigned long long addr = 0, sta = 0;
 
Quoted 8 lines
drivers/acpi/glue.c static struct acpi_bus_type *acpi_get_bus_type(struct device *dev) · line 91
Quoted 5 lines
drivers/acpi/glue.c +3 -0
        status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &addr);
        if (ACPI_SUCCESS(status) && addr == *((u64 *)addr_p)) {
+               status = acpi_bus_get_status_handle(handle, &sta);
+               if (ACPI_SUCCESS(status) && !(sta & ACPI_STA_DEVICE_ENABLED))
+                       return AE_CTRL_DEPTH;
Will this affect other devices? Since the device maybe disabled during do binding with ACPI.
After some operations, the device would be enabled.
At our platforms, DEV1 use for win8,DEV2 use for win7,DEV1 use for Linux. If these devices are disabled when do init, they will always be disabled ,so, work fine.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
At some of the platforms, the devices may be disabled during do the first binding ,if their status are changed,do they not do re-binding ?
Currently, the glue binding operation takes place when the device is created. In my mind, there is no re-binding operation after the device being created, Unless the device being removed and created again.
Thank you very much.
So, as your suggestion, it is a better solution to add a new function for the same _ADR devices,
I don't really think this is a good idea, because then it won't be clear when to use which version.
Your patch kind of makes sense (although we don't need to initialize both local variables to 0), but the Tianyu's concern is valid in principle either.

Quoted 6 lines

Quoted 3 lines
Perhaps it would be better to make do_acpi_find_child() return the disabled device if its the only one matching or the first enabled matching device otherwise?
Hi Rafael,
Thanks, i will try it , if it is ok, i will send out the patch.

Yes, please.

Thanks,
Rafael

Quoted 37 lines

Quoted 37 lines
Quoted 27 lines
Quoted 27 lines
Quoted 24 lines
Quoted 24 lines
Hi Rafael:
How about add a new function
"acpi_get_children_with_same_addr" to return a list
which contains all these child devices with same _ADR.And then do some special check in the specific
bus glue callback. Since these cases are no common.
Quoted 11 lines
drivers/acpi/glue.c +0 -1
                *ret_p = handle;
                return AE_CTRL_TERMINATE;
        }
--
1.8.1.2
--
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
--
Best regards
Tianyu Lan
Quoted 7 lines
Quoted 3 lines
--
Best regards
Tianyu Lan
--
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
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

Signature · 2 lines

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