Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Vignesh R]

On 09/16/2015 04:26 PM, Mark Brown wrote:
> On Wed, Sep 16, 2015 at 03:38:09PM +0530, Vignesh R wrote:
> 
>> But, I didn't get how to integrate with existing message queue. Memory
>> mapped read by-passes message queue of SPI core. Could you please
>> explain a bit more on integrating with message queue? Did you mean
>> locking the existing message queue when memory mapped read is being
>> requested?
> 
> Yes, and also making sure that we don't everything gets processed in
> order so memory mapped requests come after any commands needed to set
> them up and don't starve other work.
> 

Ok, thanks for the clarification!

-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Vignesh R]

On 09/15/2015 12:07 AM, Mark Brown wrote:
> On Fri, Sep 04, 2015 at 01:59:58PM +0530, Vignesh R wrote:
>> In addition to providing direct access to SPI bus, some spi controller
>> hardwares (like ti-qspi) provide special memory mapped port
>> to accesses SPI flash devices in order to increase read performance.
>> This means the controller can automatically send the SPI signals
>> required to read data from the SPI flash device.
> 
> Sorry, also meant to say here: as I kind of indicated in response to the
> flash patch I'd expect to see the SPI core know something about this and
> export an API for this which is integrated with things like the existing
> message queue.
> 


Adding an API to SPI core makes sense to me. This can take care of spi
bus locking and runtime pm.
But, I didn't get how to integrate with existing message queue. Memory
mapped read by-passes message queue of SPI core. Could you please
explain a bit more on integrating with message queue? Did you mean
locking the existing message queue when memory mapped read is being
requested?


Thanks,
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Vignesh R]

On 09/15/2015 12:05 AM, Mark Brown wrote:
> On Fri, Sep 04, 2015 at 04:55:33PM +0530, Jagan Teki wrote:
>> On 4 September 2015 at 13:59, Vignesh R  wrote:
> 
>>> + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
>>> + * mapped interface to communicate with mtd flashes.
>>> + * For this, spi  controller needs to know flash
>>> + * memory settings like read command to use, dummy
>>> + * bytes and address width. Once these settings are
>>> + * populated in hardware registers, any read
>>> + * accesses to flash's memory map region(as defined
>>> + * by SoC) through memcpy or mem-to-mem DMA copy
>>> + * will be handled by controller hardware. The
>>> + * hardware will automatically generate spi signals
>>> + * required to read data from flash and present it
>>> + * to CPU or DMA. SPI master drivers can use this
>>> + * callback to implement memory mapped read
>>> + * interface. Flash driver (like m25p80) requests
>>> + * memory mapped read via this method. The interface
>>> + * should  only be used mtd flashes and cannot be
>>> + * used with other spi devices.
> 
> This comment is *way* too verbose - probably you just need up to the
> "Once" here.
> 

Ok, I will move the extra text to commit log.


-- 
Thanks,
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Mark Brown]

On Wed, Sep 16, 2015 at 03:38:09PM +0530, Vignesh R wrote:

> But, I didn't get how to integrate with existing message queue. Memory
> mapped read by-passes message queue of SPI core. Could you please
> explain a bit more on integrating with message queue? Did you mean
> locking the existing message queue when memory mapped read is being
> requested?

Yes, and also making sure that we don't everything gets processed in
order so memory mapped requests come after any commands needed to set
them up and don't starve other work.


signature.asc
Description: Digital signature

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Jagan Teki]

On 15 September 2015 at 00:05, Mark Brown  wrote:
> On Fri, Sep 04, 2015 at 04:55:33PM +0530, Jagan Teki wrote:
>> On 4 September 2015 at 13:59, Vignesh R  wrote:
>
>> > + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
>> > + * mapped interface to communicate with mtd flashes.
>> > + * For this, spi  controller needs to know flash
>> > + * memory settings like read command to use, dummy
>> > + * bytes and address width. Once these settings are
>> > + * populated in hardware registers, any read
>> > + * accesses to flash's memory map region(as defined
>> > + * by SoC) through memcpy or mem-to-mem DMA copy
>> > + * will be handled by controller hardware. The
>> > + * hardware will automatically generate spi signals
>> > + * required to read data from flash and present it
>> > + * to CPU or DMA. SPI master drivers can use this
>> > + * callback to implement memory mapped read
>> > + * interface. Flash driver (like m25p80) requests
>> > + * memory mapped read via this method. The interface
>> > + * should  only be used mtd flashes and cannot be
>> > + * used with other spi devices.
>
> This comment is *way* too verbose - probably you just need up to the
> "Once" here.
>
>> > +   int (*spi_mtd_mmap_read)(struct  spi_device *spi,
>> > +loff_t from, size_t len, size_t *retlen,
>> > +u_char *buf, u8 read_opcode,
>> > +u8 addr_width, u8 dummy_bytes);
>
>> This looks un-manageable to know spi core or master knows what are the
>> command or opcode used by spi-nor flash and also I think mmap support
>> seems to be flash related or any justification this is spi bus
>> master/controller feature?
>
> There seem to be a reasonable number of SPI controllers out there which
> have as an extension the ability to do memory mapped reads but are
> otherwise perfectly normal SPI controllers and which rely on that for
> everything except reads.

This is true, but exposing direct spi-nor arguments or features like
read_opcode, addr_width or dummy_byte to spi layer isn't fine? because
for spi layer there is a spi to flash transition mtd m25p80.c is there
to handle is it?

thanks!
-- 
Jagan | openedev.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Mark Brown]

On Wed, Sep 16, 2015 at 06:16:55PM +0530, Jagan Teki wrote:
> On 15 September 2015 at 00:05, Mark Brown  wrote:

> > There seem to be a reasonable number of SPI controllers out there which
> > have as an extension the ability to do memory mapped reads but are
> > otherwise perfectly normal SPI controllers and which rely on that for
> > everything except reads.

> This is true, but exposing direct spi-nor arguments or features like
> read_opcode, addr_width or dummy_byte to spi layer isn't fine? because
> for spi layer there is a spi to flash transition mtd m25p80.c is there
> to handle is it?

The m25p80 driver is a generic, SPI level piece of code not a driver for
specific controller hardware.  The controller hardware drivers live in
the SPI subsystem.  In order to support this memory mapped feature we
need the controller drivers to expose an interface for controlling it.


signature.asc
Description: Digital signature

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Michal Suchanek]

On 16 September 2015 at 14:46, Jagan Teki  wrote:
> On 15 September 2015 at 00:05, Mark Brown  wrote:
>> On Fri, Sep 04, 2015 at 04:55:33PM +0530, Jagan Teki wrote:
>>> On 4 September 2015 at 13:59, Vignesh R  wrote:
>>
>>> > + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
>>> > + * mapped interface to communicate with mtd flashes.
>>> > + * For this, spi  controller needs to know flash
>>> > + * memory settings like read command to use, dummy
>>> > + * bytes and address width. Once these settings are
>>> > + * populated in hardware registers, any read
>>> > + * accesses to flash's memory map region(as defined
>>> > + * by SoC) through memcpy or mem-to-mem DMA copy
>>> > + * will be handled by controller hardware. The
>>> > + * hardware will automatically generate spi signals
>>> > + * required to read data from flash and present it
>>> > + * to CPU or DMA. SPI master drivers can use this
>>> > + * callback to implement memory mapped read
>>> > + * interface. Flash driver (like m25p80) requests
>>> > + * memory mapped read via this method. The interface
>>> > + * should  only be used mtd flashes and cannot be
>>> > + * used with other spi devices.
>>
>> This comment is *way* too verbose - probably you just need up to the
>> "Once" here.
>>
>>> > +   int (*spi_mtd_mmap_read)(struct  spi_device *spi,
>>> > +loff_t from, size_t len, size_t *retlen,
>>> > +u_char *buf, u8 read_opcode,
>>> > +u8 addr_width, u8 dummy_bytes);
>>
>>> This looks un-manageable to know spi core or master knows what are the
>>> command or opcode used by spi-nor flash and also I think mmap support
>>> seems to be flash related or any justification this is spi bus
>>> master/controller feature?
>>
>> There seem to be a reasonable number of SPI controllers out there which
>> have as an extension the ability to do memory mapped reads but are
>> otherwise perfectly normal SPI controllers and which rely on that for
>> everything except reads.
>
> This is true, but exposing direct spi-nor arguments or features like
> read_opcode, addr_width or dummy_byte to spi layer isn't fine? because
> for spi layer there is a spi to flash transition mtd m25p80.c is there
> to handle is it?

For the TI SPI controller this is programmable. So instead of
composing a SPI message that sends the opcode (and address and dummy
bytes) you set the opcode in a controller register and then perform
the memory read which sends the configured opcode to the flash memory.
So this needs to be exposed in an API which the m25p80 driver can use.

On 16 September 2015 at 12:08, Vignesh R  wrote:
>
>
> On 09/15/2015 12:07 AM, Mark Brown wrote:
>> On Fri, Sep 04, 2015 at 01:59:58PM +0530, Vignesh R wrote:
>>> In addition to providing direct access to SPI bus, some spi controller
>>> hardwares (like ti-qspi) provide special memory mapped port
>>> to accesses SPI flash devices in order to increase read performance.
>>> This means the controller can automatically send the SPI signals
>>> required to read data from the SPI flash device.
>>
>> Sorry, also meant to say here: as I kind of indicated in response to the
>> flash patch I'd expect to see the SPI core know something about this and
>> export an API for this which is integrated with things like the existing
>> message queue.
>>
>
>
> Adding an API to SPI core makes sense to me. This can take care of spi
> bus locking and runtime pm.
> But, I didn't get how to integrate with existing message queue. Memory
> mapped read by-passes message queue of SPI core. Could you please
> explain a bit more on integrating with message queue? Did you mean
> locking the existing message queue when memory mapped read is being
> requested?

Presumably when you have a function in SPI core for this it takes the
same lock transfer_one does so either you do mmap transfer or normal
SPI transfer. You can probably manually sync on transfer completion in
a driver that uses both normal SPI messages and mmap transfer - which
m25p80 does anyway.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Mark Brown]

On Fri, Sep 04, 2015 at 04:55:33PM +0530, Jagan Teki wrote:
> On 4 September 2015 at 13:59, Vignesh R  wrote:

> > + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
> > + * mapped interface to communicate with mtd flashes.
> > + * For this, spi  controller needs to know flash
> > + * memory settings like read command to use, dummy
> > + * bytes and address width. Once these settings are
> > + * populated in hardware registers, any read
> > + * accesses to flash's memory map region(as defined
> > + * by SoC) through memcpy or mem-to-mem DMA copy
> > + * will be handled by controller hardware. The
> > + * hardware will automatically generate spi signals
> > + * required to read data from flash and present it
> > + * to CPU or DMA. SPI master drivers can use this
> > + * callback to implement memory mapped read
> > + * interface. Flash driver (like m25p80) requests
> > + * memory mapped read via this method. The interface
> > + * should  only be used mtd flashes and cannot be
> > + * used with other spi devices.

This comment is *way* too verbose - probably you just need up to the
"Once" here.

> > +   int (*spi_mtd_mmap_read)(struct  spi_device *spi,
> > +loff_t from, size_t len, size_t *retlen,
> > +u_char *buf, u8 read_opcode,
> > +u8 addr_width, u8 dummy_bytes);

> This looks un-manageable to know spi core or master knows what are the
> command or opcode used by spi-nor flash and also I think mmap support
> seems to be flash related or any justification this is spi bus
> master/controller feature?

There seem to be a reasonable number of SPI controllers out there which
have as an extension the ability to do memory mapped reads but are
otherwise perfectly normal SPI controllers and which rely on that for
everything except reads.


signature.asc
Description: Digital signature

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Mark Brown]

On Fri, Sep 04, 2015 at 01:59:58PM +0530, Vignesh R wrote:
> In addition to providing direct access to SPI bus, some spi controller
> hardwares (like ti-qspi) provide special memory mapped port
> to accesses SPI flash devices in order to increase read performance.
> This means the controller can automatically send the SPI signals
> required to read data from the SPI flash device.

Sorry, also meant to say here: as I kind of indicated in response to the
flash patch I'd expect to see the SPI core know something about this and
export an API for this which is integrated with things like the existing
message queue.


signature.asc
Description: Digital signature

Re: [PATCH 1/5] spi: introduce mmap read support for spi flash devices

[Jagan Teki]

On 4 September 2015 at 13:59, Vignesh R  wrote:
> In addition to providing direct access to SPI bus, some spi controller
> hardwares (like ti-qspi) provide special memory mapped port
> to accesses SPI flash devices in order to increase read performance.
> This means the controller can automatically send the SPI signals
> required to read data from the SPI flash device.
> For this, spi controller needs to know flash specific information like
> read command to use, dummy bytes and address width. Once these settings
> are populated in hardware registers, any read accesses to flash's memory
> map region(SoC specific) through memcpy or mem-to-mem DMA copy will be
> handled by controller hardware. The hardware will automatically generate
> spi signals required to read data from flash and present it to CPU or
> DMA engine.
>
> Introduce spi_mtd_mmap_read() method to support memory mapped read
> over SPI flash devices. SPI master drivers can implement this method to
> support memory mapped read interfaces. m25p80 flash driver and other
> flash drivers can call this to request memory mapped read. The interface
> should only be used mtd flashes and cannot be used with other spi devices.
>
> Signed-off-by: Vignesh R 
> ---
>  include/linux/spi/spi.h | 21 +
>  1 file changed, 21 insertions(+)
>
> diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
> index d673072346f2..b74a3f169fc2 100644
> --- a/include/linux/spi/spi.h
> +++ b/include/linux/spi/spi.h
> @@ -293,6 +293,23 @@ static inline void spi_unregister_driver(struct 
> spi_driver *sdrv)
>   * @handle_err: the subsystem calls the driver to handle an error that occurs
>   * in the generic implementation of transfer_one_message().
>   * @unprepare_message: undo any work done by prepare_message().
> + * @spi_mtd_mmap_read: some spi-controller hardwares provide memory
> + * mapped interface to communicate with mtd flashes.
> + * For this, spi  controller needs to know flash
> + * memory settings like read command to use, dummy
> + * bytes and address width. Once these settings are
> + * populated in hardware registers, any read
> + * accesses to flash's memory map region(as defined
> + * by SoC) through memcpy or mem-to-mem DMA copy
> + * will be handled by controller hardware. The
> + * hardware will automatically generate spi signals
> + * required to read data from flash and present it
> + * to CPU or DMA. SPI master drivers can use this
> + * callback to implement memory mapped read
> + * interface. Flash driver (like m25p80) requests
> + * memory mapped read via this method. The interface
> + * should  only be used mtd flashes and cannot be
> + * used with other spi devices.
>   * @cs_gpios: Array of GPIOs to use as chip select lines; one per CS
>   * number. Any individual value may be -ENOENT for CS lines that
>   * are not GPIOs (driven by the SPI controller itself).
> @@ -438,6 +455,10 @@ struct spi_master {
>struct spi_message *message);
> int (*unprepare_message)(struct spi_master *master,
>  struct spi_message *message);
> +   int (*spi_mtd_mmap_read)(struct  spi_device *spi,
> +loff_t from, size_t len, size_t *retlen,
> +u_char *buf, u8 read_opcode,
> +u8 addr_width, u8 dummy_bytes);

This looks un-manageable to know spi core or master knows what are the
command or opcode used by spi-nor flash and also I think mmap support
seems to be flash related or any justification this is spi bus
master/controller feature?

thanks!
-- 
Jagan | openedev.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html