linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support
@ 2021-06-09 14:04 Jon Lin
  2021-06-09 14:04 ` [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller Jon Lin
                   ` (4 more replies)
  0 siblings, 5 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:04 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk



Changes in v7:
- Fix up the sclk_sfc parent error in rk3036
- Unify to "rockchip,sfc" compatible id because all the feature update
  will have a new IP version, so the driver is used for the SFC IP in
  all SoCs
- Change to use node "sfc" to name the SFC pinctrl group
- Add subnode reg property check
- Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
- Limit max_iosize to 32KB

Changes in v6:
- Add support in device trees for rv1126(Declared in series 5 but not
  submitted)
- Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
  affect interpretation and has been widely used
- Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
- Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
  in series 5 but not submitted)
- Support SFC ver4 ver5(Declared in series 5 but not submitted)
- Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
- Change to use devm_spi_alloc_master and spi_unregister_master

Changes in v5:
- Add support in device trees for rv1126
- Support sfc tx_dual, tx_quad
- Simplify the code, such as remove "rockchip_sfc_register_all"
- Support SFC ver4 ver5

Changes in v4:
- Changing patch back to an "RFC". An engineer from Rockchip
  reached out to me to let me know they are working on this patch for
  upstream, I am submitting this v4 for the community to see however
  I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
  soon and these are the ones we should pursue for mainlining. Jon's
  patch series should include support for more hardware than this
  series.
- Clean up documentation more and ensure it is correct per
  make dt_binding_check.
- Add support in device trees for rk3036, rk3308, and rv1108.
- Add ahb clock (hclk_sfc) support for rk3036.
- Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
- Change IRQ code to only mark IRQ as handled if it handles the
  specific IRQ (DMA transfer finish) it is supposed to handle.

Changes in v3:
- Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
- Changed the compatible string from rockchip,sfc to
  rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
  driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
  RV1108 SoCs, and possibly more. However, I am currently only able
  to test this on a PX30 (an RK3326). The technical reference manuals
  appear to list the same registers for each device.
- Corrected devicetree documentation for formatting and to note these
  changes.
- Replaced the maintainer with Heiko Stuebner and myself, as we will
  take ownership of this going forward.
- Noted that the device (per the reference manual) supports 4 CS, but
  I am only able to test a single CS (CS 0).
- Reordered patches to comply with upstream rules.

Changes in v2:
- Reimplemented driver using spi-mem subsystem.
- Removed power management code as I couldn't get it working properly.
- Added device tree bindings for Odroid Go Advance.

Changes in v1:
hanges made in this new series versus the v8 of the old series:
- Added function to read spi-rx-bus-width from device tree, in the
  event that the SPI chip supports 4x mode but only has 2 pins
  wired (such as the Odroid Go Advance).
- Changed device tree documentation from txt to yaml format.
- Made "reset" message a dev_dbg from a dev_info.
- Changed read and write fifo functions to remove redundant checks.
- Changed the write and read from relaxed to non-relaxed when
  starting the DMA transfer or reading the DMA IRQ.
- Changed from dma_coerce_mask_and_coherent to just
  dma_set_mask_and_coherent.
- Changed name of get_if_type to rockchip_sfc_get_if_type.

Chris Morgan (8):
  dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash
    controller
  spi: rockchip-sfc: add rockchip serial flash controller
  arm64: dts: rockchip: Add SFC to PX30
  clk: rockchip: Add support for hclk_sfc on rk3036
  arm: dts: rockchip: Add SFC to RK3036
  arm: dts: rockchip: Add SFC to RV1108
  arm64: dts: rockchip: Add SFC to RK3308
  arm64: dts: rockchip: Enable SFC for Odroid Go Advance

Jon Lin (1):
  clk: rockchip: rk3036: fix up the sclk_sfc parent error

 .../devicetree/bindings/spi/rockchip-sfc.yaml |  88 +++
 arch/arm/boot/dts/rk3036.dtsi                 |  42 ++
 arch/arm/boot/dts/rv1108.dtsi                 |  37 +
 arch/arm64/boot/dts/rockchip/px30.dtsi        |  38 +
 arch/arm64/boot/dts/rockchip/rk3308.dtsi      |  37 +
 .../boot/dts/rockchip/rk3326-odroid-go2.dts   |  16 +
 drivers/clk/rockchip/clk-rk3036.c             |   5 +-
 drivers/spi/Kconfig                           |   9 +
 drivers/spi/Makefile                          |   1 +
 drivers/spi/spi-rockchip-sfc.c                | 676 ++++++++++++++++++
 include/dt-bindings/clock/rk3036-cru.h        |   1 +
 11 files changed, 948 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
 create mode 100644 drivers/spi/spi-rockchip-sfc.c

-- 
2.17.1




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

* [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-09 14:04 [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support Jon Lin
@ 2021-06-09 14:04 ` Jon Lin
  2021-06-09 16:16   ` Rob Herring
  2021-06-10  2:43   ` Rob Herring
  2021-06-09 14:04 ` [PATCH v7 2/9] spi: rockchip-sfc: add rockchip " Jon Lin
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:04 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add bindings for the Rockchip serial flash controller. New device
specific parameter of rockchip,sfc-no-dma included in documentation.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7:
- Fix up the sclk_sfc parent error in rk3036
- Unify to "rockchip,sfc" compatible id because all the feature update
  will have a new IP version, so the driver is used for the SFC IP in
  all SoCs
- Change to use node "sfc" to name the SFC pinctrl group
- Add subnode reg property check
- Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
- Limit max_iosize to 32KB

Changes in v6:
- Add support in device trees for rv1126(Declared in series 5 but not
  submitted)
- Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
  affect interpretation and has been widely used
- Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
- Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
  in series 5 but not submitted)
- Support SFC ver4 ver5(Declared in series 5 but not submitted)
- Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
- Change to use devm_spi_alloc_master and spi_unregister_master

Changes in v5:
- Add support in device trees for rv1126
- Support sfc tx_dual, tx_quad
- Simplify the code, such as remove "rockchip_sfc_register_all"
- Support SFC ver4 ver5

Changes in v4:
- Changing patch back to an "RFC". An engineer from Rockchip
  reached out to me to let me know they are working on this patch for
  upstream, I am submitting this v4 for the community to see however
  I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
  soon and these are the ones we should pursue for mainlining. Jon's
  patch series should include support for more hardware than this
  series.
- Clean up documentation more and ensure it is correct per
  make dt_binding_check.
- Add support in device trees for rk3036, rk3308, and rv1108.
- Add ahb clock (hclk_sfc) support for rk3036.
- Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
- Change IRQ code to only mark IRQ as handled if it handles the
  specific IRQ (DMA transfer finish) it is supposed to handle.

Changes in v3:
- Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
- Changed the compatible string from rockchip,sfc to
  rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
  driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
  RV1108 SoCs, and possibly more. However, I am currently only able
  to test this on a PX30 (an RK3326). The technical reference manuals
  appear to list the same registers for each device.
- Corrected devicetree documentation for formatting and to note these
  changes.
- Replaced the maintainer with Heiko Stuebner and myself, as we will
  take ownership of this going forward.
- Noted that the device (per the reference manual) supports 4 CS, but
  I am only able to test a single CS (CS 0).
- Reordered patches to comply with upstream rules.

Changes in v2:
- Reimplemented driver using spi-mem subsystem.
- Removed power management code as I couldn't get it working properly.
- Added device tree bindings for Odroid Go Advance.

Changes in v1:
hanges made in this new series versus the v8 of the old series:
- Added function to read spi-rx-bus-width from device tree, in the
  event that the SPI chip supports 4x mode but only has 2 pins
  wired (such as the Odroid Go Advance).
- Changed device tree documentation from txt to yaml format.
- Made "reset" message a dev_dbg from a dev_info.
- Changed read and write fifo functions to remove redundant checks.
- Changed the write and read from relaxed to non-relaxed when
  starting the DMA transfer or reading the DMA IRQ.
- Changed from dma_coerce_mask_and_coherent to just
  dma_set_mask_and_coherent.
- Changed name of get_if_type to rockchip_sfc_get_if_type.

 .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
 1 file changed, 88 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml

diff --git a/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
new file mode 100644
index 000000000000..42e4198e92af
--- /dev/null
+++ b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/spi/rockchip-sfc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip Serial Flash Controller (SFC)
+
+maintainers:
+  - Heiko Stuebner <heiko@sntech.de>
+  - Chris Morgan <macromorgan@hotmail.com>
+
+allOf:
+  - $ref: spi-controller.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - const: rockchip,sfc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: Bus Clock
+      - description: Module Clock
+
+  clock-names:
+    items:
+      - const: clk_sfc
+      - const: hclk_sfc
+
+  power-domains:
+    maxItems: 1
+
+  rockchip,sfc-no-dma:
+    description: Disable DMA and utilize FIFO mode only
+    type: boolean
+
+patternProperties:
+    "^flash@[0-3]$":
+      type: object
+      properties:
+        reg:
+          minimum: 0
+          maximum: 3
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/px30-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/px30-power.h>
+
+    sfc: spi@ff3a0000 {
+        compatible = "rockchip,sfc";
+        reg = <0xff3a0000 0x4000>;
+        interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+        clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+        clock-names = "clk_sfc", "hclk_sfc";
+        pinctrl-0 = <&sfc_clk &sfc_cs &sfc_bus2>;
+        pinctrl-names = "default";
+        power-domains = <&power PX30_PD_MMC_NAND>;
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        flash@0 {
+            compatible = "jedec,spi-nor";
+            reg = <0>;
+            spi-max-frequency = <108000000>;
+            spi-rx-bus-width = <2>;
+            spi-tx-bus-width = <2>;
+        };
+    };
+
+...
-- 
2.17.1




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

* [PATCH v7 2/9] spi: rockchip-sfc: add rockchip serial flash controller
  2021-06-09 14:04 [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support Jon Lin
  2021-06-09 14:04 ` [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller Jon Lin
@ 2021-06-09 14:04 ` Jon Lin
  2021-06-09 18:45   ` Chris Morgan
  2021-06-09 14:04 ` [PATCH v7 3/9] arm64: dts: rockchip: Add SFC to PX30 Jon Lin
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:04 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add the rockchip serial flash controller (SFC) driver.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 drivers/spi/Kconfig            |   9 +
 drivers/spi/Makefile           |   1 +
 drivers/spi/spi-rockchip-sfc.c | 676 +++++++++++++++++++++++++++++++++
 3 files changed, 686 insertions(+)
 create mode 100644 drivers/spi/spi-rockchip-sfc.c

diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
index e71a4c514f7b..d89e5f3c9107 100644
--- a/drivers/spi/Kconfig
+++ b/drivers/spi/Kconfig
@@ -658,6 +658,15 @@ config SPI_ROCKCHIP
 	  The main usecase of this controller is to use spi flash as boot
 	  device.
 
+config SPI_ROCKCHIP_SFC
+	tristate "Rockchip Serial Flash Controller (SFC)"
+	depends on ARCH_ROCKCHIP || COMPILE_TEST
+	depends on HAS_IOMEM && HAS_DMA
+	help
+	  This enables support for Rockchip serial flash controller. This
+	  is a specialized controller used to access SPI flash on some
+	  Rockchip SOCs.
+
 config SPI_RB4XX
 	tristate "Mikrotik RB4XX SPI master"
 	depends on SPI_MASTER && ATH79
diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
index 13e54c45e9df..699db95c8441 100644
--- a/drivers/spi/Makefile
+++ b/drivers/spi/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_SPI_QCOM_GENI)		+= spi-geni-qcom.o
 obj-$(CONFIG_SPI_QCOM_QSPI)		+= spi-qcom-qspi.o
 obj-$(CONFIG_SPI_QUP)			+= spi-qup.o
 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_RPCIF)			+= spi-rpc-if.o
diff --git a/drivers/spi/spi-rockchip-sfc.c b/drivers/spi/spi-rockchip-sfc.c
new file mode 100644
index 000000000000..be2557c9af40
--- /dev/null
+++ b/drivers/spi/spi-rockchip-sfc.c
@@ -0,0 +1,676 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Rockchip Serial Flash Controller Driver
+ *
+ * Copyright (c) 2017-2021, Rockchip Inc.
+ * Author: Shawn Lin <shawn.lin@rock-chips.com>
+ *	   Chris Morgan <macroalpha82@gmail.com>
+ *	   Jon Lin <Jon.lin@rock-chips.com>
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/dma-mapping.h>
+#include <linux/iopoll.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/interrupt.h>
+#include <linux/spi/spi-mem.h>
+
+/* System control */
+#define SFC_CTRL			0x0
+#define  SFC_CTRL_PHASE_SEL_NEGETIVE	BIT(1)
+#define  SFC_CTRL_CMD_BITS_SHIFT	8
+#define  SFC_CTRL_ADDR_BITS_SHIFT	10
+#define  SFC_CTRL_DATA_BITS_SHIFT	12
+
+/* Interrupt mask */
+#define SFC_IMR				0x4
+#define  SFC_IMR_RX_FULL		BIT(0)
+#define  SFC_IMR_RX_UFLOW		BIT(1)
+#define  SFC_IMR_TX_OFLOW		BIT(2)
+#define  SFC_IMR_TX_EMPTY		BIT(3)
+#define  SFC_IMR_TRAN_FINISH		BIT(4)
+#define  SFC_IMR_BUS_ERR		BIT(5)
+#define  SFC_IMR_NSPI_ERR		BIT(6)
+#define  SFC_IMR_DMA			BIT(7)
+
+/* Interrupt clear */
+#define SFC_ICLR			0x8
+#define  SFC_ICLR_RX_FULL		BIT(0)
+#define  SFC_ICLR_RX_UFLOW		BIT(1)
+#define  SFC_ICLR_TX_OFLOW		BIT(2)
+#define  SFC_ICLR_TX_EMPTY		BIT(3)
+#define  SFC_ICLR_TRAN_FINISH		BIT(4)
+#define  SFC_ICLR_BUS_ERR		BIT(5)
+#define  SFC_ICLR_NSPI_ERR		BIT(6)
+#define  SFC_ICLR_DMA			BIT(7)
+
+/* FIFO threshold level */
+#define SFC_FTLR			0xc
+#define  SFC_FTLR_TX_SHIFT		0
+#define  SFC_FTLR_TX_MASK		0x1f
+#define  SFC_FTLR_RX_SHIFT		8
+#define  SFC_FTLR_RX_MASK		0x1f
+
+/* Reset FSM and FIFO */
+#define SFC_RCVR			0x10
+#define  SFC_RCVR_RESET			BIT(0)
+
+/* Enhanced mode */
+#define SFC_AX				0x14
+
+/* Address Bit number */
+#define SFC_ABIT			0x18
+
+/* Interrupt status */
+#define SFC_ISR				0x1c
+#define  SFC_ISR_RX_FULL_SHIFT		BIT(0)
+#define  SFC_ISR_RX_UFLOW_SHIFT		BIT(1)
+#define  SFC_ISR_TX_OFLOW_SHIFT		BIT(2)
+#define  SFC_ISR_TX_EMPTY_SHIFT		BIT(3)
+#define  SFC_ISR_TX_FINISH_SHIFT	BIT(4)
+#define  SFC_ISR_BUS_ERR_SHIFT		BIT(5)
+#define  SFC_ISR_NSPI_ERR_SHIFT		BIT(6)
+#define  SFC_ISR_DMA_SHIFT		BIT(7)
+
+/* FIFO status */
+#define SFC_FSR				0x20
+#define  SFC_FSR_TX_IS_FULL		BIT(0)
+#define  SFC_FSR_TX_IS_EMPTY		BIT(1)
+#define  SFC_FSR_RX_IS_EMPTY		BIT(2)
+#define  SFC_FSR_RX_IS_FULL		BIT(3)
+#define  SFC_FSR_TXLV_MASK		GENMASK(12, 8)
+#define  SFC_FSR_TXLV_SHIFT		8
+#define  SFC_FSR_RXLV_MASK		GENMASK(20, 16)
+#define  SFC_FSR_RXLV_SHIFT		16
+
+/* FSM status */
+#define SFC_SR				0x24
+#define  SFC_SR_IS_IDLE			0x0
+#define  SFC_SR_IS_BUSY			0x1
+
+/* Raw interrupt status */
+#define SFC_RISR			0x28
+#define  SFC_RISR_RX_FULL		BIT(0)
+#define  SFC_RISR_RX_UNDERFLOW		BIT(1)
+#define  SFC_RISR_TX_OVERFLOW		BIT(2)
+#define  SFC_RISR_TX_EMPTY		BIT(3)
+#define  SFC_RISR_TRAN_FINISH		BIT(4)
+#define  SFC_RISR_BUS_ERR		BIT(5)
+#define  SFC_RISR_NSPI_ERR		BIT(6)
+#define  SFC_RISR_DMA			BIT(7)
+
+/* Version */
+#define SFC_VER				0x2C
+#define  SFC_VER_3			0x3
+#define  SFC_VER_4			0x4
+#define  SFC_VER_5			0x5
+
+/* Master trigger */
+#define SFC_DMA_TRIGGER			0x80
+
+/* Src or Dst addr for master */
+#define SFC_DMA_ADDR			0x84
+
+/* Length control register extension 32GB */
+#define SFC_LEN_CTRL			0x88
+#define SFC_LEN_CTRL_TRB_SEL		1
+#define SFC_LEN_EXT			0x8C
+
+/* Command */
+#define SFC_CMD				0x100
+#define  SFC_CMD_IDX_SHIFT		0
+#define  SFC_CMD_DUMMY_SHIFT		8
+#define  SFC_CMD_DIR_SHIFT		12
+#define  SFC_CMD_DIR_RD			0
+#define  SFC_CMD_DIR_WR			1
+#define  SFC_CMD_ADDR_SHIFT		14
+#define  SFC_CMD_ADDR_0BITS		0
+#define  SFC_CMD_ADDR_24BITS		1
+#define  SFC_CMD_ADDR_32BITS		2
+#define  SFC_CMD_ADDR_XBITS		3
+#define  SFC_CMD_TRAN_BYTES_SHIFT	16
+#define  SFC_CMD_CS_SHIFT		30
+
+/* Address */
+#define SFC_ADDR			0x104
+
+/* Data */
+#define SFC_DATA			0x108
+
+/* The controller and documentation reports that it supports up to 4 CS
+ * devices (0-3), however I have only been able to test a single CS (CS 0)
+ * due to the configuration of my device.
+ */
+#define SFC_MAX_CHIPSELECT_NUM		4
+
+/* The SFC can transfer max 16KB - 1 at one time
+ * we set it to 15.5KB here for alignment.
+ */
+#define SFC_MAX_IOSIZE_VER3		(512 * 31)
+
+/* DMA is only enabled for large data transmission */
+#define SFC_DMA_TRANS_THRETHOLD		(0x40)
+
+/* Maximum clock values from datasheet suggest keeping clock value under
+ * 150MHz. No minimum or average value is suggested, but the U-boot BSP driver
+ * has a minimum of 10MHz and a default of 80MHz which seems reasonable.
+ */
+#define SFC_MIN_SPEED_HZ		(10 * 1000 * 1000)
+#define SFC_DEFAULT_SPEED_HZ		(80 * 1000 * 1000)
+#define SFC_MAX_SPEED_HZ		(150 * 1000 * 1000)
+
+struct rockchip_sfc {
+	struct device *dev;
+	void __iomem *regbase;
+	struct clk *hclk;
+	struct clk *clk;
+	u32 frequency;
+	/* virtual mapped addr for dma_buffer */
+	void *buffer;
+	dma_addr_t dma_buffer;
+	struct completion cp;
+	bool use_dma;
+	u32 max_iosize;
+	u16 version;
+};
+
+static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
+{
+	int err;
+	u32 status;
+
+	writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
+
+	err = readl_poll_timeout(sfc->regbase + SFC_RCVR, status,
+				 !(status & SFC_RCVR_RESET), 20,
+				 jiffies_to_usecs(HZ));
+	if (err)
+		dev_err(sfc->dev, "SFC reset never finished\n");
+
+	/* Still need to clear the masked interrupt from RISR */
+	writel_relaxed(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
+
+	dev_dbg(sfc->dev, "reset\n");
+
+	return err;
+}
+
+static u16 rockchip_sfc_get_version(struct rockchip_sfc *sfc)
+{
+	return  (u16)(readl(sfc->regbase + SFC_VER) & 0xffff);
+}
+
+static u32 rockchip_sfc_get_max_iosize(struct rockchip_sfc *sfc)
+{
+	return SFC_MAX_IOSIZE_VER3;
+}
+
+static int rockchip_sfc_init(struct rockchip_sfc *sfc)
+{
+	writel(0, sfc->regbase + SFC_CTRL);
+	if (rockchip_sfc_get_version(sfc) >= SFC_VER_4)
+		writel(SFC_LEN_CTRL_TRB_SEL, sfc->regbase + SFC_LEN_CTRL);
+
+	return 0;
+}
+
+static inline int rockchip_sfc_get_fifo_level(struct rockchip_sfc *sfc, int wr)
+{
+	u32 fsr = readl_relaxed(sfc->regbase + SFC_FSR);
+	int level;
+
+	if (wr)
+		level = (fsr & SFC_FSR_TXLV_MASK) >> SFC_FSR_TXLV_SHIFT;
+	else
+		level = (fsr & SFC_FSR_RXLV_MASK) >> SFC_FSR_RXLV_SHIFT;
+
+	return level;
+}
+
+static int rockchip_sfc_wait_fifo_ready(struct rockchip_sfc *sfc, int wr, u32 timeout)
+{
+	unsigned long deadline = jiffies + timeout;
+	int level;
+
+	while (!(level = rockchip_sfc_get_fifo_level(sfc, wr))) {
+		if (time_after_eq(jiffies, deadline)) {
+			dev_warn(sfc->dev, "%s fifo timeout\n", wr ? "write" : "read");
+			return -ETIMEDOUT;
+		}
+		udelay(1);
+	}
+
+	return level;
+}
+
+static void rockchip_sfc_adjust_op_work(struct spi_mem_op *op)
+{
+	if (unlikely(op->dummy.nbytes && !op->addr.nbytes)) {
+		/*
+		 * SFC not support output DUMMY cycles right after CMD cycles, so
+		 * treat it as ADDR cycles.
+		 */
+		op->addr.nbytes = op->dummy.nbytes;
+		op->addr.buswidth = op->dummy.buswidth;
+		op->addr.val = 0xFFFFFFFFF;
+
+		op->dummy.nbytes = 0;
+	}
+}
+
+static int rockchip_sfc_xfer_setup(struct rockchip_sfc *sfc,
+				   struct spi_mem *mem,
+				   const struct spi_mem_op *op,
+				   u32 len)
+{
+	u32 ctrl = 0, cmd = 0;
+
+	/* set CMD */
+	cmd = op->cmd.opcode;
+	ctrl |= ((op->cmd.buswidth >> 1) << SFC_CTRL_CMD_BITS_SHIFT);
+
+	/* set ADDR */
+	if (op->addr.nbytes) {
+		if (op->addr.nbytes == 4) {
+			cmd |= SFC_CMD_ADDR_32BITS << SFC_CMD_ADDR_SHIFT;
+		} else if (op->addr.nbytes == 3) {
+			cmd |= SFC_CMD_ADDR_24BITS << SFC_CMD_ADDR_SHIFT;
+		} else {
+			cmd |= SFC_CMD_ADDR_XBITS << SFC_CMD_ADDR_SHIFT;
+			writel_relaxed(op->addr.nbytes * 8 - 1, sfc->regbase + SFC_ABIT);
+		}
+
+		ctrl |= ((op->addr.buswidth >> 1) << SFC_CTRL_ADDR_BITS_SHIFT);
+	}
+
+	/* set DUMMY */
+	if (op->dummy.nbytes) {
+		if (op->dummy.buswidth == 4)
+			cmd |= op->dummy.nbytes * 2 << SFC_CMD_DUMMY_SHIFT;
+		else if (op->dummy.buswidth == 2)
+			cmd |= op->dummy.nbytes * 4 << SFC_CMD_DUMMY_SHIFT;
+		else
+			cmd |= op->dummy.nbytes * 8 << SFC_CMD_DUMMY_SHIFT;
+	}
+
+	/* set DATA */
+	if (sfc->version >= SFC_VER_4) /* Clear it if no data to transfer */
+		writel(len, sfc->regbase + SFC_LEN_EXT);
+	else
+		cmd |= len << SFC_CMD_TRAN_BYTES_SHIFT;
+	if (len) {
+		if (op->data.dir == SPI_MEM_DATA_OUT)
+			cmd |= SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT;
+
+		ctrl |= ((op->data.buswidth >> 1) << SFC_CTRL_DATA_BITS_SHIFT);
+	}
+	if (!len && op->addr.nbytes)
+		cmd |= SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT;
+
+	/* set the Controller */
+	ctrl |= SFC_CTRL_PHASE_SEL_NEGETIVE;
+	cmd |= mem->spi->chip_select << SFC_CMD_CS_SHIFT;
+
+	dev_dbg(sfc->dev, "addr.nbytes=%x(x%d) dummy.nbytes=%x(x%d)\n",
+		op->addr.nbytes, op->addr.buswidth,
+		op->dummy.nbytes, op->dummy.buswidth);
+	dev_dbg(sfc->dev, "ctrl=%x cmd=%x addr=%llx len=%x\n",
+		ctrl, cmd, op->addr.val, len);
+
+	writel(ctrl, sfc->regbase + SFC_CTRL);
+	writel(cmd, sfc->regbase + SFC_CMD);
+	if (op->addr.nbytes)
+		writel(op->addr.val, sfc->regbase + SFC_ADDR);
+
+	return 0;
+}
+
+static int rockchip_sfc_write_fifo(struct rockchip_sfc *sfc, const u8 *buf, int len)
+{
+	u8 bytes = len & 0x3;
+	u32 dwords;
+	int tx_level;
+	u32 write_words;
+	u32 tmp = 0;
+
+	dwords = len >> 2;
+	while (dwords) {
+		tx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_WR, HZ);
+		if (tx_level < 0)
+			return tx_level;
+		write_words = min_t(u32, tx_level, dwords);
+		iowrite32_rep(sfc->regbase + SFC_DATA, buf, write_words);
+		buf += write_words << 2;
+		dwords -= write_words;
+	}
+
+	/* write the rest non word aligned bytes */
+	if (bytes) {
+		tx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_WR, HZ);
+		if (tx_level < 0)
+			return tx_level;
+		memcpy(&tmp, buf, bytes);
+		writel(tmp, sfc->regbase + SFC_DATA);
+	}
+
+	return len;
+}
+
+static int rockchip_sfc_read_fifo(struct rockchip_sfc *sfc, u8 *buf, int len)
+{
+	u8 bytes = len & 0x3;
+	u32 dwords;
+	u8 read_words;
+	int rx_level;
+	int tmp;
+
+	/* word aligned access only */
+	dwords = len >> 2;
+	while (dwords) {
+		rx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_RD, HZ);
+		if (rx_level < 0)
+			return rx_level;
+		read_words = min_t(u32, rx_level, dwords);
+		ioread32_rep(sfc->regbase + SFC_DATA, buf, read_words);
+		buf += read_words << 2;
+		dwords -= read_words;
+	}
+
+	/* read the rest non word aligned bytes */
+	if (bytes) {
+		rx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_RD, HZ);
+		if (rx_level < 0)
+			return rx_level;
+		tmp = readl_relaxed(sfc->regbase + SFC_DATA);
+		memcpy(buf, &tmp, bytes);
+	}
+
+	return len;
+}
+
+static int rockchip_sfc_fifo_transfer_dma(struct rockchip_sfc *sfc, dma_addr_t dma_buf, size_t len)
+{
+	u32 reg;
+	int err = 0;
+
+	init_completion(&sfc->cp);
+
+	writel(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
+
+	/* Enable transfer complete interrupt */
+	reg = readl(sfc->regbase + SFC_IMR);
+	reg &= ~SFC_IMR_DMA;
+	writel(reg, sfc->regbase + SFC_IMR);
+	writel((u32)dma_buf, sfc->regbase + SFC_DMA_ADDR);
+	writel(0x1, sfc->regbase + SFC_DMA_TRIGGER);
+
+	/* Wait for the interrupt. */
+	if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) {
+		dev_err(sfc->dev, "DMA wait for transfer finish timeout\n");
+		err = -ETIMEDOUT;
+	}
+
+	writel(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
+	/* Disable transfer finish interrupt */
+	reg = readl(sfc->regbase + SFC_IMR);
+	reg |= SFC_IMR_DMA;
+	writel(reg, sfc->regbase + SFC_IMR);
+
+	return len;
+}
+
+static int rockchip_sfc_xfer_data_poll(struct rockchip_sfc *sfc,
+				       const struct spi_mem_op *op, u32 len)
+{
+	dev_dbg(sfc->dev, "xfer_poll len=%x\n", len);
+
+	if (op->data.dir == SPI_MEM_DATA_OUT)
+		return rockchip_sfc_write_fifo(sfc, op->data.buf.out, len);
+	else
+		return rockchip_sfc_read_fifo(sfc, op->data.buf.in, len);
+}
+
+static int rockchip_sfc_xfer_data_dma(struct rockchip_sfc *sfc,
+				      const struct spi_mem_op *op, u32 len)
+{
+	int ret;
+
+	dev_dbg(sfc->dev, "xfer_dma len=%x\n", len);
+
+	if (op->data.dir == SPI_MEM_DATA_OUT) {
+		memcpy_toio(sfc->buffer, op->data.buf.out, len);
+		ret = rockchip_sfc_fifo_transfer_dma(sfc, sfc->dma_buffer, len);
+	} else {
+		ret = rockchip_sfc_fifo_transfer_dma(sfc, sfc->dma_buffer, len);
+		memcpy_fromio(op->data.buf.in, sfc->buffer, len);
+	}
+
+	return ret;
+}
+
+static int rockchip_sfc_xfer_done(struct rockchip_sfc *sfc, u32 timeout_us)
+{
+	int ret = 0;
+	u32 status;
+
+	ret = readl_poll_timeout(sfc->regbase + SFC_SR, status,
+				 !(status & SFC_SR_IS_BUSY),
+				 20, timeout_us);
+	if (ret) {
+		dev_err(sfc->dev, "wait sfc idle timeout\n");
+		rockchip_sfc_reset(sfc);
+
+		ret = -EIO;
+	}
+
+	return ret;
+}
+
+static int rockchip_sfc_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
+{
+	struct rockchip_sfc *sfc = spi_master_get_devdata(mem->spi->master);
+	u32 len = op->data.nbytes;
+	int ret;
+
+	if (unlikely(mem->spi->max_speed_hz != sfc->frequency)) {
+		ret = clk_set_rate(sfc->clk, mem->spi->max_speed_hz);
+		if (ret)
+			return ret;
+		sfc->frequency = mem->spi->max_speed_hz;
+		dev_dbg(sfc->dev, "set_freq=%dHz real_freq=%ldHz\n",
+			sfc->frequency, clk_get_rate(sfc->clk));
+	}
+
+	rockchip_sfc_adjust_op_work((struct spi_mem_op *)op);
+
+	rockchip_sfc_xfer_setup(sfc, mem, op, len);
+	if (len) {
+		if (likely(sfc->use_dma) && !(len & 0x3) && len >= SFC_DMA_TRANS_THRETHOLD)
+			ret = rockchip_sfc_xfer_data_dma(sfc, op, len);
+		else
+			ret = rockchip_sfc_xfer_data_poll(sfc, op, len);
+
+		if (ret != len) {
+			dev_err(sfc->dev, "xfer data failed ret %d dir %d\n", ret, op->data.dir);
+
+			return -EIO;
+		}
+	}
+
+	return rockchip_sfc_xfer_done(sfc, 100000);
+}
+
+static int rockchip_sfc_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
+{
+	struct rockchip_sfc *sfc = spi_master_get_devdata(mem->spi->master);
+
+	op->data.nbytes = min(op->data.nbytes, sfc->max_iosize);
+
+	return 0;
+}
+
+static const struct spi_controller_mem_ops rockchip_sfc_mem_ops = {
+	.exec_op = rockchip_sfc_exec_mem_op,
+	.adjust_op_size = rockchip_sfc_adjust_op_size,
+};
+
+static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id)
+{
+	struct rockchip_sfc *sfc = dev_id;
+	u32 reg;
+
+	reg = readl(sfc->regbase + SFC_RISR);
+
+	/* Clear interrupt */
+	writel_relaxed(reg, sfc->regbase + SFC_ICLR);
+
+	if (reg & SFC_RISR_DMA)
+		complete(&sfc->cp);
+
+	return IRQ_HANDLED;
+}
+
+static int rockchip_sfc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct spi_master *master;
+	struct resource *res;
+	struct rockchip_sfc *sfc;
+	int ret;
+
+	master = devm_spi_alloc_master(&pdev->dev, sizeof(*sfc));
+	if (!master)
+		return -ENOMEM;
+
+	master->mem_ops = &rockchip_sfc_mem_ops;
+	master->dev.of_node = pdev->dev.of_node;
+	master->mode_bits = SPI_TX_QUAD | SPI_TX_DUAL | SPI_RX_QUAD | SPI_RX_DUAL;
+	master->min_speed_hz = SFC_MIN_SPEED_HZ;
+	master->max_speed_hz = SFC_MAX_SPEED_HZ;
+	master->num_chipselect = SFC_MAX_CHIPSELECT_NUM;
+
+	sfc = spi_master_get_devdata(master);
+	sfc->dev = dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	sfc->regbase = devm_ioremap_resource(dev, res);
+	if (IS_ERR(sfc->regbase))
+		return PTR_ERR(sfc->regbase);
+
+	sfc->clk = devm_clk_get(&pdev->dev, "clk_sfc");
+	if (IS_ERR(sfc->clk)) {
+		dev_err(&pdev->dev, "Failed to get sfc interface clk\n");
+		return PTR_ERR(sfc->clk);
+	}
+
+	sfc->hclk = devm_clk_get(&pdev->dev, "hclk_sfc");
+	if (IS_ERR(sfc->hclk)) {
+		dev_err(&pdev->dev, "Failed to get sfc ahb clk\n");
+		return PTR_ERR(sfc->hclk);
+	}
+
+	sfc->use_dma = !of_property_read_bool(sfc->dev->of_node,
+					      "rockchip,sfc-no-dma");
+
+	if (sfc->use_dma) {
+		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
+		if (ret) {
+			dev_warn(dev, "Unable to set dma mask\n");
+			return ret;
+		}
+
+		sfc->buffer = dmam_alloc_coherent(dev, SFC_MAX_IOSIZE_VER3,
+						  &sfc->dma_buffer,
+						  GFP_KERNEL);
+		if (!sfc->buffer)
+			return -ENOMEM;
+	}
+
+	ret = clk_prepare_enable(sfc->hclk);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable ahb clk\n");
+		goto err_hclk;
+	}
+
+	ret = clk_prepare_enable(sfc->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to enable interface clk\n");
+		goto err_clk;
+	}
+
+	/* Find the irq */
+	ret = platform_get_irq(pdev, 0);
+	if (ret < 0) {
+		dev_err(dev, "Failed to get the irq\n");
+		goto err_irq;
+	}
+
+	ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler,
+			       0, pdev->name, sfc);
+	if (ret) {
+		dev_err(dev, "Failed to request irq\n");
+
+		return ret;
+	}
+
+	ret = rockchip_sfc_init(sfc);
+	if (ret)
+		goto err_irq;
+
+	sfc->max_iosize = rockchip_sfc_get_max_iosize(sfc);
+	sfc->version = rockchip_sfc_get_version(sfc);
+
+	ret = spi_register_master(master);
+	if (ret)
+		goto err_irq;
+
+	return 0;
+
+err_irq:
+	clk_disable_unprepare(sfc->clk);
+err_clk:
+	clk_disable_unprepare(sfc->hclk);
+err_hclk:
+	return ret;
+}
+
+static int rockchip_sfc_remove(struct platform_device *pdev)
+{
+	struct spi_master *master = platform_get_drvdata(pdev);
+	struct rockchip_sfc *sfc = platform_get_drvdata(pdev);
+
+	spi_unregister_master(master);
+
+	clk_disable_unprepare(sfc->clk);
+	clk_disable_unprepare(sfc->hclk);
+
+	return 0;
+}
+
+static const struct of_device_id rockchip_sfc_dt_ids[] = {
+	{ .compatible = "rockchip,sfc"},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
+
+static struct platform_driver rockchip_sfc_driver = {
+	.driver = {
+		.name	= "rockchip-sfc",
+		.of_match_table = rockchip_sfc_dt_ids,
+	},
+	.probe	= rockchip_sfc_probe,
+	.remove	= rockchip_sfc_remove,
+};
+module_platform_driver(rockchip_sfc_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver");
+MODULE_AUTHOR("Shawn Lin <shawn.lin@rock-chips.com>");
+MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
+MODULE_AUTHOR("Jon Lin <Jon.lin@rock-chips.com>");
-- 
2.17.1




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

* [PATCH v7 3/9] arm64: dts: rockchip: Add SFC to PX30
  2021-06-09 14:04 [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support Jon Lin
  2021-06-09 14:04 ` [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller Jon Lin
  2021-06-09 14:04 ` [PATCH v7 2/9] spi: rockchip-sfc: add rockchip " Jon Lin
@ 2021-06-09 14:04 ` Jon Lin
  2021-06-09 14:04 ` [PATCH v7 4/9] clk: rockchip: rk3036: fix up the sclk_sfc parent error Jon Lin
  2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
  4 siblings, 0 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:04 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add a devicetree entry for the Rockchip SFC for the PX30 SOC.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 arch/arm64/boot/dts/rockchip/px30.dtsi | 38 ++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/px30.dtsi b/arch/arm64/boot/dts/rockchip/px30.dtsi
index 09baa8a167ce..d854f2577067 100644
--- a/arch/arm64/boot/dts/rockchip/px30.dtsi
+++ b/arch/arm64/boot/dts/rockchip/px30.dtsi
@@ -966,6 +966,18 @@
 		status = "disabled";
 	};
 
+	sfc: spi@ff3a0000 {
+		compatible = "rockchip,sfc";
+		reg = <0x0 0xff3a0000 0x0 0x4000>;
+		interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+		clock-names = "clk_sfc", "hclk_sfc";
+		pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus4>;
+		pinctrl-names = "default";
+		power-domains = <&power PX30_PD_MMC_NAND>;
+		status = "disabled";
+	};
+
 	nfc: nand-controller@ff3b0000 {
 		compatible = "rockchip,px30-nfc";
 		reg = <0x0 0xff3b0000 0x0 0x4000>;
@@ -1967,6 +1979,32 @@
 			};
 		};
 
+		sfc {
+			sfc_bus4: sfc-bus4 {
+				rockchip,pins =
+					<1 RK_PA0 3 &pcfg_pull_none>,
+					<1 RK_PA1 3 &pcfg_pull_none>,
+					<1 RK_PA2 3 &pcfg_pull_none>,
+					<1 RK_PA3 3 &pcfg_pull_none>;
+			};
+
+			sfc_bus2: sfc-bus2 {
+				rockchip,pins =
+					<1 RK_PA0 3 &pcfg_pull_none>,
+					<1 RK_PA1 3 &pcfg_pull_none>;
+			};
+
+			sfc_cs0: sfc-cs0 {
+				rockchip,pins =
+					<1 RK_PA4 3 &pcfg_pull_none>;
+			};
+
+			sfc_clk: sfc-clk {
+				rockchip,pins =
+					<1 RK_PB1 3 &pcfg_pull_none>;
+			};
+		};
+
 		lcdc {
 			lcdc_rgb_dclk_pin: lcdc-rgb-dclk-pin {
 				rockchip,pins =
-- 
2.17.1




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

* [PATCH v7 4/9] clk: rockchip: rk3036: fix up the sclk_sfc parent error
  2021-06-09 14:04 [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support Jon Lin
                   ` (2 preceding siblings ...)
  2021-06-09 14:04 ` [PATCH v7 3/9] arm64: dts: rockchip: Add SFC to PX30 Jon Lin
@ 2021-06-09 14:04 ` Jon Lin
  2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
  4 siblings, 0 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:04 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Elaine Zhang

Choose the correct pll

Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 drivers/clk/rockchip/clk-rk3036.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 91d56ad45817..1986856d94b2 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -121,6 +121,7 @@ PNAME(mux_pll_src_3plls_p)	= { "apll", "dpll", "gpll" };
 PNAME(mux_timer_p)		= { "xin24m", "pclk_peri_src" };
 
 PNAME(mux_pll_src_apll_dpll_gpll_usb480m_p)	= { "apll", "dpll", "gpll", "usb480m" };
+PNAME(mux_pll_src_dmyapll_dpll_gpll_xin24_p)   = { "dummy_apll", "dpll", "gpll", "xin24m" };
 
 PNAME(mux_mmc_src_p)	= { "apll", "dpll", "gpll", "xin24m" };
 PNAME(mux_i2s_pre_p)	= { "i2s_src", "i2s_frac", "ext_i2s", "xin12m" };
@@ -340,7 +341,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 			RK2928_CLKSEL_CON(16), 8, 2, MFLAGS, 10, 5, DFLAGS,
 			RK2928_CLKGATE_CON(10), 4, GFLAGS),
 
-	COMPOSITE(SCLK_SFC, "sclk_sfc", mux_pll_src_apll_dpll_gpll_usb480m_p, 0,
+	COMPOSITE(SCLK_SFC, "sclk_sfc", mux_pll_src_dmyapll_dpll_gpll_xin24_p, 0,
 			RK2928_CLKSEL_CON(16), 0, 2, MFLAGS, 2, 5, DFLAGS,
 			RK2928_CLKGATE_CON(10), 5, GFLAGS),
 
-- 
2.17.1




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

* [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036
  2021-06-09 14:04 [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support Jon Lin
                   ` (3 preceding siblings ...)
  2021-06-09 14:04 ` [PATCH v7 4/9] clk: rockchip: rk3036: fix up the sclk_sfc parent error Jon Lin
@ 2021-06-09 14:13 ` Jon Lin
  2021-06-09 14:13   ` [PATCH v7 6/9] arm: dts: rockchip: Add SFC to RK3036 Jon Lin
                     ` (3 more replies)
  4 siblings, 4 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:13 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add support for the bus clock for the serial flash controller on the
rk3036. Taken from the Rockchip BSP kernel but not tested on real
hardware (as I lack a 3036 based SoC to test).

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 drivers/clk/rockchip/clk-rk3036.c      | 2 +-
 include/dt-bindings/clock/rk3036-cru.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/rockchip/clk-rk3036.c b/drivers/clk/rockchip/clk-rk3036.c
index 1986856d94b2..828af715d92e 100644
--- a/drivers/clk/rockchip/clk-rk3036.c
+++ b/drivers/clk/rockchip/clk-rk3036.c
@@ -404,7 +404,7 @@ static struct rockchip_clk_branch rk3036_clk_branches[] __initdata = {
 	GATE(HCLK_OTG0, "hclk_otg0", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(5), 13, GFLAGS),
 	GATE(HCLK_OTG1, "hclk_otg1", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(7), 3, GFLAGS),
 	GATE(HCLK_I2S, "hclk_i2s", "hclk_peri", 0, RK2928_CLKGATE_CON(7), 2, GFLAGS),
-	GATE(0, "hclk_sfc", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 14, GFLAGS),
+	GATE(HCLK_SFC, "hclk_sfc", "hclk_peri", 0, RK2928_CLKGATE_CON(3), 14, GFLAGS),
 	GATE(HCLK_MAC, "hclk_mac", "hclk_peri", 0, RK2928_CLKGATE_CON(3), 5, GFLAGS),
 
 	/* pclk_peri gates */
diff --git a/include/dt-bindings/clock/rk3036-cru.h b/include/dt-bindings/clock/rk3036-cru.h
index 35a5a01f9697..a96a9870ad59 100644
--- a/include/dt-bindings/clock/rk3036-cru.h
+++ b/include/dt-bindings/clock/rk3036-cru.h
@@ -81,6 +81,7 @@
 #define HCLK_OTG0		449
 #define HCLK_OTG1		450
 #define HCLK_NANDC		453
+#define HCLK_SFC		454
 #define HCLK_SDMMC		456
 #define HCLK_SDIO		457
 #define HCLK_EMMC		459
-- 
2.17.1




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

* [PATCH v7 6/9] arm: dts: rockchip: Add SFC to RK3036
  2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
@ 2021-06-09 14:13   ` Jon Lin
  2021-06-09 14:13   ` [PATCH v7 7/9] arm: dts: rockchip: Add SFC to RV1108 Jon Lin
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:13 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add a devicetree entry for the Rockchip SFC for the RK3036 SOC.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 arch/arm/boot/dts/rk3036.dtsi | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/arm/boot/dts/rk3036.dtsi b/arch/arm/boot/dts/rk3036.dtsi
index e24230d50a78..06e37be81393 100644
--- a/arch/arm/boot/dts/rk3036.dtsi
+++ b/arch/arm/boot/dts/rk3036.dtsi
@@ -206,6 +206,17 @@
 		status = "disabled";
 	};
 
+	sfc: spi@10208000 {
+		compatible = "rockchip,sfc";
+		reg = <0x10208000 0x4000>;
+		interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+		clock-names = "clk_sfc", "hclk_sfc";
+		pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus4>;
+		pinctrl-names = "default";
+		status = "disabled";
+	};
+
 	sdmmc: mmc@10214000 {
 		compatible = "rockchip,rk3036-dw-mshc", "rockchip,rk3288-dw-mshc";
 		reg = <0x10214000 0x4000>;
@@ -684,6 +695,37 @@
 			};
 		};
 
+		sfc {
+			sfc_bus4: sfc-bus4 {
+				rockchip,pins =
+					<1 RK_PD0 3 &pcfg_pull_none>,
+					<1 RK_PD1 3 &pcfg_pull_none>,
+					<1 RK_PD2 3 &pcfg_pull_none>,
+					<1 RK_PD3 3 &pcfg_pull_none>;
+			};
+
+			sfc_bus2: sfc-bus2 {
+				rockchip,pins =
+					<1 RK_PD0 3 &pcfg_pull_none>,
+					<1 RK_PD1 3 &pcfg_pull_none>;
+			};
+
+			sfc_cs0: sfc-cs0 {
+				rockchip,pins =
+					<2 RK_PA2 3 &pcfg_pull_none>;
+			};
+
+			sfc_cs1: sfc-cs1 {
+				rockchip,pins =
+					<2 RK_PA3 3 &pcfg_pull_none>;
+			};
+
+			sfc_clk: sfc-clk {
+				rockchip,pins =
+					<2 RK_PA4 3 &pcfg_pull_none>;
+			};
+		};
+
 		emac {
 			emac_xfer: emac-xfer {
 				rockchip,pins = <2 RK_PB2 1 &pcfg_pull_default>, /* crs_dvalid */
-- 
2.17.1




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

* [PATCH v7 7/9] arm: dts: rockchip: Add SFC to RV1108
  2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
  2021-06-09 14:13   ` [PATCH v7 6/9] arm: dts: rockchip: Add SFC to RK3036 Jon Lin
@ 2021-06-09 14:13   ` Jon Lin
  2021-06-09 14:13   ` [PATCH v7 8/9] arm64: dts: rockchip: Add SFC to RK3308 Jon Lin
  2021-06-09 14:13   ` [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance Jon Lin
  3 siblings, 0 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:13 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add a devicetree entry for the Rockchip SFC for the RV1108 SOC.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 arch/arm/boot/dts/rv1108.dtsi | 37 +++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm/boot/dts/rv1108.dtsi b/arch/arm/boot/dts/rv1108.dtsi
index 884872ca5207..6d4f289aff53 100644
--- a/arch/arm/boot/dts/rv1108.dtsi
+++ b/arch/arm/boot/dts/rv1108.dtsi
@@ -536,6 +536,17 @@
 		status = "disabled";
 	};
 
+	sfc: spi@301c0000 {
+		compatible = "rockchip,sfc";
+		reg = <0x301c0000 0x4000>;
+		interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+		clock-names = "clk_sfc", "hclk_sfc";
+		pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus4>;
+		pinctrl-names = "default";
+		status = "disabled";
+	};
+
 	gmac: eth@30200000 {
 		compatible = "rockchip,rv1108-gmac";
 		reg = <0x30200000 0x10000>;
@@ -704,6 +715,32 @@
 			};
 		};
 
+		sfc {
+			sfc_bus4: sfc-bus4 {
+				rockchip,pins =
+					<2 RK_PA0 3 &pcfg_pull_none>,
+					<2 RK_PA1 3 &pcfg_pull_none>,
+					<2 RK_PA2 3 &pcfg_pull_none>,
+					<2 RK_PA3 3 &pcfg_pull_none>;
+			};
+
+			sfc_bus2: sfc-bus2 {
+				rockchip,pins =
+					<2 RK_PA0 3 &pcfg_pull_none>,
+					<2 RK_PA1 3 &pcfg_pull_none>;
+			};
+
+			sfc_cs0: sfc-cs0 {
+				rockchip,pins =
+					<2 RK_PB4 3 &pcfg_pull_none>;
+			};
+
+			sfc_clk: sfc-clk {
+				rockchip,pins =
+					<2 RK_PB7 2 &pcfg_pull_none>;
+			};
+		};
+
 		gmac {
 			rmii_pins: rmii-pins {
 				rockchip,pins =	<1 RK_PC5 2 &pcfg_pull_none>,
-- 
2.17.1




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

* [PATCH v7 8/9] arm64: dts: rockchip: Add SFC to RK3308
  2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
  2021-06-09 14:13   ` [PATCH v7 6/9] arm: dts: rockchip: Add SFC to RK3036 Jon Lin
  2021-06-09 14:13   ` [PATCH v7 7/9] arm: dts: rockchip: Add SFC to RV1108 Jon Lin
@ 2021-06-09 14:13   ` Jon Lin
  2021-06-09 14:13   ` [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance Jon Lin
  3 siblings, 0 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:13 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

Add a devicetree entry for the Rockchip SFC for the RK3308 SOC.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 arch/arm64/boot/dts/rockchip/rk3308.dtsi | 37 ++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3308.dtsi b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
index 0c5fa9801e6f..cb8d96235986 100644
--- a/arch/arm64/boot/dts/rockchip/rk3308.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3308.dtsi
@@ -637,6 +637,17 @@
 		status = "disabled";
 	};
 
+	sfc: spi@ff4c0000 {
+		compatible = "rockchip,sfc";
+		reg = <0x0 0xff4c0000 0x0 0x4000>;
+		interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
+		clock-names = "clk_sfc", "hclk_sfc";
+		pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus4>;
+		pinctrl-names = "default";
+		status = "disabled";
+	};
+
 	cru: clock-controller@ff500000 {
 		compatible = "rockchip,rk3308-cru";
 		reg = <0x0 0xff500000 0x0 0x1000>;
@@ -910,6 +921,32 @@
 			};
 		};
 
+		sfc {
+			sfc_bus4: sfc-bus4 {
+				rockchip,pins =
+					<3 RK_PA0 3 &pcfg_pull_none>,
+					<3 RK_PA1 3 &pcfg_pull_none>,
+					<3 RK_PA2 3 &pcfg_pull_none>,
+					<3 RK_PA3 3 &pcfg_pull_none>;
+			};
+
+			sfc_bus2: sfc-bus2 {
+				rockchip,pins =
+					<3 RK_PA0 3 &pcfg_pull_none>,
+					<3 RK_PA1 3 &pcfg_pull_none>;
+			};
+
+			sfc_cs0: sfc-cs0 {
+				rockchip,pins =
+					<3 RK_PA4 3 &pcfg_pull_none>;
+			};
+
+			sfc_clk: sfc-clk {
+				rockchip,pins =
+					<3 RK_PA5 3 &pcfg_pull_none>;
+			};
+		};
+
 		gmac {
 			rmii_pins: rmii-pins {
 				rockchip,pins =
-- 
2.17.1




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

* [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance
  2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
                     ` (2 preceding siblings ...)
  2021-06-09 14:13   ` [PATCH v7 8/9] arm64: dts: rockchip: Add SFC to RK3308 Jon Lin
@ 2021-06-09 14:13   ` Jon Lin
  2021-06-10 17:36     ` Chris Morgan
  3 siblings, 1 reply; 20+ messages in thread
From: Jon Lin @ 2021-06-09 14:13 UTC (permalink / raw)
  To: linux-spi
  Cc: jon.lin, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

From: Chris Morgan <macromorgan@hotmail.com>

This enables the Rockchip Serial Flash Controller for the Odroid Go
Advance. Note that while the attached SPI NOR flash and the controller
both support quad read mode, only 2 of the required 4 pins are present.
The rx and tx bus width is set to 2 for this reason.

Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
---

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
Changes in v1: None

 .../boot/dts/rockchip/rk3326-odroid-go2.dts      | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
index 49c97f76df77..f78e11dd8447 100644
--- a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
@@ -484,6 +484,22 @@
 	status = "okay";
 };
 
+&sfc {
+	pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus2>;
+	pinctrl-names = "default";
+	#address-cells = <1>;
+	#size-cells = <0>;
+	status = "okay";
+
+	flash@0 {
+		compatible = "jedec,spi-nor";
+		reg = <0>;
+		spi-max-frequency = <108000000>;
+		spi-rx-bus-width = <2>;
+		spi-tx-bus-width = <2>;
+	};
+};
+
 &tsadc {
 	status = "okay";
 };
-- 
2.17.1




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

* Re: [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-09 14:04 ` [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller Jon Lin
@ 2021-06-09 16:16   ` Rob Herring
  2021-06-10  2:43   ` Rob Herring
  1 sibling, 0 replies; 20+ messages in thread
From: Rob Herring @ 2021-06-09 16:16 UTC (permalink / raw)
  To: Jon Lin
  Cc: yifeng.zhao, sugar.zhang, hjc, mturquette, robh+dt, linux-clk,
	linux-rockchip, linux-mtd, Chris Morgan, macroalpha82, p.yadav,
	linux-spi, jbx6244, heiko, linux-arm-kernel, devicetree, sboyd,
	broonie, linux-kernel

On Wed, 09 Jun 2021 22:04:04 +0800, Jon Lin wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add bindings for the Rockchip serial flash controller. New device
> specific parameter of rockchip,sfc-no-dma included in documentation.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---
> 
> Changes in v7:
> - Fix up the sclk_sfc parent error in rk3036
> - Unify to "rockchip,sfc" compatible id because all the feature update
>   will have a new IP version, so the driver is used for the SFC IP in
>   all SoCs
> - Change to use node "sfc" to name the SFC pinctrl group
> - Add subnode reg property check
> - Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
> - Limit max_iosize to 32KB
> 
> Changes in v6:
> - Add support in device trees for rv1126(Declared in series 5 but not
>   submitted)
> - Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
>   affect interpretation and has been widely used
> - Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
> - Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
>   in series 5 but not submitted)
> - Support SFC ver4 ver5(Declared in series 5 but not submitted)
> - Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
> - Change to use devm_spi_alloc_master and spi_unregister_master
> 
> Changes in v5:
> - Add support in device trees for rv1126
> - Support sfc tx_dual, tx_quad
> - Simplify the code, such as remove "rockchip_sfc_register_all"
> - Support SFC ver4 ver5
> 
> Changes in v4:
> - Changing patch back to an "RFC". An engineer from Rockchip
>   reached out to me to let me know they are working on this patch for
>   upstream, I am submitting this v4 for the community to see however
>   I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
>   soon and these are the ones we should pursue for mainlining. Jon's
>   patch series should include support for more hardware than this
>   series.
> - Clean up documentation more and ensure it is correct per
>   make dt_binding_check.
> - Add support in device trees for rk3036, rk3308, and rv1108.
> - Add ahb clock (hclk_sfc) support for rk3036.
> - Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
> - Change IRQ code to only mark IRQ as handled if it handles the
>   specific IRQ (DMA transfer finish) it is supposed to handle.
> 
> Changes in v3:
> - Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
> - Changed the compatible string from rockchip,sfc to
>   rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
>   driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
>   RV1108 SoCs, and possibly more. However, I am currently only able
>   to test this on a PX30 (an RK3326). The technical reference manuals
>   appear to list the same registers for each device.
> - Corrected devicetree documentation for formatting and to note these
>   changes.
> - Replaced the maintainer with Heiko Stuebner and myself, as we will
>   take ownership of this going forward.
> - Noted that the device (per the reference manual) supports 4 CS, but
>   I am only able to test a single CS (CS 0).
> - Reordered patches to comply with upstream rules.
> 
> Changes in v2:
> - Reimplemented driver using spi-mem subsystem.
> - Removed power management code as I couldn't get it working properly.
> - Added device tree bindings for Odroid Go Advance.
> 
> Changes in v1:
> hanges made in this new series versus the v8 of the old series:
> - Added function to read spi-rx-bus-width from device tree, in the
>   event that the SPI chip supports 4x mode but only has 2 pins
>   wired (such as the Odroid Go Advance).
> - Changed device tree documentation from txt to yaml format.
> - Made "reset" message a dev_dbg from a dev_info.
> - Changed read and write fifo functions to remove redundant checks.
> - Changed the write and read from relaxed to non-relaxed when
>   starting the DMA transfer or reading the DMA IRQ.
> - Changed from dma_coerce_mask_and_coherent to just
>   dma_set_mask_and_coherent.
> - Changed name of get_if_type to rockchip_sfc_get_if_type.
> 
>  .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:
./Documentation/devicetree/bindings/spi/rockchip-sfc.yaml:45:5: [warning] wrong indentation: expected 2 but found 4 (indentation)

dtschema/dtc warnings/errors:
\ndoc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1489897

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


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

* Re: [PATCH v7 2/9] spi: rockchip-sfc: add rockchip serial flash controller
  2021-06-09 14:04 ` [PATCH v7 2/9] spi: rockchip-sfc: add rockchip " Jon Lin
@ 2021-06-09 18:45   ` Chris Morgan
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Morgan @ 2021-06-09 18:45 UTC (permalink / raw)
  To: Jon Lin
  Cc: linux-spi, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, devicetree,
	linux-arm-kernel, linux-kernel, mturquette, sboyd, linux-clk,
	Chris Morgan

On Wed, Jun 09, 2021 at 10:04:05PM +0800, Jon Lin wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add the rockchip serial flash controller (SFC) driver.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---

Getting failures again intermittently in v7 now when writing
the random file to offset 0xc00000 using the mtdblock subsystem
or using flashrom.

Full log with debugging enabled here for MTD Block:
https://gist.github.com/macromorgan/4f54d9188bfbfd8d5e5e199f8a8abe87

Full log with debugging enabled here for Flashrom:
https://gist.github.com/macromorgan/2af35cd3716649a687ccf44aa2091a47

I might be thinking of a red herring or something, but do you think
this could have anything to do with the fact that switching to the
spi-mem framework (instead of constructing our own op struct like
before) would have anything to do with this? More specifically, the
len field used to be bits, didn't it, but now it's bytes?

I'll play around with that. For now though this is still not working
on my V3 SFC hardware.

Thank you.

> 
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> Changes in v1: None
> 
>  drivers/spi/Kconfig            |   9 +
>  drivers/spi/Makefile           |   1 +
>  drivers/spi/spi-rockchip-sfc.c | 676 +++++++++++++++++++++++++++++++++
>  3 files changed, 686 insertions(+)
>  create mode 100644 drivers/spi/spi-rockchip-sfc.c
> 
> diff --git a/drivers/spi/Kconfig b/drivers/spi/Kconfig
> index e71a4c514f7b..d89e5f3c9107 100644
> --- a/drivers/spi/Kconfig
> +++ b/drivers/spi/Kconfig
> @@ -658,6 +658,15 @@ config SPI_ROCKCHIP
>  	  The main usecase of this controller is to use spi flash as boot
>  	  device.
>  
> +config SPI_ROCKCHIP_SFC
> +	tristate "Rockchip Serial Flash Controller (SFC)"
> +	depends on ARCH_ROCKCHIP || COMPILE_TEST
> +	depends on HAS_IOMEM && HAS_DMA
> +	help
> +	  This enables support for Rockchip serial flash controller. This
> +	  is a specialized controller used to access SPI flash on some
> +	  Rockchip SOCs.
> +
>  config SPI_RB4XX
>  	tristate "Mikrotik RB4XX SPI master"
>  	depends on SPI_MASTER && ATH79
> diff --git a/drivers/spi/Makefile b/drivers/spi/Makefile
> index 13e54c45e9df..699db95c8441 100644
> --- a/drivers/spi/Makefile
> +++ b/drivers/spi/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_SPI_QCOM_GENI)		+= spi-geni-qcom.o
>  obj-$(CONFIG_SPI_QCOM_QSPI)		+= spi-qcom-qspi.o
>  obj-$(CONFIG_SPI_QUP)			+= spi-qup.o
>  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_RPCIF)			+= spi-rpc-if.o
> diff --git a/drivers/spi/spi-rockchip-sfc.c b/drivers/spi/spi-rockchip-sfc.c
> new file mode 100644
> index 000000000000..be2557c9af40
> --- /dev/null
> +++ b/drivers/spi/spi-rockchip-sfc.c
> @@ -0,0 +1,676 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Rockchip Serial Flash Controller Driver
> + *
> + * Copyright (c) 2017-2021, Rockchip Inc.
> + * Author: Shawn Lin <shawn.lin@rock-chips.com>
> + *	   Chris Morgan <macroalpha82@gmail.com>
> + *	   Jon Lin <Jon.lin@rock-chips.com>
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/iopoll.h>
> +#include <linux/mm.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/interrupt.h>
> +#include <linux/spi/spi-mem.h>
> +
> +/* System control */
> +#define SFC_CTRL			0x0
> +#define  SFC_CTRL_PHASE_SEL_NEGETIVE	BIT(1)
> +#define  SFC_CTRL_CMD_BITS_SHIFT	8
> +#define  SFC_CTRL_ADDR_BITS_SHIFT	10
> +#define  SFC_CTRL_DATA_BITS_SHIFT	12
> +
> +/* Interrupt mask */
> +#define SFC_IMR				0x4
> +#define  SFC_IMR_RX_FULL		BIT(0)
> +#define  SFC_IMR_RX_UFLOW		BIT(1)
> +#define  SFC_IMR_TX_OFLOW		BIT(2)
> +#define  SFC_IMR_TX_EMPTY		BIT(3)
> +#define  SFC_IMR_TRAN_FINISH		BIT(4)
> +#define  SFC_IMR_BUS_ERR		BIT(5)
> +#define  SFC_IMR_NSPI_ERR		BIT(6)
> +#define  SFC_IMR_DMA			BIT(7)
> +
> +/* Interrupt clear */
> +#define SFC_ICLR			0x8
> +#define  SFC_ICLR_RX_FULL		BIT(0)
> +#define  SFC_ICLR_RX_UFLOW		BIT(1)
> +#define  SFC_ICLR_TX_OFLOW		BIT(2)
> +#define  SFC_ICLR_TX_EMPTY		BIT(3)
> +#define  SFC_ICLR_TRAN_FINISH		BIT(4)
> +#define  SFC_ICLR_BUS_ERR		BIT(5)
> +#define  SFC_ICLR_NSPI_ERR		BIT(6)
> +#define  SFC_ICLR_DMA			BIT(7)
> +
> +/* FIFO threshold level */
> +#define SFC_FTLR			0xc
> +#define  SFC_FTLR_TX_SHIFT		0
> +#define  SFC_FTLR_TX_MASK		0x1f
> +#define  SFC_FTLR_RX_SHIFT		8
> +#define  SFC_FTLR_RX_MASK		0x1f
> +
> +/* Reset FSM and FIFO */
> +#define SFC_RCVR			0x10
> +#define  SFC_RCVR_RESET			BIT(0)
> +
> +/* Enhanced mode */
> +#define SFC_AX				0x14
> +
> +/* Address Bit number */
> +#define SFC_ABIT			0x18
> +
> +/* Interrupt status */
> +#define SFC_ISR				0x1c
> +#define  SFC_ISR_RX_FULL_SHIFT		BIT(0)
> +#define  SFC_ISR_RX_UFLOW_SHIFT		BIT(1)
> +#define  SFC_ISR_TX_OFLOW_SHIFT		BIT(2)
> +#define  SFC_ISR_TX_EMPTY_SHIFT		BIT(3)
> +#define  SFC_ISR_TX_FINISH_SHIFT	BIT(4)
> +#define  SFC_ISR_BUS_ERR_SHIFT		BIT(5)
> +#define  SFC_ISR_NSPI_ERR_SHIFT		BIT(6)
> +#define  SFC_ISR_DMA_SHIFT		BIT(7)
> +
> +/* FIFO status */
> +#define SFC_FSR				0x20
> +#define  SFC_FSR_TX_IS_FULL		BIT(0)
> +#define  SFC_FSR_TX_IS_EMPTY		BIT(1)
> +#define  SFC_FSR_RX_IS_EMPTY		BIT(2)
> +#define  SFC_FSR_RX_IS_FULL		BIT(3)
> +#define  SFC_FSR_TXLV_MASK		GENMASK(12, 8)
> +#define  SFC_FSR_TXLV_SHIFT		8
> +#define  SFC_FSR_RXLV_MASK		GENMASK(20, 16)
> +#define  SFC_FSR_RXLV_SHIFT		16
> +
> +/* FSM status */
> +#define SFC_SR				0x24
> +#define  SFC_SR_IS_IDLE			0x0
> +#define  SFC_SR_IS_BUSY			0x1
> +
> +/* Raw interrupt status */
> +#define SFC_RISR			0x28
> +#define  SFC_RISR_RX_FULL		BIT(0)
> +#define  SFC_RISR_RX_UNDERFLOW		BIT(1)
> +#define  SFC_RISR_TX_OVERFLOW		BIT(2)
> +#define  SFC_RISR_TX_EMPTY		BIT(3)
> +#define  SFC_RISR_TRAN_FINISH		BIT(4)
> +#define  SFC_RISR_BUS_ERR		BIT(5)
> +#define  SFC_RISR_NSPI_ERR		BIT(6)
> +#define  SFC_RISR_DMA			BIT(7)
> +
> +/* Version */
> +#define SFC_VER				0x2C
> +#define  SFC_VER_3			0x3
> +#define  SFC_VER_4			0x4
> +#define  SFC_VER_5			0x5
> +
> +/* Master trigger */
> +#define SFC_DMA_TRIGGER			0x80
> +
> +/* Src or Dst addr for master */
> +#define SFC_DMA_ADDR			0x84
> +
> +/* Length control register extension 32GB */
> +#define SFC_LEN_CTRL			0x88
> +#define SFC_LEN_CTRL_TRB_SEL		1
> +#define SFC_LEN_EXT			0x8C
> +
> +/* Command */
> +#define SFC_CMD				0x100
> +#define  SFC_CMD_IDX_SHIFT		0
> +#define  SFC_CMD_DUMMY_SHIFT		8
> +#define  SFC_CMD_DIR_SHIFT		12
> +#define  SFC_CMD_DIR_RD			0
> +#define  SFC_CMD_DIR_WR			1
> +#define  SFC_CMD_ADDR_SHIFT		14
> +#define  SFC_CMD_ADDR_0BITS		0
> +#define  SFC_CMD_ADDR_24BITS		1
> +#define  SFC_CMD_ADDR_32BITS		2
> +#define  SFC_CMD_ADDR_XBITS		3
> +#define  SFC_CMD_TRAN_BYTES_SHIFT	16
> +#define  SFC_CMD_CS_SHIFT		30
> +
> +/* Address */
> +#define SFC_ADDR			0x104
> +
> +/* Data */
> +#define SFC_DATA			0x108
> +
> +/* The controller and documentation reports that it supports up to 4 CS
> + * devices (0-3), however I have only been able to test a single CS (CS 0)
> + * due to the configuration of my device.
> + */
> +#define SFC_MAX_CHIPSELECT_NUM		4
> +
> +/* The SFC can transfer max 16KB - 1 at one time
> + * we set it to 15.5KB here for alignment.
> + */
> +#define SFC_MAX_IOSIZE_VER3		(512 * 31)
> +
> +/* DMA is only enabled for large data transmission */
> +#define SFC_DMA_TRANS_THRETHOLD		(0x40)
> +
> +/* Maximum clock values from datasheet suggest keeping clock value under
> + * 150MHz. No minimum or average value is suggested, but the U-boot BSP driver
> + * has a minimum of 10MHz and a default of 80MHz which seems reasonable.
> + */
> +#define SFC_MIN_SPEED_HZ		(10 * 1000 * 1000)
> +#define SFC_DEFAULT_SPEED_HZ		(80 * 1000 * 1000)
> +#define SFC_MAX_SPEED_HZ		(150 * 1000 * 1000)
> +
> +struct rockchip_sfc {
> +	struct device *dev;
> +	void __iomem *regbase;
> +	struct clk *hclk;
> +	struct clk *clk;
> +	u32 frequency;
> +	/* virtual mapped addr for dma_buffer */
> +	void *buffer;
> +	dma_addr_t dma_buffer;
> +	struct completion cp;
> +	bool use_dma;
> +	u32 max_iosize;
> +	u16 version;
> +};
> +
> +static int rockchip_sfc_reset(struct rockchip_sfc *sfc)
> +{
> +	int err;
> +	u32 status;
> +
> +	writel_relaxed(SFC_RCVR_RESET, sfc->regbase + SFC_RCVR);
> +
> +	err = readl_poll_timeout(sfc->regbase + SFC_RCVR, status,
> +				 !(status & SFC_RCVR_RESET), 20,
> +				 jiffies_to_usecs(HZ));
> +	if (err)
> +		dev_err(sfc->dev, "SFC reset never finished\n");
> +
> +	/* Still need to clear the masked interrupt from RISR */
> +	writel_relaxed(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
> +
> +	dev_dbg(sfc->dev, "reset\n");
> +
> +	return err;
> +}
> +
> +static u16 rockchip_sfc_get_version(struct rockchip_sfc *sfc)
> +{
> +	return  (u16)(readl(sfc->regbase + SFC_VER) & 0xffff);
> +}
> +
> +static u32 rockchip_sfc_get_max_iosize(struct rockchip_sfc *sfc)
> +{
> +	return SFC_MAX_IOSIZE_VER3;
> +}
> +
> +static int rockchip_sfc_init(struct rockchip_sfc *sfc)
> +{
> +	writel(0, sfc->regbase + SFC_CTRL);
> +	if (rockchip_sfc_get_version(sfc) >= SFC_VER_4)
> +		writel(SFC_LEN_CTRL_TRB_SEL, sfc->regbase + SFC_LEN_CTRL);
> +
> +	return 0;
> +}
> +
> +static inline int rockchip_sfc_get_fifo_level(struct rockchip_sfc *sfc, int wr)
> +{
> +	u32 fsr = readl_relaxed(sfc->regbase + SFC_FSR);
> +	int level;
> +
> +	if (wr)
> +		level = (fsr & SFC_FSR_TXLV_MASK) >> SFC_FSR_TXLV_SHIFT;
> +	else
> +		level = (fsr & SFC_FSR_RXLV_MASK) >> SFC_FSR_RXLV_SHIFT;
> +
> +	return level;
> +}
> +
> +static int rockchip_sfc_wait_fifo_ready(struct rockchip_sfc *sfc, int wr, u32 timeout)
> +{
> +	unsigned long deadline = jiffies + timeout;
> +	int level;
> +
> +	while (!(level = rockchip_sfc_get_fifo_level(sfc, wr))) {
> +		if (time_after_eq(jiffies, deadline)) {
> +			dev_warn(sfc->dev, "%s fifo timeout\n", wr ? "write" : "read");
> +			return -ETIMEDOUT;
> +		}
> +		udelay(1);
> +	}
> +
> +	return level;
> +}
> +
> +static void rockchip_sfc_adjust_op_work(struct spi_mem_op *op)
> +{
> +	if (unlikely(op->dummy.nbytes && !op->addr.nbytes)) {
> +		/*
> +		 * SFC not support output DUMMY cycles right after CMD cycles, so
> +		 * treat it as ADDR cycles.
> +		 */
> +		op->addr.nbytes = op->dummy.nbytes;
> +		op->addr.buswidth = op->dummy.buswidth;
> +		op->addr.val = 0xFFFFFFFFF;
> +
> +		op->dummy.nbytes = 0;
> +	}
> +}
> +
> +static int rockchip_sfc_xfer_setup(struct rockchip_sfc *sfc,
> +				   struct spi_mem *mem,
> +				   const struct spi_mem_op *op,
> +				   u32 len)
> +{
> +	u32 ctrl = 0, cmd = 0;
> +
> +	/* set CMD */
> +	cmd = op->cmd.opcode;
> +	ctrl |= ((op->cmd.buswidth >> 1) << SFC_CTRL_CMD_BITS_SHIFT);
> +
> +	/* set ADDR */
> +	if (op->addr.nbytes) {
> +		if (op->addr.nbytes == 4) {
> +			cmd |= SFC_CMD_ADDR_32BITS << SFC_CMD_ADDR_SHIFT;
> +		} else if (op->addr.nbytes == 3) {
> +			cmd |= SFC_CMD_ADDR_24BITS << SFC_CMD_ADDR_SHIFT;
> +		} else {
> +			cmd |= SFC_CMD_ADDR_XBITS << SFC_CMD_ADDR_SHIFT;
> +			writel_relaxed(op->addr.nbytes * 8 - 1, sfc->regbase + SFC_ABIT);
> +		}
> +
> +		ctrl |= ((op->addr.buswidth >> 1) << SFC_CTRL_ADDR_BITS_SHIFT);
> +	}
> +
> +	/* set DUMMY */
> +	if (op->dummy.nbytes) {
> +		if (op->dummy.buswidth == 4)
> +			cmd |= op->dummy.nbytes * 2 << SFC_CMD_DUMMY_SHIFT;
> +		else if (op->dummy.buswidth == 2)
> +			cmd |= op->dummy.nbytes * 4 << SFC_CMD_DUMMY_SHIFT;
> +		else
> +			cmd |= op->dummy.nbytes * 8 << SFC_CMD_DUMMY_SHIFT;
> +	}
> +
> +	/* set DATA */
> +	if (sfc->version >= SFC_VER_4) /* Clear it if no data to transfer */
> +		writel(len, sfc->regbase + SFC_LEN_EXT);
> +	else
> +		cmd |= len << SFC_CMD_TRAN_BYTES_SHIFT;
> +	if (len) {
> +		if (op->data.dir == SPI_MEM_DATA_OUT)
> +			cmd |= SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT;
> +
> +		ctrl |= ((op->data.buswidth >> 1) << SFC_CTRL_DATA_BITS_SHIFT);
> +	}
> +	if (!len && op->addr.nbytes)
> +		cmd |= SFC_CMD_DIR_WR << SFC_CMD_DIR_SHIFT;
> +
> +	/* set the Controller */
> +	ctrl |= SFC_CTRL_PHASE_SEL_NEGETIVE;
> +	cmd |= mem->spi->chip_select << SFC_CMD_CS_SHIFT;
> +
> +	dev_dbg(sfc->dev, "addr.nbytes=%x(x%d) dummy.nbytes=%x(x%d)\n",
> +		op->addr.nbytes, op->addr.buswidth,
> +		op->dummy.nbytes, op->dummy.buswidth);
> +	dev_dbg(sfc->dev, "ctrl=%x cmd=%x addr=%llx len=%x\n",
> +		ctrl, cmd, op->addr.val, len);
> +
> +	writel(ctrl, sfc->regbase + SFC_CTRL);
> +	writel(cmd, sfc->regbase + SFC_CMD);
> +	if (op->addr.nbytes)
> +		writel(op->addr.val, sfc->regbase + SFC_ADDR);
> +
> +	return 0;
> +}
> +
> +static int rockchip_sfc_write_fifo(struct rockchip_sfc *sfc, const u8 *buf, int len)
> +{
> +	u8 bytes = len & 0x3;

Since len now comes from the op->data.nbytes it's already in bytes right?

> +	u32 dwords;
> +	int tx_level;
> +	u32 write_words;
> +	u32 tmp = 0;
> +
> +	dwords = len >> 2;
> +	while (dwords) {
> +		tx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_WR, HZ);
> +		if (tx_level < 0)
> +			return tx_level;
> +		write_words = min_t(u32, tx_level, dwords);
> +		iowrite32_rep(sfc->regbase + SFC_DATA, buf, write_words);
> +		buf += write_words << 2;
> +		dwords -= write_words;
> +	}
> +
> +	/* write the rest non word aligned bytes */
> +	if (bytes) {
> +		tx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_WR, HZ);
> +		if (tx_level < 0)
> +			return tx_level;
> +		memcpy(&tmp, buf, bytes);
> +		writel(tmp, sfc->regbase + SFC_DATA);
> +	}
> +
> +	return len;
> +}
> +
> +static int rockchip_sfc_read_fifo(struct rockchip_sfc *sfc, u8 *buf, int len)
> +{
> +	u8 bytes = len & 0x3;
> +	u32 dwords;
> +	u8 read_words;
> +	int rx_level;
> +	int tmp;
> +
> +	/* word aligned access only */
> +	dwords = len >> 2;
> +	while (dwords) {
> +		rx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_RD, HZ);
> +		if (rx_level < 0)
> +			return rx_level;
> +		read_words = min_t(u32, rx_level, dwords);
> +		ioread32_rep(sfc->regbase + SFC_DATA, buf, read_words);
> +		buf += read_words << 2;
> +		dwords -= read_words;
> +	}
> +
> +	/* read the rest non word aligned bytes */
> +	if (bytes) {
> +		rx_level = rockchip_sfc_wait_fifo_ready(sfc, SFC_CMD_DIR_RD, HZ);
> +		if (rx_level < 0)
> +			return rx_level;
> +		tmp = readl_relaxed(sfc->regbase + SFC_DATA);
> +		memcpy(buf, &tmp, bytes);
> +	}
> +
> +	return len;
> +}
> +
> +static int rockchip_sfc_fifo_transfer_dma(struct rockchip_sfc *sfc, dma_addr_t dma_buf, size_t len)
> +{
> +	u32 reg;
> +	int err = 0;
> +
> +	init_completion(&sfc->cp);
> +
> +	writel(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
> +
> +	/* Enable transfer complete interrupt */
> +	reg = readl(sfc->regbase + SFC_IMR);
> +	reg &= ~SFC_IMR_DMA;
> +	writel(reg, sfc->regbase + SFC_IMR);
> +	writel((u32)dma_buf, sfc->regbase + SFC_DMA_ADDR);
> +	writel(0x1, sfc->regbase + SFC_DMA_TRIGGER);
> +
> +	/* Wait for the interrupt. */
> +	if (!wait_for_completion_timeout(&sfc->cp, msecs_to_jiffies(2000))) {
> +		dev_err(sfc->dev, "DMA wait for transfer finish timeout\n");
> +		err = -ETIMEDOUT;
> +	}
> +
> +	writel(0xFFFFFFFF, sfc->regbase + SFC_ICLR);
> +	/* Disable transfer finish interrupt */
> +	reg = readl(sfc->regbase + SFC_IMR);
> +	reg |= SFC_IMR_DMA;
> +	writel(reg, sfc->regbase + SFC_IMR);
> +
> +	return len;
> +}
> +
> +static int rockchip_sfc_xfer_data_poll(struct rockchip_sfc *sfc,
> +				       const struct spi_mem_op *op, u32 len)
> +{
> +	dev_dbg(sfc->dev, "xfer_poll len=%x\n", len);
> +
> +	if (op->data.dir == SPI_MEM_DATA_OUT)
> +		return rockchip_sfc_write_fifo(sfc, op->data.buf.out, len);
> +	else
> +		return rockchip_sfc_read_fifo(sfc, op->data.buf.in, len);
> +}
> +
> +static int rockchip_sfc_xfer_data_dma(struct rockchip_sfc *sfc,
> +				      const struct spi_mem_op *op, u32 len)
> +{
> +	int ret;
> +
> +	dev_dbg(sfc->dev, "xfer_dma len=%x\n", len);
> +
> +	if (op->data.dir == SPI_MEM_DATA_OUT) {
> +		memcpy_toio(sfc->buffer, op->data.buf.out, len);
> +		ret = rockchip_sfc_fifo_transfer_dma(sfc, sfc->dma_buffer, len);
> +	} else {
> +		ret = rockchip_sfc_fifo_transfer_dma(sfc, sfc->dma_buffer, len);
> +		memcpy_fromio(op->data.buf.in, sfc->buffer, len);
> +	}
> +
> +	return ret;
> +}
> +
> +static int rockchip_sfc_xfer_done(struct rockchip_sfc *sfc, u32 timeout_us)
> +{
> +	int ret = 0;
> +	u32 status;
> +
> +	ret = readl_poll_timeout(sfc->regbase + SFC_SR, status,
> +				 !(status & SFC_SR_IS_BUSY),
> +				 20, timeout_us);
> +	if (ret) {
> +		dev_err(sfc->dev, "wait sfc idle timeout\n");
> +		rockchip_sfc_reset(sfc);
> +
> +		ret = -EIO;
> +	}
> +
> +	return ret;
> +}
> +
> +static int rockchip_sfc_exec_mem_op(struct spi_mem *mem, const struct spi_mem_op *op)
> +{
> +	struct rockchip_sfc *sfc = spi_master_get_devdata(mem->spi->master);
> +	u32 len = op->data.nbytes;
> +	int ret;
> +
> +	if (unlikely(mem->spi->max_speed_hz != sfc->frequency)) {
> +		ret = clk_set_rate(sfc->clk, mem->spi->max_speed_hz);
> +		if (ret)
> +			return ret;
> +		sfc->frequency = mem->spi->max_speed_hz;
> +		dev_dbg(sfc->dev, "set_freq=%dHz real_freq=%ldHz\n",
> +			sfc->frequency, clk_get_rate(sfc->clk));
> +	}
> +
> +	rockchip_sfc_adjust_op_work((struct spi_mem_op *)op);
> +
> +	rockchip_sfc_xfer_setup(sfc, mem, op, len);
> +	if (len) {
> +		if (likely(sfc->use_dma) && !(len & 0x3) && len >= SFC_DMA_TRANS_THRETHOLD)

len here is in bytes, but the poll function takes it in bits right? I
think the DMA function works in bytes.

> +			ret = rockchip_sfc_xfer_data_dma(sfc, op, len);
> +		else
> +			ret = rockchip_sfc_xfer_data_poll(sfc, op, len);
> +
> +		if (ret != len) {
> +			dev_err(sfc->dev, "xfer data failed ret %d dir %d\n", ret, op->data.dir);
> +
> +			return -EIO;
> +		}
> +	}
> +
> +	return rockchip_sfc_xfer_done(sfc, 100000);
> +}
> +
> +static int rockchip_sfc_adjust_op_size(struct spi_mem *mem, struct spi_mem_op *op)
> +{
> +	struct rockchip_sfc *sfc = spi_master_get_devdata(mem->spi->master);
> +
> +	op->data.nbytes = min(op->data.nbytes, sfc->max_iosize);
> +
> +	return 0;
> +}
> +
> +static const struct spi_controller_mem_ops rockchip_sfc_mem_ops = {
> +	.exec_op = rockchip_sfc_exec_mem_op,
> +	.adjust_op_size = rockchip_sfc_adjust_op_size,
> +};
> +
> +static irqreturn_t rockchip_sfc_irq_handler(int irq, void *dev_id)
> +{
> +	struct rockchip_sfc *sfc = dev_id;
> +	u32 reg;
> +
> +	reg = readl(sfc->regbase + SFC_RISR);
> +
> +	/* Clear interrupt */
> +	writel_relaxed(reg, sfc->regbase + SFC_ICLR);
> +
> +	if (reg & SFC_RISR_DMA)
> +		complete(&sfc->cp);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int rockchip_sfc_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct spi_master *master;
> +	struct resource *res;
> +	struct rockchip_sfc *sfc;
> +	int ret;
> +
> +	master = devm_spi_alloc_master(&pdev->dev, sizeof(*sfc));
> +	if (!master)
> +		return -ENOMEM;
> +
> +	master->mem_ops = &rockchip_sfc_mem_ops;
> +	master->dev.of_node = pdev->dev.of_node;
> +	master->mode_bits = SPI_TX_QUAD | SPI_TX_DUAL | SPI_RX_QUAD | SPI_RX_DUAL;
> +	master->min_speed_hz = SFC_MIN_SPEED_HZ;
> +	master->max_speed_hz = SFC_MAX_SPEED_HZ;
> +	master->num_chipselect = SFC_MAX_CHIPSELECT_NUM;
> +
> +	sfc = spi_master_get_devdata(master);
> +	sfc->dev = dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	sfc->regbase = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(sfc->regbase))
> +		return PTR_ERR(sfc->regbase);
> +
> +	sfc->clk = devm_clk_get(&pdev->dev, "clk_sfc");
> +	if (IS_ERR(sfc->clk)) {
> +		dev_err(&pdev->dev, "Failed to get sfc interface clk\n");
> +		return PTR_ERR(sfc->clk);
> +	}
> +
> +	sfc->hclk = devm_clk_get(&pdev->dev, "hclk_sfc");
> +	if (IS_ERR(sfc->hclk)) {
> +		dev_err(&pdev->dev, "Failed to get sfc ahb clk\n");
> +		return PTR_ERR(sfc->hclk);
> +	}
> +
> +	sfc->use_dma = !of_property_read_bool(sfc->dev->of_node,
> +					      "rockchip,sfc-no-dma");
> +
> +	if (sfc->use_dma) {
> +		ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
> +		if (ret) {
> +			dev_warn(dev, "Unable to set dma mask\n");
> +			return ret;
> +		}
> +
> +		sfc->buffer = dmam_alloc_coherent(dev, SFC_MAX_IOSIZE_VER3,
> +						  &sfc->dma_buffer,
> +						  GFP_KERNEL);
> +		if (!sfc->buffer)
> +			return -ENOMEM;
> +	}
> +
> +	ret = clk_prepare_enable(sfc->hclk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to enable ahb clk\n");
> +		goto err_hclk;
> +	}
> +
> +	ret = clk_prepare_enable(sfc->clk);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Failed to enable interface clk\n");
> +		goto err_clk;
> +	}
> +
> +	/* Find the irq */
> +	ret = platform_get_irq(pdev, 0);
> +	if (ret < 0) {
> +		dev_err(dev, "Failed to get the irq\n");
> +		goto err_irq;
> +	}
> +
> +	ret = devm_request_irq(dev, ret, rockchip_sfc_irq_handler,
> +			       0, pdev->name, sfc);
> +	if (ret) {
> +		dev_err(dev, "Failed to request irq\n");
> +
> +		return ret;
> +	}
> +
> +	ret = rockchip_sfc_init(sfc);
> +	if (ret)
> +		goto err_irq;
> +
> +	sfc->max_iosize = rockchip_sfc_get_max_iosize(sfc);
> +	sfc->version = rockchip_sfc_get_version(sfc);
> +
> +	ret = spi_register_master(master);
> +	if (ret)
> +		goto err_irq;
> +
> +	return 0;
> +
> +err_irq:
> +	clk_disable_unprepare(sfc->clk);
> +err_clk:
> +	clk_disable_unprepare(sfc->hclk);
> +err_hclk:
> +	return ret;
> +}
> +
> +static int rockchip_sfc_remove(struct platform_device *pdev)
> +{
> +	struct spi_master *master = platform_get_drvdata(pdev);
> +	struct rockchip_sfc *sfc = platform_get_drvdata(pdev);
> +
> +	spi_unregister_master(master);
> +
> +	clk_disable_unprepare(sfc->clk);
> +	clk_disable_unprepare(sfc->hclk);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id rockchip_sfc_dt_ids[] = {
> +	{ .compatible = "rockchip,sfc"},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, rockchip_sfc_dt_ids);
> +
> +static struct platform_driver rockchip_sfc_driver = {
> +	.driver = {
> +		.name	= "rockchip-sfc",
> +		.of_match_table = rockchip_sfc_dt_ids,
> +	},
> +	.probe	= rockchip_sfc_probe,
> +	.remove	= rockchip_sfc_remove,
> +};
> +module_platform_driver(rockchip_sfc_driver);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("Rockchip Serial Flash Controller Driver");
> +MODULE_AUTHOR("Shawn Lin <shawn.lin@rock-chips.com>");
> +MODULE_AUTHOR("Chris Morgan <macromorgan@hotmail.com>");
> +MODULE_AUTHOR("Jon Lin <Jon.lin@rock-chips.com>");
> -- 
> 2.17.1
> 
> 
> 

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

* Re: [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-09 14:04 ` [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller Jon Lin
  2021-06-09 16:16   ` Rob Herring
@ 2021-06-10  2:43   ` Rob Herring
  2021-06-10  3:02     ` Kever Yang
  1 sibling, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-06-10  2:43 UTC (permalink / raw)
  To: Jon Lin
  Cc: linux-spi, broonie, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

On Wed, Jun 09, 2021 at 10:04:04PM +0800, Jon Lin wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> Add bindings for the Rockchip serial flash controller. New device
> specific parameter of rockchip,sfc-no-dma included in documentation.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---
> 
> Changes in v7:
> - Fix up the sclk_sfc parent error in rk3036
> - Unify to "rockchip,sfc" compatible id because all the feature update
>   will have a new IP version, so the driver is used for the SFC IP in
>   all SoCs
> - Change to use node "sfc" to name the SFC pinctrl group
> - Add subnode reg property check
> - Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
> - Limit max_iosize to 32KB
> 
> Changes in v6:
> - Add support in device trees for rv1126(Declared in series 5 but not
>   submitted)
> - Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
>   affect interpretation and has been widely used
> - Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
> - Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
>   in series 5 but not submitted)
> - Support SFC ver4 ver5(Declared in series 5 but not submitted)
> - Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
> - Change to use devm_spi_alloc_master and spi_unregister_master
> 
> Changes in v5:
> - Add support in device trees for rv1126
> - Support sfc tx_dual, tx_quad
> - Simplify the code, such as remove "rockchip_sfc_register_all"
> - Support SFC ver4 ver5
> 
> Changes in v4:
> - Changing patch back to an "RFC". An engineer from Rockchip
>   reached out to me to let me know they are working on this patch for
>   upstream, I am submitting this v4 for the community to see however
>   I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
>   soon and these are the ones we should pursue for mainlining. Jon's
>   patch series should include support for more hardware than this
>   series.
> - Clean up documentation more and ensure it is correct per
>   make dt_binding_check.
> - Add support in device trees for rk3036, rk3308, and rv1108.
> - Add ahb clock (hclk_sfc) support for rk3036.
> - Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
> - Change IRQ code to only mark IRQ as handled if it handles the
>   specific IRQ (DMA transfer finish) it is supposed to handle.
> 
> Changes in v3:
> - Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
> - Changed the compatible string from rockchip,sfc to
>   rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
>   driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
>   RV1108 SoCs, and possibly more. However, I am currently only able
>   to test this on a PX30 (an RK3326). The technical reference manuals
>   appear to list the same registers for each device.
> - Corrected devicetree documentation for formatting and to note these
>   changes.
> - Replaced the maintainer with Heiko Stuebner and myself, as we will
>   take ownership of this going forward.
> - Noted that the device (per the reference manual) supports 4 CS, but
>   I am only able to test a single CS (CS 0).
> - Reordered patches to comply with upstream rules.
> 
> Changes in v2:
> - Reimplemented driver using spi-mem subsystem.
> - Removed power management code as I couldn't get it working properly.
> - Added device tree bindings for Odroid Go Advance.
> 
> Changes in v1:
> hanges made in this new series versus the v8 of the old series:
> - Added function to read spi-rx-bus-width from device tree, in the
>   event that the SPI chip supports 4x mode but only has 2 pins
>   wired (such as the Odroid Go Advance).
> - Changed device tree documentation from txt to yaml format.
> - Made "reset" message a dev_dbg from a dev_info.
> - Changed read and write fifo functions to remove redundant checks.
> - Changed the write and read from relaxed to non-relaxed when
>   starting the DMA transfer or reading the DMA IRQ.
> - Changed from dma_coerce_mask_and_coherent to just
>   dma_set_mask_and_coherent.
> - Changed name of get_if_type to rockchip_sfc_get_if_type.
> 
>  .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
>  1 file changed, 88 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> new file mode 100644
> index 000000000000..42e4198e92af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/spi/rockchip-sfc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip Serial Flash Controller (SFC)
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@sntech.de>
> +  - Chris Morgan <macromorgan@hotmail.com>
> +
> +allOf:
> +  - $ref: spi-controller.yaml#
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: rockchip,sfc

Use 'enum' instead of oneOf+const.

You need an SoC specific compatible.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Bus Clock
> +      - description: Module Clock
> +
> +  clock-names:
> +    items:
> +      - const: clk_sfc
> +      - const: hclk_sfc

Do you have these backwards? 'hclk' is usually an AHB bus clock.

Also, '_sfc' is redundant.

> +
> +  power-domains:
> +    maxItems: 1
> +
> +  rockchip,sfc-no-dma:
> +    description: Disable DMA and utilize FIFO mode only
> +    type: boolean
> +
> +patternProperties:
> +    "^flash@[0-3]$":
> +      type: object
> +      properties:
> +        reg:
> +          minimum: 0
> +          maximum: 3
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/px30-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/px30-power.h>
> +
> +    sfc: spi@ff3a0000 {
> +        compatible = "rockchip,sfc";
> +        reg = <0xff3a0000 0x4000>;
> +        interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>;
> +        clocks = <&cru SCLK_SFC>, <&cru HCLK_SFC>;
> +        clock-names = "clk_sfc", "hclk_sfc";
> +        pinctrl-0 = <&sfc_clk &sfc_cs &sfc_bus2>;
> +        pinctrl-names = "default";
> +        power-domains = <&power PX30_PD_MMC_NAND>;
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        flash@0 {
> +            compatible = "jedec,spi-nor";
> +            reg = <0>;
> +            spi-max-frequency = <108000000>;
> +            spi-rx-bus-width = <2>;
> +            spi-tx-bus-width = <2>;
> +        };
> +    };
> +
> +...
> -- 
> 2.17.1

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

* Re: [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-10  2:43   ` Rob Herring
@ 2021-06-10  3:02     ` Kever Yang
  2021-06-11 16:32       ` Ezequiel Garcia
  0 siblings, 1 reply; 20+ messages in thread
From: Kever Yang @ 2021-06-10  3:02 UTC (permalink / raw)
  To: Rob Herring, Jon Lin
  Cc: linux-spi, broonie, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, macroalpha82,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk, Chris Morgan

Hi Rob,

On 2021/6/10 上午10:43, Rob Herring wrote:
> On Wed, Jun 09, 2021 at 10:04:04PM +0800, Jon Lin wrote:
>> From: Chris Morgan <macromorgan@hotmail.com>
>>
>> Add bindings for the Rockchip serial flash controller. New device
>> specific parameter of rockchip,sfc-no-dma included in documentation.
>>
>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
>> ---
>>
>> Changes in v7:
>> - Fix up the sclk_sfc parent error in rk3036
>> - Unify to "rockchip,sfc" compatible id because all the feature update
>>    will have a new IP version, so the driver is used for the SFC IP in
>>    all SoCs
>> - Change to use node "sfc" to name the SFC pinctrl group
>> - Add subnode reg property check
>> - Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
>> - Limit max_iosize to 32KB
>>
>> Changes in v6:
>> - Add support in device trees for rv1126(Declared in series 5 but not
>>    submitted)
>> - Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
>>    affect interpretation and has been widely used
>> - Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
>> - Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
>>    in series 5 but not submitted)
>> - Support SFC ver4 ver5(Declared in series 5 but not submitted)
>> - Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
>> - Change to use devm_spi_alloc_master and spi_unregister_master
>>
>> Changes in v5:
>> - Add support in device trees for rv1126
>> - Support sfc tx_dual, tx_quad
>> - Simplify the code, such as remove "rockchip_sfc_register_all"
>> - Support SFC ver4 ver5
>>
>> Changes in v4:
>> - Changing patch back to an "RFC". An engineer from Rockchip
>>    reached out to me to let me know they are working on this patch for
>>    upstream, I am submitting this v4 for the community to see however
>>    I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
>>    soon and these are the ones we should pursue for mainlining. Jon's
>>    patch series should include support for more hardware than this
>>    series.
>> - Clean up documentation more and ensure it is correct per
>>    make dt_binding_check.
>> - Add support in device trees for rk3036, rk3308, and rv1108.
>> - Add ahb clock (hclk_sfc) support for rk3036.
>> - Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
>> - Change IRQ code to only mark IRQ as handled if it handles the
>>    specific IRQ (DMA transfer finish) it is supposed to handle.
>>
>> Changes in v3:
>> - Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
>> - Changed the compatible string from rockchip,sfc to
>>    rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
>>    driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
>>    RV1108 SoCs, and possibly more. However, I am currently only able
>>    to test this on a PX30 (an RK3326). The technical reference manuals
>>    appear to list the same registers for each device.
>> - Corrected devicetree documentation for formatting and to note these
>>    changes.
>> - Replaced the maintainer with Heiko Stuebner and myself, as we will
>>    take ownership of this going forward.
>> - Noted that the device (per the reference manual) supports 4 CS, but
>>    I am only able to test a single CS (CS 0).
>> - Reordered patches to comply with upstream rules.
>>
>> Changes in v2:
>> - Reimplemented driver using spi-mem subsystem.
>> - Removed power management code as I couldn't get it working properly.
>> - Added device tree bindings for Odroid Go Advance.
>>
>> Changes in v1:
>> hanges made in this new series versus the v8 of the old series:
>> - Added function to read spi-rx-bus-width from device tree, in the
>>    event that the SPI chip supports 4x mode but only has 2 pins
>>    wired (such as the Odroid Go Advance).
>> - Changed device tree documentation from txt to yaml format.
>> - Made "reset" message a dev_dbg from a dev_info.
>> - Changed read and write fifo functions to remove redundant checks.
>> - Changed the write and read from relaxed to non-relaxed when
>>    starting the DMA transfer or reading the DMA IRQ.
>> - Changed from dma_coerce_mask_and_coherent to just
>>    dma_set_mask_and_coherent.
>> - Changed name of get_if_type to rockchip_sfc_get_if_type.
>>
>>   .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
>>   1 file changed, 88 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
>> new file mode 100644
>> index 000000000000..42e4198e92af
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
>> @@ -0,0 +1,88 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/spi/rockchip-sfc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Rockchip Serial Flash Controller (SFC)
>> +
>> +maintainers:
>> +  - Heiko Stuebner <heiko@sntech.de>
>> +  - Chris Morgan <macromorgan@hotmail.com>
>> +
>> +allOf:
>> +  - $ref: spi-controller.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: rockchip,sfc
> Use 'enum' instead of oneOf+const.
>
> You need an SoC specific compatible.


The rockchip sfc controller is a standalone IP with version register, 
and the driver can

handle all the feature difference inside the IP, so we would like to use 
a more generic

compatible name instead of bind to any of SoC name. So can we use 
"rockchip,sfc"

like "snps,designware-spi", which is a generic one, instead of an SoC 
specific compatible?


Thanks,

- Kever




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

* Re: [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance
  2021-06-09 14:13   ` [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance Jon Lin
@ 2021-06-10 17:36     ` Chris Morgan
  2021-06-11  2:26       ` Jon Lin
  0 siblings, 1 reply; 20+ messages in thread
From: Chris Morgan @ 2021-06-10 17:36 UTC (permalink / raw)
  To: Jon Lin
  Cc: linux-spi, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, devicetree,
	linux-arm-kernel, linux-kernel, mturquette, sboyd, linux-clk,
	Chris Morgan

On Wed, Jun 09, 2021 at 10:13:48PM +0800, Jon Lin wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
> 
> This enables the Rockchip Serial Flash Controller for the Odroid Go
> Advance. Note that while the attached SPI NOR flash and the controller
> both support quad read mode, only 2 of the required 4 pins are present.
> The rx and tx bus width is set to 2 for this reason.
> 
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> ---
> 
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2: None
> Changes in v1: None
> 
>  .../boot/dts/rockchip/rk3326-odroid-go2.dts      | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
> index 49c97f76df77..f78e11dd8447 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
> @@ -484,6 +484,22 @@
>  	status = "okay";
>  };
>  
> +&sfc {
> +	pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus2>;
> +	pinctrl-names = "default";
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	status = "okay";
> +
> +	flash@0 {
> +		compatible = "jedec,spi-nor";
> +		reg = <0>;
> +		spi-max-frequency = <108000000>;
> +		spi-rx-bus-width = <2>;
> +		spi-tx-bus-width = <2>;

Note that I am still working with Jon Lin to research this, but it was
found in testing that if I set the tx bus width to 1 the problems I
encountered in earlier are resolved. At this time I do not know if it
is an issue with the driver for the flash controller, or if the NOR, or
board itself has some sort of errata which prevent dual tx from working
correctly. Note that as of right now the flash chip I am using (an
XTX XT25F128B) is not currently supported in mainline, so it's very
possible this is some sort of errata with the chip. It's also possible
that there is something with the board that is interferring with dual
mode TX.  When Jon comes back that he has tested dual mode on the SFC
with a different board/chip I will recommend that we change the tx
bus width here to a 1, and then once the XT25F128B gets mainlined we
can see if someone else has issues with dual tx mode so we can note
that as a problem with the chip. Or maybe there is something weird
with dual tx mode yet on the SFC driver/controller, I don't know yet.
I'm all too happy to work with a Rockchip engineer so things like
this can be determined before we hit mainline. :-)

The XTX25F128B driver is currently awaiting a decision on how to handle
continuation codes, as this chip ID should be using continuation codes,
but doesn't appear to return them when you query for manufacturer ID.
So I should also note in the commit here that the SFC will still be
unusable on the Odroid Go Advance until the XTX25F128B is also
mainlined.

Thank you.

> +	};
> +};
> +
>  &tsadc {
>  	status = "okay";
>  };
> -- 
> 2.17.1
> 
> 
> 

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

* Re: [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance
  2021-06-10 17:36     ` Chris Morgan
@ 2021-06-11  2:26       ` Jon Lin
       [not found]         ` <SN6PR06MB5342327048383CC3D416C93DA5349@SN6PR06MB5342.namprd06.prod.outlook.com>
  0 siblings, 1 reply; 20+ messages in thread
From: Jon Lin @ 2021-06-11  2:26 UTC (permalink / raw)
  To: Chris Morgan
  Cc: linux-spi, broonie, robh+dt, heiko, jbx6244, hjc, yifeng.zhao,
	sugar.zhang, linux-rockchip, linux-mtd, p.yadav, devicetree,
	linux-arm-kernel, linux-kernel, mturquette, sboyd, linux-clk,
	Chris Morgan

Hi Chris

May you attach the XT25F128B device code to me, and I'll try to work it out.

On 6/11/21 1:36 AM, Chris Morgan wrote:
> On Wed, Jun 09, 2021 at 10:13:48PM +0800, Jon Lin wrote:
>> From: Chris Morgan <macromorgan@hotmail.com>
>>
>> This enables the Rockchip Serial Flash Controller for the Odroid Go
>> Advance. Note that while the attached SPI NOR flash and the controller
>> both support quad read mode, only 2 of the required 4 pins are present.
>> The rx and tx bus width is set to 2 for this reason.
>>
>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
>> ---
>>
>> Changes in v7: None
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v3: None
>> Changes in v2: None
>> Changes in v1: None
>>
>>   .../boot/dts/rockchip/rk3326-odroid-go2.dts      | 16 ++++++++++++++++
>>   1 file changed, 16 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
>> index 49c97f76df77..f78e11dd8447 100644
>> --- a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
>> +++ b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
>> @@ -484,6 +484,22 @@
>>   	status = "okay";
>>   };
>>   
>> +&sfc {
>> +	pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus2>;
>> +	pinctrl-names = "default";
>> +	#address-cells = <1>;
>> +	#size-cells = <0>;
>> +	status = "okay";
>> +
>> +	flash@0 {
>> +		compatible = "jedec,spi-nor";
>> +		reg = <0>;
>> +		spi-max-frequency = <108000000>;
>> +		spi-rx-bus-width = <2>;
>> +		spi-tx-bus-width = <2>;
> Note that I am still working with Jon Lin to research this, but it was
> found in testing that if I set the tx bus width to 1 the problems I
> encountered in earlier are resolved. At this time I do not know if it
> is an issue with the driver for the flash controller, or if the NOR, or
> board itself has some sort of errata which prevent dual tx from working
> correctly. Note that as of right now the flash chip I am using (an
> XTX XT25F128B) is not currently supported in mainline, so it's very
> possible this is some sort of errata with the chip. It's also possible
> that there is something with the board that is interferring with dual
> mode TX.  When Jon comes back that he has tested dual mode on the SFC
> with a different board/chip I will recommend that we change the tx
> bus width here to a 1, and then once the XT25F128B gets mainlined we
> can see if someone else has issues with dual tx mode so we can note
> that as a problem with the chip. Or maybe there is something weird
> with dual tx mode yet on the SFC driver/controller, I don't know yet.
> I'm all too happy to work with a Rockchip engineer so things like
> this can be determined before we hit mainline. :-)
>
> The XTX25F128B driver is currently awaiting a decision on how to handle
> continuation codes, as this chip ID should be using continuation codes,
> but doesn't appear to return them when you query for manufacturer ID.
> So I should also note in the commit here that the SFC will still be
> unusable on the Odroid Go Advance until the XTX25F128B is also
> mainlined.
>
> Thank you.
>
>> +	};
>> +};
>> +
>>   &tsadc {
>>   	status = "okay";
>>   };
>> -- 
>> 2.17.1
>>
>>
>>
>
>



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

* Re: [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance
       [not found]         ` <SN6PR06MB5342327048383CC3D416C93DA5349@SN6PR06MB5342.namprd06.prod.outlook.com>
@ 2021-06-11  3:54           ` Jon Lin
  0 siblings, 0 replies; 20+ messages in thread
From: Jon Lin @ 2021-06-11  3:54 UTC (permalink / raw)
  To: Chris Morgan
  Cc: Chris Morgan, linux-spi, broonie, robh+dt, heiko, jbx6244, hjc,
	yifeng.zhao, sugar.zhang, linux-rockchip, linux-mtd, p.yadav,
	devicetree, linux-arm-kernel, linux-kernel, mturquette, sboyd,
	linux-clk


On 6/11/21 11:38 AM, Chris Morgan wrote:
> On Fri, Jun 11, 2021 at 10:26:35AM +0800, Jon Lin wrote:
>> Hi Chris
>>
>> May you attach the XT25F128B device code to me, and I'll try to work it out.
> Sure, here is the patch I am using:
>
> https://patchwork.ozlabs.org/project/linux-mtd/patch/SN6PR06MB5342C82F372F37FB8E21B327A57A9@SN6PR06MB5342.namprd06.prod.outlook.com/

this patch works well in my rk3308 tx-2 rx-2 XT25F128BSSIGU case.

# dd if=/tmp/rand.img of=/dev/mtdblock0 bs=4096 seek=1024
1024+0 records in
1024+0 records out
#
# dd if=/dev/mtd0 of=/tmp/rand1.img bs=4096 skip=1024 count=1024

1024+0 records in
1024+0 records out
#
#
# md5sum /tmp/*.img
83e45a56766168b47e6db1d41b1b403d  /tmp/rand.img
83e45a56766168b47e6db1d41b1b403d  /tmp/rand1.img
#
# dmesg | grep XT25F128BSSIGU
[    0.200738] spi-nor spi3.0: XT25F128BSSIGU (16384 Kbytes)
#

>
>> On 6/11/21 1:36 AM, Chris Morgan wrote:
>>> On Wed, Jun 09, 2021 at 10:13:48PM +0800, Jon Lin wrote:
>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>
>>>> This enables the Rockchip Serial Flash Controller for the Odroid Go
>>>> Advance. Note that while the attached SPI NOR flash and the controller
>>>> both support quad read mode, only 2 of the required 4 pins are present.
>>>> The rx and tx bus width is set to 2 for this reason.
>>>>
>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>>> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
>>>> ---
>>>>
>>>> Changes in v7: None
>>>> Changes in v6: None
>>>> Changes in v5: None
>>>> Changes in v4: None
>>>> Changes in v3: None
>>>> Changes in v2: None
>>>> Changes in v1: None
>>>>
>>>>    .../boot/dts/rockchip/rk3326-odroid-go2.dts      | 16 ++++++++++++++++
>>>>    1 file changed, 16 insertions(+)
>>>>
>>>> diff --git a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
>>>> index 49c97f76df77..f78e11dd8447 100644
>>>> --- a/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
>>>> +++ b/arch/arm64/boot/dts/rockchip/rk3326-odroid-go2.dts
>>>> @@ -484,6 +484,22 @@
>>>>    	status = "okay";
>>>>    };
>>>> +&sfc {
>>>> +	pinctrl-0 = <&sfc_clk &sfc_cs0 &sfc_bus2>;
>>>> +	pinctrl-names = "default";
>>>> +	#address-cells = <1>;
>>>> +	#size-cells = <0>;
>>>> +	status = "okay";
>>>> +
>>>> +	flash@0 {
>>>> +		compatible = "jedec,spi-nor";
>>>> +		reg = <0>;
>>>> +		spi-max-frequency = <108000000>;
>>>> +		spi-rx-bus-width = <2>;
>>>> +		spi-tx-bus-width = <2>;
>>> Note that I am still working with Jon Lin to research this, but it was
>>> found in testing that if I set the tx bus width to 1 the problems I
>>> encountered in earlier are resolved. At this time I do not know if it
>>> is an issue with the driver for the flash controller, or if the NOR, or
>>> board itself has some sort of errata which prevent dual tx from working
>>> correctly. Note that as of right now the flash chip I am using (an
>>> XTX XT25F128B) is not currently supported in mainline, so it's very
>>> possible this is some sort of errata with the chip. It's also possible
>>> that there is something with the board that is interferring with dual
>>> mode TX.  When Jon comes back that he has tested dual mode on the SFC
>>> with a different board/chip I will recommend that we change the tx
>>> bus width here to a 1, and then once the XT25F128B gets mainlined we
>>> can see if someone else has issues with dual tx mode so we can note
>>> that as a problem with the chip. Or maybe there is something weird
>>> with dual tx mode yet on the SFC driver/controller, I don't know yet.
>>> I'm all too happy to work with a Rockchip engineer so things like
>>> this can be determined before we hit mainline. :-)
>>>
>>> The XTX25F128B driver is currently awaiting a decision on how to handle
>>> continuation codes, as this chip ID should be using continuation codes,
>>> but doesn't appear to return them when you query for manufacturer ID.
>>> So I should also note in the commit here that the SFC will still be
>>> unusable on the Odroid Go Advance until the XTX25F128B is also
>>> mainlined.
>>>
>>> Thank you.
>>>
>>>> +	};
>>>> +};
>>>> +
>>>>    &tsadc {
>>>>    	status = "okay";
>>>>    };
>>>> -- 
>>>> 2.17.1
>>>>
>>>>
>>>>
>>>
>>
>
>



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

* Re: [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-10  3:02     ` Kever Yang
@ 2021-06-11 16:32       ` Ezequiel Garcia
  2021-06-16 15:38         ` Rob Herring
  0 siblings, 1 reply; 20+ messages in thread
From: Ezequiel Garcia @ 2021-06-11 16:32 UTC (permalink / raw)
  To: Kever Yang
  Cc: Rob Herring, Jon Lin, linux-spi, Mark Brown, Heiko Stuebner,
	Johan Jonker, hjc, yifeng.zhao, sugar.zhang,
	open list:ARM/Rockchip SoC...,
	linux-mtd, p.yadav, macroalpha82, devicetree, linux-arm-kernel,
	Linux Kernel Mailing List, Michael Turquette, Stephen Boyd,
	linux-clk, Chris Morgan

Hi all,

On Thu, 10 Jun 2021 at 00:04, Kever Yang <kever.yang@rock-chips.com> wrote:
>
> Hi Rob,
>
> On 2021/6/10 上午10:43, Rob Herring wrote:
> > On Wed, Jun 09, 2021 at 10:04:04PM +0800, Jon Lin wrote:
> >> From: Chris Morgan <macromorgan@hotmail.com>
> >>
> >> Add bindings for the Rockchip serial flash controller. New device
> >> specific parameter of rockchip,sfc-no-dma included in documentation.
> >>
> >> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> >> ---
> >>
> >> Changes in v7:
> >> - Fix up the sclk_sfc parent error in rk3036
> >> - Unify to "rockchip,sfc" compatible id because all the feature update
> >>    will have a new IP version, so the driver is used for the SFC IP in
> >>    all SoCs
> >> - Change to use node "sfc" to name the SFC pinctrl group
> >> - Add subnode reg property check
> >> - Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
> >> - Limit max_iosize to 32KB
> >>
> >> Changes in v6:
> >> - Add support in device trees for rv1126(Declared in series 5 but not
> >>    submitted)
> >> - Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
> >>    affect interpretation and has been widely used
> >> - Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
> >> - Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
> >>    in series 5 but not submitted)
> >> - Support SFC ver4 ver5(Declared in series 5 but not submitted)
> >> - Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
> >> - Change to use devm_spi_alloc_master and spi_unregister_master
> >>
> >> Changes in v5:
> >> - Add support in device trees for rv1126
> >> - Support sfc tx_dual, tx_quad
> >> - Simplify the code, such as remove "rockchip_sfc_register_all"
> >> - Support SFC ver4 ver5
> >>
> >> Changes in v4:
> >> - Changing patch back to an "RFC". An engineer from Rockchip
> >>    reached out to me to let me know they are working on this patch for
> >>    upstream, I am submitting this v4 for the community to see however
> >>    I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
> >>    soon and these are the ones we should pursue for mainlining. Jon's
> >>    patch series should include support for more hardware than this
> >>    series.
> >> - Clean up documentation more and ensure it is correct per
> >>    make dt_binding_check.
> >> - Add support in device trees for rk3036, rk3308, and rv1108.
> >> - Add ahb clock (hclk_sfc) support for rk3036.
> >> - Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
> >> - Change IRQ code to only mark IRQ as handled if it handles the
> >>    specific IRQ (DMA transfer finish) it is supposed to handle.
> >>
> >> Changes in v3:
> >> - Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
> >> - Changed the compatible string from rockchip,sfc to
> >>    rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
> >>    driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
> >>    RV1108 SoCs, and possibly more. However, I am currently only able
> >>    to test this on a PX30 (an RK3326). The technical reference manuals
> >>    appear to list the same registers for each device.
> >> - Corrected devicetree documentation for formatting and to note these
> >>    changes.
> >> - Replaced the maintainer with Heiko Stuebner and myself, as we will
> >>    take ownership of this going forward.
> >> - Noted that the device (per the reference manual) supports 4 CS, but
> >>    I am only able to test a single CS (CS 0).
> >> - Reordered patches to comply with upstream rules.
> >>
> >> Changes in v2:
> >> - Reimplemented driver using spi-mem subsystem.
> >> - Removed power management code as I couldn't get it working properly.
> >> - Added device tree bindings for Odroid Go Advance.
> >>
> >> Changes in v1:
> >> hanges made in this new series versus the v8 of the old series:
> >> - Added function to read spi-rx-bus-width from device tree, in the
> >>    event that the SPI chip supports 4x mode but only has 2 pins
> >>    wired (such as the Odroid Go Advance).
> >> - Changed device tree documentation from txt to yaml format.
> >> - Made "reset" message a dev_dbg from a dev_info.
> >> - Changed read and write fifo functions to remove redundant checks.
> >> - Changed the write and read from relaxed to non-relaxed when
> >>    starting the DMA transfer or reading the DMA IRQ.
> >> - Changed from dma_coerce_mask_and_coherent to just
> >>    dma_set_mask_and_coherent.
> >> - Changed name of get_if_type to rockchip_sfc_get_if_type.
> >>
> >>   .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
> >>   1 file changed, 88 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> >> new file mode 100644
> >> index 000000000000..42e4198e92af
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> >> @@ -0,0 +1,88 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +%YAML 1.2
> >> +---
> >> +$id: http://devicetree.org/schemas/spi/rockchip-sfc.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Rockchip Serial Flash Controller (SFC)
> >> +
> >> +maintainers:
> >> +  - Heiko Stuebner <heiko@sntech.de>
> >> +  - Chris Morgan <macromorgan@hotmail.com>
> >> +
> >> +allOf:
> >> +  - $ref: spi-controller.yaml#
> >> +
> >> +properties:
> >> +  compatible:
> >> +    oneOf:
> >> +      - const: rockchip,sfc
> > Use 'enum' instead of oneOf+const.
> >
> > You need an SoC specific compatible.
>
>
> The rockchip sfc controller is a standalone IP with version register,
> and the driver can
>
> handle all the feature difference inside the IP, so we would like to use
> a more generic
>
> compatible name instead of bind to any of SoC name. So can we use
> "rockchip,sfc"
>
> like "snps,designware-spi", which is a generic one, instead of an SoC
> specific compatible?
>

IIUC, the way this works is along these lines:

* The SFC driver can only care for the rockchip,sfc compatible string
and, if suitable, use the IP version register mentioned by Kever [1].

* The bindings doc specifies both the SoC-specific and the generic one
with:

      - items:
          - enum:
              - rockchip,px30-sfc
          - const: rockchip,sfc

* The device tree lists both as well:

compatible = "rockchip,px30-sfc", "rockchip,sfc";

This can apply to all IP cores really; and will allow some
compatibility between the downstream/vendor device tree
and upstream.

This scheme is indeed more convoluted than just
picking any SoC name for the compatible string, and
use that compatible string for all the SoCs (given they
are all compatible, again as per [1]).

IOW, you only have "rockchip,px30-sfc" in the bindings,
in the devicetree files and in the driver.

[1] https://lkml.org/lkml/2021/6/8/2030

Thanks!
Ezequiel

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

* Re: [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-11 16:32       ` Ezequiel Garcia
@ 2021-06-16 15:38         ` Rob Herring
  2021-06-17  1:51           ` Kever Yang
  0 siblings, 1 reply; 20+ messages in thread
From: Rob Herring @ 2021-06-16 15:38 UTC (permalink / raw)
  To: Ezequiel Garcia, Kever Yang
  Cc: Jon Lin, linux-spi, Mark Brown, Heiko Stuebner, Johan Jonker,
	黄家钗,
	Yifeng Zhao, Sugar Zhang, open list:ARM/Rockchip SoC...,
	linux-mtd, Pratyush Yadav, Chris Morgan, devicetree,
	linux-arm-kernel, Linux Kernel Mailing List, Michael Turquette,
	Stephen Boyd, linux-clk, Chris Morgan

On Fri, Jun 11, 2021 at 10:33 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Hi all,
>
> On Thu, 10 Jun 2021 at 00:04, Kever Yang <kever.yang@rock-chips.com> wrote:
> >
> > Hi Rob,
> >
> > On 2021/6/10 上午10:43, Rob Herring wrote:
> > > On Wed, Jun 09, 2021 at 10:04:04PM +0800, Jon Lin wrote:
> > >> From: Chris Morgan <macromorgan@hotmail.com>
> > >>
> > >> Add bindings for the Rockchip serial flash controller. New device
> > >> specific parameter of rockchip,sfc-no-dma included in documentation.
> > >>
> > >> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> > >> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
> > >> ---
> > >>
> > >> Changes in v7:
> > >> - Fix up the sclk_sfc parent error in rk3036
> > >> - Unify to "rockchip,sfc" compatible id because all the feature update
> > >>    will have a new IP version, so the driver is used for the SFC IP in
> > >>    all SoCs
> > >> - Change to use node "sfc" to name the SFC pinctrl group
> > >> - Add subnode reg property check
> > >> - Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
> > >> - Limit max_iosize to 32KB
> > >>
> > >> Changes in v6:
> > >> - Add support in device trees for rv1126(Declared in series 5 but not
> > >>    submitted)
> > >> - Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
> > >>    affect interpretation and has been widely used
> > >> - Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
> > >> - Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
> > >>    in series 5 but not submitted)
> > >> - Support SFC ver4 ver5(Declared in series 5 but not submitted)
> > >> - Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
> > >> - Change to use devm_spi_alloc_master and spi_unregister_master
> > >>
> > >> Changes in v5:
> > >> - Add support in device trees for rv1126
> > >> - Support sfc tx_dual, tx_quad
> > >> - Simplify the code, such as remove "rockchip_sfc_register_all"
> > >> - Support SFC ver4 ver5
> > >>
> > >> Changes in v4:
> > >> - Changing patch back to an "RFC". An engineer from Rockchip
> > >>    reached out to me to let me know they are working on this patch for
> > >>    upstream, I am submitting this v4 for the community to see however
> > >>    I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
> > >>    soon and these are the ones we should pursue for mainlining. Jon's
> > >>    patch series should include support for more hardware than this
> > >>    series.
> > >> - Clean up documentation more and ensure it is correct per
> > >>    make dt_binding_check.
> > >> - Add support in device trees for rk3036, rk3308, and rv1108.
> > >> - Add ahb clock (hclk_sfc) support for rk3036.
> > >> - Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
> > >> - Change IRQ code to only mark IRQ as handled if it handles the
> > >>    specific IRQ (DMA transfer finish) it is supposed to handle.
> > >>
> > >> Changes in v3:
> > >> - Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
> > >> - Changed the compatible string from rockchip,sfc to
> > >>    rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
> > >>    driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
> > >>    RV1108 SoCs, and possibly more. However, I am currently only able
> > >>    to test this on a PX30 (an RK3326). The technical reference manuals
> > >>    appear to list the same registers for each device.
> > >> - Corrected devicetree documentation for formatting and to note these
> > >>    changes.
> > >> - Replaced the maintainer with Heiko Stuebner and myself, as we will
> > >>    take ownership of this going forward.
> > >> - Noted that the device (per the reference manual) supports 4 CS, but
> > >>    I am only able to test a single CS (CS 0).
> > >> - Reordered patches to comply with upstream rules.
> > >>
> > >> Changes in v2:
> > >> - Reimplemented driver using spi-mem subsystem.
> > >> - Removed power management code as I couldn't get it working properly.
> > >> - Added device tree bindings for Odroid Go Advance.
> > >>
> > >> Changes in v1:
> > >> hanges made in this new series versus the v8 of the old series:
> > >> - Added function to read spi-rx-bus-width from device tree, in the
> > >>    event that the SPI chip supports 4x mode but only has 2 pins
> > >>    wired (such as the Odroid Go Advance).
> > >> - Changed device tree documentation from txt to yaml format.
> > >> - Made "reset" message a dev_dbg from a dev_info.
> > >> - Changed read and write fifo functions to remove redundant checks.
> > >> - Changed the write and read from relaxed to non-relaxed when
> > >>    starting the DMA transfer or reading the DMA IRQ.
> > >> - Changed from dma_coerce_mask_and_coherent to just
> > >>    dma_set_mask_and_coherent.
> > >> - Changed name of get_if_type to rockchip_sfc_get_if_type.
> > >>
> > >>   .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
> > >>   1 file changed, 88 insertions(+)
> > >>   create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> > >>
> > >> diff --git a/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> > >> new file mode 100644
> > >> index 000000000000..42e4198e92af
> > >> --- /dev/null
> > >> +++ b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
> > >> @@ -0,0 +1,88 @@
> > >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > >> +%YAML 1.2
> > >> +---
> > >> +$id: http://devicetree.org/schemas/spi/rockchip-sfc.yaml#
> > >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > >> +
> > >> +title: Rockchip Serial Flash Controller (SFC)
> > >> +
> > >> +maintainers:
> > >> +  - Heiko Stuebner <heiko@sntech.de>
> > >> +  - Chris Morgan <macromorgan@hotmail.com>
> > >> +
> > >> +allOf:
> > >> +  - $ref: spi-controller.yaml#
> > >> +
> > >> +properties:
> > >> +  compatible:
> > >> +    oneOf:
> > >> +      - const: rockchip,sfc
> > > Use 'enum' instead of oneOf+const.
> > >
> > > You need an SoC specific compatible.
> >
> >
> > The rockchip sfc controller is a standalone IP with version register,
> > and the driver can
> >
> > handle all the feature difference inside the IP, so we would like to use
> > a more generic

Okay, if the version register can be relied on, then this is fine.
Just add a comment that further differentiation is done using a
version register.

> >
> > compatible name instead of bind to any of SoC name. So can we use
> > "rockchip,sfc"
> >
> > like "snps,designware-spi", which is a generic one, instead of an SoC
> > specific compatible?

That's a licensed IP which is a bit different. Though generic names on
those are useless too. There's different versions and different
integration quirks.

> >
>
> IIUC, the way this works is along these lines:
>
> * The SFC driver can only care for the rockchip,sfc compatible string
> and, if suitable, use the IP version register mentioned by Kever [1].
>
> * The bindings doc specifies both the SoC-specific and the generic one
> with:
>
>       - items:
>           - enum:
>               - rockchip,px30-sfc
>           - const: rockchip,sfc
>
> * The device tree lists both as well:
>
> compatible = "rockchip,px30-sfc", "rockchip,sfc";
>
> This can apply to all IP cores really; and will allow some
> compatibility between the downstream/vendor device tree
> and upstream.
>
> This scheme is indeed more convoluted than just
> picking any SoC name for the compatible string, and
> use that compatible string for all the SoCs (given they
> are all compatible, again as per [1]).
>
> IOW, you only have "rockchip,px30-sfc" in the bindings,
> in the devicetree files and in the driver.

This is fine too, but again if a version or capability register is
sufficient, no need to put this into DT. Maybe someday h/w designers
will clue in and always have version and/or capability registers.

Rob

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

* Re: [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller
  2021-06-16 15:38         ` Rob Herring
@ 2021-06-17  1:51           ` Kever Yang
  0 siblings, 0 replies; 20+ messages in thread
From: Kever Yang @ 2021-06-17  1:51 UTC (permalink / raw)
  To: Rob Herring, Ezequiel Garcia
  Cc: Jon Lin, linux-spi, Mark Brown, Heiko Stuebner, Johan Jonker,
	黄家钗,
	Yifeng Zhao, Sugar Zhang, open list:ARM/Rockchip SoC...,
	linux-mtd, Pratyush Yadav, Chris Morgan, devicetree,
	linux-arm-kernel, Linux Kernel Mailing List, Michael Turquette,
	Stephen Boyd, linux-clk, Chris Morgan

Hi Rob,

On 2021/6/16 下午11:38, Rob Herring wrote:
> On Fri, Jun 11, 2021 at 10:33 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
>> Hi all,
>>
>> On Thu, 10 Jun 2021 at 00:04, Kever Yang <kever.yang@rock-chips.com> wrote:
>>> Hi Rob,
>>>
>>> On 2021/6/10 上午10:43, Rob Herring wrote:
>>>> On Wed, Jun 09, 2021 at 10:04:04PM +0800, Jon Lin wrote:
>>>>> From: Chris Morgan <macromorgan@hotmail.com>
>>>>>
>>>>> Add bindings for the Rockchip serial flash controller. New device
>>>>> specific parameter of rockchip,sfc-no-dma included in documentation.
>>>>>
>>>>> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>>>>> Signed-off-by: Jon Lin <jon.lin@rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v7:
>>>>> - Fix up the sclk_sfc parent error in rk3036
>>>>> - Unify to "rockchip,sfc" compatible id because all the feature update
>>>>>     will have a new IP version, so the driver is used for the SFC IP in
>>>>>     all SoCs
>>>>> - Change to use node "sfc" to name the SFC pinctrl group
>>>>> - Add subnode reg property check
>>>>> - Add rockchip_sfc_adjust_op_size to workaround in CMD + DUMMY case
>>>>> - Limit max_iosize to 32KB
>>>>>
>>>>> Changes in v6:
>>>>> - Add support in device trees for rv1126(Declared in series 5 but not
>>>>>     submitted)
>>>>> - Change to use "clk_sfc" "hclk_sfc" as clock lable, since it does not
>>>>>     affect interpretation and has been widely used
>>>>> - Support sfc tx_dual, tx_quad(Declared in series 5 but not submitted)
>>>>> - Simplify the code, such as remove "rockchip_sfc_register_all"(Declared
>>>>>     in series 5 but not submitted)
>>>>> - Support SFC ver4 ver5(Declared in series 5 but not submitted)
>>>>> - Add author Chris Morgan and Jon Lin to spi-rockchip-sfc.c
>>>>> - Change to use devm_spi_alloc_master and spi_unregister_master
>>>>>
>>>>> Changes in v5:
>>>>> - Add support in device trees for rv1126
>>>>> - Support sfc tx_dual, tx_quad
>>>>> - Simplify the code, such as remove "rockchip_sfc_register_all"
>>>>> - Support SFC ver4 ver5
>>>>>
>>>>> Changes in v4:
>>>>> - Changing patch back to an "RFC". An engineer from Rockchip
>>>>>     reached out to me to let me know they are working on this patch for
>>>>>     upstream, I am submitting this v4 for the community to see however
>>>>>     I expect Jon Lin (jon.lin@rock-chips.com) will submit new patches
>>>>>     soon and these are the ones we should pursue for mainlining. Jon's
>>>>>     patch series should include support for more hardware than this
>>>>>     series.
>>>>> - Clean up documentation more and ensure it is correct per
>>>>>     make dt_binding_check.
>>>>> - Add support in device trees for rk3036, rk3308, and rv1108.
>>>>> - Add ahb clock (hclk_sfc) support for rk3036.
>>>>> - Change rockchip_sfc_wait_fifo_ready() to use a switch statement.
>>>>> - Change IRQ code to only mark IRQ as handled if it handles the
>>>>>     specific IRQ (DMA transfer finish) it is supposed to handle.
>>>>>
>>>>> Changes in v3:
>>>>> - Changed the name of the clocks to sfc/ahb (from clk-sfc/clk-hsfc).
>>>>> - Changed the compatible string from rockchip,sfc to
>>>>>     rockchip,rk3036-sfc. A quick glance at the datasheets suggests this
>>>>>     driver should work for the PX30, RK180x, RK3036, RK312x, RK3308 and
>>>>>     RV1108 SoCs, and possibly more. However, I am currently only able
>>>>>     to test this on a PX30 (an RK3326). The technical reference manuals
>>>>>     appear to list the same registers for each device.
>>>>> - Corrected devicetree documentation for formatting and to note these
>>>>>     changes.
>>>>> - Replaced the maintainer with Heiko Stuebner and myself, as we will
>>>>>     take ownership of this going forward.
>>>>> - Noted that the device (per the reference manual) supports 4 CS, but
>>>>>     I am only able to test a single CS (CS 0).
>>>>> - Reordered patches to comply with upstream rules.
>>>>>
>>>>> Changes in v2:
>>>>> - Reimplemented driver using spi-mem subsystem.
>>>>> - Removed power management code as I couldn't get it working properly.
>>>>> - Added device tree bindings for Odroid Go Advance.
>>>>>
>>>>> Changes in v1:
>>>>> hanges made in this new series versus the v8 of the old series:
>>>>> - Added function to read spi-rx-bus-width from device tree, in the
>>>>>     event that the SPI chip supports 4x mode but only has 2 pins
>>>>>     wired (such as the Odroid Go Advance).
>>>>> - Changed device tree documentation from txt to yaml format.
>>>>> - Made "reset" message a dev_dbg from a dev_info.
>>>>> - Changed read and write fifo functions to remove redundant checks.
>>>>> - Changed the write and read from relaxed to non-relaxed when
>>>>>     starting the DMA transfer or reading the DMA IRQ.
>>>>> - Changed from dma_coerce_mask_and_coherent to just
>>>>>     dma_set_mask_and_coherent.
>>>>> - Changed name of get_if_type to rockchip_sfc_get_if_type.
>>>>>
>>>>>    .../devicetree/bindings/spi/rockchip-sfc.yaml | 88 +++++++++++++++++++
>>>>>    1 file changed, 88 insertions(+)
>>>>>    create mode 100644 Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
>>>>> new file mode 100644
>>>>> index 000000000000..42e4198e92af
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/spi/rockchip-sfc.yaml
>>>>> @@ -0,0 +1,88 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/spi/rockchip-sfc.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: Rockchip Serial Flash Controller (SFC)
>>>>> +
>>>>> +maintainers:
>>>>> +  - Heiko Stuebner <heiko@sntech.de>
>>>>> +  - Chris Morgan <macromorgan@hotmail.com>
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: spi-controller.yaml#
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    oneOf:
>>>>> +      - const: rockchip,sfc
>>>> Use 'enum' instead of oneOf+const.
>>>>
>>>> You need an SoC specific compatible.
>>>
>>> The rockchip sfc controller is a standalone IP with version register,
>>> and the driver can
>>>
>>> handle all the feature difference inside the IP, so we would like to use
>>> a more generic
> Okay, if the version register can be relied on, then this is fine.
> Just add a comment that further differentiation is done using a
> version register.


Thanks for your confirm, this will make things much simple for driver 
maintain.

@Jon, please update your patch per Rob's requirement.


Thanks,
- Kever



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

end of thread, other threads:[~2021-06-17  1:51 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-09 14:04 [PATCH v7 0/9] Add Rockchip SFC(serial flash controller) support Jon Lin
2021-06-09 14:04 ` [PATCH v7 1/9] dt-bindings: rockchip-sfc: Bindings for Rockchip serial flash controller Jon Lin
2021-06-09 16:16   ` Rob Herring
2021-06-10  2:43   ` Rob Herring
2021-06-10  3:02     ` Kever Yang
2021-06-11 16:32       ` Ezequiel Garcia
2021-06-16 15:38         ` Rob Herring
2021-06-17  1:51           ` Kever Yang
2021-06-09 14:04 ` [PATCH v7 2/9] spi: rockchip-sfc: add rockchip " Jon Lin
2021-06-09 18:45   ` Chris Morgan
2021-06-09 14:04 ` [PATCH v7 3/9] arm64: dts: rockchip: Add SFC to PX30 Jon Lin
2021-06-09 14:04 ` [PATCH v7 4/9] clk: rockchip: rk3036: fix up the sclk_sfc parent error Jon Lin
2021-06-09 14:13 ` [PATCH v7 5/9] clk: rockchip: Add support for hclk_sfc on rk3036 Jon Lin
2021-06-09 14:13   ` [PATCH v7 6/9] arm: dts: rockchip: Add SFC to RK3036 Jon Lin
2021-06-09 14:13   ` [PATCH v7 7/9] arm: dts: rockchip: Add SFC to RV1108 Jon Lin
2021-06-09 14:13   ` [PATCH v7 8/9] arm64: dts: rockchip: Add SFC to RK3308 Jon Lin
2021-06-09 14:13   ` [PATCH v7 9/9] arm64: dts: rockchip: Enable SFC for Odroid Go Advance Jon Lin
2021-06-10 17:36     ` Chris Morgan
2021-06-11  2:26       ` Jon Lin
     [not found]         ` <SN6PR06MB5342327048383CC3D416C93DA5349@SN6PR06MB5342.namprd06.prod.outlook.com>
2021-06-11  3:54           ` Jon Lin

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