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

[Michal Suchanek <hramrach@xxxxxxxxx>]

On 6 August 2015 at 11:02, Mark Brown <broonie@xxxxxxxxxx> wrote:
> On Wed, Aug 05, 2015 at 02:56:09PM +0200, Michal Suchanek wrote:
>> On 5 August 2015 at 14:44, Mark Brown <broonie@xxxxxxxxxx> 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-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html