Change in coreboot[master]: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode

[Matt DeVillier (Code Review)]

Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode

Braswell boards don't work well with the eMMC and SD controller
in ACPI in payloads other than depthcharge - SeaBIOS requires
an onerous workaround, and Tianocore can't see the devices
at all. To make the common use-case work better, switch
from ACPI mode to PCI mode.

Test: build/boot cyan variants with SeaBIOS and Tianocore
payloads, verify eMMC and SD card visible and bootable.

Change-Id: I71947603e22a37fe2c8ef4eaac8a3aa0d0ed1cec
Signed-off-by: Matt DeVillier 
---
M src/mainboard/google/cyan/devicetree.cb
1 file changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/40002/1

diff --git a/src/mainboard/google/cyan/devicetree.cb b/src/mainboard/google/cyan/devicetree.cb
index d5f7a3f..5df1a5a 100644
--- a/src/mainboard/google/cyan/devicetree.cb
+++ b/src/mainboard/google/cyan/devicetree.cb
@@ -19,7 +19,7 @@
    # Set the parameters for SiliconInit
    ############################################################

-	register "PcdSdcardMode" = "PCH_ACPI_MODE"
+	register "PcdSdcardMode" = "PCH_PCI_MODE"
    register "PcdEnableHsuart0" = "0"
    register "PcdEnableHsuart1" = "1"
    register "PcdEnableAzalia" = "1"
@@ -36,7 +36,7 @@
    register "PcdEnableI2C6" = "0"
    register "PunitPwrConfigDisable" = "0"	# Enable SVID
    register "ChvSvidConfig" = "SVID_PMIC_CONFIG"
-	register "PcdEmmcMode" = "PCH_ACPI_MODE"
+	register "PcdEmmcMode" = "PCH_PCI_MODE"
    register "PcdUsb3ClkSsc" = "1"
    register "PcdDispClkSsc" = "1"
    register "PcdSataClkSsc" = "1"
@@ -84,10 +84,10 @@
    # LPE audio codec settings
    register "lpe_codec_clk_src" = "LPE_CLK_SRC_XTAL" # 19.2MHz clock

-	# Enable devices in ACPI mode
+	# Enable LPSS and LPE devices in ACPI mode
    register "lpss_acpi_mode" = "1"
-	register "emmc_acpi_mode" = "1"
-	register "sd_acpi_mode" = "1"
+	register "emmc_acpi_mode" = "0"
+	register "sd_acpi_mode" = "0"
    register "lpe_acpi_mode" = "1"

# Disable SLP_X stretching after SUS power well fail.

[Paul Menzel (Code Review)]

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@10 
PS1, Line 10: SeaBIOS requires
            : an onerous workaround
Do you have a reference for that?

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@13 
PS1, Line 13: from ACPI mode to PCI mode.
Re-flow for 72/75 characters?

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@14 
PS1, Line 14: 
Any idea, what the reasoning for ACPI mode was from Chromium OS? Will the OS set this up for itself?

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins),

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#2).

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode

Braswell boards don't work well with the eMMC and SD controller
in ACPI in payloads other than depthcharge - SeaBIOS requires
an onerous workaround (manually determining the PCI BAR0 address
for each eMMC and SD controller, then adding adding etc/sdcard
entries to the CBFS), and Tianocore can't see the devices at all.
To make the common use-case work better, switch to PCI mode.

Test: build/boot cyan variants with SeaBIOS and Tianocore
payloads, verify eMMC and SD card visible and bootable.

Change-Id: I71947603e22a37fe2c8ef4eaac8a3aa0d0ed1cec
Signed-off-by: Matt DeVillier 
---
M src/mainboard/google/cyan/devicetree.cb
1 file changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/40002/2

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

Patch Set 2:

(3 comments)

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@10 
PS1, Line 10: SeaBIOS requires
            : an onerous workaround
> Do you have a reference for that?

https://www.seabios.org/Runtime_config#Other_Configuration_items, sdcard entry. Basically, for each eMMC or SD device, you have to create an etc/sdcard entry in the cbfs which contains the PCI BAR0 address for the controller, so that SeaBIOS can init it. There's also no way to generate this at compile time since BAR0 isn't known ahead of time.

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@13 
PS1, Line 13: from ACPI mode to PCI mode.
> Re-flow for 72/75 characters?

Done

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@14 
PS1, Line 14:
> Any idea, what the reasoning for ACPI mode was from Chromium OS? Will the OS set this up for itself?

in the ChromeOS scenario, depthcharge will init the controllers so they are available to boot the OS. I believe this was just a carry over from Baytrail. All other non-ChromeOS Braswell/Cherry Trail devices have their eMMC/SD controllers in PCI mode, not ACPI.

[Angel Pons (Code Review)]

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

Patch Set 2: Code-Review+2

(1 comment)

https://review.coreboot.org/c/coreboot/+/40002/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40002/2//COMMIT_MSG@16 
PS2, Line 16: Test: build/boot cyan variants with SeaBIOS and Tianocore
Are they seen under both Windows and Linux?

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

Patch Set 2:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40002/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40002/2//COMMIT_MSG@16 
PS2, Line 16: Test: build/boot cyan variants with SeaBIOS and Tianocore
> Are they seen under both Windows and Linux?

the OSes themselves can see/use the eMMC/SD card in both PCI and ACPI modes, it's just a payload limitation.

[Matt DeVillier (Code Review)]

Hello build bot (Jenkins), Angel Pons,

I'd like you to reexamine a change. Please visit

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

to look at the new patch set (#3).

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode

Braswell boards don't work well with the eMMC and SD controller
in ACPI in payloads other than depthcharge - SeaBIOS requires
an onerous workaround (manually determining the PCI BAR0 address
for each eMMC and SD controller, then adding adding etc/sdcard
entries to the CBFS), and Tianocore can't see the devices at all.
To make the common use-case work better, switch to PCI mode.

Test: build/boot cyan variants with SeaBIOS and Tianocore
payloads, verify eMMC and SD card visible and bootable to
both payloads and OSes.

Change-Id: I71947603e22a37fe2c8ef4eaac8a3aa0d0ed1cec
Signed-off-by: Matt DeVillier 
---
M src/mainboard/google/cyan/devicetree.cb
1 file changed, 5 insertions(+), 5 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/40002/3

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

Patch Set 3:

(1 comment)

https://review.coreboot.org/c/coreboot/+/40002/2//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40002/2//COMMIT_MSG@16 
PS2, Line 16: Test: build/boot cyan variants with SeaBIOS and Tianocore
> the OSes themselves can see/use the eMMC/SD card in both PCI and ACPI modes, it's just a payload lim […]

Done

[Matt DeVillier (Code Review)]

Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

Patch Set 3:

(2 comments)

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG 
Commit Message:

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@10 
PS1, Line 10: SeaBIOS requires
            : an onerous workaround
> https://www.seabios.org/Runtime_config#Other_Configuration_items, sdcard entry. […]

Done

https://review.coreboot.org/c/coreboot/+/40002/1//COMMIT_MSG@14 
PS1, Line 14:
> in the ChromeOS scenario, depthcharge will init the controllers so they are available to boot the OS […]

Done

[Matt DeVillier (Code Review)]

Matt DeVillier has submitted this change. ( https://review.coreboot.org/c/coreboot/+/40002 )

Change subject: mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode
......................................................................

mb/google/cyan: Switch eMMC and SD from ACPI to PCI mode

Braswell boards don't work well with the eMMC and SD controller
in ACPI in payloads other than depthcharge - SeaBIOS requires
an onerous workaround (manually determining the PCI BAR0 address
for each eMMC and SD controller, then adding adding etc/sdcard
entries to the CBFS), and Tianocore can't see the devices at all.
To make the common use-case work better, switch to PCI mode.

Test: build/boot cyan variants with SeaBIOS and Tianocore
payloads, verify eMMC and SD card visible and bootable to
both payloads and OSes.

Change-Id: I71947603e22a37fe2c8ef4eaac8a3aa0d0ed1cec
Signed-off-by: Matt DeVillier 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40002
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M src/mainboard/google/cyan/devicetree.cb
1 file changed, 5 insertions(+), 5 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved

diff --git a/src/mainboard/google/cyan/devicetree.cb b/src/mainboard/google/cyan/devicetree.cb
index 328a608..b13f3f9 100644
--- a/src/mainboard/google/cyan/devicetree.cb
+++ b/src/mainboard/google/cyan/devicetree.cb
@@ -19,7 +19,7 @@
    # Set the parameters for SiliconInit
    ############################################################

-	register "PcdSdcardMode" = "PCH_ACPI_MODE"
+	register "PcdSdcardMode" = "PCH_PCI_MODE"
    register "PcdEnableHsuart0" = "0"
    register "PcdEnableHsuart1" = "1"
    register "PcdEnableAzalia" = "1"
@@ -36,7 +36,7 @@
    register "PcdEnableI2C6" = "0"
    register "PunitPwrConfigDisable" = "0"	# Enable SVID
    register "ChvSvidConfig" = "SVID_PMIC_CONFIG"
-	register "PcdEmmcMode" = "PCH_ACPI_MODE"
+	register "PcdEmmcMode" = "PCH_PCI_MODE"
    register "PcdUsb3ClkSsc" = "1"
    register "PcdDispClkSsc" = "1"
    register "PcdSataClkSsc" = "1"
@@ -84,10 +84,10 @@
    # LPE audio codec settings
    register "lpe_codec_clk_src" = "LPE_CLK_SRC_XTAL" # 19.2MHz clock

-	# Enable devices in ACPI mode
+	# Enable LPSS and LPE devices in ACPI mode
    register "lpss_acpi_mode" = "1"
-	register "emmc_acpi_mode" = "1"
-	register "sd_acpi_mode" = "1"
+	register "emmc_acpi_mode" = "0"
+	register "sd_acpi_mode" = "0"
    register "lpe_acpi_mode" = "1"

# Disable SLP_X stretching after SUS power well fail.