Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200116 > unrolled thread

Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-04 18:00 +0200
    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read "R, Vignesh" <vigneshr@ti.com> - 2015-08-04 20:10 +0200
      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 07:30 +0200
        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-05 07:40 +0200
          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 08:00 +0200
      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-05 14:00 +0200
        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 14:50 +0200
          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-05 14:50 +0200
            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-05 15:00 +0200
              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 11:10 +0200
                Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 12:10 +0200
                  Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-06 12:30 +0200
                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 13:10 +0200
                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 13:10 +0200
                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-06 14:30 +0200
                      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-06 16:00 +0200
                        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Geert Uytterhoeven <geert@linux-m68k.org> - 2015-08-06 18:20 +0200
                          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 20:30 +0200
                          Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-08-06 23:40 +0200
                            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-07 09:40 +0200
                              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-07 10:40 +0200
                            Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Martin Sperl <martin@sperl.org> - 2015-08-07 10:30 +0200
                              Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-07 12:20 +0200
                                Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Vignesh R <vigneshr@ti.com> - 2015-08-12 11:30 +0200
                        Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 18:50 +0200
                      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 20:30 +0200
                  Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 13:30 +0200
                    Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Michal Suchanek <hramrach@gmail.com> - 2015-08-06 13:50 +0200
                      Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read Mark Brown <broonie@kernel.org> - 2015-08-06 18:10 +0200

Page 1 of 2  [1] 2  Next page →

#1200116 — Re: [RFC PATCH 1/5] spi: introduce flag for memory mapped read

[Multipart message — attachments visible in raw view] — view raw

On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:

> @use_mmap_mode: Some SPI controller chips are optimized for interacting
> with serial flash memories. These chips have memory mapped interface,
> through which entire serial flash memory slave can be read/written as if
> though they are physical memories (like RAM). Using this interface,
> flash can be accessed using memcpy() function and the spi controller
> hardware will take care of communicating with serial flash over SPI.
> Setting this flag will indicate the SPI controller driver that the
> spi_message is from mtd layer to read from/write to flash. The SPI
> master driver can then appropriately switch the controller to memory
> mapped interface to read from/write to flash, based on this flag (See
> drivers/spi/spi-ti-qspi.c for example).
> NOTE: If the SPI controller chip lacks memory mapped interface, then the
> driver will ignore this flag and use normal SPI protocol to read
> from/write to flash. Communication with non-flash SPI devices is not
> possible using the memory mapped interface.

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?

[toc] | [next] | [standalone]

#1200198

On 8/4/2015 9:21 PM, Mark Brown wrote:
> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
> 
>> @use_mmap_mode: Some SPI controller chips are optimized for interacting
>> with serial flash memories. These chips have memory mapped interface,
>> through which entire serial flash memory slave can be read/written as if
>> though they are physical memories (like RAM). Using this interface,
>> flash can be accessed using memcpy() function and the spi controller
>> hardware will take care of communicating with serial flash over SPI.
>> Setting this flag will indicate the SPI controller driver that the
>> spi_message is from mtd layer to read from/write to flash. The SPI
>> master driver can then appropriately switch the controller to memory
>> mapped interface to read from/write to flash, based on this flag (See
>> drivers/spi/spi-ti-qspi.c for example).
>> NOTE: If the SPI controller chip lacks memory mapped interface, then the
>> driver will ignore this flag and use normal SPI protocol to read
>> from/write to flash. Communication with non-flash SPI devices is not
>> possible using the memory mapped interface.
> 
> 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?
> 

TI QSPI controller has two blocks:
1. SPI_CORE: This is generic(normal) spi mode. This can be used to
communicate with any SPI devices (serial flashes as well as non-flash
devices like touchscreen).
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.

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.

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

[toc] | [prev] | [next] | [standalone]

#1200418

Hello,

On 4 August 2015 at 19:59, R, Vignesh <vigneshr@ti.com> wrote:
>
>
> On 8/4/2015 9:21 PM, Mark Brown wrote:
>> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>>
>>> @use_mmap_mode: Some SPI controller chips are optimized for interacting
>>> with serial flash memories. These chips have memory mapped interface,
>>> through which entire serial flash memory slave can be read/written as if
>>> though they are physical memories (like RAM). Using this interface,
>>> flash can be accessed using memcpy() function and the spi controller
>>> hardware will take care of communicating with serial flash over SPI.
>>> Setting this flag will indicate the SPI controller driver that the
>>> spi_message is from mtd layer to read from/write to flash. The SPI
>>> master driver can then appropriately switch the controller to memory
>>> mapped interface to read from/write to flash, based on this flag (See
>>> drivers/spi/spi-ti-qspi.c for example).
>>> NOTE: If the SPI controller chip lacks memory mapped interface, then the
>>> driver will ignore this flag and use normal SPI protocol to read
>>> from/write to flash. Communication with non-flash SPI devices is not
>>> possible using the memory mapped interface.
>>
>> 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?
>>
>
> TI QSPI controller has two blocks:
> 1. SPI_CORE: This is generic(normal) spi mode. This can be used to
> communicate with any SPI devices (serial flashes as well as non-flash
> devices like touchscreen).
> 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.
>
> 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.
>

Maybe it can be set on the SPI slave rather than each message.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200422

On 08/05/2015 10:51 AM, Michal Suchanek wrote:
> Hello,
> 
> On 4 August 2015 at 19:59, R, Vignesh <vigneshr@ti.com> wrote:
>>
>>
>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>>> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>>>
>>>> @use_mmap_mode: Some SPI controller chips are optimized for interacting
>>>> with serial flash memories. These chips have memory mapped interface,
>>>> through which entire serial flash memory slave can be read/written as if
>>>> though they are physical memories (like RAM). Using this interface,
>>>> flash can be accessed using memcpy() function and the spi controller
>>>> hardware will take care of communicating with serial flash over SPI.
>>>> Setting this flag will indicate the SPI controller driver that the
>>>> spi_message is from mtd layer to read from/write to flash. The SPI
>>>> master driver can then appropriately switch the controller to memory
>>>> mapped interface to read from/write to flash, based on this flag (See
>>>> drivers/spi/spi-ti-qspi.c for example).
>>>> NOTE: If the SPI controller chip lacks memory mapped interface, then the
>>>> driver will ignore this flag and use normal SPI protocol to read
>>>> from/write to flash. Communication with non-flash SPI devices is not
>>>> possible using the memory mapped interface.
>>>
>>> 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?
>>>
>>
>> TI QSPI controller has two blocks:
>> 1. SPI_CORE: This is generic(normal) spi mode. This can be used to
>> communicate with any SPI devices (serial flashes as well as non-flash
>> devices like touchscreen).
>> 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.
>>
>> 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.
>>
> 
> Maybe it can be set on the SPI slave rather than each message.

You mean to add flag to spi_device struct? That's ok for me.

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

[toc] | [prev] | [next] | [standalone]

#1200426

On 5 August 2015 at 07:35, Vignesh R <vigneshr@ti.com> wrote:
>
>
> On 08/05/2015 10:51 AM, Michal Suchanek wrote:
>> Hello,
>>
>> On 4 August 2015 at 19:59, R, Vignesh <vigneshr@ti.com> wrote:
>>>
>>>
>>> On 8/4/2015 9:21 PM, Mark Brown wrote:
>>>> On Mon, Aug 03, 2015 at 10:27:19AM +0530, Vignesh R wrote:
>>>>

>>>
>>> TI QSPI controller has two blocks:
>>> 1. SPI_CORE: This is generic(normal) spi mode. This can be used to
>>> communicate with any SPI devices (serial flashes as well as non-flash
>>> devices like touchscreen).
>>> 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.
>>>
>>> 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.
>>>
>>
>> Maybe it can be set on the SPI slave rather than each message.
>
> You mean to add flag to spi_device struct? That's ok for me.
>

There are already mode flags so you can just add one more.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200664

[Multipart message — attachments visible in raw view] — view raw

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.

[toc] | [prev] | [next] | [standalone]

#1200702

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1200703

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:
> On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:

> > 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.

Why care - if something is physically in the same format as a flash read
command how would a device be able to tell that it wasn't actually a
flash read command?  The signals sent on the bus are going to be
identical anyway.

[toc] | [prev] | [next] | [standalone]

#1200709

On 5 August 2015 at 14:44, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:
>> On 5 August 2015 at 13:50, Mark Brown <broonie@kernel.org> wrote:
>
>> > 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.
>
> Why care - if something is physically in the same format as a flash read
> command how would a device be able to tell that it wasn't actually a
> flash read command?  The signals sent on the bus are going to be
> identical anyway.

Not only must the command be the same but also the response must be tha same.

The flash chip responds by sending arbitrary amount of data. Given
that transfer_one gets only the part that sends the read command and
the part to do the actual read may or may not follow this is getting a
bit hairy. Add in dummy bytes due to fast-read lag and page write
wrap-around and you get something that you definitely do not want
unless you are really sure that there is a flash memory on the other
end of the wire.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201596

[Multipart message — attachments visible in raw view] — view raw

On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote:
> On 5 August 2015 at 14:44, Mark Brown <broonie@kernel.org> wrote:
> > On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:

> >> 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.

> > Why care - if something is physically in the same format as a flash read
> > command how would a device be able to tell that it wasn't actually a
> > flash read command?  The signals sent on the bus are going to be
> > identical anyway.

> Not only must the command be the same but also the response must be tha same.

What difference would that make?  The caller is sending a single SPI
operation and this is a user visible thing...

> The flash chip responds by sending arbitrary amount of data. Given
> that transfer_one gets only the part that sends the read command and
> the part to do the actual read may or may not follow this is getting a
> bit hairy. Add in dummy bytes due to fast-read lag and page write
> wrap-around and you get something that you definitely do not want
> unless you are really sure that there is a flash memory on the other
> end of the wire.

So if you're doing this you may have a good reason to implement
transfer_one_message() instead.  Or perhaps implement it in the core and
provide operations to do the map and unmap.  And of course if this sort
of requirement exists that's an obvious thing that must be documented
in the interfaces but isn't.

We need a lot more thought about the interface here, the lack of any
explanation of what the interface is supposed to be and the fact that
all questions about it are being answered in terms of describing the
specific system are both a bit worrying.

[toc] | [prev] | [next] | [standalone]

#1201614

On 6 August 2015 at 11:02, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote:
>> On 5 August 2015 at 14:44, Mark Brown <broonie@kernel.org> wrote:
>> > On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:
>
>> >> 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.
>
>> > Why care - if something is physically in the same format as a flash read
>> > command how would a device be able to tell that it wasn't actually a
>> > flash read command?  The signals sent on the bus are going to be
>> > identical anyway.
>
>> Not only must the command be the same but also the response must be tha same.
>
> What difference would that make?  The caller is sending a single SPI
> operation and this is a user visible thing...
>
>> The flash chip responds by sending arbitrary amount of data. Given
>> that transfer_one gets only the part that sends the read command and
>> the part to do the actual read may or may not follow this is getting a
>> bit hairy. Add in dummy bytes due to fast-read lag and page write
>> wrap-around and you get something that you definitely do not want
>> unless you are really sure that there is a flash memory on the other
>> end of the wire.
>
> So if you're doing this you may have a good reason to implement
> transfer_one_message() instead.  Or perhaps implement it in the core and
> provide operations to do the map and unmap.  And of course if this sort
> of requirement exists that's an obvious thing that must be documented
> in the interfaces but isn't.
>
> We need a lot more thought about the interface here, the lack of any
> explanation of what the interface is supposed to be and the fact that
> all questions about it are being answered in terms of describing the
> specific system are both a bit worrying.

Disclaimer: I am not familiar with the hardware for which this patch
adds support.

However, I am familiar m25p80.c and as I understand it the controller
is basically supposed to implement m25p80.c in hardware when this flag
is set.

If I was using m25p80.c to talk to anything but an actual flash chip
it would get me quite worried.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201625

On Thu, Aug 06, 2015 at 12:01:37PM +0200, Michal Suchanek wrote:
> Disclaimer: I am not familiar with the hardware for which this patch
> adds support.
> 
> However, I am familiar m25p80.c and as I understand it the controller
> is basically supposed to implement m25p80.c in hardware when this flag
> is set.

That, to me, sounds like what you have is:

---m25p80 specific interface--->SPI bus--->m25p80 device

Where the m25p80 specific interface does not expose direct access to the
SPI bus?

If that's the case, then maybe you should consider whether using the SPI
bus infrastructure is really the best way forward.  Would it make more
sense instead to adopt a different software structure, something more
high-level like:

     +-------------------------------------------+
     |          m25p80 high-level driver         |
     +----------------------+--------------------+
     |   SPI m25p80 driver  |                    |
     +----------------------+                    |
     |      SPI layer       |  Special driver    |
     +----------------------+                    |
     |    SPI bus driver    |                    |
     +----------------------+--------------------+
     |     SPI hardware     |  Special hardware  |
     +----------------------+--------------------+

Rather than what you seem to be trying to do, which seems to be:

     +----------------------+
     |   SPI m25p80 driver  |
     +----------------------+
     |      SPI layer       |
     +----------------------+
     |  Translation driver  |
     +----------------------+
     |   Special hardware   |
     +----------------------+

where this requires M25P80 specific hacks to be introduced into the SPI
layer so that you can communicate additional information between the SPI
M25P80 driver and the translation driver.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201644

[Multipart message — attachments visible in raw view] — view raw

On Thu, Aug 06, 2015 at 11:22:25AM +0100, Russell King - ARM Linux wrote:

> If that's the case, then maybe you should consider whether using the SPI
> bus infrastructure is really the best way forward.  Would it make more
> sense instead to adopt a different software structure, something more
> high-level like:

>      +-------------------------------------------+
>      |          m25p80 high-level driver         |
>      +----------------------+--------------------+
>      |   SPI m25p80 driver  |                    |
>      +----------------------+                    |
>      |      SPI layer       |  Special driver    |
>      +----------------------+                    |
>      |    SPI bus driver    |                    |
>      +----------------------+--------------------+
>      |     SPI hardware     |  Special hardware  |
>      +----------------------+--------------------+

Yes, that's what's been talked about before - for some of these devices
they're sufficiently flash specialist that we just don't bother exposing
a SPI interface at all though AIUI they could be persuaded to do it.  It
isn't entirely clear that we want exactly that split, if the devices are
reasonable SPI controllers we will want to handle the case where they
have flash and non-flash devices on the same bus.  For that there is
going to be some generalisable work possible for managing switching
between memory mapped and SPI modes where those are mutually exclusive,
especially if the switch between them isn't free.

[toc] | [prev] | [next] | [standalone]

#1201649

On 6 August 2015 at 12:22, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 06, 2015 at 12:01:37PM +0200, Michal Suchanek wrote:
>> Disclaimer: I am not familiar with the hardware for which this patch
>> adds support.
>>
>> However, I am familiar m25p80.c and as I understand it the controller
>> is basically supposed to implement m25p80.c in hardware when this flag
>> is set.
>
> That, to me, sounds like what you have is:
>
> ---m25p80 specific interface--->SPI bus--->m25p80 device
>
> Where the m25p80 specific interface does not expose direct access to the
> SPI bus?

The m25p80 specific hardware interface is presumably optional so you
can use it or not. The description is a bit vague, though.

In fsl-qspi the driver does not make it optional. I am not sure that
controller can be used for non-m25p80 slaves.


>
> If that's the case, then maybe you should consider whether using the SPI
> bus infrastructure is really the best way forward.  Would it make more
> sense instead to adopt a different software structure, something more
> high-level like:
>
>      +-------------------------------------------+
>      |          m25p80 high-level driver      =spi-nor   |
>      +----------------------+--------------------+
>      |   SPI m25p80 driver  |                    |
>      +----------------------+                    |
>      |      SPI layer       |  Special driver    =fsl-qspi|
>      +----------------------+                    |
>      |    SPI bus driver    |                    |
>      +----------------------+--------------------+
>      |     SPI hardware     |  Special hardware  |
>      +----------------------+--------------------+
>

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201696

On 08/06/2015 03:52 PM, Russell King - ARM Linux wrote:
> On Thu, Aug 06, 2015 at 12:01:37PM +0200, Michal Suchanek wrote:
>> Disclaimer: I am not familiar with the hardware for which this patch
>> adds support.
>>
>> However, I am familiar m25p80.c and as I understand it the controller
>> is basically supposed to implement m25p80.c in hardware when this flag
>> is set.
> 
> That, to me, sounds like what you have is:
> 
> ---m25p80 specific interface--->SPI bus--->m25p80 device
> 
> Where the m25p80 specific interface does not expose direct access to the
> SPI bus?
> 

Let me give overview of ti-qspi controller:

There are two interfaces in the controller, one exposes direct access to
SPI bus and the other doesn't. It is possible to dynamically switch
between these ports by writing to QSPI_SPI_SWITCH_REG.

The two interface are [1]:
1) Generic SPI interface: (config port): with this interface, ti-qspi
controller can communicate with *any* spi device (flash and non-flash).
This interface is provides direct access to SPI bus.

2) SPI memory mapped interface (memory mapped port): This is m25p80
specific interface which can be use to read data from flash.
But if the flash has to be configured to some particular mode like QUAD
READ MODE, then the controller needs to be put in to config port to read
and modify serial flash's config register and then switch to memory
mapped port in order to read data stored on flash.

For example on DRA74 evm, if a 64 MB flash is connected as a slave,
entire flash memory is visible from 0x5C000000 to 0x5FFFFFFF L3_MAIN
address. In order to read using memory mapped port following will be the
sequence:

1.Write to flash config register via config port to switch to QUAD MODE
(or any mode that flash supports).
2. Populate QSPI_SPI_SETUP_REGx with flash read command, number of
address bytes to use and dummy bytes required.
3. Switch to memory mapped port by writing to QSPI_SPI_SWITCH_REG.
4. Now, its possible to perform read from 0x5C000000 to 0x5FFFFFFF using
memcpy. The qspi controller hardware will communicate over SPI bus and
get the data. This data is directly sent to RAM via SoC's interconnect.

Advantages of memory mapped port are: improved read performance,
MEM_TO_MEM DMA support can be added (ti-qspi hardware as such does not
provide DMA events).

Advantages of config port: can be used to communicate with *any* SPI
device, provides direct read/write access to SPI bus.

On the whole following are my requirements:
1. to be able to communicate with non -flash SPI devices via config port
( this functionality is supported by current driver, I dont want to
break it). Or pump any spi_message on to SPI bus directly.
2. take advantage of memory mapped port in order to increase read
throughput( and use dma in future) when the slave is a m25p80 type flash.
3. handle m25p80 as well as other slave on multiple chipselects.

I just need to know whether the user that requested the transfer is
m25p80 driver. If yes, ti-qspi driver can take advantage of memory
mapped interface, else just use config port to access SPI bus directly.

Writing separate driver based on spi-nor framework to interface with
m25p80 is not an option because, I would lose the ability to interface
with non-flash devices.

The spi_message that is received in transfer_one_message() is too
generic to imply the slave device that is on the other side of the wire.
IMO, the read command does not imply that the slave is m25p80 flash
(besides the read opcodes vary across vendors of m25p80 and across modes).

As Michal suggested, adding a flag to spi_device to distinguish whether
the slave is a m25p80 flash type will help spi master to handle
optimizations specific to m25p80s while being generic enough to handle
all other spi devices. Is that ok? Is there any other way to imply what
slave as at the other end?

[1] TRM: http://www.ti.com/lit/ug/spruhz6/spruhz6.pdf 24.5.4 QSPI
Functional Description
-- 
Regards
Vignesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201758

On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote:
> On the whole following are my requirements:
> 1. to be able to communicate with non -flash SPI devices via config port
> ( this functionality is supported by current driver, I dont want to
> break it). Or pump any spi_message on to SPI bus directly.
> 2. take advantage of memory mapped port in order to increase read
> throughput( and use dma in future) when the slave is a m25p80 type flash.
> 3. handle m25p80 as well as other slave on multiple chipselects.
> 
> I just need to know whether the user that requested the transfer is
> m25p80 driver. If yes, ti-qspi driver can take advantage of memory
> mapped interface, else just use config port to access SPI bus directly.

The problem with this approach is that it's an abomination.  It's adding
a SPI-user specific hack which is detected by a specific driver.  That's
really not sane - what happens when we have lots of these kinds of "I'm
an X SPI-user" with drivers detecting that?  It's not maintainable in the
long term.

Yes, your requirements _today_ seem simple and easy, but you're only
thinking about today, not tomorrow when you've moved on and someone else
has to maintain the mess left behind (or delete it from mainline because
they're sick of dealing with a hack.)

> The spi_message that is received in transfer_one_message() is too
> generic to imply the slave device that is on the other side of the wire.
> IMO, the read command does not imply that the slave is m25p80 flash
> (besides the read opcodes vary across vendors of m25p80 and across modes).

I can see both sides of the argument.

Mark is saying: if the SPI driver detects that the message to be transmitted
is a read command followed by the appropriate number of dummy bytes, and
then the data being read _and_ it's using quad-mode access, and the hardware
generates _exactly_ that in hardware using the memory mapped mode, there is
no reason _not_ to use the hardware to achieve that SPI transaction.  The
bus activity will be identical to what happens when the SPI controller is
used manually to achieve that bus sequence.

You're saying: but the documentation says you can't use it for anything
except m25p80.  If you look at 24.5.4.1.2, it tells you what the SFI
generates on the bus, which is:

1. CS active
2. Read command byte sent
3. 1-4 address bytes sent
4. 0-3 dummy bytes sent
5. data bytes read from bus
6. CS inactive

So, Mark's point is "if we can detect a transaction which fits _that_
bus activity, there's no reason not to use this acceleration for the
transaction."

What you're failing to counter with is: we don't have enough information
in the SPI driver to know how many dummy bytes there are between the
address bytes and the data read from the bus.

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.

I think that comprehensively rules out Mark's idea with the SPI core as
it stands today: there is no way for the SPI driver to reliably detect
a message like a SFI read command by parsing the SPI transmitted message
prior to sending it as we can never be sure whether there are dummy bytes
to be transmitted.

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:

- transmit 2 to 5 bytes where the first byte is a recognised read command
- transmit 0 to 3 dummy bytes
- read some bytes

then it can make use of the SFI mode to accelerate the operation.

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.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201885

On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote:
>> On the whole following are my requirements:
>> 1. to be able to communicate with non -flash SPI devices via config port
>> ( this functionality is supported by current driver, I dont want to
>> break it). Or pump any spi_message on to SPI bus directly.
>> 2. take advantage of memory mapped port in order to increase read
>> throughput( and use dma in future) when the slave is a m25p80 type flash.
>> 3. handle m25p80 as well as other slave on multiple chipselects.
>>
>> I just need to know whether the user that requested the transfer is
>> m25p80 driver. If yes, ti-qspi driver can take advantage of memory
>> mapped interface, else just use config port to access SPI bus directly.
>
> The problem with this approach is that it's an abomination.  It's adding
> a SPI-user specific hack which is detected by a specific driver.  That's
> really not sane - what happens when we have lots of these kinds of "I'm
> an X SPI-user" with drivers detecting that?  It's not maintainable in the
> long term.
>
> Yes, your requirements _today_ seem simple and easy, but you're only
> thinking about today, not tomorrow when you've moved on and someone else
> has to maintain the mess left behind (or delete it from mainline because
> they're sick of dealing with a hack.)
>
>> The spi_message that is received in transfer_one_message() is too
>> generic to imply the slave device that is on the other side of the wire.
>> IMO, the read command does not imply that the slave is m25p80 flash
>> (besides the read opcodes vary across vendors of m25p80 and across modes).
>
> I can see both sides of the argument.
>
> Mark is saying: if the SPI driver detects that the message to be transmitted
> is a read command followed by the appropriate number of dummy bytes, and
> then the data being read _and_ it's using quad-mode access, and the hardware
> generates _exactly_ that in hardware using the memory mapped mode, there is
> no reason _not_ to use the hardware to achieve that SPI transaction.  The
> bus activity will be identical to what happens when the SPI controller is
> used manually to achieve that bus sequence.
>
> You're saying: but the documentation says you can't use it for anything
> except m25p80.  If you look at 24.5.4.1.2, it tells you what the SFI
> generates on the bus, which is:
>
> 1. CS active
> 2. Read command byte sent
> 3. 1-4 address bytes sent
> 4. 0-3 dummy bytes sent
> 5. data bytes read from bus
> 6. CS inactive
>
> So, Mark's point is "if we can detect a transaction which fits _that_
> bus activity, there's no reason not to use this acceleration for the
> transaction."
>
> What you're failing to counter with is: we don't have enough information
> in the SPI driver to know how many dummy bytes there are between the
> address bytes and the data read from the bus.

Irrespective of the dummy bytes.
What if the spi device is not a FLASH ROM, but some other device,
which receives a data packet that accidentally looks like an m25p80 READ
command?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1201965

On 6 August 2015 at 18:14, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote:
>>> On the whole following are my requirements:
>>> 1. to be able to communicate with non -flash SPI devices via config port
>>> ( this functionality is supported by current driver, I dont want to
>>> break it). Or pump any spi_message on to SPI bus directly.
>>> 2. take advantage of memory mapped port in order to increase read
>>> throughput( and use dma in future) when the slave is a m25p80 type flash.
>>> 3. handle m25p80 as well as other slave on multiple chipselects.
>>>
>>> I just need to know whether the user that requested the transfer is
>>> m25p80 driver. If yes, ti-qspi driver can take advantage of memory
>>> mapped interface, else just use config port to access SPI bus directly.
>>
>> The problem with this approach is that it's an abomination.  It's adding
>> a SPI-user specific hack which is detected by a specific driver.  That's
>> really not sane - what happens when we have lots of these kinds of "I'm
>> an X SPI-user" with drivers detecting that?  It's not maintainable in the
>> long term.
>>
>> Yes, your requirements _today_ seem simple and easy, but you're only
>> thinking about today, not tomorrow when you've moved on and someone else
>> has to maintain the mess left behind (or delete it from mainline because
>> they're sick of dealing with a hack.)
>>
>>> The spi_message that is received in transfer_one_message() is too
>>> generic to imply the slave device that is on the other side of the wire.
>>> IMO, the read command does not imply that the slave is m25p80 flash
>>> (besides the read opcodes vary across vendors of m25p80 and across modes).
>>
>> I can see both sides of the argument.
>>
>> Mark is saying: if the SPI driver detects that the message to be transmitted
>> is a read command followed by the appropriate number of dummy bytes, and
>> then the data being read _and_ it's using quad-mode access, and the hardware
>> generates _exactly_ that in hardware using the memory mapped mode, there is
>> no reason _not_ to use the hardware to achieve that SPI transaction.  The
>> bus activity will be identical to what happens when the SPI controller is
>> used manually to achieve that bus sequence.
>>
>> You're saying: but the documentation says you can't use it for anything
>> except m25p80.  If you look at 24.5.4.1.2, it tells you what the SFI
>> generates on the bus, which is:
>>
>> 1. CS active
>> 2. Read command byte sent
>> 3. 1-4 address bytes sent
>> 4. 0-3 dummy bytes sent
>> 5. data bytes read from bus
>> 6. CS inactive
>>
>> So, Mark's point is "if we can detect a transaction which fits _that_
>> bus activity, there's no reason not to use this acceleration for the
>> transaction."
>>
>> What you're failing to counter with is: we don't have enough information
>> in the SPI driver to know how many dummy bytes there are between the
>> address bytes and the data read from the bus.
>
> Irrespective of the dummy bytes.
> What if the spi device is not a FLASH ROM, but some other device,
> which receives a data packet that accidentally looks like an m25p80 READ
> command?
>

Presumably the driver would interpret some random part of the message
as address and map the reply in your address space at that address
from the flash mmap base.

If you happen to overflow the flash memory mmap space the behaviour
will probably not be well defined.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202090

On Thu, Aug 06, 2015 at 06:14:00PM +0200, Geert Uytterhoeven wrote:
> On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote:
> >> On the whole following are my requirements:
> >> 1. to be able to communicate with non -flash SPI devices via config port
> >> ( this functionality is supported by current driver, I dont want to
> >> break it). Or pump any spi_message on to SPI bus directly.
> >> 2. take advantage of memory mapped port in order to increase read
> >> throughput( and use dma in future) when the slave is a m25p80 type flash.
> >> 3. handle m25p80 as well as other slave on multiple chipselects.
> >>
> >> I just need to know whether the user that requested the transfer is
> >> m25p80 driver. If yes, ti-qspi driver can take advantage of memory
> >> mapped interface, else just use config port to access SPI bus directly.
> >
> > The problem with this approach is that it's an abomination.  It's adding
> > a SPI-user specific hack which is detected by a specific driver.  That's
> > really not sane - what happens when we have lots of these kinds of "I'm
> > an X SPI-user" with drivers detecting that?  It's not maintainable in the
> > long term.
> >
> > Yes, your requirements _today_ seem simple and easy, but you're only
> > thinking about today, not tomorrow when you've moved on and someone else
> > has to maintain the mess left behind (or delete it from mainline because
> > they're sick of dealing with a hack.)
> >
> >> The spi_message that is received in transfer_one_message() is too
> >> generic to imply the slave device that is on the other side of the wire.
> >> IMO, the read command does not imply that the slave is m25p80 flash
> >> (besides the read opcodes vary across vendors of m25p80 and across modes).
> >
> > I can see both sides of the argument.
> >
> > Mark is saying: if the SPI driver detects that the message to be transmitted
> > is a read command followed by the appropriate number of dummy bytes, and
> > then the data being read _and_ it's using quad-mode access, and the hardware
> > generates _exactly_ that in hardware using the memory mapped mode, there is
> > no reason _not_ to use the hardware to achieve that SPI transaction.  The
> > bus activity will be identical to what happens when the SPI controller is
> > used manually to achieve that bus sequence.
> >
> > You're saying: but the documentation says you can't use it for anything
> > except m25p80.  If you look at 24.5.4.1.2, it tells you what the SFI
> > generates on the bus, which is:
> >
> > 1. CS active
> > 2. Read command byte sent
> > 3. 1-4 address bytes sent
> > 4. 0-3 dummy bytes sent
> > 5. data bytes read from bus
> > 6. CS inactive
> >
> > So, Mark's point is "if we can detect a transaction which fits _that_
> > bus activity, there's no reason not to use this acceleration for the
> > transaction."
> >
> > What you're failing to counter with is: we don't have enough information
> > in the SPI driver to know how many dummy bytes there are between the
> > address bytes and the data read from the bus.
> 
> Irrespective of the dummy bytes.
> What if the spi device is not a FLASH ROM, but some other device,
> which receives a data packet that accidentally looks like an m25p80 READ
> command?

Well, for the most part it looks like it should still work, but there
could be a gotcha, but first, let's get rid of a myth there.

The QSPI is _not_ specific to the M25P80.  The manual says nothing
about being specific to that device.  What it says is that it's for
SPI NOR memory.  It will work with bus widths of 1, 2 or 4 data lines,
so it probably works with non-M25P80 SPI NOR devices too - and the fact
that the read and write commands are completely programmable suggests
that using it with SPI NOR devices which do not use the M25P80 read
command value is intended.


The SFI is a state machine based translator which sits behind the SPI
interface (look at the manual).  It sequence sthe SPI bus through a
series of standard SPI states which happen to be the states I detailed
above.

Now, the first byte of the SFI-generated SPI message can be programmed
to any 8 bit value.  So the first byte of the SPI message is totally
under software control.  The next one to four bytes which comprise the
"address" can be controlled to by deciding where in the memory map to
start reading from.  Hence, the value of those bytes is also totally
under software control.  The number of dummy bytes can be programmed
too.  So far so good.

So, if we know that we have a SPI message which says "send 0x01 0x20
0x30, send one dummy byte, read 32 bytes", if we program the SFI to
send a read command as 0x01, program an address length of 2 bytes
with one dummy byte, and then read the next 32 bytes at the appropriate
offset in the memory mapping to cause the next two bytes to be 0x20,
0x30, then what we end up with on the bus is:

	send 0x01, 0x20, 0x30
	send one dummy byte

That much is good, but now is the problem - how does the SFI know that
we're going to require to read 32 bytes?  I think the answer to that
is that it doesn't know, so it probably just reads the number of bytes
which the access on the SoC bus is asking for, which makes it
indeterminant from a software point of view to control how many bytes
will be read without provoking another "send 0x01, next address, dummy
byte" sequence.

So, I'm now on the side of not parsing commands in the SPI driver, and
back on the idea that this needs to be handled in some other manner
which doesn't involve polluting the SPI core with flag-hacks.

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1202376

On 6 August 2015 at 23:33, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Thu, Aug 06, 2015 at 06:14:00PM +0200, Geert Uytterhoeven wrote:
>> On Thu, Aug 6, 2015 at 3:51 PM, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Thu, Aug 06, 2015 at 05:55:23PM +0530, Vignesh R wrote:
>> >> On the whole following are my requirements:
>> >> 1. to be able to communicate with non -flash SPI devices via config port
>> >> ( this functionality is supported by current driver, I dont want to
>> >> break it). Or pump any spi_message on to SPI bus directly.
>> >> 2. take advantage of memory mapped port in order to increase read
>> >> throughput( and use dma in future) when the slave is a m25p80 type flash.
>> >> 3. handle m25p80 as well as other slave on multiple chipselects.
>> >>
>> >> I just need to know whether the user that requested the transfer is
>> >> m25p80 driver. If yes, ti-qspi driver can take advantage of memory
>> >> mapped interface, else just use config port to access SPI bus directly.
>> >
>> > The problem with this approach is that it's an abomination.  It's adding
>> > a SPI-user specific hack which is detected by a specific driver.  That's
>> > really not sane - what happens when we have lots of these kinds of "I'm
>> > an X SPI-user" with drivers detecting that?  It's not maintainable in the
>> > long term.
>> >
>> > Yes, your requirements _today_ seem simple and easy, but you're only
>> > thinking about today, not tomorrow when you've moved on and someone else
>> > has to maintain the mess left behind (or delete it from mainline because
>> > they're sick of dealing with a hack.)
>> >
>> >> The spi_message that is received in transfer_one_message() is too
>> >> generic to imply the slave device that is on the other side of the wire.
>> >> IMO, the read command does not imply that the slave is m25p80 flash
>> >> (besides the read opcodes vary across vendors of m25p80 and across modes).
>> >
>> > I can see both sides of the argument.
>> >
>> > Mark is saying: if the SPI driver detects that the message to be transmitted
>> > is a read command followed by the appropriate number of dummy bytes, and
>> > then the data being read _and_ it's using quad-mode access, and the hardware
>> > generates _exactly_ that in hardware using the memory mapped mode, there is
>> > no reason _not_ to use the hardware to achieve that SPI transaction.  The
>> > bus activity will be identical to what happens when the SPI controller is
>> > used manually to achieve that bus sequence.
>> >
>> > You're saying: but the documentation says you can't use it for anything
>> > except m25p80.  If you look at 24.5.4.1.2, it tells you what the SFI
>> > generates on the bus, which is:
>> >
>> > 1. CS active
>> > 2. Read command byte sent
>> > 3. 1-4 address bytes sent
>> > 4. 0-3 dummy bytes sent
>> > 5. data bytes read from bus
>> > 6. CS inactive
>> >
>> > So, Mark's point is "if we can detect a transaction which fits _that_
>> > bus activity, there's no reason not to use this acceleration for the
>> > transaction."
>> >
>> > What you're failing to counter with is: we don't have enough information
>> > in the SPI driver to know how many dummy bytes there are between the
>> > address bytes and the data read from the bus.
>>
>> Irrespective of the dummy bytes.
>> What if the spi device is not a FLASH ROM, but some other device,
>> which receives a data packet that accidentally looks like an m25p80 READ
>> command?
>
> Well, for the most part it looks like it should still work, but there
> could be a gotcha, but first, let's get rid of a myth there.
>
> The QSPI is _not_ specific to the M25P80.  The manual says nothing
> about being specific to that device.  What it says is that it's for
> SPI NOR memory.  It will work with bus widths of 1, 2 or 4 data lines,
> so it probably works with non-M25P80 SPI NOR devices too - and the fact
> that the read and write commands are completely programmable suggests
> that using it with SPI NOR devices which do not use the M25P80 read
> command value is intended.
>
...
>
> That much is good, but now is the problem - how does the SFI know that
> we're going to require to read 32 bytes?  I think the answer to that
> is that it doesn't know, so it probably just reads the number of bytes
> which the access on the SoC bus is asking for, which makes it
> indeterminant from a software point of view to control how many bytes
> will be read without provoking another "send 0x01, next address, dummy
> byte" sequence.
>
> So, I'm now on the side of not parsing commands in the SPI driver, and
> back on the idea that this needs to be handled in some other manner
> which doesn't involve polluting the SPI core with flag-hacks.

OK, so we can agree that using this hardware acceleration for any kind
of transfer indiscriminately is not a very good idea.

Now since the description is clearer it's obvious that ti-qspi cannot
work fully mmapped as fsl-qspi does because the setup has to be done
over normal spi access and using non-m25p80 devices on the same bus is
a requirement.

The place where it is known if a transfer can use the mmap access is m25p80.c

So my suggestion is

 - add a new method for spi master that gets the read opcode, dummy
length, address, address length, buffer, buffer length and performs
read from the flash memory in a hardware-specific way

- add a check in m25p80.c that the master supports this feature and if
so use it (eg check that the method is non-null)

Presumably if some new SPI controllers with similar feature are
supported in the future they can use the same inteface because you
pass on everything the m25p80 read knows.

Thanks

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web