linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
@ 2019-12-06 13:41 Chris Brandt
  2019-12-06 13:41 ` [PATCH v2 1/6] spi: Add SPIBSC driver Chris Brandt
                   ` (6 more replies)
  0 siblings, 7 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:41 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically designed for
accessing Serial flash devices (QSPI, HyperFlash, Octa Flash). In the hardware
manuals, it is almost always labeled as the "Renesas SPI Multi I/O Bus Controller".
However, the HW IP is usually referred to within Renesas as the "SPI BSC".
Yes, the R-Car team nicknamed it RPC (for "Reduced Pin Count" flash) after HyperFash
support was added...but I personally think that RPC is not a good name for this
HW block.


This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.

The testing mostly consisted of formatting an area as JFFS2 and doing copying
of files and such.

While the HW changed a little between the RZ/A1 and RZ/A2 generations, the IP
block in the RZ/A2M was taken from the R-Car H3 design, so in theory this
driver should work for R-Car Gen3 as well.

=========================
Version 2 changes
=========================
* I got rid of all the critical clock stuff. The idea is is that if you are
  planning on using the SPI BSC, even in XIP mode, it should be described in DT.

* There is no actual 'runtime pm' implmented in the driver at the moment, and
  so just the standard enable/disable clock API is used.

* The compatible string "jedec,spi-nor" will be used to determine if a spi controller
  needs to be regitered or not. At the moment there is no setup needed for
  running in XIP mode, so we just need to signal that the peripheral clock should
  be left on and then we're done.




Chris Brandt (6):
  spi: Add SPIBSC driver
  dt-bindings: spi: Document Renesas SPIBSC bindings
  clk: renesas: r7s9210: Add SPIBSC clock
  ARM: dts: r7s72100: Add SPIBSC devices
  ARM: dts: r7s9210: Add SPIBSC device
  ARM: dts: gr-peach: Enable SPIBSC

 .../bindings/spi/renesas,spibsc.yaml          | 115 ++++
 arch/arm/boot/dts/r7s72100-gr-peach.dts       |   5 +
 arch/arm/boot/dts/r7s72100.dtsi               |  25 +-
 arch/arm/boot/dts/r7s9210.dtsi                |  11 +
 drivers/clk/renesas/r7s9210-cpg-mssr.c        |   1 +
 drivers/spi/Kconfig                           |   8 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-spibsc.c                      | 612 ++++++++++++++++++
 8 files changed, 776 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
 create mode 100644 drivers/spi/spi-spibsc.c

-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
@ 2019-12-06 13:41 ` Chris Brandt
  2019-12-12 19:36   ` Sergei Shtylyov
  2019-12-06 13:41 ` [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings Chris Brandt
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:41 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

Add a driver for the SPIBSC controller in Renesas SoC devices.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * Kconfig should only depended on ARCH_RENESAS, no individual SoCs
 * Added COMPILE_TEST to Kconfig
 * Made entire header C++ style comments
 * Removed pm_runtime code
 * Removed function spibsc_print_data() and just use print_hex_dump_debug()
 * Added lines spaces in between functions
 * Reduced TEND timeout from 25ms to 32us
 * Added fall through comment to case statement
 * Moved message checking from transfer_one_message to prepare_message
 * Change failures of message checking prints from dev_dbg to dev_err
 * Change failures of message checking to return -EINVAL instead of -EIO
 * Check bits_per_word using .bits_per_word_mask instead of in.setup()
 * Check for a subnode with "jedec,spi-nor" to determine the HW mode
 * removed "probed" dev_info print at the end
 * Removed compatible  "renesas,spibsc"
---
 drivers/spi/Kconfig      |   8 +
 drivers/spi/Makefile     |   1 +
 drivers/spi/spi-spibsc.c | 612 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 621 insertions(+)
 create mode 100644 drivers/spi/spi-spibsc.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index 870f7797b56b..c80b46cd8d35 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -575,6 +575,14 @@ config SPI_RSPI
 	help
 	  SPI driver for Renesas RSPI and QSPI blocks.
 
+config SPI_SPIBSC
+	tristate "Renesas SPI Multi I/O Bus Controller"
+	depends on ARCH_RENESAS || COMPILE_TEST
+	help
+	  Also referred to as the SPI Bus Space Controller (SPIBSC).
+	  This controller was designed specifically for accessing serial flash
+	  devices.
+
 config SPI_QCOM_QSPI
 	tristate "QTI QSPI controller"
 	depends on ARCH_QCOM
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index bb49c9e6d0a0..9525256c4d51 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -99,6 +99,7 @@ obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
 obj-$(CONFIG_SPI_SIFIVE)		+= spi-sifive.o
 obj-$(CONFIG_SPI_SIRF)		+= spi-sirf.o
 obj-$(CONFIG_SPI_SLAVE_MT27XX)          += spi-slave-mt27xx.o
+obj-$(CONFIG_SPI_SPIBSC)		+= spi-spibsc.o
 obj-$(CONFIG_SPI_SPRD)			+= spi-sprd.o
 obj-$(CONFIG_SPI_SPRD_ADI)		+= spi-sprd-adi.o
 obj-$(CONFIG_SPI_STM32) 		+= spi-stm32.o
diff --git a/drivers/spi/spi-spibsc.c b/drivers/spi/spi-spibsc.c
new file mode 100644
index 000000000000..2c5bb249cdc5
--- /dev/null
+++ b/drivers/spi/spi-spibsc.c
@@ -0,0 +1,612 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// SPI Bus Space Controller (SPIBSC) bus driver
+// Otherwise known as a SPI Multi I/O Bus Controller
+//
+// Copyright (C) 2019 Renesas Electronics
+// Copyright (C) 2019 Chris Brandt
+//
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/delay.h>
+#include <linux/spi/spi.h>
+
+/* SPIBSC registers */
+#define	CMNCR	0x00
+#define	SSLDR	0x04
+#define SPBCR	0x08
+#define DRCR	0x0c
+#define DRCMR	0x10
+#define DREAR	0x14
+#define DROPR	0x18
+#define DRENR	0x1c
+#define	SMCR	0x20
+#define	SMCMR	0x24
+#define	SMADR	0x28
+#define	SMOPR	0x2c
+#define	SMENR	0x30
+#define SMRDR0	0x38
+#define SMRDR1	0x3c
+#define	SMWDR0	0x40
+#define SMWDR1	0x44
+#define	CMNSR	0x48
+#define SMDMCR	0x60
+#define SMDRENR	0x64
+
+/* CMNCR */
+#define	CMNCR_MD	BIT(31)
+#define	CMNCR_SFDE	BIT(24)
+#define	CMNCR_MOIIO3(x)	(((u32)(x) & 0x3) << 22)
+#define	CMNCR_MOIIO2(x)	(((u32)(x) & 0x3) << 20)
+#define	CMNCR_MOIIO1(x)	(((u32)(x) & 0x3) << 18)
+#define	CMNCR_MOIIO0(x)	(((u32)(x) & 0x3) << 16)
+#define	CMNCR_IO3FV(x)	(((u32)(x) & 0x3) << 14)
+#define	CMNCR_IO2FV(x)	(((u32)(x) & 0x3) << 12)
+#define	CMNCR_IO0FV(x)	(((u32)(x) & 0x3) << 8)
+#define	CMNCR_CPHAT	BIT(6)
+#define	CMNCR_CPHAR	BIT(5)
+#define	CMNCR_SSLP	BIT(4)
+#define	CMNCR_CPOL	BIT(3)
+#define	CMNCR_BSZ(n)	(((u32)(n) & 0x3) << 0)
+#define	OUT_0		(0u)
+#define	OUT_1		(1u)
+#define	OUT_REV		(2u)
+#define	OUT_HIZ		(3u)
+#define	BSZ_SINGLE	(0)
+#define	BSZ_DUAL	(1)
+#define CMNCR_INIT	(CMNCR_MD | \
+			CMNCR_SFDE | \
+			CMNCR_MOIIO3(OUT_HIZ) | \
+			CMNCR_MOIIO2(OUT_HIZ) | \
+			CMNCR_MOIIO1(OUT_HIZ) | \
+			CMNCR_MOIIO0(OUT_HIZ) | \
+			CMNCR_IO3FV(OUT_HIZ) | \
+			CMNCR_IO2FV(OUT_HIZ) | \
+			CMNCR_IO0FV(OUT_HIZ) | \
+			CMNCR_CPHAR | \
+			CMNCR_BSZ(BSZ_SINGLE))
+
+/* SSLDR */
+#define	SSLDR_SPNDL(x)	(((u32)(x) & 0x7) << 16)
+#define	SSLDR_SLNDL(x)	(((u32)(x) & 0x7) << 8)
+#define	SSLDR_SCKDL(x)	(((u32)(x) & 0x7) << 0)
+#define	SSLDR_INIT	(SSLDR_SPNDL(3) | SSLDR_SLNDL(3) | SSLDR_SCKDL(3))
+
+/* SPBCR */
+#define	SPBCR_SPBR(x)	(((u32)(x) & 0xff) << 8)
+#define	SPBCR_BRDV(x)	(((u32)(x) & 0x3) << 0)
+
+/* DRCR (read mode) */
+#define	DRCR_SSLN	BIT(24)
+#define	DRCR_RBURST(x)	(((u32)(x) & 0xf) << 16)
+#define	DRCR_RCF	BIT(9)
+#define	DRCR_RBE	BIT(8)
+#define	DRCR_SSLE	BIT(0)
+
+/* DROPR (read mode) */
+#define	DROPR_OPD3(o)	(((u32)(o) & 0xff) << 24)
+#define	DROPR_OPD2(o)	(((u32)(o) & 0xff) << 16)
+#define	DROPR_OPD1(o)	(((u32)(o) & 0xff) << 8)
+#define	DROPR_OPD0(o)	(((u32)(o) & 0xff) << 0)
+
+/* DRENR (read mode) */
+#define	DRENR_CDE	BIT(14)
+#define	DRENR_OCDE	BIT(12)
+#define	DRENR_OPDE(o)	(((u32)(o) & 0xf) << 4)
+
+/* SMCR (spi mode) */
+#define	SMCR_SSLKP	BIT(8)
+#define	SMCR_SPIRE	BIT(2)
+#define	SMCR_SPIWE	BIT(1)
+#define	SMCR_SPIE	BIT(0)
+
+/* SMCMR (spi mode) */
+#define	SMCMR_CMD(c)	(((u32)(c) & 0xff) << 16)
+#define	SMCMR_OCMD(o)	(((u32)(o) & 0xff) << 0)
+
+/* SMENR (spi mode) */
+#define	SMENR_SPIDE(b)	(((u32)(b) & 0xf) << 0)
+#define	SPIDE_8BITS	(0x8)
+#define	SPIDE_16BITS	(0xc)
+#define	SPIDE_32BITS	(0xf)
+
+/* CMNSR (spi mode) */
+#define	CMNSR_SSLF	BIT(1)
+#define	CMNSR_TEND	BIT(0)
+
+#define MAX_CMD_LEN 6
+
+/* HW Option Flags */
+#define HAS_SPBCR BIT(0)
+
+struct spibsc_priv {
+	void __iomem *base;	/* register base */
+	void __iomem *mmio;	/* memory mapped io space of SPI Flash */
+	int mmio_size;
+	struct spi_controller *master;
+	struct device *dev;
+	struct clk *clk;
+	int last_xfer;
+	int tx_only;
+	u32 flags;
+};
+
+static void spibsc_write(struct spibsc_priv *sbsc, int reg, u32 val)
+{
+	iowrite32(val, sbsc->base + reg);
+}
+
+static void spibsc_write8(struct spibsc_priv *sbsc, int reg, u8 val)
+{
+	iowrite8(val, sbsc->base + reg);
+}
+
+static void spibsc_write16(struct spibsc_priv *sbsc, int reg, u16 val)
+{
+	iowrite16(val, sbsc->base + reg);
+}
+
+static u32 spibsc_read(struct spibsc_priv *sbsc, int reg)
+{
+	return ioread32(sbsc->base + reg);
+}
+
+static int spibsc_wait_trans_completion(struct spibsc_priv *sbsc)
+{
+	int t = 320; /* 32us = 4 bytes at 1MHz */
+
+	while (t--) {
+		if (spibsc_read(sbsc, CMNSR) & CMNSR_TEND)
+			return 0;
+		ndelay(100);
+	}
+
+	dev_err(sbsc->dev, "Timeout waiting for TEND\n");
+	return -ETIMEDOUT;
+}
+
+/*
+ * This function sends data using 'SPI operation mode'. It is intended to be
+ * used only with SPI Flash commands that do not require any reading back from
+ * the SPI flash.
+ */
+static int spibsc_send_data(struct spibsc_priv *sbsc, const u8 *data, int len)
+{
+	u32 smcr, smenr, smwdr0;
+	int ret, unit, sslkp;
+
+	/* print data (for debugging) */
+#if defined(DEBUG_PRINT_DATA)
+	print_hex_dump_debug("TX data: ", DUMP_PREFIX_NONE, 16, 1, data, len,
+			     false);
+#endif
+
+	while (len > 0) {
+		if (len >= 4) {
+			unit = 4;
+			smenr = SMENR_SPIDE(SPIDE_32BITS);
+		} else {
+			unit = len;
+			if (unit == 3)
+				unit = 2;
+
+			if (unit >= 2)
+				smenr = SMENR_SPIDE(SPIDE_16BITS);
+			else
+				smenr = SMENR_SPIDE(SPIDE_8BITS);
+		}
+
+		/* set 4bytes data, bit stream */
+		smwdr0 = *data++;
+		if (unit >= 2)
+			smwdr0 |= (u32)(*data++ << 8);
+		if (unit >= 3)
+			smwdr0 |= (u32)(*data++ << 16);
+		if (unit >= 4)
+			smwdr0 |= (u32)(*data++ << 24);
+
+		/* mask unwrite area */
+		if (unit == 3)
+			smwdr0 |= 0xFF000000;
+		else if (unit == 2)
+			smwdr0 |= 0xFFFF0000;
+		else if (unit == 1)
+			smwdr0 |= 0xFFFFFF00;
+
+		/* write send data. */
+		if (unit == 2)
+			spibsc_write16(sbsc, SMWDR0, (u16)smwdr0);
+		else if (unit == 1)
+			spibsc_write8(sbsc, SMWDR0, (u8)smwdr0);
+		else
+			spibsc_write(sbsc, SMWDR0, smwdr0);
+
+		len -= unit;
+		if ((len <= 0) && sbsc->last_xfer)
+			sslkp = 0;
+		else
+			sslkp = 1;
+
+		/* set params */
+		spibsc_write(sbsc, SMCMR, 0);
+		spibsc_write(sbsc, SMADR, 0);
+		spibsc_write(sbsc, SMOPR, 0);
+		spibsc_write(sbsc, SMENR, smenr);
+
+		/* start spi transfer */
+		smcr = SMCR_SPIE|SMCR_SPIWE;
+		if (sslkp)
+			smcr |= SMCR_SSLKP;
+		spibsc_write(sbsc, SMCR, smcr);
+
+		ret = spibsc_wait_trans_completion(sbsc);
+		if (ret)
+			return  ret;
+	}
+	return 0;
+}
+
+/*
+ * This function uses 'Data Read' mode to send the command (and address) then
+ * read data back out.
+ * The HW was designed such that you program the registers with the command,
+ * base address, additional command data, etc... But, that makes things too
+ * difficult because it means this driver has to pick out those parameters from
+ * the data stream that was passed.
+ * Instead, we will ignore how the HW was 'supposed' to be used and just
+ * blindly put the Tx data (commands) to send in the registers in the order
+ * in which we know they will be transmitted:
+ *
+ * [DRCMR.CMD]+[DRCMR.OCMD]+[DROPR.OPD3]+[DROPR.OPD2]+[DROPR.OPD1]+[DROPR.OPD0]
+ *
+ * We can send up to 6 bytes this way.
+ * We will tell the HW to skip sending the 'address' because we are secretly
+ * including it in the "command" and that way we can leave the address registers
+ * blank.
+ *
+ * Note that when reading data, the HW will read in 8-byte units which usually
+ * is not an issue for SPI Flash devices.
+ */
+static int spibsc_send_recv_data(struct spibsc_priv *sbsc, u8 *tx_data,
+				 int tx_len, u8 *rx_data, int rx_len)
+{
+	u32 drcmr, drenr, dropr;
+	u8 opde;
+
+	dev_dbg(sbsc->dev, "%s (tx=%d, rx=%d)\n", __func__, tx_len, rx_len);
+
+	if (tx_len > MAX_CMD_LEN) {
+		dev_err(sbsc->dev, "Command length too long (%d)", tx_len);
+		return -EIO;
+	}
+
+	if (rx_len > sbsc->mmio_size) {
+		dev_err(sbsc->dev, "Receive length too long (%d)", rx_len);
+		return -EIO;
+	}
+
+	/* Setup Data Read mode
+	 * Flush read cache and enable burst mode. Burst mode is required
+	 * in order to keep chip select low between read transfers, but it
+	 * also means data is read in 8-byte intervals.
+	 */
+	spibsc_write(sbsc, DRCR, DRCR_SSLN | DRCR_RCF | DRCR_RBE | DRCR_SSLE);
+	spibsc_read(sbsc, DRCR);
+
+	/* Enter Data Read mode */
+	spibsc_write(sbsc, CMNCR, 0x01FFF300);
+	drcmr = 0;
+	drenr = 0;
+	dropr = 0;
+	opde = 0;
+
+	if (tx_len >= 1) {
+		/* Use 'Command' register for the 1st byte */
+		drcmr |= SMCMR_CMD(tx_data[0]);
+		drenr |= DRENR_CDE;
+	}
+
+	if (tx_len >= 2) {
+		/* Use 'Optional Command' register for the 2nd byte */
+		drcmr |= SMCMR_OCMD(tx_data[1]);
+		drenr |= DRENR_OCDE;
+	}
+
+	/* Use 'Option Data' for 3rd-6th bytes */
+	switch (tx_len) {
+	case 6:
+		dropr |= DROPR_OPD0(tx_data[5]);
+		opde |= (1 << 0);
+		/* fall through */
+	case 5:
+		dropr |= DROPR_OPD1(tx_data[4]);
+		opde |= (1 << 1);
+		/* fall through */
+	case 4:
+		dropr |= DROPR_OPD2(tx_data[3]);
+		opde |= (1 << 2);
+		/* fall through */
+	case 3:
+		dropr |= DROPR_OPD3(tx_data[2]);
+		opde |= (1 << 3);
+		drenr |= DRENR_OPDE(opde);
+	}
+
+	spibsc_write(sbsc, DRCMR, drcmr);
+	spibsc_write(sbsc, DROPR, dropr);
+	spibsc_write(sbsc, DRENR, drenr);
+
+	/* This internal bus access is what will trigger the actual Tx/Rx
+	 * operation. Remember, we don't care about the address.
+	 */
+	memcpy(rx_data, sbsc->mmio, rx_len);
+
+	/* Deactivate chip select */
+	spibsc_write(sbsc, DRCR, DRCR_SSLN);
+
+	/* Print data (for debugging) */
+#if defined(DEBUG_PRINT_DATA)
+	print_hex_dump_debug("TX data: ", DUMP_PREFIX_NONE, 16, 1, tx_data,
+			     tx_len, false);
+	print_hex_dump_debug("RX data: ", DUMP_PREFIX_NONE, 16, 1, rx_data,
+			     rx_len, false);
+#endif
+	return 0;
+}
+
+static int spibsc_prepare_message(struct spi_controller *ctlr,
+				  struct spi_message *msg)
+{
+	struct spibsc_priv *sbsc = spi_controller_get_devdata(ctlr);
+	struct spi_transfer *t, *t_last;
+
+	sbsc->last_xfer = 0;
+	sbsc->tx_only = 1;
+
+	t_last = list_last_entry(&msg->transfers, struct spi_transfer,
+				 transfer_list);
+
+	t = list_first_entry(&msg->transfers, struct spi_transfer,
+			     transfer_list);
+
+	if (t->rx_buf) {
+		dev_err(sbsc->dev, "Cannot Rx without Tx first!\n");
+		return -EINVAL;
+	}
+
+	list_for_each_entry(t, &msg->transfers, transfer_list) {
+
+		if (t->rx_buf) {
+			sbsc->tx_only = 0;
+			if (t != t_last) {
+				dev_err(sbsc->dev, "RX transaction is not the last transaction!\n");
+				return -EINVAL;
+			}
+		}
+		if (t->cs_change) {
+			dev_err(sbsc->dev, "cs_change not supported");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int spibsc_transfer_one_message(struct spi_controller *master,
+				       struct spi_message *msg)
+{
+	struct spibsc_priv *sbsc = spi_controller_get_devdata(master);
+	struct spi_transfer *t, *t_last;
+	u8 tx_data[MAX_CMD_LEN];
+	u8 tx_len;
+	int ret;
+
+	t_last = list_last_entry(&msg->transfers, struct spi_transfer,
+				 transfer_list);
+
+	/* Tx Only (SPI Mode is used) */
+	if (sbsc->tx_only) {
+
+		dev_dbg(sbsc->dev, "%s: TX only\n", __func__);
+
+		/* Initialize for SPI Mode */
+		spibsc_write(sbsc, CMNCR, CMNCR_INIT);
+
+		/* Send messages */
+		list_for_each_entry(t, &msg->transfers, transfer_list) {
+
+			if (t == t_last)
+				sbsc->last_xfer = 1;
+
+			ret = spibsc_send_data(sbsc, t->tx_buf, t->len);
+			if (ret)
+				break;
+
+			msg->actual_length += t->len;
+		}
+
+		goto done;
+	}
+
+	/* Tx, then RX (Data Read Mode is used) */
+	dev_dbg(sbsc->dev, "%s: Tx then Rx\n", __func__);
+
+	/* Buffer up the transmit portion (cmd + addr) so we can send it all at
+	 * once
+	 */
+	tx_len = 0;
+	list_for_each_entry(t, &msg->transfers, transfer_list) {
+		if (t->tx_buf) {
+			if ((tx_len + t->len) > sizeof(tx_data)) {
+				dev_err(sbsc->dev, "Command too big (%d)\n",
+					tx_len + t->len);
+				return -EINVAL;
+			}
+			memcpy(tx_data + tx_len, t->tx_buf, t->len);
+			tx_len += t->len;
+		}
+
+		if (t->rx_buf)
+			ret = spibsc_send_recv_data(sbsc, tx_data, tx_len,
+						    t->rx_buf,  t->len);
+
+		msg->actual_length += t->len;
+	}
+
+done:
+	msg->status = ret;
+	spi_finalize_current_message(master);
+
+	return ret;
+}
+
+static int spibsc_setup(struct spi_device *spi)
+{
+	struct spibsc_priv *sbsc = spi_controller_get_devdata(spi->master);
+	u32 max_speed_hz;
+	unsigned long rate;
+	u8 spbr;
+	u8 div_ratio;
+
+	if (sbsc->flags & HAS_SPBCR) {
+		max_speed_hz = spi->max_speed_hz;
+		rate = clk_get_rate(sbsc->clk);
+
+		div_ratio = 2;
+		spbr = 1;
+		while ((rate / div_ratio) > max_speed_hz) {
+			spbr++;
+			div_ratio += 2;
+			if (spbr == 255)
+				break;
+		}
+		spibsc_write(sbsc, SPBCR, SPBCR_SPBR(spbr) | SPBCR_BRDV(0));
+		dev_dbg(sbsc->dev, "Clock set to %ld Hz\n", rate/div_ratio);
+	}
+
+	return 0;
+}
+
+static int spibsc_probe(struct platform_device *pdev)
+{
+	struct spi_controller *master;
+	struct device_node *flash;
+	struct spibsc_priv *sbsc;
+	struct resource *res;
+	int ret;
+
+	master = spi_alloc_master(&pdev->dev, sizeof(*sbsc));
+	if (!master)
+		return -ENOMEM;
+
+	sbsc = spi_controller_get_devdata(master);
+	dev_set_drvdata(&pdev->dev, sbsc);
+	sbsc->master	= master;
+	sbsc->dev	= &pdev->dev;
+
+	sbsc->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(sbsc->clk)) {
+		dev_err(&pdev->dev, "cannot get clock\n");
+		ret = PTR_ERR(sbsc->clk);
+		goto error;
+	}
+	clk_prepare_enable(sbsc->clk);
+
+	/* Check what mode the SPIBSC will be used in */
+	flash = of_get_next_child(pdev->dev.of_node, NULL);
+	if (!flash || !of_device_is_compatible(flash, "jedec,spi-nor")) {
+		/*
+		 * We will assume that the HW will not be used in SPI mode,
+		 * but instead only in memory mapped read mode. We will also
+		 * assume that the interface has already been configured
+		 * by the boot loader. At this point, no other setup is needed
+		 * and there is no reason to register a SPI controller.
+		 * We need to keep the clock enabled, otherwise it will get
+		 * turned off at the and of kernel boot if the kernel thinks no
+		 * one is using it.
+		 */
+		dev_info(&pdev->dev, "External Address Space Read Mode\n");
+		return 0;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	sbsc->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(sbsc->base)) {
+		ret = PTR_ERR(sbsc->base);
+		goto error;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+	sbsc->mmio_size = resource_size(res);
+	sbsc->mmio = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(sbsc->mmio)) {
+		ret = PTR_ERR(sbsc->mmio);
+		goto error;
+	}
+
+	sbsc->flags = (u32) of_device_get_match_data(&pdev->dev);
+
+	master->bus_num = pdev->id;
+	master->num_chipselect	= 1;
+	master->mode_bits		= SPI_CPOL | SPI_CPHA;
+	master->bits_per_word_mask	= SPI_BPW_MASK(8);
+	master->setup			= spibsc_setup;
+	master->prepare_message		= spibsc_prepare_message;
+	master->transfer_one_message	= spibsc_transfer_one_message;
+	master->dev.of_node		= pdev->dev.of_node;
+
+	/* Initialize HW */
+	spibsc_write(sbsc, CMNCR, CMNCR_INIT);
+	spibsc_write(sbsc, DRCR, DRCR_RCF);
+	spibsc_write(sbsc, SSLDR, SSLDR_INIT);
+
+	ret = devm_spi_register_controller(&pdev->dev, master);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "spi_register_controller error.\n");
+		goto error;
+	}
+
+	return 0;
+
+error:
+	if (sbsc->clk && !IS_ERR(sbsc->clk))
+		clk_disable_unprepare(sbsc->clk);
+
+	spi_controller_put(master);
+
+	return ret;
+}
+
+static int spibsc_remove(struct platform_device *pdev)
+{
+	struct spibsc_priv *sbsc = dev_get_drvdata(&pdev->dev);
+
+	clk_disable_unprepare(sbsc->clk);
+
+	return 0;
+}
+
+static const struct of_device_id of_spibsc_match[] = {
+	{ .compatible = "renesas,r7s72100-spibsc", .data = (void *)HAS_SPBCR},
+	{ .compatible = "renesas,r7s9210-spibsc"},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, of_spibsc_match);
+
+static struct platform_driver spibsc_driver = {
+	.probe = spibsc_probe,
+	.remove = spibsc_remove,
+	.driver = {
+		.name = "spibsc",
+		.owner = THIS_MODULE,
+		.of_match_table = of_spibsc_match,
+	},
+};
+module_platform_driver(spibsc_driver);
+
+MODULE_DESCRIPTION("Renesas SPIBSC SPI Flash driver");
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Chris Brandt");
-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
  2019-12-06 13:41 ` [PATCH v2 1/6] spi: Add SPIBSC driver Chris Brandt
@ 2019-12-06 13:41 ` Chris Brandt
  2019-12-09 14:09   ` Geert Uytterhoeven
  2019-12-06 13:41 ` [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock Chris Brandt
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:41 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

Document the bindings used by the Renesas SPI bus space controller.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * change to YAML format
 * add interrupts property
 * Used more terms directly from the hardware manual
---
 .../bindings/spi/renesas,spibsc.yaml          | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/renesas,spibsc.yaml

diff --git a/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
new file mode 100644
index 000000000000..afbdc0824cc6
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/renesas,spibsc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas SPI Bus Space Controller (SPIBSC) Device Tree Bindings
+
+description: |
+  Otherwise referred to as the "SPI Multi I/O Bus Controller" in SoC hardware
+  manuals. This controller was designed specifically for accessing Serial flash
+  devices such as SPI Flash, HyperFlash and OctaFlash. The HW can operate in two
+  modes. One mode allows for normal byte by byte access (refereed to as
+  "Manual Mode"). The other mode allows for direct memory mapped access (read
+  only) to the flash area by the CPU (refereed to as "External Address Space
+  Read Mode").
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+maintainers:
+  - Chris Brandt <chris.brandt@renesas.com>
+
+properties:
+  compatible:
+    oneOf:
+      - items:
+          - enum:
+              - renesas,r7s72100-spibsc     # RZ/A1
+              - renesas,r7s9210-spibsc      # RZ/A2
+
+  reg:
+    minItems: 2
+    maxItems: 2
+    items:
+      - description: Registers
+      - description: Memory Mapped Address Space
+
+  interrupts:
+    description: Some HW versions do not contain interrupts
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  flash:
+    description: |
+      (Optional) In order to use the HW for R/W access ("Manual Mode"), a "flash"
+      subnode must be present with a "compatible" property that contains
+      "jedec,spi-nor". If a spi-nor property is not found, the HW is assumed to be
+      already operating in "External Address Space Read Mode".
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - '#address-cells'
+  - '#size-cells'
+
+examples:
+  - |
+    # This example is for "Manual Mode"
+    spibsc: spi@1f800000 {
+        compatible = "renesas,r7s9210-spibsc";
+        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
+        clocks = <&cpg CPG_MOD 83>;
+        power-domains = <&cpg>;
+        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        flash@0 {
+            compatible = "jedec,spi-nor";
+            reg = <0>;
+            spi-max-frequency = <40000000>;
+
+            partitions {
+                compatible = "fixed-partitions";
+                #address-cells = <1>;
+                #size-cells = <1>;
+
+                partition@0000000 {
+                    label = "u-boot";
+                    reg = <0x00000000 0x80000>;
+                };
+            };
+        };
+
+    # This example is for "External Address Space Read Mode"
+    spibsc: spi@1f800000 {
+        compatible = "renesas,r7s9210-spibsc";
+        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
+        clocks = <&cpg CPG_MOD 83>;
+        power-domains = <&cpg>;
+        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+    };
+    flash@20000000 {
+        compatible = "mtd-rom";
+        probe-type = "direct-mapped";
+        reg = <0x20000000 0x4000000>;
+        bank-width = <4>;
+        device-width = <1>;
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partition@80000 {
+            label ="uboot_env";
+            reg = <0x00080000 0x010000>;
+            read-only;
+        };
+    };
+
+...
-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
  2019-12-06 13:41 ` [PATCH v2 1/6] spi: Add SPIBSC driver Chris Brandt
  2019-12-06 13:41 ` [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings Chris Brandt
@ 2019-12-06 13:41 ` Chris Brandt
  2019-12-06 18:40   ` Sergei Shtylyov
  2019-12-06 13:42 ` [PATCH v2 4/6] ARM: dts: r7s72100: Add SPIBSC devices Chris Brandt
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:41 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

The SPIBSC clocks are marked as critical because for XIP systems, the
kernel will be running from QSPI flash and cannot be turned off.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * Removed spibsc from critical clock section
---
 drivers/clk/renesas/r7s9210-cpg-mssr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/renesas/r7s9210-cpg-mssr.c b/drivers/clk/renesas/r7s9210-cpg-mssr.c
index 14093503c085..261d88ae990b 100644
--- a/drivers/clk/renesas/r7s9210-cpg-mssr.c
+++ b/drivers/clk/renesas/r7s9210-cpg-mssr.c
@@ -93,6 +93,7 @@ static const struct mssr_mod_clk r7s9210_mod_clks[] __initconst = {
 	DEF_MOD_STB("ether1",	 64,	R7S9210_CLK_B),
 	DEF_MOD_STB("ether0",	 65,	R7S9210_CLK_B),
 
+	DEF_MOD_STB("spibsc",	 83,	R7S9210_CLK_P1),
 	DEF_MOD_STB("i2c3",	 84,	R7S9210_CLK_P1),
 	DEF_MOD_STB("i2c2",	 85,	R7S9210_CLK_P1),
 	DEF_MOD_STB("i2c1",	 86,	R7S9210_CLK_P1),
-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v2 4/6] ARM: dts: r7s72100: Add SPIBSC devices
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
                   ` (2 preceding siblings ...)
  2019-12-06 13:41 ` [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock Chris Brandt
@ 2019-12-06 13:42 ` Chris Brandt
  2019-12-06 13:42 ` [PATCH v2 5/6] ARM: dts: r7s9210: Add SPIBSC device Chris Brandt
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:42 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

Add the SPI BSC devices for the RZ/A1

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * renamed patch title
 * removed clock-critical declaration
---
 arch/arm/boot/dts/r7s72100.dtsi | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/r7s72100.dtsi b/arch/arm/boot/dts/r7s72100.dtsi
index d03dcd919d6f..548ddbf79c8f 100644
--- a/arch/arm/boot/dts/r7s72100.dtsi
+++ b/arch/arm/boot/dts/r7s72100.dtsi
@@ -101,6 +101,26 @@
 		#size-cells = <1>;
 		ranges;
 
+		spibsc0: spi@3fefa000 {
+			compatible = "renesas,r7s72100-spibsc", "renesas,spibsc";
+			reg = <0x3fefa000 0x100>, <0x18000000 0x4000000>;
+			clocks = <&mstp9_clks R7S72100_CLK_SPIBSC0>;
+			power-domains = <&cpg_clocks>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
+		spibsc1: spi@3fefb000 {
+			compatible = "renesas,r7s72100-spibsc", "renesas,spibsc";
+			reg = <0x3fefb000 0x100>, <0x1c000000 0x4000000>;
+			clocks = <&mstp9_clks R7S72100_CLK_SPIBSC1>;
+			power-domains = <&cpg_clocks>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		L2: cache-controller@3ffff000 {
 			compatible = "arm,pl310-cache";
 			reg = <0x3ffff000 0x1000>;
@@ -467,11 +487,12 @@
 			#clock-cells = <1>;
 			compatible = "renesas,r7s72100-mstp-clocks", "renesas,cpg-mstp-clocks";
 			reg = <0xfcfe0438 4>;
-			clocks = <&p0_clk>, <&p0_clk>, <&p0_clk>, <&p0_clk>;
+			clocks = <&p0_clk>, <&p0_clk>, <&p0_clk>, <&p0_clk>, <&b_clk>, <&b_clk>;
 			clock-indices = <
 				R7S72100_CLK_I2C0 R7S72100_CLK_I2C1 R7S72100_CLK_I2C2 R7S72100_CLK_I2C3
+				R7S72100_CLK_SPIBSC0 R7S72100_CLK_SPIBSC1
 			>;
-			clock-output-names = "i2c0", "i2c1", "i2c2", "i2c3";
+			clock-output-names = "i2c0", "i2c1", "i2c2", "i2c3", "spibsc0", "spibsc1";
 		};
 
 		mstp10_clks: mstp10_clks@fcfe043c {
-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v2 5/6] ARM: dts: r7s9210: Add SPIBSC device
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
                   ` (3 preceding siblings ...)
  2019-12-06 13:42 ` [PATCH v2 4/6] ARM: dts: r7s72100: Add SPIBSC devices Chris Brandt
@ 2019-12-06 13:42 ` Chris Brandt
  2019-12-06 13:42 ` [PATCH v2 6/6] ARM: dts: gr-peach: Enable SPIBSC Chris Brandt
  2019-12-07 20:28 ` [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Sergei Shtylyov
  6 siblings, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:42 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

Add SPIBSC Device support for RZ/A2.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2:
 * Changed reg range from 0x8c to 0x100
 * Added interrupts property
---
 arch/arm/boot/dts/r7s9210.dtsi | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/r7s9210.dtsi b/arch/arm/boot/dts/r7s9210.dtsi
index 72b79770e336..05c310c57c11 100644
--- a/arch/arm/boot/dts/r7s9210.dtsi
+++ b/arch/arm/boot/dts/r7s9210.dtsi
@@ -68,6 +68,17 @@
 			cache-level = <2>;
 		};
 
+		spibsc: spi@1f800000 {
+			compatible = "renesas,r7s9210-spibsc", "renesas,spibsc";
+			reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
+			clocks = <&cpg CPG_MOD 83>;
+			power-domains = <&cpg>;
+			interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		scif0: serial@e8007000 {
 			compatible = "renesas,scif-r7s9210";
 			reg = <0xe8007000 0x18>;
-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v2 6/6] ARM: dts: gr-peach: Enable SPIBSC
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
                   ` (4 preceding siblings ...)
  2019-12-06 13:42 ` [PATCH v2 5/6] ARM: dts: r7s9210: Add SPIBSC device Chris Brandt
@ 2019-12-06 13:42 ` Chris Brandt
  2019-12-07 20:28 ` [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Sergei Shtylyov
  6 siblings, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 13:42 UTC (permalink / raw)
  To: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Sergei Shtylyov, Chris Brandt

The SPIBSC is used to memory map the QSPI device for XIP of the kernel and
root file system.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/boot/dts/r7s72100-gr-peach.dts | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/boot/dts/r7s72100-gr-peach.dts b/arch/arm/boot/dts/r7s72100-gr-peach.dts
index fe1a4aa4d7cb..df3c37b4fa31 100644
--- a/arch/arm/boot/dts/r7s72100-gr-peach.dts
+++ b/arch/arm/boot/dts/r7s72100-gr-peach.dts
@@ -116,6 +116,11 @@
 	status = "okay";
 };
 
+/* Used for XIP of kernel and root file system. */
+&spibsc0 {
+	status = "okay";
+};
+
 &ether {
 	pinctrl-names = "default";
 	pinctrl-0 = <&ether_pins>;
-- 
2.23.0


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock
  2019-12-06 13:41 ` [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock Chris Brandt
@ 2019-12-06 18:40   ` Sergei Shtylyov
  2019-12-06 19:49     ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-06 18:40 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On 12/06/2019 04:41 PM, Chris Brandt wrote:

> The SPIBSC clocks are marked as critical because for XIP systems, the
> kernel will be running from QSPI flash and cannot be turned off.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
>  * Removed spibsc from critical clock section

   So you've removed it from the critical table but left the patch description
intact?

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock
  2019-12-06 18:40   ` Sergei Shtylyov
@ 2019-12-06 19:49     ` Chris Brandt
  2019-12-20 14:38       ` Geert Uytterhoeven
  0 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-06 19:49 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Fri, Dec 6, 2019, Sergei Shtylyov wrote:
> On 12/06/2019 04:41 PM, Chris Brandt wrote:
> 
> > The SPIBSC clocks are marked as critical because for XIP systems, the
> > kernel will be running from QSPI flash and cannot be turned off.
> >
> > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> > ---
> > v2:
> >  * Removed spibsc from critical clock section
> 
>    So you've removed it from the critical table but left the patch
> description intact?

Damn!
Thank you for pointing that out.
I remembered to take the comment out of one patch, but I missed this one.

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
                   ` (5 preceding siblings ...)
  2019-12-06 13:42 ` [PATCH v2 6/6] ARM: dts: gr-peach: Enable SPIBSC Chris Brandt
@ 2019-12-07 20:28 ` Sergei Shtylyov
  2019-12-09 15:10   ` Chris Brandt
  6 siblings, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-07 20:28 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello!

  Thank you for having mty on CC:, I might have missed oit otherwise... :-)

On 12/06/2019 04:41 PM, Chris Brandt wrote:

> The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically designed for
> accessing Serial flash devices (QSPI,

   The initial design did only support SPI, hence the SPI in the name.

> HyperFlash, Octa Flash). In the hardware

   Only added in "2nd generation" controllers, like on R-Car gen3, RZ/A2. 

> manuals, it is almost always labeled as the "Renesas SPI Multi I/O Bus Controller".

   Not seeing "Renesas" but the rest looks consistent across the manuals.

> However, the HW IP is usually referred to within Renesas as the "SPI BSC".

   Poor name for the 2nd generation controllers which also support at least HyperFlash.

> Yes, the R-Car team nicknamed it RPC (for "Reduced Pin Count" flash) after HyperFash
> support was added...but I personally think that RPC is not a good name for this
> HW block.

   SPIBSC is also misleading... RPC-IF seems misleading too as it's only spelled out
in the R-Car gen3 and RZ/A2H manuals. 

> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.

   In the SPI mode only, I assume?

   What I have now is the core driver (or rather a library) placed under drivers/memory/
and the SPI and HyperFlash front ends in drivers/spi/ and drivers/mtd/hyperbus/ respectfully.
I'm almost ready to post the core driver/bindings, the SPI driver still needs some Mark Brown's
comments addressed, and the HyperFlash driver is also ready but needs the existing HyperBus
infrastructure properly fixed up (having a draft patch now)...

> The testing mostly consisted of formatting an area as JFFS2 and doing copying
> of files and such.

   Did the same (or at least tried to :-) and I must admit that writing doesn't work with
any of the front ends... I still need to get this fixed.

> While the HW changed a little between the RZ/A1 and RZ/A2 generations, the IP
> block in the RZ/A2M was taken from the R-Car H3 design, so in theory this
> driver should work for R-Car Gen3 as well.

   I don't think it's a good idea to use the SPI dedicated driver on R-Car gen3, I would rather
see the RZ/A1 using the RPC-IF driver/library to reduce the code duplication...

> =========================
> Version 2 changes
> =========================
> * I got rid of all the critical clock stuff. The idea is is that if you are
>   planning on using the SPI BSC, even in XIP mode, it should be described in DT.
> 
> * There is no actual 'runtime pm' implmented in the driver at the moment, and
>   so just the standard enable/disable clock API is used.

   My code does have RPM enabled and used.

> * The compatible string "jedec,spi-nor" will be used to determine if a spi controller
>   needs to be regitered or not. At the moment there is no setup needed for
>   running in XIP mode, so we just need to signal that the peripheral clock should
>   be left on and then we're done.

[...]

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-06 13:41 ` [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings Chris Brandt
@ 2019-12-09 14:09   ` Geert Uytterhoeven
  2019-12-09 15:45     ` Chris Brandt
  2019-12-10 20:07     ` Sergei Shtylyov
  0 siblings, 2 replies; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-09 14:09 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang, Sergei Shtylyov

Hi Chris,

On Fri, Dec 6, 2019 at 2:43 PM Chris Brandt <chris.brandt@renesas.com> wrote:
> Document the bindings used by the Renesas SPI bus space controller.
>
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2:
>  * change to YAML format
>  * add interrupts property
>  * Used more terms directly from the hardware manual

Thanks for the update!

> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/renesas,spibsc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas SPI Bus Space Controller (SPIBSC) Device Tree Bindings
> +
> +description: |
> +  Otherwise referred to as the "SPI Multi I/O Bus Controller" in SoC hardware
> +  manuals. This controller was designed specifically for accessing Serial flash
> +  devices such as SPI Flash, HyperFlash and OctaFlash. The HW can operate in two
> +  modes. One mode allows for normal byte by byte access (refereed to as
> +  "Manual Mode"). The other mode allows for direct memory mapped access (read
> +  only) to the flash area by the CPU (refereed to as "External Address Space
> +  Read Mode").
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +maintainers:
> +  - Chris Brandt <chris.brandt@renesas.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,r7s72100-spibsc     # RZ/A1
> +              - renesas,r7s9210-spibsc      # RZ/A2
> +
> +  reg:
> +    minItems: 2
> +    maxItems: 2
> +    items:
> +      - description: Registers
> +      - description: Memory Mapped Address Space

The second one is not needed, if you would add "ranges" for the
memory-mapped mode.

> +
> +  interrupts:
> +    description: Some HW versions do not contain interrupts
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  flash:
> +    description: |
> +      (Optional) In order to use the HW for R/W access ("Manual Mode"), a "flash"
> +      subnode must be present with a "compatible" property that contains
> +      "jedec,spi-nor". If a spi-nor property is not found, the HW is assumed to be
> +      already operating in "External Address Space Read Mode".
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - '#address-cells'
> +  - '#size-cells'

I would make the flash subnode mandatory.

> +
> +examples:
> +  - |
> +    # This example is for "Manual Mode"
> +    spibsc: spi@1f800000 {
> +        compatible = "renesas,r7s9210-spibsc";
> +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
> +        clocks = <&cpg CPG_MOD 83>;
> +        power-domains = <&cpg>;
> +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        flash@0 {
> +            compatible = "jedec,spi-nor";
> +            reg = <0>;
> +            spi-max-frequency = <40000000>;
> +
> +            partitions {
> +                compatible = "fixed-partitions";
> +                #address-cells = <1>;
> +                #size-cells = <1>;
> +
> +                partition@0000000 {
> +                    label = "u-boot";
> +                    reg = <0x00000000 0x80000>;
> +                };
> +            };
> +        };
> +
> +    # This example is for "External Address Space Read Mode"
> +    spibsc: spi@1f800000 {
> +        compatible = "renesas,r7s9210-spibsc";
> +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
> +        clocks = <&cpg CPG_MOD 83>;
> +        power-domains = <&cpg>;
> +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +    };
> +    flash@20000000 {

This does not describe the hardware topology: the flash node should be
a subnode of the spibsc node, as it relies on the spibsc being clocked.

I think when using:

    spibsc: spi@1f800000 {
                compatible = "renesas,r7s9210-spibsc", "simple-pm-bus";
                reg = <0x1f800000 0x100>;
                clocks = <&cpg CPG_MOD 83>;
                power-domains = <&cpg>;
                interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
                #address-cells = <1>;
                #size-cells = <1>;
                ranges = <0 0x20000000 0x10000000>;

                flash@0 {
                        ...
                };
    };

and applying "[PATCH] mtd: maps: physmap: Add minimal Runtime PM
support"[1], the memory-mapped case should work, without your spibsc
driver.

Of course, you still need your driver for using the FLASH in SPI mode.

> +        compatible = "mtd-rom";
> +        probe-type = "direct-mapped";
> +        reg = <0x20000000 0x4000000>;
> +        bank-width = <4>;
> +        device-width = <1>;
> +        #address-cells = <1>;
> +        #size-cells = <1>;
1> +
> +        partition@80000 {
> +            label ="uboot_env";
> +            reg = <0x00080000 0x010000>;
> +            read-only;
> +        };
> +    };

[1] https://lore.kernel.org/linux-mtd/20191209134823.13330-1-geert+renesas@glider.be/

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-07 20:28 ` [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Sergei Shtylyov
@ 2019-12-09 15:10   ` Chris Brandt
  2019-12-11 19:09     ` Sergei Shtylyov
  0 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-09 15:10 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello Sergei,

On Sat, Dec 7, 2019, Sergei Shtylyov wrote:
> > The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically
> > designed for accessing Serial flash devices (QSPI,
> 
>    The initial design did only support SPI, hence the SPI in the name.

The more important part is the "Bus Space Controller". Meaning the main 
purpose of this hardware was to allow the CPU to access serial flash 
directly (as in, XIP).

"SPI-BSC" was the internal name for the HW but does not appear in any of
the hardware manual. The hardware manuals (even the MCUs) only say "SPI
Multi I/O Bus Controller".
Even the R-car gen3 manual says 'SPI':  "SPI Multi I/O Bus Controller 
(RPC)".

I have no idea why the R-Car people felt they needed to put "RPC" in the
hardware manual as the title of the chapter. (Although, "Multi I/O" is 
just as bad as a name)
 
I did make the request to the RZ/G team to not put "RPC" in the title of
the chapter in any future RZ/G hardware manuals.

Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash 
technologies, I would be find with a driver name of "SBSC" ("Serial Bus Space 
Controller") which at least looks closer to what is in all the hardware 
manuals.


>    SPIBSC is also misleading... RPC-IF seems misleading too as it's only
> spelled out in the R-Car gen3 and RZ/A2H manuals.

In the RZ/A2 manual, "RPC" is only used to label the 3 new external pins
that were added for HyperFlash.
  RPC_RESET# , RPC_WP# , RPC_INT#
But of course they were just copied from the R-Car manual.

But, maybe that's enough about the name for now.


> > This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
> 
>    In the SPI mode only, I assume?

Yes. At the moment, there are only requests from users for QSPI flash access
(RZ/A and RZ/G users).

The RZ/A2M EVB was laid out to support all the different combinations of
serial flashes (by populating different chips). That is why there is 
already Segger J-link support for QSPI, Hyper and Octa for the RZ/A2.

I will admit, to developed this driver for the "SPI-BSC" HW, I have been
using an XIP kernel (XIP from another HyperFlash / HyperRAM combo chip 
on the board) because I didn't feel like moving all the switches to use 
SDRAM and a uImage kernel.
The RZ/A2M has a HyperFlash controller (for R/W), a OctaBus controller 
(for R/W) and the SPI BSC (Read-only).


>    What I have now is the core driver (or rather a library) placed under
> drivers/memory/ and the SPI and HyperFlash front ends in drivers/spi/ and
> drivers/mtd/hyperbus/ respectfully.
> I'm almost ready to post the core driver/bindings, the SPI driver still needs
> some Mark Brown's comments addressed, and the HyperFlash driver is also ready
> but needs the existing HyperBus infrastructure properly fixed up (having a
> draft patch now)...

But are these for the HyperBus controller? Or the SPI-BSC controller?
They are 2 different controllers, so you would think they would have 2 different drivers.


> > The testing mostly consisted of formatting an area as JFFS2 and doing
> > copying of files and such.
> 
>    Did the same (or at least tried to :-) and I must admit that writing
> doesn't work with any of the front ends... I still need to get this fixed.


That's the part I'm confused about. I saw the last patch series that 
made it up to v17 but still didn't get in. Although, it did look very 
complicated.
You can see from my SPI-BSC driver, it's basically 2 function: a SPI 
write and SPI read. The upper layer sends you down data to write, and you 
just write it. In theory, if a HyperFlash MTD layer was sending down 
data, the commands bytes would be different, but the procedure would be the 
same.


> > While the HW changed a little between the RZ/A1 and RZ/A2 generations,
> > the IP block in the RZ/A2M was taken from the R-Car H3 design, so in
> > theory this driver should work for R-Car Gen3 as well.
> 
>    I don't think it's a good idea to use the SPI dedicated driver on R-Car
> gen3, I would rather see the RZ/A1 using the RPC-IF driver/library to reduce
> the code duplication...

I agree on not having competing drivers. Especially since future RZ/A 
and RZ/G devices will most likely continue to include this HW.

However, the driver I posted is pretty simple and works. Does the 
HyperFlash MTD library that you are proposing have a very different API than 
just 'send bytes' and 'receive bytes'?


Chris

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-09 14:09   ` Geert Uytterhoeven
@ 2019-12-09 15:45     ` Chris Brandt
  2019-12-09 19:34       ` Geert Uytterhoeven
  2019-12-10 20:07     ` Sergei Shtylyov
  1 sibling, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-09 15:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang, Sergei Shtylyov

Hi Geert,

On Mon, Dec 9, 2019, Geert Uytterhoeven wrote:
> > +  reg:
> > +    minItems: 2
> > +    maxItems: 2
> > +    items:
> > +      - description: Registers
> > +      - description: Memory Mapped Address Space
> 
> The second one is not needed, if you would add "ranges" for the memory-mapped
> mode.


So for RZ/A2M:
  ranges = <0x0 0x20000000 0x10000000>;


> > +    # This example is for "External Address Space Read Mode"
> > +    spibsc: spi@1f800000 {
> > +        compatible = "renesas,r7s9210-spibsc";
> > +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
> > +        clocks = <&cpg CPG_MOD 83>;
> > +        power-domains = <&cpg>;
> > +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +    };
> > +    flash@20000000 {
> 
> This does not describe the hardware topology: the flash node should be a
> subnode of the spibsc node, as it relies on the spibsc being clocked.

So for the "XIP" case, I originally tried adding an "mtd-rom" flash node
under the spibsc node, but then the mtd-rom part never got probed. I 
guess that was because it didn't register a SPI controller.


But, I guess if we go your method...
>     spibsc: spi@1f800000 {
>                 compatible = "renesas,r7s9210-spibsc", "simple-pm-bus";

Then after the spibsc driver fails and the "simple-pm-bus" driver tries,
it will succeed and the simple-pm-bus driver will start probing the 
subnodes (in my case, the mtd-rom).


> and applying "[PATCH] mtd: maps: physmap: Add minimal Runtime PM support"[1],
> the memory-mapped case should work, without your spibsc driver.

Good.
So we can add the SPI-BSC clocks for RZ/A1 and RZ/A2 (even without the 
SPI-BSC driver) and still have a working solution for XIP_KERNEL.



So in the end, this all seems like a very simple solution to get 
everything I wanted with minimal complexity.

But, if Sergei is going a completely different route for R-Car, I guess 
I need to understand that first what he is trying to do before I really
push for this driver getting in.
Again, this driver was only when using the SPI-BSC HW, not the full 
(different) R/W HyperFlash controller HW. That would be a separate driver.

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-09 15:45     ` Chris Brandt
@ 2019-12-09 19:34       ` Geert Uytterhoeven
  0 siblings, 0 replies; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-09 19:34 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang, Sergei Shtylyov

Hi Chris,

On Mon, Dec 9, 2019 at 4:45 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Mon, Dec 9, 2019, Geert Uytterhoeven wrote:
> > > +    # This example is for "External Address Space Read Mode"
> > > +    spibsc: spi@1f800000 {
> > > +        compatible = "renesas,r7s9210-spibsc";
> > > +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
> > > +        clocks = <&cpg CPG_MOD 83>;
> > > +        power-domains = <&cpg>;
> > > +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
> > > +        #address-cells = <1>;
> > > +        #size-cells = <0>;
> > > +    };
> > > +    flash@20000000 {
> >
> > This does not describe the hardware topology: the flash node should be a
> > subnode of the spibsc node, as it relies on the spibsc being clocked.
>
> So for the "XIP" case, I originally tried adding an "mtd-rom" flash node
> under the spibsc node, but then the mtd-rom part never got probed. I
> guess that was because it didn't register a SPI controller.

To probe subnodes, your node needs to either be compatible with e.g.
"simple-bus", or have its own driver that calls of_platform_populate().

> But, I guess if we go your method...
> >     spibsc: spi@1f800000 {
> >                 compatible = "renesas,r7s9210-spibsc", "simple-pm-bus";
>
> Then after the spibsc driver fails and the "simple-pm-bus" driver tries,
> it will succeed and the simple-pm-bus driver will start probing the
> subnodes (in my case, the mtd-rom).

Yes, and unlike "simple-bus", "simple-pm-bus" does handle Runtime PM,
so the clock will be enabled when needed.
BTW, I still think "simple-bus" should handle Runtime PM, and
"simple-pm-bus" should not exist.

> > and applying "[PATCH] mtd: maps: physmap: Add minimal Runtime PM support"[1],
> > the memory-mapped case should work, without your spibsc driver.
>
> Good.
> So we can add the SPI-BSC clocks for RZ/A1 and RZ/A2 (even without the
> SPI-BSC driver) and still have a working solution for XIP_KERNEL.
>
> So in the end, this all seems like a very simple solution to get
> everything I wanted with minimal complexity.

Exactly.

> But, if Sergei is going a completely different route for R-Car, I guess
> I need to understand that first what he is trying to do before I really
> push for this driver getting in.
> Again, this driver was only when using the SPI-BSC HW, not the full
> (different) R/W HyperFlash controller HW. That would be a separate driver.

Yeah, the "real" serial FLASH functionality needs its own driver.
How to fit all the SPI/QSPI/HF pieces together in a working driver is
still TBD.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-09 14:09   ` Geert Uytterhoeven
  2019-12-09 15:45     ` Chris Brandt
@ 2019-12-10 20:07     ` Sergei Shtylyov
  2019-12-10 20:17       ` Geert Uytterhoeven
  2019-12-10 20:23       ` Chris Brandt
  1 sibling, 2 replies; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-10 20:07 UTC (permalink / raw)
  To: Geert Uytterhoeven, Chris Brandt
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang

Hello!

On 12/09/2019 05:09 PM, Geert Uytterhoeven wrote:

>> Document the bindings used by the Renesas SPI bus space controller.
>>
>> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
>> ---
>> v2:
>>  * change to YAML format
>>  * add interrupts property
>>  * Used more terms directly from the hardware manual
> 
> Thanks for the update!
> 
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
[...]
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - items:
>> +          - enum:
>> +              - renesas,r7s72100-spibsc     # RZ/A1
>> +              - renesas,r7s9210-spibsc      # RZ/A2
>> +
>> +  reg:
>> +    minItems: 2
>> +    maxItems: 2
>> +    items:
>> +      - description: Registers
>> +      - description: Memory Mapped Address Space
> 
> The second one is not needed, if you would add "ranges" for the
> memory-mapped mode.

   I'm not sure we can do that. The flash bus is accessed via a window
with the high bits in the DREAR reg, even in the direct read mode...

> 
>> +
>> +  interrupts:
>> +    description: Some HW versions do not contain interrupts
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  flash:
>> +    description: |
>> +      (Optional) In order to use the HW for R/W access ("Manual Mode"), a "flash"
>> +      subnode must be present with a "compatible" property that contains
>> +      "jedec,spi-nor". If a spi-nor property is not found, the HW is assumed to be
>> +      already operating in "External Address Space Read Mode".
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - '#address-cells'
>> +  - '#size-cells'
> 
> I would make the flash subnode mandatory.

   Agreed.

>> +
>> +examples:
>> +  - |
>> +    # This example is for "Manual Mode"
>> +    spibsc: spi@1f800000 {
>> +        compatible = "renesas,r7s9210-spibsc";
>> +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
>> +        clocks = <&cpg CPG_MOD 83>;
>> +        power-domains = <&cpg>;
>> +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        flash@0 {
>> +            compatible = "jedec,spi-nor";
>> +            reg = <0>;
>> +            spi-max-frequency = <40000000>;
>> +
>> +            partitions {
>> +                compatible = "fixed-partitions";
>> +                #address-cells = <1>;
>> +                #size-cells = <1>;
>> +
>> +                partition@0000000 {
>> +                    label = "u-boot";
>> +                    reg = <0x00000000 0x80000>;
>> +                };
>> +            };
>> +        };
>> +
>> +    # This example is for "External Address Space Read Mode"
>> +    spibsc: spi@1f800000 {
>> +        compatible = "renesas,r7s9210-spibsc";
>> +        reg = <0x1f800000 0x100>, <0x20000000 0x10000000>;
>> +        clocks = <&cpg CPG_MOD 83>;
>> +        power-domains = <&cpg>;
>> +        interrupts = <GIC_SPI 445 IRQ_TYPE_LEVEL_HIGH>;
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +    };
>> +    flash@20000000 {
> 
> This does not describe the hardware topology: the flash node should be
> a subnode of the spibsc node, as it relies on the spibsc being clocked.

   ACK.

[...]

> Gr{oetje,eeting}s,
> 
>                         Geert

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-10 20:07     ` Sergei Shtylyov
@ 2019-12-10 20:17       ` Geert Uytterhoeven
  2019-12-10 20:33         ` Chris Brandt
  2019-12-10 20:23       ` Chris Brandt
  1 sibling, 1 reply; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-10 20:17 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang

Hi Sergei,

On Tue, Dec 10, 2019 at 9:07 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 12/09/2019 05:09 PM, Geert Uytterhoeven wrote:
> >> Document the bindings used by the Renesas SPI bus space controller.
> >> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> >> ---
> >> v2:
> >>  * change to YAML format
> >>  * add interrupts property
> >>  * Used more terms directly from the hardware manual
> >
> > Thanks for the update!
> >
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/spi/renesas,spibsc.yaml
> [...]
> >> +properties:
> >> +  compatible:
> >> +    oneOf:
> >> +      - items:
> >> +          - enum:
> >> +              - renesas,r7s72100-spibsc     # RZ/A1
> >> +              - renesas,r7s9210-spibsc      # RZ/A2
> >> +
> >> +  reg:
> >> +    minItems: 2
> >> +    maxItems: 2
> >> +    items:
> >> +      - description: Registers
> >> +      - description: Memory Mapped Address Space
> >
> > The second one is not needed, if you would add "ranges" for the
> > memory-mapped mode.
>
>    I'm not sure we can do that. The flash bus is accessed via a window
> with the high bits in the DREAR reg, even in the direct read mode...

So if the FLASH is too large, you cannot access all of it without
programming the high bits.
However, when using XIP,  you're limited to the window size anyway.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-10 20:07     ` Sergei Shtylyov
  2019-12-10 20:17       ` Geert Uytterhoeven
@ 2019-12-10 20:23       ` Chris Brandt
  1 sibling, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-10 20:23 UTC (permalink / raw)
  To: Sergei Shtylyov, Geert Uytterhoeven
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang

On Tue, Dec 10, 2019, Sergei Shtylyov wrote:
> >> +    items:
> >> +      - description: Registers
> >> +      - description: Memory Mapped Address Space
> >
> > The second one is not needed, if you would add "ranges" for the
> > memory-mapped mode.
> 
>    I'm not sure we can do that. The flash bus is accessed via a window with
> the high bits in the DREAR reg, even in the direct read mode...

The purpose of this driver was to allow R/W access using the existing 
MTD/SPI layer.
When the HW is operating in this mode, the driver basically just needs 
to read the vale of "ranges" in the DT to know the base address, and 
that's all it uses it for.

If the HW is going to be used in 'direct read mode', then the MTD/SPI 
layer will not be used. Instead the MTD/ROM driver will be used.
Configuring the HW for this mode is out of scope at the moment and 
should be done in the boot loader (just like DDR or SDRAM is configured by 
the boot loader).
The MTD/ROM driver will also handle all the ioremapping based on the 
values in the partition values under the flash node.

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings
  2019-12-10 20:17       ` Geert Uytterhoeven
@ 2019-12-10 20:33         ` Chris Brandt
  0 siblings, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-10 20:33 UTC (permalink / raw)
  To: Geert Uytterhoeven, Sergei Shtylyov
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-Renesas, linux-clk, Mason Yang

On Tue, Dec 10, 2019, Geert Uytterhoeven wrote:
> > > The second one is not needed, if you would add "ranges" for the
> > > memory-mapped mode.
> >
> >    I'm not sure we can do that. The flash bus is accessed via a window
> > with the high bits in the DREAR reg, even in the direct read mode...
> 
> So if the FLASH is too large, you cannot access all of it without programming
> the high bits.
> However, when using XIP,  you're limited to the window size anyway.

If your SPI flash supports 32-bit address commands, ("4READ" for 
example) then all 32-bit are sent out.

The 'high bits' that he's talking about is for accessing SPI that is 
beyond the XIP window size.

For example, the RZ/A1 has 64MB window size (RZ/A2=256MB). Meaning 
anything above 64MB is not accessible.
However, you could program this register to basically change the window 
starting point and make the upper 64MB of a 128MB flash visible and the
lower 64MB portion hidden.
For an XIP system, this is one way of doing a remote firmware update 
and keeping the old copy around just in case you need to revert back.

Anyway, the point is this doesn't matter in either the MTD/SPI or 
MTD/ROM operation of the driver as far as I can see.

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-09 15:10   ` Chris Brandt
@ 2019-12-11 19:09     ` Sergei Shtylyov
  2019-12-12 14:29       ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-11 19:09 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On 12/09/2019 06:10 PM, Chris Brandt wrote:

>>> The Renesas SPI Bus Space Controller (SPIBSC) HW was specifically
>>> designed for accessing Serial flash devices (QSPI,
>>
>>    The initial design did only support SPI, hence the SPI in the name.
> 
> The more important part is the "Bus Space Controller". Meaning the main 
> purpose of this hardware was to allow the CPU to access serial flash 
> directly (as in, XIP).
> 
> "SPI-BSC" was the internal name for the HW but does not appear in any of
> the hardware manual. The hardware manuals (even the MCUs) only say "SPI
> Multi I/O Bus Controller".
> Even the R-car gen3 manual says 'SPI':  "SPI Multi I/O Bus Controller 
> (RPC)".
> 
> I have no idea why the R-Car people felt they needed to put "RPC" in the
> hardware manual as the title of the chapter. (Although, "Multi I/O" is 
> just as bad as a name)
>  
> I did make the request to the RZ/G team to not put "RPC" in the title of
> the chapter in any future RZ/G hardware manuals.
> 
> Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash 
> technologies, I would be find with a driver name of "SBSC" ("Serial Bus Space 
> Controller") which at least looks closer to what is in all the hardware 
> manuals.

   How about "Serial Flash Controller" instead?

>>    SPIBSC is also misleading... RPC-IF seems misleading too as it's only
>> spelled out in the R-Car gen3 and RZ/A2H manuals.
> 
> In the RZ/A2 manual, "RPC" is only used to label the 3 new external pins
> that were added for HyperFlash.

   Sorry, I was to hasty to check the RZ/A2H manual before typing. :-/

>   RPC_RESET# , RPC_WP# , RPC_INT#
> But of course they were just copied from the R-Car manual.
> 
> But, maybe that's enough about the name for now.

   OK. :-)

>>> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
>>
>>    In the SPI mode only, I assume?
> 
> Yes. At the moment, there are only requests from users for QSPI flash access
> (RZ/A and RZ/G users).

   I keep being told by the management that we need HyperFlash too. :-)
In our BSP development, our engineers went "same hardware, 2 drivers"
way (with different "compatibles" per driver)...

> The RZ/A2M EVB was laid out to support all the different combinations of
> serial flashes (by populating different chips). That is why there is 
> already Segger J-link support for QSPI, Hyper and Octa for the RZ/A2.
> 
> I will admit, to developed this driver for the "SPI-BSC" HW, I have been
> using an XIP kernel (XIP from another HyperFlash / HyperRAM combo chip 
> on the board) because I didn't feel like moving all the switches to use 
> SDRAM and a uImage kernel.
> The RZ/A2M has a HyperFlash controller (for R/W), a OctaBus controller 
> (for R/W) and the SPI BSC (Read-only).

   Seen these...

>>    What I have now is the core driver (or rather a library) placed under
>> drivers/memory/ and the SPI and HyperFlash front ends in drivers/spi/ and
>> drivers/mtd/hyperbus/ respectfully.
>> I'm almost ready to post the core driver/bindings, the SPI driver still needs
>> some Mark Brown's comments addressed, and the HyperFlash driver is also ready
>> but needs the existing HyperBus infrastructure properly fixed up (having a
>> draft patch now)...

> But are these for the HyperBus controller? Or the SPI-BSC controller?
> They are 2 different controllers, so you would think they would have 2 different drivers.

   R-Car gen3 only has RPC-IF, no separate HyperBus controller, so the second case.

>>> The testing mostly consisted of formatting an area as JFFS2 and doing
>>> copying of files and such.
>>
>>    Did the same (or at least tried to :-) and I must admit that writing
>> doesn't work with any of the front ends... I still need to get this fixed.

   The last word from our BSP people was that JFFS2 doesn't work with the HyperFLash
dedicated BSP driver... :-/

> That's the part I'm confused about. I saw the last patch series that 
> made it up to v17 but still didn't get in. Although, it did look very 
> complicated.
> You can see from my SPI-BSC driver, it's basically 2 function: a SPI 

   I'll read/try it next thing...

> write and SPI read. The upper layer sends you down data to write, and you 
> just write it. In theory, if a HyperFlash MTD layer was sending down 
> data, the commands bytes would be different, but the procedure would be the 
> same.

   Yeah, the commands are different...

>>> While the HW changed a little between the RZ/A1 and RZ/A2 generations,
>>> the IP block in the RZ/A2M was taken from the R-Car H3 design, so in
>>> theory this driver should work for R-Car Gen3 as well.
>>
>>    I don't think it's a good idea to use the SPI dedicated driver on R-Car
>> gen3, I would rather see the RZ/A1 using the RPC-IF driver/library to reduce
>> the code duplication...
> 
> I agree on not having competing drivers. Especially since future RZ/A 
> and RZ/G devices will most likely continue to include this HW.

> However, the driver I posted is pretty simple and works. Does the 
> HyperFlash MTD

   There's no HF library, only front end driver.
   The real library covers both SPI and HF. The only difference between the two
is the h/w setup (minor difference).

> library that you are proposing have a very different API than 
> just 'send bytes' and 'receive bytes'?

   There's "prepare" and "transfer" APIs and also "direct map read" API.

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-11 19:09     ` Sergei Shtylyov
@ 2019-12-12 14:29       ` Chris Brandt
  2019-12-12 15:28         ` Mark Brown
  2019-12-16 20:31         ` Sergei Shtylyov
  0 siblings, 2 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-12 14:29 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Wed, Dec 11, 2019, Sergei Shtylyov wrote:
> > Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash
> > technologies, I would be find with a driver name of "SBSC" ("Serial
> > Bus Space
> > Controller") which at least looks closer to what is in all the
> > hardware manuals.
> 
>    How about "Serial Flash Controller" instead?

I would like that better than "RPC". At least it describes what it is.
RPC seems like a stupid name to me (but maybe that's just because I know
how that name was chosen...)
https://www.cypress.com/news/cypress-simplifies-embedded-system-design-new-low-pin-count-hyperram-memory
 "The HyperRAM and HyperFlash solution reduces pin count by at least 28 pins, ..."


As a side note, there is another HW block in Renesas that does the same 
thing as the SPI-BSC that they use in the MCU devices. That one they 
just named "QSPI".

> >>> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
> >>
> >>    In the SPI mode only, I assume?
> >
> > Yes. At the moment, there are only requests from users for QSPI flash
> > access (RZ/A and RZ/G users).
> 
>    I keep being told by the management that we need HyperFlash too. :-) In
> our BSP development, our engineers went "same hardware, 2 drivers"
> way (with different "compatibles" per driver)...

My plan was same HW, same "compatibles", same driver...but the driver 
would either register a SPI controller or a Hyperflash controller.

Note that the MMC/SDHI is the same HW but can act like 2 different peripherals.
We also have USB that can be either host or peripheral.


> >>> The testing mostly consisted of formatting an area as JFFS2 and
> >>> doing copying of files and such.
> >>
> >>    Did the same (or at least tried to :-) and I must admit that
> >> writing doesn't work with any of the front ends... I still need to get
> this fixed.
> 
>    The last word from our BSP people was that JFFS2 doesn't work with the
> HyperFLash dedicated BSP driver... :-/

Is that why this "RPC" patch series is taking so long?
It's a fairly simple piece of hardware.

When I first saw the series on the mailing list, my plan was to just wait
and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.

So, I thought I would at least put in my own driver for SPI flash now, 
and then go back and add HyperFlash/OctaFlash once I get the chips 
swapped out on one of my RZ/A2 boards.


> > However, the driver I posted is pretty simple and works. Does the
> > HyperFlash MTD
> 
>    There's no HF library, only front end driver.
>    The real library covers both SPI and HF. The only difference between the
> two is the h/w setup (minor difference).

But is this "library" something specific to Renesas devices?
That's what I'm trying to understand.

My understanding is that HyperFlash uses standard CFI commands, so all 
we need to do is register a CFI device in the driver, just like we 
register a serial flash device.

(I guess I could go look at the sample code for our RTOS package and find out)


> > library that you are proposing have a very different API than just
> > 'send bytes' and 'receive bytes'?
> 
>    There's "prepare" and "transfer" APIs and also "direct map read" API.

I wonder what is the value of the "direct map read" (other than XIP in 
RZ/A systems). If you really want to directly access the flash (no 
buffering though the MTD layer), you need to register as a mtd-rom device, 
and then you don't really need an API at all.


Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-12 14:29       ` Chris Brandt
@ 2019-12-12 15:28         ` Mark Brown
  2019-12-12 16:53           ` Chris Brandt
  2019-12-16 20:31         ` Sergei Shtylyov
  1 sibling, 1 reply; 51+ messages in thread
From: Mark Brown @ 2019-12-12 15:28 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang

[-- Attachment #1: Type: text/plain, Size: 550 bytes --]

On Thu, Dec 12, 2019 at 02:29:07PM +0000, Chris Brandt wrote:
> On Wed, Dec 11, 2019, Sergei Shtylyov wrote:

> >    The last word from our BSP people was that JFFS2 doesn't work with the
> > HyperFLash dedicated BSP driver... :-/

> Is that why this "RPC" patch series is taking so long?
> It's a fairly simple piece of hardware.

The submitter appeared to be having difficulty with feedback from the
reviewers with knowledge of the hardware, then it looks like the last
version of the patch set didn't get any comments from any of those
reviewers.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-12 15:28         ` Mark Brown
@ 2019-12-12 16:53           ` Chris Brandt
  2019-12-12 17:13             ` Mark Brown
  0 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-12 16:53 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang

On Thu, Dec 12, 2019 1, Mark Brown wrote:
> > Is that why this "RPC" patch series is taking so long?
> > It's a fairly simple piece of hardware.
> 
> The submitter appeared to be having difficulty with feedback from the
> reviewers with knowledge of the hardware, then it looks like the last
> version of the patch set didn't get any comments from any of those
> reviewers.

I went and looked at the driver and it was more complicated than what I 
did so I wasn't sure if there was some more advanced functionality or
something that was trying to be achieved.

I admit, V1 of this hardware (in the RZ/A1) was easier than V2 (RZ/A2 & R-Car),
so I had to re-write our BSP driver when it came time to support RZ/A2.
Basically, when they added HyperFlash support...it 'changed' how SPI
support was working. 

I agree with Sergei that it is always better to have a common driver for
common HW across different SoCs. However, it's not clear yet why there
needs to be increased complexity.

There were some good suggestions from the V2 series that I think complete
this driver. But I have not sent out a V3 until I can better understand
this 'competing' solution.

Chris

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-12 16:53           ` Chris Brandt
@ 2019-12-12 17:13             ` Mark Brown
  2019-12-12 17:25               ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Mark Brown @ 2019-12-12 17:13 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

On Thu, Dec 12, 2019 at 04:53:26PM +0000, Chris Brandt wrote:

> There were some good suggestions from the V2 series that I think complete
> this driver. But I have not sent out a V3 until I can better understand
> this 'competing' solution.

Oh, this is a new driver for the same hardware as the RPC driver :(

I don't really know enough about the device and there was *huge* amounts
of discussion which I'd have to try to page in so it'd be really good if
there were some agreement among those of you working with this device as
to what the best way forwards is.  I'm not sure any of the issues were
at the framework level so that's probably more sensible anyway.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-12 17:13             ` Mark Brown
@ 2019-12-12 17:25               ` Chris Brandt
  2019-12-16 15:21                 ` Mark Brown
  0 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-12 17:25 UTC (permalink / raw)
  To: Mark Brown
  Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang

On Thu, Dec 12, 2019, Mark Brown wrote:
> Oh, this is a new driver for the same hardware as the RPC driver :(

Correct. More or less.
Of the 3 devices (RZ/A1, RZ/A2, R-Car), RZ/A1 only supported SPI.
RZ/A2 and R-Car now support SPI and HyperFlash.

But in my laziness, I never upstreamed the RZ/A1 driver 2 year ago...so here we are :)

> I don't really know enough about the device and there was *huge* amounts
> of discussion which I'd have to try to page in so it'd be really good if
> there were some agreement among those of you working with this device as
> to what the best way forwards is.  I'm not sure any of the issues were
> at the framework level so that's probably more sensible anyway.

I'm trying to figure out if the differences are 'technical' or
'ideological' (ie, R-Car use cases vs RZ use cases).
Of course we can do anything we want in our vendor BSPs, but I'd like
to see a common upstream path.

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-06 13:41 ` [PATCH v2 1/6] spi: Add SPIBSC driver Chris Brandt
@ 2019-12-12 19:36   ` Sergei Shtylyov
  2019-12-12 20:19     ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-12 19:36 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On 12/06/2019 04:41 PM, Chris Brandt wrote:

> Add a driver for the SPIBSC controller in Renesas SoC devices.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

   Hm... I've just tested JFFS2 with this driver and got the same result as with
my own drivers:

root@192.168.2.11:~# mount -t jffs2 /dev/mtdblock11 /mnt/jffs2/                 
root@192.168.2.11:~# cd /mnt/jffs2/                                             
root@192.168.2.11:/mnt/jffs2# ls -l                                             
total 34                                                                        
-rwxr-xr-x 1 root root 18678 Jan 22  2000 evtest                                
-rw-r--r-- 1 root root 15169 Jan 22  2000 evtest.c                              
root@192.168.2.11:/mnt/jffs2# rm evtest                                         
root@192.168.2.11:/mnt/jffs2# ls -l                                             
total 15                                                                        
-rw-r--r-- 1 root root 15169 Jan 22  2000 evtest.c                              
root@192.168.2.11:/mnt/jffs2# cd                                                
root@192.168.2.11:~# umount /mnt/jffs2/                                         
root@192.168.2.11:~# mount -t jffs2 /dev/mtdblock11 /mnt/jffs2/                 
root@192.168.2.11:~# ls -l /mnt/jffs2/                                          
total 34                                                                        
-rwxr-xr-x 1 root root 18678 Jan 22  2000 evtest                                
-rw-r--r-- 1 root root 15169 Jan 22  2000 evtest.c                              

   As you can see, the deleted file is back after unmount/re-mount...

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-12 19:36   ` Sergei Shtylyov
@ 2019-12-12 20:19     ` Chris Brandt
  2019-12-13 10:01       ` Geert Uytterhoeven
  2019-12-13 18:36       ` Sergei Shtylyov
  0 siblings, 2 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-12 20:19 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Thu, Dec 12, 2019, Sergei Shtylyov wrote:
>    As you can see, the deleted file is back after unmount/re-mount...

Did you do a 'sync' before you unmounted?


With the RZ/A2M EVB:

Welcome to Buildroot
buildroot login: root
$ mount /dev/mtdblock3 -t jffs2 /mnt
$ ls -l /mnt
total 688
-rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox
-rw-r--r--    1 root     root             6 Oct 31 09:07 hello.txt
$ rm hello.txt
$ sync
$ umount /mnt
$
$
$ mount /dev/mtdblock3 -t jffs2 /mnt
$ ls -l /mnt
total 687
-rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox


Note that I also needed this patch in my tree.
https://patchwork.ozlabs.org/patch/1202314/


Chris

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-12 20:19     ` Chris Brandt
@ 2019-12-13 10:01       ` Geert Uytterhoeven
  2019-12-13 14:45         ` Chris Brandt
  2019-12-13 18:36       ` Sergei Shtylyov
  1 sibling, 1 reply; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-13 10:01 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hi Chris,

On Thu, Dec 12, 2019 at 9:19 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Thu, Dec 12, 2019, Sergei Shtylyov wrote:
> >    As you can see, the deleted file is back after unmount/re-mount...
>
> Did you do a 'sync' before you unmounted?

Does it fail without? If yes, that must be a jffs2 bug.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-13 10:01       ` Geert Uytterhoeven
@ 2019-12-13 14:45         ` Chris Brandt
  2019-12-13 14:48           ` Geert Uytterhoeven
  2019-12-13 19:37           ` Sergei Shtylyov
  0 siblings, 2 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-13 14:45 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hi Geert,

On Fri, Dec 13, 2019, Geert Uytterhoeven wrote:
> On Thu, Dec 12, 2019 at 9:19 PM Chris Brandt <Chris.Brandt@renesas.com>
> wrote:
> > On Thu, Dec 12, 2019, Sergei Shtylyov wrote:
> > >    As you can see, the deleted file is back after unmount/re-mount...
> >
> > Did you do a 'sync' before you unmounted?
> 
> Does it fail without? If yes, that must be a jffs2 bug.

It does not fail for me with or without the sync.

$ ls -l /mnt ; rm /mnt/test.txt ; umount /mnt
total 688
-rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox
-rw-r--r--    1 root     root            58 Oct 31 09:00 test.txt
$ mount
$
$ mount /dev/mtdblock3 -t jffs2 /mnt
$ ls -l /mnt
total 687
-rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox


I also tried this....and it works find as well.

$ cp /bin/busybox /mnt/busybox_2 ; umount /mnt

I just I was remembering that you need to call sync before you call reboot
or shutdown because those do not sync first. That's what I remember anyway.

I guess Sergei is using an R-Car board (I'm using RZ/A2M).

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-13 14:45         ` Chris Brandt
@ 2019-12-13 14:48           ` Geert Uytterhoeven
  2019-12-13 19:37           ` Sergei Shtylyov
  1 sibling, 0 replies; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-13 14:48 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hi Chris,

On Fri, Dec 13, 2019 at 3:45 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Fri, Dec 13, 2019, Geert Uytterhoeven wrote:
> > On Thu, Dec 12, 2019 at 9:19 PM Chris Brandt <Chris.Brandt@renesas.com>
> > wrote:
> > > On Thu, Dec 12, 2019, Sergei Shtylyov wrote:
> > > >    As you can see, the deleted file is back after unmount/re-mount...
> > >
> > > Did you do a 'sync' before you unmounted?
> >
> > Does it fail without? If yes, that must be a jffs2 bug.
>
> It does not fail for me with or without the sync.

Good.

> I just I was remembering that you need to call sync before you call reboot
> or shutdown because those do not sync first. That's what I remember anyway.

Yeah, "embedded" reboot commands may reboot immediately, without shutting
down the system cleanly.  Same for "reboot -f" on Debian.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-12 20:19     ` Chris Brandt
  2019-12-13 10:01       ` Geert Uytterhoeven
@ 2019-12-13 18:36       ` Sergei Shtylyov
  2019-12-13 19:40         ` Sergei Shtylyov
  1 sibling, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-13 18:36 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello!

On 12/12/2019 11:19 PM, Chris Brandt wrote:

>>    As you can see, the deleted file is back after unmount/re-mount...
> 
> Did you do a 'sync' before you unmounted?

   Now I have -- no change.

> With the RZ/A2M EVB:
> 
> Welcome to Buildroot
> buildroot login: root
> $ mount /dev/mtdblock3 -t jffs2 /mnt
> $ ls -l /mnt
> total 688
> -rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox
> -rw-r--r--    1 root     root             6 Oct 31 09:07 hello.txt
> $ rm hello.txt
> $ sync
> $ umount /mnt
> $
> $
> $ mount /dev/mtdblock3 -t jffs2 /mnt
> $ ls -l /mnt
> total 687
> -rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox
> 
> 
> Note that I also needed this patch in my tree.
> https://patchwork.ozlabs.org/patch/1202314/

   I should have mentioned that I was testing in Simon's renesas.git (thus 5.2-rc6),
this patch is not applicably there. I'll now try Geert's renesas-devel.git (5.5-rc1)...

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-13 14:45         ` Chris Brandt
  2019-12-13 14:48           ` Geert Uytterhoeven
@ 2019-12-13 19:37           ` Sergei Shtylyov
  1 sibling, 0 replies; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-13 19:37 UTC (permalink / raw)
  To: Chris Brandt, Geert Uytterhoeven
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang

On 12/13/2019 05:45 PM, Chris Brandt wrote:

> I guess Sergei is using an R-Car board (I'm using RZ/A2M).

   Renesas V3H Starter Kit board, to be precise.

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-13 18:36       ` Sergei Shtylyov
@ 2019-12-13 19:40         ` Sergei Shtylyov
  2019-12-13 20:43           ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-13 19:40 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On 12/13/2019 09:36 PM, Sergei Shtylyov wrote:

>>>    As you can see, the deleted file is back after unmount/re-mount...
>>
>> Did you do a 'sync' before you unmounted?
> 
>    Now I have -- no change.
> 
>> With the RZ/A2M EVB:
>>
>> Welcome to Buildroot
>> buildroot login: root
>> $ mount /dev/mtdblock3 -t jffs2 /mnt
>> $ ls -l /mnt
>> total 688
>> -rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox
>> -rw-r--r--    1 root     root             6 Oct 31 09:07 hello.txt
>> $ rm hello.txt
>> $ sync
>> $ umount /mnt
>> $
>> $
>> $ mount /dev/mtdblock3 -t jffs2 /mnt
>> $ ls -l /mnt
>> total 687
>> -rwsr-xr-x    1 root     root        703448 Oct 31 09:08 busybox
>>
>>
>> Note that I also needed this patch in my tree.
>> https://patchwork.ozlabs.org/patch/1202314/
> 
>    I should have mentioned that I was testing in Simon's renesas.git (thus 5.2-rc6),
> this patch is not applicably there. I'll now try Geert's renesas-devel.git (5.5-rc1)...

   The patch isn't applicable as well, and the behaviour is the same as in 5.2.-rc6 based
kernel --deleted file is back after remounting, sync or not...

>> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-13 19:40         ` Sergei Shtylyov
@ 2019-12-13 20:43           ` Chris Brandt
  2019-12-16 18:47             ` Sergei Shtylyov
  0 siblings, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-13 20:43 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Fri, Dec 13, 2019, Sergei Shtylyov wrote:
>    The patch isn't applicable as well, and the behaviour is the same as in
> 5.2.-rc6 based
> kernel --deleted file is back after remounting, sync or not...

Do the basic R/W operations works?

Here is the first test I do on my platforms. After I passed this, everything
else seems to worked pretty good (writing large files).

$ flash_eraseall -j /dev/mtd4
$ mount -t jffs2 /dev/mtdblock4 /mnt
$ echo "hello" > /mnt/hello.txt
$ sync


If the Flash was recognized at boot, then we know that the ID command (0x9F)
at least worked. Meaning read commands were at least working (which is the
difficult one for this HW...writing is easier)

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-12 17:25               ` Chris Brandt
@ 2019-12-16 15:21                 ` Mark Brown
  0 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2019-12-16 15:21 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Sergei Shtylyov, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang

[-- Attachment #1: Type: text/plain, Size: 942 bytes --]

On Thu, Dec 12, 2019 at 05:25:43PM +0000, Chris Brandt wrote:
> On Thu, Dec 12, 2019, Mark Brown wrote:

> > I don't really know enough about the device and there was *huge* amounts
> > of discussion which I'd have to try to page in so it'd be really good if
> > there were some agreement among those of you working with this device as
> > to what the best way forwards is.  I'm not sure any of the issues were
> > at the framework level so that's probably more sensible anyway.

> I'm trying to figure out if the differences are 'technical' or
> 'ideological' (ie, R-Car use cases vs RZ use cases).
> Of course we can do anything we want in our vendor BSPs, but I'd like
> to see a common upstream path.

That'd certainly be good to achieve.  Hopefully we can get some
agreement between everyone on what the best way forwards is.  I don't
recall any substantial framework concerns with either driver so I think
it's more a renesas question.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 1/6] spi: Add SPIBSC driver
  2019-12-13 20:43           ` Chris Brandt
@ 2019-12-16 18:47             ` Sergei Shtylyov
  0 siblings, 0 replies; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-16 18:47 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On 12/13/2019 11:43 PM, Chris Brandt wrote:

>>    The patch isn't applicable as well, and the behaviour is the same as in
>> 5.2.-rc6 based
>> kernel --deleted file is back after remounting, sync or not...
> 
> Do the basic R/W operations works?
> 
> Here is the first test I do on my platforms. After I passed this, everything
> else seems to worked pretty good (writing large files).
> 
> $ flash_eraseall -j /dev/mtd4
> $ mount -t jffs2 /dev/mtdblock4 /mnt
> $ echo "hello" > /mnt/hello.txt
> $ sync

   This works but the created file doesn't survive a remount.
 
> If the Flash was recognized at boot, then we know that the ID command (0x9F)
> at least worked. Meaning read commands were at least working (which is the
> difficult one for this HW...writing is easier)

   BTW, during boot I'm seeing with thsi driver:

spi spi0.0: setup: ignoring unsupported mode bits 800                           
spi-nor spi0.0: Failed to parse optional parameter table: ff81                  

   (The 2nd message is also seen with my drivers).

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-12 14:29       ` Chris Brandt
  2019-12-12 15:28         ` Mark Brown
@ 2019-12-16 20:31         ` Sergei Shtylyov
  2019-12-16 22:21           ` Chris Brandt
  2019-12-17 19:44           ` Sergei Shtylyov
  1 sibling, 2 replies; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-16 20:31 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello!

On 12/12/2019 05:29 PM, Chris Brandt wrote:

>>> Since QSPI, HyperFlash and OctaFlash are all 'serial' Flash
>>> technologies, I would be find with a driver name of "SBSC" ("Serial
>>> Bus Space
>>> Controller") which at least looks closer to what is in all the
>>> hardware manuals.
>>
>>    How about "Serial Flash Controller" instead?
> 
> I would like that better than "RPC". At least it describes what it is.
> RPC seems like a stupid name to me (but maybe that's just because I know
> how that name was chosen...)
> https://www.cypress.com/news/cypress-simplifies-embedded-system-design-new-low-pin-count-hyperram-memory
>  "The HyperRAM and HyperFlash solution reduces pin count by at least 28 pins, ..."
> 
> As a side note, there is another HW block in Renesas that does the same 
> thing as the SPI-BSC that they use in the MCU devices. That one they 

   MCU?

> just named "QSPI".

   I thought QSPI stands for quad SPI?

>>>>> This driver has been tested on an RZ/A1H RSK and RZ/A2M EVB.
>>>>
>>>>    In the SPI mode only, I assume?
>>>
>>> Yes. At the moment, there are only requests from users for QSPI flash
>>> access (RZ/A and RZ/G users).
>>
>>    I keep being told by the management that we need HyperFlash too. :-) In
>> our BSP development, our engineers went "same hardware, 2 drivers"
>> way (with different "compatibles" per driver)...
> 
> My plan was same HW, same "compatibles", same driver...but the driver 
> would either register a SPI controller or a Hyperflash controller.

   I don't think this is a very good idea (and where to place such a driver,
in drives/memory/?... With the separate driver files, you can only build the
needed driver, SPI or HyperFash (or both -- which would be the only choice
with your approach?).

> Note that the MMC/SDHI is the same HW but can act like 2 different peripherals.

   Hm, not sure I understand... Isn't MMC and SD driven by the same subsystem?

> We also have USB that can be either host or peripheral.

   I know...

>>>>> The testing mostly consisted of formatting an area as JFFS2 and
>>>>> doing copying of files and such.
>>>>
>>>>    Did the same (or at least tried to :-) and I must admit that
>>>> writing doesn't work with any of the front ends... I still need to get
>> this fixed.
>>
>>    The last word from our BSP people was that JFFS2 doesn't work with the
>> HyperFLash dedicated BSP driver... :-/

   The BSP driver was initially written for H3/M3 SoCs and did work there...
But on V3H this driver fails...

> Is that why this "RPC" patch series is taking so long?
> It's a fairly simple piece of hardware.

   No. I didn't really use our BSP drivers as a base, so basically had to
write the HyperFLash part from scratch and fix the issues as they appeared...

> When I first saw the series on the mailing list, my plan was to just wait
> and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.

   No. :-) It was worked on during all these months... I just finally decided
to stop, take a deep breath, and post what patches I had accumulated, without
the whole driver suite working first... I'm sorry it took so long....

> So, I thought I would at least put in my own driver for SPI flash now, 
> and then go back and add HyperFlash/OctaFlash once I get the chips 
> swapped out on one of my RZ/A2 boards.

   Ah! My HyperFlash driver is ready, just the HyperBus core needs some
fixing (I have a draft patch for that)...

>>> However, the driver I posted is pretty simple and works. Does the
>>> HyperFlash MTD
>>
>>    There's no HF library, only front end driver.
>>    The real library covers both SPI and HF. The only difference between the
>> two is the h/w setup (minor difference).
> 
> But is this "library" something specific to Renesas devices?

   Yes, it covers only RPC-IF (as described in the gen3 manual).

> That's what I'm trying to understand.
> 
> My understanding is that HyperFlash uses standard CFI commands, so all 

   The CFI command set driver needed some changes too (e.g. using the status
register to determine if a command is done).

> we need to do is register a CFI device in the driver, just like we 
> register a serial flash device.

> (I guess I could go look at the sample code for our RTOS package and find out)
> 
>>> library that you are proposing have a very different API than just
>>> 'send bytes' and 'receive bytes'?
>>
>>    There's "prepare" and "transfer" APIs and also "direct map read" API.

  The 1st one prepares the values to be written in either SPI mode or direct
read mode registers. Then you can call "transfer" or "direct mao read" which
would write out the register values into either set...

> I wonder what is the value of the "direct map read" (other than XIP in 
> RZ/A systems). If you really want to directly access the flash (no 
> buffering though the MTD layer), you need to register as a mtd-rom device, 
> and then you don't really need an API at all.

  I'd leave this question to Boris, else I never complete this msg. :-) 

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-16 20:31         ` Sergei Shtylyov
@ 2019-12-16 22:21           ` Chris Brandt
  2019-12-17 19:30             ` Sergei Shtylyov
  2019-12-17 19:44           ` Sergei Shtylyov
  1 sibling, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-16 22:21 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello

On Mon, Dec 16, 2019, Sergei Shtylyov wrote:
> > As a side note, there is another HW block in Renesas that does the same
> > thing as the SPI-BSC that they use in the MCU devices. That one they
> 
>    MCU?
Yup.
  But...it has no significance to this discussion though :)


> > When I first saw the series on the mailing list, my plan was to just wait
> > and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.
> 
>    No. :-) It was worked on during all these months... I just finally decided
> to stop, take a deep breath, and post what patches I had accumulated, without
> the whole driver suite working first... I'm sorry it took so long....

So at the moment, there is nothing yet for me to 'try' on the RZ/A series, correct?


> > My understanding is that HyperFlash uses standard CFI commands, so all
> 
>    The CFI command set driver needed some changes too (e.g. using the status
> register to determine if a command is done).

So the existing MTD-SPI layer knows how to talk to SPI devices.
And, you've fixed up the existing CFI layer to talk to HyperFlash devices.
But, you do not want these MTD layer to talk directly to a Renesas HW driver?
You want to put another software layer in between?


I'm guessing that from this statement....

> >>> library that you are proposing have a very different API than just
> >>> 'send bytes' and 'receive bytes'?
> >>
> >>    There's "prepare" and "transfer" APIs and also "direct map read" API.
> 
>   The 1st one prepares the values to be written in either SPI mode or direct
> read mode registers. Then you can call "transfer" or "direct mao read" which
> would write out the register values into either set...

...that you want more control of the data stream being passed to the RPC-IF driver.
Correct??

It all keeps sounding complicated, unless I'm just not understanding the code
you are trying to implement.


Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-16 22:21           ` Chris Brandt
@ 2019-12-17 19:30             ` Sergei Shtylyov
  2019-12-17 20:26               ` Geert Uytterhoeven
  2019-12-19 16:57               ` Chris Brandt
  0 siblings, 2 replies; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-17 19:30 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello!

On 12/17/2019 01:21 AM, Chris Brandt wrote:

>>> As a side note, there is another HW block in Renesas that does the same
>>> thing as the SPI-BSC that they use in the MCU devices. That one they
>>
>>    MCU?
> Yup.
>   But...it has no significance to this discussion though :)

   But what does the acronym mean?

>>> When I first saw the series on the mailing list, my plan was to just wait
>>> and then add RZ/A1 and RZ/A2 support. But....it looks like it all died.
>>
>>    No. :-) It was worked on during all these months... I just finally decided
>> to stop, take a deep breath, and post what patches I had accumulated, without
>> the whole driver suite working first... I'm sorry it took so long....
> 
> So at the moment, there is nothing yet for me to 'try' on the RZ/A series, correct?

   Why, I can send you a working version of the SPI driver, and even HF one if you're
interested.
 
>>> My understanding is that HyperFlash uses standard CFI commands, so all
>>
>>    The CFI command set driver needed some changes too (e.g. using the status
>> register to determine if a command is done).
> 
> So the existing MTD-SPI layer knows how to talk to SPI devices.

   SPI-NOR does it with a help of drivers/spi/spi-mem.c... As for the HyperFlash,
it needs a HyperBus driver (that I have a working patch for)...

> And, you've fixed up the existing CFI layer to talk to HyperFlash evices.

   Mostly Boris B. did it... :-)

> But, you do not want these MTD layer to talk directly to a Renesas HW driver?

   Yes, because the SPI-NOR and HyperBus have different driver APIs.

> You want to put another software layer in between?

   Yes.

> I'm guessing that from this statement....

>>>>> library that you are proposing have a very different API than just
>>>>> 'send bytes' and 'receive bytes'?
>>>>
>>>>    There's "prepare" and "transfer" APIs and also "direct map read" API.
>>
>>   The 1st one prepares the values to be written in either SPI mode or direct
>> read mode registers. Then you can call "transfer" or "direct mao read" which
>> would write out the register values into either set...
> 
> ...that you want more control of the data stream being passed to the RPC-IF driver.
> Correct??
> 
> It all keeps sounding complicated, unless I'm just not understanding the code
> you are trying to implement.

   Well, it reflects the fact that the "SPI mode" and "direct read" register sets
are largely identical. The "preparation" stage sets up the register values, the 
other stages use that data to set up the real register sets and then do the
needed transfers...

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-16 20:31         ` Sergei Shtylyov
  2019-12-16 22:21           ` Chris Brandt
@ 2019-12-17 19:44           ` Sergei Shtylyov
  2019-12-18  8:09             ` Boris Brezillon
  1 sibling, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-17 19:44 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Boris Brezillon

On 12/16/2019 11:31 PM, Sergei Shtylyov wrote:

[...]
>> My understanding is that HyperFlash uses standard CFI commands, so all 
> 
>    The CFI command set driver needed some changes too (e.g. using the status
> register to determine if a command is done).
> 
>> we need to do is register a CFI device in the driver, just like we 
>> register a serial flash device.
> 
>> (I guess I could go look at the sample code for our RTOS package and find out)
>>
>>>> library that you are proposing have a very different API than just
>>>> 'send bytes' and 'receive bytes'?
>>>
>>>    There's "prepare" and "transfer" APIs and also "direct map read" API.
> 
>   The 1st one prepares the values to be written in either SPI mode or direct
> read mode registers. Then you can call "transfer" or "direct mao read" which
> would write out the register values into either set...
> 
>> I wonder what is the value of the "direct map read" (other than XIP in 
>> RZ/A systems). If you really want to directly access the flash (no 
>> buffering though the MTD layer), you need to register as a mtd-rom device, 
>> and then you don't really need an API at all.
> 
>   I'd leave this question to Boris, else I never complete this msg. :-) 

   Didn't really summon him, doing that now... :-)

>> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-17 19:30             ` Sergei Shtylyov
@ 2019-12-17 20:26               ` Geert Uytterhoeven
  2019-12-19 16:57               ` Chris Brandt
  1 sibling, 0 replies; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-17 20:26 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hi Sergei,

On Tue, Dec 17, 2019 at 8:30 PM Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> On 12/17/2019 01:21 AM, Chris Brandt wrote:
> >>> As a side note, there is another HW block in Renesas that does the same
> >>> thing as the SPI-BSC that they use in the MCU devices. That one they
> >>
> >>    MCU?
> > Yup.
> >   But...it has no significance to this discussion though :)
>
>    But what does the acronym mean?

Микроконтро́ллер (англ. Micro Controller Unit, MCU)

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-17 19:44           ` Sergei Shtylyov
@ 2019-12-18  8:09             ` Boris Brezillon
  2019-12-19 16:32               ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Boris Brezillon @ 2019-12-18  8:09 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang,
	Boris Brezillon

On Tue, 17 Dec 2019 22:44:14 +0300
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote:

> On 12/16/2019 11:31 PM, Sergei Shtylyov wrote:
> 
> [...]
> >> My understanding is that HyperFlash uses standard CFI commands, so all   
> > 
> >    The CFI command set driver needed some changes too (e.g. using the status
> > register to determine if a command is done).
> >   
> >> we need to do is register a CFI device in the driver, just like we 
> >> register a serial flash device.  
> >   
> >> (I guess I could go look at the sample code for our RTOS package and find out)
> >>  
> >>>> library that you are proposing have a very different API than just
> >>>> 'send bytes' and 'receive bytes'?  
> >>>
> >>>    There's "prepare" and "transfer" APIs and also "direct map read" API.  
> > 
> >   The 1st one prepares the values to be written in either SPI mode or direct
> > read mode registers. Then you can call "transfer" or "direct mao read" which
> > would write out the register values into either set...
> >   
> >> I wonder what is the value of the "direct map read" (other than XIP in 
> >> RZ/A systems). If you really want to directly access the flash (no 
> >> buffering though the MTD layer), you need to register as a mtd-rom device, 
> >> and then you don't really need an API at all.  
> > 
> >   I'd leave this question to Boris, else I never complete this msg. :-)   
> 
>    Didn't really summon him, doing that now... :-)

The dirmap API has not been designed with XIP in mind (though we could
theoretically extend it to support XIP). The main reason we added this
feature is because most controllers have either a slow PIO based path
where you can basically send every command you want and a fast
direct-mapping path which only support specific cmds or patterns. We
also hide things behind an API instead of returning a virtual mapping
because some controllers have extra constraints when accessing the
direct mapping (alignment, minimum size, limited direct mapping window
size, ...). Not to mention that some drivers might want to use DMA to
not stall the CPU on flash accesses.

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-18  8:09             ` Boris Brezillon
@ 2019-12-19 16:32               ` Chris Brandt
  0 siblings, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-19 16:32 UTC (permalink / raw)
  To: Boris Brezillon, Sergei Shtylyov
  Cc: Mark Brown, Rob Herring, Mark Rutland, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-spi, devicetree,
	linux-renesas-soc, linux-clk, Mason Yang, Boris Brezillon

On Wed, Dec 18, 2019, Boris Brezillon wrote:
> The dirmap API has not been designed with XIP in mind (though we could
> theoretically extend it to support XIP). The main reason we added this
[snip]

Boris,

Thank you for explaining.

Chris

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-17 19:30             ` Sergei Shtylyov
  2019-12-17 20:26               ` Geert Uytterhoeven
@ 2019-12-19 16:57               ` Chris Brandt
  2019-12-19 19:01                 ` Sergei Shtylyov
  1 sibling, 1 reply; 51+ messages in thread
From: Chris Brandt @ 2019-12-19 16:57 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Tue, Dec 17, 2019, Sergei Shtylyov wrote:
>    But what does the acronym mean?
QSPI = "Quad SPI"


> > So at the moment, there is nothing yet for me to 'try' on the RZ/A series,
> correct?
> 
>    Why, I can send you a working version of the SPI driver, and even HF one
> if you're
> interested.

The point of this whole discussion is to determine if we should have 2 drivers
for the same Renesas HW IP.

There was a RPC-IF patch series that made it to v17....and is now dead.

You sent a new RFC series for a new method, but all it had was low level APIs,
no MTD framework, do it didn't really do anything.

If there was a complete patch set that I could try on the RZ/A SoCs and 
get a working SPI MTD device to show up, then I would drop my efforts of
getting my driver in and just add RZ/A support to the R-Car driver.

Geert suggested out an easy solution for the "XIP" use case for RZ/A 
devices, and that mostly happens outside this driver, so I'm not worried 
about that anymore.

Honestly, I'll be out of the office until January, so it's not like I'm 
going to do anything with it until then. But if there is a complete series
to try by then, I will see how it performs on RZ/A boards.

Chris


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-19 16:57               ` Chris Brandt
@ 2019-12-19 19:01                 ` Sergei Shtylyov
  2019-12-19 21:04                   ` Chris Brandt
  2019-12-20  1:45                   ` masonccyang
  0 siblings, 2 replies; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-19 19:01 UTC (permalink / raw)
  To: Chris Brandt, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hello!

On 12/19/2019 07:57 PM, Chris Brandt wrote:

>>> So at the moment, there is nothing yet for me to 'try' on the RZ/A series,
>> correct?
>>
>>    Why, I can send you a working version of the SPI driver, and even HF one
>> if you're
>> interested.
> 
> The point of this whole discussion is to determine if we should have 2 drivers
> for the same Renesas HW IP.
> 
> There was a RPC-IF patch series that made it to v17....and is now dead.
> 
> You sent a new RFC series for a new method, but all it had was low level APIs,
> no MTD framework, do it didn't really do anything.

   Apparently you have missed the previous RFC iteration, the MFD/SPI drivers posted
at end of May:

https://patchwork.kernel.org/patch/10969211/
https://patchwork.kernel.org/patch/10969213/
https://patchwork.kernel.org/patch/10969217/

   There's not yet merged MTD patch you'd need too:

http://patchwork.ozlabs.org/patch/1199645/

   The MFD driver was shot down by Lee Jones who has advised placing the common
code into drivers/memory/ instead... I don't want to re-post the SPI driver as
I haven't yet addressed all of Mark Brown's comments...

> If there was a complete patch set that I could try on the RZ/A SoCs and 
> get a working SPI MTD device to show up, then I would drop my efforts of
> getting my driver in and just add RZ/A support to the R-Car driver.

   Please try these patches, there's a big chance they'll work. 

[...]
> Honestly, I'll be out of the office until January, so it's not like I'm 
> going to do anything with it until then. But if there is a complete series
> to try by then, I will see how it performs on RZ/A boards.

   Hopefully I will have addressed Mark's feedback by then and post the new SPI
driver... Have happy holidays! (Ours happen on 1/1 and last till 1/8 this year.)

> Chris

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-19 19:01                 ` Sergei Shtylyov
@ 2019-12-19 21:04                   ` Chris Brandt
  2019-12-20  1:45                   ` masonccyang
  1 sibling, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-19 21:04 UTC (permalink / raw)
  To: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd
  Cc: linux-spi, devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Thu, Dec 19, 2019, Sergei Shtylyov wrote:
>    Apparently you have missed the previous RFC iteration, the MFD/SPI drivers
> posted
> at end of May:

OK, thanks. I'll go get them.
End of May....that was a while back.

>    The MFD driver was shot down by Lee Jones who has advised placing the
> common
> code into drivers/memory/ instead... I don't want to re-post the SPI driver
> as
> I haven't yet addressed all of Mark Brown's comments...

For now, I just want to check if functionally it works the same for RZ/A.

>    Please try these patches, there's a big chance they'll work.

If it does, that would be nice.
This HW is going to be continued to be used in new SoCs.


> Have happy holidays! (Ours happen on 1/1 and last till 1/8 this
> year.)

You too!

Chris

             *
            /.\
           /..'\
           /'.'\
          /.''.'\
          /.'.'.\
         /'.''.'.\
         ^^^[_]^^^



^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-19 19:01                 ` Sergei Shtylyov
  2019-12-19 21:04                   ` Chris Brandt
@ 2019-12-20  1:45                   ` masonccyang
  2019-12-20  7:55                     ` Geert Uytterhoeven
  2019-12-24 16:58                     ` Sergei Shtylyov
  1 sibling, 2 replies; 51+ messages in thread
From: masonccyang @ 2019-12-20  1:45 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Mark Brown, Chris Brandt, devicetree, Geert Uytterhoeven,
	linux-clk, linux-renesas-soc, linux-spi, Mark Rutland,
	Michael Turquette, Rob Herring, Stephen Boyd


Hello,
 
> On 12/19/2019 07:57 PM, Chris Brandt wrote:
> 
> >>> So at the moment, there is nothing yet for me to 'try' on the RZ/A 
series,
> >> correct?
> >>
> >>    Why, I can send you a working version of the SPI driver, and even 
HF one
> >> if you're
> >> interested.
> > 
> > The point of this whole discussion is to determine if we should have 2 
drivers
> > for the same Renesas HW IP.
> > 
> > There was a RPC-IF patch series that made it to v17....and is now 
dead.


It's under review by Geert Uytterhoeven

https://patchwork.kernel.org/project/linux-renesas-soc/list/?submitter=181859 


https://patchwork.kernel.org/patch/11078131/ 
https://patchwork.kernel.org/patch/11078133/ 


thanks & best regards,
Mason

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information 
and/or personal data, which is protected by applicable laws. Please be 
reminded that duplication, disclosure, distribution, or use of this e-mail 
(and/or its attachments) or any part thereof is prohibited. If you receive 
this e-mail in error, please notify us immediately and delete this mail as 
well as its attachment(s) from your system. In addition, please be 
informed that collection, processing, and/or use of personal data is 
prohibited unless expressly permitted by personal data protection laws. 
Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================



============================================================================

CONFIDENTIALITY NOTE:

This e-mail and any attachments may contain confidential information and/or personal data, which is protected by applicable laws. Please be reminded that duplication, disclosure, distribution, or use of this e-mail (and/or its attachments) or any part thereof is prohibited. If you receive this e-mail in error, please notify us immediately and delete this mail as well as its attachment(s) from your system. In addition, please be informed that collection, processing, and/or use of personal data is prohibited unless expressly permitted by personal data protection laws. Thank you for your attention and cooperation.

Macronix International Co., Ltd.

=====================================================================


^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-20  1:45                   ` masonccyang
@ 2019-12-20  7:55                     ` Geert Uytterhoeven
  2019-12-24 16:58                     ` Sergei Shtylyov
  1 sibling, 0 replies; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-20  7:55 UTC (permalink / raw)
  To: Mason Yang
  Cc: Sergei Shtylyov, Mark Brown, Chris Brandt, devicetree,
	Geert Uytterhoeven, linux-clk, linux-renesas-soc, linux-spi,
	Mark Rutland, Michael Turquette, Rob Herring, Stephen Boyd

Hi Mason,

On Fri, Dec 20, 2019 at 2:45 AM <masonccyang@mxic.com.tw> wrote:
> > On 12/19/2019 07:57 PM, Chris Brandt wrote:
> > >>> So at the moment, there is nothing yet for me to 'try' on the RZ/A
> series,
> > >> correct?
> > >>
> > >>    Why, I can send you a working version of the SPI driver, and even
> HF one
> > >> if you're
> > >> interested.
> > >
> > > The point of this whole discussion is to determine if we should have 2
> drivers
> > > for the same Renesas HW IP.
> > >
> > > There was a RPC-IF patch series that made it to v17....and is now
> dead.
>
> It's under review by Geert Uytterhoeven
>
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?submitter=181859
>
>
> https://patchwork.kernel.org/patch/11078131/
> https://patchwork.kernel.org/patch/11078133/

It's marked "Under Review" in patchwork, as there haven't been any comments
on v17 yet.

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock
  2019-12-06 19:49     ` Chris Brandt
@ 2019-12-20 14:38       ` Geert Uytterhoeven
  2019-12-20 14:50         ` Chris Brandt
  0 siblings, 1 reply; 51+ messages in thread
From: Geert Uytterhoeven @ 2019-12-20 14:38 UTC (permalink / raw)
  To: Chris Brandt
  Cc: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang

On Fri, Dec 6, 2019 at 8:49 PM Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Fri, Dec 6, 2019, Sergei Shtylyov wrote:
> > On 12/06/2019 04:41 PM, Chris Brandt wrote:
> >
> > > The SPIBSC clocks are marked as critical because for XIP systems, the
> > > kernel will be running from QSPI flash and cannot be turned off.
> > >
> > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> > > ---
> > > v2:
> > >  * Removed spibsc from critical clock section
> >
> >    So you've removed it from the critical table but left the patch
> > description intact?
>
> Damn!
> Thank you for pointing that out.
> I remembered to take the comment out of one patch, but I missed this one.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in clk-renesas-for-v5.6, with the description changed to
"Add SPIBSC clock for RZ/A2.".

Gr{oetje,eeting}s,

                        Geert

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

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 51+ messages in thread

* RE: [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock
  2019-12-20 14:38       ` Geert Uytterhoeven
@ 2019-12-20 14:50         ` Chris Brandt
  0 siblings, 0 replies; 51+ messages in thread
From: Chris Brandt @ 2019-12-20 14:50 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Sergei Shtylyov, Mark Brown, Rob Herring, Mark Rutland,
	Geert Uytterhoeven, Michael Turquette, Stephen Boyd, linux-spi,
	devicetree, linux-renesas-soc, linux-clk, Mason Yang

Hi Geert,

On Fri, Dec 20, 2019, Geert Uytterhoeven wrote:
> i.e. will queue in clk-renesas-for-v5.6, with the description changed to
> "Add SPIBSC clock for RZ/A2.".

Thank you!

Chris

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-20  1:45                   ` masonccyang
  2019-12-20  7:55                     ` Geert Uytterhoeven
@ 2019-12-24 16:58                     ` Sergei Shtylyov
  2019-12-27  0:58                       ` Mark Brown
  1 sibling, 1 reply; 51+ messages in thread
From: Sergei Shtylyov @ 2019-12-24 16:58 UTC (permalink / raw)
  To: masonccyang
  Cc: Mark Brown, Chris Brandt, devicetree, Geert Uytterhoeven,
	linux-clk, linux-renesas-soc, linux-spi, Mark Rutland,
	Michael Turquette, Rob Herring, Stephen Boyd

Hello!

On 12/20/2019 04:45 AM, masonccyang@mxic.com.tw wrote:

>>>>> So at the moment, there is nothing yet for me to 'try' on the RZ/A series,
>>>> correct?
>>>>
>>>>    Why, I can send you a working version of the SPI driver, and even HF one
>>>> if you're
>>>> interested.
>>>
>>> The point of this whole discussion is to determine if we should have 2 drivers
>>> for the same Renesas HW IP.
>>>
>>> There was a RPC-IF patch series that made it to v17....and is now dead.
> 
> It's under review by Geert Uytterhoeven
> 
> https://patchwork.kernel.org/project/linux-renesas-soc/list/?submitter=181859 
> https://patchwork.kernel.org/patch/11078131/ 
> https://patchwork.kernel.org/patch/11078133/

https://patchwork.kernel.org/patch/11078137/
https://patchwork.kernel.org/patch/11078139/ 

   It doesn't matter much what's in the renesas-soc patchwork, the patch would
be merged thru the linux-spi repo, and in their patchwork the status of your v17
patches is "New, archived"...

> thanks & best regards,
> Mason

MBR, Sergei

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v2 0/6] spi: Add Renesas SPIBSC controller
  2019-12-24 16:58                     ` Sergei Shtylyov
@ 2019-12-27  0:58                       ` Mark Brown
  0 siblings, 0 replies; 51+ messages in thread
From: Mark Brown @ 2019-12-27  0:58 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: masonccyang, Chris Brandt, devicetree, Geert Uytterhoeven,
	linux-clk, linux-renesas-soc, linux-spi, Mark Rutland,
	Michael Turquette, Rob Herring, Stephen Boyd

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Tue, Dec 24, 2019 at 07:58:21PM +0300, Sergei Shtylyov wrote:

>    It doesn't matter much what's in the renesas-soc patchwork, the patch would
> be merged thru the linux-spi repo, and in their patchwork the status of your v17
> patches is "New, archived"...

To be fair patchwork isn't exactly used in the review process for
SPI like it is with some other subsystems so other than automated
updates when things get applied or superceeded the states don't
mean a huge amount.  Like I said elsewhere in the thread I'd been
expecting some review from other people working with this
hardware since up until that point it had been pretty active.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 51+ messages in thread

end of thread, other threads:[~2019-12-27  0:59 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-06 13:41 [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Chris Brandt
2019-12-06 13:41 ` [PATCH v2 1/6] spi: Add SPIBSC driver Chris Brandt
2019-12-12 19:36   ` Sergei Shtylyov
2019-12-12 20:19     ` Chris Brandt
2019-12-13 10:01       ` Geert Uytterhoeven
2019-12-13 14:45         ` Chris Brandt
2019-12-13 14:48           ` Geert Uytterhoeven
2019-12-13 19:37           ` Sergei Shtylyov
2019-12-13 18:36       ` Sergei Shtylyov
2019-12-13 19:40         ` Sergei Shtylyov
2019-12-13 20:43           ` Chris Brandt
2019-12-16 18:47             ` Sergei Shtylyov
2019-12-06 13:41 ` [PATCH v2 2/6] dt-bindings: spi: Document Renesas SPIBSC bindings Chris Brandt
2019-12-09 14:09   ` Geert Uytterhoeven
2019-12-09 15:45     ` Chris Brandt
2019-12-09 19:34       ` Geert Uytterhoeven
2019-12-10 20:07     ` Sergei Shtylyov
2019-12-10 20:17       ` Geert Uytterhoeven
2019-12-10 20:33         ` Chris Brandt
2019-12-10 20:23       ` Chris Brandt
2019-12-06 13:41 ` [PATCH v2 3/6] clk: renesas: r7s9210: Add SPIBSC clock Chris Brandt
2019-12-06 18:40   ` Sergei Shtylyov
2019-12-06 19:49     ` Chris Brandt
2019-12-20 14:38       ` Geert Uytterhoeven
2019-12-20 14:50         ` Chris Brandt
2019-12-06 13:42 ` [PATCH v2 4/6] ARM: dts: r7s72100: Add SPIBSC devices Chris Brandt
2019-12-06 13:42 ` [PATCH v2 5/6] ARM: dts: r7s9210: Add SPIBSC device Chris Brandt
2019-12-06 13:42 ` [PATCH v2 6/6] ARM: dts: gr-peach: Enable SPIBSC Chris Brandt
2019-12-07 20:28 ` [PATCH v2 0/6] spi: Add Renesas SPIBSC controller Sergei Shtylyov
2019-12-09 15:10   ` Chris Brandt
2019-12-11 19:09     ` Sergei Shtylyov
2019-12-12 14:29       ` Chris Brandt
2019-12-12 15:28         ` Mark Brown
2019-12-12 16:53           ` Chris Brandt
2019-12-12 17:13             ` Mark Brown
2019-12-12 17:25               ` Chris Brandt
2019-12-16 15:21                 ` Mark Brown
2019-12-16 20:31         ` Sergei Shtylyov
2019-12-16 22:21           ` Chris Brandt
2019-12-17 19:30             ` Sergei Shtylyov
2019-12-17 20:26               ` Geert Uytterhoeven
2019-12-19 16:57               ` Chris Brandt
2019-12-19 19:01                 ` Sergei Shtylyov
2019-12-19 21:04                   ` Chris Brandt
2019-12-20  1:45                   ` masonccyang
2019-12-20  7:55                     ` Geert Uytterhoeven
2019-12-24 16:58                     ` Sergei Shtylyov
2019-12-27  0:58                       ` Mark Brown
2019-12-17 19:44           ` Sergei Shtylyov
2019-12-18  8:09             ` Boris Brezillon
2019-12-19 16:32               ` Chris Brandt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).