2021-06-30 12:06:06

by Shah, Nehal-bakulchandra

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

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


2021-06-30 12:06:41

by Shah, Nehal-bakulchandra

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

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 <[email protected]>
Reviewed-by: Shyam Sundar S K <[email protected]>
Reviewed-by: Liang Liang (Leo) <[email protected]>
Signed-off-by: Nehal Bakulchandra Shah <[email protected]>
---
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

2021-06-30 12:07:32

by Shah, Nehal-bakulchandra

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

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

Reviewed-by: Shyam Sundar S K <[email protected]>
Reviewed-by: Liang Liang <[email protected]>
Signed-off-by: Nehal Bakulchandra Shah <[email protected]>
---
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 <[email protected]>
+// Authors: Sanjay R Mehta <[email protected]>
+// Nehal Bakulchandra Shah <[email protected]>

#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 <[email protected]>");
+MODULE_AUTHOR("Nehal Bakulchandra Shah <[email protected]>");
MODULE_DESCRIPTION("AMD SPI Master Controller Driver");
--
2.25.1

2021-06-30 12:07:51

by Shah, Nehal-bakulchandra

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

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

Reviewed-by: Shyam Sundar S K <[email protected]>
Reviewed-by: Liang Liang <[email protected]>
Signed-off-by: Nehal Bakulchandra Shah <[email protected]>
---
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

2021-06-30 12:48:32

by Mark Brown

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

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.


Attachments:
(No filename) (778.00 B)
signature.asc (499.00 B)
Download all attachments

2021-06-30 14:46:31

by Shah, Nehal-bakulchandra

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

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



2021-06-30 16:01:25

by Mark Brown

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

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.


Attachments:
(No filename) (471.00 B)
signature.asc (499.00 B)
Download all attachments

2021-07-27 11:59:02

by Shah, Nehal-bakulchandra

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

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