All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michal Suchanek <hramrach@gmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: "R, Vignesh" <vigneshr@ti.com>,
	devicetree <devicetree@vger.kernel.org>,
	Brian Norris <computersforpeace@gmail.com>,
	Russell King <linux@arm.linux.org.uk>,
	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: Wed, 5 Aug 2015 14:40:01 +0200	[thread overview]
Message-ID: <CAOMqctQXwcHyiWBwtugSDSE_k65qrNfrwnhjQMDJkLoJMmGzUw@mail.gmail.com> (raw)
In-Reply-To: <20150805115013.GJ20873@sirena.org.uk>

On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Aug 04, 2015 at 11:29:52PM +0530, R, Vignesh wrote:
>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>> > On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>
>> > I still can't tell from the above what this interface is supposed to do.
>> > It sounds like the use of memory mapped mode is supposed to be
>> > transparent to users, it should just affect how the controller interacts
>> > with the hardware, but if that's the case why do we need to expose it to
>> > users at all?  Shouldn't the driver just use memory mapped mode if it's
>> > faster?
>
>> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
>> allows reading and writing to an SPI flash device only. Used to speed up
>> flash reads. It _cannot_ be used to communicate with non flash devices.
>> Now, the spi_message that ti-qspi receives in transfer_one() callback
>> can be from mtd device(in which case SFI_MM_IF can be used) or from any
>> other non flash SPI device (in which case SFI_MM_IF must not be used
>> instead SPI_CORE is to be used) but there is no way(is there?) to
>> distinguish where spi_message is from. Therefore I introduced flag
>> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
>> this helps the ti-qspi driver to determine that the user is flash device
>> and thus can do read via SFI_MM_IF. If this flag is not set then the
>> user is assumed to be non flash SPI driver and will use SPI_CORE block
>> to communicate.
>
> So if you're trying to do this you need to document it adequately so
> that other people can understand what it is supposed to do and how to
> use and implement it.  People can't really tell how the interface is
> supposed to work based on what was in the patch and the above isn't
> really helping.  For example, how does this change or restrict what the
> contents of the spi_message are?
>
>> On the whole, I just need a way to determine that the user is a flash
>> device in order to switch to memory mapped interface.
>
> As far as I can tell you want to set a per spi_message flag saying that
> the message is a flash read command?  If that's what this is trying to
> do then why do you need to set the flag at all?  If the message is in a
> clearly defined format and it's more efficient to use this mmap mode
> then surely the driver can just recognise that the format is approprate
> and switch into mmap mode without being explicitly told - I'm not clear
> what the flag adds here.

ehm, the read command is just one byte.

I don't think sending 03 or other random byte as the first byte of a
SPI transfer can be used as reliable detection that we are talking to
a SPI flash memory.

Thanks

Michal

WARNING: multiple messages have this Message-ID (diff)
From: Michal Suchanek <hramrach-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: "R, Vignesh" <vigneshr-l0cyMroinI0@public.gmane.org>,
	devicetree <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	Brian Norris
	<computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Russell King <linux-lFZ/pmaqli7XmaaqVzeoHQ@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: Wed, 5 Aug 2015 14:40:01 +0200	[thread overview]
Message-ID: <CAOMqctQXwcHyiWBwtugSDSE_k65qrNfrwnhjQMDJkLoJMmGzUw@mail.gmail.com> (raw)
In-Reply-To: <20150805115013.GJ20873-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>

On 5 August 2015 at 13:50, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Tue, Aug 04, 2015 at 11:29:52PM +0530, R, Vignesh wrote:
>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>> > On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>
>> > I still can't tell from the above what this interface is supposed to do.
>> > It sounds like the use of memory mapped mode is supposed to be
>> > transparent to users, it should just affect how the controller interacts
>> > with the hardware, but if that's the case why do we need to expose it to
>> > users at all?  Shouldn't the driver just use memory mapped mode if it's
>> > faster?
>
>> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
>> allows reading and writing to an SPI flash device only. Used to speed up
>> flash reads. It _cannot_ be used to communicate with non flash devices.
>> Now, the spi_message that ti-qspi receives in transfer_one() callback
>> can be from mtd device(in which case SFI_MM_IF can be used) or from any
>> other non flash SPI device (in which case SFI_MM_IF must not be used
>> instead SPI_CORE is to be used) but there is no way(is there?) to
>> distinguish where spi_message is from. Therefore I introduced flag
>> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
>> this helps the ti-qspi driver to determine that the user is flash device
>> and thus can do read via SFI_MM_IF. If this flag is not set then the
>> user is assumed to be non flash SPI driver and will use SPI_CORE block
>> to communicate.
>
> So if you're trying to do this you need to document it adequately so
> that other people can understand what it is supposed to do and how to
> use and implement it.  People can't really tell how the interface is
> supposed to work based on what was in the patch and the above isn't
> really helping.  For example, how does this change or restrict what the
> contents of the spi_message are?
>
>> On the whole, I just need a way to determine that the user is a flash
>> device in order to switch to memory mapped interface.
>
> As far as I can tell you want to set a per spi_message flag saying that
> the message is a flash read command?  If that's what this is trying to
> do then why do you need to set the flag at all?  If the message is in a
> clearly defined format and it's more efficient to use this mmap mode
> then surely the driver can just recognise that the format is approprate
> and switch into mmap mode without being explicitly told - I'm not clear
> what the flag adds here.

ehm, the read command is just one byte.

I don't think sending 03 or other random byte as the first byte of a
SPI transfer can be used as reliable detection that we are talking to
a SPI flash memory.

Thanks

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

WARNING: multiple messages have this Message-ID (diff)
From: hramrach@gmail.com (Michal Suchanek)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH 1/5] spi: introduce flag for memory mapped read
Date: Wed, 5 Aug 2015 14:40:01 +0200	[thread overview]
Message-ID: <CAOMqctQXwcHyiWBwtugSDSE_k65qrNfrwnhjQMDJkLoJMmGzUw@mail.gmail.com> (raw)
In-Reply-To: <20150805115013.GJ20873@sirena.org.uk>

On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:
> On Tue, Aug 04, 2015 at 11:29:52PM +0530, R, Vignesh wrote:
>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>> > On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>
>> > I still can't tell from the above what this interface is supposed to do.
>> > It sounds like the use of memory mapped mode is supposed to be
>> > transparent to users, it should just affect how the controller interacts
>> > with the hardware, but if that's the case why do we need to expose it to
>> > users at all?  Shouldn't the driver just use memory mapped mode if it's
>> > faster?
>
>> 2. SFI_MM_IF(SPI memory mapped interface): The SFI_MM_IF block only
>> allows reading and writing to an SPI flash device only. Used to speed up
>> flash reads. It _cannot_ be used to communicate with non flash devices.
>> Now, the spi_message that ti-qspi receives in transfer_one() callback
>> can be from mtd device(in which case SFI_MM_IF can be used) or from any
>> other non flash SPI device (in which case SFI_MM_IF must not be used
>> instead SPI_CORE is to be used) but there is no way(is there?) to
>> distinguish where spi_message is from. Therefore I introduced flag
>> (use_mmap_mode) to struct spi_message. mtd driver will set flag to true,
>> this helps the ti-qspi driver to determine that the user is flash device
>> and thus can do read via SFI_MM_IF. If this flag is not set then the
>> user is assumed to be non flash SPI driver and will use SPI_CORE block
>> to communicate.
>
> So if you're trying to do this you need to document it adequately so
> that other people can understand what it is supposed to do and how to
> use and implement it.  People can't really tell how the interface is
> supposed to work based on what was in the patch and the above isn't
> really helping.  For example, how does this change or restrict what the
> contents of the spi_message are?
>
>> On the whole, I just need a way to determine that the user is a flash
>> device in order to switch to memory mapped interface.
>
> As far as I can tell you want to set a per spi_message flag saying that
> the message is a flash read command?  If that's what this is trying to
> do then why do you need to set the flag at all?  If the message is in a
> clearly defined format and it's more efficient to use this mmap mode
> then surely the driver can just recognise that the format is approprate
> and switch into mmap mode without being explicitly told - I'm not clear
> what the flag adds here.

ehm, the read command is just one byte.

I don't think sending 03 or other random byte as the first byte of a
SPI transfer can be used as reliable detection that we are talking to
a SPI flash memory.

Thanks

Michal

  reply	other threads:[~2015-08-05 12:40 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 [this message]
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
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=CAOMqctQXwcHyiWBwtugSDSE_k65qrNfrwnhjQMDJkLoJMmGzUw@mail.gmail.com \
    --to=hramrach@gmail.com \
    --cc=b32955@freescale.com \
    --cc=broonie@kernel.org \
    --cc=computersforpeace@gmail.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dwmw2@infradead.org \
    --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.