| Message ID | 1380898293-13755-4-git-send-email-sourav.poddar@ti.com |
|---|---|
| State | Superseded |
| Delegated to: | Jagannadha Sutradharudu Teki |
| Headers | show |
Please use the commit msg head as "sf: .." On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar <sourav.poddar@ti.com> wrote: > Qspi controller can have a memory mapped port which can be used for > data read. Added support to enable memory mapped port read. > > This patch enables the following: > - It enables exchange of memory map address between mtd and qspi > through the introduction of "memory_map" flag. > - Add support to communicate to the driver that memory mapped > transfer is to be started through introduction of new flags like > "SPI_XFER_MEM_MAP" and "SPI_XFER_MEM_MAP_END". > > This will enable the spi controller to do memory mapped configurations > if required. > > Signed-off-by: Sourav Poddar <sourav.poddar@ti.com> > --- > drivers/mtd/spi/sf_ops.c | 2 ++ > drivers/mtd/spi/sf_probe.c | 1 + > include/spi.h | 3 +++ > 3 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c > index c009af5..bee4128 100644 > --- a/drivers/mtd/spi/sf_ops.c > +++ b/drivers/mtd/spi/sf_ops.c > @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, > > /* Handle memory-mapped SPI */ > if (flash->memory_map) { > + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); > memcpy(data, flash->memory_map + offset, len); > + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); Is it correct, can you check it once. where is SPI_XFER_MEM_MAP_END used? Looks like you have used mem-map for only reads is it? if so where is SPI_XFER_BEGIN is using? Please use _MMAP instead of _MEM_MAP for simple naming convention.
On Saturday 05 October 2013 01:36 AM, Jagan Teki wrote: > Please use the commit msg head as "sf: .." Ok. > On Fri, Oct 4, 2013 at 8:21 PM, Sourav Poddar<sourav.poddar@ti.com> wrote: >> Qspi controller can have a memory mapped port which can be used for >> data read. Added support to enable memory mapped port read. >> >> This patch enables the following: >> - It enables exchange of memory map address between mtd and qspi >> through the introduction of "memory_map" flag. >> - Add support to communicate to the driver that memory mapped >> transfer is to be started through introduction of new flags like >> "SPI_XFER_MEM_MAP" and "SPI_XFER_MEM_MAP_END". >> >> This will enable the spi controller to do memory mapped configurations >> if required. >> >> Signed-off-by: Sourav Poddar<sourav.poddar@ti.com> >> --- >> drivers/mtd/spi/sf_ops.c | 2 ++ >> drivers/mtd/spi/sf_probe.c | 1 + >> include/spi.h | 3 +++ >> 3 files changed, 6 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c >> index c009af5..bee4128 100644 >> --- a/drivers/mtd/spi/sf_ops.c >> +++ b/drivers/mtd/spi/sf_ops.c >> @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, >> >> /* Handle memory-mapped SPI */ >> if (flash->memory_map) { >> + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); >> memcpy(data, flash->memory_map + offset, len); >> + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); > Is it correct, can you check it once. > where is SPI_XFER_MEM_MAP_END used? It will be used in the driver. check 4/6 patch of this series. > Looks like you have used mem-map for only reads is it? if so where is > SPI_XFER_BEGIN is using? Yes, only memory mapped read is supported. Ideally, we dont need BEGIN flag for memory mapped cases. I have explained a bit more on your similar comment on patch 4/6. > Please use _MMAP instead of _MEM_MAP for simple naming convention. > OK.
On Fri, Oct 04, 2013 at 20:21 +0530, Sourav Poddar wrote: > > diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c > index c009af5..bee4128 100644 > --- a/drivers/mtd/spi/sf_ops.c > +++ b/drivers/mtd/spi/sf_ops.c > @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, > > /* Handle memory-mapped SPI */ > if (flash->memory_map) { > + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); > memcpy(data, flash->memory_map + offset, len); > + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); > return 0; > } Feedback has been sent before, but I'm afraid the motivation wasn't received appropriately. Shouldn't the memcpy() call be surrounded by _MAP and _MAP_END (please note the _END in the second spi_xfer() invocation)? The current patch doesn't "close" the transfer, which appears to pass tests but isn't correct. virtually yours Gerhard Sittig
On Sunday 06 October 2013 03:03 PM, Gerhard Sittig wrote: > On Fri, Oct 04, 2013 at 20:21 +0530, Sourav Poddar wrote: >> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c >> index c009af5..bee4128 100644 >> --- a/drivers/mtd/spi/sf_ops.c >> +++ b/drivers/mtd/spi/sf_ops.c >> @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, >> >> /* Handle memory-mapped SPI */ >> if (flash->memory_map) { >> + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); >> memcpy(data, flash->memory_map + offset, len); >> + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); >> return 0; >> } > Feedback has been sent before, but I'm afraid the motivation > wasn't received appropriately. Sorry, If I missed any mails. > Shouldn't the memcpy() call be surrounded by _MAP and _MAP_END > (please note the _END in the second spi_xfer() invocation)? The > current patch doesn't "close" the transfer, which appears to pass > tests but isn't correct. > > Yes, you are correct. Second xfer should be with a END flag. I will add it in my next version, thanks for pointing out. > virtually yours > Gerhard Sittig
diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c index c009af5..bee4128 100644 --- a/drivers/mtd/spi/sf_ops.c +++ b/drivers/mtd/spi/sf_ops.c @@ -269,7 +269,9 @@ int spi_flash_cmd_read_ops(struct spi_flash *flash, u32 offset, /* Handle memory-mapped SPI */ if (flash->memory_map) { + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); memcpy(data, flash->memory_map + offset, len); + spi_xfer(flash->spi, 0, NULL, NULL, SPI_XFER_MEM_MAP); return 0; } diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c index 1525636..6aa7086 100644 --- a/drivers/mtd/spi/sf_probe.c +++ b/drivers/mtd/spi/sf_probe.c @@ -203,6 +203,7 @@ struct spi_flash *spi_flash_validate_params(struct spi_slave *spi, u8 *idcode) flash->page_size = (ext_jedec == 0x4d00) ? 512 : 256; flash->sector_size = params->sector_size; flash->size = flash->sector_size * params->nr_sectors; + flash->memory_map = spi->memory_map; /* Compute erase sector and command */ if (params->flags & SECT_4K) { diff --git a/include/spi.h b/include/spi.h index c44ebe8..d5c4e08 100644 --- a/include/spi.h +++ b/include/spi.h @@ -27,6 +27,8 @@ /* SPI transfer flags */ #define SPI_XFER_BEGIN 0x01 /* Assert CS before transfer */ #define SPI_XFER_END 0x02 /* Deassert CS after transfer */ +#define SPI_XFER_MEM_MAP 0x08 /* Memory Mapped start */ +#define SPI_XFER_MEM_MAP_END 0x10 /* Memory Mapped End */ /* Header byte that marks the start of the message */ #define SPI_PREAMBLE_END_BYTE 0xec @@ -46,6 +48,7 @@ struct spi_slave { unsigned int bus; unsigned int cs; unsigned int max_write_size; + void *memory_map; }; /**
Qspi controller can have a memory mapped port which can be used for data read. Added support to enable memory mapped port read. This patch enables the following: - It enables exchange of memory map address between mtd and qspi through the introduction of "memory_map" flag. - Add support to communicate to the driver that memory mapped transfer is to be started through introduction of new flags like "SPI_XFER_MEM_MAP" and "SPI_XFER_MEM_MAP_END". This will enable the spi controller to do memory mapped configurations if required. Signed-off-by: Sourav Poddar <sourav.poddar@ti.com> --- drivers/mtd/spi/sf_ops.c | 2 ++ drivers/mtd/spi/sf_probe.c | 1 + include/spi.h | 3 +++ 3 files changed, 6 insertions(+), 0 deletions(-)