[U-Boot] [PATCH v2 14/38] spi: Add support for memory-mapped flash

Simon Glass sjg at chromium.org
Fri Oct 18 20:37:09 UTC 2019


Hi Bin,

On Fri, 18 Oct 2019 at 09:38, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon,
>
> On Fri, Oct 18, 2019 at 10:14 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Thu, 17 Oct 2019 at 20:32, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Fri, Oct 18, 2019 at 10:22 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, 17 Oct 2019 at 08:28, Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Vignesh,
> > > > >
> > > > > On Wed, 16 Oct 2019 at 04:28, Vignesh Raghavendra <vigneshr at ti.com> wrote:
> > > > > >
> > > > > > Hi Simon,
> > > > > >
> > > > > > On 12/10/19 10:03 AM, Bin Meng wrote:
> > > > > > > Hi Simon,
> > > > > > >
> > > > > > > On Sat, Oct 12, 2019 at 11:08 AM Simon Glass <sjg at chromium.org> wrote:
> > > > > > >>
> > > > > > >> Hi Bin,
> > > > > > >>
> > > > > > >> On Wed, 9 Oct 2019 at 07:55, Bin Meng <bmeng.cn at gmail.com> wrote:
> > > > > > >>>
> > > > > > >>> Hi Simon,
> > > > > > >>>
> > > > > > >>> On Wed, Sep 25, 2019 at 10:12 PM Simon Glass <sjg at chromium.org> wrote:
> > > > > > >>>>
> > > > > > >>>> On x86 platforms the SPI flash can be mapped into memory so that the
> > > > > > >>>> contents can be read with normal memory accesses.
> > > > > > >>>>
> > > > > > >>>> Add a new SPI flash method to find the location of the SPI flash in
> > > > > > >>>> memory. This differs from the existing device-tree "memory-map" mechanism
> > > > > > >>>> in that the location can be discovered at run-time.
> > > > > > >>>>
> > > > > >
> > > > > > Whats is the usecase? Why can't spi_flash_read() be used instead?
> > > > > > Flash + Controller driver can underneath take care of using memory
> > > > > > mapped mode to read data from flash right while making sure that access
> > > > > > is within valid window?
> > > > >
> > > > > I can see spi_flash_read_dm() but it does not support returning a
> > > > > pointer to the data, only reading it.
> > > > >
> > > > > Also I cannot find any documentation about any of this. I've been
> > > > > looking in the doc/ directory.
> > > > >
> > > > > I found the spi_mem.h file but it doesn't mention the meaning of the
> > > > > in and out buffer pointers so I don't know how to use them.
> > > > >
> > > > > Is there an API missing or just comments/documentation?
> > > >
> > > > Apart from this I have found that the ich_spi driver does not work for
> > > > APL since it apparently only supports 'hardware sequencing'. I did try
> > > > getting software sequencing to work, but no dice.
> > > >
> > > > In addition, I found that enabling SPI flash, etc. added about 6KB of code!
> > > >
> > > > So I think it might be best to have two SPI drivers for x86 - one for
> > > > software sequencing and one for hardware?
> > >
> > > So if this is true, I think we can create Kconfig options (HW_SEQ and
> > > SW_SEQ) for ICH SPI driver, and select either one in the SoC Kconfig
> > > file.
> >
> > I think at least for TPL I'm going to need to bypass all the code as
> > it is just too big for TPL. What do you think?
>
> So what about other boards/drivers? Say we have drv_foo.c, is the best
> practice to create a separate drv_foo_tpl.c for use with TPL?

Yes, although in general we can export the functions from the core
driver, so the TPL one becomes pretty bare-bones - e.g. it handles
platdata but uses the operations of the core drivers.

>
> >
> > Should we have a separate HW SEQ driver? If you look at the HW SEQ
> > driver I have, it is very little code.
> >
> > Or are you saying we should have a combined driver with Kconfig
> > options to enable either or both of SW SEQ and HW SEQ?
> >
>
> Yes I was suggesting enable either or both SW and HW SEQ in the same driver.

OK I will take a look at that.

My code-size concern still stands though, but let's see how it looks.

>
> > One other wrinkle is that I have to have a separate driver anyway,
> > since of-platdata requires it. It is not possible to use of-platdata
> > in a generic driver since the struct definitions rely on the
> > compatible string.
> >
> > >
> > > But I see from the Linux kernel driver, it has:
> > >
> > > 300static int intel_spi_init(struct intel_spi *ispi)
> > > 301{
> > > 302        u32 opmenu0, opmenu1, lvscc, uvscc, val;
> > > 303        int i;
> > > 304
> > > 305        switch (ispi->info->type) {
> > > 306        case INTEL_SPI_BYT:
> > > 307                ispi->sregs = ispi->base + BYT_SSFSTS_CTL;
> > > 308                ispi->pregs = ispi->base + BYT_PR;
> > > 309                ispi->nregions = BYT_FREG_NUM;
> > > 310                ispi->pr_num = BYT_PR_NUM;
> > > 311                ispi->swseq_reg = true;
> > > 312
> > > 313                if (writeable) {
> > > 314                        /* Disable write protection */
> > > 315                        val = readl(ispi->base + BYT_BCR);
> > > 316                        if (!(val & BYT_BCR_WPD)) {
> > > 317                                val |= BYT_BCR_WPD;
> > > 318                                writel(val, ispi->base + BYT_BCR);
> > > 319                                val = readl(ispi->base + BYT_BCR);
> > > 320                        }
> > > 321
> > > 322                        ispi->writeable = !!(val & BYT_BCR_WPD);
> > > 323                }
> > > 324
> > > 325                break;
> > > 326
> > > 327        case INTEL_SPI_LPT:
> > > 328                ispi->sregs = ispi->base + LPT_SSFSTS_CTL;
> > > 329                ispi->pregs = ispi->base + LPT_PR;
> > > 330                ispi->nregions = LPT_FREG_NUM;
> > > 331                ispi->pr_num = LPT_PR_NUM;
> > > 332                ispi->swseq_reg = true;
> > > 333                break;
> > > 334
> > > 335        case INTEL_SPI_BXT:
> > > 336                ispi->sregs = ispi->base + BXT_SSFSTS_CTL;
> > > 337                ispi->pregs = ispi->base + BXT_PR;
> > > 338                ispi->nregions = BXT_FREG_NUM;
> > > 339                ispi->pr_num = BXT_PR_NUM;
> > > 340                ispi->erase_64k = true;
> > > 341                break;
> > >
> > > So for INTEL_SPI_BXT (which is for ApolloLake I believe)
> > > ispi->swseq_reg is not set to true which means it uses hardware
> > > sequencer, which seems to contradict with what you found.
> >
> > I found that it only supports hardware sequencing, so this matches.
>
> OK, I see. So the BXT only supports HW SEQ while LPT and BYT support
> both SW and HW SEQ.

OK, ta. How do I look up what LPT and BYT are?

Regards,
Simon


More information about the U-Boot mailing list