Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND
Make sure to match devices on the root bus only.
Fixes ACPI exception: AE_NOT_FOUND Tested on Supermicro X11SSH-TF.
Change-Id: I6eca37a1792287502a46a90144f2f0d8e12ae5d4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/skylake/acpi.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/35621/1
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c index c3757b0..580bb95 100644 --- a/src/soc/intel/skylake/acpi.c +++ b/src/soc/intel/skylake/acpi.c @@ -760,6 +760,9 @@ if (dev->path.type != DEVICE_PATH_PCI) return NULL;
+ if (dev->bus && dev->bus->secondary > 0) + return NULL; + switch (dev->path.pci.devfn) { case SA_DEVFN_ROOT: return "MCHC"; case SA_DEVFN_IGD: return "GFX0";
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG@11 PS2, Line 11: Fixes ACPI exception: AE_NOT_FOUND What was causing this exactly? What ACPI code was being generated with a wrong name?
Duncan Laurie has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG@11 PS2, Line 11: Fixes ACPI exception: AE_NOT_FOUND
What was causing this exactly? What ACPI code was being generated with a wrong name?
It might have resolved to nothing and ended up with an empty device name?
In any event I should probably add this to the other SOC ACPI name resolve functions.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG@11 PS2, Line 11: Fixes ACPI exception: AE_NOT_FOUND
It might have resolved to nothing and ended up with an empty device name? […]
It was generating an ACPI path for devices behind PCI bridges (with devfn 0), that of course couldn't be resolved. Now it returns NULL, doesn't generate an ACPI path, which is correct, as the SSDT gen returns.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG@11 PS2, Line 11: Fixes ACPI exception: AE_NOT_FOUND
It was generating an ACPI path for devices behind PCI bridges (with devfn 0), that of course couldn't be resolved. Now it returns NULL, doesn't generate an ACPI path, which is correct, as the SSDT gen returns.
Could you please add this information to the commit description as it provides more context to the change and the original failure?
https://review.coreboot.org/c/coreboot/+/35621/2/src/soc/intel/skylake/acpi.... File src/soc/intel/skylake/acpi.c:
https://review.coreboot.org/c/coreboot/+/35621/2/src/soc/intel/skylake/acpi.... PS2, Line 763: if (dev->bus && dev->bus->secondary > 0) Please add a comment here indicating the purpose of the check.
Hello Kyösti Mälkki, Naresh Solanki, Aaron Durbin, Patrick Rudolph, Subrata Banik, Michael Niewöhner, Robbie Zhang, Matt DeVillier, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Pratikkumar V Prajapati, Lee Leahy, Maxim Polyakov, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35621
to look at the new patch set (#3).
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND
Make sure to match devices on the root bus only. This fixes an issue where the SoC returned "MCHC" as ACPI name for devices behind bridge devices, as the DEVFN matched.
Fixes observed "ACPI exception: AE_NOT_FOUND" in dmesg, as the ACPI path no longer contains invalid names.
Tested on Supermicro X11SSH-TF.
Change-Id: I6eca37a1792287502a46a90144f2f0d8e12ae5d4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/skylake/acpi.c 1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/35621/3
Hello Kyösti Mälkki, Naresh Solanki, Aaron Durbin, Patrick Rudolph, Subrata Banik, Michael Niewöhner, Robbie Zhang, Matt DeVillier, Duncan Laurie, build bot (Jenkins), Furquan Shaikh, Pratikkumar V Prajapati, Lee Leahy, Maxim Polyakov, Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35621
to look at the new patch set (#4).
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND
Make sure to match devices on the root bus only. This fixes an issue where the SoC returned "MCHC" as ACPI name for devices behind bridge devices, as the DEVFN matched.
Fixes observed "ACPI exception: AE_NOT_FOUND" in dmesg, as the ACPI path no longer contains invalid names.
Tested on Supermicro X11SSH-TF.
Change-Id: I6eca37a1792287502a46a90144f2f0d8e12ae5d4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/skylake/acpi.c 1 file changed, 4 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/35621/4
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 4:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/35621/2//COMMIT_MSG@11 PS2, Line 11: Fixes ACPI exception: AE_NOT_FOUND
It was generating an ACPI path for devices behind PCI bridges (with devfn 0), that of course could […]
Done
https://review.coreboot.org/c/coreboot/+/35621/2/src/soc/intel/skylake/acpi.... File src/soc/intel/skylake/acpi.c:
https://review.coreboot.org/c/coreboot/+/35621/2/src/soc/intel/skylake/acpi.... PS2, Line 763: if (dev->bus && dev->bus->secondary > 0)
Please add a comment here indicating the purpose of the check.
Done
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 4: Code-Review+1
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35621 )
Change subject: soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND ......................................................................
soc/intel/skylake: Fix ACPI exception AE_NOT_FOUND
Make sure to match devices on the root bus only. This fixes an issue where the SoC returned "MCHC" as ACPI name for devices behind bridge devices, as the DEVFN matched.
Fixes observed "ACPI exception: AE_NOT_FOUND" in dmesg, as the ACPI path no longer contains invalid names.
Tested on Supermicro X11SSH-TF.
Change-Id: I6eca37a1792287502a46a90144f2f0d8e12ae5d4 Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/35621 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Aaron Durbin adurbin@chromium.org --- M src/soc/intel/skylake/acpi.c 1 file changed, 4 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Aaron Durbin: Looks good to me, approved
diff --git a/src/soc/intel/skylake/acpi.c b/src/soc/intel/skylake/acpi.c index c3757b0..d67b502 100644 --- a/src/soc/intel/skylake/acpi.c +++ b/src/soc/intel/skylake/acpi.c @@ -760,6 +760,10 @@ if (dev->path.type != DEVICE_PATH_PCI) return NULL;
+ /* Only match devices on the root bus */ + if (dev->bus && dev->bus->secondary > 0) + return NULL; + switch (dev->path.pci.devfn) { case SA_DEVFN_ROOT: return "MCHC"; case SA_DEVFN_IGD: return "GFX0";
-
Aaron Durbin (Code Review) -
Duncan Laurie (Code Review)
-
Patrick Georgi (Code Review)
-
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)