This driver supports AMD based SPI Controller for AMD SOCs.This
driver supports SPI operations using FIFO mode of transfer.
Signed-off-by: Sanjay R Mehta <[email protected]>
---
MAINTAINERS | 5 +
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-amd.c | 341 ++++++++++++++++++++++++++++++++++++++++++++++++++
4 files changed, 353 insertions(+)
create mode 100644 drivers/spi/spi-amd.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6158a14..4455b92 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -870,6 +870,11 @@ F: drivers/gpu/drm/amd/include/vi_structs.h
F: drivers/gpu/drm/amd/include/v9_structs.h
F: include/uapi/linux/kfd_ioctl.h
+AMD SPI DRIVER
+M: Sanjay R Mehta <[email protected]>
+S: Maintained
+F: drivers/spi/spi-amd.c
+
AMD MP2 I2C DRIVER
M: Elie Morisse <[email protected]>
M: Nehal Shah <[email protected]>
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index d6ed0c3..1f4fa9a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -886,6 +886,12 @@ config SPI_ZYNQMP_GQSPI
help
Enables Xilinx GQSPI controller driver for Zynq UltraScale+ MPSoC.
+config SPI_AMD
+ tristate "AMD SPI controller"
+ depends on SPI_MASTER || COMPILE_TEST
+ help
+ Enables SPI controller driver for AMD SoC.
+
#
# Add new SPI master controllers in alphabetical order above this line
#
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 9b65ec5..ceb7249 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -123,6 +123,7 @@ obj-$(CONFIG_SPI_XLP) += spi-xlp.o
obj-$(CONFIG_SPI_XTENSA_XTFPGA) += spi-xtensa-xtfpga.o
obj-$(CONFIG_SPI_ZYNQ_QSPI) += spi-zynq-qspi.o
obj-$(CONFIG_SPI_ZYNQMP_GQSPI) += spi-zynqmp-gqspi.o
+obj-$(CONFIG_SPI_AMD) += spi-amd.o
# SPI slave protocol handlers
obj-$(CONFIG_SPI_SLAVE_TIME) += spi-slave-time.o
diff --git a/drivers/spi/spi-amd.c b/drivers/spi/spi-amd.c
new file mode 100644
index 0000000..69e51a8
--- /dev/null
+++ b/drivers/spi/spi-amd.c
@@ -0,0 +1,341 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
+/*
+ * AMD SPI controller driver
+ *
+ * Copyright (c) 2020, Advanced Micro Devices, Inc.
+ *
+ * Author: Sanjay R Mehta <[email protected]>
+ */
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi.h>
+
+#define DRIVER_NAME "amd_spi"
+
+#define AMD_SPI_CTRL0_REG 0x00
+#define AMD_SPI_EXEC_CMD BIT(16)
+#define AMD_SPI_FIFO_CLEAR BIT(20)
+#define AMD_SPI_BUSY BIT(31)
+
+#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_MEM_SIZE 200
+
+/* M_CMD OP codes for SPI */
+#define SPI_XFER_TX 1
+#define SPI_XFER_RX 2
+
+struct amd_spi {
+ void __iomem *io_remap_addr;
+ unsigned long io_base_addr;
+ u32 rom_addr;
+ struct spi_master *master;
+ u8 chip_select;
+};
+
+static inline u8 amd_spi_readreg8(struct spi_master *master, int idx)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+ return ioread8((u8 __iomem *)amd_spi->io_remap_addr + idx);
+}
+
+static inline void amd_spi_writereg8(struct spi_master *master, int idx,
+ u8 val)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+ iowrite8(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
+}
+
+static inline void amd_spi_setclear_reg8(struct spi_master *master, int idx,
+ u8 set, u8 clear)
+{
+ u8 tmp = amd_spi_readreg8(master, idx);
+
+ tmp = (tmp & ~clear) | set;
+ amd_spi_writereg8(master, idx, tmp);
+}
+
+static inline u32 amd_spi_readreg32(struct spi_master *master, int idx)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+ return ioread32((u8 __iomem *)amd_spi->io_remap_addr + idx);
+}
+
+static inline void amd_spi_writereg32(struct spi_master *master, int idx,
+ u32 val)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+ iowrite32(val, ((u8 __iomem *)amd_spi->io_remap_addr + idx));
+}
+
+static inline void amd_spi_setclear_reg32(struct spi_master *master, int idx,
+ u32 set, u32 clear)
+{
+ u32 tmp = amd_spi_readreg32(master, idx);
+
+ tmp = (tmp & ~clear) | set;
+ amd_spi_writereg32(master, idx, tmp);
+}
+
+static void amd_spi_select_chip(struct spi_master *master)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+ u8 chip_select = amd_spi->chip_select;
+
+ amd_spi_setclear_reg8(master, AMD_SPI_ALT_CS_REG, chip_select,
+ AMD_SPI_ALT_CS_MASK);
+}
+
+static void amd_spi_clear_fifo_ptr(struct spi_master *master)
+{
+ amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_FIFO_CLEAR,
+ AMD_SPI_FIFO_CLEAR);
+}
+
+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);
+}
+
+static inline void amd_spi_set_rx_count(struct spi_master *master,
+ u8 rx_count)
+{
+ amd_spi_setclear_reg8(master, AMD_SPI_RX_COUNT_REG, rx_count, 0xff);
+}
+
+static inline void amd_spi_set_tx_count(struct spi_master *master,
+ u8 tx_count)
+{
+ amd_spi_setclear_reg8(master, AMD_SPI_TX_COUNT_REG, tx_count, 0xff);
+}
+
+static void amd_spi_execute_opcode(struct spi_master *master)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+ bool spi_busy;
+
+ /* Set ExecuteOpCode bit in the CTRL0 register */
+ amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_EXEC_CMD,
+ AMD_SPI_EXEC_CMD);
+
+ /* 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;
+ while (spi_busy) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ schedule();
+ set_current_state(TASK_RUNNING);
+ spi_busy = (ioread32((u8 __iomem *)amd_spi->io_remap_addr +
+ AMD_SPI_CTRL0_REG) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
+ }
+}
+
+static int amd_spi_master_setup(struct spi_device *spi)
+{
+ struct spi_master *master = spi->master;
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+ amd_spi->chip_select = spi->chip_select;
+ amd_spi_select_chip(master);
+
+ return 0;
+}
+
+static int amd_spi_fifo_xfer(struct amd_spi *amd_spi,
+ struct spi_message *message)
+{
+ struct spi_master *master = amd_spi->master;
+ struct spi_transfer *xfer = NULL;
+ u8 cmd_opcode, opcode = 0;
+ u8 *buffer = NULL;
+ u32 m_cmd = 0;
+ u32 i = 0, saved_index = 0;
+ u32 tx_len = 0, rx_len = 0;
+
+ list_for_each_entry(xfer, &message->transfers,
+ transfer_list) {
+ if (xfer->rx_buf)
+ m_cmd = SPI_XFER_RX;
+ else if (xfer->tx_buf)
+ m_cmd = SPI_XFER_TX;
+
+ if (m_cmd & SPI_XFER_TX) {
+ buffer = (u8 *)xfer->tx_buf;
+
+ if (opcode != 1) {
+ tx_len = xfer->len - 1;
+ cmd_opcode = *(u8 *)xfer->tx_buf;
+ buffer++;
+ amd_spi_set_opcode(master, cmd_opcode);
+ opcode = 1;
+ } else {
+ /* Store no. of bytes to be sent into
+ * FIFO
+ */
+ tx_len = xfer->len;
+ }
+
+ /* Write data into the FIFO. */
+
+ for (i = 0; i < tx_len; i++) {
+ iowrite8(buffer[i],
+ ((u8 __iomem *)amd_spi->io_remap_addr +
+ AMD_SPI_FIFO_BASE +
+ i + saved_index));
+ }
+
+ amd_spi_set_tx_count(master,
+ tx_len + saved_index);
+
+ /*
+ * Saving the index, from where next
+ * spi_transfer's data will be stored in FIFO.
+ */
+ saved_index = i;
+
+ } else if (m_cmd & SPI_XFER_RX) {
+ /* Store no. of bytes to be received from
+ * FIFO
+ */
+ rx_len = xfer->len;
+ buffer = (u8 *)xfer->rx_buf;
+ }
+ }
+
+ amd_spi_set_rx_count(master, rx_len);
+ amd_spi_clear_fifo_ptr(master);
+
+ /* Execute command */
+ amd_spi_execute_opcode(master);
+
+ if (m_cmd & SPI_XFER_RX) {
+ /* Read data from FIFO to receive buffer */
+ for (i = 0; i < rx_len; i++)
+ buffer[i] = ioread8((u8 __iomem *)amd_spi->io_remap_addr
+ + AMD_SPI_FIFO_BASE
+ + tx_len + i);
+ }
+
+ /* Update statistics */
+ message->actual_length = tx_len + rx_len + 1;
+ /* complete the transaction */
+ message->status = 0;
+ spi_finalize_current_message(master);
+
+ return 0;
+}
+
+static int amd_spi_master_transfer(struct spi_master *master,
+ struct spi_message *msg)
+{
+ struct amd_spi *amd_spi = spi_master_get_devdata(master);
+
+ /*
+ * Extract spi_transfers from the spi message and
+ * program the controller.
+ */
+ amd_spi_fifo_xfer(amd_spi, msg);
+
+ return 0;
+}
+
+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 */
+ master = spi_alloc_master(dev, sizeof(struct amd_spi));
+ if (!master) {
+ dev_err(dev, "Error allocating SPI master\n");
+ return -ENOMEM;
+ }
+
+ amd_spi = spi_master_get_devdata(master);
+ amd_spi->master = master;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ amd_spi->io_remap_addr = devm_ioremap_resource(&pdev->dev, res);
+
+ if (!amd_spi->io_remap_addr) {
+ dev_err(dev, "ioremap of SPI registers failed\n");
+ err = -ENOMEM;
+ 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 = 0;
+ master->setup = amd_spi_master_setup;
+ master->transfer_one_message = amd_spi_master_transfer;
+
+ /* Register the controller with SPI framework */
+ err = spi_register_master(master);
+ if (err) {
+ dev_err(dev, "error registering SPI controller\n");
+ goto err_iounmap;
+ }
+ platform_set_drvdata(pdev, amd_spi);
+
+ return 0;
+
+err_iounmap:
+ iounmap(amd_spi->io_remap_addr);
+err_free_master:
+ spi_master_put(master);
+
+ return 0;
+}
+
+static int amd_spi_remove(struct platform_device *pdev)
+{
+ struct amd_spi *amd_spi = platform_get_drvdata(pdev);
+
+ spi_unregister_master(amd_spi->master);
+ spi_master_put(amd_spi->master);
+ platform_set_drvdata(pdev, NULL);
+
+ return 0;
+}
+
+static const struct acpi_device_id spi_acpi_match[] = {
+ { "AMDI0061", 0 },
+ {},
+};
+MODULE_DEVICE_TABLE(acpi, spi_acpi_match);
+
+static struct platform_driver amd_spi_driver = {
+ .driver = {
+ .name = "amd_spi",
+ .acpi_match_table = ACPI_PTR(spi_acpi_match),
+ },
+ .probe = amd_spi_probe,
+ .remove = amd_spi_remove,
+};
+
+module_platform_driver(amd_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Sanjay Mehta <[email protected]>");
+MODULE_DESCRIPTION("AMD SPI Master Controller Driver");
--
2.7.4
On Sun, Apr 12, 2020 at 02:28:31PM -0500, Sanjay R Mehta wrote:
> +++ b/drivers/spi/spi-amd.c
> @@ -0,0 +1,341 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
> +/*
> + * AMD SPI controller driver
> + *
Please make the entire comment a C++ one so things look more
intentional.
> +#define DRIVER_NAME "amd_spi"
This is unused.
> +/* M_CMD OP codes for SPI */
> +#define SPI_XFER_TX 1
> +#define SPI_XFER_RX 2
These constants should be namespaced, they're likely to collide with
generic additions.
> +static void amd_spi_execute_opcode(struct spi_master *master)
> +{
> + struct amd_spi *amd_spi = spi_master_get_devdata(master);
> + bool spi_busy;
> +
> + /* Set ExecuteOpCode bit in the CTRL0 register */
> + amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_EXEC_CMD,
> + AMD_SPI_EXEC_CMD);
> +
> + /* 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;
> + while (spi_busy) {
> + set_current_state(TASK_INTERRUPTIBLE);
> + schedule();
> + set_current_state(TASK_RUNNING);
> + spi_busy = (ioread32((u8 __iomem *)amd_spi->io_remap_addr +
> + AMD_SPI_CTRL0_REG) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
> + }
This is a weird way to busy wait - usually you'd use a cpu_relax()
rather than a schedule(). There's also no timeout here so we could busy
wait for ever if something goes wrong.
> +static int amd_spi_master_setup(struct spi_device *spi)
> +{
> + struct spi_master *master = spi->master;
> + struct amd_spi *amd_spi = spi_master_get_devdata(master);
> +
> + amd_spi->chip_select = spi->chip_select;
> + amd_spi_select_chip(master);
This looks like it will potentially affect devices other than the
current one. setup() may be called while other devices are active it
shouldn't do that.
> + } else if (m_cmd & SPI_XFER_RX) {
> + /* Store no. of bytes to be received from
> + * FIFO
> + */
> + rx_len = xfer->len;
> + buffer = (u8 *)xfer->rx_buf;
> + /* Read data from FIFO to receive buffer */
> + for (i = 0; i < rx_len; i++)
> + buffer[i] = ioread8((u8 __iomem *)amd_spi->io_remap_addr
> + + AMD_SPI_FIFO_BASE
> + + tx_len + i);
This will only work for messages with a single receive transfer, if
there are multiple transfers then you'll need to store multiple buffers
and their lengths.
> +static int amd_spi_master_transfer(struct spi_master *master,
> + struct spi_message *msg)
> +{
> + struct amd_spi *amd_spi = spi_master_get_devdata(master);
> +
> + /*
> + * Extract spi_transfers from the spi message and
> + * program the controller.
> + */
> + amd_spi_fifo_xfer(amd_spi, msg);
> +
> + return 0;
> +}
This function is completely redundant, just inline amd_spi_fifo_xfer().
It also ignores all errors which isn't great.
> + /* Initialize the spi_master fields */
> + master->bus_num = 0;
> + master->num_chipselect = 4;
> + master->mode_bits = 0;
> + master->flags = 0;
This device is single duplex so should flag that.
> + err = spi_register_master(master);
> + if (err) {
> + dev_err(dev, "error registering SPI controller\n");
> + goto err_iounmap;
It's best to print the error code to help people debug things.
On 4/14/2020 4:46 PM, Mark Brown wrote:
> On Sun, Apr 12, 2020 at 02:28:31PM -0500, Sanjay R Mehta wrote:
>
>> +++ b/drivers/spi/spi-amd.c
>> @@ -0,0 +1,341 @@
>> +// SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause
>> +/*
>> + * AMD SPI controller driver
>> + *
>
> Please make the entire comment a C++ one so things look more
> intentional.
>
>> +#define DRIVER_NAME "amd_spi"
>
> This is unused.
>
>> +/* M_CMD OP codes for SPI */
>> +#define SPI_XFER_TX 1
>> +#define SPI_XFER_RX 2
>
> These constants should be namespaced, they're likely to collide with
> generic additions.
>
>> +static void amd_spi_execute_opcode(struct spi_master *master)
>> +{
>> + struct amd_spi *amd_spi = spi_master_get_devdata(master);
>> + bool spi_busy;
>> +
>> + /* Set ExecuteOpCode bit in the CTRL0 register */
>> + amd_spi_setclear_reg32(master, AMD_SPI_CTRL0_REG, AMD_SPI_EXEC_CMD,
>> + AMD_SPI_EXEC_CMD);
>> +
>> + /* 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;
>> + while (spi_busy) {
>> + set_current_state(TASK_INTERRUPTIBLE);
>> + schedule();
>> + set_current_state(TASK_RUNNING);
>> + spi_busy = (ioread32((u8 __iomem *)amd_spi->io_remap_addr +
>> + AMD_SPI_CTRL0_REG) & AMD_SPI_BUSY) == AMD_SPI_BUSY;
>> + }
>
> This is a weird way to busy wait - usually you'd use a cpu_relax()
> rather than a schedule(). There's also no timeout here so we could busy
> wait for ever if something goes wrong.
>
>> +static int amd_spi_master_setup(struct spi_device *spi)
>> +{
>> + struct spi_master *master = spi->master;
>> + struct amd_spi *amd_spi = spi_master_get_devdata(master);
>> +
>> + amd_spi->chip_select = spi->chip_select;
>> + amd_spi_select_chip(master);
>
> This looks like it will potentially affect devices other than the
> current one. setup() may be called while other devices are active it
> shouldn't do that.
>
>> + } else if (m_cmd & SPI_XFER_RX) {
>> + /* Store no. of bytes to be received from
>> + * FIFO
>> + */
>> + rx_len = xfer->len;
>> + buffer = (u8 *)xfer->rx_buf;
>
>> + /* Read data from FIFO to receive buffer */
>> + for (i = 0; i < rx_len; i++)
>> + buffer[i] = ioread8((u8 __iomem *)amd_spi->io_remap_addr
>> + + AMD_SPI_FIFO_BASE
>> + + tx_len + i);
>
> This will only work for messages with a single receive transfer, if
> there are multiple transfers then you'll need to store multiple buffers
> and their lengths.
>
>> +static int amd_spi_master_transfer(struct spi_master *master,
>> + struct spi_message *msg)
>> +{
>> + struct amd_spi *amd_spi = spi_master_get_devdata(master);
>> +
>> + /*
>> + * Extract spi_transfers from the spi message and
>> + * program the controller.
>> + */
>> + amd_spi_fifo_xfer(amd_spi, msg);
>> +
>> + return 0;
>> +}
>
> This function is completely redundant, just inline amd_spi_fifo_xfer().
> It also ignores all errors which isn't great.
>
>> + /* Initialize the spi_master fields */
>> + master->bus_num = 0;
>> + master->num_chipselect = 4;
>> + master->mode_bits = 0;
>> + master->flags = 0;
>
> This device is single duplex so should flag that.
>
>> + err = spi_register_master(master);
>> + if (err) {
>> + dev_err(dev, "error registering SPI controller\n");
>> + goto err_iounmap;
>
> It's best to print the error code to help people debug things.
Thanks Mark for the feedback. Will make all the suggested changes.
>