[PATCH v3 0/3] spi:amd:Support for new generation of AMD SOCs.

[PATCH v3 0/3] spi:amd:Support for new generation of AMD SOCs.

[Nehal Bakulchandra Shah]

Fix of limitation  of max 72 bytes size of fifo transfer.
Also, new generation SOC support added with modification with 
register and few of the helper functions.
As driver is only supported for X86 based platform modified the KConfig
with ACPI dependency to fix kernel test robot errors.

Changes in v3:
-Update the Kconfig with ACPI dependency

Changes in v2:
-Split the patch
-Incorporate review comments

Changes in v1:
-Initial patch for adding support of new generation of SOC -Fix for 72 bytes of FIFO Size

Nehal Bakulchandra Shah (3):
  spi:amd: Add support for latest platform
  spi:amd: Fix for transfer large size of data
  spi:amd:Fix for compilation error for non X86 platforms.

 drivers/spi/Kconfig   |   2 +-
 drivers/spi/spi-amd.c | 149 +++++++++++++++++++++++++++++++++++-------
 2 files changed, 126 insertions(+), 25 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] spi:amd: Add support for latest platform

[Nehal Bakulchandra Shah]

-Add device ID for new generation of platform.
-Modify spi_busy and opcode commands based on controller version.

Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Liang Liang <liang.liang@amd.com>
Signed-off-by: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@amd.com>
---
 drivers/spi/spi-amd.c | 72 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 63 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 3cf76096a76d..2150f54512d9 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -14,10 +14,12 @@
 #include <linux/spi/spi.h>
 
 #define AMD_SPI_CTRL0_REG	0x00
+#define AMD_SPI_OPCODE_REG  0x45
+#define AMD_SPI_CMD_TRIGGER_REG 0x47
 #define AMD_SPI_EXEC_CMD	BIT(16)
 #define AMD_SPI_FIFO_CLEAR	BIT(20)
 #define AMD_SPI_BUSY		BIT(31)
-
+#define AMD_SPI_TRIGGER_CMD	BIT(7)
 #define AMD_SPI_OPCODE_MASK	0xFF
 
 #define AMD_SPI_ALT_CS_REG	0x1D
@@ -34,11 +36,31 @@
 #define AMD_SPI_XFER_TX		1
 #define AMD_SPI_XFER_RX		2
 
+#ifdef CONFIG_ACPI
+struct amd_spi_devtype_data {
+	u32 spi_status;
+	u8	version;
+};
+
+static const struct amd_spi_devtype_data spi_v1 = {
+	.spi_status	= AMD_SPI_CTRL0_REG,
+	.version	= 0,
+};
+
+static const struct amd_spi_devtype_data spi_v2 = {
+	.spi_status	= AMD_SPI_STATUS_REG,
+	.version	= 1,
+};
+#endif
+
 struct amd_spi {
 	void __iomem *io_remap_addr;
 	unsigned long io_base_addr;
 	u32 rom_addr;
 	u8 chip_select;
+	const struct amd_spi_devtype_data *devtype_data;
+	struct spi_device *spi_dev;
+	struct spi_master *master;
 };
 
 static inline u8 amd_spi_readreg8(struct spi_master *master, int idx)
@@ -98,6 +120,14 @@ static void amd_spi_select_chip(struct spi_master *master)
 			      AMD_SPI_ALT_CS_MASK);
 }
 
+static void amd_spi_clear_chip(struct spi_master *master)
+{
+	struct amd_spi *amd_spi = spi_master_get_devdata(master);
+	u8 chip_select = amd_spi->chip_select;
+
+	amd_spi_writereg8(master, AMD_SPI_ALT_CS_REG, chip_select & 0XFC);
+}
+
 static void amd_spi_clear_fifo_ptr(struct spi_master *master)
 {
 	amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_FIFO_CLEAR,
@@ -106,8 +136,13 @@ static void amd_spi_clear_fifo_ptr(struct spi_master *master)
 
 static void amd_spi_set_opcode(struct spi_master *master, u8 cmd_opcode)
 {
-	amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, cmd_opcode,
-			       AMD_SPI_OPCODE_MASK);
+	struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+	if (!amd_spi->devtype_data->version)
+		amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, cmd_opcode,
+				       AMD_SPI_OPCODE_MASK);
+	else
+		amd_spi_writereg8(master, AMD_SPI_OPCODE_REG, cmd_opcode);
 }
 
 static inline void amd_spi_set_rx_count(struct spi_master *master,
@@ -126,17 +161,20 @@ static inline int amd_spi_busy_wait(struct amd_spi *amd_spi)
 {
 	bool spi_busy;
 	int timeout = 100000;
+	u32 status_reg = amd_spi->devtype_data->spi_status;
 
 	/* poll for SPI bus to become idle */
 	spi_busy = (ioread32((u8 __iomem *)amd_spi->io_remap_addr +
-		    AMD_SPI_CTRL0_REG) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
+				status_reg) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
+
 	while (spi_busy) {
 		usleep_range(10, 20);
 		if (timeout-- < 0)
 			return -ETIMEDOUT;
 
+		/* poll for SPI bus to become idle */
 		spi_busy = (ioread32((u8 __iomem *)amd_spi->io_remap_addr +
-			    AMD_SPI_CTRL0_REG) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
+				status_reg) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
 	}
 
 	return 0;
@@ -146,9 +184,16 @@ static void amd_spi_execute_opcode(struct spi_master *master)
 {
 	struct amd_spi *amd_spi = spi_master_get_devdata(master);
 
+	/*Check for busy wait*/
+	amd_spi_busy_wait(amd_spi);
+
 	/* Set ExecuteOpCode bit in the CTRL0 register */
-	amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_EXEC_CMD,
-			       AMD_SPI_EXEC_CMD);
+	if (!amd_spi->devtype_data->version)
+		amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_EXEC_CMD,
+				       AMD_SPI_EXEC_CMD);
+	else
+		amd_spi_setclear_reg8(master, AMD_SPI_CMD_TRIGGER_REG, AMD_SPI_TRIGGER_CMD,
+				      AMD_SPI_TRIGGER_CMD);
 
 	amd_spi_busy_wait(amd_spi);
 }
@@ -241,7 +286,8 @@ static int amd_spi_master_transfer(struct spi_master *master,
 	 * program the controller.
 	 */
 	amd_spi_fifo_xfer(amd_spi, master, msg);
-
+	if (amd_spi->devtype_data->version)
+		amd_spi_clear_chip(master);
 	return 0;
 }
 
@@ -266,6 +312,11 @@ static int amd_spi_probe(struct platform_device *pdev)
 		dev_err(dev, "error %d ioremap of SPI registers failed\n", err);
 		goto err_free_master;
 	}
+	amd_spi->devtype_data = device_get_match_data(dev);
+	if (!amd_spi->devtype_data) {
+		err = -ENODEV;
+		goto err_free_master;
+	}
 	dev_dbg(dev, "io_remap_address: %p\n", amd_spi->io_remap_addr);
 
 	/* Initialize the spi_master fields */
@@ -293,7 +344,10 @@ static int amd_spi_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id spi_acpi_match[] = {
-	{ "AMDI0061", 0 },
+	{ "AMDI0061",
+	.driver_data = (kernel_ulong_t)&spi_v1 },
+	{ "AMDI0062",
+	.driver_data = (kernel_ulong_t)&spi_v2 },
 	{},
 };
 MODULE_DEVICE_TABLE(acpi, spi_acpi_match);
-- 
2.25.1

[PATCH v3 2/3] spi:amd: Fix for transfer large size of data

[Nehal Bakulchandra Shah]

Hardware has 72 bytes of FIFO.This patch addresses the same by means
of software workaround.

Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Liang Liang <liang.liang@amd.com>
Signed-off-by: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@amd.com>
---
 drivers/spi/spi-amd.c | 77 ++++++++++++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 15 deletions(-)

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 2150f54512d9..2849f901a075 100644
--- a/drivers/spi/spi-amd.c
+++ b/drivers/spi/spi-amd.c
@@ -2,9 +2,10 @@
 //
 // AMD SPI controller driver
 //
-// Copyright (c) 2020, Advanced Micro Devices, Inc.
+// Copyright (c) 2020-2021, Advanced Micro Devices, Inc.
 //
-// Author: Sanjay R Mehta <sanju.mehta@amd.com>
+// Authors: Sanjay R Mehta <sanju.mehta@amd.com>
+//          Nehal Bakulchandra Shah <nehal-bakulchandra.shah@amd.com>
 
 #include <linux/acpi.h>
 #include <linux/init.h>
@@ -29,7 +30,7 @@
 #define AMD_SPI_TX_COUNT_REG	0x48
 #define AMD_SPI_RX_COUNT_REG	0x4B
 #define AMD_SPI_STATUS_REG	0x4C
-
+#define AMD_SPI_FIFO_SIZE	72
 #define AMD_SPI_MEM_SIZE	200
 
 /* M_CMD OP codes for SPI */
@@ -215,8 +216,8 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 	u8 cmd_opcode;
 	u8 *buf = NULL;
 	u32 m_cmd = 0;
-	u32 i = 0;
-	u32 tx_len = 0, rx_len = 0;
+	u32 i = 0, it = 0, tx_index = 0, rx_index = 0;
+	u32 tx_len = 0, rx_len = 0, iters = 0, remaining =  0;
 
 	list_for_each_entry(xfer, &message->transfers,
 			    transfer_list) {
@@ -230,17 +231,40 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 			tx_len = xfer->len - 1;
 			cmd_opcode = *(u8 *)xfer->tx_buf;
 			buf++;
+
+			tx_index = 0;
+			iters = tx_len / AMD_SPI_FIFO_SIZE;
+			remaining = tx_len % AMD_SPI_FIFO_SIZE;
+
+			for (it = 0; it < iters; it++) {
+				amd_spi_clear_fifo_ptr(master);
+				amd_spi_set_opcode(master, cmd_opcode);
+
+				amd_spi_set_tx_count(master, AMD_SPI_FIFO_SIZE);
+				/* Write data into the FIFO. */
+				for (i = 0; i < AMD_SPI_FIFO_SIZE; i++) {
+					iowrite8(buf[tx_index],
+						 ((u8 __iomem *)amd_spi->io_remap_addr +
+						 AMD_SPI_FIFO_BASE + i));
+					tx_index++;
+				}
+
+				/* Execute command */
+				amd_spi_execute_opcode(master);
+			}
+
+			amd_spi_clear_fifo_ptr(master);
 			amd_spi_set_opcode(master, cmd_opcode);
 
+			amd_spi_set_tx_count(master, remaining);
 			/* Write data into the FIFO. */
-			for (i = 0; i < tx_len; i++) {
-				iowrite8(buf[i],
+			for (i = 0; i < remaining; i++) {
+				iowrite8(buf[tx_index],
 					 ((u8 __iomem *)amd_spi->io_remap_addr +
-					 AMD_SPI_FIFO_BASE + i));
+					AMD_SPI_FIFO_BASE + i));
+				tx_index++;
 			}
 
-			amd_spi_set_tx_count(master, tx_len);
-			amd_spi_clear_fifo_ptr(master);
 			/* Execute command */
 			amd_spi_execute_opcode(master);
 		}
@@ -250,16 +274,38 @@ static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 			 * FIFO
 			 */
 			rx_len = xfer->len;
+			rx_index = 0;
+			iters = rx_len / AMD_SPI_FIFO_SIZE;
+			remaining = rx_len % AMD_SPI_FIFO_SIZE;
 			buf = (u8 *)xfer->rx_buf;
-			amd_spi_set_rx_count(master, rx_len);
+
+			for (it = 0 ; it < iters; it++) {
+				amd_spi_clear_fifo_ptr(master);
+
+				amd_spi_set_rx_count(master, AMD_SPI_FIFO_SIZE);
+
+				/* Execute command */
+				amd_spi_execute_opcode(master);
+				/* Read data from FIFO to receive buffer  */
+				for (i = 0; i < AMD_SPI_FIFO_SIZE; i++) {
+					buf[rx_index] = amd_spi_readreg8(master, AMD_SPI_FIFO_BASE +
+									tx_len + i);
+					rx_index++;
+				}
+			}
+
 			amd_spi_clear_fifo_ptr(master);
+
+			amd_spi_set_rx_count(master, remaining);
+
 			/* Execute command */
 			amd_spi_execute_opcode(master);
 			/* Read data from FIFO to receive buffer  */
-			for (i = 0; i < rx_len; i++)
-				buf[i] = amd_spi_readreg8(master,
-							  AMD_SPI_FIFO_BASE +
-							  tx_len + i);
+			for (i = 0; i < remaining; i++) {
+				buf[rx_index] = amd_spi_readreg8(master, AMD_SPI_FIFO_BASE +
+								tx_len + i);
+				rx_index++;
+			}
 		}
 	}
 
@@ -365,4 +411,5 @@ module_platform_driver(amd_spi_driver);
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_AUTHOR("Sanjay Mehta <sanju.mehta@amd.com>");
+MODULE_AUTHOR("Nehal Bakulchandra Shah <nehal-bakulchandra.shah@amd.com>");
 MODULE_DESCRIPTION("AMD SPI Master Controller Driver");
-- 
2.25.1

[PATCH v3 3/3] spi:amd:Fix for compilation error for non X86 platforms.

[Nehal Bakulchandra Shah]

Update the KConfig with dependency for ACPI as driver is only
supported for x86 platform. This fixes the compilation error
reported by kernel test robot.

Reported-by: kernel test robot <lkp@intel.com>
Reviewed-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
Reviewed-by: Liang Liang (Leo) <liang.liang@amd.com>
Signed-off-by: Nehal Bakulchandra Shah <Nehal-Bakulchandra.shah@amd.com>
---
 drivers/spi/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index e71a4c514f7b..532387929085 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -978,7 +978,7 @@ config SPI_ZYNQMP_GQSPI
 
 config SPI_AMD
 	tristate "AMD SPI controller"
-	depends on SPI_MASTER || COMPILE_TEST
+	depends on (SPI_MASTER && ACPI) || COMPILE_TEST
 	help
 	  Enables SPI controller driver for AMD SoC.
 
-- 
2.25.1

Re: [PATCH v3 3/3] spi:amd:Fix for compilation error for non X86 platforms.

[Mark Brown]

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

On Wed, Jun 30, 2021 at 05:34:25PM +0530, Nehal Bakulchandra Shah wrote:

> Update the KConfig with dependency for ACPI as driver is only
> supported for x86 platform. This fixes the compilation error
> reported by kernel test robot.

If one of the earlier patches in the series fails to build you need to
fix that patch so you don't break bisection or cause people to spend
time finding errors in the earlier patches you fix later.

In any case this doesn't do what the commit log says, ACPI is supported
on at least arm64 so if you genuinely need an x86 dependency ACPI isn't
going to cut it and COMPILE_TEST means the driver will still be built on
everything.  I don't recall anything from 0day that looked like it was
anything to do with dependencies though.

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

Re: [PATCH v3 3/3] spi:amd:Fix for compilation error for non X86 platforms.

[Shah, Nehal-bakulchandra]

Hi Mark
On 6/30/2021 6:17 PM, Mark Brown wrote:
> On Wed, Jun 30, 2021 at 05:34:25PM +0530, Nehal Bakulchandra Shah wrote:
>
>> Update the KConfig with dependency for ACPI as driver is only
>> supported for x86 platform. This fixes the compilation error
>> reported by kernel test robot.
> If one of the earlier patches in the series fails to build you need to
> fix that patch so you don't break bisection or cause people to spend
> time finding errors in the earlier patches you fix later.
>
> In any case this doesn't do what the commit log says, ACPI is supported
> on at least arm64 so if you genuinely need an x86 dependency ACPI isn't
> going to cut it and COMPILE_TEST means the driver will still be built on
> everything.  I don't recall anything from 0day that looked like it was
> anything to do with dependencies though.
so now should i RESEND this patch with suggested changes,i.e removing 
ACPI depedency
change and removing COMPILE_TEST?
Thanks
Nehal

Re: [PATCH v3 3/3] spi:amd:Fix for compilation error for non X86 platforms.

[Mark Brown]

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

On Wed, Jun 30, 2021 at 08:14:12PM +0530, Shah, Nehal-bakulchandra wrote:

> > everything.  I don't recall anything from 0day that looked like it was
> > anything to do with dependencies though.

> so now should i RESEND this patch with suggested changes,i.e removing ACPI
> depedency
> change and removing COMPILE_TEST?

No, you should fix the actual problem - like I say it looked like just a
regular coding error, not something due to an actual dependency.

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

Re: [PATCH v3 3/3] spi:amd:Fix for compilation error for non X86 platforms.

[Shah, Nehal-bakulchandra]

Hi Mark
On 6/30/2021 6:17 PM, Mark Brown wrote:
> On Wed, Jun 30, 2021 at 05:34:25PM +0530, Nehal Bakulchandra Shah wrote:
>
>> Update the KConfig with dependency for ACPI as driver is only
>> supported for x86 platform. This fixes the compilation error
>> reported by kernel test robot.
> If one of the earlier patches in the series fails to build you need to
> fix that patch so you don't break bisection or cause people to spend
> time finding errors in the earlier patches you fix later.
>
> In any case this doesn't do what the commit log says, ACPI is supported
> on at least arm64 so if you genuinely need an x86 dependency ACPI isn't
> going to cut it and COMPILE_TEST means the driver will still be built on
> everything.  I don't recall anything from 0day that looked like it was
> anything to do with dependencies though.

Thanks for reviewing this patch series, however we have decided to drop 
this patch series and our partner (Cirrus) will send the new patch 
series with

more finer changes.

Regards

Nehal Shah