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

[Simon Glass]

Hi Wolfgang,

On Fri, 14 Aug 2020 at 08:04, Wolfgang Wallner
<wolfgang.wallner at 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