On 4/19/21 3:06 AM, Jagan Teki wrote:
On Sat, Apr 3, 2021 at 4:37 AM Sean Anderson <[email protected]> wrote:

On 4/2/21 7:05 PM, Sean Anderson wrote:
This series adds support for enhanced SPI modes. It was tested on a K210 (DWC
SSI with QSPI flash).

If anyone has a designware device with QSPI flash attached (especially a DW SSI
APB device), I'd greatly appreciate them testing out this patch series. Given
that there has been no testing of v2 over the past month, I don't think lack of
testing should hold up this series.

Changes in v3:
- Dropped merged patches
- Rebased on u-boot/master

Changes in v2:
- Add more information to exec_op debug message
- Actually mask interrupts
- Merge CAP_{DUAL,QUAD,OCTAL} into CAP_ENHANCED
- Fix some inconsistencies in register naming and usage
- Moved some hunks between commits so things make more sense

Sean Anderson (10):
    mtd: spi-mem: Export spi_mem_default_supports_op
    spi: spi-mem: Add debug message for spi-mem ops
    spi: dw: Log status register on timeout
    spi: dw: Actually mask interrupts
    spi: dw: Switch to capabilities
    spi: dw: Rewrite poll_transfer logic
    spi: dw: Add ENHANCED cap
    spi: dw: Define registers for enhanced mode
    spi: dw: Support enhanced SPI
    spi: dw: Support clock stretching

   drivers/spi/designware_spi.c | 647 ++++++++++++++++++++++++-----------
   drivers/spi/spi-mem.c        |   7 +
   include/spi-mem.h            |   3 +
   3 files changed, 451 insertions(+), 206 deletions(-)


Looks like I forgot to bump the version. This should be v3. I can resend if 
necessary.

Yes, Please.

There are few comments,

1. Patch "spi: spi-mem: Add debug message for spi-mem ops"

As we discussed in the previous version, drop the unnecessary debug
statements after ops execution as this patch trying to add more
verbose to before ops execution.

From last time:

+       dev_dbg(slave->dev,
+               "exec %02Xh %u-%u-%u addr=%llx dummy cycles=%u data bytes=%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,
+               op->data.nbytes);

This looks unnecessary to me, we have debug prints at the end of the
function, which indeed sufficient.


This is insufficient. First, that information is printed after the op
is executed. This is too late if one is trying to debug (e.g.) a hang in
the spi driver. It's also missing opcode, widths, address, and dummy
bytes. The only duplicated info here is nbytes. Since this is debugging
information, I think we should print what is most useful for people
debugging this subsystem.

I would like if you could respond to my response before I remove this.


2. Comments in Patch v2,06/10

I will look into that before resending.

--Sean


Jagan.



Reply via email to