All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sourav Poddar <sourav.poddar@ti.com>
To: Trent Piepho <tpiepho@gmail.com>,
	Peter Korsgaard <peter@korsgaard.com>,
	Mark Brown <broonie@kernel.org>
Cc: "spi-devel-general@lists.sourceforge.net"
	<spi-devel-general@lists.sourceforge.net>,
	computersforpeace@gmail.com,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	dwmw2@infradead.org, balbi@ti.com
Subject: Re: [PATCH 1/3] spi/qspi: Add memory mapped read support.
Date: Thu, 10 Oct 2013 14:22:28 +0530	[thread overview]
Message-ID: <52566ACC.1080100@ti.com> (raw)
In-Reply-To: <CA+7tXii4J_Hie2v-DaSevFHn-YqriiVowhpXCP9H+U51_tbhvQ@mail.gmail.com>

Hi All,
On Thursday 10 October 2013 07:57 AM, Trent Piepho wrote:
> On Wed, Oct 9, 2013 at 12:01 PM, Peter Korsgaard<peter@korsgaard.com>  wrote:
>>>>>>> "Mark" == Mark Brown<broonie@kernel.org>  writes:
>>   Mark>  I'm not convinced that this is the most useful API, it sounds like the
>>   Mark>  hardware can "memory map" the entire flash chip so the whole SPI
>>   Mark>  framework seems like overhead.
>>
>>   Mark>  It also seems seems like it's going to involve the CPU being
>>   Mark>  stalled waiting for reads to complete instead of asking the SPI
>>   Mark>  controller to DMA the data to RAM and allowing the CPU to get on
>>   Mark>  with other things - replacing the explicit transmission of
>>   Mark>  commands with memory to memory DMAs might be advantageous but
>>   Mark>  replacing DMA with memcpy() would need numbers to show that it
>>   Mark>  was a win.
>>
>> Indeed. I can see how such a feature could be useful in E.G. a lowlevel
>> bootloader (because of simplicity), but am less convinced about it in
>> Linux where we could conceivable do something else useful while waiting
>> on the spi controller.
> I've found that the SPI layer adds rather a lot of overhead to SPI
> transactions.  It appears to come mostly from using another thread to
> run the queue.  A fast SPI message of a few dozen bytes ends up having
> more overhead from the SPI layer than the time it takes the driver to
> do the actual transfer.
>
> So memory mapped mode via some kind of SPI hack seems like a bad
> design.  All the SPI layer overhead and you don't get DMA.  Memory
> mapped SPI could be a win, but I think you'd need to do it at the MTD
> layer with a mapping driver that could read the mmapped SPI flash
> directly.
Yes, you are correct in all your comments above. I also feel that SPI 
framework
should be bypassed. But the subject patch is derived based on the 
following points/limitation:

1. There is a setup register in QSPI, that need to be filled, as of now 
I am filling it
     in my driver as a MACRO.
2. Controller repsonds to memory mapped read for read opcodes, so during the
read path we should tell the controller to switch to memory mapped port.

[Trent]: With mapping driver, I believe you are hinting at 
drivers/mtd/maps? I had
a look at it and what I got is that it is used/suitable for parallel 
flashes and not the
serial flashes.

All in all, Just at the beginning of the read api, I could have done 
memory mapped read and bypass
the spi framework. But, prior to that above 1, 2 point need to be 
executed and that need to be
communicated to controller driver.

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

WARNING: multiple messages have this Message-ID (diff)
From: Sourav Poddar <sourav.poddar@ti.com>
To: Trent Piepho <tpiepho@gmail.com>,
	Peter Korsgaard <peter@korsgaard.com>,
	Mark Brown <broonie@kernel.org>
Cc: "spi-devel-general@lists.sourceforge.net"
	<spi-devel-general@lists.sourceforge.net>,
	computersforpeace@gmail.com,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	dwmw2@infradead.org, balbi@ti.com
Subject: Re: [PATCH 1/3] spi/qspi: Add memory mapped read support.
Date: Thu, 10 Oct 2013 14:22:28 +0530	[thread overview]
Message-ID: <52566ACC.1080100@ti.com> (raw)
In-Reply-To: <CA+7tXii4J_Hie2v-DaSevFHn-YqriiVowhpXCP9H+U51_tbhvQ@mail.gmail.com>

Hi All,
On Thursday 10 October 2013 07:57 AM, Trent Piepho wrote:
> On Wed, Oct 9, 2013 at 12:01 PM, Peter Korsgaard<peter@korsgaard.com>  wrote:
>>>>>>> "Mark" == Mark Brown<broonie@kernel.org>  writes:
>>   Mark>  I'm not convinced that this is the most useful API, it sounds like the
>>   Mark>  hardware can "memory map" the entire flash chip so the whole SPI
>>   Mark>  framework seems like overhead.
>>
>>   Mark>  It also seems seems like it's going to involve the CPU being
>>   Mark>  stalled waiting for reads to complete instead of asking the SPI
>>   Mark>  controller to DMA the data to RAM and allowing the CPU to get on
>>   Mark>  with other things - replacing the explicit transmission of
>>   Mark>  commands with memory to memory DMAs might be advantageous but
>>   Mark>  replacing DMA with memcpy() would need numbers to show that it
>>   Mark>  was a win.
>>
>> Indeed. I can see how such a feature could be useful in E.G. a lowlevel
>> bootloader (because of simplicity), but am less convinced about it in
>> Linux where we could conceivable do something else useful while waiting
>> on the spi controller.
> I've found that the SPI layer adds rather a lot of overhead to SPI
> transactions.  It appears to come mostly from using another thread to
> run the queue.  A fast SPI message of a few dozen bytes ends up having
> more overhead from the SPI layer than the time it takes the driver to
> do the actual transfer.
>
> So memory mapped mode via some kind of SPI hack seems like a bad
> design.  All the SPI layer overhead and you don't get DMA.  Memory
> mapped SPI could be a win, but I think you'd need to do it at the MTD
> layer with a mapping driver that could read the mmapped SPI flash
> directly.
Yes, you are correct in all your comments above. I also feel that SPI 
framework
should be bypassed. But the subject patch is derived based on the 
following points/limitation:

1. There is a setup register in QSPI, that need to be filled, as of now 
I am filling it
     in my driver as a MACRO.
2. Controller repsonds to memory mapped read for read opcodes, so during the
read path we should tell the controller to switch to memory mapped port.

[Trent]: With mapping driver, I believe you are hinting at 
drivers/mtd/maps? I had
a look at it and what I got is that it is used/suitable for parallel 
flashes and not the
serial flashes.

All in all, Just at the beginning of the read api, I could have done 
memory mapped read and bypass
the spi framework. But, prior to that above 1, 2 point need to be 
executed and that need to be
communicated to controller driver.

  reply	other threads:[~2013-10-10  8:52 UTC|newest]

Thread overview: 104+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-09 15:24 [PATCH 0/3]Add quad/memory mapped support for SPI flash Sourav Poddar
2013-10-09 15:24 ` Sourav Poddar
2013-10-09 15:24 ` [PATCH 1/3] spi/qspi: Add memory mapped read support Sourav Poddar
2013-10-09 15:24   ` Sourav Poddar
2013-10-09 16:07   ` Mark Brown
2013-10-09 16:07     ` Mark Brown
     [not found]     ` <20131009160759.GQ21581-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-10-09 16:54       ` Sourav Poddar
2013-10-09 16:54         ` Sourav Poddar
2013-10-09 17:40         ` Mark Brown
2013-10-09 17:40           ` Mark Brown
2013-10-09 18:15           ` Sourav Poddar
2013-10-09 18:15             ` Sourav Poddar
2013-10-09 18:41             ` Mark Brown
2013-10-09 18:41               ` Mark Brown
     [not found]           ` <20131009174027.GS21581-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-10-09 19:01             ` Peter Korsgaard
2013-10-09 19:01               ` Peter Korsgaard
2013-10-09 19:36               ` Mark Brown
2013-10-09 19:36                 ` Mark Brown
     [not found]               ` <87hacq1d5k.fsf-D6SC8u56vOOJDPpyT6T3/w@public.gmane.org>
2013-10-10  2:27                 ` Trent Piepho
2013-10-10  2:27                   ` Trent Piepho
2013-10-10  8:52                   ` Sourav Poddar [this message]
2013-10-10  8:52                     ` Sourav Poddar
2013-10-10 10:14                     ` Mark Brown
2013-10-10 10:14                       ` Mark Brown
2013-10-10 10:17                       ` Sourav Poddar
2013-10-10 10:17                         ` Sourav Poddar
2013-10-10 11:08                       ` Sourav Poddar
2013-10-10 11:08                         ` Sourav Poddar
2013-10-11 10:08                         ` Mark Brown
2013-10-11 10:08                           ` Mark Brown
2013-10-15  6:06                           ` Sourav Poddar
2013-10-15  6:06                             ` Sourav Poddar
2013-10-15 11:16                             ` Mark Brown
2013-10-15 11:16                               ` Mark Brown
2013-10-15 11:49                               ` Sourav Poddar
2013-10-15 11:49                                 ` Sourav Poddar
2013-10-15 12:46                                 ` Mark Brown
2013-10-15 12:46                                   ` Mark Brown
     [not found]                                   ` <20131015124656.GM2443-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-10-15 13:23                                     ` Sourav Poddar
2013-10-15 13:23                                       ` Sourav Poddar
2013-10-15 15:53                                       ` Mark Brown
2013-10-15 15:53                                         ` Mark Brown
     [not found]                                       ` <525D41E2.30206-l0cyMroinI0@public.gmane.org>
2013-10-15 15:33                                         ` Gupta, Pekon
2013-10-15 15:33                                           ` Gupta, Pekon
2013-10-15 16:01                                           ` Mark Brown
2013-10-15 16:01                                             ` Mark Brown
2013-10-15 16:54                                             ` Gupta, Pekon
2013-10-15 16:54                                               ` Gupta, Pekon
2013-10-15 18:01                                         ` Brian Norris
2013-10-15 18:01                                           ` Brian Norris
2013-10-15 18:10                                           ` Sourav Poddar
2013-10-15 18:10                                             ` Sourav Poddar
     [not found]                                           ` <20131015180142.GS23337-bU/DPfM3abD4WzifrMjOTkcViWtcw2C0@public.gmane.org>
2013-10-15 18:13                                             ` Trent Piepho
2013-10-15 18:13                                               ` Trent Piepho
2013-10-15 18:33                                               ` Gupta, Pekon
2013-10-15 18:33                                                 ` Gupta, Pekon
2013-10-15 20:52                                                 ` Mark Brown
2013-10-15 20:52                                                   ` Mark Brown
     [not found]                                                   ` <20131015205254.GX2443-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-10-15 21:03                                                     ` Trent Piepho
2013-10-15 21:03                                                       ` Trent Piepho
2013-10-15 22:10                                                       ` Mark Brown
2013-10-15 22:10                                                         ` Mark Brown
     [not found]                                                 ` <20980858CB6D3A4BAE95CA194937D5E73EA23640-yXqyApvAXouIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2013-10-17 12:24                                                   ` Sourav Poddar
2013-10-17 12:24                                                     ` Sourav Poddar
2013-10-17 12:38                                                     ` Mark Brown
2013-10-17 12:38                                                       ` Mark Brown
2013-10-17 13:03                                                       ` Gupta, Pekon
2013-10-17 13:03                                                         ` Gupta, Pekon
2013-10-17 23:42                                                         ` Mark Brown
2013-10-18  4:06                                                           ` Sourav Poddar
2013-10-18  5:56                                                             ` Trent Piepho
2013-10-18  6:10                                                               ` Sourav Poddar
2013-10-18  7:27                                                                 ` Sourav Poddar
2013-10-18 10:31                                                                   ` Mark Brown
2013-10-18 11:48                                                                     ` Sourav Poddar
2013-10-18 13:08                                                                       ` Mark Brown
2013-10-18 14:47                                                                         ` Sourav Poddar
2013-10-15 20:59                                               ` Mark Brown
2013-10-15 20:59                                                 ` Mark Brown
     [not found]                     ` <52566ACC.1080100-l0cyMroinI0@public.gmane.org>
2013-10-11  9:30                       ` Gupta, Pekon
2013-10-11  9:30                         ` Gupta, Pekon
2013-10-10 10:10                   ` Mark Brown
2013-10-10 10:10                     ` Mark Brown
     [not found]                     ` <20131010101052.GF21581-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
2013-10-10 23:53                       ` Trent Piepho
2013-10-11  9:59                         ` Mark Brown
2013-10-11  9:59                           ` Mark Brown
2013-10-09 15:24 ` [PATCHv3 2/3] drivers: mtd: devices: Add quad " Sourav Poddar
2013-10-09 15:24   ` Sourav Poddar
     [not found]   ` <1381332284-21822-3-git-send-email-sourav.poddar-l0cyMroinI0@public.gmane.org>
2013-10-09 18:15     ` Jagan Teki
2013-10-09 18:15       ` Jagan Teki
     [not found]       ` <CAD6G_RShZMkSpVzvXWEE0+sDX=pcnf7ndChndgDG5_T4EVL2vQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-11  7:10         ` Gupta, Pekon
2013-10-11  7:10           ` Gupta, Pekon
2013-10-24  1:06   ` Brian Norris
2013-10-24  5:44     ` Sourav Poddar
2013-10-24  7:34       ` Brian Norris
2013-10-24  8:44         ` Sourav Poddar
2013-10-24 17:07           ` Brian Norris
2013-10-24 17:55             ` Sourav Poddar
2013-10-09 15:24 ` [RFC/PATCH 3/3] drivers: mtd: devices: Add memory mapped " Sourav Poddar
2013-10-09 15:24   ` Sourav Poddar
2013-10-09 15:45   ` Mark Brown
2013-10-09 15:45     ` Mark Brown
2013-10-24  0:22 ` [PATCH 0/3]Add quad/memory mapped support for SPI flash Brian Norris
2013-10-24  4:51   ` Sourav Poddar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52566ACC.1080100@ti.com \
    --to=sourav.poddar@ti.com \
    --cc=balbi@ti.com \
    --cc=broonie@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=peter@korsgaard.com \
    --cc=spi-devel-general@lists.sourceforge.net \
    --cc=tpiepho@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.