On 6 August 2015 at 11:02, Mark Brown [off-list ref] wrote:

On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote:

quoted

On 5 August 2015 at 14:44, Mark Brown [off-list ref] wrote:

quoted

On Wed, Aug 05, 2015 at 02:40:01PM +0200, Michal Suchanek wrote:

quoted

quoted

quoted

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.

quoted

quoted

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.

quoted

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

quoted

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