[U-Boot] [PATCH v2 14/38] spi: Add support for memory-mapped flash
Simon Glass
sjg at chromium.org
Wed Oct 16 16:40:48 UTC 2019
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?
This used to be implemented but is not supported anymore. I think we
should wait until the DM SPI flash migration is complete before trying
again.
Also it is not just reading. The data is used in-place in some cases,
so we do want to know the map region.
>
> >>>> 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.
The problem is that it is ops within ops so doesn't follow driver model.
>
> Some SPI controllers provide interfaces to work with any type of SPI
> flashes like SPI NOR, SPI NAND, SPI SRAM etc. They may also provide
> specialized accelerated interface to work with them. spi_mem_ops
> abstracts these details and provides flash agnostic interface b/w flash
> driver and SPI controller.
> This means single SPI controller driver can support different variety of
> SPI flashes such as SPI NOR (QSPI/OSPI), SPI NAND, etc
> Most drivers implementing spi_mem_ops provide memory mapped access to
> flash as well. (I see ich.c does too).
>
> If this HW is a generic SPI controller that can talk to any or subset of
> SPI NOR flashes (such as the ones listed in
> drivers/mtd/spi/spi-nor-ids.c) and provides a way to configure commands
> for SPI Flash operations, then this needs to be modeled as SPI
> controller implementing spi_mem_ops. spi-nor-core.c will take care flash
> details.
Yes I'm going to try to make use of the existing ish driver as Bin
suggested. That might help avoid 'working around' the driver.
Regards,
Simon
More information about the U-Boot
mailing list