Re: [PATCH 2/3] spi: spacemit: introduce SpacemiT K1 SPI controller driver

From: Alex Elder

Date: Thu Sep 18 2025 - 09:45:52 EST

On 9/18/25 7:41 AM, Yixun Lan wrote:

Hi Alex,

A few comments below which are mostly related to coding style or my personal taste

Great, thank you! I plan to adopt several of your suggestions, but
in a few cases I explain why the code looks the way it does. And
I ask for your response in some cases--I'm willing to change but
want to know what you think after you read what I say.

On 17:07 Wed 17 Sep , Alex Elder wrote:

This patch introduces the driver for the SPI controller found in the
SpacemiT K1 SoC. Currently the driver supports master mode only.
The SPI hardware implements RX and TX FIFOs, 32 entries each, and
supports both PIO and DMA mode transfers.

Signed-off-by: Alex Elder <elder@xxxxxxxxxxxx>
---
drivers/spi/Kconfig | 8 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-spacemit-k1.c | 985 ++++++++++++++++++++++++++++++++++
3 files changed, 994 insertions(+)
create mode 100644 drivers/spi/spi-spacemit-k1.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 82fa5eb3b8684..915a52407a85a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -577,6 +577,14 @@ config SPI_KSPI2
This driver can also be built as a module. If so, the module
will be called spi-kspi2.
+config SPI_SPACEMIT_K1
keep it sorted? seems you adjust the name but forget to reorder it

Could be. It is my intention to keep this and many other things
sorted. I'll fix this.

+ tristate "K1 SPI Controller"
+ depends on ARCH_SPACEMIT || COMPILE_TEST
+ depends on OF
+ default ARCH_SPACEMIT
+ help
+ Enable support for the SpacemiT K1 SPI controller.
+
config SPI_LM70_LLP
tristate "Parallel port adapter for LM70 eval board (DEVELOPMENT)"
depends on PARPORT
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index eefaeca097456..29d55eeb9abb4 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_SPI_INGENIC) += spi-ingenic.o
obj-$(CONFIG_SPI_INTEL) += spi-intel.o
obj-$(CONFIG_SPI_INTEL_PCI) += spi-intel-pci.o
obj-$(CONFIG_SPI_INTEL_PLATFORM) += spi-intel-platform.o
+obj-$(CONFIG_SPI_SPACEMIT_K1) += spi-spacemit-k1.o

same, sort

Yes I'll put this in sorted order too, thanks for noticing.

obj-$(CONFIG_SPI_LANTIQ_SSC) += spi-lantiq-ssc.o
obj-$(CONFIG_SPI_JCORE) += spi-jcore.o
obj-$(CONFIG_SPI_KSPI2) += spi-kspi2.o
diff --git a/drivers/spi/spi-spacemit-k1.c b/drivers/spi/spi-spacemit-k1.c
new file mode 100644
index 0000000000000..6edf75efe7c68
--- /dev/null
+++ b/drivers/spi/spi-spacemit-k1.c
@@ -0,0 +1,985 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Support for SpacemiT K1 SPI controller
+ *
+ * Copyright (C) 2025 by RISCstar Solutions Corporation. All rights reserved.
+ * Copyright (c) 2023, spacemit Corporation.

keep "(C)" consistent? capitalized
s/spacemit/SpacemiT/, or I'd suggest to keep it align with vendor-prefixes.yaml..

First, the (C) is verbatim what our company lawyer said to use.

Second, I did not want to change anything on the copyright that
was originally provided by SpacemiT. I agree with you (on both
points, actually), but the SpacemiT one is not my copyright to
modify.

So I intend to keep both of these as-is.

+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/dmaengine.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/scatterlist.h>
+#include <linux/sizes.h>
+#include <linux/spi/spi.h>
+#include <linux/units.h>
+
+#include "internals.h"
+
+/* This is used if the spi-max-frequency property is not present */
+#define K1_SPI_MAX_SPEED_HZ 25600000
+
+/* DMA constraints */
+#define K1_SPI_DMA_ALIGNMENT 64
+#define K1_SPI_MAX_DMA_LEN SZ_512K
+
+/* SpacemiT K1 SPI Registers */
+
+/* SSP Top Control Register */
+#define SSP_TOP_CTRL 0x00
+#define TOP_SSE BIT(0) /* Enable port */
+#define TOP_FRF_MASK GENMASK(2, 1) /* Frame format */
+#define TOP_FRF_MOTOROLA 0 /* Motorola SPI */
+#define TOP_DSS_MASK GENMASK(9, 5) /* Data size (1-32) */
+#define TOP_SPO BIT(10) /* Polarity: 0=low */
+#define TOP_SPH BIT(11) /* Half-cycle phase */
+#define TOP_LBM BIT(12) /* Loopback mode */
+#define TOP_TRAIL BIT(13) /* Trailing bytes */
+#define TOP_HOLD_FRAME_LOW BIT(14) /* Master mode */
+

it's hard to distinguish the register vs BIT definition,
how about putting extra space to slightly ajdust the arrange,
something like:

#define SSP_TOP_CTRL 0x00
#define TOP_SSE BIT(0) /* Enable port */
#define TOP_FRF_MASK GENMASK(2, 1) /* Frame format */

Both of the above two are fields within the SSP_TOP_CTRL register.

...

The formatting convention I used is (roughly):
- One tab: register offset
- Two tabs: bit/multi-bit field within register
- Three tabs: values relevant to a field

The TOP_FRF_MOTOROLA symbol is a meaningful value that can be
assigned in the TOP_FRF_MASK, in the SSP_TOP_CTRL register.

In e-mail, the formatting doesn't all look as it should.

+/* SSP FIFO Control Register */
+#define SSP_FIFO_CTRL 0x04
+#define FIFO_TFT_MASK GENMASK(4, 0) /* TX FIFO threshold */
+#define FIFO_RFT_MASK GENMASK(9, 5) /* RX FIFO threshold */
+#define FIFO_TSRE BIT(10) /* TX service request */
+#define FIFO_RSRE BIT(11) /* RX service request */
+
+/* SSP Interrupt Enable Register */
+#define SSP_INT_EN 0x08
+#define SSP_INT_EN_TINTE BIT(1) /* RX timeout */
+#define SSP_INT_EN_RIE BIT(2) /* RX FIFO */
+#define SSP_INT_EN_TIE BIT(3) /* TX FIFO */
+#define SSP_INT_EN_RIM BIT(4) /* RX FIFO overrun */
+#define SSP_INT_EN_TIM BIT(5) /* TX FIFO underrun */
+
+/* SSP Time Out Register */
+#define SSP_TIMEOUT 0x0c
+#define SSP_TIMEOUT_MASK GENMASK(23, 0)
+
+/* SSP Data Register */
+#define SSP_DATAR 0x10
+
+/* SSP Status Register */
+#define SSP_STATUS 0x14
+#define SSP_STATUS_BSY BIT(0) /* SPI/I2S busy */
+#define SSP_STATUS_TNF BIT(6) /* TX FIFO not full */
+#define SSP_STATUS_TFL GENMASK(11, 7) /* TX FIFO level */
+#define SSP_STATUS_TUR BIT(12) /* TX FIFO underrun */
+#define SSP_STATUS_RNE BIT(14) /* RX FIFO not empty */
+#define SSP_STATUS_RFL GENMASK(19, 15) /* RX FIFO level */
+#define SSP_STATUS_ROR BIT(20) /* RX FIFO overrun */
+
+/* The FIFO sizes and thresholds are the same for RX and TX */
+#define K1_SPI_FIFO_SIZE 32
+#define K1_SPI_THRESH (K1_SPI_FIFO_SIZE / 2)
+
+struct k1_spi_io {
+ enum dma_data_direction dir;
+ struct dma_chan *chan;
+ void *buf;
+ unsigned int resid;
+ u32 nents;
+ struct sg_table sgt;

can you reorder the struct members to make it slightly more neat?
I know people tend to keep struct first and group same functions together..

I ordered them first logically, and second based on alignment.
- The direction and channel are directly related to DMA
- The buffer is and its residual byte count are related
- The nents is the number of entries mapped in the SGT;
I put it before the SGT because it and the residual
count above fit within 8 bytes

If you feel strongly the readability is too bad I can change
the order to be Christmas tree like, or something. Please
advise.

+};
+
+struct k1_spi_driver_data {
+ struct spi_controller *controller;
+ struct device *dev;
+ void __iomem *ioaddr;

I'd prefer s/ioaddr/base/

Actually I prefer that too. This came from the
vendor and I never changed it. Good suggestion,
I will change it.

+ unsigned long bus_rate;
+ struct clk *clk;
+ unsigned long rate;
+ u32 rx_timeout;
+ int irq;
+
+ struct k1_spi_io rx;
+ struct k1_spi_io tx;
+
+ void *dummy; /* DMA disabled if NULL */

this variable really make me confused, and the comment here hardly helps,
can you think of a better name? will it be used in DMA mode or also in PIO mode?

The name "dummy" matches a similar field in the SPI core code.
It is a buffer used (only) for DMA transfers when the target
(in or out) is a NULL pointer.

The comment is a statement of how a null pointer is interpreted
(implemented by k1_spi_dma_enabled()).

I'm OK with the name, but if you have a different one that you
would not find confusing, please let me know.

+ u32 data_reg_addr; /* DMA address of the data register */

s/data_reg_addr/ssp_data/? I just feel uncomfortable with redundant 'reg_addr'

My convention is normally "virt" or maybe "base" to represent
a virtual address, and "addr" to represent I/O addresses.

This symbol represents the physical address that underlies the
"SSP Data Register", which fills the TX FIFO when written and
drains the RX FIFO when read.

How about "data_addr"? I know you wouldn't like "reg_addr".

+
+ struct spi_message *message; /* Current message */
+
+ /* Current transfer information; not valid if message is null */
+ unsigned int len;
+ u32 bytes; /* Bytes used for bits_per_word */
+ bool dma_mapped;
+ struct completion completion; /* Transfer completion */
+};
+
+static bool k1_spi_dma_enabled(struct k1_spi_driver_data *drv_data)
+{
+ return !!drv_data->dummy;
+}
+
+static bool k1_spi_map_dma_buffer(struct k1_spi_io *io, size_t len, void *dummy)
+{
+ struct device *dmadev = io->chan->device->dev;
+ unsigned int nents = DIV_ROUND_UP(len, SZ_2K);
+ struct sg_table *sgt = &io->sgt;
+ void *bufp = io->buf ? : dummy;

simply s/bufp/buf/, embed pointer info into variable doesn't really help
let's leave compiler to check

I called it "buf" originally and changed it to "bufp" because it
actually advances through the buffer (either the one in the IO
structure or the dummy buffer) in the loop.

I don't understand your comment about the compiler.

+ struct scatterlist *sg;
+ unsigned int i;
+
+ if (nents != sgt->nents) {
+ sg_free_table(sgt);
+ if (sg_alloc_table(sgt, nents, GFP_KERNEL))
+ return false;
+ }
+
+ for_each_sg(sgt->sgl, sg, nents, i) {
+ size_t bytes = min_t(size_t, len, SZ_2K);
+
+ sg_set_buf(sg, bufp, bytes);
+ if (bufp != dummy)
+ bufp += bytes;
+ len -= bytes;
+ }
+ io->nents = dma_map_sg(dmadev, sgt->sgl, nents, io->dir);
+
+ return !!io->nents;
+}

. . .

+static bool k1_spi_transfer_start_dma(struct k1_spi_driver_data *drv_data)
+{
+ struct dma_async_tx_descriptor *rx_desc;
+ struct dma_async_tx_descriptor *tx_desc;
+ struct device *dev = drv_data->dev;
+ void __iomem *virt;

s/virt/reg/, more natural

Not more natural to me. It is a virtual address mapped to
I/O memory.

+ u32 val;
+
+ rx_desc = k1_spi_prepare_dma_io(drv_data, &drv_data->rx);
+ if (!rx_desc) {
+ dev_err(dev, "failed to get DMA RX descriptor\n");
+ return false;
+ }
+
+ tx_desc = k1_spi_prepare_dma_io(drv_data, &drv_data->tx);
+ if (!tx_desc) {
+ dev_err(dev, "failed to get DMA TX descriptor\n");
+ return false;
+ }
+
+ virt = drv_data->ioaddr + SSP_TOP_CTRL;
+ val = readl(virt);
+ val |= TOP_TRAIL; /* Trailing bytes handled by DMA */
+ writel(val, virt);

I'd prefer to do like this, it's more easy for people to grep..
val = readl(drv_data->ioaddr + SSP_TOP_CTRL) | TOP_TRAIL;
writel(val, drv_data->ioaddr + SSP_TOP_CTRL);

This is an idiom I use to make it very clear that:
- The address being read is exactly the same as what's being
written
- The value read is being updated with bits/values

I find that putting the "| TOP_TRAIL" on the same line as the
readl() call obscures things a bit. Like my eye doesn't notice
it as readiliy somehow...

Yours is a pure coding style comment. There are two pieces, and
I'd like you to tell me how strongly you feel about them:
- Using virt to grab the address being written and read (versus
just using drv_data->ioaddr + SSP_TOP_CTRL twice)
- Put the "| TOP_TRAIL" on the same line as the readl() (versus
having that be assigned on a separate line).
To me, the second one is more important than the first.

Let me know how strongly you feel about these and I'll update
my convention througout.

+
+ virt = drv_data->ioaddr + SSP_FIFO_CTRL;
+ val = readl(virt);
+ val |= FIFO_TSRE | FIFO_RSRE;
+ writel(val, virt);
+
+ /* When RX is complete we also know TX has completed */
+ rx_desc->callback = k1_spi_callback;
+ rx_desc->callback_param = &drv_data->completion;
+
+ dmaengine_submit(rx_desc);
+ dmaengine_submit(tx_desc);
+
+ dma_async_issue_pending(drv_data->rx.chan);
+ dma_async_issue_pending(drv_data->tx.chan);
+
+ return true;
+}

. . .

+static bool k1_spi_read(struct k1_spi_driver_data *drv_data)
+{
+ struct k1_spi_io *rx = &drv_data->rx;
+ unsigned int count;
+ u32 val;
+
+ if (!rx->resid)
+ return true; /* Nothing more to receive */
+
+ /* We'll read as many slots in the FIFO as there are available */
+ val = readl(drv_data->ioaddr + SSP_STATUS);
+ /* The number of open slots is one more than what's in the field */
+ count = FIELD_GET(SSP_STATUS_RFL, val) + 1;
+
+ /* A full FIFO count means the FIFO is either full or empty */
+
+ if (count == K1_SPI_FIFO_SIZE)
+ if (!(val & SSP_STATUS_RNE))
+ return false; /* Nothing available to read */
+
+ count = min(count, rx->resid);
+ while (count--)
+ k1_spi_read_word(drv_data);
+
+ return !rx->resid;
+}
+
+static void k1_spi_write_word(struct k1_spi_driver_data *drv_data)
+{
+ struct k1_spi_io *tx = &drv_data->tx;
+ u32 bytes;
+ u32 val;

u32 val = 0;

There is no need to initialize val. The value of bytes
will only be 1, 2, or 4.

But I suppose the compiler (or a static analyzer) might
complain, so I'll do as you suggest...

+
+ bytes = drv_data->bytes;
+ if (tx->buf) {
+ if (bytes == 1)
+ val = *(u8 *)tx->buf;
+ else if (bytes == 2)
+ val = *(u16 *)tx->buf;
+ else if (bytes == 4)
+ val = *(u32 *)tx->buf;
+ tx->buf += bytes;

..

+ } else {
+ val = 0; /* Null writer; write 1, 2, or 4 zero bytes */
+ }

for the logic, assign val = 0 at first place? then you can kill this 'else'

Yes I'll do that (drop the else block) too.

+
+ tx->resid -= bytes;
+ writel(val, drv_data->ioaddr + SSP_DATAR);
+}
+
+static bool k1_spi_write(struct k1_spi_driver_data *drv_data)
+{
+ struct k1_spi_io *tx = &drv_data->tx;
+ unsigned int count;
+ u32 val;
+
+ if (!tx->resid)
+ return true; /* Nothing more to send */
+
+ /* See how many slots in the TX FIFO are available */
+ val = readl(drv_data->ioaddr + SSP_STATUS);
+ count = FIELD_GET(SSP_STATUS_TFL, val);
+
+ /* A zero count means the FIFO is either full or empty */
+ if (!count) {
+ if (val & SSP_STATUS_TNF)
+ count = K1_SPI_FIFO_SIZE;
+ else
+ return false; /* No room in the FIFO */
+ }
+
+ /*
+ * Limit how much we try to send at a time, to reduce the
+ * chance the other side can overrun our RX FIFO.
+ */
+ count = min3(count, K1_SPI_THRESH, tx->resid);
+ while (count--)
+ k1_spi_write_word(drv_data);
+
+ return !tx->resid;
+}

. . .

+static void k1_spi_host_init(struct k1_spi_driver_data *drv_data, int id)
+{
+ struct device_node *np = dev_of_node(drv_data->dev);
+ struct spi_controller *host = drv_data->controller;
+ struct device *dev = drv_data->dev;
+ u32 bus_num;
+ int ret;
+
+ host->dev.of_node = np;
+ host->dev.parent = drv_data->dev;
+ if (!of_property_read_u32(np, "spacemit,k1-ssp-id", &bus_num))
+ host->bus_num = bus_num;
+ else
+ host->bus_num = id;
+ host->mode_bits = SPI_CPOL | SPI_CPHA | SPI_LOOP;
+ host->bits_per_word_mask = SPI_BPW_RANGE_MASK(4, 32);
+ host->num_chipselect = 1;
+
+ if (k1_spi_dma_enabled(drv_data))
+ host->dma_alignment = K1_SPI_DMA_ALIGNMENT;
+ host->cleanup = k1_spi_cleanup;
+ host->setup = k1_spi_setup;
+ host->transfer_one_message = k1_spi_transfer_one_message;
+

..

+ ret = of_property_read_u32(np, "spi-max-frequency", &host->max_speed_hz);
+ if (ret < 0) {
+ if (ret != -EINVAL)
+ dev_warn(dev, "bad spi-max-frequency, using %u\n",
+ K1_SPI_MAX_SPEED_HZ);

the err msg does't really usefull..

You mean there should be no error message, or that it
should say something else?

+ host->max_speed_hz = K1_SPI_MAX_SPEED_HZ;
+ }

so in any case, there will be an assignment, I'd rather simplify it as

if (of_property_read_u32(np, "spi-max-frequency", &host->max_speed_hz);
host->max_speed_hz = K1_SPI_MAX_SPEED_HZ;

I add the warning so it's clear we're using something different
than is specified in the DT.

I really appreciate your careful review.

-Alex

+}
+
+/* Set our registers to a known initial state */
+static void
+k1_spi_register_reset(struct k1_spi_driver_data *drv_data, bool initial)
+{
+ u32 val = 0;
+
+ writel(0, drv_data->ioaddr + SSP_TOP_CTRL);
+
+ if (initial) {
+ /*
+ * The TX and RX FIFO thresholds are the same no matter
+ * what the speed or bits per word, so we can just set
+ * them once. The thresholds are one more than the values
+ * in the register.
+ */
+ val = FIELD_PREP(FIFO_RFT_MASK, K1_SPI_THRESH - 1);
+ val |= FIELD_PREP(FIFO_TFT_MASK, K1_SPI_THRESH - 1);
+ }
+ writel(val, drv_data->ioaddr + SSP_FIFO_CTRL);
+
+ writel(0, drv_data->ioaddr + SSP_INT_EN);
+ writel(0, drv_data->ioaddr + SSP_TIMEOUT);
+
+ /* Clear any pending interrupt conditions */
+ val = readl(drv_data->ioaddr + SSP_STATUS);
+ writel(val, drv_data->ioaddr + SSP_STATUS);
+}
+
+static irqreturn_t k1_spi_ssp_isr(int irq, void *dev_id)
+{
+ struct k1_spi_driver_data *drv_data = dev_id;
+ void __iomem *virt;
+ bool rx_done;
+ bool tx_done;
+ u32 val;
+
+ /* Get status and clear pending interrupts */
+ virt = drv_data->ioaddr + SSP_STATUS;
+ val = readl(virt);
+ writel(val, virt);
+
+ if (!drv_data->message)
+ return IRQ_NONE;
+
+ /* Check for a TX underrun or RX underrun first */
+ if (val & (SSP_STATUS_TUR | SSP_STATUS_ROR)) {
+ /* Disable all interrupts on error */
+ writel(0, drv_data->ioaddr + SSP_INT_EN);
+
+ drv_data->message->status = -EIO;
+ complete(&drv_data->completion);
+
+ return IRQ_HANDLED;
+ }
+
+ /* Drain the RX FIFO first, then transmit what we can */
+ rx_done = k1_spi_read(drv_data);
+ tx_done = k1_spi_write(drv_data);
+
+ /* Disable interrupts if we're done transferring either direction */
+ if (rx_done || tx_done) {
+ virt = drv_data->ioaddr + SSP_INT_EN;
+
+ /* If both are done, disable all interrupts */
+ if (rx_done && tx_done) {
+ val = 0;
+ } else {
+ val = readl(virt);
+ if (rx_done)
+ val &= ~(SSP_INT_EN_TINTE | SSP_INT_EN_RIE);
+ if (tx_done)
+ val &= ~SSP_INT_EN_TIE;
+ }
+ writel(val, virt);
+ }
+
+ if (rx_done && tx_done)
+ complete(&drv_data->completion);
+
+ return IRQ_HANDLED;
+}
+
+static int k1_spi_probe(struct platform_device *pdev)
+{
+ struct k1_spi_driver_data *drv_data;
+ struct device *dev = &pdev->dev;
+ struct reset_control *reset;
+ struct spi_controller *host;
+ struct resource *iores;
+ struct clk *clk_bus;
+ int ret;
+
+ host = devm_spi_alloc_host(dev, sizeof(*drv_data));
+ if (!host)
+ return -ENOMEM;
+ drv_data = spi_controller_get_devdata(host);
+ drv_data->controller = host;
+ platform_set_drvdata(pdev, drv_data);
+ drv_data->dev = dev;
+ init_completion(&drv_data->completion);
+
+ drv_data->ioaddr = devm_platform_get_and_ioremap_resource(pdev, 0,
+ &iores);
+ if (IS_ERR(drv_data->ioaddr))
+ return dev_err_probe(dev, PTR_ERR(drv_data->ioaddr),
+ "error mapping memory\n");
+ drv_data->data_reg_addr = iores->start + SSP_DATAR;
+
+ ret = devm_k1_spi_dma_setup(drv_data);
+ if (ret)
+ return dev_err_probe(dev, ret, "error setting up DMA\n");
+
+ k1_spi_host_init(drv_data, pdev->id);
+
+ clk_bus = devm_clk_get_enabled(dev, "bus");
+ if (IS_ERR(clk_bus))
+ return dev_err_probe(dev, PTR_ERR(clk_bus),
+ "error getting/enabling bus clock\n");
+ drv_data->bus_rate = clk_get_rate(clk_bus);
+
+ drv_data->clk = devm_clk_get_enabled(dev, "core");
+ if (IS_ERR(drv_data->clk))
+ return dev_err_probe(dev, PTR_ERR(drv_data->clk),
+ "error getting/enabling core clock\n");
+
+ reset = devm_reset_control_get_exclusive_deasserted(dev, NULL);
+ if (IS_ERR(reset))
+ return dev_err_probe(dev, PTR_ERR(reset),
+ "error getting/deasserting reset\n");
+
+ k1_spi_register_reset(drv_data, true);
+
+ drv_data->irq = platform_get_irq(pdev, 0);
+ if (drv_data->irq < 0)
+ return dev_err_probe(dev, drv_data->irq, "error getting IRQ\n");
+
+ ret = devm_request_irq(dev, drv_data->irq, k1_spi_ssp_isr,
+ IRQF_SHARED, dev_name(dev), drv_data);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "error requesting IRQ\n");
+
+ ret = devm_spi_register_controller(dev, host);
+ if (ret)
+ dev_err(dev, "error registering controller\n");
+
+ return ret;
+}
+
+static void k1_spi_remove(struct platform_device *pdev)
+{
+ struct k1_spi_driver_data *drv_data = platform_get_drvdata(pdev);
+
+ k1_spi_register_reset(drv_data, false);
+}
+
+static struct platform_driver k1_spi_driver = {
+ .driver = {
+ .name = "k1x-spi",
+ .of_match_table = k1_spi_dt_ids,
+ },
+ .probe = k1_spi_probe,
+ .remove = k1_spi_remove,
+};
+
+module_platform_driver(k1_spi_driver);
+
+MODULE_DESCRIPTION("SpacemiT K1 SPI controller driver");
+MODULE_LICENSE("GPL");
--
2.48.1