lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250918124120-GYA1273705@gentoo.org>
Date: Thu, 18 Sep 2025 20:41:20 +0800
From: Yixun Lan <dlan@...too.org>
To: Alex Elder <elder@...cstar.com>
Cc: broonie@...nel.org, robh@...nel.org, krzk+dt@...nel.org,
	conor+dt@...nel.org, linux-spi@...r.kernel.org,
	devicetree@...r.kernel.org, paul.walmsley@...ive.com,
	palmer@...belt.com, aou@...s.berkeley.edu, alex@...ti.fr,
	p.zabel@...gutronix.de, spacemit@...ts.linux.dev,
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] spi: spacemit: introduce SpacemiT K1 SPI controller
 driver

Hi Alex,

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

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@...cstar.com>
> ---
>  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

> +	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
>  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..

> + */
> +
> +#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 */
...

> +/* 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..

> +};
> +
> +struct k1_spi_driver_data {
> +	struct spi_controller *controller;
> +	struct device *dev;
> +	void __iomem *ioaddr;
I'd prefer s/ioaddr/base/
> +	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?

> +	u32 data_reg_addr;		/* DMA address of the data register */
s/data_reg_addr/ssp_data/? I just feel uncomfortable with redundant '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
> +	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 void k1_spi_unmap_dma_buffer(struct k1_spi_io *io)
> +{
> +	struct sg_table *sgt = &io->sgt;
> +
> +	dma_unmap_sg(io->chan->device->dev, sgt->sgl, io->nents, io->dir);
> +	io->nents = 0;
> +}
> +
> +static bool k1_spi_map_dma_buffers(struct k1_spi_driver_data *drv_data)
> +{
> +	u32 dma_burst_size;
> +	void *dummy;
> +
> +	if (!k1_spi_dma_enabled(drv_data))
> +		return false;
> +
> +	dma_burst_size = K1_SPI_THRESH * drv_data->bytes;
> +
> +	/* Don't bother with DMA if we can't do even a single burst */
> +	if (drv_data->len < dma_burst_size)
> +		return false;
> +
> +	/* We won't use DMA if the transfer is too big, either */
> +	if (drv_data->len > K1_SPI_MAX_DMA_LEN)
> +		return false;
> +
> +	/* Map both directions for DMA; if either fails, we'll use PIO */
> +	dummy = drv_data->dummy;
> +	if (!k1_spi_map_dma_buffer(&drv_data->rx, drv_data->len, dummy))
> +		return false;
> +
> +	if (k1_spi_map_dma_buffer(&drv_data->tx, drv_data->len, dummy))
> +		return true;		/* Success! */
> +
> +	/* Failed to map the RX buffer; undo the TX mapping */
> +	k1_spi_unmap_dma_buffer(&drv_data->rx);
> +
> +	return false;
> +}
> +
> +static struct dma_async_tx_descriptor *
> +k1_spi_prepare_dma_io(struct k1_spi_driver_data *drv_data, struct k1_spi_io *io)
> +{
> +	u32 addr = drv_data->data_reg_addr;
> +	struct dma_slave_config cfg = { };
> +	enum dma_transfer_direction dir;
> +	enum dma_slave_buswidth width;
> +	u32 dma_burst_size;
> +	int ret;
> +
> +	dir = io->dir == DMA_TO_DEVICE ? DMA_MEM_TO_DEV
> +				       : DMA_DEV_TO_MEM;
> +
> +	width = drv_data->bytes == 1 ? DMA_SLAVE_BUSWIDTH_1_BYTE :
> +		drv_data->bytes == 2 ? DMA_SLAVE_BUSWIDTH_2_BYTES
> +				     : DMA_SLAVE_BUSWIDTH_4_BYTES;
> +
> +	dma_burst_size = K1_SPI_THRESH * drv_data->bytes;
> +
> +	cfg.direction = dir;
> +	if (dir == DMA_MEM_TO_DEV) {
> +		cfg.dst_addr = addr;
> +		cfg.dst_addr_width = width;
> +		cfg.dst_maxburst = dma_burst_size;
> +	} else {
> +		cfg.src_addr = addr;
> +		cfg.src_addr_width = width;
> +		cfg.src_maxburst = dma_burst_size;
> +	}
> +
> +	ret = dmaengine_slave_config(io->chan, &cfg);
> +	if (ret)
> +		return NULL;
> +
> +	return dmaengine_prep_slave_sg(io->chan, io->sgt.sgl, io->nents, dir,
> +				       DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> +}
> +
> +/* DMA completion callback */
> +static void k1_spi_callback(void *data)
> +{
> +	complete(data);
> +}
> +
> +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
> +	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);

> +
> +	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 void k1_spi_transfer_end_dma(struct k1_spi_driver_data *drv_data)
> +{
> +	void __iomem *virt;
> +	u32 val;
> +
> +	virt = drv_data->ioaddr + SSP_FIFO_CTRL;
> +	val = readl(virt);
> +	val &= ~(FIFO_TSRE | FIFO_RSRE);
> +	writel(val, virt);
> +
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val &= ~TOP_TRAIL;		/* Trailing bytes handled by the CPU */
> +	writel(val, virt);
> +
> +	/* Signal an error if an RX overflow or TX underflow occurred */
> +	val = readl(drv_data->ioaddr + SSP_STATUS);
> +	if (val & (SSP_STATUS_TUR | SSP_STATUS_ROR))
> +		drv_data->message->status = -EIO;
> +}
> +
> +/* Discard any data in the RX FIFO */
> +static void k1_spi_flush(struct k1_spi_driver_data *drv_data)
> +{
> +	u32 val = readl(drv_data->ioaddr + SSP_STATUS);
> +	u32 count;
> +
> +	/* If there's nothing in the FIFO, we're done */
> +	if (!(val & SSP_STATUS_RNE))
> +		return;
> +
> +	/* Read and discard what's there (one more than what the field says) */
> +	count = FIELD_GET(SSP_STATUS_RFL, val) + 1;
> +	do
> +		(void)readl(drv_data->ioaddr + SSP_DATAR);
> +	while (--count);
> +}
> +
> +/* Set the transfer speed; the SPI core code ensures it is supported */
> +static bool k1_spi_set_speed(struct k1_spi_driver_data *drv_data, u32 rate)
> +{
> +	struct clk *clk = drv_data->clk;
> +	u64 nsec_per_word;
> +	u64 bus_ticks;
> +
> +	if (clk_set_rate(clk, rate))
> +		return false;
> +
> +	drv_data->rate = clk_get_rate(clk);
> +
> +	/*
> +	 * Compute the RX FIFO inactivity timeout value that should be used.
> +	 * The inactivity timer restarts with each word that lands in the
> +	 * FIFO.  If two or more "word transfer times" pass without any new
> +	 * data in the RX FIFO, we might as well read what's there.
> +	 *
> +	 * The rate at which words land in the FIFO is determined by the
> +	 * word size and the transfer rate.  One bit is transferred per
> +	 * clock tick, and 8 (or 16 or 32) bits are transferred per word.
> +	 *
> +	 * So we can get word transfer time (in nanoseconds) from:
> +	 *   nsec_per_tick = NANOHZ_PER_HZ / drv_data->rate;
> +	 *   ticks_per_word = BITS_PER_BYTE * drv_data->bytes;
> +	 * We do the divide last for better accuracy.
> +	 */
> +	nsec_per_word = NANOHZ_PER_HZ * BITS_PER_BYTE * drv_data->bytes;
> +	nsec_per_word = DIV_ROUND_UP(nsec_per_word, drv_data->rate);
> +
> +	/*
> +	 * The timeout (which we'll set to three word transfer times) is
> +	 * expressed as a number of APB clock ticks.
> +	 *   bus_ticks = 3 * nsec * (drv_data->bus_rate / NANOHZ_PER_HZ)
> +	 */
> +	bus_ticks = 3 * nsec_per_word * drv_data->bus_rate;
> +	drv_data->rx_timeout = DIV_ROUND_UP(bus_ticks, NANOHZ_PER_HZ);
> +
> +	return true;
> +}
> +
> +static void k1_spi_read_word(struct k1_spi_driver_data *drv_data)
> +{
> +	struct k1_spi_io *rx = &drv_data->rx;
> +	u32 bytes = drv_data->bytes;
> +	u32 val;
> +
> +	val = readl(drv_data->ioaddr + SSP_DATAR);
> +	rx->resid -= bytes;
> +
> +	if (!rx->buf)
> +		return;	/* Null reader: discard the data */
> +
> +	if (bytes == 1)
> +		*(u8 *)rx->buf = val;
> +	else if (bytes == 1)
> +		*(u16 *)rx->buf = val;
> +	else
> +		*(u32 *)rx->buf = val;
> +
> +	rx->buf += bytes;
> +}
> +
> +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;
> +
> +	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'
> +
> +	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 bool k1_spi_transfer_start(struct k1_spi_driver_data *drv_data,
> +				  struct spi_transfer *transfer)
> +{
> +	void __iomem *virt;
> +	u32 val;
> +
> +	/* Bits per word can change on a per-transfer basis */
> +	drv_data->bytes = spi_bpw_to_bytes(transfer->bits_per_word);
> +
> +	/* Each transfer can also specify a different rate */
> +	if (!k1_spi_set_speed(drv_data, transfer->speed_hz)) {
> +		dev_err(drv_data->dev, "failed to set transfer speed\n");
> +		return false;
> +	}
> +
> +	k1_spi_flush(drv_data);
> +
> +	/* Record the current transfer information */
> +	drv_data->rx.buf = transfer->rx_buf;
> +	drv_data->rx.resid = transfer->len;
> +	drv_data->tx.buf = (void *)transfer->tx_buf;
> +	drv_data->tx.resid = transfer->len;
> +	drv_data->len = transfer->len;
> +
> +	drv_data->dma_mapped = k1_spi_map_dma_buffers(drv_data);
> +
> +	/* Set the RX timeout period (required for both DMA and PIO) */
> +	val = FIELD_PREP(SSP_TIMEOUT_MASK, drv_data->rx_timeout);
> +	writel(val, drv_data->ioaddr + SSP_TIMEOUT);
> +
> +	/* Data size is one more than the value in the TOP_DSS field */
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val |= FIELD_PREP(TOP_DSS_MASK, transfer->bits_per_word - 1);
> +	writel(val, virt);
> +
> +	/* Clear any existing interrupt conditions */
> +	val = readl(drv_data->ioaddr + SSP_STATUS);
> +	writel(val, drv_data->ioaddr + SSP_STATUS);
> +
> +	/* Enable the hardware */
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val |= TOP_SSE;
> +	writel(val, virt);
> +
> +	/* DMA transfers are programmmed, then initiated */
> +	if (drv_data->dma_mapped)
> +		return k1_spi_transfer_start_dma(drv_data);
> +
> +	/*
> +	 * For PIO transfers, interrupts will cause words to get
> +	 * transferred.  The interrupts will get disabled as the
> +	 * transfer completes.  We'll write what we can to get
> +	 * things started.
> +	 */
> +	(void)k1_spi_write(drv_data);
> +
> +	val = SSP_INT_EN_RIM | SSP_INT_EN_TIM;
> +	val |= SSP_INT_EN_TINTE | SSP_INT_EN_RIE | SSP_INT_EN_TIE;
> +	writel(val, drv_data->ioaddr + SSP_INT_EN);
> +
> +	return true;
> +}
> +
> +static void k1_spi_transfer_end(struct k1_spi_driver_data *drv_data,
> +				struct spi_transfer *transfer)
> +{
> +	struct spi_message *message = drv_data->message;
> +	void __iomem *virt;
> +	u32 val;
> +
> +	if (drv_data->dma_mapped)
> +		k1_spi_transfer_end_dma(drv_data);
> +
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val &= ~TOP_SSE;
> +	writel(val, virt);
> +
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val &= ~TOP_DSS_MASK;
> +	writel(val, virt);
> +
> +	writel(0, drv_data->ioaddr + SSP_TIMEOUT);
> +
> +	if (drv_data->dma_mapped) {
> +		k1_spi_unmap_dma_buffer(&drv_data->tx);
> +		k1_spi_unmap_dma_buffer(&drv_data->rx);
> +	}
> +
> +	spi_transfer_delay_exec(transfer);
> +
> +	if (!message->status)
> +		message->actual_length += drv_data->len;
> +}
> +
> +static void k1_spi_transfer_wait(struct k1_spi_driver_data *drv_data)
> +{
> +	struct completion *completion = &drv_data->completion;
> +	struct spi_message *message = drv_data->message;
> +	unsigned long timeout;
> +	int ret;
> +
> +	/* Length in bits to be transferred */
> +	timeout = BITS_PER_BYTE * drv_data->bytes * drv_data->len;
> +	/* Time (usec) to transfer that many bits at the current bit rate */
> +	timeout = DIV_ROUND_UP(timeout * MICROHZ_PER_HZ, drv_data->rate);
> +	/* Convert that (+ 25%) to jiffies for the wait call */
> +	timeout = usecs_to_jiffies(5 * timeout / 4);
> +
> +	ret = wait_for_completion_interruptible_timeout(completion, timeout);
> +	if (ret > 0)
> +		return;
> +
> +	message->status = -EIO;
> +	if (ret && drv_data->dma_mapped) {
> +		dmaengine_terminate_sync(drv_data->tx.chan);
> +		dmaengine_terminate_sync(drv_data->rx.chan);
> +	}
> +}
> +
> +static int k1_spi_transfer_one_message(struct spi_controller *host,
> +					   struct spi_message *message)
> +{
> +	struct k1_spi_driver_data *drv_data = spi_controller_get_devdata(host);
> +	struct completion *completion = &drv_data->completion;
> +	struct spi_transfer *transfer;
> +	void __iomem *virt;
> +	u32 val;
> +
> +	drv_data->message = message;
> +
> +	/* Message status starts out successful; set to -EIO on error */
> +	message->status = 0;
> +
> +	/* Hold frame low to avoid losing transferred data */
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val |= TOP_HOLD_FRAME_LOW;
> +	writel(val, virt);
> +
> +	list_for_each_entry(transfer, &message->transfers, transfer_list) {
> +		reinit_completion(completion);
> +
> +		/* Issue the next transfer */
> +		if (!k1_spi_transfer_start(drv_data, transfer)) {
> +			message->status = -EIO;
> +			break;
> +		}
> +
> +		k1_spi_transfer_wait(drv_data);
> +
> +		k1_spi_transfer_end(drv_data, transfer);
> +
> +		/* If an error has occurred, we're done */
> +		if (message->status)
> +			break;
> +	}
> +
> +	drv_data->message = NULL;
> +
> +	spi_finalize_current_message(drv_data->controller);
> +
> +	virt = drv_data->ioaddr + SSP_TOP_CTRL;
> +	val = readl(virt);
> +	val &= ~TOP_HOLD_FRAME_LOW;
> +	writel(val, virt);
> +
> +	return 0;
> +}
> +
> +/*
> + * The client can call the setup function multiple times, and each call
> + * can specify a different SPI mode (and transfer speed).  Each transfer
> + * can specify its own speed though, and the core code ensures each
> + * transfer's speed is set to something nonzero and supported by both
> + * the controller and the device).  We just set the speed for each
> + * transfer.
> + */
> +static int k1_spi_setup(struct spi_device *spi)
> +{
> +	struct k1_spi_driver_data *drv_data;
> +	u32 val;
> +
> +	drv_data = spi_controller_get_devdata(spi->controller);
> +
> +	/*
> +	 * Configure the message format for this device.  We only
> +	 * support Motorola SPI format in master mode.
> +	 */
> +	val = FIELD_PREP(TOP_FRF_MASK, TOP_FRF_MOTOROLA);
> +	val |= TOP_HOLD_FRAME_LOW;	/* Master mode */
> +
> +	/* Translate the mode into the value used to program the hardware. */
> +	if (spi->mode & SPI_CPHA)
> +		val |= TOP_SPH;		/* 1/2 cycle */
> +	if (spi->mode & SPI_CPOL)
> +		val |= TOP_SPO;		/* active low */
> +	if (spi->mode & SPI_LOOP)
> +		val |= TOP_LBM;		/* enable loopback */
> +	writel(val, drv_data->ioaddr + SSP_TOP_CTRL);
> +
> +	return 0;
> +}
> +
> +static void k1_spi_cleanup(struct spi_device *spi)
> +{
> +	struct k1_spi_driver_data *drv_data;
> +
> +	drv_data = spi_controller_get_devdata(spi->controller);
> +
> +	writel(0, drv_data->ioaddr + SSP_TOP_CTRL);
> +}
> +
> +static int k1_spi_dma_setup_io(struct k1_spi_driver_data *drv_data, bool rx)
> +{
> +	const char *name = rx ? "rx" : "tx";
> +	struct device *dev = drv_data->dev;
> +	struct dma_chan *chan;
> +	struct k1_spi_io *io;
> +
> +	chan = devm_dma_request_chan(dev, name);
> +	if (IS_ERR(chan))
> +		return PTR_ERR(chan);
> +
> +	io = rx ? &drv_data->rx : &drv_data->tx;
> +	io->dir = rx ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	io->chan = chan;
> +
> +	return 0;
> +}
> +
> +static void k1_spi_dma_cleanup_io(struct k1_spi_driver_data *drv_data, bool rx)
> +{
> +	struct k1_spi_io *io = rx ? &drv_data->rx : &drv_data->tx;
> +
> +	dmaengine_terminate_sync(io->chan);
> +	sg_free_table(&io->sgt);
> +}
> +
> +static int k1_spi_dma_setup(struct k1_spi_driver_data *drv_data)
> +{
> +	struct device *dev = drv_data->dev;
> +	int rx_ret;
> +	int tx_ret;
> +
> +	/* We must get both DMA channels, or neither of them */
> +	rx_ret = k1_spi_dma_setup_io(drv_data, true);
> +	if (rx_ret == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +
> +	tx_ret = k1_spi_dma_setup_io(drv_data, false);
> +
> +	/* If neither is specified, we don't use DMA */
> +	if (rx_ret == -ENODEV && tx_ret == -ENODEV)
> +		return 0;
> +
> +	if (rx_ret || tx_ret)
> +		goto err_cleanup;
> +
> +	drv_data->dummy = devm_kzalloc(dev, SZ_2K, GFP_KERNEL);
> +	if (drv_data->dummy)
> +		return 0;		/* Success! */
> +
> +	dev_warn(dev, "error allocating DMA dummy buffer; DMA disabled\n");
> +err_cleanup:
> +	if (tx_ret) {
> +		if (tx_ret != -EPROBE_DEFER)
> +			dev_err(dev, "error requesting DMA TX DMA channel\n");
> +	} else {
> +		k1_spi_dma_cleanup_io(drv_data, false);
> +	}
> +
> +	if (rx_ret)
> +		dev_err(dev, "error requesting DMA RX DMA channel\n");
> +	else
> +		k1_spi_dma_cleanup_io(drv_data, true);
> +
> +	if (tx_ret == -EPROBE_DEFER)
> +		return -EPROBE_DEFER;
> +
> +	/* Return success if we don't get the dummy buffer; PIO will be used */
> +
> +	return rx_ret ? : tx_ret ? : 0;
> +}
> +
> +static void k1_spi_dma_cleanup(struct device *dev, void *res)
> +{
> +	struct k1_spi_driver_data *drv_data = res;
> +
> +	if (k1_spi_dma_enabled(drv_data)) {
> +		k1_spi_dma_cleanup_io(drv_data, false);
> +		k1_spi_dma_cleanup_io(drv_data, true);
> +	}
> +}
> +
> +static int devm_k1_spi_dma_setup(struct k1_spi_driver_data *drv_data)
> +{
> +	struct k1_spi_driver_data **ptr;
> +	int ret;
> +
> +	ptr = devres_alloc(k1_spi_dma_cleanup, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return -ENOMEM;
> +
> +	ret = k1_spi_dma_setup(drv_data);
> +	if (ret) {
> +		devres_free(ptr);
> +		return ret;
> +	}
> +
> +	*ptr = drv_data;
> +	devres_add(drv_data->dev, ptr);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id k1_spi_dt_ids[] = {
> +	{ .compatible = "spacemit,k1-spi", },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, k1_spi_dt_ids);
> +
> +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..
> +		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;
		

> +}
> +
> +/* 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
> 

-- 
Yixun Lan (dlan)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ