Change in coreboot[master]: mb/google/hatch: Change SerialIoDevMode for I2C, SPI, and UART

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/34246 )

Change subject: mb/google/hatch: Change SerialIoDevMode for I2C, SPI, and UART
......................................................................

mb/google/hatch: Change SerialIoDevMode for I2C, SPI, and UART

If the devices modes are set as PchSerialIoPci, and the devices
have not been configured yet (no BAR programmed), then the FSP
will put the devices into their D3 state.  This causes coreboot's
later configuration of the device to fail.  If the Mode is instead
set to SkipInit, the IRQs are still programmed, but the device
is left uninitalized, and coreboot will initialize later in ramstage.
It should be noted that without this change, each I2C bus was causing
an additional 10ms timeout waiting for the device to become available.

BUG=b:135941367
BRANCH=none
TEST=No message about I2C timeouts seen in boot log

Change-Id: I87d1b6d1b8ac2453d2f50975fce133e39c17c7e3
Signed-off-by: Tim Wawrzynczak 
---
M src/mainboard/google/hatch/variants/hatch/overridetree.cb
M src/mainboard/google/hatch/variants/helios/overridetree.cb
M src/mainboard/google/hatch/variants/kindred/overridetree.cb
M src/mainboard/google/hatch/variants/kohaku/overridetree.cb
4 files changed, 32 insertions(+), 33 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/46/34246/1

diff --git a/src/mainboard/google/hatch/variants/hatch/overridetree.cb b/src/mainboard/google/hatch/variants/hatch/overridetree.cb
index 18878ce..dac8942 100644
--- a/src/mainboard/google/hatch/variants/hatch/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/hatch/overridetree.cb
@@ -1,14 +1,14 @@
 chip soc/intel/cannonlake

register "SerialIoDevMode" = "{
-		[PchSerialIoIndexI2C0]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C1]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C2]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C3]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C4]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C5]  = PchSerialIoPci,
-		[PchSerialIoIndexSPI0] = PchSerialIoPci,
-		[PchSerialIoIndexSPI1] = PchSerialIoPci,
+		[PchSerialIoIndexI2C0]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C1]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C2]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C3]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C4]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C5]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI0] = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI1] = PchSerialIoSkipInit,
    	[PchSerialIoIndexSPI2] = PchSerialIoDisabled,
    	[PchSerialIoIndexUART0] = PchSerialIoSkipInit,
    	[PchSerialIoIndexUART1] = PchSerialIoDisabled,
diff --git a/src/mainboard/google/hatch/variants/helios/overridetree.cb b/src/mainboard/google/hatch/variants/helios/overridetree.cb
index 3f71c99..5a77d8b 100644
--- a/src/mainboard/google/hatch/variants/helios/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/helios/overridetree.cb
@@ -1,13 +1,13 @@
 chip soc/intel/cannonlake
    register "SerialIoDevMode" = "{
-		[PchSerialIoIndexI2C0] = PchSerialIoPci,
-		[PchSerialIoIndexI2C1] = PchSerialIoPci,
-		[PchSerialIoIndexI2C2] = PchSerialIoPci,
-		[PchSerialIoIndexI2C3] = PchSerialIoPci,
-		[PchSerialIoIndexI2C4] = PchSerialIoPci,
-		[PchSerialIoIndexI2C5] = PchSerialIoPci,
-		[PchSerialIoIndexSPI0] = PchSerialIoPci,
-		[PchSerialIoIndexSPI1] = PchSerialIoPci,
+		[PchSerialIoIndexI2C0] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C1] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C2] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C3] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C4] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C5] = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI0] = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI1] = PchSerialIoSkipInit,
    	[PchSerialIoIndexSPI2] = PchSerialIoDisabled,
    	[PchSerialIoIndexUART0] = PchSerialIoSkipInit,
    	[PchSerialIoIndexUART1] = PchSerialIoDisabled,
diff --git a/src/mainboard/google/hatch/variants/kindred/overridetree.cb b/src/mainboard/google/hatch/variants/kindred/overridetree.cb
index 8993706..17b7b71 100644
--- a/src/mainboard/google/hatch/variants/kindred/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/kindred/overridetree.cb
@@ -1,14 +1,13 @@
 chip soc/intel/cannonlake
-
    register "SerialIoDevMode" = "{
-		[PchSerialIoIndexI2C0]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C1]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C2]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C3]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C4]  = PchSerialIoPci,
-		[PchSerialIoIndexI2C5]  = PchSerialIoPci,
-		[PchSerialIoIndexSPI0] = PchSerialIoPci,
-		[PchSerialIoIndexSPI1] = PchSerialIoPci,
+		[PchSerialIoIndexI2C0]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C1]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C2]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C3]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C4]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C5]  = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI0] = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI1] = PchSerialIoSkipInit,
    	[PchSerialIoIndexSPI2] = PchSerialIoDisabled,
    	[PchSerialIoIndexUART0] = PchSerialIoSkipInit,
    	[PchSerialIoIndexUART1] = PchSerialIoDisabled,
diff --git a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb
index cf422ed..e0f0ab7 100644
--- a/src/mainboard/google/hatch/variants/kohaku/overridetree.cb
+++ b/src/mainboard/google/hatch/variants/kohaku/overridetree.cb
@@ -1,13 +1,13 @@
 chip soc/intel/cannonlake
    register "SerialIoDevMode" = "{
-		[PchSerialIoIndexI2C0] = PchSerialIoPci,
-		[PchSerialIoIndexI2C1] = PchSerialIoPci,
-		[PchSerialIoIndexI2C2] = PchSerialIoPci,
-		[PchSerialIoIndexI2C3] = PchSerialIoPci,
-		[PchSerialIoIndexI2C4] = PchSerialIoPci,
-		[PchSerialIoIndexI2C5] = PchSerialIoPci,
-		[PchSerialIoIndexSPI0] = PchSerialIoPci,
-		[PchSerialIoIndexSPI1] = PchSerialIoPci,
+		[PchSerialIoIndexI2C0] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C1] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C2] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C3] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C4] = PchSerialIoSkipInit,
+		[PchSerialIoIndexI2C5] = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI0] = PchSerialIoSkipInit,
+		[PchSerialIoIndexSPI1] = PchSerialIoSkipInit,
    	[PchSerialIoIndexSPI2] = PchSerialIoDisabled,
    	[PchSerialIoIndexUART0] = PchSerialIoSkipInit,
    	[PchSerialIoIndexUART1] = PchSerialIoDisabled,

[Furquan Shaikh (Code Review)]

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

Change subject: mb/google/hatch: Change SerialIoDevMode for I2C, SPI, and UART
......................................................................

Patch Set 1:

(1 comment)

I

https://review.coreboot.org/c/coreboot/+/34246/1/src/mainboard/google/hatch/... 
File src/mainboard/google/hatch/variants/hatch/overridetree.cb:

https://review.coreboot.org/c/coreboot/+/34246/1/src/mainboard/google/hatch/... 
PS1, Line 4: PchSerialIoSkipInit
It looks like PchSerialIoSkipInit is not a valid setting for I2C and SPI. See https://review.coreboot.org/cgit/coreboot.git/tree/src/vendorcode/intel/fsp/....

We need to update soc common code to ensure that it does not allow this setting.

[Tim Wawrzynczak (Code Review)]

Tim Wawrzynczak has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/34246 )

Change subject: mb/google/hatch: Change SerialIoDevMode for I2C, SPI, and UART
......................................................................

Abandoned

Confusion over SkipInit