[PATCH 0/3] Realtek SPI-NAND controller

[PATCH 0/3] Realtek SPI-NAND controller

[Chris Packham]

This series adds support for the SPI-NAND flash controller on the RTL9300
family of SoCs.

There are 2 physical chip selects which are called SPI_MST_CS0 and SPI_MST_CS1
in the datasheet. Via some pin-strapping these can be assigned to either the
SPI-NOR controller or the SPI-NAND controller. Which means you can end up with
the following permutations

  SPI-Flash
  Boot Model SPI_MST_CS0 SPI_MST_CS1
  ---------- ----------- -----------
  NOR x1     NOR-CS0     X
  NOR x2     NOR-CS0     NOR-CS1
  NAND x1    NAND-CS0    X
  NAND x2    NAND-CS0    NAND-CS1
  NOR+NAND   NOR-CS0     NAND-CS0

Chris Packham (3):
  dt-bindings: spi: Add realtek,rtl9300-snand
  mips: dts: realtek: Add SPI NAND controller
  spi: spi-mem: Add Realtek SPI-NAND controller

 .../bindings/spi/realtek,rtl9300-snand.yaml   |  58 +++
 MAINTAINERS                                   |   6 +
 arch/mips/boot/dts/realtek/rtl930x.dtsi       |  12 +
 drivers/spi/Kconfig                           |  11 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-realtek-rtl-snand.c           | 408 ++++++++++++++++++
 6 files changed, 496 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
 create mode 100644 drivers/spi/spi-realtek-rtl-snand.c

-- 
2.46.2

[PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Chris Packham]

Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
controller supports
 * Serial/Dual/Quad data with
 * PIO and DMA data read/write operation
 * Configurable flash access timing

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 +++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml

diff --git a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
new file mode 100644
index 000000000000..c66aea24cb35
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
@@ -0,0 +1,58 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
+
+maintainers:
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+description:
+  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It supports
+  typical SPI-NAND page cache operations in single, dual or quad IO mode.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - realtek,rtl9301-snand
+          - realtek,rtl9302b-snand
+          - realtek,rtl9302c-snand
+          - realtek,rtl9303-snand
+      - const: realtek,rtl9300-snand
+
+  reg:
+    items:
+      - description: SPI NAND controller registers address and size
+
+  interrupts:
+    items:
+      - description: SPI NAND controller interrupt
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+allOf:
+  - $ref: /schemas/spi/spi-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi@1a400 {
+      compatible = "realtek,rtl9302c-snand", "realtek,rtl9300-snand";
+      reg = <0x1a400 0x44>;
+      interrupt-parent = <&intc>;
+      interrupts = <19>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      flash@0 {
+        compatible = "spi-nand";
+        reg = <0>;
+      };
+    };
-- 
2.46.2

[PATCH 2/3] mips: dts: realtek: Add SPI NAND controller

[Chris Packham]

Add the SPI-NAND controller on the RTL9300 family of devices. This
supports serial/dual/quad data width and DMA for read/program
operations.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 arch/mips/boot/dts/realtek/rtl930x.dtsi | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/mips/boot/dts/realtek/rtl930x.dtsi b/arch/mips/boot/dts/realtek/rtl930x.dtsi
index f271940f82be..aab1823139e1 100644
--- a/arch/mips/boot/dts/realtek/rtl930x.dtsi
+++ b/arch/mips/boot/dts/realtek/rtl930x.dtsi
@@ -32,6 +32,8 @@ lx_clk: clock-175mhz {
 };
 
 &soc {
+	ranges = <0x0 0x18000000 0x20000>;
+
 	intc: interrupt-controller@3000 {
 		compatible = "realtek,rtl9300-intc", "realtek,rtl-intc";
 		reg = <0x3000 0x18>, <0x3018 0x18>;
@@ -59,6 +61,16 @@ timer0: timer@3200 {
 		interrupts = <7>, <8>, <9>, <10>, <11>;
 		clocks = <&lx_clk>;
 	};
+
+	snand: spi@1a400 {
+		compatible = "realtek,rtl9302c-snand", "realtek,rtl9300-snand";
+		reg = <0x1a400 0x44>;
+		interrupt-parent = <&intc>;
+		interrupts = <19>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		status = "disabled";
+	};
 };
 
 &uart0 {
-- 
2.46.2

[PATCH 3/3] spi: spi-mem: Add Realtek SPI-NAND controller

[Chris Packham]

Add a driver for the SPI-NAND controller on the RTL9300 family of
devices.

The controller supports
* Serial/Dual/Quad data with
* PIO and DMA data read/write operation
* Configurable flash access timing

There is a separate ECC controller on the RTL9300 which isn't currently
supported (instead we rely on the on-die ECC supported by most SPI-NAND
chips).

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 MAINTAINERS                         |   6 +
 drivers/spi/Kconfig                 |  11 +
 drivers/spi/Makefile                |   1 +
 drivers/spi/spi-realtek-rtl-snand.c | 408 ++++++++++++++++++++++++++++
 4 files changed, 426 insertions(+)
 create mode 100644 drivers/spi/spi-realtek-rtl-snand.c

diff --git a/MAINTAINERS b/MAINTAINERS
index f328373463b0..881afb224c50 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19272,6 +19272,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/net/dsa/realtek.yaml
 F:	drivers/net/dsa/realtek/*
 
+REALTEK SPI-NAND
+M:	Chris Pacham <chris.packham@alliedtelesis.co.nz>
+S:	Maintained
+F:	Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
+F:	drivers/spi/spi-realtek-rtl-snand.c
+
 REALTEK WIRELESS DRIVER (rtlwifi family)
 M:	Ping-Ke Shih <pkshih@realtek.com>
 L:	linux-wireless@vger.kernel.org
diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index ec1550c698d5..33228a607c4b 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -843,6 +843,17 @@ config SPI_PXA2XX
 config SPI_PXA2XX_PCI
 	def_tristate SPI_PXA2XX && PCI && COMMON_CLK
 
+config SPI_REALTEK_SNAND
+	tristate "Realtek SPI-NAND Flash Controller"
+	depends on MACH_REALTEK_RTL || COMPILE_TEST
+	select REGMAP
+	help
+	  This enables support for the SPI-NAND Flash controller on
+	  Realtek SoCs.
+
+	  This driver does not support generic SPI. The implementation
+	  only supports the spi-mem interface.
+
 config SPI_ROCKCHIP
 	tristate "Rockchip SPI controller driver"
 	depends on ARCH_ROCKCHIP || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index a9b1bc259b68..9a3338236645 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -119,6 +119,7 @@ obj-$(CONFIG_SPI_ROCKCHIP)		+= spi-rockchip.o
 obj-$(CONFIG_SPI_ROCKCHIP_SFC)		+= spi-rockchip-sfc.o
 obj-$(CONFIG_SPI_RB4XX)			+= spi-rb4xx.o
 obj-$(CONFIG_MACH_REALTEK_RTL)		+= spi-realtek-rtl.o
+obj-$(CONFIG_SPI_REALTEK_SNAND)		+= spi-realtek-rtl-snand.o
 obj-$(CONFIG_SPI_RPCIF)			+= spi-rpc-if.o
 obj-$(CONFIG_SPI_RSPI)			+= spi-rspi.o
 obj-$(CONFIG_SPI_RZV2M_CSI)		+= spi-rzv2m-csi.o
diff --git a/drivers/spi/spi-realtek-rtl-snand.c b/drivers/spi/spi-realtek-rtl-snand.c
new file mode 100644
index 000000000000..759991613d95
--- /dev/null
+++ b/drivers/spi/spi-realtek-rtl-snand.c
@@ -0,0 +1,408 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <linux/completion.h>
+#include <linux/dma-mapping.h>
+#include <linux/interrupt.h>
+#include <linux/mod_devicetable.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+
+#define SNAFCFR 0x00
+#define   SNAFCFR_DMA_IE BIT(20)
+#define SNAFCCR 0x04
+#define SNAFWCMR 0x08
+#define SNAFRCMR 0x0c
+#define SNAFRDR 0x10
+#define SNAFWDR 0x14
+#define SNAFDTR 0x18
+#define SNAFDRSAR 0x1c
+#define SNAFDIR 0x20
+#define   SNAFDIR_DMA_IP BIT(0)
+#define SNAFDLR 0x24
+#define SNAFSR 0x40
+#define  SNAFSR_NFCOS BIT(3)
+#define  SNAFSR_NFDRS BIT(2)
+#define  SNAFSR_NFDWS BIT(1)
+
+#define CMR_LEN(len) ((len) - 1)
+#define CMR_WID(width) (((width) >> 1) << 28)
+
+struct rtl_snand {
+	struct device *dev;
+	struct regmap *regmap;
+	struct completion comp;
+};
+
+static irqreturn_t rtl_snand_irq(int irq, void *data)
+{
+	struct rtl_snand *snand = data;
+	u32 val = 0;
+
+	regmap_read(snand->regmap, SNAFSR, &val);
+	if (val & (SNAFSR_NFCOS | SNAFSR_NFDRS | SNAFSR_NFDWS))
+		return IRQ_NONE;
+
+	regmap_write(snand->regmap, SNAFDIR, SNAFDIR_DMA_IP);
+	complete(&snand->comp);
+
+	return IRQ_HANDLED;
+}
+
+static bool rtl_snand_supports_op(struct spi_mem *mem,
+				  const struct spi_mem_op *op)
+{
+	if (!spi_mem_default_supports_op(mem, op))
+		return false;
+	if (op->cmd.nbytes != 1 || op->cmd.buswidth != 1)
+		return false;
+	return true;
+}
+
+static int rtl_snand_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
+{
+	return 0;
+}
+
+static void rtl_snand_set_cs(struct rtl_snand *snand, int cs, bool active)
+{
+	u32 val;
+
+	if (active)
+		val = ~(1 << (4 * cs));
+	else
+		val = ~0;
+
+	regmap_write(snand->regmap, SNAFCCR, val);
+}
+
+static int rtl_snand_wait_ready(struct rtl_snand *snand)
+{
+	u32 val;
+
+	return regmap_read_poll_timeout(snand->regmap, SNAFSR, val, !(val & SNAFSR_NFCOS),
+					0, 2 * USEC_PER_MSEC);
+}
+
+static int rtl_snand_xfer_head(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
+{
+	int ret;
+	u32 val, len = 0;
+
+	rtl_snand_set_cs(snand, cs, true);
+
+	val = op->cmd.opcode << 24;
+	len = 1;
+	if (op->addr.nbytes && op->addr.buswidth == 1) {
+		val |= op->addr.val << ((3 - op->addr.nbytes) * 8);
+		len += op->addr.nbytes;
+	}
+
+	ret = rtl_snand_wait_ready(snand);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(snand->regmap, SNAFWCMR, CMR_LEN(len));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(snand->regmap, SNAFWDR, val);
+	if (ret)
+		return ret;
+
+	ret = rtl_snand_wait_ready(snand);
+	if (ret)
+		return ret;
+
+	if (op->addr.buswidth > 1) {
+		val = op->addr.val << ((3 - op->addr.nbytes) * 8);
+		len = op->addr.nbytes;
+
+		ret = regmap_write(snand->regmap, SNAFWCMR,
+				   CMR_WID(op->addr.buswidth) | CMR_LEN(len));
+		if (ret)
+			return ret;
+
+		ret = regmap_write(snand->regmap, SNAFWDR, val);
+		if (ret)
+			return ret;
+
+		ret = rtl_snand_wait_ready(snand);
+		if (ret)
+			return ret;
+	}
+
+	if (op->dummy.nbytes) {
+		val = 0;
+
+		ret = regmap_write(snand->regmap, SNAFWCMR,
+				   CMR_WID(op->dummy.buswidth) | CMR_LEN(op->dummy.nbytes));
+		if (ret)
+			return ret;
+
+		ret = regmap_write(snand->regmap, SNAFWDR, val);
+		if (ret)
+			return ret;
+
+		ret = rtl_snand_wait_ready(snand);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static void rtl_snand_xfer_tail(struct rtl_snand *snand, int cs)
+{
+	rtl_snand_set_cs(snand, cs, false);
+}
+
+static int rtl_snand_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
+{
+	unsigned int pos, nbytes;
+	int ret;
+	u32 val, len = 0;
+
+	ret = rtl_snand_xfer_head(snand, cs, op);
+	if (ret)
+		goto out_deselect;
+
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		pos = 0;
+		len = op->data.nbytes;
+
+		while (pos < len) {
+			nbytes = len - pos;
+			if (nbytes > 4)
+				nbytes = 4;
+
+			ret = rtl_snand_wait_ready(snand);
+			if (ret)
+				goto out_deselect;
+
+			ret = regmap_write(snand->regmap, SNAFRCMR,
+					   CMR_WID(op->data.buswidth) | CMR_LEN(nbytes));
+			if (ret)
+				goto out_deselect;
+
+			ret = rtl_snand_wait_ready(snand);
+			if (ret)
+				goto out_deselect;
+
+			ret = regmap_read(snand->regmap, SNAFRDR, &val);
+			if (ret)
+				goto out_deselect;
+
+			memcpy(op->data.buf.in + pos, &val, nbytes);
+
+			pos += nbytes;
+		}
+	} else if (op->data.dir == SPI_MEM_DATA_OUT) {
+		pos = 0;
+		len = op->data.nbytes;
+
+		while (pos < len) {
+			nbytes = len - pos;
+			if (nbytes > 4)
+				nbytes = 4;
+
+			memcpy(&val, op->data.buf.out + pos, nbytes);
+
+			pos += nbytes;
+
+			ret = regmap_write(snand->regmap, SNAFWCMR, CMR_LEN(nbytes));
+			if (ret)
+				goto out_deselect;
+
+			ret = regmap_write(snand->regmap, SNAFWDR, val);
+			if (ret)
+				goto out_deselect;
+
+			ret = rtl_snand_wait_ready(snand);
+			if (ret)
+				goto out_deselect;
+		}
+	}
+
+out_deselect:
+	rtl_snand_xfer_tail(snand, cs);
+
+	if (ret)
+		dev_err(snand->dev, "transfer failed %d\n", ret);
+
+	return ret;
+}
+
+static int rtl_snand_dma_xfer(struct rtl_snand *snand, int cs, const struct spi_mem_op *op)
+{
+	int ret;
+	dma_addr_t buf_dma;
+	enum dma_data_direction dir;
+	u32 trig;
+
+	ret = rtl_snand_xfer_head(snand, cs, op);
+	if (ret)
+		goto out_deselect;
+
+	if (op->data.dir == SPI_MEM_DATA_IN) {
+		dir = DMA_FROM_DEVICE;
+		trig = 0;
+	} else if (op->data.dir == SPI_MEM_DATA_OUT) {
+		dir = DMA_TO_DEVICE;
+		trig = 1;
+	} else {
+		ret = -EOPNOTSUPP;
+		goto out_deselect;
+	}
+
+	buf_dma = dma_map_single(snand->dev, op->data.buf.in, op->data.nbytes, dir);
+	ret = dma_mapping_error(snand->dev, buf_dma);
+	if (ret)
+		goto out_deselect;
+
+	ret = regmap_write(snand->regmap, SNAFDIR, SNAFDIR_DMA_IP);
+	if (ret)
+		goto out_unmap;
+
+	ret = regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, SNAFCFR_DMA_IE);
+	if (ret)
+		goto out_unmap;
+
+	reinit_completion(&snand->comp);
+
+	ret = regmap_write(snand->regmap, SNAFDRSAR, buf_dma);
+	if (ret)
+		goto out_disable_int;
+
+	ret = regmap_write(snand->regmap, SNAFDLR,
+			   CMR_WID(op->data.buswidth) | (op->data.nbytes & 0xffff));
+	if (ret)
+		goto out_disable_int;
+
+	ret = regmap_write(snand->regmap, SNAFDTR, trig);
+	if (ret)
+		goto out_disable_int;
+
+	if (!wait_for_completion_timeout(&snand->comp, usecs_to_jiffies(20000)))
+		ret = -ETIMEDOUT;
+
+	if (ret)
+		goto out_disable_int;
+
+out_disable_int:
+	regmap_update_bits(snand->regmap, SNAFCFR, SNAFCFR_DMA_IE, 0);
+out_unmap:
+	dma_unmap_single(snand->dev, buf_dma, op->data.nbytes, dir);
+out_deselect:
+	rtl_snand_xfer_tail(snand, cs);
+
+	if (ret)
+		dev_err(snand->dev, "transfer failed %d\n", ret);
+
+	return ret;
+}
+
+static bool rtl_snand_dma_op(const struct spi_mem_op *op)
+{
+	switch (op->data.dir) {
+	case SPI_MEM_DATA_IN:
+	case SPI_MEM_DATA_OUT:
+		return op->data.nbytes > 32;
+	default:
+		return false;
+	}
+}
+
+static int rtl_snand_exec_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+	struct rtl_snand *snand = spi_controller_get_devdata(mem->spi->controller);
+	int cs = spi_get_chipselect(mem->spi, 0);
+
+	dev_dbg(snand->dev, "cs %d op cmd %02x %d:%d, dummy %d:%d, addr %08llx@%d:%d, data %d:%d\n",
+		cs, op->cmd.opcode,
+		op->cmd.buswidth, op->cmd.nbytes, op->dummy.buswidth,
+		op->dummy.nbytes, op->addr.val, op->addr.buswidth,
+		op->addr.nbytes, op->data.buswidth, op->data.nbytes);
+
+	if (rtl_snand_dma_op(op))
+		return rtl_snand_dma_xfer(snand, cs, op);
+	else
+		return rtl_snand_xfer(snand, cs, op);
+}
+
+static const struct spi_controller_mem_ops rtl_snand_mem_ops = {
+	.adjust_op_size = rtl_snand_adjust_op_size,
+	.supports_op = rtl_snand_supports_op,
+	.exec_op = rtl_snand_exec_op,
+};
+
+static const struct of_device_id rtl_snand_match[] = {
+	{ .compatible = "realtek,rtl9300-snand" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, rtl_snand_match);
+
+static int rtl_snand_probe(struct platform_device *pdev)
+{
+	struct rtl_snand *snand;
+	struct device *dev = &pdev->dev;
+	struct spi_controller *ctrl;
+	void __iomem *base;
+	const struct regmap_config rc = {
+		.reg_bits	= 32,
+		.val_bits	= 32,
+		.reg_stride	= 4,
+		.cache_type	= REGCACHE_NONE,
+	};
+	int irq, ret;
+
+	ctrl = devm_spi_alloc_host(dev, sizeof(*snand));
+	if (!ctrl)
+		return -ENOMEM;
+
+	snand = spi_controller_get_devdata(ctrl);
+	snand->dev = dev;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	snand->regmap = devm_regmap_init_mmio(dev, base, &rc);
+	if (IS_ERR(snand->regmap))
+		return PTR_ERR(snand->regmap);
+
+	init_completion(&snand->comp);
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	ret = dma_set_mask(snand->dev, DMA_BIT_MASK(32));
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set DMA mask\n");
+
+	ret = devm_request_irq(dev, irq, rtl_snand_irq, 0, "rtl-snand", snand);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to request irq\n");
+
+	ctrl->num_chipselect = 2;
+	ctrl->mem_ops = &rtl_snand_mem_ops;
+	ctrl->bits_per_word_mask = SPI_BPW_MASK(8);
+	ctrl->mode_bits = SPI_RX_DUAL | SPI_RX_QUAD | SPI_TX_DUAL | SPI_TX_QUAD;
+	device_set_node(&ctrl->dev, dev_fwnode(dev));
+
+	return devm_spi_register_controller(dev, ctrl);
+}
+
+static struct platform_driver rtl_snand_driver = {
+	.driver = {
+		.name = "realtek-rtl-snand",
+		.of_match_table = rtl_snand_match,
+	},
+	.probe = rtl_snand_probe,
+};
+module_platform_driver(rtl_snand_driver);
+
+MODULE_DESCRIPTION("Realtek SPI-NAND Flash Controller Driver");
+MODULE_LICENSE("GPL");
-- 
2.46.2

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Krzysztof Kozlowski]

On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
> controller supports
>  * Serial/Dual/Quad data with
>  * PIO and DMA data read/write operation
>  * Configurable flash access timing
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
> new file mode 100644
> index 000000000000..c66aea24cb35
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
> +
> +maintainers:
> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
> +
> +description:
> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It supports
> +  typical SPI-NAND page cache operations in single, dual or quad IO mode.
> +
> +properties:
> +  compatible:
> +    items:

Why 9300 cannot be alone? What does 9300 mean even? Wildcards and family
models are not allowed in general.

> +      - enum:
> +          - realtek,rtl9301-snand
> +          - realtek,rtl9302b-snand
> +          - realtek,rtl9302c-snand
> +          - realtek,rtl9303-snand
> +      - const: realtek,rtl9300-snand
> +
> +  reg:
> +    items:
> +      - description: SPI NAND controller registers address and size

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Krzysztof Kozlowski]

On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
> controller supports
>  * Serial/Dual/Quad data with
>  * PIO and DMA data read/write operation
>  * Configurable flash access timing
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 +++++++++++++++++++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
> new file mode 100644
> index 000000000000..c66aea24cb35
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
> @@ -0,0 +1,58 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
> +
> +maintainers:
> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
> +
> +description:
> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It supports
> +  typical SPI-NAND page cache operations in single, dual or quad IO mode.
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - realtek,rtl9301-snand
> +          - realtek,rtl9302b-snand
> +          - realtek,rtl9302c-snand
> +          - realtek,rtl9303-snand
> +      - const: realtek,rtl9300-snand
> +
> +  reg:
> +    items:
> +      - description: SPI NAND controller registers address and size
> +

Also: why no clocks? Binding is supposed to be complete. If it cannot,
you should explain it in the commit msg.

Best regards,
Krzysztof

Re: [PATCH 3/3] spi: spi-mem: Add Realtek SPI-NAND controller

[Mark Brown]

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

On Mon, Oct 07, 2024 at 12:33:47PM +1300, Chris Packham wrote:

> +REALTEK SPI-NAND
> +M:	Chris Pacham <chris.packham@alliedtelesis.co.nz>

Pacham?

> +static int rtl_snand_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> +	return 0;
> +}
> +

If the framework doesn't already support this callback being missing we
should probably make it so.

Otherwise this looks good.

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

Re: [PATCH 3/3] spi: spi-mem: Add Realtek SPI-NAND controller

[Chris Packham]

On 8/10/24 03:42, Mark Brown wrote:
> On Mon, Oct 07, 2024 at 12:33:47PM +1300, Chris Packham wrote:
>
>> +REALTEK SPI-NAND
>> +M:	Chris Pacham <chris.packham@alliedtelesis.co.nz>
> Pacham?
Darn, that'll teach me for hurriedly going through the checkpatch warnings.
>
>> +static int rtl_snand_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
>> +{
>> +	return 0;
>> +}
>> +
> If the framework doesn't already support this callback being missing we
> should probably make it so.
The framework handles this being missing. This is just left over from 
the initial stub I created when I started. Will remove in v2.
>
> Otherwise this looks good.

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Chris Packham]

On 7/10/24 19:40, Krzysztof Kozlowski wrote:
> On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
>> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
>> controller supports
>>   * Serial/Dual/Quad data with
>>   * PIO and DMA data read/write operation
>>   * Configurable flash access timing
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 +++++++++++++++++++
>>   1 file changed, 58 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>> new file mode 100644
>> index 000000000000..c66aea24cb35
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>> @@ -0,0 +1,58 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
>> +
>> +maintainers:
>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>> +
>> +description:
>> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It supports
>> +  typical SPI-NAND page cache operations in single, dual or quad IO mode.
>> +
>> +properties:
>> +  compatible:
>> +    items:
> Why 9300 cannot be alone? What does 9300 mean even? Wildcards and family
> models are not allowed in general.

The main thing about the RTL9300 is that that is what all the Realtek 
documents use to refer to these chips and the specific numbers are akin 
to the manufacturing part number that you'd actually order (maybe that's 
a bit of a stretch).

The SoC/CPU block probably does exist as a separate silicon die that 
they connect to the different switch blocks in the chips that they sell 
but I don't think you can get "just" the SoC. There is every chance that 
we'll see that same SoC/CPU block pop up in new chips (I see references 
to a RTL9302D in some documents). I'd like to be able to support these 
chips using "rtl9300" but if that's violating the wildcard rule I can 
drop it.

>> +      - enum:
>> +          - realtek,rtl9301-snand
>> +          - realtek,rtl9302b-snand
>> +          - realtek,rtl9302c-snand
>> +          - realtek,rtl9303-snand
>> +      - const: realtek,rtl9300-snand
>> +
>> +  reg:
>> +    items:
>> +      - description: SPI NAND controller registers address and size
> Also: why no clocks? Binding is supposed to be complete. If it cannot,
> you should explain it in the commit msg.

I didn't add it because I had no need for it in my driver. But as you've 
said previously the binding shouldn't care what the driver does.

I do have the clocking info from the datasheets. I'll add it in v2.

> Best regards,
> Krzysztof
>
>

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Chris Packham]

On 8/10/24 08:58, Chris Packham wrote:
>
> On 7/10/24 19:40, Krzysztof Kozlowski wrote:
>> On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
>>> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
>>> controller supports
>>>   * Serial/Dual/Quad data with
>>>   * PIO and DMA data read/write operation
>>>   * Configurable flash access timing
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>   .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 
>>> +++++++++++++++++++
>>>   1 file changed, 58 insertions(+)
>>>   create mode 100644 
>>> Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>
>>> diff --git 
>>> a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml 
>>> b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>> new file mode 100644
>>> index 000000000000..c66aea24cb35
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>> @@ -0,0 +1,58 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
>>> +
>>> +maintainers:
>>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> +
>>> +description:
>>> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It 
>>> supports
>>> +  typical SPI-NAND page cache operations in single, dual or quad IO 
>>> mode.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>> Why 9300 cannot be alone? What does 9300 mean even? Wildcards and family
>> models are not allowed in general.
>
> The main thing about the RTL9300 is that that is what all the Realtek 
> documents use to refer to these chips and the specific numbers are 
> akin to the manufacturing part number that you'd actually order (maybe 
> that's a bit of a stretch).
>
> The SoC/CPU block probably does exist as a separate silicon die that 
> they connect to the different switch blocks in the chips that they 
> sell but I don't think you can get "just" the SoC. There is every 
> chance that we'll see that same SoC/CPU block pop up in new chips (I 
> see references to a RTL9302D in some documents). I'd like to be able 
> to support these chips using "rtl9300" but if that's violating the 
> wildcard rule I can drop it.
>
Maybe it's helpful to think of the RTL9300 as the IP block that is 
integrated into the RTL9301, RTL9302B, etc.

>>> +      - enum:
>>> +          - realtek,rtl9301-snand
>>> +          - realtek,rtl9302b-snand
>>> +          - realtek,rtl9302c-snand
>>> +          - realtek,rtl9303-snand
>>> +      - const: realtek,rtl9300-snand
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: SPI NAND controller registers address and size
>> Also: why no clocks? Binding is supposed to be complete. If it cannot,
>> you should explain it in the commit msg.
>
> I didn't add it because I had no need for it in my driver. But as 
> you've said previously the binding shouldn't care what the driver does.
>
> I do have the clocking info from the datasheets. I'll add it in v2.
>
>> Best regards,
>> Krzysztof
>>
>>

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Krzysztof Kozlowski]

On 07/10/2024 21:58, Chris Packham wrote:
> 
> On 7/10/24 19:40, Krzysztof Kozlowski wrote:
>> On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
>>> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
>>> controller supports
>>>   * Serial/Dual/Quad data with
>>>   * PIO and DMA data read/write operation
>>>   * Configurable flash access timing
>>>
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>   .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 +++++++++++++++++++
>>>   1 file changed, 58 insertions(+)
>>>   create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>> new file mode 100644
>>> index 000000000000..c66aea24cb35
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>> @@ -0,0 +1,58 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
>>> +
>>> +maintainers:
>>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> +
>>> +description:
>>> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It supports
>>> +  typical SPI-NAND page cache operations in single, dual or quad IO mode.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    items:
>> Why 9300 cannot be alone? What does 9300 mean even? Wildcards and family
>> models are not allowed in general.
> 
> The main thing about the RTL9300 is that that is what all the Realtek 
> documents use to refer to these chips and the specific numbers are akin 
> to the manufacturing part number that you'd actually order (maybe that's 
> a bit of a stretch).
> 
> The SoC/CPU block probably does exist as a separate silicon die that 
> they connect to the different switch blocks in the chips that they sell 
> but I don't think you can get "just" the SoC. There is every chance that 
> we'll see that same SoC/CPU block pop up in new chips (I see references 
> to a RTL9302D in some documents). I'd like to be able to support these 
> chips using "rtl9300" but if that's violating the wildcard rule I can 
> drop it.

Yeah, that's violating the wildcard rule. You cannot even guarantee that
9300 will match future designs.

> 
>>> +      - enum:
>>> +          - realtek,rtl9301-snand
>>> +          - realtek,rtl9302b-snand
>>> +          - realtek,rtl9302c-snand
>>> +          - realtek,rtl9303-snand
>>> +      - const: realtek,rtl9300-snand
>>> +
>>> +  reg:
>>> +    items:
>>> +      - description: SPI NAND controller registers address and size
>> Also: why no clocks? Binding is supposed to be complete. If it cannot,
>> you should explain it in the commit msg.
> 
> I didn't add it because I had no need for it in my driver. But as you've 
> said previously the binding shouldn't care what the driver does.
> 
> I do have the clocking info from the datasheets. I'll add it in v2.


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Chris Packham]

On 8/10/24 19:59, Krzysztof Kozlowski wrote:
> On 07/10/2024 21:58, Chris Packham wrote:
>> On 7/10/24 19:40, Krzysztof Kozlowski wrote:
>>> On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
>>>> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
>>>> controller supports
>>>>    * Serial/Dual/Quad data with
>>>>    * PIO and DMA data read/write operation
>>>>    * Configurable flash access timing
>>>>
>>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>>> ---
>>>>    .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 +++++++++++++++++++
>>>>    1 file changed, 58 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>> new file mode 100644
>>>> index 000000000000..c66aea24cb35
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>> @@ -0,0 +1,58 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://scanmail.trustwave.com/?c=20988&d=2tiE5xx2mR7Mo-BCj_ZnEp9_tDM1bfG85uPlEm-9ag&u=http%3a%2f%2fdevicetree%2eorg%2fschemas%2fspi%2frealtek%2crtl9300-snand%2eyaml%23
>>>> +$schema: http://scanmail.trustwave.com/?c=20988&d=2tiE5xx2mR7Mo-BCj_ZnEp9_tDM1bfG85uCwRjixZQ&u=http%3a%2f%2fdevicetree%2eorg%2fmeta-schemas%2fcore%2eyaml%23
>>>> +
>>>> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
>>>> +
>>>> +maintainers:
>>>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>>>> +
>>>> +description:
>>>> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It supports
>>>> +  typical SPI-NAND page cache operations in single, dual or quad IO mode.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    items:
>>> Why 9300 cannot be alone? What does 9300 mean even? Wildcards and family
>>> models are not allowed in general.
>> The main thing about the RTL9300 is that that is what all the Realtek
>> documents use to refer to these chips and the specific numbers are akin
>> to the manufacturing part number that you'd actually order (maybe that's
>> a bit of a stretch).
>>
>> The SoC/CPU block probably does exist as a separate silicon die that
>> they connect to the different switch blocks in the chips that they sell
>> but I don't think you can get "just" the SoC. There is every chance that
>> we'll see that same SoC/CPU block pop up in new chips (I see references
>> to a RTL9302D in some documents). I'd like to be able to support these
>> chips using "rtl9300" but if that's violating the wildcard rule I can
>> drop it.
> Yeah, that's violating the wildcard rule. You cannot even guarantee that
> 9300 will match future designs.

When the dust settles I'll try do clean up the things I've already had 
in flight. Hopefully it's not too late to just change things rather than 
needing to support the incorrect wildcards as deprecated.

I have been meaning to clean up the mips dtsi files so that there is one 
for each of the rtl9301, rtl9302b etc but wanted to wait for my other 
changes to land. Sorry for creating a bit of a mess.

>>>> +      - enum:
>>>> +          - realtek,rtl9301-snand
>>>> +          - realtek,rtl9302b-snand
>>>> +          - realtek,rtl9302c-snand
>>>> +          - realtek,rtl9303-snand
>>>> +      - const: realtek,rtl9300-snand
>>>> +
>>>> +  reg:
>>>> +    items:
>>>> +      - description: SPI NAND controller registers address and size
>>> Also: why no clocks? Binding is supposed to be complete. If it cannot,
>>> you should explain it in the commit msg.
>> I didn't add it because I had no need for it in my driver. But as you've
>> said previously the binding shouldn't care what the driver does.
>>
>> I do have the clocking info from the datasheets. I'll add it in v2.
>
> Best regards,
> Krzysztof
>

Re: [PATCH 1/3] dt-bindings: spi: Add realtek,rtl9300-snand

[Krzysztof Kozlowski]

On 07/10/2024 22:49, Chris Packham wrote:
> 
> On 8/10/24 08:58, Chris Packham wrote:
>>
>> On 7/10/24 19:40, Krzysztof Kozlowski wrote:
>>> On Mon, Oct 07, 2024 at 12:33:45PM +1300, Chris Packham wrote:
>>>> Add a dtschema for the SPI-NAND controller on the RTL9300 SoCs. The
>>>> controller supports
>>>>   * Serial/Dual/Quad data with
>>>>   * PIO and DMA data read/write operation
>>>>   * Configurable flash access timing
>>>>
>>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>>> ---
>>>>   .../bindings/spi/realtek,rtl9300-snand.yaml   | 58 
>>>> +++++++++++++++++++
>>>>   1 file changed, 58 insertions(+)
>>>>   create mode 100644 
>>>> Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>>
>>>> diff --git 
>>>> a/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml 
>>>> b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>> new file mode 100644
>>>> index 000000000000..c66aea24cb35
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/spi/realtek,rtl9300-snand.yaml
>>>> @@ -0,0 +1,58 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/spi/realtek,rtl9300-snand.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: SPI-NAND Flash Controller for Realtek RTL9300 SoCs
>>>> +
>>>> +maintainers:
>>>> +  - Chris Packham <chris.packham@alliedtelesis.co.nz>
>>>> +
>>>> +description:
>>>> +  The Realtek RTL9300 SoCs have a built in SPI-NAND controller. It 
>>>> supports
>>>> +  typical SPI-NAND page cache operations in single, dual or quad IO 
>>>> mode.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    items:
>>> Why 9300 cannot be alone? What does 9300 mean even? Wildcards and family
>>> models are not allowed in general.
>>
>> The main thing about the RTL9300 is that that is what all the Realtek 
>> documents use to refer to these chips and the specific numbers are 
>> akin to the manufacturing part number that you'd actually order (maybe 
>> that's a bit of a stretch).
>>
>> The SoC/CPU block probably does exist as a separate silicon die that 
>> they connect to the different switch blocks in the chips that they 
>> sell but I don't think you can get "just" the SoC. There is every 
>> chance that we'll see that same SoC/CPU block pop up in new chips (I 
>> see references to a RTL9302D in some documents). I'd like to be able 
>> to support these chips using "rtl9300" but if that's violating the 
>> wildcard rule I can drop it.
>>
> Maybe it's helpful to think of the RTL9300 as the IP block that is 
> integrated into the RTL9301, RTL9302B, etc.

Yeah, it could work but we discourage this pattern. New devices from
930x might not be compatible with 9300 and then it is unclear what
"9300" actually mean.

The generic recommendation: please go with specific compatibles and use
one specific compatible as fallback for others.

Best regards,
Krzysztof