diff mbox series

spi:amd: Add support for latest platform

Message ID 20210520133946.2263172-1-Nehal-Bakulchandra.shah@amd.com (mailing list archive)
State New, archived
Headers show
Series spi:amd: Add support for latest platform | expand

Commit Message

Nehal-bakulchandra Shah May 20, 2021, 1:39 p.m. UTC 
*Support for latest platform
*Hardware Fifo has 72 bytes limitation so fix for the larger data size.

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/spi-amd.c | 160 +++++++++++++++++++++++++++++++++---------
 1 file changed, 128 insertions(+), 32 deletions(-)

Comments

Mark Brown May 20, 2021, 6:33 p.m. UTC | #1 
On Thu, May 20, 2021 at 07:09:46PM +0530, Nehal Bakulchandra Shah wrote:

> *Support for latest platform
> *Hardware Fifo has 72 bytes limitation so fix for the larger data size.

These two things sound like they should be separate patches, and it
looks like there are some other changes mixed in here which aren't
called out in the changelog.  This should be a patch series with one
change per patch, that makes things much easier to review.

>  	while (spi_busy) {
> -		usleep_range(10, 20);
> +		usleep_range(10, 40);

Why change the delay?  This looks like a separate patch.

>  	return 0;
> @@ -146,9 +180,14 @@ static void amd_spi_execute_opcode(struct spi_master *master)
>  {
>  	struct amd_spi *amd_spi = spi_master_get_devdata(master);
>  
> +	amd_spi_busy_wait(amd_spi);
>  	/* Set ExecuteOpCode bit in the CTRL0 register */

A blank line after the busy wait, and it'd be good to have a comment
saying why there's a busy wait before actually doing the operation since
this looks quite odd.

> @@ -241,7 +325,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);

Does this disable the chip select?  Should there be a separate set_cs()
operation?

>  	amd_spi = spi_master_get_devdata(master);
> -	amd_spi->io_remap_addr = devm_platform_ioremap_resource(pdev, 0);
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	amd_spi->io_remap_addr = devm_ioremap_resource(&pdev->dev, res);
> +

res is never referenced so it's not clear why this change is being made?

>  	/* Initialize the spi_master fields */
>  	master->bus_num = 0;
>  	master->num_chipselect = 4;
>  	master->mode_bits = 0;
> -	master->flags = SPI_MASTER_HALF_DUPLEX;
>  	master->setup = amd_spi_master_setup;
>  	master->transfer_one_message = amd_spi_master_transfer;

I'm not seeing a change anywhere that looks like it adds full duplex
support for the v1 hardware (or v2 for that matter) and this isn't
called out in the changelog.
kernel test robot May 22, 2021, 8:09 p.m. UTC | #2 
Hi Nehal,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v5.13-rc2 next-20210521]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nehal-Bakulchandra-Shah/spi-amd-Add-support-for-latest-platform/20210522-234047
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: riscv-randconfig-r022-20210522 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project e84a9b9bb3051c35dea993cdad7b3d2575638f85)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/91df9b56ff78696d45a2a38475f1d84819d0c27b
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nehal-Bakulchandra-Shah/spi-amd-Add-support-for-latest-platform/20210522-234047
        git checkout 91df9b56ff78696d45a2a38475f1d84819d0c27b
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/spi/spi-amd.c:42:42: warning: unused variable 'spi_v1' [-Wunused-const-variable]
   static const struct amd_spi_devtype_data spi_v1 = {
                                            ^
>> drivers/spi/spi-amd.c:47:42: warning: unused variable 'spi_v2' [-Wunused-const-variable]
   static const struct amd_spi_devtype_data spi_v2 = {
                                            ^
   2 warnings generated.

Kconfig warnings: (for reference only)
   WARNING: unmet direct dependencies detected for ERRATA_SIFIVE
   Depends on RISCV_ERRATA_ALTERNATIVE
   Selected by
   - SOC_SIFIVE


vim +/spi_v1 +42 drivers/spi/spi-amd.c

    41	
  > 42	static const struct amd_spi_devtype_data spi_v1 = {
    43		.spi_status	= AMD_SPI_CTRL0_REG,
    44		.version	= 0,
    45	};
    46	
  > 47	static const struct amd_spi_devtype_data spi_v2 = {
    48		.spi_status	= AMD_SPI_STATUS_REG,
    49		.version	= 1,
    50	};
    51	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
index 3cf76096a76d..071c01eedbc3 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>
@@ -14,31 +15,48 @@ 
 #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
 #define AMD_SPI_ALT_CS_MASK	0x3
-
 #define AMD_SPI_FIFO_BASE	0x80
 #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 */
 #define AMD_SPI_XFER_TX		1
 #define AMD_SPI_XFER_RX		2
 
+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,
+};
+
 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 +116,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 +132,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 +157,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);
+		usleep_range(10, 40);
 		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 +180,14 @@  static void amd_spi_execute_opcode(struct spi_master *master)
 {
 	struct amd_spi *amd_spi = spi_master_get_devdata(master);
 
+	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);
 }
@@ -170,8 +209,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) {
@@ -182,20 +221,43 @@  static inline int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
 
 		if (m_cmd & AMD_SPI_XFER_TX) {
 			buf = (u8 *)xfer->tx_buf;
-			tx_len = xfer->len - 1;
 			cmd_opcode = *(u8 *)xfer->tx_buf;
+			tx_len = xfer->len - 1;
 			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);
 		}
@@ -205,16 +267,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++;
+			}
 		}
 	}
 
@@ -241,7 +325,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;
 }
 
@@ -250,6 +335,7 @@  static int amd_spi_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct spi_master *master;
 	struct amd_spi *amd_spi;
+	struct resource *res;
 	int err = 0;
 
 	/* Allocate storage for spi_master and driver private data */
@@ -260,19 +346,25 @@  static int amd_spi_probe(struct platform_device *pdev)
 	}
 
 	amd_spi = spi_master_get_devdata(master);
-	amd_spi->io_remap_addr = devm_platform_ioremap_resource(pdev, 0);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	amd_spi->io_remap_addr = devm_ioremap_resource(&pdev->dev, res);
+
 	if (IS_ERR(amd_spi->io_remap_addr)) {
 		err = PTR_ERR(amd_spi->io_remap_addr);
 		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 */
 	master->bus_num = 0;
 	master->num_chipselect = 4;
 	master->mode_bits = 0;
-	master->flags = SPI_MASTER_HALF_DUPLEX;
 	master->setup = amd_spi_master_setup;
 	master->transfer_one_message = amd_spi_master_transfer;
 
@@ -293,7 +385,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);
@@ -311,4 +406,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");