diff mbox

ACPI / scan: Prefer devices without _HID/_CID for _ADR matching

Message ID 2022116.4lq1KAmrJA@aspire.rjw.lan (mailing list archive)
State New, archived
Headers show

Commit Message

Rafael J. Wysocki Dec. 30, 2016, 1:27 a.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The way acpi_find_child_device() works currently is that, if there
are two (or more) devices with the same _ADR value in the same
namespace scope (which is not specifically allowed by the spec and
the OS behavior in that case is not defined), the first one of them
found to be present (with the help of _STA) will be returned.

This covers the majority of cases, but is not sufficient if some of
the devices in question have a _HID (or _CID) returning some valid
ACPI/PNP device IDs (which is disallowed by the spec) and the
ASL writers' expectation appears to be that the OS will match
devices without a valid ACPI/PNP device ID against a given bus
address first.

To cover this special case as well, modify find_child_checks()
to prefer devices without ACPI/PNP device IDs over devices that
have them.

Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

I'm not actually sure if this is sufficient to fix the original 80860F14 uid "2"
sd-controller problem on CherryTrail.  Hans, can you please check?

If it is not sufficient, we may need to fail find_child_checks() right away
for devices with non-empty pnp.ids lists.

Thanks,
Rafael

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


--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Hans de Goede Jan. 1, 2017, 8:30 p.m. UTC | #1 
Hi,

On 30-12-16 02:27, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> The way acpi_find_child_device() works currently is that, if there
> are two (or more) devices with the same _ADR value in the same
> namespace scope (which is not specifically allowed by the spec and
> the OS behavior in that case is not defined), the first one of them
> found to be present (with the help of _STA) will be returned.
>
> This covers the majority of cases, but is not sufficient if some of
> the devices in question have a _HID (or _CID) returning some valid
> ACPI/PNP device IDs (which is disallowed by the spec) and the
> ASL writers' expectation appears to be that the OS will match
> devices without a valid ACPI/PNP device ID against a given bus
> address first.
>
> To cover this special case as well, modify find_child_checks()
> to prefer devices without ACPI/PNP device IDs over devices that
> have them.
>
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>
> I'm not actually sure if this is sufficient to fix the original 80860F14 uid "2"
> sd-controller problem on CherryTrail.  Hans, can you please check?

Ok, just booted a kernel with this patch replacing my own attempt
at fixing this, and the kernel still sees and initializes the
mmc controller in question correctly with this patch:

Tested-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> Thanks,
> Rafael
>
> ---
>  drivers/acpi/glue.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/glue.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/glue.c
> +++ linux-pm/drivers/acpi/glue.c
> @@ -98,7 +98,15 @@ static int find_child_checks(struct acpi
>  	if (check_children && list_empty(&adev->children))
>  		return -ENODEV;
>
> -	return sta_present ? FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE;
> +	/*
> +	 * If the device has a _HID (or _CID) returning a valid ACPI/PNP
> +	 * device ID, it is better to make it look less attractive here, so that
> +	 * the other device with the same _ADR value (that may not have a valid
> +	 * device ID) can be matched going forward.  [This means a second spec
> +	 * violation in a row, so whatever we do here is best effort anyway.]
> +	 */
> +	return sta_present && list_empty(&adev->pnp.ids) ?
> +			FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE;
>  }
>
>  struct acpi_device *acpi_find_child_device(struct acpi_device *parent,
>
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Jan. 2, 2017, 10:53 a.m. UTC | #2 
On Fri, Dec 30, 2016 at 02:27:31AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> The way acpi_find_child_device() works currently is that, if there
> are two (or more) devices with the same _ADR value in the same
> namespace scope (which is not specifically allowed by the spec and
> the OS behavior in that case is not defined), the first one of them
> found to be present (with the help of _STA) will be returned.
> 
> This covers the majority of cases, but is not sufficient if some of
> the devices in question have a _HID (or _CID) returning some valid
> ACPI/PNP device IDs (which is disallowed by the spec) and the
> ASL writers' expectation appears to be that the OS will match
> devices without a valid ACPI/PNP device ID against a given bus
> address first.
> 
> To cover this special case as well, modify find_child_checks()
> to prefer devices without ACPI/PNP device IDs over devices that
> have them.
> 
> Suggested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Thanks for taking care of this Rafael :)
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -98,7 +98,15 @@  static int find_child_checks(struct acpi
 	if (check_children && list_empty(&adev->children))
 		return -ENODEV;
 
-	return sta_present ? FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE;
+	/*
+	 * If the device has a _HID (or _CID) returning a valid ACPI/PNP
+	 * device ID, it is better to make it look less attractive here, so that
+	 * the other device with the same _ADR value (that may not have a valid
+	 * device ID) can be matched going forward.  [This means a second spec
+	 * violation in a row, so whatever we do here is best effort anyway.]
+	 */
+	return sta_present && list_empty(&adev->pnp.ids) ?
+			FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE;
 }
 
 struct acpi_device *acpi_find_child_device(struct acpi_device *parent,