
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@chromium.org wrote:
Hi Bin,
On Wed, 9 Oct 2019 at 07:55, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 25, 2019 at 10:12 PM Simon Glass sjg@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?
Signed-off-by: Simon Glass sjg@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.
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.
Regards, Bin