Change in coreboot[master]: soc/intel/cannonlake: Configure GPIOs again after FSP-S is done
Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31250 )
Change subject: soc/intel/cannonlake: Configure GPIOs again after FSP-S is done ......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS5:
I had used this diff:
diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c index 294218c42f..34d46bfe4d 100644 --- a/src/soc/intel/common/block/gpio/gpio.c +++ b/src/soc/intel/common/block/gpio/gpio.c @@ -265,10 +265,10 @@ static void gpio_configure_pad(const struct pad_config *cfg) soc_pad_conf &= mask[i]; soc_pad_conf |= pad_conf & ~mask[i];
if (IS_ENABLED(CONFIG_DEBUG_GPIO))
if (soc_pad_conf != pad_conf) printk(BIOS_DEBUG,
"gpio_padcfg [0x%02x, %02zd] DW%d [0x%08x : 0x%08x"" : 0x%08x]\n",
"%d: gpio_padcfg [0x%02x, %02zd] DW%d [0x%08x : 0x%08x"" : 0x%08x]\n", cfg->pad, comm->port, relative_pad_in_comm(comm, cfg->pad), i, pad_conf,/* old value */ cfg->pad_config[i],/* value passed from gpio table */And this was the output I got for hatch:
17: gpio_padcfg [0x6e, 17] DW0 [0x40000600 : 0x40000400 : 0x40000400] 51: gpio_padcfg [0x6e, 51] DW1 [0x00003c6c : 0x00000000 : 0x0000006c] 52: gpio_padcfg [0x6e, 52] DW1 [0x00003c6d : 0x00000000 : 0x0000006d] 53: gpio_padcfg [0x6e, 53] DW1 [0x00003c6e : 0x00000000 : 0x0000006e] 54: gpio_padcfg [0x6e, 54] DW1 [0x00003c6f : 0x00000000 : 0x0000006f] 55: gpio_padcfg [0x6e, 55] DW1 [0x00003c70 : 0x00000000 : 0x00000070] 58: gpio_padcfg [0x6e, 58] DW0 [0x40000500 : 0x40000100 : 0x40000100]
sd card pins, configured by fsp when scs/sdcard is enabled; I assume they were fixed by CB:34900
31: gpio_padcfg [0x6e, 31] DW0 [0x40000700 : 0x40000400 : 0x40000400] 33: gpio_padcfg [0x6e, 33] DW0 [0x40000702 : 0x40000400 : 0x40000402]
clkreq; affected pads are B5-B10; if these get overridden, then the clkreq settings in the devicetree mismatch pad config; iow one of both is wrong
201: gpio_padcfg [0x6a, 20] DW0 [0x40000500 : 0x40000100 : 0x40000100] 202: gpio_padcfg [0x6a, 21] DW0 [0x40100500 : 0x40900100 : 0x40900100]
C20/C21, that's UART2, which is disabled in the dt, which results in SerialIoUartMode[2]=0 and should keep fsp from changing the pads... strange
206: gpio_padcfg [0x6a, 25] DW0 [0x40000402 : 0x40000800 : 0x40000802]
That one is set to native when SataPortsInterlockSw[x]=1; however, no board sets it. strange o.O
220: gpio_padcfg [0x6a, 39] DW0 [0x40000500 : 0x40000100 : 0x40000100] 221: gpio_padcfg [0x6a, 40] DW0 [0x40000500 : 0x40000100 : 0x40000100] 223: gpio_padcfg [0x6a, 42] DW0 [0x40000500 : 0x40000100 : 0x40000100] 225: gpio_padcfg [0x6a, 44] DW0 [0x40000500 : 0x40000100 : 0x40000100] 227: gpio_padcfg [0x6a, 46] DW0 [0x40000500 : 0x40000100 : 0x40000100]
DPPC, HPD, etc. fixed by CB:31520
I have this in notes and internal bugs:
In some cases, FSP is just configuring a GPIO for native mode when the board requires it to be a GPI or GPO. In other cases, the termination is set differently and in some cases no-connect GPIOs are being configured as native functions.
Again from internal bug, I see that this was fixed in FSP or coreboot by adding and initializing required UPDs. Examples: https://review.coreboot.org/c/coreboot/+/31520 https://review.coreboot.org/c/coreboot/+/34900
Eventually, for TGL/JSL, Intel added `GpioOverride` that skips all GPIO configuration in FSP if this UPD is set by coreboot.
Yeah, would be nice to have that for other platforms as well :/
-
Michael Niewöhner (Code Review)