Hi Mark,
This Renesas R-Car D3 RPC SPI driver is based on Boris's
new spi-mem direct mapping read/write mode[1][2] and
test on R-Car D3 Draak board.
thanks for your review.
best regards,
Mason
[1] https://patchwork.kernel.org/patch/10670753/
[2] https://patchwork.kernel.org/patch/10670747/
Mason Yang (2):
spi: Add Renesas R-Car RPC SPI controller driver
dt-binding: spi: Document Renesas R-Car RPC controller bindings
.../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 +
drivers/spi/Kconfig | 6 +
drivers/spi/Makefile | 1 +
drivers/spi/spi-renesas-rpc.c | 750 +++++++++++++++++++++
4 files changed, 790 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
create mode 100644 drivers/spi/spi-renesas-rpc.c
--
1.9.1
Document the bindings used by the Renesas R-Car D3 RPC controller.
Signed-off-by: Mason Yang <[email protected]>
---
.../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
1 file changed, 33 insertions(+)
create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
new file mode 100644
index 0000000..8286cc8
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
@@ -0,0 +1,33 @@
+Renesas R-Car D3 RPC controller Device Tree Bindings
+----------------------------------------------------
+
+Required properties:
+- compatible: should be "renesas,rpc-r8a77995"
+- #address-cells: should be 1
+- #size-cells: should be 0
+- reg: should contain 2 entries, one for the registers and one for the direct
+ mapping area
+- reg-names: should contain "rpc_regs" and "dirmap"
+- interrupts: interrupt line connected to the RPC SPI controller
+- clock-names: should contain "clk_rpc"
+- clocks: should contain 1 entries for the CPG Module 917 clocks
+
+Example:
+
+ rpc: spi@ee200000 {
+ compatible = "renesas,rpc-r8a77995";
+ reg = <0 0xee200000 0 0x8100>, <0 0x08000000 0 0x4000000>;
+ reg-names = "rpc_regs", "dirmap";
+ clocks = <&cpg CPG_MOD 917>;
+ clock-names = "clk_rpc";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ flash@0 {
+ compatible = "jedec,spi-nor";
+ reg = <0>;
+ spi-max-frequency = <40000000>;
+ spi-tx-bus-width = <1>;
+ spi-rx-bus-width = <1>;
+ };
+ };
--
1.9.1
Add a driver for Renesas R-Car D3 RPC SPI controller driver.
Signed-off-by: Mason Yang <[email protected]>
---
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
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 <[email protected]>
+//
+
+#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 <[email protected]>");
+MODULE_DESCRIPTION("Renesas R-Car D3 RPC SPI controller driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
On 11/19/2018 11:01 AM, Mason Yang wrote:
> Document the bindings used by the Renesas R-Car D3 RPC controller.
>
> Signed-off-by: Mason Yang <[email protected]>
> ---
> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> new file mode 100644
> index 0000000..8286cc8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> @@ -0,0 +1,33 @@
> +Renesas R-Car D3 RPC controller Device Tree Bindings
> +----------------------------------------------------
> +
> +Required properties:
> +- compatible: should be "renesas,rpc-r8a77995"
> +- #address-cells: should be 1
> +- #size-cells: should be 0
> +- reg: should contain 2 entries, one for the registers and one for the direct
> + mapping area
> +- reg-names: should contain "rpc_regs" and "dirmap"
> +- interrupts: interrupt line connected to the RPC SPI controller
Do you also plan to support the RPC HF mode ? And if so, how would that
look in the bindings ? I'd like to avoid having to redesign the bindings
later to handle both the SPI and HF modes.
The HF is roughly a CFI flash with different bus interface.
btw U-Boot has drivers for both the HF and SPI mode:
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/mtd/renesas_rpc_hf.c
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/spi/renesas_rpc_spi.c
> +- clock-names: should contain "clk_rpc"
> +- clocks: should contain 1 entries for the CPG Module 917 clocks
> +
> +Example:
> +
> + rpc: spi@ee200000 {
> + compatible = "renesas,rpc-r8a77995";
> + reg = <0 0xee200000 0 0x8100>, <0 0x08000000 0 0x4000000>;
> + reg-names = "rpc_regs", "dirmap";
> + clocks = <&cpg CPG_MOD 917>;
> + clock-names = "clk_rpc";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + flash@0 {
> + compatible = "jedec,spi-nor";
> + reg = <0>;
> + spi-max-frequency = <40000000>;
> + spi-tx-bus-width = <1>;
> + spi-rx-bus-width = <1>;
> + };
> + };
>
--
Best regards,
Marek Vasut
On Mon, 19 Nov 2018 14:49:31 +0100
Marek Vasut <[email protected]> wrote:
> On 11/19/2018 11:01 AM, Mason Yang wrote:
> > Document the bindings used by the Renesas R-Car D3 RPC controller.
> >
> > Signed-off-by: Mason Yang <[email protected]>
> > ---
> > .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> > 1 file changed, 33 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >
> > diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > new file mode 100644
> > index 0000000..8286cc8
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> > @@ -0,0 +1,33 @@
> > +Renesas R-Car D3 RPC controller Device Tree Bindings
> > +----------------------------------------------------
> > +
> > +Required properties:
> > +- compatible: should be "renesas,rpc-r8a77995"
> > +- #address-cells: should be 1
> > +- #size-cells: should be 0
> > +- reg: should contain 2 entries, one for the registers and one for the direct
> > + mapping area
> > +- reg-names: should contain "rpc_regs" and "dirmap"
> > +- interrupts: interrupt line connected to the RPC SPI controller
>
> Do you also plan to support the RPC HF mode ? And if so, how would that
> look in the bindings ?
Not sure this approach is still accepted, but that's how we solved the
problem for the flexcom block [1].
[1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
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 <[email protected]>
> +//
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 <[email protected]>");
> +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");
>
--
Best regards,
Marek Vasut
On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> On Mon, 19 Nov 2018 14:49:31 +0100
> Marek Vasut <[email protected]> wrote:
>
>> On 11/19/2018 11:01 AM, Mason Yang wrote:
>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
>>>
>>> Signed-off-by: Mason Yang <[email protected]>
>>> ---
>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
>>> 1 file changed, 33 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>> new file mode 100644
>>> index 0000000..8286cc8
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>> @@ -0,0 +1,33 @@
>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
>>> +----------------------------------------------------
>>> +
>>> +Required properties:
>>> +- compatible: should be "renesas,rpc-r8a77995"
>>> +- #address-cells: should be 1
>>> +- #size-cells: should be 0
>>> +- reg: should contain 2 entries, one for the registers and one for the direct
>>> + mapping area
>>> +- reg-names: should contain "rpc_regs" and "dirmap"
>>> +- interrupts: interrupt line connected to the RPC SPI controller
>>
>> Do you also plan to support the RPC HF mode ? And if so, how would that
>> look in the bindings ?
>
> Not sure this approach is still accepted, but that's how we solved the
> problem for the flexcom block [1].
>
> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
That looks pretty horrible.
In U-Boot we check whether the device hanging under the controller node
is JEDEC SPI flash or CFI flash and based on that decide what the config
of the controller should be (SPI or HF). Not sure that's much better,but
at least it doesn't need extra nodes which do not really represent any
kind of real hardware.
--
Best regards,
Marek Vasut
On Mon, 19 Nov 2018 15:14:07 +0100
Marek Vasut <[email protected]> wrote:
> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 14:49:31 +0100
> > Marek Vasut <[email protected]> wrote:
> >
> >> On 11/19/2018 11:01 AM, Mason Yang wrote:
> >>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>
> >>> Signed-off-by: Mason Yang <[email protected]>
> >>> ---
> >>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> >>> 1 file changed, 33 insertions(+)
> >>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>> new file mode 100644
> >>> index 0000000..8286cc8
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>> @@ -0,0 +1,33 @@
> >>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>> +----------------------------------------------------
> >>> +
> >>> +Required properties:
> >>> +- compatible: should be "renesas,rpc-r8a77995"
> >>> +- #address-cells: should be 1
> >>> +- #size-cells: should be 0
> >>> +- reg: should contain 2 entries, one for the registers and one for the direct
> >>> + mapping area
> >>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>> +- interrupts: interrupt line connected to the RPC SPI controller
> >>
> >> Do you also plan to support the RPC HF mode ? And if so, how would that
> >> look in the bindings ?
> >
> > Not sure this approach is still accepted, but that's how we solved the
> > problem for the flexcom block [1].
> >
> > [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>
> That looks pretty horrible.
>
> In U-Boot we check whether the device hanging under the controller node
> is JEDEC SPI flash or CFI flash and based on that decide what the config
> of the controller should be (SPI or HF). Not sure that's much better,but
> at least it doesn't need extra nodes which do not really represent any
> kind of real hardware.
>
The subnodes are not needed, you can just have a property that tells in
which mode the controller is supposed to operate, and the MFD would
create a sub-device that points to the same device_node. Or we can have
a single driver that decides what to declare (a spi_controller or flash
controller), but you'd still have to decide where to place this
driver...
On 11/19/2018 03:43 PM, Boris Brezillon wrote:
> On Mon, 19 Nov 2018 15:14:07 +0100
> Marek Vasut <[email protected]> wrote:
>
>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
>>> On Mon, 19 Nov 2018 14:49:31 +0100
>>> Marek Vasut <[email protected]> wrote:
>>>
>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
>>>>>
>>>>> Signed-off-by: Mason Yang <[email protected]>
>>>>> ---
>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
>>>>> 1 file changed, 33 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>> new file mode 100644
>>>>> index 0000000..8286cc8
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>> @@ -0,0 +1,33 @@
>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
>>>>> +----------------------------------------------------
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: should be "renesas,rpc-r8a77995"
>>>>> +- #address-cells: should be 1
>>>>> +- #size-cells: should be 0
>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
>>>>> + mapping area
>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
>>>>
>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
>>>> look in the bindings ?
>>>
>>> Not sure this approach is still accepted, but that's how we solved the
>>> problem for the flexcom block [1].
>>>
>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>>
>> That looks pretty horrible.
>>
>> In U-Boot we check whether the device hanging under the controller node
>> is JEDEC SPI flash or CFI flash and based on that decide what the config
>> of the controller should be (SPI or HF). Not sure that's much better,but
>> at least it doesn't need extra nodes which do not really represent any
>> kind of real hardware.
>>
>
> The subnodes are not needed, you can just have a property that tells in
> which mode the controller is supposed to operate, and the MFD would
> create a sub-device that points to the same device_node.
Do you even need a dedicated property ? I think you can decide purely on
what node is hanging under the controller (jedec spi nor or cfi nor).
> Or we can have
> a single driver that decides what to declare (a spi_controller or flash
> controller), but you'd still have to decide where to place this
> driver...
I'd definitely prefer a single driver.
--
Best regards,
Marek Vasut
On Mon, 19 Nov 2018 16:12:41 +0100
Marek Vasut <[email protected]> wrote:
> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 15:14:07 +0100
> > Marek Vasut <[email protected]> wrote:
> >
> >> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> >>> On Mon, 19 Nov 2018 14:49:31 +0100
> >>> Marek Vasut <[email protected]> wrote:
> >>>
> >>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
> >>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>>>
> >>>>> Signed-off-by: Mason Yang <[email protected]>
> >>>>> ---
> >>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> >>>>> 1 file changed, 33 insertions(+)
> >>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>> new file mode 100644
> >>>>> index 0000000..8286cc8
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>> @@ -0,0 +1,33 @@
> >>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>>>> +----------------------------------------------------
> >>>>> +
> >>>>> +Required properties:
> >>>>> +- compatible: should be "renesas,rpc-r8a77995"
> >>>>> +- #address-cells: should be 1
> >>>>> +- #size-cells: should be 0
> >>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
> >>>>> + mapping area
> >>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>>>> +- interrupts: interrupt line connected to the RPC SPI controller
> >>>>
> >>>> Do you also plan to support the RPC HF mode ? And if so, how would that
> >>>> look in the bindings ?
> >>>
> >>> Not sure this approach is still accepted, but that's how we solved the
> >>> problem for the flexcom block [1].
> >>>
> >>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >>
> >> That looks pretty horrible.
> >>
> >> In U-Boot we check whether the device hanging under the controller node
> >> is JEDEC SPI flash or CFI flash and based on that decide what the config
> >> of the controller should be (SPI or HF). Not sure that's much better,but
> >> at least it doesn't need extra nodes which do not really represent any
> >> kind of real hardware.
> >>
> >
> > The subnodes are not needed, you can just have a property that tells in
> > which mode the controller is supposed to operate, and the MFD would
> > create a sub-device that points to the same device_node.
>
> Do you even need a dedicated property ? I think you can decide purely on
> what node is hanging under the controller (jedec spi nor or cfi nor).
Yes, that could work if they have well-known compatibles. As soon as
people start using flash-specific compats (like some people do for
their SPI NORs) it becomes a maintenance burden.
>
> > Or we can have
> > a single driver that decides what to declare (a spi_controller or flash
> > controller), but you'd still have to decide where to place this
> > driver...
>
> I'd definitely prefer a single driver.
>
Where would you put this driver? I really don't like the idea of having
MTD drivers spread over the tree. Don't know what's Mark's opinion on
this matter.
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 <[email protected]>
> > +//
> 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.
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 <[email protected]>
>>> +//
>
>> 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 ;-)
--
Best regards,
Marek Vasut
On 11/19/2018 04:21 PM, Boris Brezillon wrote:
> On Mon, 19 Nov 2018 16:12:41 +0100
> Marek Vasut <[email protected]> wrote:
>
>> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
>>> On Mon, 19 Nov 2018 15:14:07 +0100
>>> Marek Vasut <[email protected]> wrote:
>>>
>>>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
>>>>> On Mon, 19 Nov 2018 14:49:31 +0100
>>>>> Marek Vasut <[email protected]> wrote:
>>>>>
>>>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
>>>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
>>>>>>>
>>>>>>> Signed-off-by: Mason Yang <[email protected]>
>>>>>>> ---
>>>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
>>>>>>> 1 file changed, 33 insertions(+)
>>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..8286cc8
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>> @@ -0,0 +1,33 @@
>>>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
>>>>>>> +----------------------------------------------------
>>>>>>> +
>>>>>>> +Required properties:
>>>>>>> +- compatible: should be "renesas,rpc-r8a77995"
>>>>>>> +- #address-cells: should be 1
>>>>>>> +- #size-cells: should be 0
>>>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
>>>>>>> + mapping area
>>>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
>>>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
>>>>>>
>>>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
>>>>>> look in the bindings ?
>>>>>
>>>>> Not sure this approach is still accepted, but that's how we solved the
>>>>> problem for the flexcom block [1].
>>>>>
>>>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>>>>
>>>> That looks pretty horrible.
>>>>
>>>> In U-Boot we check whether the device hanging under the controller node
>>>> is JEDEC SPI flash or CFI flash and based on that decide what the config
>>>> of the controller should be (SPI or HF). Not sure that's much better,but
>>>> at least it doesn't need extra nodes which do not really represent any
>>>> kind of real hardware.
>>>>
>>>
>>> The subnodes are not needed, you can just have a property that tells in
>>> which mode the controller is supposed to operate, and the MFD would
>>> create a sub-device that points to the same device_node.
>>
>> Do you even need a dedicated property ? I think you can decide purely on
>> what node is hanging under the controller (jedec spi nor or cfi nor).
>
> Yes, that could work if they have well-known compatibles. As soon as
> people start using flash-specific compats (like some people do for
> their SPI NORs) it becomes a maintenance burden.
Which, on this controller, is very likely never gonna happen. Once it
does , we can add a custom property.
>>> Or we can have
>>> a single driver that decides what to declare (a spi_controller or flash
>>> controller), but you'd still have to decide where to place this
>>> driver...
>>
>> I'd definitely prefer a single driver.
>>
>
> Where would you put this driver? I really don't like the idea of having
> MTD drivers spread over the tree. Don't know what's Mark's opinion on
> this matter.
Well, it's both CFI (hyperflash) and SF (well, SPI flash) controller, so
where would this go ?
--
Best regards,
Marek Vasut
On Mon, 19 Nov 2018 23:11:31 +0100
Marek Vasut <[email protected]> wrote:
> On 11/19/2018 04:21 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 16:12:41 +0100
> > Marek Vasut <[email protected]> wrote:
> >
> >> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
> >>> On Mon, 19 Nov 2018 15:14:07 +0100
> >>> Marek Vasut <[email protected]> wrote:
> >>>
> >>>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> >>>>> On Mon, 19 Nov 2018 14:49:31 +0100
> >>>>> Marek Vasut <[email protected]> wrote:
> >>>>>
> >>>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
> >>>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>>>>>
> >>>>>>> Signed-off-by: Mason Yang <[email protected]>
> >>>>>>> ---
> >>>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> >>>>>>> 1 file changed, 33 insertions(+)
> >>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>> new file mode 100644
> >>>>>>> index 0000000..8286cc8
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>> @@ -0,0 +1,33 @@
> >>>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>>>>>> +----------------------------------------------------
> >>>>>>> +
> >>>>>>> +Required properties:
> >>>>>>> +- compatible: should be "renesas,rpc-r8a77995"
> >>>>>>> +- #address-cells: should be 1
> >>>>>>> +- #size-cells: should be 0
> >>>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
> >>>>>>> + mapping area
> >>>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
> >>>>>>
> >>>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
> >>>>>> look in the bindings ?
> >>>>>
> >>>>> Not sure this approach is still accepted, but that's how we solved the
> >>>>> problem for the flexcom block [1].
> >>>>>
> >>>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >>>>
> >>>> That looks pretty horrible.
> >>>>
> >>>> In U-Boot we check whether the device hanging under the controller node
> >>>> is JEDEC SPI flash or CFI flash and based on that decide what the config
> >>>> of the controller should be (SPI or HF). Not sure that's much better,but
> >>>> at least it doesn't need extra nodes which do not really represent any
> >>>> kind of real hardware.
> >>>>
> >>>
> >>> The subnodes are not needed, you can just have a property that tells in
> >>> which mode the controller is supposed to operate, and the MFD would
> >>> create a sub-device that points to the same device_node.
> >>
> >> Do you even need a dedicated property ? I think you can decide purely on
> >> what node is hanging under the controller (jedec spi nor or cfi nor).
> >
> > Yes, that could work if they have well-known compatibles. As soon as
> > people start using flash-specific compats (like some people do for
> > their SPI NORs) it becomes a maintenance burden.
>
> Which, on this controller, is very likely never gonna happen. Once it
> does , we can add a custom property.
>
> >>> Or we can have
> >>> a single driver that decides what to declare (a spi_controller or flash
> >>> controller), but you'd still have to decide where to place this
> >>> driver...
> >>
> >> I'd definitely prefer a single driver.
> >>
> >
> > Where would you put this driver? I really don't like the idea of having
> > MTD drivers spread over the tree. Don't know what's Mark's opinion on
> > this matter.
>
> Well, it's both CFI (hyperflash) and SF (well, SPI flash) controller, so
> where would this go ?
>
The spi-mem layer is in drivers/spi/ so it could go in drivers/spi/
(spi-mem controller) or drivers/mtd/ (CFI controller). Looks like you
didn't closely follow what has happened in the spi-nor subsystem
lately :P.
On 11/19/2018 11:19 PM, Boris Brezillon wrote:
> On Mon, 19 Nov 2018 23:11:31 +0100
> Marek Vasut <[email protected]> wrote:
>
>> On 11/19/2018 04:21 PM, Boris Brezillon wrote:
>>> On Mon, 19 Nov 2018 16:12:41 +0100
>>> Marek Vasut <[email protected]> wrote:
>>>
>>>> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
>>>>> On Mon, 19 Nov 2018 15:14:07 +0100
>>>>> Marek Vasut <[email protected]> wrote:
>>>>>
>>>>>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
>>>>>>> On Mon, 19 Nov 2018 14:49:31 +0100
>>>>>>> Marek Vasut <[email protected]> wrote:
>>>>>>>
>>>>>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
>>>>>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Mason Yang <[email protected]>
>>>>>>>>> ---
>>>>>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
>>>>>>>>> 1 file changed, 33 insertions(+)
>>>>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>>>
>>>>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>>> new file mode 100644
>>>>>>>>> index 0000000..8286cc8
>>>>>>>>> --- /dev/null
>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>>> @@ -0,0 +1,33 @@
>>>>>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
>>>>>>>>> +----------------------------------------------------
>>>>>>>>> +
>>>>>>>>> +Required properties:
>>>>>>>>> +- compatible: should be "renesas,rpc-r8a77995"
>>>>>>>>> +- #address-cells: should be 1
>>>>>>>>> +- #size-cells: should be 0
>>>>>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
>>>>>>>>> + mapping area
>>>>>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
>>>>>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
>>>>>>>>
>>>>>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
>>>>>>>> look in the bindings ?
>>>>>>>
>>>>>>> Not sure this approach is still accepted, but that's how we solved the
>>>>>>> problem for the flexcom block [1].
>>>>>>>
>>>>>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>>>>>>
>>>>>> That looks pretty horrible.
>>>>>>
>>>>>> In U-Boot we check whether the device hanging under the controller node
>>>>>> is JEDEC SPI flash or CFI flash and based on that decide what the config
>>>>>> of the controller should be (SPI or HF). Not sure that's much better,but
>>>>>> at least it doesn't need extra nodes which do not really represent any
>>>>>> kind of real hardware.
>>>>>>
>>>>>
>>>>> The subnodes are not needed, you can just have a property that tells in
>>>>> which mode the controller is supposed to operate, and the MFD would
>>>>> create a sub-device that points to the same device_node.
>>>>
>>>> Do you even need a dedicated property ? I think you can decide purely on
>>>> what node is hanging under the controller (jedec spi nor or cfi nor).
>>>
>>> Yes, that could work if they have well-known compatibles. As soon as
>>> people start using flash-specific compats (like some people do for
>>> their SPI NORs) it becomes a maintenance burden.
>>
>> Which, on this controller, is very likely never gonna happen. Once it
>> does , we can add a custom property.
>>
>>>>> Or we can have
>>>>> a single driver that decides what to declare (a spi_controller or flash
>>>>> controller), but you'd still have to decide where to place this
>>>>> driver...
>>>>
>>>> I'd definitely prefer a single driver.
>>>>
>>>
>>> Where would you put this driver? I really don't like the idea of having
>>> MTD drivers spread over the tree. Don't know what's Mark's opinion on
>>> this matter.
>>
>> Well, it's both CFI (hyperflash) and SF (well, SPI flash) controller, so
>> where would this go ?
>>
>
> The spi-mem layer is in drivers/spi/ so it could go in drivers/spi/
> (spi-mem controller) or drivers/mtd/ (CFI controller).
drivers/mtd is probably a better option, since it's not a generic SPI
controller.
--
Best regards,
Marek Vasut
On Mon, 19 Nov 2018 23:22:45 +0100
Marek Vasut <[email protected]> wrote:
> On 11/19/2018 11:19 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 23:11:31 +0100
> > Marek Vasut <[email protected]> wrote:
> >
> >> On 11/19/2018 04:21 PM, Boris Brezillon wrote:
> >>> On Mon, 19 Nov 2018 16:12:41 +0100
> >>> Marek Vasut <[email protected]> wrote:
> >>>
> >>>> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
> >>>>> On Mon, 19 Nov 2018 15:14:07 +0100
> >>>>> Marek Vasut <[email protected]> wrote:
> >>>>>
> >>>>>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> >>>>>>> On Mon, 19 Nov 2018 14:49:31 +0100
> >>>>>>> Marek Vasut <[email protected]> wrote:
> >>>>>>>
> >>>>>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
> >>>>>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>>>>>>>
> >>>>>>>>> Signed-off-by: Mason Yang <[email protected]>
> >>>>>>>>> ---
> >>>>>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> >>>>>>>>> 1 file changed, 33 insertions(+)
> >>>>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>>>
> >>>>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>>> new file mode 100644
> >>>>>>>>> index 0000000..8286cc8
> >>>>>>>>> --- /dev/null
> >>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>>> @@ -0,0 +1,33 @@
> >>>>>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>>>>>>>> +----------------------------------------------------
> >>>>>>>>> +
> >>>>>>>>> +Required properties:
> >>>>>>>>> +- compatible: should be "renesas,rpc-r8a77995"
> >>>>>>>>> +- #address-cells: should be 1
> >>>>>>>>> +- #size-cells: should be 0
> >>>>>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
> >>>>>>>>> + mapping area
> >>>>>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>>>>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
> >>>>>>>>
> >>>>>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
> >>>>>>>> look in the bindings ?
> >>>>>>>
> >>>>>>> Not sure this approach is still accepted, but that's how we solved the
> >>>>>>> problem for the flexcom block [1].
> >>>>>>>
> >>>>>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >>>>>>
> >>>>>> That looks pretty horrible.
> >>>>>>
> >>>>>> In U-Boot we check whether the device hanging under the controller node
> >>>>>> is JEDEC SPI flash or CFI flash and based on that decide what the config
> >>>>>> of the controller should be (SPI or HF). Not sure that's much better,but
> >>>>>> at least it doesn't need extra nodes which do not really represent any
> >>>>>> kind of real hardware.
> >>>>>>
> >>>>>
> >>>>> The subnodes are not needed, you can just have a property that tells in
> >>>>> which mode the controller is supposed to operate, and the MFD would
> >>>>> create a sub-device that points to the same device_node.
> >>>>
> >>>> Do you even need a dedicated property ? I think you can decide purely on
> >>>> what node is hanging under the controller (jedec spi nor or cfi nor).
> >>>
> >>> Yes, that could work if they have well-known compatibles. As soon as
> >>> people start using flash-specific compats (like some people do for
> >>> their SPI NORs) it becomes a maintenance burden.
> >>
> >> Which, on this controller, is very likely never gonna happen. Once it
> >> does , we can add a custom property.
> >>
> >>>>> Or we can have
> >>>>> a single driver that decides what to declare (a spi_controller or flash
> >>>>> controller), but you'd still have to decide where to place this
> >>>>> driver...
> >>>>
> >>>> I'd definitely prefer a single driver.
> >>>>
> >>>
> >>> Where would you put this driver? I really don't like the idea of having
> >>> MTD drivers spread over the tree. Don't know what's Mark's opinion on
> >>> this matter.
> >>
> >> Well, it's both CFI (hyperflash) and SF (well, SPI flash) controller, so
> >> where would this go ?
> >>
> >
> > The spi-mem layer is in drivers/spi/ so it could go in drivers/spi/
> > (spi-mem controller) or drivers/mtd/ (CFI controller).
>
> drivers/mtd is probably a better option, since it's not a generic SPI
> controller.
>
No, spi-mem controller drivers should go in drivers/spi/ even if they
don't implement the generic SPI interface (it's allowed to only
implement the spi_mem interface).
>
On 11/19/2018 11:25 PM, Boris Brezillon wrote:
> On Mon, 19 Nov 2018 23:22:45 +0100
> Marek Vasut <[email protected]> wrote:
>
>> On 11/19/2018 11:19 PM, Boris Brezillon wrote:
>>> On Mon, 19 Nov 2018 23:11:31 +0100
>>> Marek Vasut <[email protected]> wrote:
>>>
>>>> On 11/19/2018 04:21 PM, Boris Brezillon wrote:
>>>>> On Mon, 19 Nov 2018 16:12:41 +0100
>>>>> Marek Vasut <[email protected]> wrote:
>>>>>
>>>>>> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
>>>>>>> On Mon, 19 Nov 2018 15:14:07 +0100
>>>>>>> Marek Vasut <[email protected]> wrote:
>>>>>>>
>>>>>>>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
>>>>>>>>> On Mon, 19 Nov 2018 14:49:31 +0100
>>>>>>>>> Marek Vasut <[email protected]> wrote:
>>>>>>>>>
>>>>>>>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
>>>>>>>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Mason Yang <[email protected]>
>>>>>>>>>>> ---
>>>>>>>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
>>>>>>>>>>> 1 file changed, 33 insertions(+)
>>>>>>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>>>>>
>>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>>>>> new file mode 100644
>>>>>>>>>>> index 0000000..8286cc8
>>>>>>>>>>> --- /dev/null
>>>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>>>>>>>>>>> @@ -0,0 +1,33 @@
>>>>>>>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
>>>>>>>>>>> +----------------------------------------------------
>>>>>>>>>>> +
>>>>>>>>>>> +Required properties:
>>>>>>>>>>> +- compatible: should be "renesas,rpc-r8a77995"
>>>>>>>>>>> +- #address-cells: should be 1
>>>>>>>>>>> +- #size-cells: should be 0
>>>>>>>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
>>>>>>>>>>> + mapping area
>>>>>>>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
>>>>>>>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
>>>>>>>>>>
>>>>>>>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
>>>>>>>>>> look in the bindings ?
>>>>>>>>>
>>>>>>>>> Not sure this approach is still accepted, but that's how we solved the
>>>>>>>>> problem for the flexcom block [1].
>>>>>>>>>
>>>>>>>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
>>>>>>>>
>>>>>>>> That looks pretty horrible.
>>>>>>>>
>>>>>>>> In U-Boot we check whether the device hanging under the controller node
>>>>>>>> is JEDEC SPI flash or CFI flash and based on that decide what the config
>>>>>>>> of the controller should be (SPI or HF). Not sure that's much better,but
>>>>>>>> at least it doesn't need extra nodes which do not really represent any
>>>>>>>> kind of real hardware.
>>>>>>>>
>>>>>>>
>>>>>>> The subnodes are not needed, you can just have a property that tells in
>>>>>>> which mode the controller is supposed to operate, and the MFD would
>>>>>>> create a sub-device that points to the same device_node.
>>>>>>
>>>>>> Do you even need a dedicated property ? I think you can decide purely on
>>>>>> what node is hanging under the controller (jedec spi nor or cfi nor).
>>>>>
>>>>> Yes, that could work if they have well-known compatibles. As soon as
>>>>> people start using flash-specific compats (like some people do for
>>>>> their SPI NORs) it becomes a maintenance burden.
>>>>
>>>> Which, on this controller, is very likely never gonna happen. Once it
>>>> does , we can add a custom property.
>>>>
>>>>>>> Or we can have
>>>>>>> a single driver that decides what to declare (a spi_controller or flash
>>>>>>> controller), but you'd still have to decide where to place this
>>>>>>> driver...
>>>>>>
>>>>>> I'd definitely prefer a single driver.
>>>>>>
>>>>>
>>>>> Where would you put this driver? I really don't like the idea of having
>>>>> MTD drivers spread over the tree. Don't know what's Mark's opinion on
>>>>> this matter.
>>>>
>>>> Well, it's both CFI (hyperflash) and SF (well, SPI flash) controller, so
>>>> where would this go ?
>>>>
>>>
>>> The spi-mem layer is in drivers/spi/ so it could go in drivers/spi/
>>> (spi-mem controller) or drivers/mtd/ (CFI controller).
>>
>> drivers/mtd is probably a better option, since it's not a generic SPI
>> controller.
>>
>
> No, spi-mem controller drivers should go in drivers/spi/ even if they
> don't implement the generic SPI interface (it's allowed to only
> implement the spi_mem interface).
Except this is not only SPI MEM controller, this is also hyperflash
(that is, CFI) controller. It can drive both types of chips. Thus , I
think it fits better in drivers/mtd/ .
--
Best regards,
Marek Vasut
On Mon, 19 Nov 2018 23:29:00 +0100
Marek Vasut <[email protected]> wrote:
> On 11/19/2018 11:25 PM, Boris Brezillon wrote:
> > On Mon, 19 Nov 2018 23:22:45 +0100
> > Marek Vasut <[email protected]> wrote:
> >
> >> On 11/19/2018 11:19 PM, Boris Brezillon wrote:
> >>> On Mon, 19 Nov 2018 23:11:31 +0100
> >>> Marek Vasut <[email protected]> wrote:
> >>>
> >>>> On 11/19/2018 04:21 PM, Boris Brezillon wrote:
> >>>>> On Mon, 19 Nov 2018 16:12:41 +0100
> >>>>> Marek Vasut <[email protected]> wrote:
> >>>>>
> >>>>>> On 11/19/2018 03:43 PM, Boris Brezillon wrote:
> >>>>>>> On Mon, 19 Nov 2018 15:14:07 +0100
> >>>>>>> Marek Vasut <[email protected]> wrote:
> >>>>>>>
> >>>>>>>> On 11/19/2018 03:10 PM, Boris Brezillon wrote:
> >>>>>>>>> On Mon, 19 Nov 2018 14:49:31 +0100
> >>>>>>>>> Marek Vasut <[email protected]> wrote:
> >>>>>>>>>
> >>>>>>>>>> On 11/19/2018 11:01 AM, Mason Yang wrote:
> >>>>>>>>>>> Document the bindings used by the Renesas R-Car D3 RPC controller.
> >>>>>>>>>>>
> >>>>>>>>>>> Signed-off-by: Mason Yang <[email protected]>
> >>>>>>>>>>> ---
> >>>>>>>>>>> .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 ++++++++++++++++++++++
> >>>>>>>>>>> 1 file changed, 33 insertions(+)
> >>>>>>>>>>> create mode 100644 Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>>>>>
> >>>>>>>>>>> diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>>>>> new file mode 100644
> >>>>>>>>>>> index 0000000..8286cc8
> >>>>>>>>>>> --- /dev/null
> >>>>>>>>>>> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> >>>>>>>>>>> @@ -0,0 +1,33 @@
> >>>>>>>>>>> +Renesas R-Car D3 RPC controller Device Tree Bindings
> >>>>>>>>>>> +----------------------------------------------------
> >>>>>>>>>>> +
> >>>>>>>>>>> +Required properties:
> >>>>>>>>>>> +- compatible: should be "renesas,rpc-r8a77995"
> >>>>>>>>>>> +- #address-cells: should be 1
> >>>>>>>>>>> +- #size-cells: should be 0
> >>>>>>>>>>> +- reg: should contain 2 entries, one for the registers and one for the direct
> >>>>>>>>>>> + mapping area
> >>>>>>>>>>> +- reg-names: should contain "rpc_regs" and "dirmap"
> >>>>>>>>>>> +- interrupts: interrupt line connected to the RPC SPI controller
> >>>>>>>>>>
> >>>>>>>>>> Do you also plan to support the RPC HF mode ? And if so, how would that
> >>>>>>>>>> look in the bindings ?
> >>>>>>>>>
> >>>>>>>>> Not sure this approach is still accepted, but that's how we solved the
> >>>>>>>>> problem for the flexcom block [1].
> >>>>>>>>>
> >>>>>>>>> [1]https://elixir.bootlin.com/linux/v4.20-rc3/source/Documentation/devicetree/bindings/mfd/atmel-flexcom.txt
> >>>>>>>>
> >>>>>>>> That looks pretty horrible.
> >>>>>>>>
> >>>>>>>> In U-Boot we check whether the device hanging under the controller node
> >>>>>>>> is JEDEC SPI flash or CFI flash and based on that decide what the config
> >>>>>>>> of the controller should be (SPI or HF). Not sure that's much better,but
> >>>>>>>> at least it doesn't need extra nodes which do not really represent any
> >>>>>>>> kind of real hardware.
> >>>>>>>>
> >>>>>>>
> >>>>>>> The subnodes are not needed, you can just have a property that tells in
> >>>>>>> which mode the controller is supposed to operate, and the MFD would
> >>>>>>> create a sub-device that points to the same device_node.
> >>>>>>
> >>>>>> Do you even need a dedicated property ? I think you can decide purely on
> >>>>>> what node is hanging under the controller (jedec spi nor or cfi nor).
> >>>>>
> >>>>> Yes, that could work if they have well-known compatibles. As soon as
> >>>>> people start using flash-specific compats (like some people do for
> >>>>> their SPI NORs) it becomes a maintenance burden.
> >>>>
> >>>> Which, on this controller, is very likely never gonna happen. Once it
> >>>> does , we can add a custom property.
> >>>>
> >>>>>>> Or we can have
> >>>>>>> a single driver that decides what to declare (a spi_controller or flash
> >>>>>>> controller), but you'd still have to decide where to place this
> >>>>>>> driver...
> >>>>>>
> >>>>>> I'd definitely prefer a single driver.
> >>>>>>
> >>>>>
> >>>>> Where would you put this driver? I really don't like the idea of having
> >>>>> MTD drivers spread over the tree. Don't know what's Mark's opinion on
> >>>>> this matter.
> >>>>
> >>>> Well, it's both CFI (hyperflash) and SF (well, SPI flash) controller, so
> >>>> where would this go ?
> >>>>
> >>>
> >>> The spi-mem layer is in drivers/spi/ so it could go in drivers/spi/
> >>> (spi-mem controller) or drivers/mtd/ (CFI controller).
> >>
> >> drivers/mtd is probably a better option, since it's not a generic SPI
> >> controller.
> >>
> >
> > No, spi-mem controller drivers should go in drivers/spi/ even if they
> > don't implement the generic SPI interface (it's allowed to only
> > implement the spi_mem interface).
>
> Except this is not only SPI MEM controller, this is also hyperflash
> (that is, CFI) controller. It can drive both types of chips. Thus , I
> think it fits better in drivers/mtd/ .
>
Okay, then I guess we need an ack from Mark on that.
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
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
Hi Mason,
On Mon, Nov 19, 2018 at 11:01 AM Mason Yang <[email protected]> wrote:
> Add a driver for Renesas R-Car D3 RPC SPI controller driver.
>
> Signed-off-by: Mason Yang <[email protected]>
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 -- [email protected]
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
Hi Mason,
On Mon, Nov 19, 2018 at 11:02 AM Mason Yang <[email protected]> wrote:
> Document the bindings used by the Renesas R-Car D3 RPC controller.
>
> Signed-off-by: Mason Yang <[email protected]>
Thanks for your patch
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
> @@ -0,0 +1,33 @@
> +Renesas R-Car D3 RPC controller Device Tree Bindings
R-Car Gen3
> +----------------------------------------------------
> +
> +Required properties:
> +- compatible: should be "renesas,rpc-r8a77995"
Please use "renesas,r8a77995-rpc".
> +- #address-cells: should be 1
> +- #size-cells: should be 0
> +- reg: should contain 2 entries, one for the registers and one for the direct
> + mapping area
> +- reg-names: should contain "rpc_regs" and "dirmap"
> +- interrupts: interrupt line connected to the RPC SPI controller
> +- clock-names: should contain "clk_rpc"
> +- clocks: should contain 1 entries for the CPG Module 917 clocks
... for the module's clock
> +
> +Example:
> +
> + rpc: spi@ee200000 {
> + compatible = "renesas,rpc-r8a77995";
> + reg = <0 0xee200000 0 0x8100>, <0 0x08000000 0 0x4000000>;
> + reg-names = "rpc_regs", "dirmap";
> + clocks = <&cpg CPG_MOD 917>;
> + clock-names = "clk_rpc";
> + #address-cells = <1>;
> + #size-cells = <0>;
power-domains?
resets?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Tue, 20 Nov 2018 09:01:29 +0100
Geert Uytterhoeven <[email protected]> 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.
On 11/20/2018 08:23 AM, [email protected] wrote:
> Hi Marek,
Hi,
>> Marek Vasut <[email protected]>
>> 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 <[email protected]>");
>> > +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).
--
Best regards,
Marek Vasut
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-20181120]
[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: x86_64-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=x86_64
All error/warnings (new ones prefixed by >>):
>> drivers/spi/spi-renesas-rpc.c:283:54: warning: incorrect type in argument 2 (different address spaces)
drivers/spi/spi-renesas-rpc.c:283:54: expected void const volatile [noderef] <asn:2>*addr
drivers/spi/spi-renesas-rpc.c:283:54: got void *<noident>
>> drivers/spi/spi-renesas-rpc.c:369:31: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
>> drivers/spi/spi-renesas-rpc.c:372:13: error: using member 'info' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:375:41: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:379:41: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:392:50: error: using member 'info' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:400:31: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:403:13: error: using member 'info' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:409:41: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:413:41: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:445:31: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:447:17: error: using member 'info' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:447:37: error: using member 'info' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:450:42: error: using member 'mem' in incomplete struct spi_mem_dirmap_desc
drivers/spi/spi-renesas-rpc.c:454:17: error: using member 'info' in incomplete struct spi_mem_dirmap_desc
>> drivers/spi/spi-renesas-rpc.c:484:10: error: unknown field name in initializer
drivers/spi/spi-renesas-rpc.c:485:10: error: unknown field name in initializer
drivers/spi/spi-renesas-rpc.c:486:10: error: unknown field name in initializer
>> drivers/spi/spi-renesas-rpc.c:372:13: warning: unknown expression (8 46)
drivers/spi/spi-renesas-rpc.c:403:13: warning: unknown expression (8 46)
drivers/spi/spi-renesas-rpc.c:447:23: warning: unknown expression (8 46)
drivers/spi/spi-renesas-rpc.c:447:43: warning: unknown expression (8 46)
drivers/spi/spi-renesas-rpc.c:454:36: warning: unknown expression (8 46)
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: warning: excess elements in struct initializer
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: 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: warning: excess elements in struct initializer
drivers/spi/spi-renesas-rpc.c:486:18: note: (near initialization for 'rpc_spi_mem_ops')
cc1: some warnings being treated as errors
vim +/mem +369 drivers/spi/spi-renesas-rpc.c
c4789a83 Mason Yang 2018-11-19 226
c4789a83 Mason Yang 2018-11-19 227 static int rpc_spi_io_xfer(struct rpc_spi *rpc,
c4789a83 Mason Yang 2018-11-19 228 const void *tx_buf, void *rx_buf)
c4789a83 Mason Yang 2018-11-19 229 {
c4789a83 Mason Yang 2018-11-19 230 u32 smenr, smcr, data, pos = 0;
c4789a83 Mason Yang 2018-11-19 231 int ret = 0;
c4789a83 Mason Yang 2018-11-19 232
c4789a83 Mason Yang 2018-11-19 233 writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
c4789a83 Mason Yang 2018-11-19 234 RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
c4789a83 Mason Yang 2018-11-19 235 writel(0x0, rpc->regs + RPC_SMDRENR);
c4789a83 Mason Yang 2018-11-19 236
c4789a83 Mason Yang 2018-11-19 237 if (tx_buf) {
c4789a83 Mason Yang 2018-11-19 238 writel(rpc->cmd, rpc->regs + RPC_SMCMR);
c4789a83 Mason Yang 2018-11-19 239 writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
c4789a83 Mason Yang 2018-11-19 240 writel(rpc->addr, rpc->regs + RPC_SMADR);
c4789a83 Mason Yang 2018-11-19 241 smenr = rpc->smenr;
c4789a83 Mason Yang 2018-11-19 242
c4789a83 Mason Yang 2018-11-19 243 while (pos < rpc->xferlen) {
c4789a83 Mason Yang 2018-11-19 244 u32 nbytes = rpc->xferlen - pos;
c4789a83 Mason Yang 2018-11-19 245
c4789a83 Mason Yang 2018-11-19 246 writel(*(u32 *)(tx_buf + pos), rpc->regs + RPC_SMWDR0);
c4789a83 Mason Yang 2018-11-19 247
c4789a83 Mason Yang 2018-11-19 248 if (nbytes > 4) {
c4789a83 Mason Yang 2018-11-19 249 nbytes = 4;
c4789a83 Mason Yang 2018-11-19 250 smcr = rpc->smcr |
c4789a83 Mason Yang 2018-11-19 251 RPC_SMCR_SPIE | RPC_SMCR_SSLKP;
c4789a83 Mason Yang 2018-11-19 252 } else {
c4789a83 Mason Yang 2018-11-19 253 smcr = rpc->smcr | RPC_SMCR_SPIE;
c4789a83 Mason Yang 2018-11-19 254 }
c4789a83 Mason Yang 2018-11-19 255
c4789a83 Mason Yang 2018-11-19 256 writel(smenr, rpc->regs + RPC_SMENR);
c4789a83 Mason Yang 2018-11-19 257 writel(smcr, rpc->regs + RPC_SMCR);
c4789a83 Mason Yang 2018-11-19 258 ret = wait_msg_xfer_end(rpc);
c4789a83 Mason Yang 2018-11-19 259 if (ret)
c4789a83 Mason Yang 2018-11-19 260 goto out;
c4789a83 Mason Yang 2018-11-19 261
c4789a83 Mason Yang 2018-11-19 262 pos += nbytes;
c4789a83 Mason Yang 2018-11-19 263 smenr = rpc->smenr & ~RPC_SMENR_CDE &
c4789a83 Mason Yang 2018-11-19 264 ~RPC_SMENR_ADE(0xf);
c4789a83 Mason Yang 2018-11-19 265 }
c4789a83 Mason Yang 2018-11-19 266 } else if (rx_buf) {
c4789a83 Mason Yang 2018-11-19 267 while (pos < rpc->xferlen) {
c4789a83 Mason Yang 2018-11-19 268 u32 nbytes = rpc->xferlen - pos;
c4789a83 Mason Yang 2018-11-19 269
c4789a83 Mason Yang 2018-11-19 270 if (nbytes > 4)
c4789a83 Mason Yang 2018-11-19 271 nbytes = 4;
c4789a83 Mason Yang 2018-11-19 272
c4789a83 Mason Yang 2018-11-19 273 writel(rpc->cmd, rpc->regs + RPC_SMCMR);
c4789a83 Mason Yang 2018-11-19 274 writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
c4789a83 Mason Yang 2018-11-19 275 writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
c4789a83 Mason Yang 2018-11-19 276 writel(rpc->smenr, rpc->regs + RPC_SMENR);
c4789a83 Mason Yang 2018-11-19 277 writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
c4789a83 Mason Yang 2018-11-19 278 ret = wait_msg_xfer_end(rpc);
c4789a83 Mason Yang 2018-11-19 279 if (ret)
c4789a83 Mason Yang 2018-11-19 280 goto out;
c4789a83 Mason Yang 2018-11-19 281
c4789a83 Mason Yang 2018-11-19 282 data = readl(rpc->regs + RPC_SMRDR0);
c4789a83 Mason Yang 2018-11-19 @283 memcpy_fromio(rx_buf + pos, (void *)&data, nbytes);
c4789a83 Mason Yang 2018-11-19 284 pos += nbytes;
c4789a83 Mason Yang 2018-11-19 285 }
c4789a83 Mason Yang 2018-11-19 286 } else {
c4789a83 Mason Yang 2018-11-19 287 writel(rpc->cmd, rpc->regs + RPC_SMCMR);
c4789a83 Mason Yang 2018-11-19 288 writel(rpc->dummy, rpc->regs + RPC_SMDMCR);
c4789a83 Mason Yang 2018-11-19 289 writel(rpc->addr + pos, rpc->regs + RPC_SMADR);
c4789a83 Mason Yang 2018-11-19 290 writel(rpc->smenr, rpc->regs + RPC_SMENR);
c4789a83 Mason Yang 2018-11-19 291 writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
c4789a83 Mason Yang 2018-11-19 292 ret = wait_msg_xfer_end(rpc);
c4789a83 Mason Yang 2018-11-19 293 }
c4789a83 Mason Yang 2018-11-19 294 out:
c4789a83 Mason Yang 2018-11-19 295 return ret;
c4789a83 Mason Yang 2018-11-19 296 }
c4789a83 Mason Yang 2018-11-19 297
c4789a83 Mason Yang 2018-11-19 298 static void rpc_spi_mem_set_prep_op_cfg(struct spi_device *spi,
c4789a83 Mason Yang 2018-11-19 299 const struct spi_mem_op *op,
c4789a83 Mason Yang 2018-11-19 300 u64 *offs, size_t *len)
c4789a83 Mason Yang 2018-11-19 301 {
c4789a83 Mason Yang 2018-11-19 302 struct rpc_spi *rpc = spi_master_get_devdata(spi->master);
c4789a83 Mason Yang 2018-11-19 303
c4789a83 Mason Yang 2018-11-19 304 rpc->cmd = RPC_SMCMR_CMD(op->cmd.opcode);
c4789a83 Mason Yang 2018-11-19 305 rpc->smenr = RPC_SMENR_CDE |
c4789a83 Mason Yang 2018-11-19 306 RPC_SMENR_CDB(fls(op->cmd.buswidth >> 1));
c4789a83 Mason Yang 2018-11-19 307 rpc->totalxferlen = 1;
c4789a83 Mason Yang 2018-11-19 308 rpc->xferlen = 0;
c4789a83 Mason Yang 2018-11-19 309 rpc->addr = 0;
c4789a83 Mason Yang 2018-11-19 310
c4789a83 Mason Yang 2018-11-19 311 if (op->addr.nbytes) {
c4789a83 Mason Yang 2018-11-19 312 rpc->smenr |= RPC_SMENR_ADB(fls(op->addr.buswidth >> 1));
c4789a83 Mason Yang 2018-11-19 313 if (op->addr.nbytes == 4)
c4789a83 Mason Yang 2018-11-19 314 rpc->smenr |= RPC_SMENR_ADE(0xf);
c4789a83 Mason Yang 2018-11-19 315 else
c4789a83 Mason Yang 2018-11-19 316 rpc->smenr |= RPC_SMENR_ADE(0x7);
c4789a83 Mason Yang 2018-11-19 317
c4789a83 Mason Yang 2018-11-19 318 if (!offs && !len)
c4789a83 Mason Yang 2018-11-19 319 rpc->addr = *(u32 *)offs;
c4789a83 Mason Yang 2018-11-19 320 else
c4789a83 Mason Yang 2018-11-19 321 rpc->addr = op->addr.val;
c4789a83 Mason Yang 2018-11-19 322 rpc->totalxferlen += op->addr.nbytes;
c4789a83 Mason Yang 2018-11-19 323 }
c4789a83 Mason Yang 2018-11-19 324
c4789a83 Mason Yang 2018-11-19 325 if (op->dummy.nbytes) {
c4789a83 Mason Yang 2018-11-19 326 rpc->smenr |= RPC_SMENR_DME;
c4789a83 Mason Yang 2018-11-19 327 rpc->dummy = RPC_SMDMCR_DMCYC(op->dummy.nbytes);
c4789a83 Mason Yang 2018-11-19 328 rpc->totalxferlen += op->dummy.nbytes;
c4789a83 Mason Yang 2018-11-19 329 }
c4789a83 Mason Yang 2018-11-19 330
c4789a83 Mason Yang 2018-11-19 331 if (op->data.nbytes || (offs && len)) {
c4789a83 Mason Yang 2018-11-19 332 rpc->smenr |= RPC_SMENR_SPIDE(rpc_bits_xfer(op->data.nbytes)) |
c4789a83 Mason Yang 2018-11-19 333 RPC_SMENR_SPIDB(fls(op->data.buswidth >> 1));
c4789a83 Mason Yang 2018-11-19 334
c4789a83 Mason Yang 2018-11-19 335 if (op->data.dir == SPI_MEM_DATA_IN) {
c4789a83 Mason Yang 2018-11-19 336 rpc->smcr = RPC_SMCR_SPIRE;
c4789a83 Mason Yang 2018-11-19 337 rpc->xfer_dir = SPI_MEM_DATA_IN;
c4789a83 Mason Yang 2018-11-19 338 } else if (op->data.dir == SPI_MEM_DATA_OUT) {
c4789a83 Mason Yang 2018-11-19 339 rpc->smcr = RPC_SMCR_SPIWE;
c4789a83 Mason Yang 2018-11-19 340 rpc->xfer_dir = SPI_MEM_DATA_OUT;
c4789a83 Mason Yang 2018-11-19 341 }
c4789a83 Mason Yang 2018-11-19 342
c4789a83 Mason Yang 2018-11-19 343 if (offs && len) {
c4789a83 Mason Yang 2018-11-19 344 rpc->xferlen = *(u32 *)len;
c4789a83 Mason Yang 2018-11-19 345 rpc->totalxferlen += *(u32 *)len;
c4789a83 Mason Yang 2018-11-19 346 } else {
c4789a83 Mason Yang 2018-11-19 347 rpc->xferlen = op->data.nbytes;
c4789a83 Mason Yang 2018-11-19 348 rpc->totalxferlen += op->data.nbytes;
c4789a83 Mason Yang 2018-11-19 349 }
c4789a83 Mason Yang 2018-11-19 350 }
c4789a83 Mason Yang 2018-11-19 351 }
c4789a83 Mason Yang 2018-11-19 352
c4789a83 Mason Yang 2018-11-19 353 static bool rpc_spi_mem_supports_op(struct spi_mem *mem,
c4789a83 Mason Yang 2018-11-19 354 const struct spi_mem_op *op)
c4789a83 Mason Yang 2018-11-19 355 {
c4789a83 Mason Yang 2018-11-19 356 if (op->data.buswidth > 4 || op->addr.buswidth > 4 ||
c4789a83 Mason Yang 2018-11-19 357 op->dummy.buswidth > 4 || op->cmd.buswidth > 4)
c4789a83 Mason Yang 2018-11-19 358 return false;
c4789a83 Mason Yang 2018-11-19 359
c4789a83 Mason Yang 2018-11-19 360 if (op->addr.nbytes > 4)
c4789a83 Mason Yang 2018-11-19 361 return false;
c4789a83 Mason Yang 2018-11-19 362
c4789a83 Mason Yang 2018-11-19 363 return true;
c4789a83 Mason Yang 2018-11-19 364 }
c4789a83 Mason Yang 2018-11-19 365
c4789a83 Mason Yang 2018-11-19 366 static ssize_t rpc_spi_mem_dirmap_read(struct spi_mem_dirmap_desc *desc,
c4789a83 Mason Yang 2018-11-19 367 u64 offs, size_t len, void *buf)
c4789a83 Mason Yang 2018-11-19 368 {
c4789a83 Mason Yang 2018-11-19 @369 struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
c4789a83 Mason Yang 2018-11-19 370 int ret;
c4789a83 Mason Yang 2018-11-19 371
c4789a83 Mason Yang 2018-11-19 @372 if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
c4789a83 Mason Yang 2018-11-19 373 return -EINVAL;
c4789a83 Mason Yang 2018-11-19 374
c4789a83 Mason Yang 2018-11-19 375 ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
c4789a83 Mason Yang 2018-11-19 376 if (ret)
c4789a83 Mason Yang 2018-11-19 377 return ret;
c4789a83 Mason Yang 2018-11-19 378
c4789a83 Mason Yang 2018-11-19 379 rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
c4789a83 Mason Yang 2018-11-19 380 &desc->info.op_tmpl, &offs, &len);
c4789a83 Mason Yang 2018-11-19 381
c4789a83 Mason Yang 2018-11-19 382 writel(RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
c4789a83 Mason Yang 2018-11-19 383 RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
c4789a83 Mason Yang 2018-11-19 384
c4789a83 Mason Yang 2018-11-19 385 writel(RPC_DRCR_RBURST(0x1f) | RPC_DRCR_RBE, rpc->regs + RPC_DRCR);
c4789a83 Mason Yang 2018-11-19 386 writel(rpc->cmd, rpc->regs + RPC_DRCMR);
c4789a83 Mason Yang 2018-11-19 387 writel(RPC_DREAR_EAC, rpc->regs + RPC_DREAR);
c4789a83 Mason Yang 2018-11-19 388 writel(0, rpc->regs + RPC_DROPR);
c4789a83 Mason Yang 2018-11-19 389 writel(rpc->smenr, rpc->regs + RPC_DRENR);
c4789a83 Mason Yang 2018-11-19 390 writel(rpc->dummy, rpc->regs + RPC_DRDMCR);
c4789a83 Mason Yang 2018-11-19 391 writel(0x0, rpc->regs + RPC_DRDRENR);
c4789a83 Mason Yang 2018-11-19 392 memcpy_fromio(buf, rpc->linear.map + desc->info.offset + offs, len);
c4789a83 Mason Yang 2018-11-19 393
c4789a83 Mason Yang 2018-11-19 394 return len;
c4789a83 Mason Yang 2018-11-19 395 }
c4789a83 Mason Yang 2018-11-19 396
c4789a83 Mason Yang 2018-11-19 397 static ssize_t rpc_spi_mem_dirmap_write(struct spi_mem_dirmap_desc *desc,
c4789a83 Mason Yang 2018-11-19 398 u64 offs, size_t len, const void *buf)
c4789a83 Mason Yang 2018-11-19 399 {
c4789a83 Mason Yang 2018-11-19 @400 struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
c4789a83 Mason Yang 2018-11-19 401 int tx_offs, ret;
c4789a83 Mason Yang 2018-11-19 402
c4789a83 Mason Yang 2018-11-19 403 if (WARN_ON(offs + desc->info.offset + len > U32_MAX))
c4789a83 Mason Yang 2018-11-19 404 return -EINVAL;
c4789a83 Mason Yang 2018-11-19 405
c4789a83 Mason Yang 2018-11-19 406 if (WARN_ON(len > RPC_WBUF_SIZE))
c4789a83 Mason Yang 2018-11-19 407 return -EIO;
c4789a83 Mason Yang 2018-11-19 408
c4789a83 Mason Yang 2018-11-19 409 ret = rpc_spi_set_freq(rpc, desc->mem->spi->max_speed_hz);
c4789a83 Mason Yang 2018-11-19 410 if (ret)
c4789a83 Mason Yang 2018-11-19 411 return ret;
c4789a83 Mason Yang 2018-11-19 412
c4789a83 Mason Yang 2018-11-19 @413 rpc_spi_mem_set_prep_op_cfg(desc->mem->spi,
c4789a83 Mason Yang 2018-11-19 414 &desc->info.op_tmpl, &offs, &len);
c4789a83 Mason Yang 2018-11-19 415
c4789a83 Mason Yang 2018-11-19 416 writel(RPC_CMNCR_MD | RPC_CMNCR_SFDE | RPC_CMNCR_MOIIO_HIZ |
c4789a83 Mason Yang 2018-11-19 417 RPC_CMNCR_IOFV_HIZ | RPC_CMNCR_BSZ(0), rpc->regs + RPC_CMNCR);
c4789a83 Mason Yang 2018-11-19 418 writel(0x0, rpc->regs + RPC_SMDRENR);
c4789a83 Mason Yang 2018-11-19 419
c4789a83 Mason Yang 2018-11-19 420 writel(RPC_PHYCNT_CAL | 0x260 | RPC_PHYCNT_WBUF2 | RPC_PHYCNT_WBUF,
c4789a83 Mason Yang 2018-11-19 421 rpc->regs + RPC_PHYCNT);
c4789a83 Mason Yang 2018-11-19 422
c4789a83 Mason Yang 2018-11-19 423 for (tx_offs = 0; tx_offs < RPC_WBUF_SIZE; tx_offs += 4)
c4789a83 Mason Yang 2018-11-19 424 writel(*(u32 *)(buf + tx_offs), rpc->regs + RPC_WBUF + tx_offs);
c4789a83 Mason Yang 2018-11-19 425
c4789a83 Mason Yang 2018-11-19 426 writel(rpc->cmd, rpc->regs + RPC_SMCMR);
c4789a83 Mason Yang 2018-11-19 427 writel(offs, rpc->regs + RPC_SMADR);
c4789a83 Mason Yang 2018-11-19 428 writel(rpc->smenr, rpc->regs + RPC_SMENR);
c4789a83 Mason Yang 2018-11-19 429 writel(rpc->smcr | RPC_SMCR_SPIE, rpc->regs + RPC_SMCR);
c4789a83 Mason Yang 2018-11-19 430 ret = wait_msg_xfer_end(rpc);
c4789a83 Mason Yang 2018-11-19 431 if (ret)
c4789a83 Mason Yang 2018-11-19 432 goto out;
c4789a83 Mason Yang 2018-11-19 433
c4789a83 Mason Yang 2018-11-19 434 writel(RPC_DRCR_RCF, rpc->regs + RPC_DRCR);
c4789a83 Mason Yang 2018-11-19 435 writel(RPC_PHYCNT_CAL | RPC_PHYCNT_STRTIM(0) | 0x260,
c4789a83 Mason Yang 2018-11-19 436 rpc->regs + RPC_PHYCNT);
c4789a83 Mason Yang 2018-11-19 437
c4789a83 Mason Yang 2018-11-19 438 return len;
c4789a83 Mason Yang 2018-11-19 439 out:
c4789a83 Mason Yang 2018-11-19 440 return ret;
c4789a83 Mason Yang 2018-11-19 441 }
c4789a83 Mason Yang 2018-11-19 442
c4789a83 Mason Yang 2018-11-19 443 static int rpc_spi_mem_dirmap_create(struct spi_mem_dirmap_desc *desc)
c4789a83 Mason Yang 2018-11-19 444 {
c4789a83 Mason Yang 2018-11-19 445 struct rpc_spi *rpc = spi_master_get_devdata(desc->mem->spi->master);
c4789a83 Mason Yang 2018-11-19 446
c4789a83 Mason Yang 2018-11-19 447 if (desc->info.offset + desc->info.length > U32_MAX)
c4789a83 Mason Yang 2018-11-19 448 return -ENOTSUPP;
c4789a83 Mason Yang 2018-11-19 449
c4789a83 Mason Yang 2018-11-19 450 if (!rpc_spi_mem_supports_op(desc->mem, &desc->info.op_tmpl))
c4789a83 Mason Yang 2018-11-19 451 return -ENOTSUPP;
c4789a83 Mason Yang 2018-11-19 452
c4789a83 Mason Yang 2018-11-19 453 if (!rpc->linear.map &&
c4789a83 Mason Yang 2018-11-19 454 desc->info.op_tmpl.data.dir == SPI_MEM_DATA_IN)
c4789a83 Mason Yang 2018-11-19 455 return -ENOTSUPP;
c4789a83 Mason Yang 2018-11-19 456
c4789a83 Mason Yang 2018-11-19 457 return 0;
c4789a83 Mason Yang 2018-11-19 458 }
c4789a83 Mason Yang 2018-11-19 459
c4789a83 Mason Yang 2018-11-19 460 static int rpc_spi_mem_exec_op(struct spi_mem *mem,
c4789a83 Mason Yang 2018-11-19 461 const struct spi_mem_op *op)
c4789a83 Mason Yang 2018-11-19 462 {
c4789a83 Mason Yang 2018-11-19 463 struct rpc_spi *rpc = spi_master_get_devdata(mem->spi->master);
c4789a83 Mason Yang 2018-11-19 464 int ret;
c4789a83 Mason Yang 2018-11-19 465
c4789a83 Mason Yang 2018-11-19 466 ret = rpc_spi_set_freq(rpc, mem->spi->max_speed_hz);
c4789a83 Mason Yang 2018-11-19 467 if (ret)
c4789a83 Mason Yang 2018-11-19 468 return ret;
c4789a83 Mason Yang 2018-11-19 469
c4789a83 Mason Yang 2018-11-19 470 rpc_spi_mem_set_prep_op_cfg(mem->spi, op, NULL, NULL);
c4789a83 Mason Yang 2018-11-19 471
c4789a83 Mason Yang 2018-11-19 472 ret = rpc_spi_io_xfer(rpc,
c4789a83 Mason Yang 2018-11-19 473 op->data.dir == SPI_MEM_DATA_OUT ?
c4789a83 Mason Yang 2018-11-19 474 op->data.buf.out : NULL,
c4789a83 Mason Yang 2018-11-19 475 op->data.dir == SPI_MEM_DATA_IN ?
c4789a83 Mason Yang 2018-11-19 476 op->data.buf.in : NULL);
c4789a83 Mason Yang 2018-11-19 477
c4789a83 Mason Yang 2018-11-19 478 return ret;
c4789a83 Mason Yang 2018-11-19 479 }
c4789a83 Mason Yang 2018-11-19 480
c4789a83 Mason Yang 2018-11-19 481 static const struct spi_controller_mem_ops rpc_spi_mem_ops = {
c4789a83 Mason Yang 2018-11-19 482 .supports_op = rpc_spi_mem_supports_op,
c4789a83 Mason Yang 2018-11-19 483 .exec_op = rpc_spi_mem_exec_op,
c4789a83 Mason Yang 2018-11-19 @484 .dirmap_create = rpc_spi_mem_dirmap_create,
c4789a83 Mason Yang 2018-11-19 485 .dirmap_read = rpc_spi_mem_dirmap_read,
c4789a83 Mason Yang 2018-11-19 486 .dirmap_write = rpc_spi_mem_dirmap_write,
c4789a83 Mason Yang 2018-11-19 487 };
c4789a83 Mason Yang 2018-11-19 488
:::::: The code at line 369 was first introduced by commit
:::::: c4789a83a8be18f144419fba933a3f5ca1b78837 spi: Add Renesas R-Car RPC SPI controller driver
:::::: TO: Mason Yang <[email protected]>
:::::: CC: 0day robot <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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 :(
On Tue, 20 Nov 2018 14:09:05 +0100
Marek Vasut <[email protected]> wrote:
> On 11/20/2018 08:23 AM, [email protected] wrote:
> > Hi Marek,
>
> Hi,
>
> >> Marek Vasut <[email protected]>
> >> 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).
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 :-)
--
Best regards,
Marek Vasut
On 11/20/2018 06:42 AM, [email protected] wrote:
> Hi Marek,
>
> Marek Vasut <[email protected]> 已在 2018/11/19 下午 09:49:31 上寫入:
>
>> Marek Vasut <[email protected]>
>> 2018/11/19 下午 09:49
>>
>> To
>>
>> Mason Yang <[email protected]>, [email protected],
>> [email protected], [email protected], linux-
>> [email protected], [email protected], Simon Horman
>> <[email protected]>,
>>
>> cc
>>
>> [email protected], [email protected], Geert
>> Uytterhoeven <[email protected]>, [email protected]
>>
>> Subject
>>
>> Re: [PATCH 2/2] dt-binding: spi: Document Renesas R-Car RPC
>> controller bindings
>>
>> On 11/19/2018 11:01 AM, Mason Yang wrote:
>> > Document the bindings used by the Renesas R-Car D3 RPC controller.
>> >
>> > Signed-off-by: Mason Yang <[email protected]>
>> > ---
>> > .../devicetree/bindings/spi/spi-renesas-rpc.txt | 33 +++++++++
>> +++++++++++++
>> > 1 file changed, 33 insertions(+)
>> > create mode 100644 Documentation/devicetree/bindings/spi/spi-
>> renesas-rpc.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-
>> rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>> > new file mode 100644
>> > index 0000000..8286cc8
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>> > @@ -0,0 +1,33 @@
>> > +Renesas R-Car D3 RPC controller Device Tree Bindings
>> > +----------------------------------------------------
>> > +
>> > +Required properties:
>> > +- compatible: should be "renesas,rpc-r8a77995"
>> > +- #address-cells: should be 1
>> > +- #size-cells: should be 0
>> > +- reg: should contain 2 entries, one for the registers and one
>> for the direct
>> > + mapping area
>> > +- reg-names: should contain "rpc_regs" and "dirmap"
>> > +- interrupts: interrupt line connected to the RPC SPI controller
>>
>> Do you also plan to support the RPC HF mode ? And if so, how would that
>> look in the bindings ? I'd like to avoid having to redesign the bindings
>> later to handle both the SPI and HF modes.
>>
>
> I patched this RPC SPI/Octa driver is for mx25uw51245g and you may also
> refer to Boris's patch. [1][2][3]
Does this mean the driver is specific to one particular kind of flash ?
> [1] https://patchwork.kernel.org/cover/10638055/
> [2] https://patchwork.kernel.org/patch/10638057/
> [3] https://patchwork.kernel.org/patch/10638085/
--
Best regards,
Marek Vasut
On 11/20/2018 02:32 PM, Boris Brezillon wrote:
> On Tue, 20 Nov 2018 14:09:05 +0100
> Marek Vasut <[email protected]> wrote:
>
>> On 11/20/2018 08:23 AM, [email protected] wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>>> Marek Vasut <[email protected]>
>>>> 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.
--
Best regards,
Marek Vasut
On 11/21/2018 01:53 AM, [email protected] wrote:
> Hi Marek,
Hi,
>> Marek Vasut <[email protected]>
>> 2018/11/20 下午 08:57
>>
>> >> > diff --git a/Documentation/devicetree/bindings/spi/spi-renesas-
>> >> rpc.txt b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>> >> > new file mode 100644
>> >> > index 0000000..8286cc8
>> >> > --- /dev/null
>> >> > +++ b/Documentation/devicetree/bindings/spi/spi-renesas-rpc.txt
>> >> > @@ -0,0 +1,33 @@
>> >> > +Renesas R-Car D3 RPC controller Device Tree Bindings
>> >> > +----------------------------------------------------
>> >> > +
>> >> > +Required properties:
>> >> > +- compatible: should be "renesas,rpc-r8a77995"
>> >> > +- #address-cells: should be 1
>> >> > +- #size-cells: should be 0
>> >> > +- reg: should contain 2 entries, one for the registers and one
>> >> for the direct
>> >> > + mapping area
>> >> > +- reg-names: should contain "rpc_regs" and "dirmap"
>> >> > +- interrupts: interrupt line connected to the RPC SPI controller
>> >>
>> >> Do you also plan to support the RPC HF mode ? And if so, how would that
>> >> look in the bindings ? I'd like to avoid having to redesign the
> bindings
>> >> later to handle both the SPI and HF modes.
>> >>
>> >
>> > I patched this RPC SPI/Octa driver is for mx25uw51245g and you may also
>> > refer to Boris's patch. [1][2][3]
>>
>> Does this mean the driver is specific to one particular kind of flash ?
>>
>
> No, this driver supports all SPI flash, spi, qspi and octa flash.
>
> The target is Octa 8-8-8 DDR2 mode once spi-nor layer is merged.[1][2][3]
The HyperFlash must also be supported, cfr my previous comment that I'd
like to avoid redesigning the bindings again when the HF mode is added.
--
Best regards,
Marek Vasut
On 11/23/2018 01:45 AM, [email protected] 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 ?
[...]
--
Best regards,
Marek Vasut