[PATCH] x86: spi: Only use the fast SPI peripheral when support

[Simon Glass]

Hi Bin,

On Tue, 31 Mar 2020 at 07:16, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 31 Mar 2020 at 03:50, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Mar 31, 2020 at 7:57 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Bin,
> > >
> > > On Mon, 30 Mar 2020 at 03:42, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sun, Mar 29, 2020 at 4:58 AM Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Bin,
> > > > >
> > > > > On Thu, 26 Mar 2020 at 10:38, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On Fri, Mar 27, 2020 at 12:20 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > HI Bin,
> > > > > > >
> > > > > > > On Wed, 25 Mar 2020 at 01:25, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > > >
> > > > > > > > Hi Simon,
> > > > > > > >
> > > > > > > > On Tue, Mar 24, 2020 at 9:45 PM Simon Glass <sjg at chromium.org> wrote:
> > > > > > > > >
> > > > > > > > > At present we query the memory map on boards which don't support it. Fix
> > > > > > > > > this by only doing it on Apollo Lake.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I wonder isn't this check already covered in mrccache_get_region() below:
> > > > > > > >
> > > > > > > > ret = dm_spi_get_mmap(dev, &map_base, &map_size, &offset);
> > > > > > > > if (!ret) {
> > > > > > > > entry->base = map_base;
> > > > > > > > } else {
> > > > > > > > ret = dev_read_u32_array(dev, "memory-map", reg, 2);
> > > > > > > > if (ret)
> > > > > > > > return log_msg_ret("Cannot find memory map\n", ret);
> > > > > > > > entry->base = reg[0];
> > > > > > > > }
> > > > > > >
> > > > > > > Yes it is, so long as dm_spi_get_mmap() returns an error, as it does
> > > > > > > with my patch.
> > > > > >
> > > > > > So does ich_get_mmap_bus() returns 0 on chromebook_link?
> > > > >
> > > > > Well on link the SPI peripheral is not a PCI device but a child of the
> > > > > PCH. It is possible to read the registers but at present this only
> > > > > works once you have the mmio_base (i.e. the PCH device is probed).
> > > > > This function needs to work before probing (since FSP-S access needs
> > > > > to happen without probing PCI).
> > > > >
> > > > > I suspect it would be possible to read the PCH base without probing
> > > > > it, but it does add quite a bit of special-case code. What do you
> > > > > think?
> > > >
> > > > I've looked at this. So this function mrccache_get_region() is broken
> > > > on Minnowmax too. The call to uclass_find_first_device() returns
> > > > nothing because SPI flash is not probed hence no SF device is found.
> > >
> > > OK, so add to the DT, or do something else?
> >
> > There is already "memor-map" propert (see below) y in Chromebook DTS
> > so I am not sure what else needs to be added to the DT, as you
> > mentioned?
> >
> > spi: spi {
> > #address-cells = <1>;
> > #size-cells = <0>;
> > compatible = "intel,ich9-spi";
> > u-boot,dm-pre-reloc;
> > spi-flash at 0 {
> > #size-cells = <1>;
> > #address-cells = <1>;
> > u-boot,dm-pre-reloc;
> > reg = <0>;
> > compatible = "winbond,w25q64",
> > "jedec,spi-nor";
> > memory-map = <0xff800000 0x00800000>;
> > rw-mrc-cache {
> > label = "rw-mrc-cache";
> > reg = <0x003e0000 0x00010000>;
> > u-boot,dm-pre-reloc;
> > };
> > };
> > };
> >
> > So isn't the failure on Link caused by uclass_find_first_device()? I
> > think replacing uclass_find_first_device() with uclass_first_device()
> > will fix failures for at least Minnowmax? The comment says:
> >
> > /*
> > * Find the flash chip within the SPI controller node. Avoid probing
> > * the device here since it may put it into a strange state where the
> > * memory map cannot be read.
> > */
> >
> > What issue did you see if we probe the SPI controller and flash?
>
> I think you have the wrong end of the stick and conflating the
> bahaviour on several different platforms.
>
> link - uses memory-map, hence this patch to make sure that mmap
> returns an error so that link falls back to memory-map
> coral - uses mmap
> minnowmax - not sure, but I suspect it needs memory-map too

At present link and samus are broken without this patch.

It just reverts us back to the behaviour before the ICH TPL support.
What can we do to get this applied?

Regards,
Simon