[PATCH 05/14] spi: spi-mem: Add debug message for spi-mem ops

[Pratyush Yadav]

On 01/02/21 10:33AM, Sean Anderson wrote:
> On 2/1/21 7:18 AM, Pratyush Yadav wrote:
> > On 31/01/21 07:34PM, Sean Anderson wrote:
> > > This prints some basic metadata about the SPI memory op. This information
> > > may be used to debug SPI drivers (e.g. determining the expected SPI mode).
> > > It is also helpful for verifying that the data on the wire matches the data
> > > intended to be transmitted (e.g. with a logic analyzer). The opcode is
> > > printed with a format of %02Xh to match the notation commonly used in flash
> > > datasheets.
> > > 
> > > Signed-off-by: Sean Anderson <seanga2 at gmail.com>
> > > ---
> > > 
> > >   drivers/spi/spi-mem.c | 6 ++++++
> > >   1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/drivers/spi/spi-mem.c b/drivers/spi/spi-mem.c
> > > index c095ae9505..eb83c11910 100644
> > > --- a/drivers/spi/spi-mem.c
> > > +++ b/drivers/spi/spi-mem.c
> > > @@ -220,6 +220,12 @@ int spi_mem_exec_op(struct spi_slave *slave, const struct spi_mem_op *op)
> > >   	int ret;
> > >   	int i;
> > > +	dev_dbg(slave->dev, "exec %02Xh %u-%u-%u addr=%llx wait=%u len=%u\n",
> > > +		op->cmd.opcode, op->cmd.buswidth, op->addr.buswidth,
> > > +		op->data.buswidth, op->addr.val,
> > > +		op->dummy.buswidth ? op->dummy.nbytes * 8 / op->dummy.buswidth : 0,
> > 
> > SPI MEM deals with dummy bytes not cycles [0]. Just print them directly
> > instead of converting them to cycles first.
> 
> I chose to do it this way, since as far as I can tell, most quad-spi
> drivers (with the exception of mtk_snfi_spi.c, octeon_spi.c, spi-sifive.c,
> and ti_qspi.c) deal with cycles and not bytes. The spi-nor system also
> specifies dummies in cycles and only converts them to bytes when
> creating a struct spi_mem_op. SPI flash datasheets also typically refer
> to dummies in cycles; since there is no data being transmitted, they
> cannot be truly measured in bytes. Lastly, it is much easier to only
> work in cycles when looking at logic analyzer traces. When switching
> between 1-1-4 and 1-4-4 instructions, the meaning of "cycles" remains
> the same, while "bytes" has a variable amount of time.

I agree that dummy cycles is a better unit than dummy bytes, but that is 
not what is currently being used. You are interpreting the dummy.nbytes 
field rather than simply printing it and letting the reader see what it 
means. We don't pass dummy cycles to the controller. We pass dummy 
bytes. So it makes sense to print that in the debug message.

Anyway, I feel like we are getting into bikeshedding territory. So 
either is fine by me. Choose whichever makes more sense to you, keeping 
my arguments in mind.
 
> > Also agree with Bin that "dummy cycles=" (or dummy bytes if you are
> > printing bytes directly) is clearer.
> 
> Sure.
> > 
> > [0] For now. This might change later. See
> > https://lore.kernel.org/linux-mtd/6396018a-485f-6eb4-7742-bdb5c4335ed4@microchip.com/
> 
> Would be nice :)
> 
> --Sean
> 
> > 
> > With these fixed,
> > 
> > Reviewed-by: Pratyush Yadav <p.yadav at ti.com>
> > 
> > > +		op->data.nbytes);
> > > +
> > >   	if (!spi_mem_supports_op(slave, op))
> > >   		return -ENOTSUPP;
> > 
> 

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.