[PATCH v3 0/2] spi: cadence-quadpsi: Add support for the Cadence QSPI controller
From: Ramuthevar,Vadivel MuruganX
Date: Mon Dec 09 2019 - 03:40:42 EST
From: "Ramuthevar Vadivel Muruganx" <vadivel.muruganx.ramuthevar@xxxxxxxxxxxxxxx>
Add support for the Cadence QSPI controller. This controller is
present in the Intel Lightning Mountain(LGM) SoCs, Altera and TI SoCs.
This driver has been tested on the Intel LGM SoCs.
This driver does not support generic SPI and also the implementation
only supports spi-mem interface to replace the existing driver in
mtd/spi-nor/cadence-quadspi.c, the existing driver only support SPI-NOR
flash memory.
Thank you Vignesh for the valuable review comments and guidance,
sorry for the delay since waitting for the merge window to rebase
and send it now by one-shot.
changes from V2:
1. Supports Octal mode as well. So its 1/2/4/8-bit wide
2. Please be consistent with function names. Start all functions with
cadence_qspi_. If need you could have a shorter function prefix like cqspi_
Also, this and few other functions below can be static and they are not
used outside of this module.
3. Could you please keep polling routines from original driver
(drivers/mtd/spi-nor/cadence-quadspi.c). If you really want to improve
these helpers, please do those changes in a separate patch later on in
the series. This will greatly help in reviewing the code.
Also, I see that you have dropped many inline comments in the code.
These comments are quite important and I suggest not to drop them unless
they are no longer applicable
3. s/docoder/decoder
4. Please use cqspi_wait_for_bit() that uses readl_relaxed_poll_timeout()
here. There are couple corner cases that above code does not take care of.
5. Why do we need flash type specific handling here? Does not spi-nand
driver provide appropriate dummy_clk values? Why is the driver always
adding 8 dummy clks always?
6. Nope, no custom handling or interpreting of opcodes in driver.
Just set addrwidth to quad in CQSPI_REG_WR_INSTR when op->addr.buswidth
is 4.
7. This was wait_for_completion_timeout() previously. Have you tested
interrupting the sleep and verified that driver handles this case correctly?
8 rdid_length harcoded values are changed as generic
9. Please get ride of all flash type specific handling... There should be
no need for it. IF there are gaps in then lets discuss and fix it in
spi-nand and spi-nor frameworks and not in the driver.
spi-mem user should just take spi_mem_op template passed by the core and
execute it. No assumptions wrt flash type or opcode should be done.
10. Decision to use STIG was INDAC mode can be done using:
if (op->data.dir == SPI_MEM_DATA_IN && op->data.buf.in) {
if (!op->addr.nbytes)
mode = CQSPI_STIG_READ;
else
mode = IDC_READ_MODE;
} else {
if (!op->addr.nbytes || !op->data.buf.out)
mode = CQSPI_STIG_WRITE;
else
mode = IDC_WRITE_MODE;
}
No need for rdid_length etc.
11. This is a big gap and there is lot of code sharing that can be done b/w
INDAC and DAC mode.
Looks like driver isn't quite close to being ready. I suggest you also
take a look at patches for spi-mem conversion of cadence-quadspi driver
in U-Boot as well:
https://patchwork.ozlabs.org/cover/1176362/
12. Please dont add any new DT properties, there is no need for them. See
how this was handled in existing driver and reuse it
13. Slave DMA was never supported and is not needed.
14. Octal?
15. NACK, should be GPLv2 and no need for boilerplate text below when SPDX
Identifier is present.
Ramuthevar Vadivel Murugan (2):
dt-bindings: spi: Add schema for Cadence QSPI Controller driver
spi: cadence-quadpsi: Add support for the Cadence QSPI controller
.../devicetree/bindings/spi/cadence,qspi.yaml | 162 +++
drivers/spi/Kconfig | 8 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-cadence-quadspi.c | 1228 ++++++++++++++++++++
drivers/spi/spi-cadence-quadspi.h | 254 ++++
5 files changed, 1653 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/cadence,qspi.yaml
create mode 100644 drivers/spi/spi-cadence-quadspi.c
create mode 100644 drivers/spi/spi-cadence-quadspi.h
--
2.11.0