[PATCH v2 0/3] spi: spi_amd: Add DMA write and Kconfig changes

[PATCH v2 0/3] spi: spi_amd: Add DMA write and Kconfig changes

[Raju Rangoju]

This series include following changes to spi_amd driver:
- Changes to replace read{q,b} functions with direct memory copy logic on
  DMA buffer.
- Support for HID2 DMA single mode basic write operation for the HID2
  SPI controller.
- Changes to add missing Kconfig dependencies.

v1->v2:
-------
- Add a new patch to replace read{q,b} functions on DMA buffer.
- Address the review comments by replacing write{q,b} functions
  with manual data copy on DMA buffer.
- Add a new patch to address missing Kconfig dependencies.

Raju Rangoju (3):
  spi: spi_amd: Remove read{q,b} usage on DMA buffer
  spi: spi_amd: Add HIDDMA basic write support
  spi: spi_amd: Update Kconfig dependencies

 drivers/spi/Kconfig   |   3 +-
 drivers/spi/spi-amd.c | 151 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 125 insertions(+), 29 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] spi: spi_amd: Remove read{q,b} usage on DMA buffer

[Raju Rangoju]

Add changes to replace the usage of read{q,b} MMIO accessors with direct
memory copy logic for reading data from DMA buffer.

Co-developed-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Signed-off-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/spi/spi-amd.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 90b1f7daa072..52d893d41b12 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -590,15 +590,21 @@ static void amd_spi_mem_data_in(struct amd_spi *amd_spi,
 	 * Use index mode otherwise.
 	 */
 	if (amd_spi->version == AMD_HID2_SPI && amd_is_spi_read_cmd(op->cmd.opcode)) {
+		u64 *dma_buf64 = (u64 *)amd_spi->dma_virt_addr;
+		u8 *dma_buf;
+
 		amd_spi_hiddma_read(amd_spi, op);
 
-		for (i = 0; left_data >= 8; i++, left_data -= 8)
-			*buf_64++ = readq((u8 __iomem *)amd_spi->dma_virt_addr + (i * 8));
+		/* Copy data from DMA buffer */
+		while (left_data >= 8) {
+			*buf_64++ = *dma_buf64++;
+			left_data -= 8;
+		}
 
 		buf = (u8 *)buf_64;
-		for (i = 0; i < left_data; i++)
-			buf[i] = readb((u8 __iomem *)amd_spi->dma_virt_addr +
-				       (nbytes - left_data + i));
+		dma_buf = (u8 *)dma_buf64;
+		while (left_data--)
+			*buf++ = *dma_buf++;
 
 		/* Reset HID RX memory logic */
 		data = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_CNTRL);
-- 
2.34.1

[PATCH v2 2/3] spi: spi_amd: Add HIDDMA basic write support

[Raju Rangoju]

SPI index mode has hardware limitation of transferring only 64 bytes per
transaction due to fixed number of FIFO registers. This constraint leads to
performance issues when reading/writing data to/from NAND/NOR flash
devices, as the controller must issue multiple requests to read/write
64-byte chunks, even if the slave can transfer up to 2 or 4 KB in a single
transaction.

The AMD HID2 SPI controller supports DMA mode, allowing for reading/writing
up to 4 KB of data in a single transaction. The existing spi_amd driver
already supports HID2 DMA read operations.

This patch introduces changes to implement HID2 DMA single mode basic write
support for the HID2 SPI controller.

Co-developed-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Signed-off-by: Krishnamoorthi M <krishnamoorthi.m@amd.com>
Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/spi/spi-amd.c | 135 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 112 insertions(+), 23 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 52d893d41b12..02e7fe095a0b 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -54,10 +54,13 @@
 #define AMD_SPI_SPD7_MASK	GENMASK(13, AMD_SPI_SPD7_SHIFT)
 
 #define AMD_SPI_HID2_INPUT_RING_BUF0	0X100
+#define AMD_SPI_HID2_OUTPUT_BUF0	0x140
 #define AMD_SPI_HID2_CNTRL		0x150
 #define AMD_SPI_HID2_INT_STATUS		0x154
 #define AMD_SPI_HID2_CMD_START		0x156
 #define AMD_SPI_HID2_INT_MASK		0x158
+#define AMD_SPI_HID2_WRITE_CNTRL0	0x160
+#define AMD_SPI_HID2_WRITE_CNTRL1	0x164
 #define AMD_SPI_HID2_READ_CNTRL0	0x170
 #define AMD_SPI_HID2_READ_CNTRL1	0x174
 #define AMD_SPI_HID2_READ_CNTRL2	0x180
@@ -83,6 +86,10 @@
 #define AMD_SPI_OP_READ_1_1_4_4B	0x6c    /* Read data bytes (Quad Output SPI) */
 #define AMD_SPI_OP_READ_1_4_4_4B	0xec    /* Read data bytes (Quad I/O SPI) */
 
+/* SPINAND write command opcodes */
+#define AMD_SPI_OP_PP			0x02	/* Page program */
+#define AMD_SPI_OP_PP_RANDOM		0x84	/* Page program */
+
 enum amd_spi_speed {
 	F_66_66MHz,
 	F_33_33MHz,
@@ -419,6 +426,17 @@ static inline bool amd_is_spi_read_cmd(const u16 op)
 	}
 }
 
+static inline bool amd_is_spi_write_cmd(const u16 op)
+{
+	switch (op) {
+	case AMD_SPI_OP_PP:
+	case AMD_SPI_OP_PP_RANDOM:
+		return true;
+	default:
+		return false;
+	}
+}
+
 static bool amd_spi_supports_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
@@ -429,7 +447,7 @@ static bool amd_spi_supports_op(struct spi_mem *mem,
 		return false;
 
 	/* AMD SPI controllers support quad mode only for read operations */
-	if (amd_is_spi_read_cmd(op->cmd.opcode)) {
+	if (amd_is_spi_read_cmd(op->cmd.opcode) || amd_is_spi_write_cmd(op->cmd.opcode)) {
 		if (op->data.buswidth > 4)
 			return false;
 
@@ -438,7 +456,8 @@ static bool amd_spi_supports_op(struct spi_mem *mem,
 		 * doesn't support 4-byte address commands.
 		 */
 		if (amd_spi->version == AMD_HID2_SPI) {
-			if (amd_is_spi_read_cmd_4b(op->cmd.opcode) ||
+			if ((amd_is_spi_read_cmd_4b(op->cmd.opcode) ||
+			     amd_is_spi_write_cmd(op->cmd.opcode)) &&
 			    op->data.nbytes > AMD_SPI_HID2_DMA_SIZE)
 				return false;
 		} else if (op->data.nbytes > AMD_SPI_MAX_DATA) {
@@ -464,7 +483,8 @@ static int amd_spi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 	 * controller index mode supports maximum of 64 bytes in a single
 	 * transaction.
 	 */
-	if (amd_spi->version == AMD_HID2_SPI && amd_is_spi_read_cmd(op->cmd.opcode))
+	if (amd_spi->version == AMD_HID2_SPI && (amd_is_spi_read_cmd(op->cmd.opcode) ||
+						 amd_is_spi_write_cmd(op->cmd.opcode)))
 		op->data.nbytes = clamp_val(op->data.nbytes, 0, AMD_SPI_HID2_DMA_SIZE);
 	else
 		op->data.nbytes = clamp_val(op->data.nbytes, 0, AMD_SPI_MAX_DATA);
@@ -488,32 +508,96 @@ static void amd_spi_set_addr(struct amd_spi *amd_spi,
 	}
 }
 
+static void amd_spi_hiddma_write(struct amd_spi *amd_spi, const struct spi_mem_op *op)
+{
+	u16 hid_cmd_start, val;
+	u32 hid_regval;
+
+	/*
+	 * Program the HID2 output Buffer0. 4k aligned buf_memory_addr[31:12],
+	 * buf_size[2:0].
+	 */
+	hid_regval = amd_spi->phy_dma_buf | BIT(0);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_OUTPUT_BUF0, hid_regval);
+
+	/* Program max write length in hid2_write_control1 register */
+	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_WRITE_CNTRL1);
+	hid_regval = (hid_regval & ~GENMASK(15, 0)) | ((op->data.nbytes) + 3);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_WRITE_CNTRL1, hid_regval);
+
+	/* Set cmd start bit in hid2_cmd_start register to trigger HID basic write operation */
+	hid_cmd_start = amd_spi_readreg16(amd_spi, AMD_SPI_HID2_CMD_START);
+	amd_spi_writereg16(amd_spi, AMD_SPI_HID2_CMD_START, (hid_cmd_start | BIT(2)));
+
+	/* Check interrupt status of HIDDMA basic write operation in hid2_int_status register */
+	readw_poll_timeout(amd_spi->io_remap_addr + AMD_SPI_HID2_INT_STATUS, val,
+			   (val & BIT(2)), AMD_SPI_IO_SLEEP_US, AMD_SPI_IO_TIMEOUT_US);
+
+	/* Clear the interrupts by writing to hid2_int_status register */
+	val = amd_spi_readreg16(amd_spi, AMD_SPI_HID2_INT_STATUS);
+	amd_spi_writereg16(amd_spi, AMD_SPI_HID2_INT_STATUS, val);
+}
+
 static void amd_spi_mem_data_out(struct amd_spi *amd_spi,
 				 const struct spi_mem_op *op)
 {
 	int base_addr = AMD_SPI_FIFO_BASE + op->addr.nbytes;
 	u64 *buf_64 = (u64 *)op->data.buf.out;
+	u64 addr_val = op->addr.val;
 	u32 nbytes = op->data.nbytes;
 	u32 left_data = nbytes;
 	u8 *buf;
 	int i;
 
-	amd_spi_set_opcode(amd_spi, op->cmd.opcode);
-	amd_spi_set_addr(amd_spi, op);
+	/*
+	 * Condition for using HID write mode. Only for writing complete page data, use HID write.
+	 * Use index mode otherwise.
+	 */
+	if (amd_spi->version == AMD_HID2_SPI && amd_is_spi_write_cmd(op->cmd.opcode)) {
+		u64 *dma_buf64 = (u64 *)(amd_spi->dma_virt_addr + op->addr.nbytes + op->cmd.nbytes);
+		u8 *dma_buf = (u8 *)amd_spi->dma_virt_addr;
 
-	for (i = 0; left_data >= 8; i++, left_data -= 8)
-		amd_spi_writereg64(amd_spi, base_addr + op->dummy.nbytes + (i * 8), *buf_64++);
+		/* Copy opcode and address to DMA buffer */
+		*dma_buf = op->cmd.opcode;
 
-	buf = (u8 *)buf_64;
-	for (i = 0; i < left_data; i++) {
-		amd_spi_writereg8(amd_spi, base_addr + op->dummy.nbytes + nbytes + i - left_data,
-				  buf[i]);
-	}
+		dma_buf = (u8 *)dma_buf64;
+		for (i = 0; i < op->addr.nbytes; i++) {
+			*--dma_buf = addr_val & GENMASK(7, 0);
+			addr_val >>= 8;
+		}
 
-	amd_spi_set_tx_count(amd_spi, op->addr.nbytes + op->data.nbytes);
-	amd_spi_set_rx_count(amd_spi, 0);
-	amd_spi_clear_fifo_ptr(amd_spi);
-	amd_spi_execute_opcode(amd_spi);
+		/* Copy data to DMA buffer */
+		while (left_data >= 8) {
+			*dma_buf64++ = *buf_64++;
+			left_data -= 8;
+		}
+
+		buf = (u8 *)buf_64;
+		dma_buf = (u8 *)dma_buf64;
+		while (left_data--)
+			*dma_buf++ = *buf++;
+
+		amd_spi_hiddma_write(amd_spi, op);
+	} else {
+		amd_spi_set_opcode(amd_spi, op->cmd.opcode);
+		amd_spi_set_addr(amd_spi, op);
+
+		for (i = 0; left_data >= 8; i++, left_data -= 8)
+			amd_spi_writereg64(amd_spi, base_addr + op->dummy.nbytes + (i * 8),
+					   *buf_64++);
+
+		buf = (u8 *)buf_64;
+		for (i = 0; i < left_data; i++) {
+			amd_spi_writereg8(amd_spi,
+					  base_addr + op->dummy.nbytes + nbytes + i - left_data,
+					  buf[i]);
+		}
+
+		amd_spi_set_tx_count(amd_spi, op->addr.nbytes + op->data.nbytes);
+		amd_spi_set_rx_count(amd_spi, 0);
+		amd_spi_clear_fifo_ptr(amd_spi);
+		amd_spi_execute_opcode(amd_spi);
+	}
 }
 
 static void amd_spi_hiddma_read(struct amd_spi *amd_spi, const struct spi_mem_op *op)
@@ -708,23 +792,28 @@ static int amd_spi_setup_hiddma(struct amd_spi *amd_spi, struct device *dev)
 {
 	u32 hid_regval;
 
-	/* Allocate DMA buffer to use for HID basic read operation */
-	amd_spi->dma_virt_addr = dma_alloc_coherent(dev, AMD_SPI_HID2_DMA_SIZE,
-						    &amd_spi->phy_dma_buf, GFP_KERNEL);
+	/* Allocate DMA buffer to use for HID basic read and write operations. For write
+	 * operations, the DMA buffer should include the opcode, address bytes and dummy
+	 * bytes(if any) in addition to the data bytes. Additionally, the hardware requires
+	 * that the buffer address be 4K aligned. So, allocate DMA buffer of size
+	 * 2 * AMD_SPI_HID2_DMA_SIZE.
+	 */
+	amd_spi->dma_virt_addr = dmam_alloc_coherent(dev, AMD_SPI_HID2_DMA_SIZE * 2,
+						     &amd_spi->phy_dma_buf, GFP_KERNEL);
 	if (!amd_spi->dma_virt_addr)
 		return -ENOMEM;
 
 	/*
 	 * Enable interrupts and set mask bits in hid2_int_mask register to generate interrupt
-	 * properly for HIDDMA basic read operations.
+	 * properly for HIDDMA basic read and write operations.
 	 */
 	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_INT_MASK);
-	hid_regval = (hid_regval & GENMASK(31, 8)) | BIT(19);
+	hid_regval = (hid_regval & GENMASK(31, 8)) | BIT(18) | BIT(19);
 	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_INT_MASK, hid_regval);
 
-	/* Configure buffer unit(4k) in hid2_control register */
+	/* Configure buffer unit(4k) and write threshold in hid2_control register */
 	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_CNTRL);
-	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_CNTRL, hid_regval & ~BIT(3));
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_CNTRL, (hid_regval | GENMASK(13, 12)) & ~BIT(3));
 
 	return 0;
 }
-- 
2.34.1

[PATCH v2 3/3] spi: spi_amd: Update Kconfig dependencies

[Raju Rangoju]

Add X86 and SPI_MEM as dependencies for the spi_amd driver to ensure it is
built only on relevant platforms and with the required SPI memory
framework.

Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/spi/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index b03a7005f1bd..c51da3fc3604 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -1267,7 +1267,8 @@ config SPI_ZYNQMP_GQSPI
 config SPI_AMD
 	tristate "AMD SPI controller"
 	depends on PCI
-	depends on SPI_MASTER || COMPILE_TEST
+	depends on SPI_MASTER || X86 || COMPILE_TEST
+	depends on SPI_MEM
 	help
 	  Enables SPI controller driver for AMD SoC.
 
-- 
2.34.1

Re: [PATCH v2 3/3] spi: spi_amd: Update Kconfig dependencies

[Geert Uytterhoeven]

Hi Raju,

Thanks for your patch, which is now commit dbb79974193a2932 ("spi:
spi_amd: Update Kconfig dependencies") upstream.

On Fri, 16 May 2025 at 12:09, Raju Rangoju <Raju.Rangoju@amd.com> wrote:
> Add X86 and SPI_MEM as dependencies for the spi_amd driver to ensure it is
> built only on relevant platforms and with the required SPI memory
> framework.

This sounds like you are restricting the driver further (BTW, what are
"relevant platforms"?)...

> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>

> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -1267,7 +1267,8 @@ config SPI_ZYNQMP_GQSPI
>  config SPI_AMD
>         tristate "AMD SPI controller"
>         depends on PCI
> -       depends on SPI_MASTER || COMPILE_TEST
> +       depends on SPI_MASTER || X86 || COMPILE_TEST

... but this broadens it instead?
And how can it be used on X86 if SPI_MASTER=n?

Seeing an ACPI match table in drivers/spi/spi-amd.c, perhaps you wanted
to have a dependency on ACPI (or X86 && ACPI) somewhere?

BTW, as you now have a single Kconfig symbol gating both spi-amd.c
and spi-amd-pci.c, this means you can no longer build spi-amd.c if
CONFIG_PCI=n.

> +       depends on SPI_MEM
>         help
>           Enables SPI controller driver for AMD SoC.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v2 3/3] spi: spi_amd: Update Kconfig dependencies

[Rangoju, Raju]

On 5/28/2025 1:02 PM, Geert Uytterhoeven wrote:
> Hi Raju,
> 
> Thanks for your patch, which is now commit dbb79974193a2932 ("spi:
> spi_amd: Update Kconfig dependencies") upstream.
> 
> On Fri, 16 May 2025 at 12:09, Raju Rangoju <Raju.Rangoju@amd.com> wrote:
>> Add X86 and SPI_MEM as dependencies for the spi_amd driver to ensure it is
>> built only on relevant platforms and with the required SPI memory
>> framework.
> 
> This sounds like you are restricting the driver further (BTW, what are
> "relevant platforms"?)...

AMD x86 platforms.

> 
>> Co-developed-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>> Signed-off-by: Akshata MukundShetty <akshata.mukundshetty@amd.com>
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> 
>> --- a/drivers/spi/Kconfig
>> +++ b/drivers/spi/Kconfig
>> @@ -1267,7 +1267,8 @@ config SPI_ZYNQMP_GQSPI
>>   config SPI_AMD
>>          tristate "AMD SPI controller"
>>          depends on PCI
>> -       depends on SPI_MASTER || COMPILE_TEST
>> +       depends on SPI_MASTER || X86 || COMPILE_TEST
> 
> ... but this broadens it instead?
> And how can it be used on X86 if SPI_MASTER=n?
> 
> Seeing an ACPI match table in drivers/spi/spi-amd.c, perhaps you wanted
> to have a dependency on ACPI (or X86 && ACPI) somewhere?
> 
> BTW, as you now have a single Kconfig symbol gating both spi-amd.c
> and spi-amd-pci.c, this means you can no longer build spi-amd.c if
> CONFIG_PCI=n.

Sure Greert. Thanks for the suggestion.
We are planning to have separate Kconfig for both ACPI and PCI based 
drivers in the upcoming series.

> 
>> +       depends on SPI_MEM
>>          help
>>            Enables SPI controller driver for AMD SoC.
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
> 

Re: [PATCH v2 0/3] spi: spi_amd: Add DMA write and Kconfig changes

[Mark Brown]

On Fri, 16 May 2025 15:36:55 +0530, Raju Rangoju wrote:
> This series include following changes to spi_amd driver:
> - Changes to replace read{q,b} functions with direct memory copy logic on
>   DMA buffer.
> - Support for HID2 DMA single mode basic write operation for the HID2
>   SPI controller.
> - Changes to add missing Kconfig dependencies.
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/3] spi: spi_amd: Remove read{q,b} usage on DMA buffer
      commit: 8cd079e69dc51e707b0a7ce105b01f6dbb66ddc1
[2/3] spi: spi_amd: Add HIDDMA basic write support
      commit: a5733666c775eb852409261d7a6363883d97ff93
[3/3] spi: spi_amd: Update Kconfig dependencies
      commit: dbb79974193a2932e828ebbd216efb428c81dc63

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark