2024-05-09 01:06:02

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH v4 0/5] Marvell HW overlay support for Cadence xSPI

This patch series is adding support for second version of Marvell HW
overlay for Cadence xSPI IP block.
Overlay is HW change made around orginal xSPI block.
Overlay extends xSPI features, with clock configuration, interrupt
masking and full-duplex, variable length SPI operations.
All that functionalites allows xSPI block to operate not only with
memory devices, but also with simple SPI devices, or TPM devices.

Changes:
v4:
Rename new Marvell registers to keep naming conventions
Rename mrvl,xspi-nor to marvell,cnxx,xspi-nor
Various fixed for cdns,xspi.yaml file:
- Remove unnecesary parameters
- Link register xferbase with marvell,cn10-xspi-nor
- Move default values to .c file from device-tree
Clock configuration optimization
ACPI fixes:
- Remove incorrect ACPI match table
Added .data field to device_id, fixes for matching in ACPI and dtb case
Minor style comment changes

v3:
Removed all kconfig changes
Added device-tree mrvl,xspi-nor tag

v2:
Support for second overlay iteration

v1:
-

v0:
Initial support for v1 overlay

Piyush Malgujar (1):
spi: cadence: Allow to read basic xSPI configuration from ACPI

Witold Sadowski (4):
spi: cadence: Ensure data lines set to low during dummy-cycle period
dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for
Cadence XSPI
spi: cadence: Add Marvell xSPI IP overlay changes
spi: cadence: Add MRVL overlay xfer operation support

.../devicetree/bindings/spi/cdns,xspi.yaml | 78 ++-
drivers/spi/spi-cadence-xspi.c | 623 +++++++++++++++++-
2 files changed, 668 insertions(+), 33 deletions(-)

--
2.43.0



2024-05-09 01:06:18

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH v4 1/5] spi: cadence: Ensure data lines set to low during dummy-cycle period

During dummy-cycles xSPI will switch GPIO into Hi-Z mode. In that dummy
period voltage on data lines will slowly drop, what can cause
unintentional modebyte transmission. Value send to SPI memory chip will
depend on last address, and clock frequency.
To prevent unforeseen consequences of that behaviour, force send
single modebyte(0x00).
Modebyte will be send only if number of dummy-cycles is not equal
to 0. Code must also reduce dummycycle byte count by one - as one byte
is send as modebyte.

Signed-off-by: Witold Sadowski <[email protected]>
---
drivers/spi/spi-cadence-xspi.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 8648b8eb080d..cdce2e280f66 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -145,6 +145,9 @@
#define CDNS_XSPI_STIG_DONE_FLAG BIT(0)
#define CDNS_XSPI_TRD_STATUS 0x0104

+#define MODE_NO_OF_BYTES GENMASK(25, 24)
+#define MODEBYTES_COUNT 1
+
/* Helper macros for filling command registers */
#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase) ( \
FIELD_PREP(CDNS_XSPI_CMD_INSTR_TYPE, (data_phase) ? \
@@ -157,9 +160,10 @@
FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR3, ((op)->addr.val >> 24) & 0xFF) | \
FIELD_PREP(CDNS_XSPI_CMD_P1_R2_ADDR4, ((op)->addr.val >> 32) & 0xFF))

-#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op) ( \
+#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op, modebytes) ( \
FIELD_PREP(CDNS_XSPI_CMD_P1_R3_ADDR5, ((op)->addr.val >> 40) & 0xFF) | \
FIELD_PREP(CDNS_XSPI_CMD_P1_R3_CMD, (op)->cmd.opcode) | \
+ FIELD_PREP(MODE_NO_OF_BYTES, modebytes) | \
FIELD_PREP(CDNS_XSPI_CMD_P1_R3_NUM_ADDR_BYTES, (op)->addr.nbytes))

#define CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_4(op, chipsel) ( \
@@ -173,12 +177,12 @@
#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_2(op) \
FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R2_DCNT_L, (op)->data.nbytes & 0xFFFF)

-#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op) ( \
+#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op, dummybytes) ( \
FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R3_DCNT_H, \
((op)->data.nbytes >> 16) & 0xffff) | \
FIELD_PREP(CDNS_XSPI_CMD_DSEQ_R3_NUM_OF_DUMMY, \
(op)->dummy.buswidth != 0 ? \
- (((op)->dummy.nbytes * 8) / (op)->dummy.buswidth) : \
+ (((dummybytes) * 8) / (op)->dummy.buswidth) : \
0))

#define CDNS_XSPI_CMD_FLD_DSEQ_CMD_4(op, chipsel) ( \
@@ -351,6 +355,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
u32 cmd_regs[6];
u32 cmd_status;
int ret;
+ int dummybytes = op->dummy.nbytes;

ret = cdns_xspi_wait_for_controller_idle(cdns_xspi);
if (ret < 0)
@@ -365,7 +370,12 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
memset(cmd_regs, 0, sizeof(cmd_regs));
cmd_regs[1] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_1(op, data_phase);
cmd_regs[2] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_2(op);
- cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op);
+ if (dummybytes != 0) {
+ cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op, 1);
+ dummybytes--;
+ } else {
+ cmd_regs[3] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_3(op, 0);
+ }
cmd_regs[4] = CDNS_XSPI_CMD_FLD_P1_INSTR_CMD_4(op,
cdns_xspi->cur_cs);

@@ -375,7 +385,7 @@ static int cdns_xspi_send_stig_command(struct cdns_xspi_dev *cdns_xspi,
cmd_regs[0] = CDNS_XSPI_STIG_DONE_FLAG;
cmd_regs[1] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_1(op);
cmd_regs[2] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_2(op);
- cmd_regs[3] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op);
+ cmd_regs[3] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_3(op, dummybytes);
cmd_regs[4] = CDNS_XSPI_CMD_FLD_DSEQ_CMD_4(op,
cdns_xspi->cur_cs);

--
2.43.0


2024-05-09 01:06:27

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH v4 2/5] dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for Cadence XSPI

Add new bindings for v2 Marvell xSPI overlay:
mrvl,xspi-nor compatible string
New compatible string to distinguish between orginal and modified xSPI
block

PHY configuration registers
Allow to change orginal xSPI PHY configuration values. If not set, and
Marvell overlay is enabled, safe defaults will be written into xSPI PHY

Optional base for xfer register set
Additional reg field to allocate xSPI Marvell overlay XFER block

Signed-off-by: Witold Sadowski <[email protected]>
---
.../devicetree/bindings/spi/cdns,xspi.yaml | 78 +++++++++++++++----
1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
index eb0f92468185..094f8b7ffc49 100644
--- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
+++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
@@ -17,22 +17,43 @@ description: |

allOf:
- $ref: spi-controller.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: marvell,cn10-xspi-nor
+ then:
+ properties:
+ reg-names:
+ items:
+ - const: io
+ - const: sdma
+ - const: aux
+ - const: xferbase
+ reg:
+ items:
+ - description: address and length of the controller register set
+ - description: address and length of the Slave DMA data port
+ - description: address and length of the auxiliary registers
+ - description: address and length of the xfer registers
+ else:
+ properties:
+ reg-names:
+ items:
+ - const: io
+ - const: sdma
+ - const: aux
+ reg:
+ items:
+ - description: address and length of the controller register set
+ - description: address and length of the Slave DMA data port
+ - description: address and length of the auxiliary registers

properties:
compatible:
- const: cdns,xspi-nor
-
- reg:
- items:
- - description: address and length of the controller register set
- - description: address and length of the Slave DMA data port
- - description: address and length of the auxiliary registers
-
- reg-names:
- items:
- - const: io
- - const: sdma
- - const: aux
+ enum:
+ - cdns,xspi-nor
+ - marvell,cn10-xspi-nor

interrupts:
maxItems: 1
@@ -68,6 +89,37 @@ examples:
reg = <0>;
};

+ flash@1 {
+ compatible = "jedec,spi-nor";
+ spi-max-frequency = <75000000>;
+ reg = <1>;
+ };
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ bus {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ spi@d0010000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "marvell,cn10-xspi-nor";
+ reg = <0x0 0xa0010000 0x0 0x1040>,
+ <0x0 0xb0000000 0x0 0x1000>,
+ <0x0 0xa0020000 0x0 0x100>,
+ <0x0 0xa0090000 0x0 0x100>;
+ reg-names = "io", "sdma", "aux", "xferbase";
+ interrupts = <0 90 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gic>;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ spi-max-frequency = <75000000>;
+ reg = <0>;
+ };
+
flash@1 {
compatible = "jedec,spi-nor";
spi-max-frequency = <75000000>;
--
2.43.0


2024-05-09 01:06:44

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH v4 4/5] spi: cadence: Allow to read basic xSPI configuration from ACPI

From: Piyush Malgujar <[email protected]>

These changes enables to read the configs from ACPI tables as required
for successful probing in ACPI uefi environment.
In case of ACPI disabled/dts based environment, it will continue to
read configs from dts as before

Signed-off-by: Piyush Malgujar <[email protected]>
Signed-off-by: Witold Sadowski <[email protected]>
---
drivers/spi/spi-cadence-xspi.c | 84 +++++++++++++++++++++++++++++++---
1 file changed, 77 insertions(+), 7 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index 4bfcfa2f8835..f6bae59b34e5 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -2,6 +2,7 @@
// Cadence XSPI flash controller driver
// Copyright (C) 2020-21 Cadence

+#include <linux/acpi.h>
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/err.h>
@@ -14,6 +15,7 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
+#include <linux/property.h>
#include <linux/spi/spi.h>
#include <linux/spi/spi-mem.h>
#include <linux/bitfield.h>
@@ -647,6 +649,67 @@ static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
(dir != SPI_MEM_NO_DATA));
}

+#ifdef CONFIG_ACPI
+static bool cdns_xspi_supports_op(struct spi_mem *mem,
+ const struct spi_mem_op *op)
+{
+ struct spi_device *spi = mem->spi;
+ const union acpi_object *obj;
+ struct acpi_device *adev;
+
+ adev = ACPI_COMPANION(&spi->dev);
+
+ if (!acpi_dev_get_property(adev, "spi-tx-bus-width", ACPI_TYPE_INTEGER,
+ &obj)) {
+ switch (obj->integer.value) {
+ case 1:
+ break;
+ case 2:
+ spi->mode |= SPI_TX_DUAL;
+ break;
+ case 4:
+ spi->mode |= SPI_TX_QUAD;
+ break;
+ case 8:
+ spi->mode |= SPI_TX_OCTAL;
+ break;
+ default:
+ dev_warn(&spi->dev,
+ "spi-tx-bus-width %lld not supported\n",
+ obj->integer.value);
+ break;
+ }
+ }
+
+ if (!acpi_dev_get_property(adev, "spi-rx-bus-width", ACPI_TYPE_INTEGER,
+ &obj)) {
+ switch (obj->integer.value) {
+ case 1:
+ break;
+ case 2:
+ spi->mode |= SPI_RX_DUAL;
+ break;
+ case 4:
+ spi->mode |= SPI_RX_QUAD;
+ break;
+ case 8:
+ spi->mode |= SPI_RX_OCTAL;
+ break;
+ default:
+ dev_warn(&spi->dev,
+ "spi-rx-bus-width %lld not supported\n",
+ obj->integer.value);
+ break;
+ }
+ }
+
+ if (!spi_mem_default_supports_op(mem, op))
+ return false;
+
+ return true;
+}
+#endif
+
static int cdns_xspi_mem_op_execute(struct spi_mem *mem,
const struct spi_mem_op *op)
{
@@ -670,6 +733,9 @@ static int cdns_xspi_adjust_mem_op_size(struct spi_mem *mem, struct spi_mem_op *
}

static const struct spi_controller_mem_ops cadence_xspi_mem_ops = {
+#ifdef CONFIG_ACPI
+ .supports_op = cdns_xspi_supports_op,
+#endif
.exec_op = cdns_xspi_mem_op_execute,
.adjust_op_size = cdns_xspi_adjust_mem_op_size,
};
@@ -721,21 +787,20 @@ static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)

static int cdns_xspi_of_get_plat_data(struct platform_device *pdev)
{
- struct device_node *node_prop = pdev->dev.of_node;
- struct device_node *node_child;
+ struct fwnode_handle *fwnode_child;
unsigned int cs;

- for_each_child_of_node(node_prop, node_child) {
- if (!of_device_is_available(node_child))
+ device_for_each_child_node(&pdev->dev, fwnode_child) {
+ if (!fwnode_device_is_available(fwnode_child))
continue;

- if (of_property_read_u32(node_child, "reg", &cs)) {
+ if (fwnode_property_read_u32(fwnode_child, "reg", &cs)) {
dev_err(&pdev->dev, "Couldn't get memory chip select\n");
- of_node_put(node_child);
+ fwnode_handle_put(fwnode_child);
return -ENXIO;
} else if (cs >= CDNS_XSPI_MAX_BANKS) {
dev_err(&pdev->dev, "reg (cs) parameter value too large\n");
- of_node_put(node_child);
+ fwnode_handle_put(fwnode_child);
return -ENXIO;
}
}
@@ -789,6 +854,11 @@ static int cdns_xspi_probe(struct platform_device *pdev)
SPI_MODE_0 | SPI_MODE_3;

drv_data = of_device_get_match_data(dev);
+ if (!drv_data) {
+ drv_data = acpi_device_get_match_data(dev);
+ if (!drv_data)
+ return -ENODEV;
+ }

host->mem_ops = &cadence_xspi_mem_ops;
host->dev.of_node = pdev->dev.of_node;
--
2.43.0


2024-05-09 01:07:03

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH v4 3/5] spi: cadence: Add Marvell xSPI IP overlay changes

Add support for basic v2 Marvell overlay block. Support for basic
operation is added here: clock configuration, PHY configuration,
interrupt configuration(enabling)
Clock divider block is build on top of Cadence xSPI IP, and divides
external 800MHz clock. It allows only for a few different clock speeds
starting from 6.25MHz up to 200MHz.
PHY configuration can be read from device-tree, if parameter is not
present - safe defaults will be used..
In addition to handle interrupt propoerly driver must clear MSI-X
interrupt bit, in addition to clearing xSPI interrupt bit. Interrupt
masking must be disabled.

Signed-off-by: Witold Sadowski <[email protected]>
---
drivers/spi/spi-cadence-xspi.c | 273 ++++++++++++++++++++++++++++++++-
1 file changed, 265 insertions(+), 8 deletions(-)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index cdce2e280f66..4bfcfa2f8835 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -193,6 +193,43 @@
((op)->data.dir == SPI_MEM_DATA_IN) ? \
CDNS_XSPI_STIG_CMD_DIR_READ : CDNS_XSPI_STIG_CMD_DIR_WRITE))

+/* PHY default values */
+#define REGS_DLL_PHY_CTRL 0x00000707
+#define CTB_RFILE_PHY_CTRL 0x00004000
+#define RFILE_PHY_TSEL 0x00000000
+#define RFILE_PHY_DQ_TIMING 0x00000101
+#define RFILE_PHY_DQS_TIMING 0x00700404
+#define RFILE_PHY_GATE_LPBK_CTRL 0x00200030
+#define RFILE_PHY_DLL_MASTER_CTRL 0x00800000
+#define RFILE_PHY_DLL_SLAVE_CTRL 0x0000ff01
+
+/* PHY config registers */
+#define CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL 0x1034
+#define CDNS_XSPI_PHY_CTB_RFILE_PHY_CTRL 0x0080
+#define CDNS_XSPI_PHY_CTB_RFILE_PHY_TSEL 0x0084
+#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQ_TIMING 0x0000
+#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQS_TIMING 0x0004
+#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_GATE_LPBK_CTRL 0x0008
+#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_MASTER_CTRL 0x000c
+#define CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_SLAVE_CTRL 0x0010
+#define CDNS_XSPI_DATASLICE_RFILE_PHY_DLL_OBS_REG_0 0x001c
+
+#define CDNS_XSPI_DLL_RST_N BIT(24)
+#define CDNS_XSPI_DLL_LOCK BIT(0)
+
+/* Marvell overlay registers - clock */
+#define MRVL_XSPI_CLK_CTRL_AUX_REG 0x2020
+#define MRVL_XSPI_CLK_ENABLE BIT(0)
+#define MRVL_XSPI_CLK_DIV GENMASK(4, 1)
+#define MRVL_XSPI_IRQ_ENABLE BIT(6)
+#define MRVL_XSPI_CLOCK_IO_HZ 800000000
+#define MRVL_XSPI_CLOCK_DIVIDED(div) ((MRVL_XSPI_CLOCK_IO_HZ) / (div))
+#define MRVL_DEFAULT_CLK 25000000
+
+/* Marvell overlay registers - MSI-X */
+#define MRVL_XSPI_SPIX_INTR_AUX 0x2000
+#define MRVL_MSIX_CLEAR_IRQ 0x01
+
enum cdns_xspi_stig_instr_type {
CDNS_XSPI_STIG_INSTR_TYPE_0,
CDNS_XSPI_STIG_INSTR_TYPE_1,
@@ -212,6 +249,7 @@ enum cdns_xspi_stig_cmd_dir {
struct cdns_xspi_dev {
struct platform_device *pdev;
struct device *dev;
+ bool mrvl_hw_overlay;

void __iomem *iobase;
void __iomem *auxbase;
@@ -232,6 +270,118 @@ struct cdns_xspi_dev {
u8 hw_num_banks;
};

+struct cdns_xspi_driver_data {
+ bool mrvl_hw_overlay;
+};
+
+static struct cdns_xspi_driver_data mrvl_driver_data = {
+ .mrvl_hw_overlay = true,
+};
+
+static struct cdns_xspi_driver_data cdns_driver_data = {
+ .mrvl_hw_overlay = false,
+};
+
+const int cdns_mrvl_xspi_clk_div_list[] = {
+ 4, //0x0 = Divide by 4. SPI clock is 200 MHz.
+ 6, //0x1 = Divide by 6. SPI clock is 133.33 MHz.
+ 8, //0x2 = Divide by 8. SPI clock is 100 MHz.
+ 10, //0x3 = Divide by 10. SPI clock is 80 MHz.
+ 12, //0x4 = Divide by 12. SPI clock is 66.666 MHz.
+ 16, //0x5 = Divide by 16. SPI clock is 50 MHz.
+ 18, //0x6 = Divide by 18. SPI clock is 44.44 MHz.
+ 20, //0x7 = Divide by 20. SPI clock is 40 MHz.
+ 24, //0x8 = Divide by 24. SPI clock is 33.33 MHz.
+ 32, //0x9 = Divide by 32. SPI clock is 25 MHz.
+ 40, //0xA = Divide by 40. SPI clock is 20 MHz.
+ 50, //0xB = Divide by 50. SPI clock is 16 MHz.
+ 64, //0xC = Divide by 64. SPI clock is 12.5 MHz.
+ 128 //0xD = Divide by 128. SPI clock is 6.25 MHz.
+};
+
+static bool cdns_xspi_reset_dll(struct cdns_xspi_dev *cdns_xspi)
+{
+ u32 dll_cntrl = readl(cdns_xspi->iobase +
+ CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL);
+ u32 dll_lock;
+
+ /* Reset DLL */
+ dll_cntrl |= CDNS_XSPI_DLL_RST_N;
+ writel(dll_cntrl, cdns_xspi->iobase +
+ CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL);
+
+ /* Wait for DLL lock */
+ return readl_relaxed_poll_timeout(cdns_xspi->iobase +
+ CDNS_XSPI_INTR_STATUS_REG,
+ dll_lock, ((dll_lock & CDNS_XSPI_DLL_LOCK) == 1), 10, 10000);
+}
+
+/* Static configuration of PHY */
+static bool cdns_xspi_configure_phy(struct cdns_xspi_dev *cdns_xspi)
+{
+ writel(REGS_DLL_PHY_CTRL,
+ cdns_xspi->iobase + CDNS_XSPI_RF_MINICTRL_REGS_DLL_PHY_CTRL);
+ writel(CTB_RFILE_PHY_CTRL,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_CTB_RFILE_PHY_CTRL);
+ writel(RFILE_PHY_TSEL,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_CTB_RFILE_PHY_TSEL);
+ writel(RFILE_PHY_DQ_TIMING,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQ_TIMING);
+ writel(RFILE_PHY_DQS_TIMING,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DQS_TIMING);
+ writel(RFILE_PHY_GATE_LPBK_CTRL,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_GATE_LPBK_CTRL);
+ writel(RFILE_PHY_DLL_MASTER_CTRL,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_MASTER_CTRL);
+ writel(RFILE_PHY_DLL_SLAVE_CTRL,
+ cdns_xspi->auxbase + CDNS_XSPI_PHY_DATASLICE_RFILE_PHY_DLL_SLAVE_CTRL);
+
+ return cdns_xspi_reset_dll(cdns_xspi);
+}
+
+/* Find max avalible clock */
+static bool cdns_mrvl_xspi_setup_clock(struct cdns_xspi_dev *cdns_xspi,
+ int requested_clk)
+{
+ int i = 0;
+ int clk_val;
+ u32 clk_reg;
+ bool update_clk = false;
+
+ while (i < ARRAY_SIZE(cdns_mrvl_xspi_clk_div_list)) {
+ clk_val = MRVL_XSPI_CLOCK_DIVIDED(
+ cdns_mrvl_xspi_clk_div_list[i]);
+ if (clk_val <= requested_clk)
+ break;
+ i++;
+ }
+
+ dev_dbg(cdns_xspi->dev, "Found clk div: %d, clk val: %d\n",
+ cdns_mrvl_xspi_clk_div_list[i],
+ MRVL_XSPI_CLOCK_DIVIDED(
+ cdns_mrvl_xspi_clk_div_list[i]));
+
+ clk_reg = readl(cdns_xspi->auxbase + MRVL_XSPI_CLK_CTRL_AUX_REG);
+
+ if (FIELD_GET(MRVL_XSPI_CLK_DIV, clk_reg) != i) {
+ clk_reg &= ~MRVL_XSPI_CLK_ENABLE;
+ writel(clk_reg,
+ cdns_xspi->auxbase + MRVL_XSPI_CLK_CTRL_AUX_REG);
+ clk_reg = FIELD_PREP(MRVL_XSPI_CLK_DIV, i);
+ clk_reg &= ~MRVL_XSPI_CLK_DIV;
+ clk_reg |= FIELD_PREP(MRVL_XSPI_CLK_DIV, i);
+ clk_reg |= MRVL_XSPI_CLK_ENABLE;
+ clk_reg |= MRVL_XSPI_IRQ_ENABLE;
+ update_clk = true;
+ }
+
+ if (update_clk)
+ writel(clk_reg,
+ cdns_xspi->auxbase + MRVL_XSPI_CLK_CTRL_AUX_REG);
+
+ return update_clk;
+}
+
static int cdns_xspi_wait_for_controller_idle(struct cdns_xspi_dev *cdns_xspi)
{
u32 ctrl_stat;
@@ -295,6 +445,10 @@ static void cdns_xspi_set_interrupts(struct cdns_xspi_dev *cdns_xspi,
bool enabled)
{
u32 intr_enable;
+ u32 irq_status;
+
+ irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
+ writel(irq_status, cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);

intr_enable = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_ENABLE_REG);
if (enabled)
@@ -319,6 +473,9 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
return -EIO;
}

+ writel(FIELD_PREP(CDNS_XSPI_CTRL_WORK_MODE, CDNS_XSPI_WORK_MODE_STIG),
+ cdns_xspi->iobase + CDNS_XSPI_CTRL_CONFIG_REG);
+
ctrl_features = readl(cdns_xspi->iobase + CDNS_XSPI_CTRL_FEATURES_REG);
cdns_xspi->hw_num_banks = FIELD_GET(CDNS_XSPI_NUM_BANKS, ctrl_features);
cdns_xspi_set_interrupts(cdns_xspi, false);
@@ -326,6 +483,70 @@ static int cdns_xspi_controller_init(struct cdns_xspi_dev *cdns_xspi)
return 0;
}

+static void mrvl_ioreadq(void __iomem *addr, void *buf, int len)
+{
+ int i = 0;
+ int rcount = len / 8;
+ int rcount_nf = len % 8;
+ uint64_t tmp;
+ uint64_t *buf64 = (uint64_t *)buf;
+
+ if (((uint64_t)buf % 8) == 0) {
+ for (i = 0; i < rcount; i++)
+ *buf64++ = readq(addr);
+ } else {
+ for (i = 0; i < rcount; i++) {
+ tmp = readq(addr);
+ memcpy(buf+(i*8), &tmp, 8);
+ }
+ }
+
+ if (rcount_nf != 0) {
+ tmp = readq(addr);
+ memcpy(buf+(i*8), &tmp, rcount_nf);
+ }
+}
+
+static void mrvl_iowriteq(void __iomem *addr, const void *buf, int len)
+{
+ int i = 0;
+ int rcount = len / 8;
+ int rcount_nf = len % 8;
+ uint64_t tmp;
+ uint64_t *buf64 = (uint64_t *)buf;
+
+ if (((uint64_t)buf % 8) == 0) {
+ for (i = 0; i < rcount; i++)
+ writeq(*buf64++, addr);
+ } else {
+ for (i = 0; i < rcount; i++) {
+ memcpy(&tmp, buf+(i*8), 8);
+ writeq(tmp, addr);
+ }
+ }
+
+ if (rcount_nf != 0) {
+ memcpy(&tmp, buf+(i*8), rcount_nf);
+ writeq(tmp, addr);
+ }
+}
+
+static void cdns_xspi_sdma_memread(struct cdns_xspi_dev *cdns_xspi, int len)
+{
+ if (cdns_xspi->mrvl_hw_overlay)
+ mrvl_ioreadq(cdns_xspi->sdmabase, cdns_xspi->in_buffer, len);
+ else
+ ioread8_rep(cdns_xspi->sdmabase, cdns_xspi->in_buffer, len);
+}
+
+static void cdns_xspi_sdma_memwrite(struct cdns_xspi_dev *cdns_xspi, int len)
+{
+ if (cdns_xspi->mrvl_hw_overlay)
+ mrvl_iowriteq(cdns_xspi->sdmabase, cdns_xspi->out_buffer, len);
+ else
+ iowrite8_rep(cdns_xspi->sdmabase, cdns_xspi->out_buffer, len);
+}
+
static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)
{
u32 sdma_size, sdma_trd_info;
@@ -337,13 +558,11 @@ static void cdns_xspi_sdma_handle(struct cdns_xspi_dev *cdns_xspi)

switch (sdma_dir) {
case CDNS_XSPI_SDMA_DIR_READ:
- ioread8_rep(cdns_xspi->sdmabase,
- cdns_xspi->in_buffer, sdma_size);
+ cdns_xspi_sdma_memread(cdns_xspi, sdma_size);
break;

case CDNS_XSPI_SDMA_DIR_WRITE:
- iowrite8_rep(cdns_xspi->sdmabase,
- cdns_xspi->out_buffer, sdma_size);
+ cdns_xspi_sdma_memwrite(cdns_xspi, sdma_size);
break;
}
}
@@ -421,6 +640,9 @@ static int cdns_xspi_mem_op(struct cdns_xspi_dev *cdns_xspi,
if (cdns_xspi->cur_cs != spi_get_chipselect(mem->spi, 0))
cdns_xspi->cur_cs = spi_get_chipselect(mem->spi, 0);

+ if (cdns_xspi->mrvl_hw_overlay)
+ cdns_mrvl_xspi_setup_clock(cdns_xspi, mem->spi->max_speed_hz);
+
return cdns_xspi_send_stig_command(cdns_xspi, op,
(dir != SPI_MEM_NO_DATA));
}
@@ -461,6 +683,10 @@ static irqreturn_t cdns_xspi_irq_handler(int this_irq, void *dev)
irq_status = readl(cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);
writel(irq_status, cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG);

+ if (cdns_xspi->mrvl_hw_overlay)
+ writel(MRVL_MSIX_CLEAR_IRQ,
+ cdns_xspi->auxbase + MRVL_XSPI_SPIX_INTR_AUX);
+
if (irq_status &
(CDNS_XSPI_SDMA_ERROR | CDNS_XSPI_SDMA_TRIGGER |
CDNS_XSPI_STIG_DONE)) {
@@ -534,11 +760,23 @@ static void cdns_xspi_print_phy_config(struct cdns_xspi_dev *cdns_xspi)
readl(cdns_xspi->auxbase + CDNS_XSPI_CCP_PHY_DLL_SLAVE_CTRL));
}

+static int cdns_xspi_setup(struct spi_device *spi_dev)
+{
+ struct cdns_xspi_dev *cdns_xspi = spi_controller_get_devdata(
+ spi_dev->controller);
+
+ if (cdns_xspi->mrvl_hw_overlay)
+ cdns_mrvl_xspi_setup_clock(cdns_xspi, spi_dev->max_speed_hz);
+
+ return 0;
+}
+
static int cdns_xspi_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
struct spi_controller *host = NULL;
struct cdns_xspi_dev *cdns_xspi = NULL;
+ const struct cdns_xspi_driver_data *drv_data;
struct resource *res;
int ret;

@@ -550,10 +788,16 @@ static int cdns_xspi_probe(struct platform_device *pdev)
SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_OCTAL | SPI_RX_OCTAL |
SPI_MODE_0 | SPI_MODE_3;

+ drv_data = of_device_get_match_data(dev);
+
host->mem_ops = &cadence_xspi_mem_ops;
host->dev.of_node = pdev->dev.of_node;
+ host->dev.fwnode = pdev->dev.fwnode;
host->bus_num = -1;

+ if (drv_data->mrvl_hw_overlay)
+ host->setup = cdns_xspi_setup;
+
platform_set_drvdata(pdev, host);

cdns_xspi = spi_controller_get_devdata(host);
@@ -565,23 +809,27 @@ static int cdns_xspi_probe(struct platform_device *pdev)
init_completion(&cdns_xspi->auto_cmd_complete);
init_completion(&cdns_xspi->sdma_complete);

+ cdns_xspi->mrvl_hw_overlay = drv_data->mrvl_hw_overlay;
+
ret = cdns_xspi_of_get_plat_data(pdev);
if (ret)
return -ENODEV;

- cdns_xspi->iobase = devm_platform_ioremap_resource_byname(pdev, "io");
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ cdns_xspi->iobase = devm_ioremap_resource(dev, res);
if (IS_ERR(cdns_xspi->iobase)) {
dev_err(dev, "Failed to remap controller base address\n");
return PTR_ERR(cdns_xspi->iobase);
}

- res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sdma");
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
cdns_xspi->sdmabase = devm_ioremap_resource(dev, res);
if (IS_ERR(cdns_xspi->sdmabase))
return PTR_ERR(cdns_xspi->sdmabase);
cdns_xspi->sdmasize = resource_size(res);

- cdns_xspi->auxbase = devm_platform_ioremap_resource_byname(pdev, "aux");
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+ cdns_xspi->auxbase = devm_ioremap_resource(dev, res);
if (IS_ERR(cdns_xspi->auxbase)) {
dev_err(dev, "Failed to remap AUX address\n");
return PTR_ERR(cdns_xspi->auxbase);
@@ -598,8 +846,12 @@ static int cdns_xspi_probe(struct platform_device *pdev)
return ret;
}

- cdns_xspi_print_phy_config(cdns_xspi);
+ if (drv_data->mrvl_hw_overlay) {
+ cdns_mrvl_xspi_setup_clock(cdns_xspi, MRVL_DEFAULT_CLK);
+ cdns_xspi_configure_phy(cdns_xspi);
+ }

+ cdns_xspi_print_phy_config(cdns_xspi);
ret = cdns_xspi_controller_init(cdns_xspi);
if (ret) {
dev_err(dev, "Failed to initialize controller\n");
@@ -622,6 +874,11 @@ static int cdns_xspi_probe(struct platform_device *pdev)
static const struct of_device_id cdns_xspi_of_match[] = {
{
.compatible = "cdns,xspi-nor",
+ .data = &cdns_driver_data,
+ },
+ {
+ .compatible = "marvell,cn10-xspi-nor",
+ .data = &mrvl_driver_data,
},
{ /* end of table */}
};
--
2.43.0


2024-05-09 01:08:02

by Witold Sadowski

[permalink] [raw]
Subject: [PATCH v4 5/5] spi: cadence: Add MRVL overlay xfer operation support

MRVL Xfer overlay extend xSPI capabilities, to support non-memory SPI
operations. Marvell overlay combined with generic command allows to
create full-duplex SPI transactions. It also allows to create
transaction with undetermined transaction length - with cs_hold
parameter, and ability to extend CS signal assertion, even if xSPI block
requests CS signal de-assertion.

Signed-off-by: Witold Sadowski <[email protected]>
---
drivers/spi/spi-cadence-xspi.c | 246 +++++++++++++++++++++++++++++++++
1 file changed, 246 insertions(+)

diff --git a/drivers/spi/spi-cadence-xspi.c b/drivers/spi/spi-cadence-xspi.c
index f6bae59b34e5..001b97d07406 100644
--- a/drivers/spi/spi-cadence-xspi.c
+++ b/drivers/spi/spi-cadence-xspi.c
@@ -219,6 +219,7 @@
#define CDNS_XSPI_DLL_RST_N BIT(24)
#define CDNS_XSPI_DLL_LOCK BIT(0)

+
/* Marvell overlay registers - clock */
#define MRVL_XSPI_CLK_CTRL_AUX_REG 0x2020
#define MRVL_XSPI_CLK_ENABLE BIT(0)
@@ -232,6 +233,22 @@
#define MRVL_XSPI_SPIX_INTR_AUX 0x2000
#define MRVL_MSIX_CLEAR_IRQ 0x01

+/* Marvell overlay registers - xfer */
+#define MRVL_XFER_FUNC_CTRL 0x210
+#define MRVL_XFER_FUNC_CTRL_READ_DATA(i) (0x000 + 8 * (i))
+#define MRVL_XFER_SOFT_RESET BIT(11)
+#define MRVL_XFER_CS_N_HOLD GENMASK(9, 6)
+#define MRVL_XFER_RECEIVE_ENABLE BIT(4)
+#define MRVL_XFER_FUNC_ENABLE BIT(3)
+#define MRVL_XFER_CLK_CAPTURE_POL BIT(2)
+#define MRVL_XFER_CLK_DRIVE_POL BIT(1)
+#define MRVL_XFER_FUNC_START BIT(0)
+#define MRVL_XFER_QWORD_COUNT 32
+#define MRVL_XFER_QWORD_BYTECOUNT 8
+
+#define MRVL_XSPI_POLL_TIMEOUT_US 1000
+#define MRVL_XSPI_POLL_DELAY_US 10
+
enum cdns_xspi_stig_instr_type {
CDNS_XSPI_STIG_INSTR_TYPE_0,
CDNS_XSPI_STIG_INSTR_TYPE_1,
@@ -256,6 +273,7 @@ struct cdns_xspi_dev {
void __iomem *iobase;
void __iomem *auxbase;
void __iomem *sdmabase;
+ void __iomem *xferbase;

int irq;
int cur_cs;
@@ -270,6 +288,9 @@ struct cdns_xspi_dev {
const void *out_buffer;

u8 hw_num_banks;
+
+ bool xfer_in_progress;
+ int current_xfer_qword;
};

struct cdns_xspi_driver_data {
@@ -836,6 +857,220 @@ static int cdns_xspi_setup(struct spi_device *spi_dev)
return 0;
}

+static int cdns_xspi_prepare_generic(int cs, const void *dout, int len, int glue, u32 *cmd_regs)
+{
+ u8 *data = (u8 *)dout;
+ int i;
+ int data_counter = 0;
+
+ memset(cmd_regs, 0x00, 6*4);
+
+ if (len > 7) {
+ for (i = (len >= 10 ? 2 : len - 8); i >= 0 ; i--)
+ cmd_regs[3] |= data[data_counter++] << (8*i);
+ }
+ if (len > 3) {
+ for (i = (len >= 7 ? 3 : len - 4); i >= 0; i--)
+ cmd_regs[2] |= data[data_counter++] << (8*i);
+ }
+ for (i = (len >= 3 ? 2 : len - 1); i >= 0 ; i--)
+ cmd_regs[1] |= data[data_counter++] << (8 + 8*i);
+
+ cmd_regs[1] |= 96;
+ cmd_regs[3] |= len << 24;
+ cmd_regs[4] |= cs << 12;
+
+ if (glue == 1)
+ cmd_regs[4] |= 1 << 28;
+
+ return 0;
+}
+
+static unsigned char reverse_bits(unsigned char num)
+{
+ unsigned int count = sizeof(num) * 8 - 1;
+ unsigned int reverse_num = num;
+
+ num >>= 1;
+ while (num) {
+ reverse_num <<= 1;
+ reverse_num |= num & 1;
+ num >>= 1;
+ count--;
+ }
+ reverse_num <<= count;
+ return reverse_num;
+}
+
+static void cdns_xspi_read_single_qword(struct cdns_xspi_dev *cdns_xspi, u8 **buffer)
+{
+ u64 d = readq(cdns_xspi->xferbase +
+ MRVL_XFER_FUNC_CTRL_READ_DATA(cdns_xspi->current_xfer_qword));
+ u8 *ptr = (u8 *)&d;
+ int k;
+
+ for (k = 0; k < 8; k++) {
+ u8 val = reverse_bits((ptr[k]));
+ **buffer = val;
+ *buffer = *buffer + 1;
+ }
+
+ cdns_xspi->current_xfer_qword++;
+ cdns_xspi->current_xfer_qword %= MRVL_XFER_QWORD_COUNT;
+}
+
+static void cdns_xspi_finish_read(struct cdns_xspi_dev *cdns_xspi, u8 **buffer, u32 data_count)
+{
+ u64 d = readq(cdns_xspi->xferbase +
+ MRVL_XFER_FUNC_CTRL_READ_DATA(cdns_xspi->current_xfer_qword));
+ u8 *ptr = (u8 *)&d;
+ int k;
+
+ for (k = 0; k < data_count % MRVL_XFER_QWORD_BYTECOUNT; k++) {
+ u8 val = reverse_bits((ptr[k]));
+ **buffer = val;
+ *buffer = *buffer + 1;
+ }
+
+ cdns_xspi->current_xfer_qword++;
+ cdns_xspi->current_xfer_qword %= MRVL_XFER_QWORD_COUNT;
+}
+
+static int cdns_xspi_prepare_transfer(int cs, int dir, int len, u32 *cmd_regs)
+{
+ memset(cmd_regs, 0x00, 6*4);
+
+ cmd_regs[1] |= 127;
+ cmd_regs[2] |= len << 16;
+ cmd_regs[4] |= dir << 4; //dir = 0 read, dir =1 write
+ cmd_regs[4] |= cs << 12;
+
+ return 0;
+}
+
+static bool cdns_xspi_stig_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
+{
+ u32 ctrl_stat;
+
+ return readl_relaxed_poll_timeout
+ (cdns_xspi->iobase + CDNS_XSPI_CTRL_STATUS_REG,
+ ctrl_stat,
+ ((ctrl_stat & BIT(3)) == 0),
+ sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
+ sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);
+}
+
+static bool cdns_xspi_sdma_ready(struct cdns_xspi_dev *cdns_xspi, bool sleep)
+{
+ u32 ctrl_stat;
+
+ return readl_relaxed_poll_timeout
+ (cdns_xspi->iobase + CDNS_XSPI_INTR_STATUS_REG,
+ ctrl_stat,
+ (ctrl_stat & CDNS_XSPI_SDMA_TRIGGER),
+ sleep ? MRVL_XSPI_POLL_DELAY_US : 0,
+ sleep ? MRVL_XSPI_POLL_TIMEOUT_US : 0);
+}
+
+static int cdns_xspi_transfer_one_message_b0(struct spi_controller *controller,
+ struct spi_message *m)
+{
+ struct cdns_xspi_dev *cdns_xspi = spi_controller_get_devdata(controller);
+ struct spi_device *spi = m->spi;
+ struct spi_transfer *t = NULL;
+
+ const int max_len = MRVL_XFER_QWORD_BYTECOUNT * MRVL_XFER_QWORD_COUNT;
+ int current_cycle_count;
+ int cs = spi_get_chipselect(spi, 0);
+ int cs_change = 0;
+
+ /* Enable xfer state machine */
+ if (!cdns_xspi->xfer_in_progress) {
+ u32 xfer_control = readl(cdns_xspi->xferbase + MRVL_XFER_FUNC_CTRL);
+
+ cdns_xspi->current_xfer_qword = 0;
+ cdns_xspi->xfer_in_progress = true;
+ xfer_control |= (MRVL_XFER_RECEIVE_ENABLE |
+ MRVL_XFER_CLK_CAPTURE_POL |
+ MRVL_XFER_FUNC_START |
+ MRVL_XFER_SOFT_RESET |
+ FIELD_PREP(MRVL_XFER_CS_N_HOLD, (1 << cs)));
+ xfer_control &= ~(MRVL_XFER_FUNC_ENABLE | MRVL_XFER_CLK_DRIVE_POL);
+ writel(xfer_control, cdns_xspi->xferbase + MRVL_XFER_FUNC_CTRL);
+ }
+
+ list_for_each_entry(t, &m->transfers, transfer_list) {
+ u8 *txd = (u8 *) t->tx_buf;
+ u8 *rxd = (u8 *) t->rx_buf;
+ u8 data[10];
+ u32 cmd_regs[6];
+
+ if (!txd)
+ txd = data;
+
+ cdns_xspi->in_buffer = txd + 1;
+ cdns_xspi->out_buffer = txd + 1;
+
+ while (t->len) {
+
+ current_cycle_count = t->len > max_len ? max_len : t->len;
+
+ if (current_cycle_count < 10) {
+ cdns_xspi_prepare_generic(cs, txd, current_cycle_count,
+ false, cmd_regs);
+ cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
+ if (cdns_xspi_stig_ready(cdns_xspi, true))
+ return -EIO;
+ } else {
+ cdns_xspi_prepare_generic(cs, txd, 1, true, cmd_regs);
+ cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
+ cdns_xspi_prepare_transfer(cs, 1, current_cycle_count - 1,
+ cmd_regs);
+ cdns_xspi_trigger_command(cdns_xspi, cmd_regs);
+ if (cdns_xspi_sdma_ready(cdns_xspi, true))
+ return -EIO;
+ cdns_xspi_sdma_handle(cdns_xspi);
+ if (cdns_xspi_stig_ready(cdns_xspi, true))
+ return -EIO;
+
+ cdns_xspi->in_buffer += current_cycle_count;
+ cdns_xspi->out_buffer += current_cycle_count;
+ }
+
+ if (rxd) {
+ int j;
+
+ for (j = 0; j < current_cycle_count / 8; j++)
+ cdns_xspi_read_single_qword(cdns_xspi, &rxd);
+ cdns_xspi_finish_read(cdns_xspi, &rxd, current_cycle_count);
+ } else {
+ cdns_xspi->current_xfer_qword += current_cycle_count /
+ MRVL_XFER_QWORD_BYTECOUNT;
+ if (current_cycle_count % MRVL_XFER_QWORD_BYTECOUNT)
+ cdns_xspi->current_xfer_qword++;
+
+ cdns_xspi->current_xfer_qword %= MRVL_XFER_QWORD_COUNT;
+ }
+ cs_change = t->cs_change;
+ t->len -= current_cycle_count;
+ }
+ }
+
+ if (!cs_change) {
+ u32 xfer_control = readl(cdns_xspi->xferbase + MRVL_XFER_FUNC_CTRL);
+
+ xfer_control &= ~(MRVL_XFER_RECEIVE_ENABLE |
+ MRVL_XFER_SOFT_RESET);
+ writel(xfer_control, cdns_xspi->xferbase + MRVL_XFER_FUNC_CTRL);
+ cdns_xspi->xfer_in_progress = false;
+ }
+
+ m->status = 0;
+ spi_finalize_current_message(controller);
+
+ return 0;
+}
+
static int cdns_xspi_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -905,6 +1140,16 @@ static int cdns_xspi_probe(struct platform_device *pdev)
return PTR_ERR(cdns_xspi->auxbase);
}

+ if (cdns_xspi->mrvl_hw_overlay) {
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
+ cdns_xspi->xferbase = devm_ioremap_resource(dev, res);
+ if (IS_ERR(cdns_xspi->xferbase)) {
+ dev_info(dev, "XFER register base not found, set it\n");
+ // For compatibility with older firmware
+ cdns_xspi->xferbase = cdns_xspi->iobase + 0x8000;
+ }
+ }
+
cdns_xspi->irq = platform_get_irq(pdev, 0);
if (cdns_xspi->irq < 0)
return -ENXIO;
@@ -919,6 +1164,7 @@ static int cdns_xspi_probe(struct platform_device *pdev)
if (drv_data->mrvl_hw_overlay) {
cdns_mrvl_xspi_setup_clock(cdns_xspi, MRVL_DEFAULT_CLK);
cdns_xspi_configure_phy(cdns_xspi);
+ host->transfer_one_message = cdns_xspi_transfer_one_message_b0;
}

cdns_xspi_print_phy_config(cdns_xspi);
--
2.43.0


2024-05-09 17:22:29

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for Cadence XSPI

Hey Witold,

On Wed, May 08, 2024 at 06:05:20PM -0700, Witold Sadowski wrote:

> allOf:
> - $ref: spi-controller.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: marvell,cn10-xspi-nor
> + then:
> + properties:
> + reg-names:
> + items:
> + - const: io
> + - const: sdma
> + - const: aux
> + - const: xferbase
> + reg:
> + items:
> + - description: address and length of the controller register set
> + - description: address and length of the Slave DMA data port
> + - description: address and length of the auxiliary registers
> + - description: address and length of the xfer registers
> + else:
> + properties:
> + reg-names:
> + items:
> + - const: io
> + - const: sdma
> + - const: aux
> + reg:
> + items:
> + - description: address and length of the controller register set
> + - description: address and length of the Slave DMA data port
> + - description: address and length of the auxiliary registers

The usual approach here is to define the loosest possible constraints
at the top level, so unconditionally define the xfer register region,
and then constrain things based on compatible. In this case, you can set
minItems to 3 unconditionally and then do (in psuedocode):
if:
marvell
then:
reg:
minitems: 4
else
reg:
maxItems: 3

Additionally, when the allOf: is more then just references to other
documents, it should be moved below the required list.

Thanks,
Conor.


Attachments:
(No filename) (1.70 kB)
signature.asc (235.00 B)
Download all attachments

2024-05-21 10:29:26

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 2/5] dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for Cadence XSPI

On 09/05/2024 03:05, Witold Sadowski wrote:
> Add new bindings for v2 Marvell xSPI overlay:
> mrvl,xspi-nor compatible string

Where?

Why double space?

subject prefix: spi goes first

Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters


> New compatible string to distinguish between orginal and modified xSPI
> block
>
> PHY configuration registers
> Allow to change orginal xSPI PHY configuration values. If not set, and
> Marvell overlay is enabled, safe defaults will be written into xSPI PHY
>
> Optional base for xfer register set
> Additional reg field to allocate xSPI Marvell overlay XFER block

I have troubles reading this. Is this some sort of one long sentence or
a list?

>
> Signed-off-by: Witold Sadowski <[email protected]>
> ---
> .../devicetree/bindings/spi/cdns,xspi.yaml | 78 +++++++++++++++----
> 1 file changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> index eb0f92468185..094f8b7ffc49 100644
> --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> @@ -17,22 +17,43 @@ description: |
>
> allOf:
> - $ref: spi-controller.yaml#
> + - if:

Move the allOf after required block.

> + properties:
> + compatible:
> + contains:
> + const: marvell,cn10-xspi-nor
> + then:
> + properties:
> + reg-names:
> + items:
> + - const: io
> + - const: sdma
> + - const: aux
> + - const: xferbase
> + reg:
> + items:
> + - description: address and length of the controller register set
> + - description: address and length of the Slave DMA data port
> + - description: address and length of the auxiliary registers
> + - description: address and length of the xfer registers
> + else:
> + properties:
> + reg-names:
> + items:
> + - const: io
> + - const: sdma
> + - const: aux
> + reg:
> + items:
> + - description: address and length of the controller register set
> + - description: address and length of the Slave DMA data port
> + - description: address and length of the auxiliary registers
>
> properties:
> compatible:
> - const: cdns,xspi-nor
> -
> - reg:
> - items:
> - - description: address and length of the controller register set
> - - description: address and length of the Slave DMA data port
> - - description: address and length of the auxiliary registers

Widest constraints stay here.

> -
> - reg-names:
> - items:
> - - const: io
> - - const: sdma
> - - const: aux

Widest constraints stay here.

> + enum:
> + - cdns,xspi-nor
> + - marvell,cn10-xspi-nor
>
> interrupts:
> maxItems: 1
> @@ -68,6 +89,37 @@ examples:
> reg = <0>;
> };
>
> + flash@1 {
> + compatible = "jedec,spi-nor";
> + spi-max-frequency = <75000000>;
> + reg = <1>;
> + };
> + };
> + };
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + bus {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + spi@d0010000 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "marvell,cn10-xspi-nor";
> + reg = <0x0 0xa0010000 0x0 0x1040>,
> + <0x0 0xb0000000 0x0 0x1000>,
> + <0x0 0xa0020000 0x0 0x100>,
> + <0x0 0xa0090000 0x0 0x100>;

No need for new example for difference in one property.

Best regards,
Krzysztof


2024-05-21 10:32:24

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 3/5] spi: cadence: Add Marvell xSPI IP overlay changes

On 09/05/2024 03:05, Witold Sadowski wrote:
> Add support for basic v2 Marvell overlay block. Support for basic
> operation is added here: clock configuration, PHY configuration,
> interrupt configuration(enabling)
> Clock divider block is build on top of Cadence xSPI IP, and divides
> external 800MHz clock. It allows only for a few different clock speeds
> starting from 6.25MHz up to 200MHz.
> PHY configuration can be read from device-tree, if parameter is not
> present - safe defaults will be used..
> In addition to handle interrupt propoerly driver must clear MSI-X
> interrupt bit, in addition to clearing xSPI interrupt bit. Interrupt
> masking must be disabled.

Please use full sentences, properly wrapped, continued, readable. There
are typos above, double full-stops and it looks like one sentence per
paragraph... or some sort of list.

>
> Signed-off-by: Witold Sadowski <[email protected]>
> ---


..

>
> cdns_xspi = spi_controller_get_devdata(host);
> @@ -565,23 +809,27 @@ static int cdns_xspi_probe(struct platform_device *pdev)
> init_completion(&cdns_xspi->auto_cmd_complete);
> init_completion(&cdns_xspi->sdma_complete);
>
> + cdns_xspi->mrvl_hw_overlay = drv_data->mrvl_hw_overlay;
> +
> ret = cdns_xspi_of_get_plat_data(pdev);
> if (ret)
> return -ENODEV;
>
> - cdns_xspi->iobase = devm_platform_ioremap_resource_byname(pdev, "io");
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + cdns_xspi->iobase = devm_ioremap_resource(dev, res);

Why are you changing this to two calls? The wrapper is there on purpose.

Anyway, does not look related to this patch.


> if (IS_ERR(cdns_xspi->iobase)) {
> dev_err(dev, "Failed to remap controller base address\n");
> return PTR_ERR(cdns_xspi->iobase);
> }
>
> - res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sdma");
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> cdns_xspi->sdmabase = devm_ioremap_resource(dev, res);
> if (IS_ERR(cdns_xspi->sdmabase))
> return PTR_ERR(cdns_xspi->sdmabase);
> cdns_xspi->sdmasize = resource_size(res);
>
> - cdns_xspi->auxbase = devm_platform_ioremap_resource_byname(pdev, "aux");
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> + cdns_xspi->auxbase = devm_ioremap_resource(dev, res);
> if (IS_ERR(cdns_xspi->auxbase)) {
> dev_err(dev, "Failed to remap AUX address\n");
> return PTR_ERR(cdns_xspi->auxbase);
> @@ -598,8 +846,12 @@ static int cdns_xspi_probe(struct platform_device *pdev)
> return ret;
> }


Best regards,
Krzysztof


2024-05-21 10:33:46

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v4 5/5] spi: cadence: Add MRVL overlay xfer operation support

On 09/05/2024 03:05, Witold Sadowski wrote:
> MRVL Xfer overlay extend xSPI capabilities, to support non-memory SPI
> operations. Marvell overlay combined with generic command allows to
> create full-duplex SPI transactions. It also allows to create
> transaction with undetermined transaction length - with cs_hold
> parameter, and ability to extend CS signal assertion, even if xSPI block
> requests CS signal de-assertion.
>


> +
> static int cdns_xspi_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -905,6 +1140,16 @@ static int cdns_xspi_probe(struct platform_device *pdev)
> return PTR_ERR(cdns_xspi->auxbase);
> }
>
> + if (cdns_xspi->mrvl_hw_overlay) {
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> + cdns_xspi->xferbase = devm_ioremap_resource(dev, res);

Use proper wrapper/helper for these two. This looks like you are working
on old, downstream kernel.

Best regards,
Krzysztof


2024-05-27 08:21:02

by Witold Sadowski

[permalink] [raw]
Subject: RE: [EXTERNAL] Re: [PATCH v4 5/5] spi: cadence: Add MRVL overlay xfer operation support

> ----------------------------------------------------------------------
> On 09/05/2024 03:05, Witold Sadowski wrote:
> > MRVL Xfer overlay extend xSPI capabilities, to support non-memory SPI
> > operations. Marvell overlay combined with generic command allows to
> > create full-duplex SPI transactions. It also allows to create
> > transaction with undetermined transaction length - with cs_hold
> > parameter, and ability to extend CS signal assertion, even if xSPI
> > block requests CS signal de-assertion.
> >
>
>
> > +
> > static int cdns_xspi_probe(struct platform_device *pdev) {
> > struct device *dev = &pdev->dev;
> > @@ -905,6 +1140,16 @@ static int cdns_xspi_probe(struct platform_device
> *pdev)
> > return PTR_ERR(cdns_xspi->auxbase);
> > }
> >
> > + if (cdns_xspi->mrvl_hw_overlay) {
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 3);
> > + cdns_xspi->xferbase = devm_ioremap_resource(dev, res);
>
> Use proper wrapper/helper for these two. This looks like you are working
> on old, downstream kernel.

Ok, changed to devm_devm_platform_ioremap_resource()

>
> Best regards,
> Krzysztof

Regards
Witek

2024-05-27 08:22:53

by Witold Sadowski

[permalink] [raw]
Subject: RE: [EXTERNAL] Re: [PATCH v4 3/5] spi: cadence: Add Marvell xSPI IP overlay changes

> On 09/05/2024 03:05, Witold Sadowski wrote:
> > Add support for basic v2 Marvell overlay block. Support for basic
> > operation is added here: clock configuration, PHY configuration,
> > interrupt configuration(enabling) Clock divider block is build on top
> > of Cadence xSPI IP, and divides external 800MHz clock. It allows only
> > for a few different clock speeds starting from 6.25MHz up to 200MHz.
> > PHY configuration can be read from device-tree, if parameter is not
> > present - safe defaults will be used..
> > In addition to handle interrupt propoerly driver must clear MSI-X
> > interrupt bit, in addition to clearing xSPI interrupt bit. Interrupt
> > masking must be disabled.
>
> Please use full sentences, properly wrapped, continued, readable. There
> are typos above, double full-stops and it looks like one sentence per
> paragraph... or some sort of list.

All commits will be reworked to improve readability, and spelling
>
> >
> > Signed-off-by: Witold Sadowski <[email protected]>
> > ---
>
>
> ...
>
> >
> > cdns_xspi = spi_controller_get_devdata(host); @@ -565,23 +809,27 @@
> > static int cdns_xspi_probe(struct platform_device *pdev)
> > init_completion(&cdns_xspi->auto_cmd_complete);
> > init_completion(&cdns_xspi->sdma_complete);
> >
> > + cdns_xspi->mrvl_hw_overlay = drv_data->mrvl_hw_overlay;
> > +
> > ret = cdns_xspi_of_get_plat_data(pdev);
> > if (ret)
> > return -ENODEV;
> >
> > - cdns_xspi->iobase = devm_platform_ioremap_resource_byname(pdev,
> "io");
> > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + cdns_xspi->iobase = devm_ioremap_resource(dev, res);
>
> Why are you changing this to two calls? The wrapper is there on purpose.
>
> Anyway, does not look related to this patch.

Yes, it will be moved to ACPI patch, and use devm_* call.

>
> Best regards,
> Krzysztof

Regards
Witek

2024-05-27 08:25:18

by Witold Sadowski

[permalink] [raw]
Subject: RE: [EXTERNAL] Re: [PATCH v4 2/5] dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for Cadence XSPI

> On 09/05/2024 03:05, Witold Sadowski wrote:
> > Add new bindings for v2 Marvell xSPI overlay:
> > mrvl,xspi-nor compatible string
>
> Where?
>
> Why double space?
>
> subject prefix: spi goes first
>
> Please use subject prefixes matching the subsystem. You can get them for
> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
> your patch is touching. For bindings, the preferred subjects are explained
> here:
> https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__http://www.kernel.org_doc_html_latest_devicetree_bindings_submitting-
> 2Dpatches.html-23i-2Dfor-2Dpatch-
> 2Dsubmitters&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=GKgcn-g6ZX-
> JmCL3S2qKgVQhvhv7hu2n8En-
> dZbLTa8&m=qeqJlq7clOFz9f6HAtdYw0Qlc95bPthAbwzLip4BHM3fDtQ_rIx-
> R6WPFdKtjM5T&s=d2T7Rq5gLIrXRKVDPv9VDiGgiQVD7GFAxw7lFJ1tgvg&e=
>

Looks like old compatible string was included in commit message.

>
> > New compatible string to distinguish between orginal and modified xSPI
> > block
> >
> > PHY configuration registers
> > Allow to change orginal xSPI PHY configuration values. If not set, and
> > Marvell overlay is enabled, safe defaults will be written into xSPI
> > PHY
> >
> > Optional base for xfer register set
> > Additional reg field to allocate xSPI Marvell overlay XFER block
>
> I have troubles reading this. Is this some sort of one long sentence or a
> list?
>
> >
> > Signed-off-by: Witold Sadowski <[email protected]>
> > ---
> > .../devicetree/bindings/spi/cdns,xspi.yaml | 78 +++++++++++++++----
> > 1 file changed, 65 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > index eb0f92468185..094f8b7ffc49 100644
> > --- a/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > +++ b/Documentation/devicetree/bindings/spi/cdns,xspi.yaml
> > @@ -17,22 +17,43 @@ description: |
> >
> > allOf:
> > - $ref: spi-controller.yaml#
> > + - if:
>
> Move the allOf after required block.
>
> > + properties:
> > + compatible:
> > + contains:
> > + const: marvell,cn10-xspi-nor
> > + then:
> > + properties:
> > + reg-names:
> > + items:
> > + - const: io
> > + - const: sdma
> > + - const: aux
> > + - const: xferbase
> > + reg:
> > + items:
> > + - description: address and length of the controller
> register set
> > + - description: address and length of the Slave DMA data
> port
> > + - description: address and length of the auxiliary
> registers
> > + - description: address and length of the xfer registers
> > + else:
> > + properties:
> > + reg-names:
> > + items:
> > + - const: io
> > + - const: sdma
> > + - const: aux
> > + reg:
> > + items:
> > + - description: address and length of the controller
> register set
> > + - description: address and length of the Slave DMA data
> port
> > + - description: address and length of the auxiliary
> > + registers
> >
> > properties:
> > compatible:
> > - const: cdns,xspi-nor
> > -
> > - reg:
> > - items:
> > - - description: address and length of the controller register set
> > - - description: address and length of the Slave DMA data port
> > - - description: address and length of the auxiliary registers
>
> Widest constraints stay here.
>
> > -
> > - reg-names:
> > - items:
> > - - const: io
> > - - const: sdma
> > - - const: aux
>
> Widest constraints stay here.

Yaml file will be reworked to match guidelines.

>
> > + enum:
> > + - cdns,xspi-nor
> > + - marvell,cn10-xspi-nor
> >
> > interrupts:
> > maxItems: 1
> > @@ -68,6 +89,37 @@ examples:
> > reg = <0>;
> > };
> >
> > + flash@1 {
> > + compatible = "jedec,spi-nor";
> > + spi-max-frequency = <75000000>;
> > + reg = <1>;
> > + };
> > + };
> > + };
> > + - |
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + bus {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > +
> > + spi@d0010000 {
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + compatible = "marvell,cn10-xspi-nor";
> > + reg = <0x0 0xa0010000 0x0 0x1040>,
> > + <0x0 0xb0000000 0x0 0x1000>,
> > + <0x0 0xa0020000 0x0 0x100>,
> > + <0x0 0xa0090000 0x0 0x100>;
>
> No need for new example for difference in one property.

Ok, I will keep only original one

>
> Best regards,
> Krzysztof

2024-05-27 08:26:35

by Witold Sadowski

[permalink] [raw]
Subject: RE: [EXTERNAL] Re: [PATCH v4 2/5] dt-bindings: spi: cadence: Add MRVL overlay bindings documentation for Cadence XSPI

> ----------------------------------------------------------------------
> Hey Witold,
>
> On Wed, May 08, 2024 at 06:05:20PM -0700, Witold Sadowski wrote:
>
> > allOf:
> > - $ref: spi-controller.yaml#
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: marvell,cn10-xspi-nor
> > + then:
> > + properties:
> > + reg-names:
> > + items:
> > + - const: io
> > + - const: sdma
> > + - const: aux
> > + - const: xferbase
> > + reg:
> > + items:
> > + - description: address and length of the controller
> register set
> > + - description: address and length of the Slave DMA data
> port
> > + - description: address and length of the auxiliary
> registers
> > + - description: address and length of the xfer registers
> > + else:
> > + properties:
> > + reg-names:
> > + items:
> > + - const: io
> > + - const: sdma
> > + - const: aux
> > + reg:
> > + items:
> > + - description: address and length of the controller
> register set
> > + - description: address and length of the Slave DMA data
> port
> > + - description: address and length of the auxiliary
> > + registers
>
> The usual approach here is to define the loosest possible constraints at
> the top level, so unconditionally define the xfer register region, and
> then constrain things based on compatible. In this case, you can set
> minItems to 3 unconditionally and then do (in psuedocode):
> if:
> marvell
> then:
> reg:
> minitems: 4
> else
> reg:
> maxItems: 3
>
> Additionally, when the allOf: is more then just references to other
> documents, it should be moved below the required list.

Thanks for hints.
Yaml file will be reworked to match guideliness

>
> Thanks,
> Conor.

Regards
Witek