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 devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html