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

Sean Anderson
2021-02-01T15:33:06Z

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

> 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 at microchip.com/

Would be nice :)

--Sean

>
> With these fixed,
>
> Reviewed-by: Pratyush Yadav <p.yadav@ti.com>
>
>> + op->data.nbytes);
>> +
>> if (!spi_mem_supports_op(slave, op))
>> return -ENOTSUPP;
>>
>