From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Fri, 18 Oct 2019 20:43:59 -0600
Subject: [U-Boot] [PATCH v2 12/38] x86: spi: Add a driver for the Intel
	Fast SPI interface
In-Reply-To: <CAEUhbmUvZnx7ZOJU1MjndLRZMWegUL0t-73cweBaGbTLpvoGqQ@mail.gmail.com>
References: <20190925141147.191166-1-sjg@chromium.org>
 <20190925141147.191166-13-sjg@chromium.org>
 <CAEUhbmUvZnx7ZOJU1MjndLRZMWegUL0t-73cweBaGbTLpvoGqQ@mail.gmail.com>
Message-ID: <CAPnjgZ0GwXCVtkFUtNK+68kRWNAT_uOVPmEK2WVSqQymM=bQLQ@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Bin,

On Wed, 9 Oct 2019 at 07:10, Bin Meng <bmeng.cn@gmail.com> wrote:

> Hi Simon,
>
> On Wed, Sep 25, 2019 at 10:12 PM Simon Glass <sjg@chromium.org> wrote:
> >
> > This provides access to SPI flash both through a read-only memory map and
> > with operations to erase and write flash. It supports 4KB or 64KB erase
> > sizes.
> >
>
> I read the SPI controller chapter in the datasheet of apollolake as
> well as some latest x86 desktop processors and believe they are
> compatible with the existing LPC SPI controller, the one used in ICH7
> and ICH9. In fact, in Linux, these SPI controllers are supported by
> the same intel-spi-pci.c driver (probe interface, PCI), and is
> actually using the same intel-spi.c core driver.
>
> So I think we should consider expanding the existing ICH SPI driver in
> U-Boot to support the PCI variants too.
>

As discussed elsewhere I'll give this another try.

>
> > This driver is used by Apollolake.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add the missing header file
> > - Add support for of-platdata for TPL
> >
> >  drivers/mtd/spi/Kconfig          |   9 +
> >  drivers/mtd/spi/Makefile         |   1 +
> >  drivers/mtd/spi/intel_fast_spi.c | 360 +++++++++++++++++++++++++++++++
> >  3 files changed, 370 insertions(+)
> >  create mode 100644 drivers/mtd/spi/intel_fast_spi.c
> >
>

[..]


> > +static void fast_spi_early_init(struct udevice *dev)
> > +{
> > +       struct fast_spi_platdata *plat = dev_get_platdata(dev);
> > +       pci_dev_t pdev = plat->bdf;
> > +
> > +       /* Clear BIT 1-2 SPI Command Register */
> > +       //remove
> > +       pci_x86_clrset_config(pdev, PCI_COMMAND, PCI_COMMAND_MASTER |
> > +                             PCI_COMMAND_MEMORY, 0, PCI_SIZE_8);
> > +
> > +       /* Program Temporary BAR for SPI */
> > +       pci_x86_write_config(pdev, PCI_BASE_ADDRESS_0,
> > +                            plat->mmio_base |
> PCI_BASE_ADDRESS_SPACE_MEMORY,
> > +                            PCI_SIZE_32);
> > +
> > +       /* Enable Bus Master and MMIO Space */
> > +       pci_x86_clrset_config(pdev, PCI_COMMAND, 0, PCI_COMMAND_MASTER |
> > +                             PCI_COMMAND_MEMORY, PCI_SIZE_8);
> > +
> > +       /*
> > +        * Disable the BIOS write protect so write commands are allowed.
> > +        * Enable Prefetching and caching.
> > +        */
> > +       pci_x86_clrset_config(pdev, SPIBAR_BIOS_CONTROL,
> > +                             SPIBAR_BIOS_CONTROL_EISS |
> > +                             SPIBAR_BIOS_CONTROL_CACHE_DISABLE,
> > +                             SPIBAR_BIOS_CONTROL_WPD |
> > +                             SPIBAR_BIOS_CONTROL_PREFETCH_ENABLE,
> PCI_SIZE_8);
>
> I wonder why are these needed, IIUC we rely on FSP2 to initialize the
> SoC and I believe for SPI these are some basic settings and registers
> should have already be programmed to sane values, no?
>

Which part of the FSP2? This code is executed in TPL so no part of the FSP
has been executed yet. Even when we figure out how to run FSP-T (which will
run befor ethis code) I am not sure that the FSP will do this.

Regards,
Simon