All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Brown <broonie@kernel.org>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Vignesh R <vigneshr@ti.com>, Michal Suchanek <hramrach@gmail.com>,
	devicetree <devicetree@vger.kernel.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Tony Lindgren <tony@atomide.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-spi <linux-spi@vger.kernel.org>,
	Huang Shijie <b32955@freescale.com>,
	MTD Maling List <linux-mtd@lists.infradead.org>,
	linux-omap@vger.kernel.org, David Woodhouse <dwmw2@infradead.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
Date: Thu, 6 Aug 2015 17:46:56 +0100	[thread overview]
Message-ID: <20150806164656.GY20873@sirena.org.uk> (raw)
In-Reply-To: <20150806135129.GJ7576@n2100.arm.linux.org.uk>

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

On Thu, Aug 06, 2015 at 02:51:29PM +0100, Russell King - ARM Linux wrote:

> The M25P80 driver just appends additional bytes to the message to
> achieve this:
> 
>         struct m25p *flash = nor->priv;
>         unsigned int dummy = nor->read_dummy;
> 
>         /* convert the dummy cycles to the number of bytes */
>         dummy /= 8;
> 
>         flash->command[0] = nor->read_opcode;
>         m25p_addr2cmd(nor, from, flash->command);
> 
>         t[0].tx_buf = flash->command;
>         t[0].len = m25p_cmdsz(nor) + dummy;
>         spi_message_add_tail(&t[0], &m);

> The reason that the number of dummy bytes can't be detected is because
> it's all hidden in the first transaction as the total number of bytes to
> be transmitted - and the dummy bytes are uninitialised, so you can't
> make any assumptions what value they are.  There is no way for the SPI
> driver to know whether these dummy bytes are dummy bytes or whether they
> have an effect on the targetted device.

We *could* (as you suggest below) indicate dummy transfers by having a
separate transfer which omits the transmit buffers though I'd expect
that normally that is going to be a small performance hit if interpreted
directly so we need to think what to do there.  We do get other devices
sending dummy bytes, it's sometimes a requirement for high speed
register access to give settling time for the device, so other things
would get some milage from it.

> What may make more sense from the SPI point of view is to communicate to
> all SPI drivers how many dummy bytes are to be transferred.  I'm not fully
> up on SPI, but maybe something like this:

> 	t[0].tx_buf = flash->command;
> 	t[0].len = m25p_cmdsz(nor);
> 	spi_message_add_tail(&t[0], &m);
> 	t[1].tx_buf = dummy_buffer;
> 	t[1].len = dummy;
> 	t[1].dummy = 1;
> 	spi_message_add_tail(&t[1], &m);

> This way, we're describing the transfer to the SPI core, and explicitly
> indicating that there are some dummy bytes.  The SPI driver can then
> tell that these are dummy bytes, and if the SPI message consists of:

That'd work as well, my first thought would to use NULL as a dummy
buffer pointer and let the core substitute in data for the drivers.  We
currently insist on having at least one buffer but that's fixable.

> This would not be a hack to the SPI code: we're describing to the SPI
> code what we want to achieve in terms of the activity on the bus, and
> providing that level of description then allows the SPI driver to make
> informed decisions on whether it can handle the transfer using some
> non-standard feature.

Yup.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
To: Russell King - ARM Linux <linux-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
Cc: Vignesh R <vigneshr-l0cyMroinI0@public.gmane.org>,
	Michal Suchanek
	<hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Brian Norris
	<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>,
	Linux Kernel Mailing List
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-spi <linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Huang Shijie <b32955-KZfg59tc24xl57MIdRCFDg@public.gmane.org>,
	MTD Maling List
	<linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
Date: Thu, 6 Aug 2015 17:46:56 +0100	[thread overview]
Message-ID: <20150806164656.GY20873@sirena.org.uk> (raw)
In-Reply-To: <20150806135129.GJ7576-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]

On Thu, Aug 06, 2015 at 02:51:29PM +0100, Russell King - ARM Linux wrote:

> The M25P80 driver just appends additional bytes to the message to
> achieve this:
> 
>         struct m25p *flash = nor->priv;
>         unsigned int dummy = nor->read_dummy;
> 
>         /* convert the dummy cycles to the number of bytes */
>         dummy /= 8;
> 
>         flash->command[0] = nor->read_opcode;
>         m25p_addr2cmd(nor, from, flash->command);
> 
>         t[0].tx_buf = flash->command;
>         t[0].len = m25p_cmdsz(nor) + dummy;
>         spi_message_add_tail(&t[0], &m);

> The reason that the number of dummy bytes can't be detected is because
> it's all hidden in the first transaction as the total number of bytes to
> be transmitted - and the dummy bytes are uninitialised, so you can't
> make any assumptions what value they are.  There is no way for the SPI
> driver to know whether these dummy bytes are dummy bytes or whether they
> have an effect on the targetted device.

We *could* (as you suggest below) indicate dummy transfers by having a
separate transfer which omits the transmit buffers though I'd expect
that normally that is going to be a small performance hit if interpreted
directly so we need to think what to do there.  We do get other devices
sending dummy bytes, it's sometimes a requirement for high speed
register access to give settling time for the device, so other things
would get some milage from it.

> What may make more sense from the SPI point of view is to communicate to
> all SPI drivers how many dummy bytes are to be transferred.  I'm not fully
> up on SPI, but maybe something like this:

> 	t[0].tx_buf = flash->command;
> 	t[0].len = m25p_cmdsz(nor);
> 	spi_message_add_tail(&t[0], &m);
> 	t[1].tx_buf = dummy_buffer;
> 	t[1].len = dummy;
> 	t[1].dummy = 1;
> 	spi_message_add_tail(&t[1], &m);

> This way, we're describing the transfer to the SPI core, and explicitly
> indicating that there are some dummy bytes.  The SPI driver can then
> tell that these are dummy bytes, and if the SPI message consists of:

That'd work as well, my first thought would to use NULL as a dummy
buffer pointer and let the core substitute in data for the drivers.  We
currently insist on having at least one buffer but that's fixable.

> This would not be a hack to the SPI code: we're describing to the SPI
> code what we want to achieve in terms of the activity on the bus, and
> providing that level of description then allows the SPI driver to make
> informed decisions on whether it can handle the transfer using some
> non-standard feature.

Yup.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: broonie@kernel.org (Mark Brown)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
Date: Thu, 6 Aug 2015 17:46:56 +0100	[thread overview]
Message-ID: <20150806164656.GY20873@sirena.org.uk> (raw)
In-Reply-To: <20150806135129.GJ7576@n2100.arm.linux.org.uk>

On Thu, Aug 06, 2015 at 02:51:29PM +0100, Russell King - ARM Linux wrote:

> The M25P80 driver just appends additional bytes to the message to
> achieve this:
> 
>         struct m25p *flash = nor->priv;
>         unsigned int dummy = nor->read_dummy;
> 
>         /* convert the dummy cycles to the number of bytes */
>         dummy /= 8;
> 
>         flash->command[0] = nor->read_opcode;
>         m25p_addr2cmd(nor, from, flash->command);
> 
>         t[0].tx_buf = flash->command;
>         t[0].len = m25p_cmdsz(nor) + dummy;
>         spi_message_add_tail(&t[0], &m);

> The reason that the number of dummy bytes can't be detected is because
> it's all hidden in the first transaction as the total number of bytes to
> be transmitted - and the dummy bytes are uninitialised, so you can't
> make any assumptions what value they are.  There is no way for the SPI
> driver to know whether these dummy bytes are dummy bytes or whether they
> have an effect on the targetted device.

We *could* (as you suggest below) indicate dummy transfers by having a
separate transfer which omits the transmit buffers though I'd expect
that normally that is going to be a small performance hit if interpreted
directly so we need to think what to do there.  We do get other devices
sending dummy bytes, it's sometimes a requirement for high speed
register access to give settling time for the device, so other things
would get some milage from it.

> What may make more sense from the SPI point of view is to communicate to
> all SPI drivers how many dummy bytes are to be transferred.  I'm not fully
> up on SPI, but maybe something like this:

> 	t[0].tx_buf = flash->command;
> 	t[0].len = m25p_cmdsz(nor);
> 	spi_message_add_tail(&t[0], &m);
> 	t[1].tx_buf = dummy_buffer;
> 	t[1].len = dummy;
> 	t[1].dummy = 1;
> 	spi_message_add_tail(&t[1], &m);

> This way, we're describing the transfer to the SPI core, and explicitly
> indicating that there are some dummy bytes.  The SPI driver can then
> tell that these are dummy bytes, and if the SPI message consists of:

That'd work as well, my first thought would to use NULL as a dummy
buffer pointer and let the core substitute in data for the drivers.  We
currently insist on having at least one buffer but that's fixable.

> This would not be a hack to the SPI code: we're describing to the SPI
> code what we want to achieve in terms of the activity on the bus, and
> providing that level of description then allows the SPI driver to make
> informed decisions on whether it can handle the transfer using some
> non-standard feature.

Yup.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150806/9b3dcd74/attachment-0001.sig>

  parent reply	other threads:[~2015-08-06 16:47 UTC|newest]

Thread overview: 150+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-28  8:41 [RFC PATCH 0/5] Add memory mapped read support for TI QSPI Vignesh R
2015-07-28  8:41 ` Vignesh R
2015-07-28  8:41 ` Vignesh R
2015-07-28  8:41 ` [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-31 18:17   ` Mark Brown
2015-07-31 18:17     ` Mark Brown
2015-07-31 18:17     ` Mark Brown
2015-08-03  4:57     ` Vignesh R
2015-08-03  4:57       ` Vignesh R
2015-08-03  4:57       ` Vignesh R
2015-08-04 15:51       ` Mark Brown
2015-08-04 15:51         ` Mark Brown
2015-08-04 17:59         ` R, Vignesh
2015-08-04 17:59           ` R, Vignesh
2015-08-04 17:59           ` R, Vignesh
2015-08-04 17:59           ` R, Vignesh
2015-08-05  5:21           ` Michal Suchanek
2015-08-05  5:21             ` Michal Suchanek
2015-08-05  5:21             ` Michal Suchanek
2015-08-05  5:21             ` Michal Suchanek
2015-08-05  5:35             ` Vignesh R
2015-08-05  5:35               ` Vignesh R
2015-08-05  5:35               ` Vignesh R
2015-08-05  5:35               ` Vignesh R
2015-08-05  5:57               ` Michal Suchanek
2015-08-05  5:57                 ` Michal Suchanek
2015-08-05  5:57                 ` Michal Suchanek
2015-08-05  5:57                 ` Michal Suchanek
2015-08-05 11:50           ` Mark Brown
2015-08-05 11:50             ` Mark Brown
2015-08-05 12:40             ` Michal Suchanek
2015-08-05 12:40               ` Michal Suchanek
2015-08-05 12:40               ` Michal Suchanek
2015-08-05 12:40               ` Michal Suchanek
2015-08-05 12:44               ` Mark Brown
2015-08-05 12:44                 ` Mark Brown
2015-08-05 12:44                 ` Mark Brown
2015-08-05 12:44                 ` Mark Brown
2015-08-05 12:56                 ` Michal Suchanek
2015-08-05 12:56                   ` Michal Suchanek
2015-08-05 12:56                   ` Michal Suchanek
2015-08-06  9:02                   ` Mark Brown
2015-08-06  9:02                     ` Mark Brown
2015-08-06  9:02                     ` Mark Brown
2015-08-06 10:01                     ` Michal Suchanek
2015-08-06 10:01                       ` Michal Suchanek
2015-08-06 10:01                       ` Michal Suchanek
2015-08-06 10:22                       ` Russell King - ARM Linux
2015-08-06 10:22                         ` Russell King - ARM Linux
2015-08-06 10:22                         ` Russell King - ARM Linux
2015-08-06 10:22                         ` Russell King - ARM Linux
2015-08-06 11:00                         ` Mark Brown
2015-08-06 11:00                           ` Mark Brown
2015-08-06 11:00                           ` Mark Brown
2015-08-06 11:02                         ` Michal Suchanek
2015-08-06 11:02                           ` Michal Suchanek
2015-08-06 11:02                           ` Michal Suchanek
2015-08-06 11:02                           ` Michal Suchanek
2015-08-06 12:25                         ` Vignesh R
2015-08-06 12:25                           ` Vignesh R
2015-08-06 12:25                           ` Vignesh R
2015-08-06 12:25                           ` Vignesh R
2015-08-06 12:25                           ` Vignesh R
2015-08-06 13:51                           ` Russell King - ARM Linux
2015-08-06 13:51                             ` Russell King - ARM Linux
2015-08-06 13:51                             ` Russell King - ARM Linux
2015-08-06 16:14                             ` Geert Uytterhoeven
2015-08-06 16:14                               ` Geert Uytterhoeven
2015-08-06 16:14                               ` Geert Uytterhoeven
2015-08-06 16:14                               ` Geert Uytterhoeven
2015-08-06 18:20                               ` Michal Suchanek
2015-08-06 18:20                                 ` Michal Suchanek
2015-08-06 18:20                                 ` Michal Suchanek
2015-08-06 18:20                                 ` Michal Suchanek
2015-08-06 21:33                               ` Russell King - ARM Linux
2015-08-06 21:33                                 ` Russell King - ARM Linux
2015-08-06 21:33                                 ` Russell King - ARM Linux
2015-08-06 21:33                                 ` Russell King - ARM Linux
2015-08-07  7:38                                 ` Michal Suchanek
2015-08-07  7:38                                   ` Michal Suchanek
2015-08-07  7:38                                   ` Michal Suchanek
2015-08-07  7:38                                   ` Michal Suchanek
2015-08-07  8:35                                   ` Vignesh R
2015-08-07  8:35                                     ` Vignesh R
2015-08-07  8:35                                     ` Vignesh R
2015-08-07  8:25                                 ` Martin Sperl
2015-08-07  8:25                                   ` Martin Sperl
2015-08-07  8:25                                   ` Martin Sperl
2015-08-07  8:25                                   ` Martin Sperl
2015-08-07 10:16                                   ` Michal Suchanek
2015-08-07 10:16                                     ` Michal Suchanek
2015-08-07 10:16                                     ` Michal Suchanek
2015-08-12  9:27                                     ` Vignesh R
2015-08-12  9:27                                       ` Vignesh R
2015-08-12  9:27                                       ` Vignesh R
2015-08-12  9:27                                       ` Vignesh R
2015-08-06 16:46                             ` Mark Brown [this message]
2015-08-06 16:46                               ` Mark Brown
2015-08-06 16:46                               ` Mark Brown
2015-08-06 16:46                               ` Mark Brown
2015-08-06 18:20                           ` Mark Brown
2015-08-06 18:20                             ` Mark Brown
2015-08-06 18:20                             ` Mark Brown
2015-08-06 11:23                       ` Mark Brown
2015-08-06 11:23                         ` Mark Brown
2015-08-06 11:23                         ` Mark Brown
2015-08-06 11:23                         ` Mark Brown
2015-08-06 11:42                         ` Michal Suchanek
2015-08-06 11:42                           ` Michal Suchanek
2015-08-06 11:42                           ` Michal Suchanek
2015-08-06 16:03                           ` Mark Brown
2015-08-06 16:03                             ` Mark Brown
2015-08-06 16:03                             ` Mark Brown
2015-07-28  8:41 ` [RFC PATCH 2/5] spi: spi-ti-qspi: Add memory mapped read support Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41 ` [RFC PATCH 3/5] mtd: devices: m25p80: set flag to request memory mapped read Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41 ` [RFC PATCH 4/5] ARM: dts: DRA7: Add memory map region entries for qspi Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-31 13:48   ` Sekhar Nori
2015-07-31 13:48     ` Sekhar Nori
2015-07-31 13:48     ` Sekhar Nori
2015-08-03  5:09     ` Vignesh R
2015-08-03  5:09       ` Vignesh R
2015-08-03  5:09       ` Vignesh R
2015-08-03  5:09       ` Vignesh R
2015-07-31 18:19   ` Mark Brown
2015-07-31 18:19     ` Mark Brown
2015-07-31 18:19     ` Mark Brown
2015-08-03  5:02     ` Vignesh R
2015-08-03  5:02       ` Vignesh R
2015-08-03  5:02       ` Vignesh R
2015-08-03  5:02       ` Vignesh R
2015-08-04 15:52       ` Mark Brown
2015-08-04 15:52         ` Mark Brown
2015-08-04 15:52         ` Mark Brown
2015-07-31 21:28   ` Brian Norris
2015-07-31 21:28     ` Brian Norris
2015-08-03  5:06     ` Vignesh R
2015-08-03  5:06       ` Vignesh R
2015-08-03  5:06       ` Vignesh R
2015-08-03  5:06       ` Vignesh R
2015-07-28  8:41 ` [RFC PATCH 5/5] ARM: dts: AM4372: " Vignesh R
2015-07-28  8:41   ` Vignesh R
2015-07-28  8:41   ` Vignesh R

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=20150806164656.GY20873@sirena.org.uk \
    --to=broonie@kernel.org \
    --cc=b32955@freescale.com \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --cc=hramrach@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=tony@atomide.com \
    --cc=vigneshr@ti.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.