Re: [PATCH v7 01/14] spi: spi-mem: Fix a DTR related check in spi_mem_dtr_supports_op()

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Pratyush,

p.yadav@ti.com wrote on Tue, 21 Dec 2021 00:09:19 +0530:

> On 17/12/21 05:16PM, Miquel Raynal wrote:
> > It seems that the number of command bytes must be "2" only when the
> > command itself is sent in DTR mode. The current logic checks if the
> > number of command bytes is "2" when any of the cycles is a DTR cycle. It
> > is likely that so far no device was actually mixing DTR/non-DTR cycles
> > in the same operation, explaining why this was left undetected until
> > now.  
> 
> This was intentional. spi_mem_dtr_supports_op() must only be called when 
> the operation is DTR in all phases so I did not add any sanity checks if 
> someone was using it for non-DTR ops.

Maybe that was the original intention but since then the Macronix
driver has been merged and supports (at lest does not reject) these
modes.

> In fact, I added on to this 
> function in [0] to check nbytes for other phases as well. The patch fell 
> off my radar unfortunately, and it didn't get merged.
> 
> I would like to keep this as it is since we have no user of mixed 
> DTR/non-DTR modes yet.

I don't know if the Macronix driver really supports it or if it is the
driver that is doing the wrong checks but in appearance such mixed mode
could be used.

> But if you really want to support it, please 
> apply my patch first to make sure we check for every phase, not just 
> command.
> 
> [0] https://lore.kernel.org/all/20210531181757.19458-5-p.yadav@ti.com/

Nice, I might take that as well indeed in order to make the checks a
little bit more robust.

Thanks,
Miquèl