diff mbox series

[1/2] spi: Add Renesas R-Car RPC SPI controller driver

Message ID 1542621690-10229-2-git-send-email-masonccyang@mxic.com.tw (mailing list archive)
State New, archived
Headers show
Series spi: Add Renesas R-Car D3 RPC SPI driver | expand

Commit Message

Mason Yang Nov. 19, 2018, 10:01 a.m. UTC
Add a driver for Renesas R-Car D3 RPC SPI controller driver.

Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>
---
 drivers/spi/Kconfig           |   6 +
 drivers/spi/Makefile          |   1 +
 drivers/spi/spi-renesas-rpc.c | 750 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 757 insertions(+)
 create mode 100644 drivers/spi/spi-renesas-rpc.c

Comments

Marek Vasut Nov. 19, 2018, 2:12 p.m. UTC | #1
On 11/19/2018 11:01 AM, Mason Yang wrote:
> Add a driver for Renesas R-Car D3 RPC SPI controller driver.

The RPC supports both HF and SPI, not just SPI. And it's present in all
of Gen3 , not just D3 .

[...]

> +++ b/drivers/spi/spi-renesas-rpc.c
> @@ -0,0 +1,750 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
> +// Copyright (C) 2018 Macronix International Co., Ltd.
> +//
> +// R-Car D3 RPC SPI/QSPI/Octa driver
> +//
> +// Authors:
> +//	Mason Yang <masonccyang@mxic.com.tw>
> +//

Fix multiline comment please.

> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/spi/spi.h>
> +#include <linux/spi/spi-mem.h>

[...]

> +#define RPC_CMNSR		0x0048	/* R */
> +#define RPC_CMNSR_SSLF		BIT(1)
> +#define	RPC_CMNSR_TEND		BIT(0)

#define[SPACE] instead of tab

> +#define RPC_DRDMCR		0x0058	/* R/W */
> +#define RPC_DRDRENR		0x005C	/* R/W */
> +
> +#define RPC_SMDMCR		0x0060	/* R/W */
> +#define RPC_SMDMCR_DMCYC(v)	((((v) - 1) & 0x1F) << 0)
> +
> +#define RPC_SMDRENR		0x0064	/* R/W */
> +#define RPC_SMDRENR_HYPE	(0x5 << 12)
> +#define RPC_SMDRENR_ADDRE	BIT(8)
> +#define RPC_SMDRENR_OPDRE	BIT(4)
> +#define RPC_SMDRENR_SPIDRE	BIT(0)
> +
> +#define RPC_PHYCNT		0x007C	/* R/W */
> +#define RPC_PHYCNT_CAL		BIT(31)
> +#define PRC_PHYCNT_OCTA_AA	BIT(22)
> +#define PRC_PHYCNT_OCTA_SA	BIT(23)
> +#define PRC_PHYCNT_EXDS		BIT(21)
> +#define RPC_PHYCNT_OCT		BIT(20)
> +#define RPC_PHYCNT_STRTIM(v)	(((v) & 0x7) << 15)
> +#define RPC_PHYCNT_WBUF2	BIT(4)
> +#define RPC_PHYCNT_WBUF		BIT(2)
> +#define RPC_PHYCNT_MEM(v)	(((v) & 0x3) << 0)
> +
> +#define RPC_PHYOFFSET1		0x0080	/* R/W */
> +#define RPC_PHYOFFSET2		0x0084	/* R/W */
> +
> +#define RPC_WBUF		0x8000	/* Write Buffer */
> +#define RPC_WBUF_SIZE		256	/* Write Buffer size */
> +
> +struct rpc_spi {
> +	struct clk *clk_rpc;
> +	void __iomem *regs;
> +	struct {
> +		void __iomem *map;
> +		dma_addr_t dma;
> +		size_t size;
> +	} linear;

Does this need it's own struct ?

> +	u32 cur_speed_hz;
> +	u32 cmd;
> +	u32 addr;
> +	u32 dummy;
> +	u32 smcr;
> +	u32 smenr;
> +	u32 xferlen;
> +	u32 totalxferlen;

This register cache might be a good candidate for regmap ?

> +	enum spi_mem_data_dir xfer_dir;
> +};
> +
> +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
> +{
> +	int ret;
> +
> +	if (rpc->cur_speed_hz == freq)
> +		return 0;
> +
> +	clk_disable_unprepare(rpc->clk_rpc);
> +	ret = clk_set_rate(rpc->clk_rpc, freq);
> +	if (ret)
> +		return ret;
> +
> +	ret = clk_prepare_enable(rpc->clk_rpc);
> +	if (ret)
> +		return ret;

Is this clock disable/update/enable really needed ? I'd think that
clk_set_rate() would handle the rate update correctly.

> +	rpc->cur_speed_hz = freq;
> +	return ret;
> +}
> +
> +static void rpc_spi_hw_init(struct rpc_spi *rpc)
> +{
> +	/*
> +	 * NOTE: The 0x260 are undocumented bits, but they must be set.
> +	 */

FYI:
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/spi/renesas_rpc_spi.c#l207

I think the STRTIM should be 6 .

> +	writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0x3) | 0x260,
> +	       rpc->regs + RPC_PHYCNT);
> +
> +	/*
> +	 * NOTE: The 0x31511144 and 0x431 are undocumented bits,
> +	 *	 but they must be set for RPC_PHYOFFSET1 & RPC_PHYOFFSET2.
> +	 */
> +	writel(0x31511144, rpc->regs + RPC_PHYOFFSET1);
> +	writel(0x431, rpc->regs + RPC_PHYOFFSET2);
> +
> +	writel(RPC_SSLDR_SPNDL(7) | RPC_SSLDR_SLNDL(7) |
> +	       RPC_SSLDR_SCKDL(7), rpc->regs + RPC_SSLDR);
> +}
> +
> +static int wait_msg_xfer_end(struct rpc_spi *rpc)
> +{
> +	u32 sts;
> +
> +	return readl_poll_timeout(rpc->regs + RPC_CMNSR, sts,
> +				  sts & RPC_CMNSR_TEND, 0, USEC_PER_SEC);
> +}
> +
> +static u8 rpc_bits_xfer(u32 nbytes)
> +{
> +	u8 databyte;
> +
> +	switch (nbytes) {

Did you ever test unaligned writes and reads ? There are some nasty edge
cases in those.

Also, I think you can calculate the number of set bits using a simple
function, so the switch-case might not even be needed.

> +	case 1:
> +		databyte = 0x8;
> +		break;
> +	case 2:
> +		databyte = 0xc;
> +		break;
> +	default:
> +		databyte = 0xf;
> +		break;
> +	}
> +
> +	return databyte;
> +}
> +
> +static int rpc_spi_io_xfer(struct rpc_spi *rpc,
> +			   const void *tx_buf, void *rx_buf)
> +{
> +	u32 smenr, smcr, data, pos = 0;
> +	int ret = 0;
> +
> +	writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
> +	       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
> +	writel(0x0, rpc->regs + RPC_SMDRENR);
> +
> +	if (tx_buf) {
> +		writel(rpc->cmd, rpc->regs + RPC_SMCMR);
> +		writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
> +		writel(rpc->addr, rpc->regs + RPC_SMADR);
> +		smenr = rpc->smenr;
> +
> +		while (pos < rpc->xferlen) {
> +			u32 nbytes = rpc->xferlen  - pos;
> +
> +			writel(*(u32 *)(tx_buf + pos), rpc->regs + RPC_SMWDR0);
> +
> +			if (nbytes > 4) {
> +				nbytes = 4;
> +				smcr = rpc->smcr |
> +				       RPC_SMCR_SPIE | RPC_SMCR_SSLKP;
> +			} else {
> +				smcr = rpc->smcr | RPC_SMCR_SPIE;
> +			}
> +
> +			writel(smenr, rpc->regs + RPC_SMENR);
> +			writel(smcr, rpc->regs + RPC_SMCR);
> +			ret = wait_msg_xfer_end(rpc);
> +			if (ret)
> +				goto out;
> +
> +			pos += nbytes;
> +			smenr = rpc->smenr & ~RPC_SMENR_CDE &
> +					     ~RPC_SMENR_ADE(0xf);
> +		}
> +	} else if (rx_buf) {
> +		while (pos < rpc->xferlen) {
> +			u32 nbytes = rpc->xferlen  - pos;
> +
> +			if (nbytes > 4)
> +				nbytes = 4;
> +
> +			writel(rpc->cmd, rpc->regs + RPC_SMCMR);
> +			writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
> +			writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
> +			writel(rpc->smenr, rpc->regs + RPC_SMENR);
> +			writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
> +			ret = wait_msg_xfer_end(rpc);
> +			if (ret)
> +				goto out;
> +
> +			data = readl(rpc->regs + RPC_SMRDR0);
> +			memcpy_fromio(rx_buf + pos, (void *)&data, nbytes);
> +			pos += nbytes;
> +		}
> +	} else {
> +		writel(rpc->cmd, rpc->regs + RPC_SMCMR);
> +		writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
> +		writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
> +		writel(rpc->smenr, rpc->regs + RPC_SMENR);
> +		writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
> +		ret = wait_msg_xfer_end(rpc);
> +	}
> +out:

Dont you need to stop the RPC somehow in case the transmission fails ?

> +	return ret;
> +}
> +
> +static void rpc_spi_mem_set_prep_op_cfg(struct spi_device *spi,
> +					const struct spi_mem_op *op,
> +					u64 *offs, size_t *len)
> +{
> +	struct rpc_spi *rpc = spi_master_get_devdata(spi->master);
> +
> +	rpc->cmd = RPC_SMCMR_CMD(op->cmd.opcode);
> +	rpc->smenr = RPC_SMENR_CDE |
> +		     RPC_SMENR_CDB(fls(op->cmd.buswidth >> 1));
> +	rpc->totalxferlen = 1;
> +	rpc->xferlen = 0;
> +	rpc->addr = 0;
> +
> +	if (op->addr.nbytes) {
> +		rpc->smenr |= RPC_SMENR_ADB(fls(op->addr.buswidth >> 1));
> +		if (op->addr.nbytes == 4)
> +			rpc->smenr |= RPC_SMENR_ADE(0xf);
> +		else
> +			rpc->smenr |= RPC_SMENR_ADE(0x7);
> +
> +		if (!offs && !len)
> +			rpc->addr = *(u32 *)offs;

How does this work ? Shouldn't this be just *offs to dereference the
pointer ?

> +		else
> +			rpc->addr = op->addr.val;
> +		rpc->totalxferlen += op->addr.nbytes;
> +	}
> +
> +	if (op->dummy.nbytes) {
> +		rpc->smenr |= RPC_SMENR_DME;
> +		rpc->dummy = RPC_SMDMCR_DMCYC(op->dummy.nbytes);
> +		rpc->totalxferlen += op->dummy.nbytes;
> +	}
> +
> +	if (op->data.nbytes || (offs && len)) {
> +		rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer(op->data.nbytes)) |
> +			      RPC_SMENR_SPIDB(fls(op->data.buswidth >> 1));
> +
> +		if (op->data.dir == SPI_MEM_DATA_IN) {
> +			rpc->smcr = RPC_SMCR_SPIRE;
> +			rpc->xfer_dir = SPI_MEM_DATA_IN;
> +		} else if (op->data.dir == SPI_MEM_DATA_OUT) {
> +			rpc->smcr = RPC_SMCR_SPIWE;
> +			rpc->xfer_dir = SPI_MEM_DATA_OUT;
> +		}
> +
> +		if (offs && len) {
> +			rpc->xferlen = *(u32 *)len;
> +			rpc->totalxferlen += *(u32 *)len;
> +		} else {
> +			rpc->xferlen = op->data.nbytes;
> +			rpc->totalxferlen += op->data.nbytes;
> +		}
> +	}
> +}
> +
> +static bool rpc_spi_mem_supports_op(struct spi_mem *mem,
> +				    const struct spi_mem_op *op)
> +{
> +	if (op->data.buswidth > 4 || op->addr.buswidth > 4 ||
> +	    op->dummy.buswidth > 4 || op->cmd.buswidth > 4)
> +		return false;
> +
> +	if (op->addr.nbytes > 4)
> +		return false;
> +
> +	return true;
> +}
> +
> +static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
> +				       u64 offs, size_t len, void *buf)
> +{
> +	struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
> +	int ret;
> +
> +	if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
> +		return -EINVAL;
> +
> +	ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
> +	if (ret)
> +		return ret;
> +
> +	rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
> +				    &desc->info.op_tmpl, &offs, &len);
> +
> +	writel(RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
> +	       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
> +
> +	writel(RPC_DRCR_RBURST(0x1f) | RPC_DRCR_RBE, rpc->regs + RPC_DRCR);
> +	writel(rpc->cmd, rpc->regs + RPC_DRCMR);
> +	writel(RPC_DREAR_EAC, rpc->regs + RPC_DREAR);
> +	writel(0, rpc->regs + RPC_DROPR);
> +	writel(rpc->smenr, rpc->regs + RPC_DRENR);
> +	writel(rpc->dummy, rpc->regs + RPC_DRDMCR);
> +	writel(0x0, rpc->regs + RPC_DRDRENR);
> +	memcpy_fromio(buf, rpc->linear.map + desc->info.offset + offs, len);
> +
> +	return len;
> +}
> +
> +static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
> +					u64 offs, size_t len, const void *buf)
> +{
> +	struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
> +	int tx_offs, ret;
> +
> +	if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
> +		return -EINVAL;
> +
> +	if (WARN_ON(len > RPC_WBUF_SIZE))
> +		return -EIO;
> +
> +	ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
> +	if (ret)
> +		return ret;
> +
> +	rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
> +				    &desc->info.op_tmpl, &offs, &len);
> +
> +	writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
> +	       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
> +	writel(0x0, rpc->regs + RPC_SMDRENR);
> +
> +	writel(RPC_PHYCNT_CAL | 0x260 | RPC_PHYCNT_WBUF2 | RPC_PHYCNT_WBUF,
> +	       rpc->regs + RPC_PHYCNT);
> +
> +	for (tx_offs = 0; tx_offs < RPC_WBUF_SIZE; tx_offs += 4)
> +		writel(*(u32 *)(buf + tx_offs), rpc->regs + RPC_WBUF + tx_offs);

Isn't this some memcpy_toio() or iowrite32_rep() reimplementation here ?

> +	writel(rpc->cmd, rpc->regs + RPC_SMCMR);
> +	writel(offs, rpc->regs + RPC_SMADR);
> +	writel(rpc->smenr, rpc->regs + RPC_SMENR);
> +	writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
> +	ret = wait_msg_xfer_end(rpc);
> +	if (ret)
> +		goto out;
> +
> +	writel(RPC_DRCR_RCF, rpc->regs + RPC_DRCR);
> +	writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0) | 0x260,
> +	       rpc->regs + RPC_PHYCNT);
> +
> +	return len;
> +out:

Shouldn't you shut the controller down if the xfer fails ?

> +	return ret;
> +}
> +
> +static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
> +{
> +	struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
> +
> +	if (desc->info.offset + desc->info.length > U32_MAX)
> +		return -ENOTSUPP;
> +
> +	if (!rpc_spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
> +		return -ENOTSUPP;
> +
> +	if (!rpc->linear.map &&
> +	    desc->info.op_tmpl.data.dir == SPI_MEM_DATA_IN)
> +		return -ENOTSUPP;
> +
> +	return 0;
> +}
> +
> +static int rpc_spi_mem_exec_op(struct spi_mem *mem,
> +			       const struct spi_mem_op *op)
> +{
> +	struct rpc_spi *rpc = spi_master_get_devdata(mem->spi->master);
> +	int ret;
> +
> +	ret = rpc_spi_set_freq(rpc, mem->spi->max_speed_hz);
> +	if (ret)
> +		return ret;
> +
> +	rpc_spi_mem_set_prep_op_cfg(mem->spi, op, NULL, NULL);
> +
> +	ret = rpc_spi_io_xfer(rpc,
> +			      op->data.dir == SPI_MEM_DATA_OUT ?
> +			      op->data.buf.out : NULL,
> +			      op->data.dir == SPI_MEM_DATA_IN ?
> +			      op->data.buf.in : NULL);
> +
> +	return ret;
> +}
> +
> +static const struct spi_controller_mem_ops rpc_spi_mem_ops = {
> +	.supports_op = rpc_spi_mem_supports_op,
> +	.exec_op = rpc_spi_mem_exec_op,
> +	.dirmap_create = rpc_spi_mem_dirmap_create,
> +	.dirmap_read = rpc_spi_mem_dirmap_read,
> +	.dirmap_write = rpc_spi_mem_dirmap_write,
> +};
> +
> +static void rpc_spi_transfer_setup(struct rpc_spi *rpc,
> +				   struct spi_message *msg)
> +{
> +	struct spi_transfer *t, xfer[4] = { };
> +	u32 i, xfercnt, xferpos = 0;
> +
> +	rpc->totalxferlen = 0;
> +	list_for_each_entry(t, &msg->transfers, transfer_list) {
> +		if (t->tx_buf) {
> +			xfer[xferpos].tx_buf = t->tx_buf;
> +			xfer[xferpos].tx_nbits = t->tx_nbits;
> +		}
> +
> +		if (t->rx_buf) {
> +			xfer[xferpos].rx_buf = t->rx_buf;
> +			xfer[xferpos].rx_nbits = t->rx_nbits;
> +		}
> +
> +		if (t->len) {
> +			xfer[xferpos++].len = t->len;
> +			rpc->totalxferlen += t->len;
> +		}
> +	}
> +
> +	xfercnt = xferpos;
> +	rpc->xferlen = xfer[--xferpos].len;
> +	rpc->cmd = RPC_SMCMR_CMD(((u8 *)xfer[0].tx_buf)[0]);

Is the cast needed ?

> +	rpc->smenr = RPC_SMENR_CDE | RPC_SMENR_CDB(fls(xfer[0].tx_nbits >> 1));
> +	rpc->addr = 0;
> +
> +	if (xfercnt > 2 && xfer[1].len && xfer[1].tx_buf) {
> +		rpc->smenr |= RPC_SMENR_ADB(fls(xfer[1].tx_nbits >> 1));
> +		for (i = 0; i < xfer[1].len; i++)
> +			rpc->addr |= (u32)((u8 *)xfer[1].tx_buf)[i]
> +					<< (8 * (xfer[1].len - i - 1));
> +
> +		if (xfer[1].len == 4)
> +			rpc->smenr |= RPC_SMENR_ADE(0xf);
> +		else
> +			rpc->smenr |= RPC_SMENR_ADE(0x7);
> +	}
> +
> +	switch (xfercnt) {
> +	case 2:
> +		if (xfer[1].rx_buf) {
> +			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
> +				      (xfer[1].len)) | RPC_SMENR_SPIDB(fls
> +				      (xfer[1].rx_nbits >> 1));

How much of this register value calculation could be somehow
deduplicated ? It seems to be almost the same thing copied thrice here.

> +			rpc->smcr = RPC_SMCR_SPIRE;
> +			rpc->xfer_dir = SPI_MEM_DATA_IN;
> +		} else if (xfer[1].tx_buf) {
> +			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
> +				      (xfer[1].len)) | RPC_SMENR_SPIDB(fls
> +				      (xfer[1].tx_nbits >> 1));
> +			rpc->smcr = RPC_SMCR_SPIWE;
> +			rpc->xfer_dir = SPI_MEM_DATA_OUT;
> +		}
> +		break;
> +
> +	case 3:
> +		if (xfer[2].len && xfer[2].rx_buf && !xfer[2].tx_buf) {
> +			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
> +				      (xfer[2].len)) | RPC_SMENR_SPIDB(fls
> +				      (xfer[2].rx_nbits >> 1));
> +			rpc->smcr = RPC_SMCR_SPIRE;
> +			rpc->xfer_dir = SPI_MEM_DATA_IN;
> +		} else if (xfer[2].len && xfer[2].tx_buf && !xfer[2].rx_buf) {
> +			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
> +				      (xfer[2].len)) | RPC_SMENR_SPIDB(fls
> +				      (xfer[2].tx_nbits >> 1));
> +			rpc->smcr = RPC_SMCR_SPIWE;
> +			rpc->xfer_dir = SPI_MEM_DATA_OUT;
> +		}
> +
> +		break;
> +
> +	case 4:
> +		if (xfer[2].len && xfer[2].tx_buf) {
> +			rpc->smenr |= RPC_SMENR_DME;
> +			rpc->dummy = RPC_SMDMCR_DMCYC(xfer[2].len);
> +			writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
> +		}
> +
> +		if (xfer[3].len && xfer[3].rx_buf) {
> +			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
> +				      (xfer[3].len)) | RPC_SMENR_SPIDB(fls
> +				      (xfer[3].rx_nbits >> 1));
> +			rpc->smcr = RPC_SMCR_SPIRE;
> +			rpc->xfer_dir = SPI_MEM_DATA_IN;
> +		}
> +
> +		break;
> +
> +	default:
> +		break;
> +	}
> +}
> +
> +static int rpc_spi_xfer_message(struct rpc_spi *rpc, struct spi_transfer *t)
> +{
> +	int ret;
> +
> +	ret = rpc_spi_set_freq(rpc, t->speed_hz);
> +	if (ret)
> +		return ret;
> +
> +	ret = rpc_spi_io_xfer(rpc,
> +			      rpc->xfer_dir == SPI_MEM_DATA_OUT ?
> +			      t->tx_buf : NULL,
> +			      rpc->xfer_dir == SPI_MEM_DATA_IN ?
> +			      t->rx_buf : NULL);
> +
> +	return ret;
> +}
> +
> +static int rpc_spi_transfer_one_message(struct spi_master *master,
> +					struct spi_message *msg)
> +{
> +	struct rpc_spi *rpc = spi_master_get_devdata(master);
> +	struct spi_transfer *t;
> +	int ret;
> +
> +	rpc_spi_transfer_setup(rpc, msg);
> +
> +	list_for_each_entry(t, &msg->transfers, transfer_list) {
> +		if (list_is_last(&t->transfer_list, &msg->transfers)) {

if (!list...)
 continue;

to reduce the indent level.

> +			ret = rpc_spi_xfer_message(rpc, t);
> +			if (ret)
> +				goto out;
> +		}
> +	}
> +
> +	msg->status = 0;
> +	msg->actual_length = rpc->totalxferlen;
> +out:
> +	spi_finalize_current_message(master);
> +	return 0;
> +}
> +
> +static int __maybe_unused rpc_spi_runtime_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct spi_master *master = platform_get_drvdata(pdev);
> +	struct rpc_spi *rpc = spi_master_get_devdata(master);
> +
> +	clk_disable_unprepare(rpc->clk_rpc);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused rpc_spi_runtime_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct spi_master *master = platform_get_drvdata(pdev);
> +	struct rpc_spi *rpc = spi_master_get_devdata(master);
> +	int ret;
> +
> +	ret = clk_prepare_enable(rpc->clk_rpc);
> +	if (ret)
> +		dev_err(dev, "Can't enable rpc->clk_rpc\n");
> +
> +	return ret;
> +}
> +
> +static const struct dev_pm_ops rpc_spi_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(rpc_spi_runtime_suspend,
> +			   rpc_spi_runtime_resume, NULL)
> +};
> +
> +static int rpc_spi_probe(struct platform_device *pdev)
> +{
> +	struct spi_master *master;
> +	struct resource *res;
> +	struct rpc_spi *rpc;
> +	int ret;
> +
> +	master = spi_alloc_master(&pdev->dev, sizeof(struct rpc_spi));
> +	if (!master)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, master);
> +
> +	rpc = spi_master_get_devdata(master);
> +
> +	master->dev.of_node = pdev->dev.of_node;
> +
> +	rpc->clk_rpc = devm_clk_get(&pdev->dev, "clk_rpc");
> +	if (IS_ERR(rpc->clk_rpc))
> +		return PTR_ERR(rpc->clk_rpc);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rpc_regs");
> +	rpc->regs = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(rpc->regs))
> +		return PTR_ERR(rpc->regs);
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
> +	rpc->linear.map = devm_ioremap_resource(&pdev->dev, res);
> +	if (!IS_ERR(rpc->linear.map)) {
> +		rpc->linear.dma = res->start;
> +		rpc->linear.size = resource_size(res);
> +	} else {
> +		rpc->linear.map = NULL;
> +	}
> +
> +	pm_runtime_enable(&pdev->dev);
> +	master->auto_runtime_pm = true;
> +
> +	master->num_chipselect = 1;
> +	master->mem_ops = &rpc_spi_mem_ops;
> +	master->transfer_one_message = rpc_spi_transfer_one_message;
> +
> +	master->bits_per_word_mask = SPI_BPW_MASK(8);
> +	master->mode_bits = SPI_CPOL | SPI_CPHA |
> +			SPI_RX_DUAL | SPI_TX_DUAL |
> +			SPI_RX_QUAD | SPI_TX_QUAD;
> +
> +	rpc_spi_hw_init(rpc);
> +
> +	ret = spi_register_master(master);
> +	if (ret) {
> +		dev_err(&pdev->dev, "spi_register_master failed\n");
> +		goto err_put_master;
> +	}
> +	return 0;
> +
> +err_put_master:
> +	spi_master_put(master);
> +	pm_runtime_disable(&pdev->dev);
> +
> +	return ret;
> +}
> +
> +static int rpc_spi_remove(struct platform_device *pdev)
> +{
> +	struct spi_master *master = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +	spi_unregister_master(master);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rpc_spi_of_ids[] = {
> +	{ .compatible = "renesas,rpc-r8a77995", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rpc_spi_of_ids);
> +
> +static struct platform_driver rpc_spi_driver = {
> +	.probe = rpc_spi_probe,
> +	.remove = rpc_spi_remove,
> +	.driver = {
> +		.name = "rpc-spi",
> +		.of_match_table = rpc_spi_of_ids,
> +		.pm = &rpc_spi_dev_pm_ops,
> +	},
> +};
> +module_platform_driver(rpc_spi_driver);
> +
> +MODULE_AUTHOR("Mason Yang <masonccyang@mxic.com.tw>");
> +MODULE_DESCRIPTION("Renesas R-Car D3 RPC SPI controller driver");

This is not D3 specific and not SPI-only controller btw.

> +MODULE_LICENSE("GPL v2");
>
Mark Brown Nov. 19, 2018, 3:27 p.m. UTC | #2
On Mon, Nov 19, 2018 at 03:12:00PM +0100, Marek Vasut wrote:
> On 11/19/2018 11:01 AM, Mason Yang wrote:

> > +++ b/drivers/spi/spi-renesas-rpc.c
> > @@ -0,0 +1,750 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
> > +// Copyright (C) 2018 Macronix International Co., Ltd.
> > +//
> > +// R-Car D3 RPC SPI/QSPI/Octa driver
> > +//
> > +// Authors:
> > +//	Mason Yang <masonccyang@mxic.com.tw>
> > +//

> Fix multiline comment please.

The SPDX header needs to be C++ style so I push people to make the whole
block C++ otherwise it looks messy.
Marek Vasut Nov. 19, 2018, 10:10 p.m. UTC | #3
On 11/19/2018 04:27 PM, Mark Brown wrote:
> On Mon, Nov 19, 2018 at 03:12:00PM +0100, Marek Vasut wrote:
>> On 11/19/2018 11:01 AM, Mason Yang wrote:
> 
>>> +++ b/drivers/spi/spi-renesas-rpc.c
>>> @@ -0,0 +1,750 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +//
>>> +// Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
>>> +// Copyright (C) 2018 Macronix International Co., Ltd.
>>> +//
>>> +// R-Car D3 RPC SPI/QSPI/Octa driver
>>> +//
>>> +// Authors:
>>> +//	Mason Yang <masonccyang@mxic.com.tw>
>>> +//
> 
>> Fix multiline comment please.
> 
> The SPDX header needs to be C++ style so I push people to make the whole
> block C++ otherwise it looks messy.

OK, I'm not gonna wrestle you on this, but I think it looks horrible ;-)
kernel test robot Nov. 20, 2018, 2:04 a.m. UTC | #4
Hi Mason,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on spi/for-next]
[also build test ERROR on v4.20-rc3 next-20181119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mason-Yang/spi-Add-Renesas-R-Car-RPC-SPI-controller-driver/20181120-020310
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

>> drivers/spi/spi-renesas-rpc.c:366:47: warning: 'struct spi_mem_dirmap_desc' declared inside parameter list will not be visible outside of this definition or declaration
    static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
                                                  ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/spi/spi-renesas-rpc.c:18:0:
   drivers/spi/spi-renesas-rpc.c: In function 'rpc_spi_mem_dirmap_read':
>> drivers/spi/spi-renesas-rpc.c:369:51: error: dereferencing pointer to incomplete type 'struct spi_mem_dirmap_desc'
     struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
                                                      ^
   include/linux/spi/spi.h:1333:66: note: in definition of macro 'spi_master_get_devdata'
    #define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr)
                                                                     ^~~~~
   drivers/spi/spi-renesas-rpc.c: At top level:
   drivers/spi/spi-renesas-rpc.c:397:48: warning: 'struct spi_mem_dirmap_desc' declared inside parameter list will not be visible outside of this definition or declaration
    static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
                                                   ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/spi/spi-renesas-rpc.c:18:0:
   drivers/spi/spi-renesas-rpc.c: In function 'rpc_spi_mem_dirmap_write':
   drivers/spi/spi-renesas-rpc.c:400:51: error: dereferencing pointer to incomplete type 'struct spi_mem_dirmap_desc'
     struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
                                                      ^
   include/linux/spi/spi.h:1333:66: note: in definition of macro 'spi_master_get_devdata'
    #define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr)
                                                                     ^~~~~
   drivers/spi/spi-renesas-rpc.c: At top level:
   drivers/spi/spi-renesas-rpc.c:443:45: warning: 'struct spi_mem_dirmap_desc' declared inside parameter list will not be visible outside of this definition or declaration
    static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
                                                ^~~~~~~~~~~~~~~~~~~
   In file included from drivers/spi/spi-renesas-rpc.c:18:0:
   drivers/spi/spi-renesas-rpc.c: In function 'rpc_spi_mem_dirmap_create':
   drivers/spi/spi-renesas-rpc.c:445:51: error: dereferencing pointer to incomplete type 'struct spi_mem_dirmap_desc'
     struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
                                                      ^
   include/linux/spi/spi.h:1333:66: note: in definition of macro 'spi_master_get_devdata'
    #define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr)
                                                                     ^~~~~
   drivers/spi/spi-renesas-rpc.c: At top level:
>> drivers/spi/spi-renesas-rpc.c:484:3: error: 'const struct spi_controller_mem_ops' has no member named 'dirmap_create'
     .dirmap_create = rpc_spi_mem_dirmap_create,
      ^~~~~~~~~~~~~
>> drivers/spi/spi-renesas-rpc.c:484:19: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
     .dirmap_create = rpc_spi_mem_dirmap_create,
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/spi/spi-renesas-rpc.c:484:19: note: (near initialization for 'rpc_spi_mem_ops')
>> drivers/spi/spi-renesas-rpc.c:484:19: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
   drivers/spi/spi-renesas-rpc.c:484:19: note: (near initialization for 'rpc_spi_mem_ops.supports_op')
>> drivers/spi/spi-renesas-rpc.c:485:3: error: 'const struct spi_controller_mem_ops' has no member named 'dirmap_read'
     .dirmap_read = rpc_spi_mem_dirmap_read,
      ^~~~~~~~~~~
   drivers/spi/spi-renesas-rpc.c:485:17: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
     .dirmap_read = rpc_spi_mem_dirmap_read,
                    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers/spi/spi-renesas-rpc.c:485:17: note: (near initialization for 'rpc_spi_mem_ops')
   drivers/spi/spi-renesas-rpc.c:485:17: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
   drivers/spi/spi-renesas-rpc.c:485:17: note: (near initialization for 'rpc_spi_mem_ops.adjust_op_size')
>> drivers/spi/spi-renesas-rpc.c:486:3: error: 'const struct spi_controller_mem_ops' has no member named 'dirmap_write'
     .dirmap_write = rpc_spi_mem_dirmap_write,
      ^~~~~~~~~~~~
   drivers/spi/spi-renesas-rpc.c:486:18: error: positional initialization of field in 'struct' declared with 'designated_init' attribute [-Werror=designated-init]
     .dirmap_write = rpc_spi_mem_dirmap_write,
                     ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers/spi/spi-renesas-rpc.c:486:18: note: (near initialization for 'rpc_spi_mem_ops')
   drivers/spi/spi-renesas-rpc.c:486:18: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
   drivers/spi/spi-renesas-rpc.c:486:18: note: (near initialization for 'rpc_spi_mem_ops.get_name')
   cc1: some warnings being treated as errors

vim +369 drivers/spi/spi-renesas-rpc.c

   365	
 > 366	static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
   367					       u64 offs, size_t len, void *buf)
   368	{
 > 369		struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
   370		int ret;
   371	
   372		if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
   373			return -EINVAL;
   374	
   375		ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
   376		if (ret)
   377			return ret;
   378	
   379		rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
   380					    &desc->info.op_tmpl, &offs, &len);
   381	
   382		writel(RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
   383		       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
   384	
   385		writel(RPC_DRCR_RBURST(0x1f) | RPC_DRCR_RBE, rpc->regs + RPC_DRCR);
   386		writel(rpc->cmd, rpc->regs + RPC_DRCMR);
   387		writel(RPC_DREAR_EAC, rpc->regs + RPC_DREAR);
   388		writel(0, rpc->regs + RPC_DROPR);
   389		writel(rpc->smenr, rpc->regs + RPC_DRENR);
   390		writel(rpc->dummy, rpc->regs + RPC_DRDMCR);
   391		writel(0x0, rpc->regs + RPC_DRDRENR);
   392		memcpy_fromio(buf, rpc->linear.map + desc->info.offset + offs, len);
   393	
   394		return len;
   395	}
   396	
   397	static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
   398						u64 offs, size_t len, const void *buf)
   399	{
   400		struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
   401		int tx_offs, ret;
   402	
   403		if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
   404			return -EINVAL;
   405	
   406		if (WARN_ON(len > RPC_WBUF_SIZE))
   407			return -EIO;
   408	
   409		ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
   410		if (ret)
   411			return ret;
   412	
   413		rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
   414					    &desc->info.op_tmpl, &offs, &len);
   415	
   416		writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
   417		       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
   418		writel(0x0, rpc->regs + RPC_SMDRENR);
   419	
   420		writel(RPC_PHYCNT_CAL | 0x260 | RPC_PHYCNT_WBUF2 | RPC_PHYCNT_WBUF,
   421		       rpc->regs + RPC_PHYCNT);
   422	
   423		for (tx_offs = 0; tx_offs < RPC_WBUF_SIZE; tx_offs += 4)
   424			writel(*(u32 *)(buf + tx_offs), rpc->regs + RPC_WBUF + tx_offs);
   425	
   426		writel(rpc->cmd, rpc->regs + RPC_SMCMR);
   427		writel(offs, rpc->regs + RPC_SMADR);
   428		writel(rpc->smenr, rpc->regs + RPC_SMENR);
   429		writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
   430		ret = wait_msg_xfer_end(rpc);
   431		if (ret)
   432			goto out;
   433	
   434		writel(RPC_DRCR_RCF, rpc->regs + RPC_DRCR);
   435		writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0) | 0x260,
   436		       rpc->regs + RPC_PHYCNT);
   437	
   438		return len;
   439	out:
   440		return ret;
   441	}
   442	
   443	static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
   444	{
   445		struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
   446	
   447		if (desc->info.offset + desc->info.length > U32_MAX)
   448			return -ENOTSUPP;
   449	
   450		if (!rpc_spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
   451			return -ENOTSUPP;
   452	
   453		if (!rpc->linear.map &&
   454		    desc->info.op_tmpl.data.dir == SPI_MEM_DATA_IN)
   455			return -ENOTSUPP;
   456	
   457		return 0;
   458	}
   459	
   460	static int rpc_spi_mem_exec_op(struct spi_mem *mem,
   461				       const struct spi_mem_op *op)
   462	{
   463		struct rpc_spi *rpc = spi_master_get_devdata(mem->spi->master);
   464		int ret;
   465	
   466		ret = rpc_spi_set_freq(rpc, mem->spi->max_speed_hz);
   467		if (ret)
   468			return ret;
   469	
   470		rpc_spi_mem_set_prep_op_cfg(mem->spi, op, NULL, NULL);
   471	
   472		ret = rpc_spi_io_xfer(rpc,
   473				      op->data.dir == SPI_MEM_DATA_OUT ?
   474				      op->data.buf.out : NULL,
   475				      op->data.dir == SPI_MEM_DATA_IN ?
   476				      op->data.buf.in : NULL);
   477	
   478		return ret;
   479	}
   480	
   481	static const struct spi_controller_mem_ops rpc_spi_mem_ops = {
   482		.supports_op = rpc_spi_mem_supports_op,
   483		.exec_op = rpc_spi_mem_exec_op,
 > 484		.dirmap_create = rpc_spi_mem_dirmap_create,
 > 485		.dirmap_read = rpc_spi_mem_dirmap_read,
 > 486		.dirmap_write = rpc_spi_mem_dirmap_write,
   487	};
   488	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Nov. 20, 2018, 5:49 a.m. UTC | #5
Hi Mason,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on spi/for-next]
[also build test WARNING on v4.20-rc3 next-20181119]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mason-Yang/spi-Add-Renesas-R-Car-RPC-SPI-controller-driver/20181120-020310
base:   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next
config: sh-allyesconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   drivers//spi/spi-renesas-rpc.c:366:47: warning: 'struct spi_mem_dirmap_desc' declared inside parameter list will not be visible outside of this definition or declaration
    static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
                                                  ^~~~~~~~~~~~~~~~~~~
   In file included from drivers//spi/spi-renesas-rpc.c:18:0:
   drivers//spi/spi-renesas-rpc.c: In function 'rpc_spi_mem_dirmap_read':
   drivers//spi/spi-renesas-rpc.c:369:51: error: dereferencing pointer to incomplete type 'struct spi_mem_dirmap_desc'
     struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
                                                      ^
   include/linux/spi/spi.h:1333:66: note: in definition of macro 'spi_master_get_devdata'
    #define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr)
                                                                     ^~~~~
   drivers//spi/spi-renesas-rpc.c: At top level:
   drivers//spi/spi-renesas-rpc.c:397:48: warning: 'struct spi_mem_dirmap_desc' declared inside parameter list will not be visible outside of this definition or declaration
    static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
                                                   ^~~~~~~~~~~~~~~~~~~
   In file included from drivers//spi/spi-renesas-rpc.c:18:0:
   drivers//spi/spi-renesas-rpc.c: In function 'rpc_spi_mem_dirmap_write':
   drivers//spi/spi-renesas-rpc.c:400:51: error: dereferencing pointer to incomplete type 'struct spi_mem_dirmap_desc'
     struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
                                                      ^
   include/linux/spi/spi.h:1333:66: note: in definition of macro 'spi_master_get_devdata'
    #define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr)
                                                                     ^~~~~
   drivers//spi/spi-renesas-rpc.c: At top level:
   drivers//spi/spi-renesas-rpc.c:443:45: warning: 'struct spi_mem_dirmap_desc' declared inside parameter list will not be visible outside of this definition or declaration
    static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
                                                ^~~~~~~~~~~~~~~~~~~
   In file included from drivers//spi/spi-renesas-rpc.c:18:0:
   drivers//spi/spi-renesas-rpc.c: In function 'rpc_spi_mem_dirmap_create':
   drivers//spi/spi-renesas-rpc.c:445:51: error: dereferencing pointer to incomplete type 'struct spi_mem_dirmap_desc'
     struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
                                                      ^
   include/linux/spi/spi.h:1333:66: note: in definition of macro 'spi_master_get_devdata'
    #define spi_master_get_devdata(_ctlr) spi_controller_get_devdata(_ctlr)
                                                                     ^~~~~
   drivers//spi/spi-renesas-rpc.c: At top level:
   drivers//spi/spi-renesas-rpc.c:484:3: error: 'const struct spi_controller_mem_ops' has no member named 'dirmap_create'
     .dirmap_create = rpc_spi_mem_dirmap_create,
      ^~~~~~~~~~~~~
   drivers//spi/spi-renesas-rpc.c:484:19: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
     .dirmap_create = rpc_spi_mem_dirmap_create,
                      ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers//spi/spi-renesas-rpc.c:484:19: note: (near initialization for 'rpc_spi_mem_ops.get_name')
   drivers//spi/spi-renesas-rpc.c:485:3: error: 'const struct spi_controller_mem_ops' has no member named 'dirmap_read'
     .dirmap_read = rpc_spi_mem_dirmap_read,
      ^~~~~~~~~~~
>> drivers//spi/spi-renesas-rpc.c:485:17: warning: excess elements in struct initializer
     .dirmap_read = rpc_spi_mem_dirmap_read,
                    ^~~~~~~~~~~~~~~~~~~~~~~
   drivers//spi/spi-renesas-rpc.c:485:17: note: (near initialization for 'rpc_spi_mem_ops')
   drivers//spi/spi-renesas-rpc.c:486:3: error: 'const struct spi_controller_mem_ops' has no member named 'dirmap_write'
     .dirmap_write = rpc_spi_mem_dirmap_write,
      ^~~~~~~~~~~~
   drivers//spi/spi-renesas-rpc.c:486:18: warning: excess elements in struct initializer
     .dirmap_write = rpc_spi_mem_dirmap_write,
                     ^~~~~~~~~~~~~~~~~~~~~~~~
   drivers//spi/spi-renesas-rpc.c:486:18: note: (near initialization for 'rpc_spi_mem_ops')
   cc1: some warnings being treated as errors

vim +485 drivers//spi/spi-renesas-rpc.c

   480	
   481	static const struct spi_controller_mem_ops rpc_spi_mem_ops = {
   482		.supports_op = rpc_spi_mem_supports_op,
   483		.exec_op = rpc_spi_mem_exec_op,
 > 484		.dirmap_create = rpc_spi_mem_dirmap_create,
 > 485		.dirmap_read = rpc_spi_mem_dirmap_read,
   486		.dirmap_write = rpc_spi_mem_dirmap_write,
   487	};
   488	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Geert Uytterhoeven Nov. 20, 2018, 8:01 a.m. UTC | #6
Hi Mason,

On Mon, Nov 19, 2018 at 11:01 AM Mason Yang <masonccyang@mxic.com.tw> wrote:
> Add a driver for Renesas R-Car D3 RPC SPI controller driver.
>
> Signed-off-by: Mason Yang <masonccyang@mxic.com.tw>

Thanks for your patch!

> --- /dev/null
> +++ b/drivers/spi/spi-renesas-rpc.c
> @@ -0,0 +1,750 @@

> +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
> +{
> +       int ret;
> +
> +       if (rpc->cur_speed_hz == freq)
> +               return 0;
> +
> +       clk_disable_unprepare(rpc->clk_rpc);
> +       ret = clk_set_rate(rpc->clk_rpc, freq);
> +       if (ret)
> +               return ret;
> +
> +       ret = clk_prepare_enable(rpc->clk_rpc);
> +       if (ret)
> +               return ret;

The clk_{disable_unprepare,prepare_enable}() may be needed on the Macronix
controller you based this driver on, but will be futile on Renesas SoCs.

As the RPC is part of the CPG/MSSR clock domain, its clock will be controlled
by the Runtime PM.  As you've already called pm_runtime_get_sync() from your
.probe() calback, Runtime PM will have enabled the clock.
If you disable it manually, you create an imbalance between automatic and
manual clock control.

So please don't control the clock explicitly, but always use
pm_runtime_*() calls.

> +static int __maybe_unused rpc_spi_runtime_suspend(struct device *dev)
> +{
> +       struct platform_device *pdev = to_platform_device(dev);
> +       struct spi_master *master = platform_get_drvdata(pdev);
> +       struct rpc_spi *rpc = spi_master_get_devdata(master);
> +
> +       clk_disable_unprepare(rpc->clk_rpc);

At this point, the clock is enabled due to Runtime PM, and you disable it
manually.
During system suspend, the clock will be disabled by the PM framework
again, leading to a negative enable count.
I expect you to see warning splats during system suspend.
Hence please drop the explicit clock management from this function.

I'm not familiar with the spimem framework, but for a normal SPI controller,
you want to call spi_master_resume(master) here.
See e.g. commit c1ca59c22c56930b ("spi: rspi: Fix invalid SPI use during
system suspend")

> +
> +       return 0;
> +}
> +
> +static int __maybe_unused rpc_spi_runtime_resume(struct device *dev)
> +{
> +       struct platform_device *pdev = to_platform_device(dev);
> +       struct spi_master *master = platform_get_drvdata(pdev);
> +       struct rpc_spi *rpc = spi_master_get_devdata(master);
> +       int ret;
> +
> +       ret = clk_prepare_enable(rpc->clk_rpc);
> +       if (ret)
> +               dev_err(dev, "Can't enable rpc->clk_rpc\n");

Likewise, please drop the explicit clock management here. The PM core
code will handle it through the clock domain.

+ spi_master_resume(master)

> +
> +       return ret;
> +}
> +
> +static const struct dev_pm_ops rpc_spi_dev_pm_ops = {
> +       SET_RUNTIME_PM_OPS(rpc_spi_runtime_suspend,
> +                          rpc_spi_runtime_resume, NULL)

Ah, you only use these for Runtime PM.  Not needed, as Runtime PM handles
the clock domain without any callbacks.

With spi_master_{suspend,resume}() added, you can use SIMPLE_DEV_PM_OPS(),
and make everything work during/after system suspend.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Boris Brezillon Nov. 20, 2018, 8:10 a.m. UTC | #7
On Tue, 20 Nov 2018 09:01:29 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> > --- /dev/null
> > +++ b/drivers/spi/spi-renesas-rpc.c
> > @@ -0,0 +1,750 @@  
> 
> > +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
> > +{
> > +       int ret;
> > +
> > +       if (rpc->cur_speed_hz == freq)
> > +               return 0;
> > +
> > +       clk_disable_unprepare(rpc->clk_rpc);
> > +       ret = clk_set_rate(rpc->clk_rpc, freq);
> > +       if (ret)
> > +               return ret;
> > +
> > +       ret = clk_prepare_enable(rpc->clk_rpc);
> > +       if (ret)
> > +               return ret;  
> 
> The clk_{disable_unprepare,prepare_enable}() may be needed on the Macronix
> controller you based this driver on, but will be futile on Renesas SoCs.
> 
> As the RPC is part of the CPG/MSSR clock domain, its clock will be controlled
> by the Runtime PM.  As you've already called pm_runtime_get_sync() from your
> .probe() calback, Runtime PM will have enabled the clock.
> If you disable it manually, you create an imbalance between automatic and
> manual clock control.
> 
> So please don't control the clock explicitly, but always use
> pm_runtime_*() calls.

More about that. The reason we did that on MXIC is that the clk rate
can't be changed when the clk is enabled. So we have to

1/ explicitly disable the clk that has been enabled by runtime PM
2/ set the new rate
3/ re-enable the clk

So the clk enable/disable are not unbalanced, but it's also true that
this disable/set_rate/enable dance might be unneeded on your platform.
Marek Vasut Nov. 20, 2018, 1:09 p.m. UTC | #8
On 11/20/2018 08:23 AM, masonccyang@mxic.com.tw wrote:
> Hi Marek,

Hi,

>> Marek Vasut <marek.vasut@gmail.com>
>> 2018/11/19 下午 10:12
>>
>> To
>>
>> > +
>> > +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
>> > +{
>> > +   int ret;
>> > +
>> > +   if (rpc->cur_speed_hz == freq)
>> > +      return 0;
>> > +
>> > +   clk_disable_unprepare(rpc->clk_rpc);
>> > +   ret = clk_set_rate(rpc->clk_rpc, freq);
>> > +   if (ret)
>> > +      return ret;
>> > +
>> > +   ret = clk_prepare_enable(rpc->clk_rpc);
>> > +   if (ret)
>> > +      return ret;
>>
>> Is this clock disable/update/enable really needed ? I'd think that
>> clk_set_rate() would handle the rate update correctly.
> 
> This is for run time PM mechanism in spi-mem layer and __spi_sync(),
> you may refer to another patch [1].
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-4.21&id=b942d80b0a394e8ea18fce3b032b4700439e8ca3

I think Geert commented on the clock topic, so let's move it there.
Disabling and enabling clock to change their rate looks real odd to me.

>> > +   rpc->cur_speed_hz = freq;
>> > +   return ret;
>> > +}
>> > +
>> > +static void rpc_spi_hw_init(struct rpc_spi *rpc)
>> > +{
>> > +   /*
>> > +    * NOTE: The 0x260 are undocumented bits, but they must be set.
>> > +    */
>>
>> FYI:
>>
> http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/spi/renesas_rpc_spi.c#l207
>>
>> I think the STRTIM should be 6 .
>>
> 
> In my D3 Draak board, the STRTIM is 0x3 for on board qspi flash and
> mx25uw51245g.
> And this is also refer to Renesas R-Car Gen3 bare-metal code,
> mini-monitor v4.01.

The copy of minimon I have says 6 , but maybe this is flash specific ?

[...]

>> > +         writel(rpc->cmd, rpc->regs + RPC_SMCMR);
>> > +         writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
>> > +         writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
>> > +         writel(rpc->smenr, rpc->regs + RPC_SMENR);
>> > +         writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
>> > +         ret = wait_msg_xfer_end(rpc);
>> > +         if (ret)
>> > +            goto out;
>> > +
>> > +         data = readl(rpc->regs + RPC_SMRDR0);
>> > +         memcpy_fromio(rx_buf + pos, (void *)&data, nbytes);
>> > +         pos += nbytes;
>> > +      }
>> > +   } else {
>> > +      writel(rpc->cmd, rpc->regs + RPC_SMCMR);
>> > +      writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
>> > +      writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
>> > +      writel(rpc->smenr, rpc->regs + RPC_SMENR);
>> > +      writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
>> > +      ret = wait_msg_xfer_end(rpc);
>> > +   }
>> > +out:
>>
>> Dont you need to stop the RPC somehow in case the transmission fails ?
> 
> It seems there is no any RPC registers bit to monitor xfer fail !

What happens if wait_msg_xfer_end() returns non-zero ? I guess that
means the transfer timed out ?

[...]

>> > +static const struct of_device_id rpc_spi_of_ids[] = {
>> > +   { .compatible = "renesas,rpc-r8a77995", },
>> > +   { /* sentinel */ }
>> > +};
>> > +MODULE_DEVICE_TABLE(of, rpc_spi_of_ids);
>> > +
>> > +static struct platform_driver rpc_spi_driver = {
>> > +   .probe = rpc_spi_probe,
>> > +   .remove = rpc_spi_remove,
>> > +   .driver = {
>> > +      .name = "rpc-spi",
>> > +      .of_match_table = rpc_spi_of_ids,
>> > +      .pm = &rpc_spi_dev_pm_ops,
>> > +   },
>> > +};
>> > +module_platform_driver(rpc_spi_driver);
>> > +
>> > +MODULE_AUTHOR("Mason Yang <masonccyang@mxic.com.tw>");
>> > +MODULE_DESCRIPTION("Renesas R-Car D3 RPC SPI controller driver");
>>
>> This is not D3 specific and not SPI-only controller btw.
> 
> In R-Car Gen3, there are some registers(i.e,. RPC_PHYCNT) in different
> setting
> for R-Car H3, M3-W, V3M, V3H, D3, M3-N and E3 model.
> 
> I test this patch is based on D3 Draak board, it works fine but I am not
> sure
> if these registers setting is ok for others R-Card model.
> 
> I think this could be a reference when patch others Gen3 model is needed.
You can take a look into the U-Boot driver(s) I linked, that's used on
the other SoCs you listed (except for V3H).
Mark Brown Nov. 20, 2018, 1:26 p.m. UTC | #9
On Mon, Nov 19, 2018 at 11:10:04PM +0100, Marek Vasut wrote:
> On 11/19/2018 04:27 PM, Mark Brown wrote:

> > The SPDX header needs to be C++ style so I push people to make the whole
> > block C++ otherwise it looks messy.

> OK, I'm not gonna wrestle you on this, but I think it looks horrible ;-)

I don't really like the C++ comment in the first place :(
Boris Brezillon Nov. 20, 2018, 1:32 p.m. UTC | #10
On Tue, 20 Nov 2018 14:09:05 +0100
Marek Vasut <marek.vasut@gmail.com> wrote:

> On 11/20/2018 08:23 AM, masonccyang@mxic.com.tw wrote:
> > Hi Marek,  
> 
> Hi,
> 
> >> Marek Vasut <marek.vasut@gmail.com>
> >> 2018/11/19 下午 10:12
> >>
> >> To
> >>  
> >> > +
> >> > +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
> >> > +{
> >> > +   int ret;
> >> > +
> >> > +   if (rpc->cur_speed_hz == freq)
> >> > +      return 0;
> >> > +
> >> > +   clk_disable_unprepare(rpc->clk_rpc);
> >> > +   ret = clk_set_rate(rpc->clk_rpc, freq);
> >> > +   if (ret)
> >> > +      return ret;
> >> > +
> >> > +   ret = clk_prepare_enable(rpc->clk_rpc);
> >> > +   if (ret)
> >> > +      return ret;  
> >>
> >> Is this clock disable/update/enable really needed ? I'd think that
> >> clk_set_rate() would handle the rate update correctly.  
> > 
> > This is for run time PM mechanism in spi-mem layer and __spi_sync(),
> > you may refer to another patch [1].
> > 
> > [1]
> > https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-4.21&id=b942d80b0a394e8ea18fce3b032b4700439e8ca3  
> 
> I think Geert commented on the clock topic, so let's move it there.
> Disabling and enabling clock to change their rate looks real odd to me.

Look at the CLK_SET_RATE_GATE definition and its users and you'll see
it's not unusual to have such constraints on clks. Maybe your HW does
not have such constraints, but it's not particularly odd to do that
(though it could probably be automated by the clk framework somehow).
Marek Vasut Nov. 20, 2018, 1:33 p.m. UTC | #11
On 11/20/2018 02:26 PM, Mark Brown wrote:
> On Mon, Nov 19, 2018 at 11:10:04PM +0100, Marek Vasut wrote:
>> On 11/19/2018 04:27 PM, Mark Brown wrote:
> 
>>> The SPDX header needs to be C++ style so I push people to make the whole
>>> block C++ otherwise it looks messy.
> 
>> OK, I'm not gonna wrestle you on this, but I think it looks horrible ;-)
> 
> I don't really like the C++ comment in the first place :(

Then we are in agreement :-)
Marek Vasut Nov. 20, 2018, 1:35 p.m. UTC | #12
On 11/20/2018 02:32 PM, Boris Brezillon wrote:
> On Tue, 20 Nov 2018 14:09:05 +0100
> Marek Vasut <marek.vasut@gmail.com> wrote:
> 
>> On 11/20/2018 08:23 AM, masonccyang@mxic.com.tw wrote:
>>> Hi Marek,  
>>
>> Hi,
>>
>>>> Marek Vasut <marek.vasut@gmail.com>
>>>> 2018/11/19 下午 10:12
>>>>
>>>> To
>>>>  
>>>>> +
>>>>> +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
>>>>> +{
>>>>> +   int ret;
>>>>> +
>>>>> +   if (rpc->cur_speed_hz == freq)
>>>>> +      return 0;
>>>>> +
>>>>> +   clk_disable_unprepare(rpc->clk_rpc);
>>>>> +   ret = clk_set_rate(rpc->clk_rpc, freq);
>>>>> +   if (ret)
>>>>> +      return ret;
>>>>> +
>>>>> +   ret = clk_prepare_enable(rpc->clk_rpc);
>>>>> +   if (ret)
>>>>> +      return ret;  
>>>>
>>>> Is this clock disable/update/enable really needed ? I'd think that
>>>> clk_set_rate() would handle the rate update correctly.  
>>>
>>> This is for run time PM mechanism in spi-mem layer and __spi_sync(),
>>> you may refer to another patch [1].
>>>
>>> [1]
>>> https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git/commit/?h=for-4.21&id=b942d80b0a394e8ea18fce3b032b4700439e8ca3  
>>
>> I think Geert commented on the clock topic, so let's move it there.
>> Disabling and enabling clock to change their rate looks real odd to me.
> 
> Look at the CLK_SET_RATE_GATE definition and its users and you'll see
> it's not unusual to have such constraints on clks. Maybe your HW does
> not have such constraints, but it's not particularly odd to do that
> (though it could probably be automated by the clk framework somehow).

I think you stated my concern right at the end, good, no need for me to
add to this. Yes, I don't think any random driver should deal with
peculiarities of the clock controller.
Marek Vasut Nov. 23, 2018, 1:34 p.m. UTC | #13
On 11/23/2018 01:45 AM, masonccyang@mxic.com.tw wrote:
> Hi Marek,

Hi,

>> > +
>> > +struct rpc_spi {
>> > +   struct clk *clk_rpc;
>> > +   void __iomem *regs;
>> > +   struct {
>> > +      void __iomem *map;
>> > +      dma_addr_t dma;
>> > +      size_t size;
>> > +   } linear;
>>
>> Does this need it's own struct ?
>>
> 
> yup, I think it's better.
> In case no "dirmap" in dtb and no direct mapping mode implemented.
> 
> 
>> > +   u32 cur_speed_hz;
>> > +   u32 cmd;
>> > +   u32 addr;
>> > +   u32 dummy;
>> > +   u32 smcr;
>> > +   u32 smenr;
>> > +   u32 xferlen;
>> > +   u32 totalxferlen;
>>
>> This register cache might be a good candidate for regmap ?
> 
> I don't know what does it mean ?
> Could you give me more information!

See include/linux/regmap.h and git grep regmap drivers/ for examples.

>> > +   enum spi_mem_data_dir xfer_dir;
>> > +};
>> > +
>> > +static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
>> > +{
>> > +   int ret;
>> > +
>> > +   if (rpc->cur_speed_hz == freq)
>> > +      return 0;
>> > +
>> > +   clk_disable_unprepare(rpc->clk_rpc);
>> > +   ret = clk_set_rate(rpc->clk_rpc, freq);
>> > +   if (ret)
>> > +      return ret;
>> > +
>> > +   ret = clk_prepare_enable(rpc->clk_rpc);
>> > +   if (ret)
>> > +      return ret;
>>
>> Is this clock disable/update/enable really needed ? I'd think that
>> clk_set_rate() would handle the rate update correctly.
> 
> As Gerrt mentioned, I will remove them.
> 
> 
>> > +static int wait_msg_xfer_end(struct rpc_spi *rpc)
>> > +{
>> > +   u32 sts;
>> > +
>> > +   return readl_poll_timeout(rpc->regs + RPC_CMNSR, sts,
>> > +              sts & RPC_CMNSR_TEND, 0, USEC_PER_SEC);
>> > +}
>> > +
>> > +static u8 rpc_bits_xfer(u32 nbytes)
>> > +{
>> > +   u8 databyte;
>> > +
>> > +   switch (nbytes) {
>>
>> Did you ever test unaligned writes and reads ? There are some nasty edge
>> cases in those.
>>
>> Also, I think you can calculate the number of set bits using a simple
>> function, so the switch-case might not even be needed.
>>
> 
> Any example function ?

Nope, you'd have to think of one. You need to fill $nbytes bits from top
down. I think you can somehow use GENMASK() .

>> > +   case 1:
>> > +      databyte = 0x8;
>> > +      break;
>> > +   case 2:
>> > +      databyte = 0xc;
>> > +      break;
>> > +   default:
>> > +      databyte = 0xf;
>> > +      break;
>> > +   }
>> > +
>> > +   return databyte;
>> > +}
>> > +
>> > +static int rpc_spi_io_xfer(struct rpc_spi *rpc,
>> > +            const void *tx_buf, void *rx_buf)
>> > +{
>> > +   u32 smenr, smcr, data, pos = 0;
>> > +   int ret = 0;
>> > +
>> > +   writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
>> > +          RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs +
> RPC_CMNCR);
>> > +   writel(0x0, rpc->regs + RPC_SMDRENR);
>> > +
>> > +   if (tx_buf) {
>> > +      writel(rpc->cmd, rpc->regs + RPC_SMCMR);
>> > +      writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
>> > +      writel(rpc->addr, rpc->regs + RPC_SMADR);
>> > +      smenr = rpc->smenr;
>> > +
>> > +      while (pos < rpc->xferlen) {
>> > +         u32 nbytes = rpc->xferlen  - pos;
>> > +
>> > +         writel(*(u32 *)(tx_buf + pos), rpc->regs + RPC_SMWDR0);
>> > +
>> > +         if (nbytes > 4) {
>> > +            nbytes = 4;
>> > +            smcr = rpc->smcr |
>> > +                   RPC_SMCR_SPIE | RPC_SMCR_SSLKP;
>> > +         } else {
>> > +            smcr = rpc->smcr | RPC_SMCR_SPIE;
>> > +         }
>> > +
>> > +         writel(smenr, rpc->regs + RPC_SMENR);
>> > +         writel(smcr, rpc->regs + RPC_SMCR);
>> > +         ret = wait_msg_xfer_end(rpc);
>> > +         if (ret)
>> > +            goto out;
>> > +
>> > +         pos += nbytes;
>> > +         smenr = rpc->smenr & ~RPC_SMENR_CDE &
>> > +                    ~RPC_SMENR_ADE(0xf);
>> > +      }
>> > +   } else if (rx_buf) {
>> > +      while (pos < rpc->xferlen) {
>> > +         u32 nbytes = rpc->xferlen  - pos;
>> > +
>> > +         if (nbytes > 4)
>> > +            nbytes = 4;
>> > +
>> > +         writel(rpc->cmd, rpc->regs + RPC_SMCMR);
>> > +         writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
>> > +         writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
>> > +         writel(rpc->smenr, rpc->regs + RPC_SMENR);
>> > +         writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
>> > +         ret = wait_msg_xfer_end(rpc);
>> > +         if (ret)
>> > +            goto out;
>> > +
>> > +         data = readl(rpc->regs + RPC_SMRDR0);
>> > +         memcpy_fromio(rx_buf + pos, (void *)&data, nbytes);
>> > +         pos += nbytes;
>> > +      }
>> > +   } else {
>> > +      writel(rpc->cmd, rpc->regs + RPC_SMCMR);
>> > +      writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
>> > +      writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
>> > +      writel(rpc->smenr, rpc->regs + RPC_SMENR);
>> > +      writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
>> > +      ret = wait_msg_xfer_end(rpc);
>> > +   }
>> > +out:
>>
>> Dont you need to stop the RPC somehow in case the transmission fails ?
>>
> 
> I can't find any RPC registers can do this !
> 
> Do you know how to do this ?

It should be in the RPC datasheet ? It's likely going to involve SMCR,
possibly clear SPIE bit and maybe some more.

>> > +   writel(rpc->cmd, rpc->regs + RPC_SMCMR);
>> > +   writel(offs, rpc->regs + RPC_SMADR);
>> > +   writel(rpc->smenr, rpc->regs + RPC_SMENR);
>> > +   writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
>> > +   ret = wait_msg_xfer_end(rpc);
>> > +   if (ret)
>> > +      goto out;
>> > +
>> > +   writel(RPC_DRCR_RCF, rpc->regs + RPC_DRCR);
>> > +   writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0) | 0x260,
>> > +          rpc->regs + RPC_PHYCNT);
>> > +
>> > +   return len;
>> > +out:
>>
>> Shouldn't you shut the controller down if the xfer fails ?
> 
> Any registers can shut down RPC controller ?
> SW reset ?

Possibly, can you research it ?

>> > +   return ret;
>> > +}
>> > +
>> > +static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
>> > +{
>> > +   struct rpc_spi *rpc =
> spi_master_get_devdata(desc->mem->spi->master);
>> > +
>> > +   if (desc->info.offset + desc->info.length > U32_MAX)
>> > +      return -ENOTSUPP;
>> > +
>> > +   if (!rpc_spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
>> > +      return -ENOTSUPP;
>> > +
>> > +   if (!rpc->linear.map &&
>> > +       desc->info.op_tmpl.data.dir == SPI_MEM_DATA_IN)
>> > +      return -ENOTSUPP;
>> > +
>> > +   return 0;
>> > +}
>> > +
>> > +static int rpc_spi_mem_exec_op(struct spi_mem *mem,
>> > +                const struct spi_mem_op *op)
>> > +{
>> > +   struct rpc_spi *rpc = spi_master_get_devdata(mem->spi->master);
>> > +   int ret;
>> > +
>> > +   ret = rpc_spi_set_freq(rpc, mem->spi->max_speed_hz);
>> > +   if (ret)
>> > +      return ret;
>> > +
>> > +   rpc_spi_mem_set_prep_op_cfg(mem->spi, op, NULL, NULL);
>> > +
>> > +   ret = rpc_spi_io_xfer(rpc,
>> > +               op->data.dir == SPI_MEM_DATA_OUT ?
>> > +               op->data.buf.out : NULL,
>> > +               op->data.dir == SPI_MEM_DATA_IN ?
>> > +               op->data.buf.in : NULL);
>> > +
>> > +   return ret;
>> > +}
>> > +
>> > +static const struct spi_controller_mem_ops rpc_spi_mem_ops = {
>> > +   .supports_op = rpc_spi_mem_supports_op,
>> > +   .exec_op = rpc_spi_mem_exec_op,
>> > +   .dirmap_create = rpc_spi_mem_dirmap_create,
>> > +   .dirmap_read = rpc_spi_mem_dirmap_read,
>> > +   .dirmap_write = rpc_spi_mem_dirmap_write,
>> > +};
>> > +
>> > +static void rpc_spi_transfer_setup(struct rpc_spi *rpc,
>> > +               struct spi_message *msg)
>> > +{
>> > +   struct spi_transfer *t, xfer[4] = { };
>> > +   u32 i, xfercnt, xferpos = 0;
>> > +
>> > +   rpc->totalxferlen = 0;
>> > +   list_for_each_entry(t, &msg->transfers, transfer_list) {
>> > +      if (t->tx_buf) {
>> > +         xfer[xferpos].tx_buf = t->tx_buf;
>> > +         xfer[xferpos].tx_nbits = t->tx_nbits;
>> > +      }
>> > +
>> > +      if (t->rx_buf) {
>> > +         xfer[xferpos].rx_buf = t->rx_buf;
>> > +         xfer[xferpos].rx_nbits = t->rx_nbits;
>> > +      }
>> > +
>> > +      if (t->len) {
>> > +         xfer[xferpos++].len = t->len;
>> > +         rpc->totalxferlen += t->len;
>> > +      }
>> > +   }
>> > +
>> > +   xfercnt = xferpos;
>> > +   rpc->xferlen = xfer[--xferpos].len;
>> > +   rpc->cmd = RPC_SMCMR_CMD(((u8 *)xfer[0].tx_buf)[0]);
>>
>> Is the cast needed ?
> 
> ?

Sorry, I don't understand your question.
To rephrase my original question, is the (u8 *) cast needed ?

>> > +   rpc->smenr = RPC_SMENR_CDE | RPC_SMENR_CDB(fls(xfer[0].tx_nbits
>>> 1));
>> > +   rpc->addr = 0;
>> > +
>> > +   if (xfercnt > 2 && xfer[1].len && xfer[1].tx_buf) {
>> > +      rpc->smenr |= RPC_SMENR_ADB(fls(xfer[1].tx_nbits >> 1));
>> > +      for (i = 0; i < xfer[1].len; i++)
>> > +         rpc->addr |= (u32)((u8 *)xfer[1].tx_buf)[i]
>> > +               << (8 * (xfer[1].len - i - 1));
>> > +
>> > +      if (xfer[1].len == 4)
>> > +         rpc->smenr |= RPC_SMENR_ADE(0xf);
>> > +      else
>> > +         rpc->smenr |= RPC_SMENR_ADE(0x7);
>> > +   }
>> > +
>> > +   switch (xfercnt) {
>> > +   case 2:
>> > +      if (xfer[1].rx_buf) {
>> > +         rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
>> > +                  (xfer[1].len)) | RPC_SMENR_SPIDB(fls
>> > +                  (xfer[1].rx_nbits >> 1));
>>
>> How much of this register value calculation could be somehow
>> deduplicated ? It seems to be almost the same thing copied thrice here.
> 
> I don't get your point!
> 
> The 2'nd transfer may be
> 1) spi-address
> 2) tx_buf[] for write registers.
> 3) rx_buf[] for read status.
> 
> parse them and write to rpc->addr and so on.
> Or you have a better way to do this ?

Each of the case statement options has almost the same stuff in it. Can
this be somehow reworked so that it wouldn't be three copies of almost
the same ?

[...]
diff mbox series

Patch

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 7d3a5c9..093006a 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -528,6 +528,12 @@  config SPI_RSPI
 	help
 	  SPI driver for Renesas RSPI and QSPI blocks.
 
+config SPI_RENESAS_RPC
+	tristate "Renesas R-Car D3 RPC SPI controller"
+	depends on SUPERH || ARCH_RENESAS || COMPILE_TEST
+	help
+	  SPI driver for Renesas R-Car D3 RPC.
+
 config SPI_QCOM_QSPI
 	tristate "QTI QSPI controller"
 	depends on ARCH_QCOM
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 3575205..5d5c523 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -81,6 +81,7 @@  obj-$(CONFIG_SPI_QUP)			+= spi-qup.o
 obj-$(CONFIG_SPI_ROCKCHIP)		+= spi-rockchip.o
 obj-$(CONFIG_SPI_RB4XX)			+= spi-rb4xx.o
 obj-$(CONFIG_SPI_RSPI)			+= spi-rspi.o
+obj-$(CONFIG_SPI_RENESAS_RPC)		+= spi-renesas-rpc.o
 obj-$(CONFIG_SPI_S3C24XX)		+= spi-s3c24xx-hw.o
 spi-s3c24xx-hw-y			:= spi-s3c24xx.o
 spi-s3c24xx-hw-$(CONFIG_SPI_S3C24XX_FIQ) += spi-s3c24xx-fiq.o
diff --git a/drivers/spi/spi-renesas-rpc.c b/drivers/spi/spi-renesas-rpc.c
new file mode 100644
index 0000000..00b9d8f
--- /dev/null
+++ b/drivers/spi/spi-renesas-rpc.c
@@ -0,0 +1,750 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// Copyright (C) 2018 ~ 2019 Renesas Solutions Corp.
+// Copyright (C) 2018 Macronix International Co., Ltd.
+//
+// R-Car D3 RPC SPI/QSPI/Octa driver
+//
+// Authors:
+//	Mason Yang <masonccyang@mxic.com.tw>
+//
+
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+#define RPC_CMNCR		0x0000	/* R/W */
+#define RPC_CMNCR_MD		BIT(31)
+#define RPC_CMNCR_SFDE		BIT(24)
+#define RPC_CMNCR_MOIIO3(val)	(((val) & 0x3) << 22)
+#define RPC_CMNCR_MOIIO2(val)	(((val) & 0x3) << 20)
+#define RPC_CMNCR_MOIIO1(val)	(((val) & 0x3) << 18)
+#define RPC_CMNCR_MOIIO0(val)	(((val) & 0x3) << 16)
+#define RPC_CMNCR_MOIIO_HIZ	(RPC_CMNCR_MOIIO0(3) | RPC_CMNCR_MOIIO1(3) | \
+				 RPC_CMNCR_MOIIO2(3) | RPC_CMNCR_MOIIO3(3))
+#define RPC_CMNCR_IO3FV(val)	(((val) & 0x3) << 14)
+#define RPC_CMNCR_IO2FV(val)	(((val) & 0x3) << 12)
+#define RPC_CMNCR_IO0FV(val)	(((val) & 0x3) << 8)
+#define RPC_CMNCR_IOFV_HIZ	(RPC_CMNCR_IO0FV(3) | RPC_CMNCR_IO2FV(3) | \
+				 RPC_CMNCR_IO3FV(3))
+#define RPC_CMNCR_CPHAT		BIT(6)
+#define RPC_CMNCR_CPHAR		BIT(5)
+#define RPC_CMNCR_SSLP		BIT(4)
+#define RPC_CMNCR_CPOL		BIT(3)
+#define RPC_CMNCR_BSZ(val)	(((val) & 0x3) << 0)
+
+#define RPC_SSLDR		0x0004	/* R/W */
+#define RPC_SSLDR_SPNDL(d)	(((d) & 0x7) << 16)
+#define RPC_SSLDR_SLNDL(d)	(((d) & 0x7) << 8)
+#define RPC_SSLDR_SCKDL(d)	(((d) & 0x7) << 0)
+
+#define RPC_DRCR		0x000C	/* R/W */
+#define RPC_DRCR_SSLN		BIT(24)
+#define RPC_DRCR_RBURST(v)	(((v) & 0x1F) << 16)
+#define RPC_DRCR_RCF		BIT(9)
+#define RPC_DRCR_RBE		BIT(8)
+#define RPC_DRCR_SSLE		BIT(0)
+
+#define RPC_DRCMR		0x0010	/* R/W */
+#define RPC_DRCMR_CMD(c)	(((c) & 0xFF) << 16)
+#define RPC_DRCMR_OCMD(c)	(((c) & 0xFF) << 0)
+
+#define RPC_DREAR		0x0014	/* R/W */
+#define RPC_DREAR_EAC		BIT(0)
+
+#define RPC_DROPR		0x0018	/* R/W */
+
+#define RPC_DRENR		0x001C	/* R/W */
+#define RPC_DRENR_CDB(o)	(u32)((((o) & 0x3) << 30))
+#define RPC_DRENR_OCDB(o)	(((o) & 0x3) << 28)
+#define RPC_DRENR_ADB(o)	(((o) & 0x3) << 24)
+#define RPC_DRENR_OPDB(o)	(((o) & 0x3) << 20)
+#define RPC_DRENR_SPIDB(o)	(((o) & 0x3) << 16)
+#define RPC_DRENR_DME		BIT(15)
+#define RPC_DRENR_CDE		BIT(14)
+#define RPC_DRENR_OCDE		BIT(12)
+#define RPC_DRENR_ADE(v)	(((v) & 0xF) << 8)
+#define RPC_DRENR_OPDE(v)	(((v) & 0xF) << 4)
+
+#define RPC_SMCR		0x0020	/* R/W */
+#define RPC_SMCR_SSLKP		BIT(8)
+#define RPC_SMCR_SPIRE		BIT(2)
+#define RPC_SMCR_SPIWE		BIT(1)
+#define RPC_SMCR_SPIE		BIT(0)
+
+#define RPC_SMCMR		0x0024	/* R/W */
+#define RPC_SMCMR_CMD(c)	(((c) & 0xFF) << 16)
+#define RPC_SMCMR_OCMD(c)	(((c) & 0xFF) << 0)
+
+#define RPC_SMADR		0x0028	/* R/W */
+#define RPC_SMOPR		0x002C	/* R/W */
+#define RPC_SMOPR_OPD0(o)	(((o) & 0xFF) << 0)
+#define RPC_SMOPR_OPD1(o)	(((o) & 0xFF) << 8)
+#define RPC_SMOPR_OPD2(o)	(((o) & 0xFF) << 16)
+#define RPC_SMOPR_OPD3(o)	(((o) & 0xFF) << 24)
+
+#define RPC_SMENR		0x0030	/* R/W */
+#define RPC_SMENR_CDB(o)	(((o) & 0x2) << 30)
+#define RPC_SMENR_OCDB(o)	(((o) & 0x2) << 28)
+#define RPC_SMENR_ADB(o)	(((o) & 0x2) << 24)
+#define RPC_SMENR_OPDB(o)	(((o) & 0x2) << 20)
+#define RPC_SMENR_SPIDB(o)	(((o) & 0x2) << 16)
+#define RPC_SMENR_DME		BIT(15)
+#define RPC_SMENR_CDE		BIT(14)
+#define RPC_SMENR_OCDE		BIT(12)
+#define RPC_SMENR_ADE(v)	(((v) & 0xF) << 8)
+#define RPC_SMENR_OPDE(v)	(((v) & 0xF) << 4)
+#define RPC_SMENR_SPIDE(v)	(((v) & 0xF) << 0)
+
+#define RPC_SMRDR0		0x0038	/* R */
+#define RPC_SMRDR1		0x003C	/* R */
+#define RPC_SMWDR0		0x0040	/* W */
+#define RPC_SMWDR1		0x0044	/* W */
+
+#define RPC_CMNSR		0x0048	/* R */
+#define RPC_CMNSR_SSLF		BIT(1)
+#define	RPC_CMNSR_TEND		BIT(0)
+
+#define RPC_DRDMCR		0x0058	/* R/W */
+#define RPC_DRDRENR		0x005C	/* R/W */
+
+#define RPC_SMDMCR		0x0060	/* R/W */
+#define RPC_SMDMCR_DMCYC(v)	((((v) - 1) & 0x1F) << 0)
+
+#define RPC_SMDRENR		0x0064	/* R/W */
+#define RPC_SMDRENR_HYPE	(0x5 << 12)
+#define RPC_SMDRENR_ADDRE	BIT(8)
+#define RPC_SMDRENR_OPDRE	BIT(4)
+#define RPC_SMDRENR_SPIDRE	BIT(0)
+
+#define RPC_PHYCNT		0x007C	/* R/W */
+#define RPC_PHYCNT_CAL		BIT(31)
+#define PRC_PHYCNT_OCTA_AA	BIT(22)
+#define PRC_PHYCNT_OCTA_SA	BIT(23)
+#define PRC_PHYCNT_EXDS		BIT(21)
+#define RPC_PHYCNT_OCT		BIT(20)
+#define RPC_PHYCNT_STRTIM(v)	(((v) & 0x7) << 15)
+#define RPC_PHYCNT_WBUF2	BIT(4)
+#define RPC_PHYCNT_WBUF		BIT(2)
+#define RPC_PHYCNT_MEM(v)	(((v) & 0x3) << 0)
+
+#define RPC_PHYOFFSET1		0x0080	/* R/W */
+#define RPC_PHYOFFSET2		0x0084	/* R/W */
+
+#define RPC_WBUF		0x8000	/* Write Buffer */
+#define RPC_WBUF_SIZE		256	/* Write Buffer size */
+
+struct rpc_spi {
+	struct clk *clk_rpc;
+	void __iomem *regs;
+	struct {
+		void __iomem *map;
+		dma_addr_t dma;
+		size_t size;
+	} linear;
+	u32 cur_speed_hz;
+	u32 cmd;
+	u32 addr;
+	u32 dummy;
+	u32 smcr;
+	u32 smenr;
+	u32 xferlen;
+	u32 totalxferlen;
+	enum spi_mem_data_dir xfer_dir;
+};
+
+static int rpc_spi_set_freq(struct rpc_spi *rpc, unsigned long freq)
+{
+	int ret;
+
+	if (rpc->cur_speed_hz == freq)
+		return 0;
+
+	clk_disable_unprepare(rpc->clk_rpc);
+	ret = clk_set_rate(rpc->clk_rpc, freq);
+	if (ret)
+		return ret;
+
+	ret = clk_prepare_enable(rpc->clk_rpc);
+	if (ret)
+		return ret;
+
+	rpc->cur_speed_hz = freq;
+	return ret;
+}
+
+static void rpc_spi_hw_init(struct rpc_spi *rpc)
+{
+	/*
+	 * NOTE: The 0x260 are undocumented bits, but they must be set.
+	 */
+	writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0x3) | 0x260,
+	       rpc->regs + RPC_PHYCNT);
+
+	/*
+	 * NOTE: The 0x31511144 and 0x431 are undocumented bits,
+	 *	 but they must be set for RPC_PHYOFFSET1 & RPC_PHYOFFSET2.
+	 */
+	writel(0x31511144, rpc->regs + RPC_PHYOFFSET1);
+	writel(0x431, rpc->regs + RPC_PHYOFFSET2);
+
+	writel(RPC_SSLDR_SPNDL(7) | RPC_SSLDR_SLNDL(7) |
+	       RPC_SSLDR_SCKDL(7), rpc->regs + RPC_SSLDR);
+}
+
+static int wait_msg_xfer_end(struct rpc_spi *rpc)
+{
+	u32 sts;
+
+	return readl_poll_timeout(rpc->regs + RPC_CMNSR, sts,
+				  sts & RPC_CMNSR_TEND, 0, USEC_PER_SEC);
+}
+
+static u8 rpc_bits_xfer(u32 nbytes)
+{
+	u8 databyte;
+
+	switch (nbytes) {
+	case 1:
+		databyte = 0x8;
+		break;
+	case 2:
+		databyte = 0xc;
+		break;
+	default:
+		databyte = 0xf;
+		break;
+	}
+
+	return databyte;
+}
+
+static int rpc_spi_io_xfer(struct rpc_spi *rpc,
+			   const void *tx_buf, void *rx_buf)
+{
+	u32 smenr, smcr, data, pos = 0;
+	int ret = 0;
+
+	writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
+	       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
+	writel(0x0, rpc->regs + RPC_SMDRENR);
+
+	if (tx_buf) {
+		writel(rpc->cmd, rpc->regs + RPC_SMCMR);
+		writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
+		writel(rpc->addr, rpc->regs + RPC_SMADR);
+		smenr = rpc->smenr;
+
+		while (pos < rpc->xferlen) {
+			u32 nbytes = rpc->xferlen  - pos;
+
+			writel(*(u32 *)(tx_buf + pos), rpc->regs + RPC_SMWDR0);
+
+			if (nbytes > 4) {
+				nbytes = 4;
+				smcr = rpc->smcr |
+				       RPC_SMCR_SPIE | RPC_SMCR_SSLKP;
+			} else {
+				smcr = rpc->smcr | RPC_SMCR_SPIE;
+			}
+
+			writel(smenr, rpc->regs + RPC_SMENR);
+			writel(smcr, rpc->regs + RPC_SMCR);
+			ret = wait_msg_xfer_end(rpc);
+			if (ret)
+				goto out;
+
+			pos += nbytes;
+			smenr = rpc->smenr & ~RPC_SMENR_CDE &
+					     ~RPC_SMENR_ADE(0xf);
+		}
+	} else if (rx_buf) {
+		while (pos < rpc->xferlen) {
+			u32 nbytes = rpc->xferlen  - pos;
+
+			if (nbytes > 4)
+				nbytes = 4;
+
+			writel(rpc->cmd, rpc->regs + RPC_SMCMR);
+			writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
+			writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
+			writel(rpc->smenr, rpc->regs + RPC_SMENR);
+			writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
+			ret = wait_msg_xfer_end(rpc);
+			if (ret)
+				goto out;
+
+			data = readl(rpc->regs + RPC_SMRDR0);
+			memcpy_fromio(rx_buf + pos, (void *)&data, nbytes);
+			pos += nbytes;
+		}
+	} else {
+		writel(rpc->cmd, rpc->regs + RPC_SMCMR);
+		writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
+		writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
+		writel(rpc->smenr, rpc->regs + RPC_SMENR);
+		writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
+		ret = wait_msg_xfer_end(rpc);
+	}
+out:
+	return ret;
+}
+
+static void rpc_spi_mem_set_prep_op_cfg(struct spi_device *spi,
+					const struct spi_mem_op *op,
+					u64 *offs, size_t *len)
+{
+	struct rpc_spi *rpc = spi_master_get_devdata(spi->master);
+
+	rpc->cmd = RPC_SMCMR_CMD(op->cmd.opcode);
+	rpc->smenr = RPC_SMENR_CDE |
+		     RPC_SMENR_CDB(fls(op->cmd.buswidth >> 1));
+	rpc->totalxferlen = 1;
+	rpc->xferlen = 0;
+	rpc->addr = 0;
+
+	if (op->addr.nbytes) {
+		rpc->smenr |= RPC_SMENR_ADB(fls(op->addr.buswidth >> 1));
+		if (op->addr.nbytes == 4)
+			rpc->smenr |= RPC_SMENR_ADE(0xf);
+		else
+			rpc->smenr |= RPC_SMENR_ADE(0x7);
+
+		if (!offs && !len)
+			rpc->addr = *(u32 *)offs;
+		else
+			rpc->addr = op->addr.val;
+		rpc->totalxferlen += op->addr.nbytes;
+	}
+
+	if (op->dummy.nbytes) {
+		rpc->smenr |= RPC_SMENR_DME;
+		rpc->dummy = RPC_SMDMCR_DMCYC(op->dummy.nbytes);
+		rpc->totalxferlen += op->dummy.nbytes;
+	}
+
+	if (op->data.nbytes || (offs && len)) {
+		rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer(op->data.nbytes)) |
+			      RPC_SMENR_SPIDB(fls(op->data.buswidth >> 1));
+
+		if (op->data.dir == SPI_MEM_DATA_IN) {
+			rpc->smcr = RPC_SMCR_SPIRE;
+			rpc->xfer_dir = SPI_MEM_DATA_IN;
+		} else if (op->data.dir == SPI_MEM_DATA_OUT) {
+			rpc->smcr = RPC_SMCR_SPIWE;
+			rpc->xfer_dir = SPI_MEM_DATA_OUT;
+		}
+
+		if (offs && len) {
+			rpc->xferlen = *(u32 *)len;
+			rpc->totalxferlen += *(u32 *)len;
+		} else {
+			rpc->xferlen = op->data.nbytes;
+			rpc->totalxferlen += op->data.nbytes;
+		}
+	}
+}
+
+static bool rpc_spi_mem_supports_op(struct spi_mem *mem,
+				    const struct spi_mem_op *op)
+{
+	if (op->data.buswidth > 4 || op->addr.buswidth > 4 ||
+	    op->dummy.buswidth > 4 || op->cmd.buswidth > 4)
+		return false;
+
+	if (op->addr.nbytes > 4)
+		return false;
+
+	return true;
+}
+
+static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
+				       u64 offs, size_t len, void *buf)
+{
+	struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
+	int ret;
+
+	if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
+		return -EINVAL;
+
+	ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
+	if (ret)
+		return ret;
+
+	rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
+				    &desc->info.op_tmpl, &offs, &len);
+
+	writel(RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
+	       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
+
+	writel(RPC_DRCR_RBURST(0x1f) | RPC_DRCR_RBE, rpc->regs + RPC_DRCR);
+	writel(rpc->cmd, rpc->regs + RPC_DRCMR);
+	writel(RPC_DREAR_EAC, rpc->regs + RPC_DREAR);
+	writel(0, rpc->regs + RPC_DROPR);
+	writel(rpc->smenr, rpc->regs + RPC_DRENR);
+	writel(rpc->dummy, rpc->regs + RPC_DRDMCR);
+	writel(0x0, rpc->regs + RPC_DRDRENR);
+	memcpy_fromio(buf, rpc->linear.map + desc->info.offset + offs, len);
+
+	return len;
+}
+
+static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
+					u64 offs, size_t len, const void *buf)
+{
+	struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
+	int tx_offs, ret;
+
+	if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
+		return -EINVAL;
+
+	if (WARN_ON(len > RPC_WBUF_SIZE))
+		return -EIO;
+
+	ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
+	if (ret)
+		return ret;
+
+	rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
+				    &desc->info.op_tmpl, &offs, &len);
+
+	writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
+	       RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
+	writel(0x0, rpc->regs + RPC_SMDRENR);
+
+	writel(RPC_PHYCNT_CAL | 0x260 | RPC_PHYCNT_WBUF2 | RPC_PHYCNT_WBUF,
+	       rpc->regs + RPC_PHYCNT);
+
+	for (tx_offs = 0; tx_offs < RPC_WBUF_SIZE; tx_offs += 4)
+		writel(*(u32 *)(buf + tx_offs), rpc->regs + RPC_WBUF + tx_offs);
+
+	writel(rpc->cmd, rpc->regs + RPC_SMCMR);
+	writel(offs, rpc->regs + RPC_SMADR);
+	writel(rpc->smenr, rpc->regs + RPC_SMENR);
+	writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
+	ret = wait_msg_xfer_end(rpc);
+	if (ret)
+		goto out;
+
+	writel(RPC_DRCR_RCF, rpc->regs + RPC_DRCR);
+	writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0) | 0x260,
+	       rpc->regs + RPC_PHYCNT);
+
+	return len;
+out:
+	return ret;
+}
+
+static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
+{
+	struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
+
+	if (desc->info.offset + desc->info.length > U32_MAX)
+		return -ENOTSUPP;
+
+	if (!rpc_spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
+		return -ENOTSUPP;
+
+	if (!rpc->linear.map &&
+	    desc->info.op_tmpl.data.dir == SPI_MEM_DATA_IN)
+		return -ENOTSUPP;
+
+	return 0;
+}
+
+static int rpc_spi_mem_exec_op(struct spi_mem *mem,
+			       const struct spi_mem_op *op)
+{
+	struct rpc_spi *rpc = spi_master_get_devdata(mem->spi->master);
+	int ret;
+
+	ret = rpc_spi_set_freq(rpc, mem->spi->max_speed_hz);
+	if (ret)
+		return ret;
+
+	rpc_spi_mem_set_prep_op_cfg(mem->spi, op, NULL, NULL);
+
+	ret = rpc_spi_io_xfer(rpc,
+			      op->data.dir == SPI_MEM_DATA_OUT ?
+			      op->data.buf.out : NULL,
+			      op->data.dir == SPI_MEM_DATA_IN ?
+			      op->data.buf.in : NULL);
+
+	return ret;
+}
+
+static const struct spi_controller_mem_ops rpc_spi_mem_ops = {
+	.supports_op = rpc_spi_mem_supports_op,
+	.exec_op = rpc_spi_mem_exec_op,
+	.dirmap_create = rpc_spi_mem_dirmap_create,
+	.dirmap_read = rpc_spi_mem_dirmap_read,
+	.dirmap_write = rpc_spi_mem_dirmap_write,
+};
+
+static void rpc_spi_transfer_setup(struct rpc_spi *rpc,
+				   struct spi_message *msg)
+{
+	struct spi_transfer *t, xfer[4] = { };
+	u32 i, xfercnt, xferpos = 0;
+
+	rpc->totalxferlen = 0;
+	list_for_each_entry(t, &msg->transfers, transfer_list) {
+		if (t->tx_buf) {
+			xfer[xferpos].tx_buf = t->tx_buf;
+			xfer[xferpos].tx_nbits = t->tx_nbits;
+		}
+
+		if (t->rx_buf) {
+			xfer[xferpos].rx_buf = t->rx_buf;
+			xfer[xferpos].rx_nbits = t->rx_nbits;
+		}
+
+		if (t->len) {
+			xfer[xferpos++].len = t->len;
+			rpc->totalxferlen += t->len;
+		}
+	}
+
+	xfercnt = xferpos;
+	rpc->xferlen = xfer[--xferpos].len;
+	rpc->cmd = RPC_SMCMR_CMD(((u8 *)xfer[0].tx_buf)[0]);
+	rpc->smenr = RPC_SMENR_CDE | RPC_SMENR_CDB(fls(xfer[0].tx_nbits >> 1));
+	rpc->addr = 0;
+
+	if (xfercnt > 2 && xfer[1].len && xfer[1].tx_buf) {
+		rpc->smenr |= RPC_SMENR_ADB(fls(xfer[1].tx_nbits >> 1));
+		for (i = 0; i < xfer[1].len; i++)
+			rpc->addr |= (u32)((u8 *)xfer[1].tx_buf)[i]
+					<< (8 * (xfer[1].len - i - 1));
+
+		if (xfer[1].len == 4)
+			rpc->smenr |= RPC_SMENR_ADE(0xf);
+		else
+			rpc->smenr |= RPC_SMENR_ADE(0x7);
+	}
+
+	switch (xfercnt) {
+	case 2:
+		if (xfer[1].rx_buf) {
+			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
+				      (xfer[1].len)) | RPC_SMENR_SPIDB(fls
+				      (xfer[1].rx_nbits >> 1));
+			rpc->smcr = RPC_SMCR_SPIRE;
+			rpc->xfer_dir = SPI_MEM_DATA_IN;
+		} else if (xfer[1].tx_buf) {
+			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
+				      (xfer[1].len)) | RPC_SMENR_SPIDB(fls
+				      (xfer[1].tx_nbits >> 1));
+			rpc->smcr = RPC_SMCR_SPIWE;
+			rpc->xfer_dir = SPI_MEM_DATA_OUT;
+		}
+		break;
+
+	case 3:
+		if (xfer[2].len && xfer[2].rx_buf && !xfer[2].tx_buf) {
+			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
+				      (xfer[2].len)) | RPC_SMENR_SPIDB(fls
+				      (xfer[2].rx_nbits >> 1));
+			rpc->smcr = RPC_SMCR_SPIRE;
+			rpc->xfer_dir = SPI_MEM_DATA_IN;
+		} else if (xfer[2].len && xfer[2].tx_buf && !xfer[2].rx_buf) {
+			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
+				      (xfer[2].len)) | RPC_SMENR_SPIDB(fls
+				      (xfer[2].tx_nbits >> 1));
+			rpc->smcr = RPC_SMCR_SPIWE;
+			rpc->xfer_dir = SPI_MEM_DATA_OUT;
+		}
+
+		break;
+
+	case 4:
+		if (xfer[2].len && xfer[2].tx_buf) {
+			rpc->smenr |= RPC_SMENR_DME;
+			rpc->dummy = RPC_SMDMCR_DMCYC(xfer[2].len);
+			writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
+		}
+
+		if (xfer[3].len && xfer[3].rx_buf) {
+			rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer
+				      (xfer[3].len)) | RPC_SMENR_SPIDB(fls
+				      (xfer[3].rx_nbits >> 1));
+			rpc->smcr = RPC_SMCR_SPIRE;
+			rpc->xfer_dir = SPI_MEM_DATA_IN;
+		}
+
+		break;
+
+	default:
+		break;
+	}
+}
+
+static int rpc_spi_xfer_message(struct rpc_spi *rpc, struct spi_transfer *t)
+{
+	int ret;
+
+	ret = rpc_spi_set_freq(rpc, t->speed_hz);
+	if (ret)
+		return ret;
+
+	ret = rpc_spi_io_xfer(rpc,
+			      rpc->xfer_dir == SPI_MEM_DATA_OUT ?
+			      t->tx_buf : NULL,
+			      rpc->xfer_dir == SPI_MEM_DATA_IN ?
+			      t->rx_buf : NULL);
+
+	return ret;
+}
+
+static int rpc_spi_transfer_one_message(struct spi_master *master,
+					struct spi_message *msg)
+{
+	struct rpc_spi *rpc = spi_master_get_devdata(master);
+	struct spi_transfer *t;
+	int ret;
+
+	rpc_spi_transfer_setup(rpc, msg);
+
+	list_for_each_entry(t, &msg->transfers, transfer_list) {
+		if (list_is_last(&t->transfer_list, &msg->transfers)) {
+			ret = rpc_spi_xfer_message(rpc, t);
+			if (ret)
+				goto out;
+		}
+	}
+
+	msg->status = 0;
+	msg->actual_length = rpc->totalxferlen;
+out:
+	spi_finalize_current_message(master);
+	return 0;
+}
+
+static int __maybe_unused rpc_spi_runtime_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct rpc_spi *rpc = spi_master_get_devdata(master);
+
+	clk_disable_unprepare(rpc->clk_rpc);
+
+	return 0;
+}
+
+static int __maybe_unused rpc_spi_runtime_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct rpc_spi *rpc = spi_master_get_devdata(master);
+	int ret;
+
+	ret = clk_prepare_enable(rpc->clk_rpc);
+	if (ret)
+		dev_err(dev, "Can't enable rpc->clk_rpc\n");
+
+	return ret;
+}
+
+static const struct dev_pm_ops rpc_spi_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(rpc_spi_runtime_suspend,
+			   rpc_spi_runtime_resume, NULL)
+};
+
+static int rpc_spi_probe(struct platform_device *pdev)
+{
+	struct spi_master *master;
+	struct resource *res;
+	struct rpc_spi *rpc;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(struct rpc_spi));
+	if (!master)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, master);
+
+	rpc = spi_master_get_devdata(master);
+
+	master->dev.of_node = pdev->dev.of_node;
+
+	rpc->clk_rpc = devm_clk_get(&pdev->dev, "clk_rpc");
+	if (IS_ERR(rpc->clk_rpc))
+		return PTR_ERR(rpc->clk_rpc);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rpc_regs");
+	rpc->regs = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(rpc->regs))
+		return PTR_ERR(rpc->regs);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dirmap");
+	rpc->linear.map = devm_ioremap_resource(&pdev->dev, res);
+	if (!IS_ERR(rpc->linear.map)) {
+		rpc->linear.dma = res->start;
+		rpc->linear.size = resource_size(res);
+	} else {
+		rpc->linear.map = NULL;
+	}
+
+	pm_runtime_enable(&pdev->dev);
+	master->auto_runtime_pm = true;
+
+	master->num_chipselect = 1;
+	master->mem_ops = &rpc_spi_mem_ops;
+	master->transfer_one_message = rpc_spi_transfer_one_message;
+
+	master->bits_per_word_mask = SPI_BPW_MASK(8);
+	master->mode_bits = SPI_CPOL | SPI_CPHA |
+			SPI_RX_DUAL | SPI_TX_DUAL |
+			SPI_RX_QUAD | SPI_TX_QUAD;
+
+	rpc_spi_hw_init(rpc);
+
+	ret = spi_register_master(master);
+	if (ret) {
+		dev_err(&pdev->dev, "spi_register_master failed\n");
+		goto err_put_master;
+	}
+	return 0;
+
+err_put_master:
+	spi_master_put(master);
+	pm_runtime_disable(&pdev->dev);
+
+	return ret;
+}
+
+static int rpc_spi_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+	spi_unregister_master(master);
+
+	return 0;
+}
+
+static const struct of_device_id rpc_spi_of_ids[] = {
+	{ .compatible = "renesas,rpc-r8a77995", },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rpc_spi_of_ids);
+
+static struct platform_driver rpc_spi_driver = {
+	.probe = rpc_spi_probe,
+	.remove = rpc_spi_remove,
+	.driver = {
+		.name = "rpc-spi",
+		.of_match_table = rpc_spi_of_ids,
+		.pm = &rpc_spi_dev_pm_ops,
+	},
+};
+module_platform_driver(rpc_spi_driver);
+
+MODULE_AUTHOR("Mason Yang <masonccyang@mxic.com.tw>");
+MODULE_DESCRIPTION("Renesas R-Car D3 RPC SPI controller driver");
+MODULE_LICENSE("GPL v2");