• Home
  • Line#
  • Scopes#
  • Navigate#
  • Raw
  • Download
1# Background
2
3CB:31250 ("soc/intel/cannonlake: Configure GPIOs again after FSP-S is
4done") introduced a workaround in coreboot for `soc/intel/cannonlake`
5platforms to save and restore GPIO configuration performed by
6mainboard across call to FSP Silicon Init (FSP-S). This workaround was
7required because FSP-S was configuring GPIOs differently than
8mainboard resulting in boot and runtime issues because of
9misconfigured GPIOs. This issue was observed on `google/hatch`
10mainboard and was raised with Intel to get the FSP behavior
11fixed. Until the fix in FSP was available, this workaround was used to
12ensure that the mainboards can operate correctly and were not impacted
13by the GPIO misconfiguration in FSP-S.
14
15The issues observed on `google/hatch` mainboard were fixed by adding
16(if required) and initializing appropriate FSP UPDs. This UPD
17initialization ensured that FSP did not configure any GPIOs
18differently than the mainboard configuration. Fixes included:
19 * CB:31375 ("soc/intel/cannonlake: Configure serial debug uart")
20 * CB:31520 ("soc/intel/cannonlake: Assign FSP UPDs for HPD and Data/CLK of DDI ports")
21 * CB:32176 ("mb/google/hatch: Update GPIO settings for SD card and SPI1 Chip select")
22 * CB:34900 ("soc/intel/cnl: Add provision to configure SD controller write protect pin")
23
24With the above changes merged, it was verified on `google/hatch`
25mainboard that the workaround for GPIO reconfiguration was not
26needed. However, at the time, we missed dropping the workaround in
27'soc/intel/cannonlake`. Currently, this workaround is used by the
28following mainboards:
29 * `google/drallion`
30 * `google/sarien`
31 * `purism/librem_cnl`
32 * `system76/lemp9`
33
34As verified on `google/hatch`, FSP v1263 included all UPD additions
35that were required for addressing this issue.
36
37# Proposal
38
39* The workaround can be safely dropped from `soc/intel/cannonlake`
40  only after the above mainboards have verified that FSP-S does not
41  configure any pads differently than the mainboard in coreboot. Since
42  the fix included initialization of FSP UPDs correctly, the above
43  mainboards can use the following diff to check what pads change
44  after FSP-S has run:
45
46```
47diff --git a/src/soc/intel/common/block/gpio/gpio.c b/src/soc/intel/common/block/gpio/gpio.c
48index 28e78fb366..0cce41b316 100644
49--- a/src/soc/intel/common/block/gpio/gpio.c
50+++ b/src/soc/intel/common/block/gpio/gpio.c
51@@ -303,10 +303,10 @@ static void gpio_configure_pad(const struct pad_config *cfg)
52                /* Patch GPIO settings for SoC specifically */
53                soc_pad_conf = soc_gpio_pad_config_fixup(cfg, i, soc_pad_conf);
54
55-               if (CONFIG(DEBUG_GPIO))
56+               if (soc_pad_conf != pad_conf)
57                        printk(BIOS_DEBUG,
58-                       "gpio_padcfg [0x%02x, %02zd] DW%d [0x%08x : 0x%08x"
59-                       " : 0x%08x]\n",
60+                       "%d: gpio_padcfg [0x%02x, %02zd] DW%d [0x%08x : 0x%08x"
61+                       " : 0x%08x]\n", cfg->pad,
62                        comm->port, relative_pad_in_comm(comm, cfg->pad), i,
63                        pad_conf,/* old value */
64                        cfg->pad_config[i],/* value passed from gpio table */
65```
66
67Depending upon the pads that are misconfigured by FSP-S, these
68mainboards will have to set UPDs appropriately. Once this is verified
69by the above mainboards, the workaround implemented in CB:31250 can be
70dropped.
71
72* The fix implemented in FSP/coreboot for `soc/intel/cannonlake`
73  platforms is not really the right long term solution for the
74  problem. Ideally, FSP should not be touching any GPIO configuration
75  and letting coreboot configure the pads as per mainboard
76  design. This recommendation was accepted and implemented by Intel
77  starting with Jasper Lake and Tiger Lake platforms using a single
78  UPD `GpioOverride` that coreboot can set so that FSP does not change
79  any GPIO configuration. However, this implementation is not
80  backported to any older platforms. Given the issues that we have
81  observed across different platforms, the second proposal is to:
82
83  - Add a Kconfig `CHECK_GPIO_CONFIG_CHANGES` that enables checks
84    in coreboot to stash GPIO pad configuration before various calls
85    to FSP and compares the configuration on return from FSP.
86  - This will have to be implemented as part of
87    drivers/intel/fsp/fsp2_0/ to check for the above config selection
88    and make callbacks `gpio_snapshot()` and `gpio_verify_snapshot()`
89    to identify and print information about pads that have changed
90    configuration after calls to FSP.
91  - This config can be kept disabled by default and mainboard
92    developers can enable them as and when required for debug.
93  - This will be helpful not just for the `soc/intel/cannonlake`
94    platforms that want to get rid of the above workaround, but also
95    for all future platforms using FSP to identify and catch any GPIO
96    misconfigurations that might slip in to any platforms (in case the
97    `GpioOverride` UPD is not honored by any code path within FSP).
98
99