Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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.
>
> This fixes booting on chromebook_link.
>
> Signed-off-by: Simon Glass
> Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
> ---
>
> drivers/spi/ich.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
> index a9d7715a55..9f8af45242 100644
> --- a/drivers/spi/ich.c
> +++ b/drivers/spi/ich.c
> @@ -637,7 +637,10 @@ static int ich_get_mmap(struct udevice *dev, ulong
> *map_basep, uint *map_sizep,
> uint *offsetp)
> {
> struct udevice *bus = dev_get_parent(dev);
> + struct ich_spi_platdata *plat = dev_get_platdata(bus);
>
> + if (plat->ich_version != ICHV_APL)
> + return -ENOENT;
> return ich_get_mmap_bus(bus, map_basep, map_sizep, offsetp);
> }
Although the check added here is correct, it is still an incomplete fix.
I don't understand why chromebook_link boot failure is caused by this
because as I mentioned before in mrccache_get_region() the call to
uclass_find_first_device(UCLASS_SPI_FLASH, &dev) already returns hence
there is no chance to execute dm_spi_get_mmap() on link.
Regards,
Bin
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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@0 {
> > > #size-cells = <1>;
> > > #address-cells = <1>;
> > > u-boot,dm-pre-reloc;
> > > reg = <0>;
> > > compatible = "winbond,w25q64",
> > > "jedec,spi-nor";
> > > memory-map = <0xff80 0x0080>;
> > > rw-mrc-cache {
> > > label = "rw-mrc-cache";
> > > reg = <0x003e 0x0001>;
> > > 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
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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@0 {
> > #size-cells = <1>;
> > #address-cells = <1>;
> > u-boot,dm-pre-reloc;
> > reg = <0>;
> > compatible = "winbond,w25q64",
> > "jedec,spi-nor";
> > memory-map = <0xff80 0x0080>;
> > rw-mrc-cache {
> > label = "rw-mrc-cache";
> > reg = <0x003e 0x0001>;
> > 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
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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@0 {
> #size-cells = <1>;
> #address-cells = <1>;
> u-boot,dm-pre-reloc;
> reg = <0>;
> compatible = "winbond,w25q64",
> "jedec,spi-nor";
> memory-map = <0xff80 0x0080>;
> rw-mrc-cache {
> label = "rw-mrc-cache";
> reg = <0x003e 0x0001>;
> 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
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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@0 {
#size-cells = <1>;
#address-cells = <1>;
u-boot,dm-pre-reloc;
reg = <0>;
compatible = "winbond,w25q64",
"jedec,spi-nor";
memory-map = <0xff80 0x0080>;
rw-mrc-cache {
label = "rw-mrc-cache";
reg = <0x003e 0x0001>;
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?
Regards,
Bin
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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?
Regards,
Simon
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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.
Regards,
Bin
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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?
Regards,
Simon
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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?
Regards,
Simon
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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?
Regards,
Bin
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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.
>
> > This fixes booting on chromebook_link.
> >
> > Signed-off-by: Simon Glass
> > Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
> > ---
> >
> > drivers/spi/ich.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
> > index a9d7715a55..9f8af45242 100644
> > --- a/drivers/spi/ich.c
> > +++ b/drivers/spi/ich.c
> > @@ -637,7 +637,10 @@ static int ich_get_mmap(struct udevice *dev, ulong
> > *map_basep, uint *map_sizep,
> > uint *offsetp)
> > {
> > struct udevice *bus = dev_get_parent(dev);
> > + struct ich_spi_platdata *plat = dev_get_platdata(bus);
> >
> > + if (plat->ich_version != ICHV_APL)
> > + return -ENOENT;
> > return ich_get_mmap_bus(bus, map_basep, map_sizep, offsetp);
> > }
Regards,
Simon
Re: [PATCH] x86: spi: Only use the fast SPI peripheral when support
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];
}
> This fixes booting on chromebook_link.
>
> Signed-off-by: Simon Glass
> Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
> ---
>
> drivers/spi/ich.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
> index a9d7715a55..9f8af45242 100644
> --- a/drivers/spi/ich.c
> +++ b/drivers/spi/ich.c
> @@ -637,7 +637,10 @@ static int ich_get_mmap(struct udevice *dev, ulong
> *map_basep, uint *map_sizep,
> uint *offsetp)
> {
> struct udevice *bus = dev_get_parent(dev);
> + struct ich_spi_platdata *plat = dev_get_platdata(bus);
>
> + if (plat->ich_version != ICHV_APL)
> + return -ENOENT;
> return ich_get_mmap_bus(bus, map_basep, map_sizep, offsetp);
> }
Regards,
Bin
[PATCH] x86: spi: Only use the fast SPI peripheral when support
At present we query the memory map on boards which don't support it. Fix
this by only doing it on Apollo Lake.
This fixes booting on chromebook_link.
Signed-off-by: Simon Glass
Fixes: 92842147c31 ("spi: ich: Add support for get_mmap() method")
---
drivers/spi/ich.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
index a9d7715a55..9f8af45242 100644
--- a/drivers/spi/ich.c
+++ b/drivers/spi/ich.c
@@ -637,7 +637,10 @@ static int ich_get_mmap(struct udevice *dev, ulong
*map_basep, uint *map_sizep,
uint *offsetp)
{
struct udevice *bus = dev_get_parent(dev);
+ struct ich_spi_platdata *plat = dev_get_platdata(bus);
+ if (plat->ich_version != ICHV_APL)
+ return -ENOENT;
return ich_get_mmap_bus(bus, map_basep, map_sizep, offsetp);
}
--
2.25.1.696.g5e7596f4ac-goog
