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

[Bin Meng]

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.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > Changes in v2: None
> > >
> > >  drivers/mtd/spi/sandbox_direct.c | 11 +++++++++++
> > >  drivers/mtd/spi/sf-uclass.c      | 11 +++++++++++
> > >  include/spi_flash.h              | 27 +++++++++++++++++++++++++++
> > >  test/dm/sf.c                     |  9 +++++++++
> > >  4 files changed, 58 insertions(+)
> > >
> > > diff --git a/drivers/mtd/spi/sandbox_direct.c b/drivers/mtd/spi/sandbox_direct.c
> > > index 43d8907710c..fb515edcb7c 100644
> > > --- a/drivers/mtd/spi/sandbox_direct.c
> > > +++ b/drivers/mtd/spi/sandbox_direct.c
> > > @@ -68,6 +68,16 @@ static int sandbox_direct_get_sw_write_prot(struct udevice *dev)
> > >         return priv->write_prot++ ? 1 : 0;
> > >  }
> > >
> > > +static int sandbox_direct_get_mmap(struct udevice *dev, ulong *map_basep,
> > > +                                  size_t *map_sizep, u32 *offsetp)
> > > +{
> > > +       *map_basep = 0x1000;
> > > +       *map_sizep = 0x2000;
> > > +       *offsetp = 0x100;
> > > +
> > > +       return 0;
> > > +}
> > > +
> > >  static int sandbox_direct_probe(struct udevice *dev)
> > >  {
> > >         struct sandbox_direct_priv *priv = dev_get_priv(dev);
> > > @@ -82,6 +92,7 @@ static struct dm_spi_flash_ops sandbox_direct_ops = {
> > >         .write = sandbox_direct_write,
> > >         .erase = sandbox_direct_erase,
> > >         .get_sw_write_prot = sandbox_direct_get_sw_write_prot,
> > > +       .get_mmap = sandbox_direct_get_mmap,
> > >  };
> > >
> > >  static const struct udevice_id sandbox_direct_ids[] = {
> > > diff --git a/drivers/mtd/spi/sf-uclass.c b/drivers/mtd/spi/sf-uclass.c
> > > index 719a2fd23ae..127ec7e7aa6 100644
> > > --- a/drivers/mtd/spi/sf-uclass.c
> > > +++ b/drivers/mtd/spi/sf-uclass.c
> > > @@ -28,6 +28,17 @@ int spi_flash_erase_dm(struct udevice *dev, u32 offset, size_t len)
> > >         return log_ret(sf_get_ops(dev)->erase(dev, offset, len));
> > >  }
> > >
> > > +int spi_flash_get_mmap(struct udevice *dev, ulong *map_basep, size_t *map_sizep,
> > > +                      u32 *offsetp)
> > > +{
> > > +       struct dm_spi_flash_ops *ops = sf_get_ops(dev);
> > > +
> > > +       if (!ops->get_mmap)
> > > +               return -ENOSYS;
> > > +
> > > +       return ops->get_mmap(dev, map_basep, map_sizep, offsetp);
> > > +}
> > > +
> > >  int spl_flash_get_sw_write_prot(struct udevice *dev)
> > >  {
> > >         struct dm_spi_flash_ops *ops = sf_get_ops(dev);
> > > diff --git a/include/spi_flash.h b/include/spi_flash.h
> > > index 55b4721813a..840189e22c7 100644
> > > --- a/include/spi_flash.h
> > > +++ b/include/spi_flash.h
> > > @@ -47,6 +47,19 @@ struct dm_spi_flash_ops {
> > >          *      other -ve value on error
> > >          */
> > >         int (*get_sw_write_prot)(struct udevice *dev);
> > > +
> > > +       /**
> > > +        * get_mmap() - Get memory-mapped SPI
> > > +        *
> > > +        * @dev:        SPI flash device
> > > +        * @map_basep:  Returns base memory address for mapped SPI
> > > +        * @map_sizep:  Returns size of mapped SPI
> > > +        * @offsetp:    Returns start offset of SPI flash where the map works
> > > +        *      correctly (offsets before this are not visible)
> > > +        * @return 0 if OK, -EFAULT if memory mapping is not available
> > > +        */
> >
> > I feel odd to add such an op to the flash op, as memory address is not
> > determined by the flash itself, but the SPI flash controller. We
> > probably should add the op to the SPI flash controller instead.
>
> So do you think this should be added to UCLASS_SPI?

Yes, I think so. Jagan, what's your recommendation?

>
> As it stands we don't actually use that uclass with this SPI flash
> driver - it implements the SPI_FLASH interface directly.
>
> But given that I'm going to try to use the same ich.c driver this
> should be easy enough.
>
> I've just found the weird mem_ops argument within struct dm_spi_ops...oh dear.
>

The mem_ops was added by Vignesh. I believe that was derived from the
Linux kernel.

Regards,
Bin