x86: apl: Acessing SPI flash when booting with Coreboot/U-Boot

Hi Simon,
Since commit 609b90a6a9c0 ("x86: spi: Rewrite logic for obtaining the SPI memory map") I have trouble accessing the SPI flash on my Apollo Lake board when booting with Coreboot and having U-Boot as the payload.
Accessing the SPI flash returns -91 (EPROTOTYPE).
My understanding of what happens is the following:
- ich_spi_ofdata_to_platdata() calls ich_spi_get_basics(), the parameter 'can_probe' is hardcoded to true - There is no PCH device in my devicetree (there is also no PCH driver contained in my build) - As can_probe is true, ich_spi_get_basics() tries to find a PCH device and returns -EPROTOTYPE as it finds none
As far as I see the PCH is not used in the code paths relevant to Apollo Lake. I think this behavior can not be triggered in a standalone U-Boot on Apollo Lake, as in this case arch_cpu_init_tpl() requires an LPC, which is below the PCH, and so a PCH has to be part of the devicetree anyway. In this case a PCH would be found in ich_spi_get_basics(), but it would not get used.
As a quick workaround [1], I have set can_probe hardcoded to false, and with this change I can access the SPI flash again.
Do you have any advice on how to fix this? How about making the value for can_probe depend on a check for ich_version == ICHV_APL?
regards, Wolfgang
[1] Workaround patch:
--- a/drivers/spi/ich.c +++ b/drivers/spi/ich.c @@ -1016,7 +1016,7 @@ static int ich_spi_ofdata_to_platdata(struct udevice *dev) #if !CONFIG_IS_ENABLED(OF_PLATDATA) struct ich_spi_priv *priv = dev_get_priv(dev);
- ret = ich_spi_get_basics(dev, true, &priv->pch, &plat->ich_version, + ret = ich_spi_get_basics(dev, false, &priv->pch, &plat->ich_version,

Hi Wolfgang,
On Fri, 14 Aug 2020 at 08:04, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
Since commit 609b90a6a9c0 ("x86: spi: Rewrite logic for obtaining the SPI memory map") I have trouble accessing the SPI flash on my Apollo Lake board when booting with Coreboot and having U-Boot as the payload.
Accessing the SPI flash returns -91 (EPROTOTYPE).
My understanding of what happens is the following:
- ich_spi_ofdata_to_platdata() calls ich_spi_get_basics(), the parameter 'can_probe' is hardcoded to true
- There is no PCH device in my devicetree (there is also no PCH driver contained in my build)
- As can_probe is true, ich_spi_get_basics() tries to find a PCH device and returns -EPROTOTYPE as it finds none
As far as I see the PCH is not used in the code paths relevant to Apollo Lake. I think this behavior can not be triggered in a standalone U-Boot on Apollo Lake, as in this case arch_cpu_init_tpl() requires an LPC, which is below the PCH, and so a PCH has to be part of the devicetree anyway. In this case a PCH would be found in ich_spi_get_basics(), but it would not get used.
As a quick workaround [1], I have set can_probe hardcoded to false, and with this change I can access the SPI flash again.
Do you have any advice on how to fix this? How about making the value for can_probe depend on a check for ich_version == ICHV_APL?
That might be OK I think.
I am quite unhappy with how this has turned out. It seems like we might be able to refactor it to reduce the number of cases.
regards, Wolfgang
[1] Workaround patch:
--- a/drivers/spi/ich.c +++ b/drivers/spi/ich.c @@ -1016,7 +1016,7 @@ static int ich_spi_ofdata_to_platdata(struct udevice *dev) #if !CONFIG_IS_ENABLED(OF_PLATDATA) struct ich_spi_priv *priv = dev_get_priv(dev);
ret = ich_spi_get_basics(dev, true, &priv->pch, &plat->ich_version,
ret = ich_spi_get_basics(dev, false, &priv->pch, &plat->ich_version,
Regards, Simon

Hi Simon, Wolfgang,
On Sun, Aug 16, 2020 at 11:40 AM Simon Glass sjg@chromium.org wrote:
Hi Wolfgang,
On Fri, 14 Aug 2020 at 08:04, Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
Hi Simon,
Since commit 609b90a6a9c0 ("x86: spi: Rewrite logic for obtaining the SPI memory map") I have trouble accessing the SPI flash on my Apollo Lake board when booting with Coreboot and having U-Boot as the payload.
Accessing the SPI flash returns -91 (EPROTOTYPE).
My understanding of what happens is the following:
- ich_spi_ofdata_to_platdata() calls ich_spi_get_basics(), the parameter 'can_probe' is hardcoded to true
- There is no PCH device in my devicetree (there is also no PCH driver contained in my build)
- As can_probe is true, ich_spi_get_basics() tries to find a PCH device and returns -EPROTOTYPE as it finds none
As far as I see the PCH is not used in the code paths relevant to Apollo Lake. I think this behavior can not be triggered in a standalone U-Boot on Apollo Lake, as in this case arch_cpu_init_tpl() requires an LPC, which is below the PCH, and so a PCH has to be part of the devicetree anyway. In this case a PCH would be found in ich_spi_get_basics(), but it would not get used.
As a quick workaround [1], I have set can_probe hardcoded to false, and with this change I can access the SPI flash again.
Do you have any advice on how to fix this? How about making the value for can_probe depend on a check for ich_version == ICHV_APL?
That might be OK I think.
I am quite unhappy with how this has turned out. It seems like we might be able to refactor it to reduce the number of cases.
Since this patch is viewed as only a workaround, would you propose a fix for this issue?
Regards, Bin
participants (3)
-
Bin Meng
-
Simon Glass
-
Wolfgang Wallner