From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bin Meng Date: Mon, 20 Apr 2020 18:01:20 +0800 Subject: [PATCH] x86: spi: Only use the fast SPI peripheral when support In-Reply-To: References: <20200324074524.1.Ibc9c511db58caa8a1e4c56d7e7824d7690718aeb@changeid> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Simon, On Mon, Apr 20, 2020 at 3:14 AM Simon Glass wrote: > > Hi Bin, > > On Tue, 31 Mar 2020 at 07:16, Simon Glass wrote: > > > > Hi Bin, > > > > On Tue, 31 Mar 2020 at 03:50, Bin Meng wrote: > > > > > > Hi Simon, > > > > > > On Tue, Mar 31, 2020 at 7:57 AM Simon Glass wrote: > > > > > > > > Hi Bin, > > > > > > > > On Mon, 30 Mar 2020 at 03:42, Bin Meng wrote: > > > > > > > > > > Hi Simon, > > > > > > > > > > On Sun, Mar 29, 2020 at 4:58 AM Simon Glass wrote: > > > > > > > > > > > > Hi Bin, > > > > > > > > > > > > On Thu, 26 Mar 2020 at 10:38, Bin Meng wrote: > > > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > On Fri, Mar 27, 2020 at 12:20 AM Simon Glass wrote: > > > > > > > > > > > > > > > > HI Bin, > > > > > > > > > > > > > > > > On Wed, 25 Mar 2020 at 01:25, Bin Meng wrote: > > > > > > > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > > > > > On Tue, Mar 24, 2020 at 9:45 PM Simon Glass 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? Sorry I missed this. I will take a look. Regards, Bin