[PATCH 0/9] spi: spi_amd: Performance Optimization Patch Series

[PATCH 0/9] spi: spi_amd: Performance Optimization Patch Series

[Raju Rangoju]

AMD SPI controller’s index mode performance is constrained by the hardware
limitation of the FIFO queue length. This patch series introduces
optimizations to the spi_amd driver, aiming to maximize throughput and
enhance overall performance. The changes includes,

- Enabling SPI dual and quad I/O modes
- Optimize I/O operations using efficient kernel APIs
- Optimize I/O operations by optimizing set tx/rx count functions
- Optimize I/O operations by reducing the data read calls to fetch the
data from FIFO queues
- Add changes to support AMD HID2 SPI controller
- Add changes to enhance SPI-MEM support functions to reflect hardware
capabilities
- Add changes to set SPI controller address mode before initiating the
commands
- Add changes to implement HIDDMA read operation support for HID2 SPI
controller

Raju Rangoju (9):
  spi: spi_amd: Sort headers alphabetically
  spi: spi_amd: Enable dual and quad I/O modes
  spi: spi_amd: Replace ioread/iowrite calls
  spi: spi_amd: Updates to set tx/rx count functions
  spi: spi_amd: Optimize IO operations
  spi: spi_amd: Add support for HID2 SPI controller
  spi: spi_amd: Enhance SPI-MEM support functions
  spi: spi_amd: Set controller address mode
  spi: spi_amd: Add HIDDMA basic read support

 drivers/spi/spi-amd.c | 325 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 295 insertions(+), 30 deletions(-)

-- 
2.34.1

[PATCH 1/9] spi: spi_amd: Sort headers alphabetically

[Raju Rangoju]

Sorting headers alphabetically helps locating duplicates, and makes it
easier to figure out where to insert new headers.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 2245ad54b03a..c52066360dfe 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -7,12 +7,12 @@
 // Author: Sanjay R Mehta <sanju.mehta@amd.com>
 
 #include <linux/acpi.h>
+#include <linux/delay.h>
 #include <linux/init.h>
+#include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
-#include <linux/delay.h>
 #include <linux/spi/spi.h>
-#include <linux/iopoll.h>
 #include <linux/spi/spi-mem.h>
 
 #define AMD_SPI_CTRL0_REG	0x00
-- 
2.34.1

[PATCH 2/9] spi: spi_amd: Enable dual and quad I/O modes

[Raju Rangoju]

The current spi_amd driver only supports single I/O mode, despite the AMD
SPI controller's capability for dual and quad I/O modes.  So, add support
to enable dual and quad I/O modes.

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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index c52066360dfe..54b5a4d18691 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -364,8 +364,8 @@ static bool amd_spi_supports_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
 	/* bus width is number of IO lines used to transmit */
-	if (op->cmd.buswidth > 1 || op->addr.buswidth > 1 ||
-	    op->data.buswidth > 1 || op->data.nbytes > AMD_SPI_MAX_DATA)
+	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4 ||
+	    op->data.buswidth > 4 || op->data.nbytes > AMD_SPI_MAX_DATA)
 		return false;
 
 	return spi_mem_default_supports_op(mem, op);
@@ -514,7 +514,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 	/* Initialize the spi_controller fields */
 	host->bus_num = 0;
 	host->num_chipselect = 4;
-	host->mode_bits = 0;
+	host->mode_bits = SPI_TX_DUAL | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD;
 	host->flags = SPI_CONTROLLER_HALF_DUPLEX;
 	host->max_speed_hz = AMD_SPI_MAX_HZ;
 	host->min_speed_hz = AMD_SPI_MIN_HZ;
-- 
2.34.1

Re: [PATCH 2/9] spi: spi_amd: Enable dual and quad I/O modes

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

On Wed, Sep 18, 2024 at 04:20:30PM +0530, Raju Rangoju wrote:

>  {
>  	/* bus width is number of IO lines used to transmit */
> -	if (op->cmd.buswidth > 1 || op->addr.buswidth > 1 ||
> -	    op->data.buswidth > 1 || op->data.nbytes > AMD_SPI_MAX_DATA)
> +	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4 ||
> +	    op->data.buswidth > 4 || op->data.nbytes > AMD_SPI_MAX_DATA)
>  		return false;

I'm not seeing anything where we tell the hardware about the width?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 2/9] spi: spi_amd: Enable dual and quad I/O modes

[Rangoju, Raju]

On 9/19/2024 2:13 PM, Mark Brown wrote:
> On Wed, Sep 18, 2024 at 04:20:30PM +0530, Raju Rangoju wrote:
> 
>>   {
>>   	/* bus width is number of IO lines used to transmit */
>> -	if (op->cmd.buswidth > 1 || op->addr.buswidth > 1 ||
>> -	    op->data.buswidth > 1 || op->data.nbytes > AMD_SPI_MAX_DATA)
>> +	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4 ||
>> +	    op->data.buswidth > 4 || op->data.nbytes > AMD_SPI_MAX_DATA)
>>   		return false;
> 
> I'm not seeing anything where we tell the hardware about the width?

The hardware already supports single, dual, and quad I/O modes, and this 
functionality is enabled by default. No explicit software configuration 
is necessary to select the desired I/O mode. The hardware will 
automatically determine the appropriate mode based on the programmed 
opcode. This current patch is only intended to communicate these 
hardware capabilities to upper layers.

[PATCH 3/9] spi: spi_amd: Replace ioread/iowrite calls

[Raju Rangoju]

All `ioread*` and `iowrite*` functions are better suited for architecture
independent code to ensure portability across different architectures.
Since AMD SoCs support only the x86 architecture, replacing all `ioread*`
and `iowrite*` calls with `read*` and `write*` calls can reduce the
overhead of ensuring portability and increase the speed of I/O operations.

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 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 54b5a4d18691..11ae910bb420 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -99,12 +99,12 @@ struct amd_spi {
 
 static inline u8 amd_spi_readreg8(struct amd_spi *amd_spi, int idx)
 {
-	return ioread8((u8 __iomem *)amd_spi->io_remap_addr + idx);
+	return readb((u8 __iomem *)amd_spi->io_remap_addr + idx);
 }
 
 static inline void amd_spi_writereg8(struct amd_spi *amd_spi, int idx, u8 val)
 {
-	iowrite8(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
+	writeb(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
 }
 
 static void amd_spi_setclear_reg8(struct amd_spi *amd_spi, int idx, u8 set, u8 clear)
@@ -117,12 +117,12 @@ static void amd_spi_setclear_reg8(struct amd_spi *amd_spi, int idx, u8 set, u8 c
 
 static inline u32 amd_spi_readreg32(struct amd_spi *amd_spi, int idx)
 {
-	return ioread32((u8 __iomem *)amd_spi->io_remap_addr + idx);
+	return readl((u8 __iomem *)amd_spi->io_remap_addr + idx);
 }
 
 static inline void amd_spi_writereg32(struct amd_spi *amd_spi, int idx, u32 val)
 {
-	iowrite32(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
+	writel(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
 }
 
 static inline void amd_spi_setclear_reg32(struct amd_spi *amd_spi, int idx, u32 set, u32 clear)
-- 
2.34.1

[PATCH 4/9] spi: spi_amd: Updates to set tx/rx count functions

[Raju Rangoju]

AMD SPI TX and RX counter registers are 1-byte length registers. The
existing value will be overwritten during register write, so masking is not
required.

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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 11ae910bb420..eb16063ba121 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -165,12 +165,12 @@ static int amd_spi_set_opcode(struct amd_spi *amd_spi, u8 cmd_opcode)
 
 static inline void amd_spi_set_rx_count(struct amd_spi *amd_spi, u8 rx_count)
 {
-	amd_spi_setclear_reg8(amd_spi, AMD_SPI_RX_COUNT_REG, rx_count, 0xff);
+	amd_spi_writereg8(amd_spi, AMD_SPI_RX_COUNT_REG, rx_count);
 }
 
 static inline void amd_spi_set_tx_count(struct amd_spi *amd_spi, u8 tx_count)
 {
-	amd_spi_setclear_reg8(amd_spi, AMD_SPI_TX_COUNT_REG, tx_count, 0xff);
+	amd_spi_writereg8(amd_spi, AMD_SPI_TX_COUNT_REG, tx_count);
 }
 
 static int amd_spi_busy_wait(struct amd_spi *amd_spi)
-- 
2.34.1

[PATCH 5/9] spi: spi_amd: Optimize IO operations

[Raju Rangoju]

Read and write the maximum number of data bytes at once, rather than byte
by byte. This improves AMD SPI controller driver performance by reducing
the time required to access FIFO registers. For example, with the new
changes, 64 bytes of data from the FIFO queue can be read in 8 read calls
(8 bytes per call) instead of 64 read calls(1 byte per call).

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 | 53 +++++++++++++++++++++++++++++++++----------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index eb16063ba121..c265e37cebc4 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -9,6 +9,7 @@
 #include <linux/acpi.h>
 #include <linux/delay.h>
 #include <linux/init.h>
+#include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/iopoll.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
@@ -125,6 +126,16 @@ static inline void amd_spi_writereg32(struct amd_spi *amd_spi, int idx, u32 val)
 	writel(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
 }
 
+static inline u64 amd_spi_readreg64(struct amd_spi *amd_spi, int idx)
+{
+	return readq((u8 __iomem *)amd_spi->io_remap_addr + idx);
+}
+
+static inline void amd_spi_writereg64(struct amd_spi *amd_spi, int idx, u64 val)
+{
+	writeq(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
+}
+
 static inline void amd_spi_setclear_reg32(struct amd_spi *amd_spi, int idx, u32 set, u32 clear)
 {
 	u32 tmp = amd_spi_readreg32(amd_spi, idx);
@@ -397,15 +408,23 @@ 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;
-	u8 *buf = (u8 *)op->data.buf.out;
+	u64 *buf_64 = (u64 *)op->data.buf.out;
 	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);
 
-	for (i = 0; i < nbytes; i++)
-		amd_spi_writereg8(amd_spi, (base_addr + i), buf[i]);
+	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);
@@ -416,23 +435,33 @@ static void amd_spi_mem_data_out(struct amd_spi *amd_spi,
 static void amd_spi_mem_data_in(struct amd_spi *amd_spi,
 				const struct spi_mem_op *op)
 {
-	int offset = (op->addr.nbytes == 0) ? 0 : 1;
-	u8 *buf = (u8 *)op->data.buf.in;
+	int base_addr = AMD_SPI_FIFO_BASE + op->addr.nbytes;
+	u64 *buf_64 = (u64 *)op->data.buf.in;
 	u32 nbytes = op->data.nbytes;
-	int base_addr, i;
-
-	base_addr = AMD_SPI_FIFO_BASE + op->addr.nbytes + offset;
+	u32 left_data = nbytes;
+	u8 *buf;
+	int i;
 
 	amd_spi_set_opcode(amd_spi, op->cmd.opcode);
 	amd_spi_set_addr(amd_spi, op);
-	amd_spi_set_tx_count(amd_spi, op->addr.nbytes);
-	amd_spi_set_rx_count(amd_spi, op->data.nbytes + 1);
+	amd_spi_set_tx_count(amd_spi, op->addr.nbytes + op->dummy.nbytes);
+
+	for (i = 0; i < op->dummy.nbytes; i++)
+		amd_spi_writereg8(amd_spi, (base_addr + i), 0xff);
+
+	amd_spi_set_rx_count(amd_spi, op->data.nbytes);
 	amd_spi_clear_fifo_ptr(amd_spi);
 	amd_spi_execute_opcode(amd_spi);
 	amd_spi_busy_wait(amd_spi);
 
-	for (i = 0; i < nbytes; i++)
-		buf[i] = amd_spi_readreg8(amd_spi, base_addr + i);
+	for (i = 0; left_data >= 8; i++, left_data -= 8)
+		*buf_64++ = amd_spi_readreg64(amd_spi, base_addr + op->dummy.nbytes +
+					      (i * 8));
+
+	buf = (u8 *)buf_64;
+	for (i = 0; i < left_data; i++)
+		buf[i] = amd_spi_readreg8(amd_spi, base_addr + op->dummy.nbytes +
+					  nbytes + i - left_data);
 }
 
 static int amd_spi_exec_mem_op(struct spi_mem *mem,
-- 
2.34.1

[PATCH 6/9] spi: spi_amd: Add support for HID2 SPI controller

[Raju Rangoju]

AMD SoC has HID2 SPI controller in addition to the existing SPI0
controller(AMDI0062). Add HID2 SPI controller's ACPI ID AMDI0063 with its
version ID to the list of supported devices. And, use the version ID to
differentiate the register offsets.

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 | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index c265e37cebc4..ccad969f501f 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -55,10 +55,12 @@
  * enum amd_spi_versions - SPI controller versions
  * @AMD_SPI_V1:		AMDI0061 hardware version
  * @AMD_SPI_V2:		AMDI0062 hardware version
+ * @AMD_HID2_SPI:	AMDI0063 hardware version
  */
 enum amd_spi_versions {
 	AMD_SPI_V1 = 1,
 	AMD_SPI_V2,
+	AMD_HID2_SPI,
 };
 
 enum amd_spi_speed {
@@ -167,6 +169,7 @@ static int amd_spi_set_opcode(struct amd_spi *amd_spi, u8 cmd_opcode)
 				       AMD_SPI_OPCODE_MASK);
 		return 0;
 	case AMD_SPI_V2:
+	case AMD_HID2_SPI:
 		amd_spi_writereg8(amd_spi, AMD_SPI_OPCODE_REG, cmd_opcode);
 		return 0;
 	default:
@@ -194,6 +197,7 @@ static int amd_spi_busy_wait(struct amd_spi *amd_spi)
 		reg = AMD_SPI_CTRL0_REG;
 		break;
 	case AMD_SPI_V2:
+	case AMD_HID2_SPI:
 		reg = AMD_SPI_STATUS_REG;
 		break;
 	default:
@@ -219,6 +223,7 @@ static int amd_spi_execute_opcode(struct amd_spi *amd_spi)
 				       AMD_SPI_EXEC_CMD);
 		return 0;
 	case AMD_SPI_V2:
+	case AMD_HID2_SPI:
 		/* Trigger the command execution */
 		amd_spi_setclear_reg8(amd_spi, AMD_SPI_CMD_TRIGGER_REG,
 				      AMD_SPI_TRIGGER_CMD, AMD_SPI_TRIGGER_CMD);
@@ -360,6 +365,7 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 	case AMD_SPI_V1:
 		break;
 	case AMD_SPI_V2:
+	case AMD_HID2_SPI:
 		amd_spi_clear_chip(amd_spi, spi_get_chipselect(message->spi, 0));
 		break;
 	default:
@@ -541,7 +547,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 	amd_spi->version = (uintptr_t) device_get_match_data(dev);
 
 	/* Initialize the spi_controller fields */
-	host->bus_num = 0;
+	host->bus_num = (amd_spi->version == AMD_HID2_SPI) ? 2 : 0;
 	host->num_chipselect = 4;
 	host->mode_bits = SPI_TX_DUAL | SPI_TX_QUAD | SPI_RX_DUAL | SPI_RX_QUAD;
 	host->flags = SPI_CONTROLLER_HALF_DUPLEX;
@@ -565,6 +571,7 @@ static int amd_spi_probe(struct platform_device *pdev)
 static const struct acpi_device_id spi_acpi_match[] = {
 	{ "AMDI0061", AMD_SPI_V1 },
 	{ "AMDI0062", AMD_SPI_V2 },
+	{ "AMDI0063", AMD_HID2_SPI },
 	{},
 };
 MODULE_DEVICE_TABLE(acpi, spi_acpi_match);
-- 
2.34.1

[PATCH 7/9] spi: spi_amd: Enhance SPI-MEM support functions

[Raju Rangoju]

AMD SPI0 controller supports quad mode only for read operations and has
limited support for quad mode writes. And, the AMD HID2 SPI controller
supports DMA read, allowing for up to 4 KB of data to be read in single
transaction. Update the SPI-MEM support functions to reflect these hardware
capabilities.

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 | 84 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 81 insertions(+), 3 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index ccad969f501f..f146366a67e7 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -38,6 +38,7 @@
 #define AMD_SPI_FIFO_SIZE	70
 #define AMD_SPI_MEM_SIZE	200
 #define AMD_SPI_MAX_DATA	64
+#define AMD_SPI_HID2_DMA_SIZE   4096
 
 #define AMD_SPI_ENA_REG		0x20
 #define AMD_SPI_ALT_SPD_SHIFT	20
@@ -51,6 +52,21 @@
 #define AMD_SPI_MAX_HZ		100000000
 #define AMD_SPI_MIN_HZ		800000
 
+/* SPI read command opcodes */
+#define AMD_SPI_OP_READ          0x03	/* Read data bytes (low frequency) */
+#define AMD_SPI_OP_READ_FAST     0x0b	/* Read data bytes (high frequency) */
+#define AMD_SPI_OP_READ_1_1_2    0x3b	/* Read data bytes (Dual Output SPI) */
+#define AMD_SPI_OP_READ_1_2_2    0xbb	/* Read data bytes (Dual I/O SPI) */
+#define AMD_SPI_OP_READ_1_1_4    0x6b	/* Read data bytes (Quad Output SPI) */
+#define AMD_SPI_OP_READ_1_4_4    0xeb	/* Read data bytes (Quad I/O SPI) */
+
+/* SPI read command opcodes - 4B address */
+#define AMD_SPI_OP_READ_FAST_4B		0x0c    /* Read data bytes (high frequency) */
+#define AMD_SPI_OP_READ_1_1_2_4B	0x3c    /* Read data bytes (Dual Output SPI) */
+#define AMD_SPI_OP_READ_1_2_2_4B	0xbc    /* Read data bytes (Dual I/O SPI) */
+#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) */
+
 /**
  * enum amd_spi_versions - SPI controller versions
  * @AMD_SPI_V1:		AMDI0061 hardware version
@@ -377,20 +393,82 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 	return message->status;
 }
 
+static inline bool amd_is_spi_read_cmd_4b(const u16 op)
+{
+	switch (op) {
+	case AMD_SPI_OP_READ_FAST_4B:
+	case AMD_SPI_OP_READ_1_1_2_4B:
+	case AMD_SPI_OP_READ_1_2_2_4B:
+	case AMD_SPI_OP_READ_1_1_4_4B:
+	case AMD_SPI_OP_READ_1_4_4_4B:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static inline bool amd_is_spi_read_cmd(const u16 op)
+{
+	switch (op) {
+	case AMD_SPI_OP_READ:
+	case AMD_SPI_OP_READ_FAST:
+	case AMD_SPI_OP_READ_1_1_2:
+	case AMD_SPI_OP_READ_1_2_2:
+	case AMD_SPI_OP_READ_1_1_4:
+	case AMD_SPI_OP_READ_1_4_4:
+		return true;
+	default:
+		return amd_is_spi_read_cmd_4b(op);
+	}
+}
+
 static bool amd_spi_supports_op(struct spi_mem *mem,
 				const struct spi_mem_op *op)
 {
+	struct amd_spi *amd_spi = spi_controller_get_devdata(mem->spi->controller);
+
 	/* bus width is number of IO lines used to transmit */
-	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4 ||
-	    op->data.buswidth > 4 || op->data.nbytes > AMD_SPI_MAX_DATA)
+	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4)
+		return false;
+
+	/* AMD SPI controllers support quad mode only for read operations */
+	if (amd_is_spi_read_cmd(op->cmd.opcode)) {
+		if (op->data.buswidth > 4)
+			return false;
+
+		/*
+		 * HID2 SPI controller supports DMA read up to 4K bytes and
+		 * doesn't support 4-byte address commands.
+		 */
+		if (amd_spi->version == AMD_HID2_SPI) {
+			if (amd_is_spi_read_cmd_4b(op->cmd.opcode) ||
+			    op->data.nbytes > AMD_SPI_HID2_DMA_SIZE)
+				return false;
+		} else if (op->data.nbytes > AMD_SPI_MAX_DATA) {
+			return false;
+		}
+	} else if (op->data.buswidth > 1 || op->data.nbytes > AMD_SPI_MAX_DATA) {
 		return false;
+	}
 
 	return spi_mem_default_supports_op(mem, op);
 }
 
 static int amd_spi_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
 {
-	op->data.nbytes = clamp_val(op->data.nbytes, 0, AMD_SPI_MAX_DATA);
+	struct amd_spi *amd_spi = spi_controller_get_devdata(mem->spi->controller);
+
+	/*
+	 * HID2 SPI controller DMA read mode supports reading up to 4k
+	 * bytes in single transaction, where as SPI0 and HID2 SPI
+	 * 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))
+		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);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 7/9] spi: spi_amd: Enhance SPI-MEM support functions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

On Wed, Sep 18, 2024 at 04:20:35PM +0530, Raju Rangoju wrote:

> AMD SPI0 controller supports quad mode only for read operations and has
> limited support for quad mode writes. And, the AMD HID2 SPI controller
> supports DMA read, allowing for up to 4 KB of data to be read in single
> transaction. Update the SPI-MEM support functions to reflect these hardware
> capabilities.

>  	/* bus width is number of IO lines used to transmit */
> -	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4 ||
> -	    op->data.buswidth > 4 || op->data.nbytes > AMD_SPI_MAX_DATA)

So this code you added earlier in the series is clearly buggy...  this
whole change should've been part of that patch, and the addidion of the
patch adding the new HID.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 7/9] spi: spi_amd: Enhance SPI-MEM support functions

[Rangoju, Raju]

On 9/19/2024 2:21 PM, Mark Brown wrote:
> On Wed, Sep 18, 2024 at 04:20:35PM +0530, Raju Rangoju wrote:
> 
>> AMD SPI0 controller supports quad mode only for read operations and has
>> limited support for quad mode writes. And, the AMD HID2 SPI controller
>> supports DMA read, allowing for up to 4 KB of data to be read in single
>> transaction. Update the SPI-MEM support functions to reflect these hardware
>> capabilities.
> 
>>   	/* bus width is number of IO lines used to transmit */
>> -	if (op->cmd.buswidth > 1 || op->addr.buswidth > 4 ||
>> -	    op->data.buswidth > 4 || op->data.nbytes > AMD_SPI_MAX_DATA)
> 
> So this code you added earlier in the series is clearly buggy...  this
> whole change should've been part of that patch, and the addidion of the
> patch adding the new HID.

Sure, I will take care of this in V2.

[PATCH 8/9] spi: spi_amd: Set controller address mode

[Raju Rangoju]

Add changes to set the controller address mode before initiating commands.
The AMD SPI0 controller(AMDI0062) supports both 24-bit and 32-bit address
modes, while the HID2 SPI controller(AMDI0063) supports only the 24-bit
address mode. So this change is applicable only for SPI0 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 | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index f146366a67e7..50dfdf2ab6ee 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -34,6 +34,7 @@
 #define AMD_SPI_TX_COUNT_REG	0x48
 #define AMD_SPI_RX_COUNT_REG	0x4B
 #define AMD_SPI_STATUS_REG	0x4C
+#define AMD_SPI_ADDR32CTRL_REG	0x50
 
 #define AMD_SPI_FIFO_SIZE	70
 #define AMD_SPI_MEM_SIZE	200
@@ -548,6 +549,17 @@ static void amd_spi_mem_data_in(struct amd_spi *amd_spi,
 					  nbytes + i - left_data);
 }
 
+static void amd_set_spi_addr_mode(struct amd_spi *amd_spi,
+				  const struct spi_mem_op *op)
+{
+	u32 val = amd_spi_readreg32(amd_spi, AMD_SPI_ADDR32CTRL_REG);
+
+	if (amd_is_spi_read_cmd_4b(op->cmd.opcode))
+		amd_spi_writereg32(amd_spi, AMD_SPI_ADDR32CTRL_REG, val | BIT(0));
+	else
+		amd_spi_writereg32(amd_spi, AMD_SPI_ADDR32CTRL_REG, val & ~BIT(0));
+}
+
 static int amd_spi_exec_mem_op(struct spi_mem *mem,
 			       const struct spi_mem_op *op)
 {
@@ -560,6 +572,9 @@ static int amd_spi_exec_mem_op(struct spi_mem *mem,
 	if (ret)
 		return ret;
 
+	if (amd_spi->version == AMD_SPI_V2)
+		amd_set_spi_addr_mode(amd_spi, op);
+
 	switch (op->data.dir) {
 	case SPI_MEM_DATA_IN:
 		amd_spi_mem_data_in(amd_spi, op);
-- 
2.34.1

[PATCH 9/9] spi: spi_amd: Add HIDDMA basic read support

[Raju Rangoju]

SPI index mode has hardware limitation of reading only 64 bytes per
transaction due to fixed number of FIFO registers. This constraint leads
to performance issues when reading data from NAND/NOR flash devices, as the
controller must issue multiple requests to read 64-byte chunks, even if the
slave can send up to 2 or 4 KB in single transaction. The AMD HID2 SPI
controller supports DMA mode, which allows reading up to 4 KB of data in
single transaction. This patch introduces changes to implement HID2 DMA
read 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 | 176 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 156 insertions(+), 20 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 50dfdf2ab6ee..d30a21b0b05f 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -8,6 +8,7 @@
 
 #include <linux/acpi.h>
 #include <linux/delay.h>
+#include <linux/dma-mapping.h>
 #include <linux/init.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 #include <linux/iopoll.h>
@@ -50,9 +51,21 @@
 #define AMD_SPI_SPD7_SHIFT	8
 #define AMD_SPI_SPD7_MASK	GENMASK(13, AMD_SPI_SPD7_SHIFT)
 
+#define AMD_SPI_HID2_INPUT_RING_BUF0	0X100
+#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_READ_CNTRL0	0x170
+#define AMD_SPI_HID2_READ_CNTRL1	0x174
+#define AMD_SPI_HID2_READ_CNTRL2	0x180
+
 #define AMD_SPI_MAX_HZ		100000000
 #define AMD_SPI_MIN_HZ		800000
 
+#define AMD_SPI_IO_SLEEP_US	20
+#define AMD_SPI_IO_TIMEOUT_US	2000000
+
 /* SPI read command opcodes */
 #define AMD_SPI_OP_READ          0x03	/* Read data bytes (low frequency) */
 #define AMD_SPI_OP_READ_FAST     0x0b	/* Read data bytes (high frequency) */
@@ -108,11 +121,15 @@ struct amd_spi_freq {
 /**
  * struct amd_spi - SPI driver instance
  * @io_remap_addr:	Start address of the SPI controller registers
+ * @phy_dma_buf:	Physical address of DMA buffer
+ * @dma_virt_addr:	Virtual address of DMA buffer
  * @version:		SPI controller hardware version
  * @speed_hz:		Device frequency
  */
 struct amd_spi {
 	void __iomem *io_remap_addr;
+	dma_addr_t phy_dma_buf;
+	void *dma_virt_addr;
 	enum amd_spi_versions version;
 	unsigned int speed_hz;
 };
@@ -135,6 +152,16 @@ static void amd_spi_setclear_reg8(struct amd_spi *amd_spi, int idx, u8 set, u8 c
 	amd_spi_writereg8(amd_spi, idx, tmp);
 }
 
+static inline u16 amd_spi_readreg16(struct amd_spi *amd_spi, int idx)
+{
+	return readw((u8 __iomem *)amd_spi->io_remap_addr + idx);
+}
+
+static inline void amd_spi_writereg16(struct amd_spi *amd_spi, int idx, u16 val)
+{
+	writew(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
+}
+
 static inline u32 amd_spi_readreg32(struct amd_spi *amd_spi, int idx)
 {
 	return readl((u8 __iomem *)amd_spi->io_remap_addr + idx);
@@ -517,6 +544,64 @@ static void amd_spi_mem_data_out(struct amd_spi *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)
+{
+	u16 hid_cmd_start, val;
+	u32 hid_regval;
+
+	/* Set the opcode in hid2_read_control0 register */
+	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_READ_CNTRL0);
+	hid_regval = (hid_regval & ~GENMASK(7, 0)) | op->cmd.opcode;
+
+	/*
+	 * Program the address in the hid2_read_control0 register [8:31]. The address should
+	 * be written starting from the 8th bit of the register, requiring an 8-bit shift.
+	 * Additionally, to convert a 2-byte spinand address to a 3-byte address, another
+	 * 8-bit shift is needed. Therefore, a total shift of 16 bits is required.
+	 */
+	hid_regval = (hid_regval & ~GENMASK(31, 8)) | (op->addr.val << 16);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_READ_CNTRL0, hid_regval);
+
+	/* Configure dummy clock cycles for fast read, dual, quad I/O commands */
+	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_READ_CNTRL2);
+	/* Fast read dummy cycle */
+	hid_regval &= ~GENMASK(4, 0);
+
+	/* Fast read Dual I/O dummy cycle */
+	hid_regval &= ~GENMASK(12, 8);
+
+	/* Fast read Quad I/O dummy cycle */
+	hid_regval = (hid_regval & ~GENMASK(20, 16)) | BIT(17);
+
+	/* Set no of preamble bytecount */
+	hid_regval &= ~GENMASK(27, 24);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_READ_CNTRL2, hid_regval);
+
+	/*
+	 * Program the HID2 Input Ring Buffer0. 4k aligned buf_memory_addr[31:12],
+	 * buf_size[4:0], end_input_ring[5].
+	 */
+	hid_regval = amd_spi->phy_dma_buf | BIT(5) | BIT(0);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_INPUT_RING_BUF0, hid_regval);
+
+	/* Program max read length(no of DWs) in hid2_read_control1 register */
+	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_READ_CNTRL1);
+	hid_regval = (hid_regval & ~GENMASK(15, 0)) | ((op->data.nbytes / 4) - 1);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_READ_CNTRL1, hid_regval);
+
+	/* Set cmd start bit in hid2_cmd_start register to trigger HID basic read 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(3)));
+
+	/* Check interrupt status of HIDDMA basic read operation in hid2_int_status register */
+	readw_poll_timeout(amd_spi->io_remap_addr + AMD_SPI_HID2_INT_STATUS, val,
+			   (val & BIT(3)), 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_in(struct amd_spi *amd_spi,
 				const struct spi_mem_op *op)
 {
@@ -524,29 +609,52 @@ static void amd_spi_mem_data_in(struct amd_spi *amd_spi,
 	u64 *buf_64 = (u64 *)op->data.buf.in;
 	u32 nbytes = op->data.nbytes;
 	u32 left_data = nbytes;
+	u32 data;
 	u8 *buf;
 	int i;
 
-	amd_spi_set_opcode(amd_spi, op->cmd.opcode);
-	amd_spi_set_addr(amd_spi, op);
-	amd_spi_set_tx_count(amd_spi, op->addr.nbytes + op->dummy.nbytes);
-
-	for (i = 0; i < op->dummy.nbytes; i++)
-		amd_spi_writereg8(amd_spi, (base_addr + i), 0xff);
-
-	amd_spi_set_rx_count(amd_spi, op->data.nbytes);
-	amd_spi_clear_fifo_ptr(amd_spi);
-	amd_spi_execute_opcode(amd_spi);
-	amd_spi_busy_wait(amd_spi);
-
-	for (i = 0; left_data >= 8; i++, left_data -= 8)
-		*buf_64++ = amd_spi_readreg64(amd_spi, base_addr + op->dummy.nbytes +
-					      (i * 8));
+	/*
+	 * Condition for using HID read mode. Only for reading complete page data, use HID read.
+	 * Use index mode otherwise.
+	 */
+	if (amd_spi->version == AMD_HID2_SPI && amd_is_spi_read_cmd(op->cmd.opcode)) {
+		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));
+
+		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));
+
+		/* Reset HID RX memory logic */
+		data = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_CNTRL);
+		amd_spi_writereg32(amd_spi, AMD_SPI_HID2_CNTRL, data | BIT(5));
+	} else {
+		/* Index mode */
+		amd_spi_set_opcode(amd_spi, op->cmd.opcode);
+		amd_spi_set_addr(amd_spi, op);
+		amd_spi_set_tx_count(amd_spi, op->addr.nbytes + op->dummy.nbytes);
+
+		for (i = 0; i < op->dummy.nbytes; i++)
+			amd_spi_writereg8(amd_spi, (base_addr + i), 0xff);
+
+		amd_spi_set_rx_count(amd_spi, op->data.nbytes);
+		amd_spi_clear_fifo_ptr(amd_spi);
+		amd_spi_execute_opcode(amd_spi);
+		amd_spi_busy_wait(amd_spi);
+
+		for (i = 0; left_data >= 8; i++, left_data -= 8)
+			*buf_64++ = amd_spi_readreg64(amd_spi, base_addr + op->dummy.nbytes +
+						      (i * 8));
+
+		buf = (u8 *)buf_64;
+		for (i = 0; i < left_data; i++)
+			buf[i] = amd_spi_readreg8(amd_spi, base_addr + op->dummy.nbytes +
+						  nbytes + i - left_data);
+	}
 
-	buf = (u8 *)buf_64;
-	for (i = 0; i < left_data; i++)
-		buf[i] = amd_spi_readreg8(amd_spi, base_addr + op->dummy.nbytes +
-					  nbytes + i - left_data);
 }
 
 static void amd_set_spi_addr_mode(struct amd_spi *amd_spi,
@@ -617,6 +725,31 @@ static size_t amd_spi_max_transfer_size(struct spi_device *spi)
 	return AMD_SPI_FIFO_SIZE;
 }
 
+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);
+	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.
+	 */
+	hid_regval = amd_spi_readreg32(amd_spi, AMD_SPI_HID2_INT_MASK);
+	hid_regval = (hid_regval & GENMASK(31, 8)) | BIT(19);
+	amd_spi_writereg32(amd_spi, AMD_SPI_HID2_INT_MASK, hid_regval);
+
+	/* Configure buffer unit(4k) 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));
+
+	return 0;
+}
+
 static int amd_spi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -657,7 +790,10 @@ static int amd_spi_probe(struct platform_device *pdev)
 	if (err)
 		return dev_err_probe(dev, err, "error registering SPI controller\n");
 
-	return 0;
+	if (amd_spi->version == AMD_HID2_SPI)
+		err = amd_spi_setup_hiddma(amd_spi, dev);
+
+	return err;
 }
 
 #ifdef CONFIG_ACPI
-- 
2.34.1