Change in ...coreboot[master]: soc/intel/apollolake: Fix XHCI device name in ACPI

[Karthik Ramasubramanian (Code Review)]

Hello Karthikeyan Ramasubramanian,

I'd like you to do a code review. Please visit

https://review.coreboot.org/c/coreboot/+/31147

to review the following change.

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

soc/intel/apollolake: Fix XHCI device name in ACPI

XHCI is currently named as XHC1. This leads to namespace lookup error in
the kernel when children USB ACPI devices are added under the scope of
XHCI device.

BUG=b:123296264
BRANCH=octopus
TEST=Boot to ChromeOS; Ensure that the below error is resolved in the
kernel dmesg
[    0.001000] ACPI Error: [_SB_.PCI0.XHCI.RHUB.HS03] Namespace lookup
failure, AE_NOT_FOUND (20170728/dswload-210)

Change-Id: Ia4921547fee6fb51333319b9e881501a7e75ebce
Signed-off-by: Karthikeyan Ramasubramanian 
---
M src/soc/intel/apollolake/acpi/xhci.asl
1 file changed, 1 insertion(+), 1 deletion(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/47/31147/1

diff --git a/src/soc/intel/apollolake/acpi/xhci.asl b/src/soc/intel/apollolake/acpi/xhci.asl
index 8222cb5..9f85033 100644
--- a/src/soc/intel/apollolake/acpi/xhci.asl
+++ b/src/soc/intel/apollolake/acpi/xhci.asl
@@ -14,7 +14,7 @@
  */

/* XHCI Controller 0:15.0 */
-Device (XHC1) {
+Device (XHCI) {
    Name (_ADR, 0x00150000)  /* Device 21, Function 0 */

Name (_S3D, 3)  /* D3 supported in S3 */

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG@9 
PS1, Line 9: XHC1
This code is common to both apollolake and geminilake. Where is the reference to 'XHCI' coming from which leads to the AE_NOT_FOUND problem?

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG@9 
PS1, Line 9: XHC1
> This code is common to both apollolake and geminilake. […]

Looks to me this string error is coming from src/soc/intel/apollolake/chip.c:
> -------case PCH_DEVFN_XHCI:
> ------->-------return "XHCI";

That string needs to match the ACPI Device() name.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG@9 
PS1, Line 9: XHC1
> Looks to me this string error is coming from src/soc/intel/apollolake/chip.c: […]

Correct. That path returns XHCI and not XHC1 and hence the error. Looking at soc/intel/, all SoCs except apollolake and denverton_ns seem to be using XHC1.

[Aaron Durbin (Code Review)]

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG@9 
PS1, Line 9: XHC1
> Correct. That path returns XHCI and not XHC1 and hence the error. […]

ok. I think we should expand on the commit description for a better explanation of the root cause.

[Furquan Shaikh (Code Review)]

Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/#/c/31147/1//COMMIT_MSG@9 
PS1, Line 9: XHC1
> ok. I think we should expand on the commit description for a better explanation of the root cause.

Correction:
"...all SoCs except apollolake and denverton_ns seem to be using XHCI."

[Karthik Ramasubramanian (Code Review)]

Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1:
> Patch Set 1:
>
> (1 comment)

I will provide additional description regarding the name mismatch in ASL and SoC driver. Currently we are not seeing this problem since we haven't enabled USB ACPI Driver in coreboot. In the subsequent patch to export the bluetooth reset GPIO information, USB ACPI driver is enabled and the internal USB port in which the Bluetooth controller is present is defined under the scope: _SB.PCI0.XHCI.RHUB.HS03

Scope (_SB.PCI0.XHCI.RHUB.HS03)
    {
        Name (_DDN, "Bluetooth")  // _DDN: DOS Device Name
        Name (_UPC, Package (0x04)  // _UPC: USB Port Capabilities
        {   
            0xFF,
            0xFF,
            Zero,
            Zero
        })

This when exported to the kernel is causing the namespace lookup error.

[Patrick Rudolph (Code Review)]

Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

Patch Set 1: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31147 )

Change subject: soc/intel/apollolake: Fix XHCI device name in ACPI
......................................................................

soc/intel/apollolake: Fix XHCI device name in ACPI

XHCI is currently named as XHC1. This leads to namespace lookup error in
the kernel when children USB ACPI devices are added under the scope of
XHCI device.

BUG=b:123296264
BRANCH=octopus
TEST=Boot to ChromeOS; Ensure that the below error is resolved in the
kernel dmesg
[    0.001000] ACPI Error: [_SB_.PCI0.XHCI.RHUB.HS03] Namespace lookup
failure, AE_NOT_FOUND (20170728/dswload-210)

Change-Id: Ia4921547fee6fb51333319b9e881501a7e75ebce
Signed-off-by: Karthikeyan Ramasubramanian 
Reviewed-on: https://review.coreboot.org/c/31147
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
---
M src/soc/intel/apollolake/acpi/xhci.asl
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/soc/intel/apollolake/acpi/xhci.asl b/src/soc/intel/apollolake/acpi/xhci.asl
index 8222cb5..9f85033 100644
--- a/src/soc/intel/apollolake/acpi/xhci.asl
+++ b/src/soc/intel/apollolake/acpi/xhci.asl
@@ -14,7 +14,7 @@
  */

/* XHCI Controller 0:15.0 */
-Device (XHC1) {
+Device (XHCI) {
    Name (_ADR, 0x00150000)  /* Device 21, Function 0 */

Name (_S3D, 3)  /* D3 supported in S3 */