[PATCH 0/3] spi: sophgo: add Sophgo SPI NOR controller driver

[PATCH 0/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Longbin Li]

Add SPI NOR driver for Sophgo, including read, write operations.
This driver is only suitable for NOR flash.

Longbin Li (3):
  dt-bindings: spi: add Sophgo SPI NOR controller driver
  spi: sophgo: add Sophgo SPI NOR controller driver
  riscv: dts: sophgo: add Sophgo SPI NOR controller driver

 .../bindings/spi/spi-sophgo-nor.yaml          |  52 ++
 .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  |  18 +
 arch/riscv/boot/dts/sophgo/sg2044.dtsi        |  24 +
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-sophgo-nor.c                  | 501 ++++++++++++++++++
 6 files changed, 605 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml
 create mode 100644 drivers/spi/spi-sophgo-nor.c

--
2.48.1

[PATCH 1/3] dt-bindings: spi: add Sophgo SPI NOR controller driver

[Longbin Li]

Add SPI NOR driver for Sophgo, including read, write operations.

Signed-off-by: Longbin Li <looong.bin@gmail.com>
---
 .../bindings/spi/spi-sophgo-nor.yaml          | 52 +++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml

diff --git a/Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml b/Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml
new file mode 100644
index 000000000000..0b54946c649f
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/spi-sophgo-nor.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sophgo SPI NOR controller
+
+maintainers:
+  - Longbin Li <looong.bin@gmail.com>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  compatible:
+    const: sophgo,sg2044-spifmc-nor
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  resets:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - interrupts
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi@1000000 {
+      compatible = "sophgo,sg2044-spifmc-nor";
+      reg = <0x1000000 0x4000000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+      clocks = <&clk 0>;
+      interrupts = <37 IRQ_TYPE_LEVEL_HIGH>;
+      resets = <&rst 0>;
+    };
--
2.48.1

[PATCH 2/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Longbin Li]

Add support for Sophgo SPI NOR controller in Sophgo SoC.

Signed-off-by: Longbin Li <looong.bin@gmail.com>
---
 drivers/spi/Kconfig          |   9 +
 drivers/spi/Makefile         |   1 +
 drivers/spi/spi-sophgo-nor.c | 501 +++++++++++++++++++++++++++++++++++
 3 files changed, 511 insertions(+)
 create mode 100644 drivers/spi/spi-sophgo-nor.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index ea8a31032927..6b6d7b348485 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -1021,6 +1021,15 @@ config SPI_SN_F_OSPI
 	  for connecting an SPI Flash memory over up to 8-bit wide bus.
 	  It supports indirect access mode only.

+config SPI_SOPHGO_NOR
+	tristate "Sophgo SPI NOR Controller"
+	depends on ARCH_SOPHGO || COMPILE_TEST
+	help
+	  This enables support for the Sophgo SPI NOR controller,
+	  which supports Dual/Qual read and write operations while
+	  also supporting 3Byte address devices and 4Byte address
+	  devices.
+
 config SPI_SPRD
 	tristate "Spreadtrum SPI controller"
 	depends on ARCH_SPRD || COMPILE_TEST
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 9db7554c1864..9ded1de4b2fd 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -134,6 +134,7 @@ obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
 obj-$(CONFIG_SPI_SIFIVE)		+= spi-sifive.o
 obj-$(CONFIG_SPI_SLAVE_MT27XX)          += spi-slave-mt27xx.o
 obj-$(CONFIG_SPI_SN_F_OSPI)		+= spi-sn-f-ospi.o
+obj-$(CONFIG_SPI_SOPHGO_NOR)	+= spi-sophgo-nor.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-sophgo-nor.c b/drivers/spi/spi-sophgo-nor.c
new file mode 100644
index 000000000000..1139deeac327
--- /dev/null
+++ b/drivers/spi/spi-sophgo-nor.c
@@ -0,0 +1,501 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Sophgo SPI NOR controller driver
+ *
+ * Copyright (c) 2025 Longbin Li <looong.bin@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/spi/spi-mem.h>
+
+/* Hardware register definitions */
+#define SPIFMC_CTRL				0x00
+#define SPIFMC_CTRL_CPHA			BIT(12)
+#define SPIFMC_CTRL_CPOL			BIT(13)
+#define SPIFMC_CTRL_HOLD_OL			BIT(14)
+#define SPIFMC_CTRL_WP_OL			BIT(15)
+#define SPIFMC_CTRL_LSBF			BIT(20)
+#define SPIFMC_CTRL_SRST			BIT(21)
+#define SPIFMC_CTRL_SCK_DIV_SHIFT		0
+#define SPIFMC_CTRL_FRAME_LEN_SHIFT		16
+#define SPIFMC_CTRL_SCK_DIV_MASK		0x7FF
+
+#define SPIFMC_CE_CTRL				0x04
+#define SPIFMC_CE_CTRL_CEMANUAL			BIT(0)
+#define SPIFMC_CE_CTRL_CEMANUAL_EN		BIT(1)
+
+#define SPIFMC_DLY_CTRL				0x08
+#define SPIFMC_CTRL_FM_INTVL_MASK		0x000f
+#define SPIFMC_CTRL_FM_INTVL			BIT(0)
+#define SPIFMC_CTRL_CET_MASK			0x0f00
+#define SPIFMC_CTRL_CET				BIT(8)
+
+#define SPIFMC_DMMR				0x0c
+
+#define SPIFMC_TRAN_CSR				0x10
+#define SPIFMC_TRAN_CSR_TRAN_MODE_MASK		GENMASK(1, 0)
+#define SPIFMC_TRAN_CSR_TRAN_MODE_RX		BIT(0)
+#define SPIFMC_TRAN_CSR_TRAN_MODE_TX		BIT(1)
+#define SPIFMC_TRAN_CSR_FAST_MODE		BIT(3)
+#define SPIFMC_TRAN_CSR_BUS_WIDTH_1_BIT		(0x00 << 4)
+#define SPIFMC_TRAN_CSR_BUS_WIDTH_2_BIT		(0x01 << 4)
+#define SPIFMC_TRAN_CSR_BUS_WIDTH_4_BIT		(0x02 << 4)
+#define SPIFMC_TRAN_CSR_DMA_EN			BIT(6)
+#define SPIFMC_TRAN_CSR_MISO_LEVEL		BIT(7)
+#define SPIFMC_TRAN_CSR_ADDR_BYTES_MASK		GENMASK(10, 8)
+#define SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT	8
+#define SPIFMC_TRAN_CSR_WITH_CMD		BIT(11)
+#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_MASK	GENMASK(13, 12)
+#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE	(0x00 << 12)
+#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_2_BYTE	(0x01 << 12)
+#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_4_BYTE	(0x02 << 12)
+#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE	(0x03 << 12)
+#define SPIFMC_TRAN_CSR_GO_BUSY			BIT(15)
+#define SPIFMC_TRAN_CSR_ADDR4B_SHIFT		20
+#define SPIFMC_TRAN_CSR_CMD4B_SHIFT		21
+
+#define SPIFMC_TRAN_NUM				0x14
+#define SPIFMC_FIFO_PORT			0x18
+#define SPIFMC_FIFO_PT				0x20
+
+#define SPIFMC_INT_STS				0x28
+#define SPIFMC_INT_TRAN_DONE			BIT(0)
+#define SPIFMC_INT_RD_FIFO			BIT(2)
+#define SPIFMC_INT_WR_FIFO			BIT(3)
+#define SPIFMC_INT_RX_FRAME			BIT(4)
+#define SPIFMC_INT_TX_FRAME			BIT(5)
+
+#define SPIFMC_INT_EN				0x2c
+#define SPIFMC_INT_TRAN_DONE_EN			BIT(0)
+#define SPIFMC_INT_RD_FIFO_EN			BIT(2)
+#define SPIFMC_INT_WR_FIFO_EN			BIT(3)
+#define SPIFMC_INT_RX_FRAME_EN			BIT(4)
+#define SPIFMC_INT_TX_FRAME_EN			BIT(5)
+
+#define SPIFMC_OPT				0x030
+#define SPIFMC_OPT_DISABLE_FIFO_FLUSH		BIT(1)
+
+#define SPIFMC_MAX_FIFO_DEPTH			8
+
+#define SPIFMC_MAX_READ_SIZE			0x10000
+
+struct sophgo_spifmc {
+	struct spi_controller *ctrl;
+	void __iomem *io_base;
+	struct device *dev;
+	struct mutex lock;
+	struct clk *clk;
+};
+
+static int sophgo_spifmc_wait_int(struct sophgo_spifmc *spifmc, u8 int_type)
+{
+	u32 stat;
+
+	return readl_poll_timeout(spifmc->io_base + SPIFMC_INT_STS, stat,
+				  (stat & int_type), 0, 1000000);
+}
+
+static int sophgo_spifmc_wait_xfer_size(struct sophgo_spifmc *spifmc,
+					int xfer_size)
+{
+	u8 stat;
+
+	return readl_poll_timeout(spifmc->io_base + SPIFMC_FIFO_PT, stat,
+				  ((stat & 0xf) == xfer_size), 1, 1000000);
+}
+
+static u32 sophgo_spifmc_init_reg(struct sophgo_spifmc *spifmc)
+{
+	u32 reg;
+
+	reg = readl(spifmc->io_base + SPIFMC_TRAN_CSR);
+	reg &= ~(SPIFMC_TRAN_CSR_TRAN_MODE_MASK |
+		 SPIFMC_TRAN_CSR_FAST_MODE |
+		 SPIFMC_TRAN_CSR_BUS_WIDTH_2_BIT |
+		 SPIFMC_TRAN_CSR_BUS_WIDTH_4_BIT |
+		 SPIFMC_TRAN_CSR_DMA_EN |
+		 SPIFMC_TRAN_CSR_ADDR_BYTES_MASK |
+		 SPIFMC_TRAN_CSR_WITH_CMD |
+		 SPIFMC_TRAN_CSR_FIFO_TRG_LVL_MASK);
+
+	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
+
+	return reg;
+}
+
+static ssize_t sophgo_spifmc_read_64k(struct sophgo_spifmc *spifmc,
+				      const struct spi_mem_op *op, loff_t from,
+				      size_t len, u_char *buf)
+{
+	int xfer_size, offset;
+	u32 reg;
+	int ret;
+	int i;
+
+	reg = sophgo_spifmc_init_reg(spifmc);
+	reg |= (op->addr.nbytes + op->dummy.nbytes) << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT;
+	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE;
+	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
+	reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = op->addr.nbytes - 1; i >= 0; i--)
+		writeb((from >> i * 8) & 0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = 0; i < op->dummy.nbytes; i++)
+		writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
+	writel(0, spifmc->io_base + SPIFMC_INT_STS);
+	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
+	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
+
+	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_RD_FIFO);
+	if (ret < 0)
+		return ret;
+
+	offset = 0;
+	while (offset < len) {
+		xfer_size = min_t(size_t, SPIFMC_MAX_FIFO_DEPTH, len - offset);
+
+		ret = sophgo_spifmc_wait_xfer_size(spifmc, xfer_size);
+		if (ret < 0)
+			return ret;
+
+		for (i = 0; i < xfer_size; i++)
+			buf[i + offset] = readb(spifmc->io_base + SPIFMC_FIFO_PORT);
+
+		offset += xfer_size;
+	}
+
+	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
+	if (ret < 0)
+		return ret;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+
+	return len;
+}
+
+static ssize_t sophgo_spifmc_read(struct sophgo_spifmc *spifmc,
+				  const struct spi_mem_op *op)
+{
+	size_t xfer_size;
+	size_t offset;
+	loff_t from = op->addr.val;
+	size_t len = op->data.nbytes;
+	int ret;
+	u8 *din = op->data.buf.in;
+
+	offset = 0;
+	while (offset < len) {
+		xfer_size = min_t(size_t, SPIFMC_MAX_READ_SIZE, len - offset);
+
+		ret = sophgo_spifmc_read_64k(spifmc, op, from, xfer_size, din);
+		if (ret < 0)
+			return ret;
+
+		offset += xfer_size;
+		din += xfer_size;
+		from += xfer_size;
+	}
+
+	return 0;
+}
+
+static ssize_t sophgo_spifmc_write(struct sophgo_spifmc *spifmc,
+				   const struct spi_mem_op *op)
+{
+	size_t xfer_size;
+	const u8 *dout = op->data.buf.out;
+	int i, offset;
+	size_t ret;
+	u32 reg;
+
+	reg = sophgo_spifmc_init_reg(spifmc);
+	reg |= (op->addr.nbytes + op->dummy.nbytes) << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT;
+	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE;
+	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
+	reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = op->addr.nbytes - 1; i >= 0; i--)
+		writeb((op->addr.val >> i * 8) & 0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = 0; i < op->dummy.nbytes; i++)
+		writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	writel(0, spifmc->io_base + SPIFMC_INT_STS);
+	writel(op->data.nbytes, spifmc->io_base + SPIFMC_TRAN_NUM);
+	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
+	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
+
+	ret = sophgo_spifmc_wait_xfer_size(spifmc, 0);
+	if (ret < 0)
+		return ret;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+
+	offset = 0;
+	while (offset < op->data.nbytes) {
+		xfer_size = min_t(size_t, SPIFMC_MAX_FIFO_DEPTH, op->data.nbytes - offset);
+
+		ret = sophgo_spifmc_wait_xfer_size(spifmc, 0);
+		if (ret < 0)
+			return ret;
+
+		for (i = 0; i < xfer_size; i++)
+			writeb(dout[i + offset], spifmc->io_base + SPIFMC_FIFO_PORT);
+
+		offset += xfer_size;
+	}
+
+	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
+	if (ret < 0)
+		return ret;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+
+	return 0;
+}
+
+static ssize_t sophgo_spifmc_tran_cmd(struct sophgo_spifmc *spifmc,
+				      const struct spi_mem_op *op)
+{
+	int i, ret;
+	u32 reg;
+
+	reg = sophgo_spifmc_init_reg(spifmc);
+	reg |= (op->addr.nbytes + op->dummy.nbytes) << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT;
+	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE;
+	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = op->addr.nbytes - 1; i >= 0; i--)
+		writeb((op->addr.val >> i * 8) & 0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = 0; i < op->dummy.nbytes; i++)
+		writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	writel(0, spifmc->io_base + SPIFMC_INT_STS);
+	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
+	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
+
+	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
+	if (ret < 0)
+		return ret;
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+
+	return 0;
+}
+
+static void sophgo_spifmc_trans(struct sophgo_spifmc *spifmc,
+				const struct spi_mem_op *op)
+{
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		sophgo_spifmc_read(spifmc, op);
+	else if (op->data.dir == SPI_MEM_DATA_OUT)
+		sophgo_spifmc_write(spifmc, op);
+	else
+		sophgo_spifmc_tran_cmd(spifmc, op);
+}
+
+static ssize_t sophgo_spifmc_trans_reg(struct sophgo_spifmc *spifmc,
+				       const struct spi_mem_op *op)
+{
+	const u8 *dout = NULL;
+	u8 *din = NULL;
+	size_t len = op->data.nbytes;
+	u32 reg;
+	int ret;
+	int i;
+
+	if (op->data.dir == SPI_MEM_DATA_IN)
+		din = op->data.buf.in;
+	else
+		dout = op->data.buf.out;
+
+	reg = sophgo_spifmc_init_reg(spifmc);
+	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE;
+	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
+
+	if (din) {
+		reg |= SPIFMC_TRAN_CSR_BUS_WIDTH_1_BIT;
+		reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
+		reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
+
+		writel(SPIFMC_OPT_DISABLE_FIFO_FLUSH, spifmc->io_base + SPIFMC_OPT);
+	} else {
+		/*
+		 * If write values to the Status Register,
+		 * configure TRAN_CSR register as the same as
+		 * sophgo_spifmc_read_reg.
+		 */
+		if (op->cmd.opcode == 0x01) {
+			reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
+			reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
+			writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
+		}
+	}
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
+
+	for (i = 0; i < len; i++) {
+		if (din)
+			writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
+		else
+			writeb(dout[i], spifmc->io_base + SPIFMC_FIFO_PORT);
+	}
+
+	writel(0, spifmc->io_base + SPIFMC_INT_STS);
+	writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
+	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
+	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
+
+	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
+	if (ret < 0)
+		return ret;
+
+	if (din) {
+		while (len--)
+			*din++ = readb(spifmc->io_base + SPIFMC_FIFO_PORT);
+	}
+
+	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
+
+	return 0;
+}
+
+static int sophgo_spifmc_exec_op(struct spi_mem *mem,
+				 const struct spi_mem_op *op)
+{
+	struct sophgo_spifmc *spifmc;
+
+	spifmc = spi_controller_get_devdata(mem->spi->controller);
+
+	mutex_lock(&spifmc->lock);
+
+	if (op->addr.nbytes == 0)
+		sophgo_spifmc_trans_reg(spifmc, op);
+	else
+		sophgo_spifmc_trans(spifmc, op);
+
+	mutex_unlock(&spifmc->lock);
+
+	return 0;
+}
+
+static const struct spi_controller_mem_ops sophgo_spifmc_mem_ops = {
+	.exec_op = sophgo_spifmc_exec_op,
+};
+
+static void sophgo_spifmc_init(struct sophgo_spifmc *spifmc)
+{
+	u32 tran_csr;
+	u32 reg;
+
+	writel(0, spifmc->io_base + SPIFMC_DMMR);
+
+	reg = readl(spifmc->io_base + SPIFMC_CTRL);
+	reg |= SPIFMC_CTRL_SRST;
+	reg &= ~((1 << 11) - 1);
+	reg |= 1;
+	writel(reg, spifmc->io_base + SPIFMC_CTRL);
+
+	writel(0, spifmc->io_base + SPIFMC_CE_CTRL);
+
+	tran_csr = readl(spifmc->io_base + SPIFMC_TRAN_CSR);
+	tran_csr |= (0 << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT);
+	tran_csr |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_4_BYTE;
+	tran_csr |= SPIFMC_TRAN_CSR_WITH_CMD;
+	writel(tran_csr, spifmc->io_base + SPIFMC_TRAN_CSR);
+}
+
+static int sophgo_spifmc_probe(struct platform_device *pdev)
+{
+	struct spi_controller *ctrl;
+	struct sophgo_spifmc *spifmc;
+	void __iomem *base;
+	int ret;
+
+	ctrl = devm_spi_alloc_host(&pdev->dev, sizeof(*spifmc));
+	if (!ctrl)
+		return -ENOMEM;
+
+	spifmc = spi_controller_get_devdata(ctrl);
+	dev_set_drvdata(&pdev->dev, ctrl);
+
+	spifmc->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(spifmc->clk)) {
+		dev_err(&pdev->dev, "AHB clock not found.\n");
+		return PTR_ERR(spifmc->clk);
+	}
+
+	ret = clk_prepare_enable(spifmc->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
+		return ret;
+	}
+
+	spifmc->dev = &pdev->dev;
+	spifmc->ctrl = ctrl;
+
+	spifmc->io_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	ctrl->num_chipselect = 1;
+	ctrl->dev.of_node = pdev->dev.of_node;
+	ctrl->bits_per_word_mask = SPI_BPW_MASK(8);
+	ctrl->auto_runtime_pm = false;
+	ctrl->mem_ops = &sophgo_spifmc_mem_ops;
+	ctrl->mode_bits = SPI_RX_DUAL | SPI_TX_DUAL | SPI_RX_QUAD | SPI_TX_QUAD;
+
+	mutex_init(&spifmc->lock);
+
+	sophgo_spifmc_init(spifmc);
+	sophgo_spifmc_init_reg(spifmc);
+
+	return devm_spi_register_controller(&pdev->dev, ctrl);
+}
+
+static void sophgo_spifmc_remove(struct platform_device *pdev)
+{
+	struct sophgo_spifmc *spifmc = platform_get_drvdata(pdev);
+
+	mutex_destroy(&spifmc->lock);
+}
+
+static const struct of_device_id sophgo_spifmc_match[] = {
+	{ .compatible = "sophgo,sg2044-spifmc-nor" },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, sophgo_spifmc_match);
+
+static struct platform_driver sophgo_nor_driver = {
+	.driver = {
+		.name = "sophgo,spifmc-nor",
+		.of_match_table = sophgo_spifmc_match,
+	},
+	.probe = sophgo_spifmc_probe,
+	.remove = sophgo_spifmc_remove,
+};
+
+module_platform_driver(sophgo_nor_driver);
+
+MODULE_DESCRIPTION("Sophgo SPI NOR controller driver");
+MODULE_AUTHOR("Longbin Li <looong.bin@gmail.com>");
+MODULE_LICENSE("GPL");
--
2.48.1

[PATCH 3/3] riscv: dts: sophgo: add Sophgo SPI NOR controller driver

[Longbin Li]

Add SPI NOR device node for Sophgo.

Signed-off-by: Longbin Li <looong.bin@gmail.com>
---
 .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  | 18 ++++++++++++++
 arch/riscv/boot/dts/sophgo/sg2044.dtsi        | 24 +++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
index c50e61a50013..9c634920f37e 100644
--- a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
+++ b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
@@ -80,6 +80,24 @@ &sd {
 	status = "okay";
 };

+&spifmc0 {
+	status = "okay";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+	};
+};
+
+&spifmc1 {
+	status = "okay";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+	};
+};
+
 &uart1 {
 	status = "okay";
 };
diff --git a/arch/riscv/boot/dts/sophgo/sg2044.dtsi b/arch/riscv/boot/dts/sophgo/sg2044.dtsi
index 6eaf92dd0a90..e45f7218de04 100644
--- a/arch/riscv/boot/dts/sophgo/sg2044.dtsi
+++ b/arch/riscv/boot/dts/sophgo/sg2044.dtsi
@@ -33,6 +33,30 @@ soc {
 		dma-noncoherent;
 		ranges;

+		spifmc0: spi@7001000000 {
+			compatible = "sophgo,sg2044-spifmc-nor";
+			reg = <0x70 0x01000000 0x0 0x4000000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clocks = <&clk CLK_GATE_AHB_SPIFMC>;
+			interrupt-parent = <&intc>;
+			interrupts = <37 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&rst RST_SPIFMC0>;
+			status = "disabled";
+		};
+
+		spifmc1: spi@7005000000 {
+			compatible = "sophgo,sg2044-spifmc-nor";
+			reg = <0x70 0x05000000 0x0 0x4000000>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			clocks = <&clk CLK_GATE_AHB_SPIFMC>;
+			interrupt-parent = <&intc>;
+			interrupts = <38 IRQ_TYPE_LEVEL_HIGH>;
+			resets = <&rst RST_SPIFMC1>;
+			status = "disabled";
+		};
+
 		dmac0: dma-controller@7020000000 {
 			compatible = "snps,axi-dma-1.01a";
 			reg = <0x70 0x20000000 0x0 0x10000>;
--
2.48.1

Re: [PATCH 2/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Yixun Lan]

Hi Longbin:

On 18:12 Mon 24 Feb     , Longbin Li wrote:
> Add support for Sophgo SPI NOR controller in Sophgo SoC.
> 
> Signed-off-by: Longbin Li <looong.bin@gmail.com>
> ---
>  drivers/spi/Kconfig          |   9 +
>  drivers/spi/Makefile         |   1 +
>  drivers/spi/spi-sophgo-nor.c | 501 +++++++++++++++++++++++++++++++++++
>  3 files changed, 511 insertions(+)
>  create mode 100644 drivers/spi/spi-sophgo-nor.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index ea8a31032927..6b6d7b348485 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -1021,6 +1021,15 @@ config SPI_SN_F_OSPI
>  	  for connecting an SPI Flash memory over up to 8-bit wide bus.
>  	  It supports indirect access mode only.
> 
> +config SPI_SOPHGO_NOR
> +	tristate "Sophgo SPI NOR Controller"
> +	depends on ARCH_SOPHGO || COMPILE_TEST
> +	help
> +	  This enables support for the Sophgo SPI NOR controller,
> +	  which supports Dual/Qual read and write operations while
> +	  also supporting 3Byte address devices and 4Byte address
> +	  devices.
> +
>  config SPI_SPRD
>  	tristate "Spreadtrum SPI controller"
>  	depends on ARCH_SPRD || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 9db7554c1864..9ded1de4b2fd 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
>  obj-$(CONFIG_SPI_SIFIVE)		+= spi-sifive.o
>  obj-$(CONFIG_SPI_SLAVE_MT27XX)          += spi-slave-mt27xx.o
>  obj-$(CONFIG_SPI_SN_F_OSPI)		+= spi-sn-f-ospi.o
> +obj-$(CONFIG_SPI_SOPHGO_NOR)	+= spi-sophgo-nor.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-sophgo-nor.c b/drivers/spi/spi-sophgo-nor.c
> new file mode 100644
> index 000000000000..1139deeac327
> --- /dev/null
> +++ b/drivers/spi/spi-sophgo-nor.c
> @@ -0,0 +1,501 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Sophgo SPI NOR controller driver
> + *
> + * Copyright (c) 2025 Longbin Li <looong.bin@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/spi/spi-mem.h>
> +
> +/* Hardware register definitions */
> +#define SPIFMC_CTRL				0x00
> +#define SPIFMC_CTRL_CPHA			BIT(12)
> +#define SPIFMC_CTRL_CPOL			BIT(13)
> +#define SPIFMC_CTRL_HOLD_OL			BIT(14)
> +#define SPIFMC_CTRL_WP_OL			BIT(15)
> +#define SPIFMC_CTRL_LSBF			BIT(20)
> +#define SPIFMC_CTRL_SRST			BIT(21)
> +#define SPIFMC_CTRL_SCK_DIV_SHIFT		0
> +#define SPIFMC_CTRL_FRAME_LEN_SHIFT		16
> +#define SPIFMC_CTRL_SCK_DIV_MASK		0x7FF
> +
> +#define SPIFMC_CE_CTRL				0x04
> +#define SPIFMC_CE_CTRL_CEMANUAL			BIT(0)
> +#define SPIFMC_CE_CTRL_CEMANUAL_EN		BIT(1)
> +
> +#define SPIFMC_DLY_CTRL				0x08
> +#define SPIFMC_CTRL_FM_INTVL_MASK		0x000f
> +#define SPIFMC_CTRL_FM_INTVL			BIT(0)
> +#define SPIFMC_CTRL_CET_MASK			0x0f00
> +#define SPIFMC_CTRL_CET				BIT(8)
> +
> +#define SPIFMC_DMMR				0x0c
> +
> +#define SPIFMC_TRAN_CSR				0x10
> +#define SPIFMC_TRAN_CSR_TRAN_MODE_MASK		GENMASK(1, 0)
> +#define SPIFMC_TRAN_CSR_TRAN_MODE_RX		BIT(0)
> +#define SPIFMC_TRAN_CSR_TRAN_MODE_TX		BIT(1)
> +#define SPIFMC_TRAN_CSR_FAST_MODE		BIT(3)
> +#define SPIFMC_TRAN_CSR_BUS_WIDTH_1_BIT		(0x00 << 4)
> +#define SPIFMC_TRAN_CSR_BUS_WIDTH_2_BIT		(0x01 << 4)
> +#define SPIFMC_TRAN_CSR_BUS_WIDTH_4_BIT		(0x02 << 4)
> +#define SPIFMC_TRAN_CSR_DMA_EN			BIT(6)
> +#define SPIFMC_TRAN_CSR_MISO_LEVEL		BIT(7)
> +#define SPIFMC_TRAN_CSR_ADDR_BYTES_MASK		GENMASK(10, 8)
> +#define SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT	8
> +#define SPIFMC_TRAN_CSR_WITH_CMD		BIT(11)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_MASK	GENMASK(13, 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE	(0x00 << 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_2_BYTE	(0x01 << 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_4_BYTE	(0x02 << 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE	(0x03 << 12)
> +#define SPIFMC_TRAN_CSR_GO_BUSY			BIT(15)
> +#define SPIFMC_TRAN_CSR_ADDR4B_SHIFT		20
> +#define SPIFMC_TRAN_CSR_CMD4B_SHIFT		21
> +
> +#define SPIFMC_TRAN_NUM				0x14
> +#define SPIFMC_FIFO_PORT			0x18
> +#define SPIFMC_FIFO_PT				0x20
> +
> +#define SPIFMC_INT_STS				0x28
> +#define SPIFMC_INT_TRAN_DONE			BIT(0)
> +#define SPIFMC_INT_RD_FIFO			BIT(2)
> +#define SPIFMC_INT_WR_FIFO			BIT(3)
> +#define SPIFMC_INT_RX_FRAME			BIT(4)
> +#define SPIFMC_INT_TX_FRAME			BIT(5)
> +
> +#define SPIFMC_INT_EN				0x2c
> +#define SPIFMC_INT_TRAN_DONE_EN			BIT(0)
> +#define SPIFMC_INT_RD_FIFO_EN			BIT(2)
> +#define SPIFMC_INT_WR_FIFO_EN			BIT(3)
> +#define SPIFMC_INT_RX_FRAME_EN			BIT(4)
> +#define SPIFMC_INT_TX_FRAME_EN			BIT(5)
> +
> +#define SPIFMC_OPT				0x030
> +#define SPIFMC_OPT_DISABLE_FIFO_FLUSH		BIT(1)
> +
> +#define SPIFMC_MAX_FIFO_DEPTH			8
> +
> +#define SPIFMC_MAX_READ_SIZE			0x10000
> +
> +struct sophgo_spifmc {
> +	struct spi_controller *ctrl;
> +	void __iomem *io_base;
> +	struct device *dev;
> +	struct mutex lock;
it will be great to document the lock
> +	struct clk *clk;
> +};
> +
> +static int sophgo_spifmc_wait_int(struct sophgo_spifmc *spifmc, u8 int_type)
> +{
> +	u32 stat;
> +
> +	return readl_poll_timeout(spifmc->io_base + SPIFMC_INT_STS, stat,
> +				  (stat & int_type), 0, 1000000);
> +}
> +
> +static int sophgo_spifmc_wait_xfer_size(struct sophgo_spifmc *spifmc,
> +					int xfer_size)
> +{
> +	u8 stat;
> +
> +	return readl_poll_timeout(spifmc->io_base + SPIFMC_FIFO_PT, stat,
> +				  ((stat & 0xf) == xfer_size), 1, 1000000);
> +}
> +
> +static u32 sophgo_spifmc_init_reg(struct sophgo_spifmc *spifmc)
> +{
> +	u32 reg;
> +
> +	reg = readl(spifmc->io_base + SPIFMC_TRAN_CSR);
> +	reg &= ~(SPIFMC_TRAN_CSR_TRAN_MODE_MASK |
> +		 SPIFMC_TRAN_CSR_FAST_MODE |
> +		 SPIFMC_TRAN_CSR_BUS_WIDTH_2_BIT |
> +		 SPIFMC_TRAN_CSR_BUS_WIDTH_4_BIT |
> +		 SPIFMC_TRAN_CSR_DMA_EN |
> +		 SPIFMC_TRAN_CSR_ADDR_BYTES_MASK |
> +		 SPIFMC_TRAN_CSR_WITH_CMD |
> +		 SPIFMC_TRAN_CSR_FIFO_TRG_LVL_MASK);
> +
> +	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
> +
> +	return reg;
> +}
> +
> +static ssize_t sophgo_spifmc_read_64k(struct sophgo_spifmc *spifmc,
> +				      const struct spi_mem_op *op, loff_t from,
> +				      size_t len, u_char *buf)
> +{
> +	int xfer_size, offset;
> +	u32 reg;
> +	int ret;
> +	int i;
> +
> +	reg = sophgo_spifmc_init_reg(spifmc);
> +	reg |= (op->addr.nbytes + op->dummy.nbytes) << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT;
> +	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE;
> +	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
> +	reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = op->addr.nbytes - 1; i >= 0; i--)
> +		writeb((from >> i * 8) & 0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = 0; i < op->dummy.nbytes; i++)
> +		writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
> +	writel(0, spifmc->io_base + SPIFMC_INT_STS);
> +	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
> +	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
> +
> +	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_RD_FIFO);
> +	if (ret < 0)
> +		return ret;
> +
> +	offset = 0;
> +	while (offset < len) {
> +		xfer_size = min_t(size_t, SPIFMC_MAX_FIFO_DEPTH, len - offset);
> +
> +		ret = sophgo_spifmc_wait_xfer_size(spifmc, xfer_size);
> +		if (ret < 0)
> +			return ret;
> +
> +		for (i = 0; i < xfer_size; i++)
> +			buf[i + offset] = readb(spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +		offset += xfer_size;
> +	}
> +
> +	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +
> +	return len;
> +}
> +
> +static ssize_t sophgo_spifmc_read(struct sophgo_spifmc *spifmc,
> +				  const struct spi_mem_op *op)
> +{
> +	size_t xfer_size;
> +	size_t offset;
> +	loff_t from = op->addr.val;
> +	size_t len = op->data.nbytes;
> +	int ret;
> +	u8 *din = op->data.buf.in;
> +
> +	offset = 0;
> +	while (offset < len) {
> +		xfer_size = min_t(size_t, SPIFMC_MAX_READ_SIZE, len - offset);
> +
> +		ret = sophgo_spifmc_read_64k(spifmc, op, from, xfer_size, din);
> +		if (ret < 0)
> +			return ret;
> +
> +		offset += xfer_size;
> +		din += xfer_size;
> +		from += xfer_size;
> +	}
> +
> +	return 0;
> +}
> +
> +static ssize_t sophgo_spifmc_write(struct sophgo_spifmc *spifmc,
> +				   const struct spi_mem_op *op)
> +{
> +	size_t xfer_size;
> +	const u8 *dout = op->data.buf.out;
> +	int i, offset;
> +	size_t ret;
> +	u32 reg;
> +
> +	reg = sophgo_spifmc_init_reg(spifmc);
> +	reg |= (op->addr.nbytes + op->dummy.nbytes) << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT;
> +	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE;
> +	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
> +	reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = op->addr.nbytes - 1; i >= 0; i--)
> +		writeb((op->addr.val >> i * 8) & 0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = 0; i < op->dummy.nbytes; i++)
> +		writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	writel(0, spifmc->io_base + SPIFMC_INT_STS);
> +	writel(op->data.nbytes, spifmc->io_base + SPIFMC_TRAN_NUM);
> +	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
> +	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
> +
> +	ret = sophgo_spifmc_wait_xfer_size(spifmc, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +
> +	offset = 0;
> +	while (offset < op->data.nbytes) {
> +		xfer_size = min_t(size_t, SPIFMC_MAX_FIFO_DEPTH, op->data.nbytes - offset);
> +
> +		ret = sophgo_spifmc_wait_xfer_size(spifmc, 0);
> +		if (ret < 0)
> +			return ret;
> +
> +		for (i = 0; i < xfer_size; i++)
> +			writeb(dout[i + offset], spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +		offset += xfer_size;
> +	}
> +
> +	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +
> +	return 0;
> +}
> +
> +static ssize_t sophgo_spifmc_tran_cmd(struct sophgo_spifmc *spifmc,
> +				      const struct spi_mem_op *op)
> +{
> +	int i, ret;
> +	u32 reg;
> +
> +	reg = sophgo_spifmc_init_reg(spifmc);
> +	reg |= (op->addr.nbytes + op->dummy.nbytes) << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT;
> +	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE;
> +	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = op->addr.nbytes - 1; i >= 0; i--)
> +		writeb((op->addr.val >> i * 8) & 0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = 0; i < op->dummy.nbytes; i++)
> +		writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	writel(0, spifmc->io_base + SPIFMC_INT_STS);
> +	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
> +	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
> +
> +	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
> +	if (ret < 0)
> +		return ret;
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +
> +	return 0;
> +}
> +
> +static void sophgo_spifmc_trans(struct sophgo_spifmc *spifmc,
> +				const struct spi_mem_op *op)
> +{
> +	if (op->data.dir == SPI_MEM_DATA_IN)
> +		sophgo_spifmc_read(spifmc, op);
> +	else if (op->data.dir == SPI_MEM_DATA_OUT)
> +		sophgo_spifmc_write(spifmc, op);
> +	else
> +		sophgo_spifmc_tran_cmd(spifmc, op);
> +}
> +
> +static ssize_t sophgo_spifmc_trans_reg(struct sophgo_spifmc *spifmc,
> +				       const struct spi_mem_op *op)
> +{
> +	const u8 *dout = NULL;
> +	u8 *din = NULL;
> +	size_t len = op->data.nbytes;
> +	u32 reg;
> +	int ret;
> +	int i;
squash them which save one line:
	int i, ret;

> +
> +	if (op->data.dir == SPI_MEM_DATA_IN)
> +		din = op->data.buf.in;
> +	else
> +		dout = op->data.buf.out;
> +
> +	reg = sophgo_spifmc_init_reg(spifmc);
> +	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE;
> +	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
> +
> +	if (din) {
> +		reg |= SPIFMC_TRAN_CSR_BUS_WIDTH_1_BIT;
> +		reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
> +		reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
> +
> +		writel(SPIFMC_OPT_DISABLE_FIFO_FLUSH, spifmc->io_base + SPIFMC_OPT);
> +	} else {
> +		/*
> +		 * If write values to the Status Register,
> +		 * configure TRAN_CSR register as the same as
> +		 * sophgo_spifmc_read_reg.
> +		 */
> +		if (op->cmd.opcode == 0x01) {
> +			reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
> +			reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
> +			writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
> +		}
> +	}
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +	writeb(op->cmd.opcode, spifmc->io_base + SPIFMC_FIFO_PORT);
> +
> +	for (i = 0; i < len; i++) {
> +		if (din)
> +			writeb(0xff, spifmc->io_base + SPIFMC_FIFO_PORT);
> +		else
> +			writeb(dout[i], spifmc->io_base + SPIFMC_FIFO_PORT);
> +	}
> +
> +	writel(0, spifmc->io_base + SPIFMC_INT_STS);
> +	writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
> +	reg |= SPIFMC_TRAN_CSR_GO_BUSY;
> +	writel(reg, spifmc->io_base + SPIFMC_TRAN_CSR);
> +
> +	ret = sophgo_spifmc_wait_int(spifmc, SPIFMC_INT_TRAN_DONE);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (din) {
> +		while (len--)
> +			*din++ = readb(spifmc->io_base + SPIFMC_FIFO_PORT);
> +	}
> +
> +	writel(0, spifmc->io_base + SPIFMC_FIFO_PT);
> +
> +	return 0;
> +}
> +
> +static int sophgo_spifmc_exec_op(struct spi_mem *mem,
> +				 const struct spi_mem_op *op)
> +{
> +	struct sophgo_spifmc *spifmc;
> +
> +	spifmc = spi_controller_get_devdata(mem->spi->controller);
> +
> +	mutex_lock(&spifmc->lock);
> +
> +	if (op->addr.nbytes == 0)
> +		sophgo_spifmc_trans_reg(spifmc, op);
> +	else
> +		sophgo_spifmc_trans(spifmc, op);
> +
> +	mutex_unlock(&spifmc->lock);
> +
> +	return 0;
> +}
> +
> +static const struct spi_controller_mem_ops sophgo_spifmc_mem_ops = {
> +	.exec_op = sophgo_spifmc_exec_op,
> +};
> +
> +static void sophgo_spifmc_init(struct sophgo_spifmc *spifmc)
> +{
> +	u32 tran_csr;
> +	u32 reg;
> +
> +	writel(0, spifmc->io_base + SPIFMC_DMMR);
> +
> +	reg = readl(spifmc->io_base + SPIFMC_CTRL);
> +	reg |= SPIFMC_CTRL_SRST;
..
> +	reg &= ~((1 << 11) - 1);
so this is a mask? use macro to define, instead of using magic number
> +	reg |= 1;
> +	writel(reg, spifmc->io_base + SPIFMC_CTRL);
> +
> +	writel(0, spifmc->io_base + SPIFMC_CE_CTRL);
> +
> +	tran_csr = readl(spifmc->io_base + SPIFMC_TRAN_CSR);
> +	tran_csr |= (0 << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT);
> +	tran_csr |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_4_BYTE;
> +	tran_csr |= SPIFMC_TRAN_CSR_WITH_CMD;
> +	writel(tran_csr, spifmc->io_base + SPIFMC_TRAN_CSR);
> +}
> +
> +static int sophgo_spifmc_probe(struct platform_device *pdev)
> +{
> +	struct spi_controller *ctrl;
> +	struct sophgo_spifmc *spifmc;
> +	void __iomem *base;
> +	int ret;
> +
> +	ctrl = devm_spi_alloc_host(&pdev->dev, sizeof(*spifmc));
> +	if (!ctrl)
> +		return -ENOMEM;
> +
> +	spifmc = spi_controller_get_devdata(ctrl);
> +	dev_set_drvdata(&pdev->dev, ctrl);
> +
..
> +	spifmc->clk = devm_clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(spifmc->clk)) {
> +		dev_err(&pdev->dev, "AHB clock not found.\n");
> +		return PTR_ERR(spifmc->clk);
> +	}
> +
> +	ret = clk_prepare_enable(spifmc->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
> +		return ret;
> +	}
you can combine above with devm_clk_get_enabled(), and simplify 
return routine by using "return dev_err_probe(..)"

> +
> +	spifmc->dev = &pdev->dev;
> +	spifmc->ctrl = ctrl;
> +
> +	spifmc->io_base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	ctrl->num_chipselect = 1;
> +	ctrl->dev.of_node = pdev->dev.of_node;
> +	ctrl->bits_per_word_mask = SPI_BPW_MASK(8);
> +	ctrl->auto_runtime_pm = false;
> +	ctrl->mem_ops = &sophgo_spifmc_mem_ops;
> +	ctrl->mode_bits = SPI_RX_DUAL | SPI_TX_DUAL | SPI_RX_QUAD | SPI_TX_QUAD;
> +
> +	mutex_init(&spifmc->lock);
strictly, you still need to do error handler, e.g, destroy mutex if probe fail
> +
> +	sophgo_spifmc_init(spifmc);
> +	sophgo_spifmc_init_reg(spifmc);
> +
> +	return devm_spi_register_controller(&pdev->dev, ctrl);
> +}
> +
> +static void sophgo_spifmc_remove(struct platform_device *pdev)
> +{
> +	struct sophgo_spifmc *spifmc = platform_get_drvdata(pdev);
> +
> +	mutex_destroy(&spifmc->lock);
> +}
> +
> +static const struct of_device_id sophgo_spifmc_match[] = {
> +	{ .compatible = "sophgo,sg2044-spifmc-nor" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, sophgo_spifmc_match);
> +
> +static struct platform_driver sophgo_nor_driver = {
> +	.driver = {
> +		.name = "sophgo,spifmc-nor",
> +		.of_match_table = sophgo_spifmc_match,
> +	},
> +	.probe = sophgo_spifmc_probe,
> +	.remove = sophgo_spifmc_remove,
> +};
..
> +
spurious blank line here, drop it
> +module_platform_driver(sophgo_nor_driver);
> +
> +MODULE_DESCRIPTION("Sophgo SPI NOR controller driver");
> +MODULE_AUTHOR("Longbin Li <looong.bin@gmail.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.48.1

-- 
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55

Re: [PATCH 2/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Mark Brown]

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

On Mon, Feb 24, 2025 at 01:21:15PM +0000, Yixun Lan wrote:
> Hi Longbin:
> 
> On 18:12 Mon 24 Feb     , Longbin Li wrote:
> > Add support for Sophgo SPI NOR controller in Sophgo SoC.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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

Re: [PATCH 1/3] dt-bindings: spi: add Sophgo SPI NOR controller driver

[Rob Herring (Arm)]

On Mon, 24 Feb 2025 18:12:00 +0800, Longbin Li wrote:
> Add SPI NOR driver for Sophgo, including read, write operations.
> 
> Signed-off-by: Longbin Li <looong.bin@gmail.com>
> ---
>  .../bindings/spi/spi-sophgo-nor.yaml          | 52 +++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH 0/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Chen Wang]

hi, Longbin,

I think this patch is only for SG2044, right? However, the file name you 
used and the description in the patch all use "sophgo" directly, which 
makes it seem that it applies to all sophgo products. I suggest you 
correct this, otherwise there will be conflicts when other sophgo 
products such as SG2042 submit patches to SPIFMC.

Regards,

Chen

On 2025/2/24 18:11, Longbin Li wrote:
> Add SPI NOR driver for Sophgo, including read, write operations.
> This driver is only suitable for NOR flash.
>
> Longbin Li (3):
>    dt-bindings: spi: add Sophgo SPI NOR controller driver
>    spi: sophgo: add Sophgo SPI NOR controller driver
>    riscv: dts: sophgo: add Sophgo SPI NOR controller driver
>
>   .../bindings/spi/spi-sophgo-nor.yaml          |  52 ++
>   .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  |  18 +
>   arch/riscv/boot/dts/sophgo/sg2044.dtsi        |  24 +
>   drivers/spi/Kconfig                           |   9 +
>   drivers/spi/Makefile                          |   1 +
>   drivers/spi/spi-sophgo-nor.c                  | 501 ++++++++++++++++++
>   6 files changed, 605 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/spi/spi-sophgo-nor.yaml
>   create mode 100644 drivers/spi/spi-sophgo-nor.c
>
> --
> 2.48.1

Re: [PATCH 3/3] riscv: dts: sophgo: add Sophgo SPI NOR controller driver

[Chen Wang]

On 2025/2/24 18:12, Longbin Li wrote:
> Add SPI NOR device node for Sophgo.
>
> Signed-off-by: Longbin Li <looong.bin@gmail.com>
> ---
>   .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  | 18 ++++++++++++++
>   arch/riscv/boot/dts/sophgo/sg2044.dtsi        | 24 +++++++++++++++++++
>   2 files changed, 42 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
> index c50e61a50013..9c634920f37e 100644
> --- a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
> +++ b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
> @@ -80,6 +80,24 @@ &sd {
>   	status = "okay";
>   };
>
> +&spifmc0 {
> +	status = "okay";
> +
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +	};
> +};
> +
> +&spifmc1 {
> +	status = "okay";
> +
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +	};
> +};
> +
>   &uart1 {
>   	status = "okay";
>   };

What is your baseline? I can't see "sg2044-sophgo-sd3-10.dts " on the 
latest mainline yet.

BTW,  the name "sg2044-sophgo-sd3-10" seems a bit redundant. Wouldn't 
"sg2044-sd3-10" be better?

[......]

Re: [PATCH 3/3] riscv: dts: sophgo: add Sophgo SPI NOR controller driver

[Inochi Amaoto]

On Tue, Feb 25, 2025 at 08:23:36AM +0800, Chen Wang wrote:
> 
> On 2025/2/24 18:12, Longbin Li wrote:
> > Add SPI NOR device node for Sophgo.
> > 
> > Signed-off-by: Longbin Li <looong.bin@gmail.com>
> > ---
> >   .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  | 18 ++++++++++++++
> >   arch/riscv/boot/dts/sophgo/sg2044.dtsi        | 24 +++++++++++++++++++
> >   2 files changed, 42 insertions(+)
> > 
> > diff --git a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
> > index c50e61a50013..9c634920f37e 100644
> > --- a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
> > +++ b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
> > @@ -80,6 +80,24 @@ &sd {
> >   	status = "okay";
> >   };
> > 
> > +&spifmc0 {
> > +	status = "okay";
> > +
> > +	flash@0 {
> > +		compatible = "jedec,spi-nor";
> > +		reg = <0>;
> > +	};
> > +};
> > +
> > +&spifmc1 {
> > +	status = "okay";
> > +
> > +	flash@0 {
> > +		compatible = "jedec,spi-nor";
> > +		reg = <0>;
> > +	};
> > +};
> > +
> >   &uart1 {
> >   	status = "okay";
> >   };
> 
> What is your baseline? I can't see "sg2044-sophgo-sd3-10.dts " on the latest
> mainline yet.
> 

It seems like he forgot to remove this patch as the baseline is not
submitted.

> BTW,  the name "sg2044-sophgo-sd3-10" seems a bit redundant. Wouldn't
> "sg2044-sd3-10" be better?
> 

I do not think so. we want <soc-vendor-board> format. In this format.
sophgo is a must.

Regards,
Inochi

Re: [PATCH 2/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Chen Wang]

As I mention in anther comment, please use "SG2044" instead of "Sophgo"

On 2025/2/24 18:12, Longbin Li wrote:
> Add support for Sophgo SPI NOR controller in Sophgo SoC.
s/Sophgo/SG2044
>
> Signed-off-by: Longbin Li <looong.bin@gmail.com>
> ---
>   drivers/spi/Kconfig          |   9 +
>   drivers/spi/Makefile         |   1 +
>   drivers/spi/spi-sophgo-nor.c | 501 +++++++++++++++++++++++++++++++++++
spi-sophgo-nor.c -> spi-sg2044-nor.c
>   3 files changed, 511 insertions(+)
>   create mode 100644 drivers/spi/spi-sophgo-nor.c
>
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index ea8a31032927..6b6d7b348485 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -1021,6 +1021,15 @@ config SPI_SN_F_OSPI
>   	  for connecting an SPI Flash memory over up to 8-bit wide bus.
>   	  It supports indirect access mode only.
>
> +config SPI_SOPHGO_NOR
SPI_SOPHGO_NOR -> SPI_SG2044_NOR
> +	tristate "Sophgo SPI NOR Controller"
> +	depends on ARCH_SOPHGO || COMPILE_TEST
> +	help
> +	  This enables support for the Sophgo SPI NOR controller,
> +	  which supports Dual/Qual read and write operations while
> +	  also supporting 3Byte address devices and 4Byte address
> +	  devices.
> +
>   config SPI_SPRD
>   	tristate "Spreadtrum SPI controller"
>   	depends on ARCH_SPRD || COMPILE_TEST
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 9db7554c1864..9ded1de4b2fd 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -134,6 +134,7 @@ obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
>   obj-$(CONFIG_SPI_SIFIVE)		+= spi-sifive.o
>   obj-$(CONFIG_SPI_SLAVE_MT27XX)          += spi-slave-mt27xx.o
>   obj-$(CONFIG_SPI_SN_F_OSPI)		+= spi-sn-f-ospi.o
> +obj-$(CONFIG_SPI_SOPHGO_NOR)	+= spi-sophgo-nor.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-sophgo-nor.c b/drivers/spi/spi-sophgo-nor.c
> new file mode 100644
> index 000000000000..1139deeac327
> --- /dev/null
> +++ b/drivers/spi/spi-sophgo-nor.c
> @@ -0,0 +1,501 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Sophgo SPI NOR controller driver
> + *
> + * Copyright (c) 2025 Longbin Li <looong.bin@gmail.com>
> + */
> +
> +#include <linux/bitfield.h>
> +#include <linux/clk.h>
> +#include <linux/iopoll.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/spi/spi-mem.h>
> +
> +/* Hardware register definitions */
> +#define SPIFMC_CTRL				0x00
> +#define SPIFMC_CTRL_CPHA			BIT(12)
> +#define SPIFMC_CTRL_CPOL			BIT(13)
> +#define SPIFMC_CTRL_HOLD_OL			BIT(14)
> +#define SPIFMC_CTRL_WP_OL			BIT(15)
> +#define SPIFMC_CTRL_LSBF			BIT(20)
> +#define SPIFMC_CTRL_SRST			BIT(21)
> +#define SPIFMC_CTRL_SCK_DIV_SHIFT		0
> +#define SPIFMC_CTRL_FRAME_LEN_SHIFT		16
> +#define SPIFMC_CTRL_SCK_DIV_MASK		0x7FF
> +
> +#define SPIFMC_CE_CTRL				0x04
> +#define SPIFMC_CE_CTRL_CEMANUAL			BIT(0)
> +#define SPIFMC_CE_CTRL_CEMANUAL_EN		BIT(1)
> +
> +#define SPIFMC_DLY_CTRL				0x08
> +#define SPIFMC_CTRL_FM_INTVL_MASK		0x000f
> +#define SPIFMC_CTRL_FM_INTVL			BIT(0)
> +#define SPIFMC_CTRL_CET_MASK			0x0f00
> +#define SPIFMC_CTRL_CET				BIT(8)
> +
> +#define SPIFMC_DMMR				0x0c
> +
> +#define SPIFMC_TRAN_CSR				0x10
> +#define SPIFMC_TRAN_CSR_TRAN_MODE_MASK		GENMASK(1, 0)
> +#define SPIFMC_TRAN_CSR_TRAN_MODE_RX		BIT(0)
> +#define SPIFMC_TRAN_CSR_TRAN_MODE_TX		BIT(1)
> +#define SPIFMC_TRAN_CSR_FAST_MODE		BIT(3)
> +#define SPIFMC_TRAN_CSR_BUS_WIDTH_1_BIT		(0x00 << 4)
> +#define SPIFMC_TRAN_CSR_BUS_WIDTH_2_BIT		(0x01 << 4)
> +#define SPIFMC_TRAN_CSR_BUS_WIDTH_4_BIT		(0x02 << 4)
> +#define SPIFMC_TRAN_CSR_DMA_EN			BIT(6)
> +#define SPIFMC_TRAN_CSR_MISO_LEVEL		BIT(7)
> +#define SPIFMC_TRAN_CSR_ADDR_BYTES_MASK		GENMASK(10, 8)
> +#define SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT	8
> +#define SPIFMC_TRAN_CSR_WITH_CMD		BIT(11)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_MASK	GENMASK(13, 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE	(0x00 << 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_2_BYTE	(0x01 << 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_4_BYTE	(0x02 << 12)
> +#define SPIFMC_TRAN_CSR_FIFO_TRG_LVL_8_BYTE	(0x03 << 12)
> +#define SPIFMC_TRAN_CSR_GO_BUSY			BIT(15)
> +#define SPIFMC_TRAN_CSR_ADDR4B_SHIFT		20
> +#define SPIFMC_TRAN_CSR_CMD4B_SHIFT		21
> +
> +#define SPIFMC_TRAN_NUM				0x14
> +#define SPIFMC_FIFO_PORT			0x18
> +#define SPIFMC_FIFO_PT				0x20
> +
> +#define SPIFMC_INT_STS				0x28
> +#define SPIFMC_INT_TRAN_DONE			BIT(0)
> +#define SPIFMC_INT_RD_FIFO			BIT(2)
> +#define SPIFMC_INT_WR_FIFO			BIT(3)
> +#define SPIFMC_INT_RX_FRAME			BIT(4)
> +#define SPIFMC_INT_TX_FRAME			BIT(5)
> +
> +#define SPIFMC_INT_EN				0x2c
> +#define SPIFMC_INT_TRAN_DONE_EN			BIT(0)
> +#define SPIFMC_INT_RD_FIFO_EN			BIT(2)
> +#define SPIFMC_INT_WR_FIFO_EN			BIT(3)
> +#define SPIFMC_INT_RX_FRAME_EN			BIT(4)
> +#define SPIFMC_INT_TX_FRAME_EN			BIT(5)
> +
> +#define SPIFMC_OPT				0x030
> +#define SPIFMC_OPT_DISABLE_FIFO_FLUSH		BIT(1)
> +
> +#define SPIFMC_MAX_FIFO_DEPTH			8
> +
> +#define SPIFMC_MAX_READ_SIZE			0x10000
> +
> +struct sophgo_spifmc {
> +	struct spi_controller *ctrl;
> +	void __iomem *io_base;
> +	struct device *dev;
> +	struct mutex lock;
> +	struct clk *clk;
> +};

"sophgo_" -> "sg2044_", for structure and function names, ...

[......]

> +
> +MODULE_DESCRIPTION("Sophgo SPI NOR controller driver");
Sophgo -> SG2044
> +MODULE_AUTHOR("Longbin Li <looong.bin@gmail.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.48.1

Re: [PATCH 3/3] riscv: dts: sophgo: add Sophgo SPI NOR controller driver

[Chen Wang]

On 2025/2/25 8:38, Inochi Amaoto wrote:
> On Tue, Feb 25, 2025 at 08:23:36AM +0800, Chen Wang wrote:
>> On 2025/2/24 18:12, Longbin Li wrote:
>>> Add SPI NOR device node for Sophgo.
>>>
>>> Signed-off-by: Longbin Li <looong.bin@gmail.com>
>>> ---
>>>    .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  | 18 ++++++++++++++
>>>    arch/riscv/boot/dts/sophgo/sg2044.dtsi        | 24 +++++++++++++++++++
>>>    2 files changed, 42 insertions(+)
>>>
>>> diff --git a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
>>> index c50e61a50013..9c634920f37e 100644
>>> --- a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
>>> +++ b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
>>> @@ -80,6 +80,24 @@ &sd {
>>>    	status = "okay";
>>>    };
>>>
>>> +&spifmc0 {
>>> +	status = "okay";
>>> +
>>> +	flash@0 {
>>> +		compatible = "jedec,spi-nor";
>>> +		reg = <0>;
>>> +	};
>>> +};
>>> +
>>> +&spifmc1 {
>>> +	status = "okay";
>>> +
>>> +	flash@0 {
>>> +		compatible = "jedec,spi-nor";
>>> +		reg = <0>;
>>> +	};
>>> +};
>>> +
>>>    &uart1 {
>>>    	status = "okay";
>>>    };
>> What is your baseline? I can't see "sg2044-sophgo-sd3-10.dts " on the latest
>> mainline yet.
>>
> It seems like he forgot to remove this patch as the baseline is not
> submitted.
>
>> BTW,  the name "sg2044-sophgo-sd3-10" seems a bit redundant. Wouldn't
>> "sg2044-sd3-10" be better?
>>
> I do not think so. we want <soc-vendor-board> format. In this format.
> sophgo is a must.

All these dts* files are under "sophgo" folder, that's why I think 
"sophgo" is redundant for file name.

Anyway, if you do WANT the format, I won't object strongly.

>
> Regards,
> Inochi

Re: [PATCH 3/3] riscv: dts: sophgo: add Sophgo SPI NOR controller driver

[Chen Wang]

hi, Longbin,

sg2044.dtsi  seems also does not exist on the master yet, please double 
check.

Regards,

Chen

On 2025/2/25 8:23, Chen Wang wrote:
>
> On 2025/2/24 18:12, Longbin Li wrote:
>> Add SPI NOR device node for Sophgo.
>>
>> Signed-off-by: Longbin Li <looong.bin@gmail.com>
>> ---
>>   .../boot/dts/sophgo/sg2044-sophgo-sd3-10.dts  | 18 ++++++++++++++
>>   arch/riscv/boot/dts/sophgo/sg2044.dtsi        | 24 +++++++++++++++++++
>>   2 files changed, 42 insertions(+)
>>
>> diff --git a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts 
>> b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
>> index c50e61a50013..9c634920f37e 100644
>> --- a/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
>> +++ b/arch/riscv/boot/dts/sophgo/sg2044-sophgo-sd3-10.dts
>> @@ -80,6 +80,24 @@ &sd {
>>       status = "okay";
>>   };
>>
>> +&spifmc0 {
>> +    status = "okay";
>> +
>> +    flash@0 {
>> +        compatible = "jedec,spi-nor";
>> +        reg = <0>;
>> +    };
>> +};
>> +
>> +&spifmc1 {
>> +    status = "okay";
>> +
>> +    flash@0 {
>> +        compatible = "jedec,spi-nor";
>> +        reg = <0>;
>> +    };
>> +};
>> +
>>   &uart1 {
>>       status = "okay";
>>   };
>
> What is your baseline? I can't see "sg2044-sophgo-sd3-10.dts " on the 
> latest mainline yet.
>
> BTW,  the name "sg2044-sophgo-sd3-10" seems a bit redundant. Wouldn't 
> "sg2044-sd3-10" be better?
>
> [......]
>
>

Re: [PATCH 2/3] spi: sophgo: add Sophgo SPI NOR controller driver

[Longbin Li]

On Mon, Feb 24, 2025 at 01:21:15PM +0000, Yixun Lan wrote:
> Hi Longbin:
> 
> On 18:12 Mon 24 Feb     , Longbin Li wrote:
> > Add support for Sophgo SPI NOR controller in Sophgo SoC.
> > 
> > Signed-off-by: Longbin Li <looong.bin@gmail.com>
> > ---
> >  drivers/spi/Kconfig          |   9 +
> >  drivers/spi/Makefile         |   1 +
> >  drivers/spi/spi-sophgo-nor.c | 501 +++++++++++++++++++++++++++++++++++
> >  3 files changed, 511 insertions(+)
> >  create mode 100644 drivers/spi/spi-sophgo-nor.c
> > 
> > diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> > index ea8a31032927..6b6d7b348485 100644
> > --- a/drivers/spi/Kconfig
> > +++ b/drivers/spi/Kconfig
> > @@ -1021,6 +1021,15 @@ config SPI_SN_F_OSPI
> >  	  for connecting an SPI Flash memory over up to 8-bit wide bus.
> >  	  It supports indirect access mode only.
> > 
> > +config SPI_SOPHGO_NOR
> > +	tristate "Sophgo SPI NOR Controller"
> > +	depends on ARCH_SOPHGO || COMPILE_TEST
> > +	help
> > +	  This enables support for the Sophgo SPI NOR controller,
> > +	  which supports Dual/Qual read and write operations while
> > +	  also supporting 3Byte address devices and 4Byte address
> > +	  devices.
> > +
> >  config SPI_SPRD
> >  	tristate "Spreadtrum SPI controller"
> >  	depends on ARCH_SPRD || COMPILE_TEST
> > diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> > index 9db7554c1864..9ded1de4b2fd 100644
> > --- a/drivers/spi/Makefile
> > +++ b/drivers/spi/Makefile
> > @@ -134,6 +134,7 @@ obj-$(CONFIG_SPI_SH_SCI)		+= spi-sh-sci.o
> >  obj-$(CONFIG_SPI_SIFIVE)		+= spi-sifive.o
> >  obj-$(CONFIG_SPI_SLAVE_MT27XX)          += spi-slave-mt27xx.o
> >  obj-$(CONFIG_SPI_SN_F_OSPI)		+= spi-sn-f-ospi.o
> > +obj-$(CONFIG_SPI_SOPHGO_NOR)	+= spi-sophgo-nor.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-sophgo-nor.c b/drivers/spi/spi-sophgo-nor.c
> > new file mode 100644
> > index 000000000000..1139deeac327
> > --- /dev/null
> > +++ b/drivers/spi/spi-sophgo-nor.c
> > @@ -0,0 +1,501 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Sophgo SPI NOR controller driver
> > + *
> > + * Copyright (c) 2025 Longbin Li <looong.bin@gmail.com>
> > + */
> > +
> > [...]
> > +struct sophgo_spifmc {
> > +	struct spi_controller *ctrl;
> > +	void __iomem *io_base;
> > +	struct device *dev;
> > +	struct mutex lock;
> it will be great to document the lock

Thanks, but I don't think the function of this lock is complicated,
adding comments may be useless.

> > +	struct clk *clk;
> > +};
> > +
> > +static int sophgo_spifmc_wait_int(struct sophgo_spifmc *spifmc, u8 int_type)
> > +{
> > +	u32 stat;
> > +
> > +	return readl_poll_timeout(spifmc->io_base + SPIFMC_INT_STS, stat,
> > +				  (stat & int_type), 0, 1000000);
> > +}
> > +
> > +
> > [...]
> > +static ssize_t sophgo_spifmc_trans_reg(struct sophgo_spifmc *spifmc,
> > +				       const struct spi_mem_op *op)
> > +{
> > +	const u8 *dout = NULL;
> > +	u8 *din = NULL;
> > +	size_t len = op->data.nbytes;
> > +	u32 reg;
> > +	int ret;
> > +	int i;
> squash them which save one line:
> 	int i, ret;
> 

Thanks, I will modify it.

> > +
> > +	if (op->data.dir == SPI_MEM_DATA_IN)
> > +		din = op->data.buf.in;
> > +	else
> > +		dout = op->data.buf.out;
> > +
> > +	reg = sophgo_spifmc_init_reg(spifmc);
> > +	reg |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_1_BYTE;
> > +	reg |= SPIFMC_TRAN_CSR_WITH_CMD;
> > +
> > +	if (din) {
> > +		reg |= SPIFMC_TRAN_CSR_BUS_WIDTH_1_BIT;
> > +		reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
> > +		reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
> > +
> > +		writel(SPIFMC_OPT_DISABLE_FIFO_FLUSH, spifmc->io_base + SPIFMC_OPT);
> > +	} else {
> > +		/*
> > +		 * If write values to the Status Register,
> > +		 * configure TRAN_CSR register as the same as
> > +		 * sophgo_spifmc_read_reg.
> > +		 */
> > +		if (op->cmd.opcode == 0x01) {
> > +			reg |= SPIFMC_TRAN_CSR_TRAN_MODE_RX;
> > +			reg |= SPIFMC_TRAN_CSR_TRAN_MODE_TX;
> > +			writel(len, spifmc->io_base + SPIFMC_TRAN_NUM);
> > +		}
> > +	}
> > +
> > +
> > [...]
> > +static const struct spi_controller_mem_ops sophgo_spifmc_mem_ops = {
> > +	.exec_op = sophgo_spifmc_exec_op,
> > +};
> > +
> > +static void sophgo_spifmc_init(struct sophgo_spifmc *spifmc)
> > +{
> > +	u32 tran_csr;
> > +	u32 reg;
> > +
> > +	writel(0, spifmc->io_base + SPIFMC_DMMR);
> > +
> > +	reg = readl(spifmc->io_base + SPIFMC_CTRL);
> > +	reg |= SPIFMC_CTRL_SRST;
> ..
> > +	reg &= ~((1 << 11) - 1);
> so this is a mask? use macro to define, instead of using magic number

Yes, this is a mask to init SPI Clock Divider, I will add a macro, thanks.

> > +	reg |= 1;
> > +	writel(reg, spifmc->io_base + SPIFMC_CTRL);
> > +
> > +	writel(0, spifmc->io_base + SPIFMC_CE_CTRL);
> > +
> > +	tran_csr = readl(spifmc->io_base + SPIFMC_TRAN_CSR);
> > +	tran_csr |= (0 << SPIFMC_TRAN_CSR_ADDR_BYTES_SHIFT);
> > +	tran_csr |= SPIFMC_TRAN_CSR_FIFO_TRG_LVL_4_BYTE;
> > +	tran_csr |= SPIFMC_TRAN_CSR_WITH_CMD;
> > +	writel(tran_csr, spifmc->io_base + SPIFMC_TRAN_CSR);
> > +}
> > +
> > +static int sophgo_spifmc_probe(struct platform_device *pdev)
> > +{
> > +	struct spi_controller *ctrl;
> > +	struct sophgo_spifmc *spifmc;
> > +	void __iomem *base;
> > +	int ret;
> > +
> > +	ctrl = devm_spi_alloc_host(&pdev->dev, sizeof(*spifmc));
> > +	if (!ctrl)
> > +		return -ENOMEM;
> > +
> > +	spifmc = spi_controller_get_devdata(ctrl);
> > +	dev_set_drvdata(&pdev->dev, ctrl);
> > +
> ..
> > +	spifmc->clk = devm_clk_get(&pdev->dev, NULL);
> > +	if (IS_ERR(spifmc->clk)) {
> > +		dev_err(&pdev->dev, "AHB clock not found.\n");
> > +		return PTR_ERR(spifmc->clk);
> > +	}
> > +
> > +	ret = clk_prepare_enable(spifmc->clk);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "Unable to enable AHB clock.\n");
> > +		return ret;
> > +	}
> you can combine above with devm_clk_get_enabled(), and simplify 
> return routine by using "return dev_err_probe(..)"
> 

Thanks, I will modify it.

> > +
> > +	spifmc->dev = &pdev->dev;
> > +	spifmc->ctrl = ctrl;
> > +
> > +	spifmc->io_base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(base))
> > +		return PTR_ERR(base);
> > +
> > +	ctrl->num_chipselect = 1;
> > +	ctrl->dev.of_node = pdev->dev.of_node;
> > +	ctrl->bits_per_word_mask = SPI_BPW_MASK(8);
> > +	ctrl->auto_runtime_pm = false;
> > +	ctrl->mem_ops = &sophgo_spifmc_mem_ops;
> > +	ctrl->mode_bits = SPI_RX_DUAL | SPI_TX_DUAL | SPI_RX_QUAD | SPI_TX_QUAD;
> > +
> > +	mutex_init(&spifmc->lock);
> strictly, you still need to do error handler, e.g, destroy mutex if probe fail

Thanks, I will add it.

> > +
> > +	sophgo_spifmc_init(spifmc);
> > +	sophgo_spifmc_init_reg(spifmc);
> > +
> > +	return devm_spi_register_controller(&pdev->dev, ctrl);
> > +}
> > +
> > [...]
> 
> -- 
> Yixun Lan (dlan)
> Gentoo Linux Developer
> GPG Key ID AABEFD55