From: Mark Brown <broonie@kernel.org> To: Sourav Poddar <sourav.poddar@ti.com> Cc: spi-devel-general@lists.sourceforge.net, computersforpeace@gmail.com, dwmw2@infradead.org, balbi@ti.com, linux-mtd@lists.infradead.org Subject: Re: [PATCH 1/3] spi/qspi: Add memory mapped read support. Date: Wed, 9 Oct 2013 18:40:27 +0100 [thread overview] Message-ID: <20131009174027.GS21581@sirena.org.uk> (raw) In-Reply-To: <52558A49.5090904@ti.com> [-- Attachment #1.1: Type: text/plain, Size: 3085 bytes --] On Wed, Oct 09, 2013 at 10:24:33PM +0530, Sourav Poddar wrote: > Here is the exact feature usecase.. > TI qspi controller supports memory mapped read. These memory > mapped read configuration depends on the set_up_register, which > can be configured once during in setup apis based on the dt node > specifying whether the qspi controller supports memory mapped read > or not. What is "set_up_register"? > Once, the qspi controller is configured for a memory mapped read, the qspi > controller does not depend on the flash command that comes from the > mtd layer. > Because, this command are already configured in QSPI set up register. So this does depend on the flash commands for the specific chip, which means that this has all the same problems as the Freescale chip had with requiring the user to replicate the information about the commands that the chip supports into the device tree. It therefore seems like all the same concerns should apply, though in this case it seems like it's harder for the driver to infer things from looking at the operations being sent to it. Presumably this also only works for flash chips, or things that look like them... > Basically, its not the commands which need to be communicated from > the mtd layer,its just > the buffer to fill, len of buffer, offset from where to fill need to > be communicated. This appears to be based on an assumption that the commands would be replicated into the device trees which seems like it's both more work for users and harder to deploy. > >I'm also concerned about the interface here, it looks like this is being > >made visible to SPI devices (via a dependency on patch 3/3...) but only > >as a flag they can set - how would devices know to enable this and why > >would they want to avoid it? > Set spi->mode in qspi driver based on dt entry and use that in mtd layer to > decide whether to use memory mapped or not. But why would anything not want to use memory mapped mode if it can and why is this something that should be hard coded into the device tree? Especially with the current API... > The idea is whenever, we call mtd_read api from mtd layer, if memory > mapped is set > then sending the commands does not matter, what matters is the len > to read, buffer to fill and > "from" offset to read. Then, the intention was to use the memory_map > transfer parameter to > communicate to the driver that memory mapped read is used so that we > can just use memcopy and > return without going through the entire SPI based xfer function. I'm not convinced that this is the most useful API, it sounds like the hardware can "memory map" the entire flash chip so the whole SPI framework seems like overhead. It also seems seems like it's going to involve the CPU being stalled waiting for reads to complete instead of asking the SPI controller to DMA the data to RAM and allowing the CPU to get on with other things - replacing the explicit transmission of commands with memory to memory DMAs might be advantageous but replacing DMA with memcpy() would need numbers to show that it was a win. [-- Attachment #1.2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --] [-- Attachment #2: Type: text/plain, Size: 144 bytes --] ______________________________________________________ Linux MTD discussion mailing list http://lists.infradead.org/mailman/listinfo/linux-mtd/
WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie@kernel.org> To: Sourav Poddar <sourav.poddar@ti.com> Cc: spi-devel-general@lists.sourceforge.net, computersforpeace@gmail.com, dwmw2@infradead.org, balbi@ti.com, linux-mtd@lists.infradead.org Subject: Re: [PATCH 1/3] spi/qspi: Add memory mapped read support. Date: Wed, 9 Oct 2013 18:40:27 +0100 [thread overview] Message-ID: <20131009174027.GS21581@sirena.org.uk> (raw) In-Reply-To: <52558A49.5090904@ti.com> [-- Attachment #1: Type: text/plain, Size: 3085 bytes --] On Wed, Oct 09, 2013 at 10:24:33PM +0530, Sourav Poddar wrote: > Here is the exact feature usecase.. > TI qspi controller supports memory mapped read. These memory > mapped read configuration depends on the set_up_register, which > can be configured once during in setup apis based on the dt node > specifying whether the qspi controller supports memory mapped read > or not. What is "set_up_register"? > Once, the qspi controller is configured for a memory mapped read, the qspi > controller does not depend on the flash command that comes from the > mtd layer. > Because, this command are already configured in QSPI set up register. So this does depend on the flash commands for the specific chip, which means that this has all the same problems as the Freescale chip had with requiring the user to replicate the information about the commands that the chip supports into the device tree. It therefore seems like all the same concerns should apply, though in this case it seems like it's harder for the driver to infer things from looking at the operations being sent to it. Presumably this also only works for flash chips, or things that look like them... > Basically, its not the commands which need to be communicated from > the mtd layer,its just > the buffer to fill, len of buffer, offset from where to fill need to > be communicated. This appears to be based on an assumption that the commands would be replicated into the device trees which seems like it's both more work for users and harder to deploy. > >I'm also concerned about the interface here, it looks like this is being > >made visible to SPI devices (via a dependency on patch 3/3...) but only > >as a flag they can set - how would devices know to enable this and why > >would they want to avoid it? > Set spi->mode in qspi driver based on dt entry and use that in mtd layer to > decide whether to use memory mapped or not. But why would anything not want to use memory mapped mode if it can and why is this something that should be hard coded into the device tree? Especially with the current API... > The idea is whenever, we call mtd_read api from mtd layer, if memory > mapped is set > then sending the commands does not matter, what matters is the len > to read, buffer to fill and > "from" offset to read. Then, the intention was to use the memory_map > transfer parameter to > communicate to the driver that memory mapped read is used so that we > can just use memcopy and > return without going through the entire SPI based xfer function. I'm not convinced that this is the most useful API, it sounds like the hardware can "memory map" the entire flash chip so the whole SPI framework seems like overhead. It also seems seems like it's going to involve the CPU being stalled waiting for reads to complete instead of asking the SPI controller to DMA the data to RAM and allowing the CPU to get on with other things - replacing the explicit transmission of commands with memory to memory DMAs might be advantageous but replacing DMA with memcpy() would need numbers to show that it was a win. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 836 bytes --]
next prev parent reply other threads:[~2013-10-09 17:40 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 [this message] 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 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=20131009174027.GS21581@sirena.org.uk \ --to=broonie@kernel.org \ --cc=balbi@ti.com \ --cc=computersforpeace@gmail.com \ --cc=dwmw2@infradead.org \ --cc=linux-mtd@lists.infradead.org \ --cc=sourav.poddar@ti.com \ --cc=spi-devel-general@lists.sourceforge.net \ /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: linkBe 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.