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

Simon Glass sjg at chromium.org
Tue Mar 31 15:16:12 CEST 2020


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

Regards,
Simon


More information about the U-Boot mailing list