* [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching [not found] ` <20161229084135.GC1460@lahna.fi.intel.com> @ 2016-12-30 1:27 ` Rafael J. Wysocki 2017-01-01 20:30 ` Hans de Goede 2017-01-02 10:53 ` Mika Westerberg 0 siblings, 2 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2016-12-30 1:27 UTC (permalink / raw) To: Mika Westerberg, Hans de Goede Cc: Len Brown, Adrian Hunter, Ulf Hansson, ACPI Devel Maling List, linux-mmc, Andy Shevchenko, Linux PCI 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(-) 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, ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching 2016-12-30 1:27 ` [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching Rafael J. Wysocki @ 2017-01-01 20:30 ` Hans de Goede 2017-03-30 20:56 ` Rafael J. Wysocki 2017-01-02 10:53 ` Mika Westerberg 1 sibling, 1 reply; 6+ messages in thread From: Hans de Goede @ 2017-01-01 20:30 UTC (permalink / raw) To: Rafael J. Wysocki, Mika Westerberg Cc: Len Brown, Adrian Hunter, Ulf Hansson, ACPI Devel Maling List, linux-mmc, Andy Shevchenko, Linux PCI 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, > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching 2017-01-01 20:30 ` Hans de Goede @ 2017-03-30 20:56 ` Rafael J. Wysocki 2017-03-31 10:39 ` Hans de Goede 0 siblings, 1 reply; 6+ messages in thread From: Rafael J. Wysocki @ 2017-03-30 20:56 UTC (permalink / raw) To: Hans de Goede Cc: Mika Westerberg, Adrian Hunter, Ulf Hansson, ACPI Devel Maling List, Andy Shevchenko, Linux PCI On Sunday, January 01, 2017 09:30:06 PM Hans de Goede wrote: > 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> Unfortunately, this breaks discrete graphics enumeration in https://bugzilla.kernel.org/show_bug.cgi?id=194889 so can you please check if the patch below doesn't break the platform fixed by the above? Thanks, Rafael --- drivers/acpi/glue.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) Index: linux-pm/drivers/acpi/glue.c =================================================================== --- linux-pm.orig/drivers/acpi/glue.c +++ linux-pm/drivers/acpi/glue.c @@ -99,13 +99,13 @@ static int find_child_checks(struct acpi return -ENODEV; /* - * 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.] + * If the device has a _HID 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) ? + return sta_present && !adev->pnp.type.platform_id ? FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching 2017-03-30 20:56 ` Rafael J. Wysocki @ 2017-03-31 10:39 ` Hans de Goede 2017-03-31 21:23 ` Rafael J. Wysocki 0 siblings, 1 reply; 6+ messages in thread From: Hans de Goede @ 2017-03-31 10:39 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Mika Westerberg, Adrian Hunter, Ulf Hansson, ACPI Devel Maling List, Andy Shevchenko, Linux PCI Hi, On 30-03-17 22:56, Rafael J. Wysocki wrote: > On Sunday, January 01, 2017 09:30:06 PM Hans de Goede wrote: >> 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> > > Unfortunately, this breaks discrete graphics enumeration in > > https://bugzilla.kernel.org/show_bug.cgi?id=194889 > > so can you please check if the patch below doesn't break the platform fixed by > the above? I've just tried this and this patch does not regress the platform fixed by the original commit. Regards, Hans > > Thanks, > Rafael > > > --- > drivers/acpi/glue.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > Index: linux-pm/drivers/acpi/glue.c > =================================================================== > --- linux-pm.orig/drivers/acpi/glue.c > +++ linux-pm/drivers/acpi/glue.c > @@ -99,13 +99,13 @@ static int find_child_checks(struct acpi > return -ENODEV; > > /* > - * 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.] > + * If the device has a _HID 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) ? > + return sta_present && !adev->pnp.type.platform_id ? > FIND_CHILD_MAX_SCORE : FIND_CHILD_MIN_SCORE; > } > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching 2017-03-31 10:39 ` Hans de Goede @ 2017-03-31 21:23 ` Rafael J. Wysocki 0 siblings, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2017-03-31 21:23 UTC (permalink / raw) To: Hans de Goede Cc: Mika Westerberg, Adrian Hunter, Ulf Hansson, ACPI Devel Maling List, Andy Shevchenko, Linux PCI On Friday, March 31, 2017 12:39:35 PM Hans de Goede wrote: > Hi, > > On 30-03-17 22:56, Rafael J. Wysocki wrote: > > On Sunday, January 01, 2017 09:30:06 PM Hans de Goede wrote: > >> 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> > > > > Unfortunately, this breaks discrete graphics enumeration in > > > > https://bugzilla.kernel.org/show_bug.cgi?id=194889 > > > > so can you please check if the patch below doesn't break the platform fixed by > > the above? > > I've just tried this and this patch does not regress the platform fixed > by the original commit. OK, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching 2016-12-30 1:27 ` [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching Rafael J. Wysocki 2017-01-01 20:30 ` Hans de Goede @ 2017-01-02 10:53 ` Mika Westerberg 1 sibling, 0 replies; 6+ messages in thread From: Mika Westerberg @ 2017-01-02 10:53 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Hans de Goede, Len Brown, Adrian Hunter, Ulf Hansson, ACPI Devel Maling List, linux-mmc, Andy Shevchenko, Linux PCI 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 :) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-31 21:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20161225102148.7706-1-hdegoede@redhat.com>
[not found] ` <CAJZ5v0gjcaYtskSMueAz+GjHcLZv=4A8Qz_Lf6Bn7S0CLSkgPg@mail.gmail.com>
[not found] ` <20161229084135.GC1460@lahna.fi.intel.com>
2016-12-30 1:27 ` [PATCH] ACPI / scan: Prefer devices without _HID/_CID for _ADR matching Rafael J. Wysocki
2017-01-01 20:30 ` Hans de Goede
2017-03-30 20:56 ` Rafael J. Wysocki
2017-03-31 10:39 ` Hans de Goede
2017-03-31 21:23 ` Rafael J. Wysocki
2017-01-02 10:53 ` Mika Westerberg
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).