devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] mtd: rawnand: brcmnand: driver and doc updates
@ 2023-06-06 23:12 William Zhang
  2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: William Zhang @ 2023-06-06 23:12 UTC (permalink / raw)
  To: Broadcom Kernel List, Linux MTD List
  Cc: f.fainelli, rafal, kursad.oney, joel.peshkin, computersforpeace,
	anand.gore, dregan, kamal.dasu, tomer.yacoby, dan.beygelman,
	William Zhang, devicetree, Rob Herring, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Miquel Raynal,
	Richard Weinberger, linux-arm-kernel, Boris Brezillon, Kamal Dasu,
	Rob Herring

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

This patch series include the accumulative updates and fixes for the
brcmnand driver. The device tree document is also updated accordingly
with the new properties needed by the driver.


William Zhang (12):
  mtd: rawnand: brcmnand: Fix ECC level field setting for v7.2 controller
  mtd: rawnand: brcmnand: Fix potential false time out warning
  mtd: rawnand: brcmnand: Fix crash during the panic_write
  mtd: rawnand: brcmnand: Fix potential out-of-bounds access in oob write
  dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  ARM: dts: broadcom: bcmbca: Add NAND controller node
  arm64: dts: broadcom: bcmbca: Add NAND controller node
  mtd: rawnand: brcmnand: Rename bcm63138 nand driver
  mtd: rawnand: brcmnand: Add new compatible string
  mtd: rawnand: brcmnand: Add BCMBCA read data bus interface
  mtd: rawnand: brcmnand: Add support for getting ecc setting from strap
  mtd: rawnand: brcmnand: Support write protection setting from dts

 .../bindings/mtd/brcm,brcmnand.yaml           |  64 ++++---
 arch/arm/boot/dts/bcm47622.dtsi               |  17 ++
 arch/arm/boot/dts/bcm63138.dtsi               |  12 +-
 arch/arm/boot/dts/bcm63148.dtsi               |  17 ++
 arch/arm/boot/dts/bcm63178.dtsi               |  17 ++
 arch/arm/boot/dts/bcm6756.dtsi                |  17 ++
 arch/arm/boot/dts/bcm6846.dtsi                |  17 ++
 arch/arm/boot/dts/bcm6855.dtsi                |  17 ++
 arch/arm/boot/dts/bcm6878.dtsi                |  17 ++
 arch/arm/boot/dts/bcm947622.dts               |   4 +
 arch/arm/boot/dts/bcm963138.dts               |   4 +
 arch/arm/boot/dts/bcm963138dvt.dts            |  12 +-
 arch/arm/boot/dts/bcm963148.dts               |   4 +
 arch/arm/boot/dts/bcm963178.dts               |   4 +
 arch/arm/boot/dts/bcm96756.dts                |   4 +
 arch/arm/boot/dts/bcm96846.dts                |   4 +
 arch/arm/boot/dts/bcm96855.dts                |   4 +
 arch/arm/boot/dts/bcm96878.dts                |   4 +
 .../boot/dts/broadcom/bcmbca/bcm4908.dtsi     |   5 +-
 .../boot/dts/broadcom/bcmbca/bcm4912.dtsi     |  17 ++
 .../boot/dts/broadcom/bcmbca/bcm63146.dtsi    |  17 ++
 .../boot/dts/broadcom/bcmbca/bcm63158.dtsi    |  17 ++
 .../boot/dts/broadcom/bcmbca/bcm6813.dtsi     |  17 ++
 .../boot/dts/broadcom/bcmbca/bcm6856.dtsi     |  17 ++
 .../boot/dts/broadcom/bcmbca/bcm6858.dtsi     |  17 ++
 .../boot/dts/broadcom/bcmbca/bcm94912.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm963146.dts    |   4 +
 .../boot/dts/broadcom/bcmbca/bcm963158.dts    |   4 +
 .../boot/dts/broadcom/bcmbca/bcm96813.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm96856.dts     |   4 +
 .../boot/dts/broadcom/bcmbca/bcm96858.dts     |   4 +
 drivers/mtd/nand/raw/brcmnand/Makefile        |   2 +-
 drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c | 101 -----------
 drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c   | 138 +++++++++++++++
 drivers/mtd/nand/raw/brcmnand/brcmnand.c      | 166 ++++++++++++++----
 drivers/mtd/nand/raw/brcmnand/brcmnand.h      |   2 +
 36 files changed, 615 insertions(+), 164 deletions(-)
 delete mode 100644 drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c
 create mode 100644 drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c

-- 
2.37.3


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-06 23:12 [PATCH 00/12] mtd: rawnand: brcmnand: driver and doc updates William Zhang
@ 2023-06-06 23:12 ` William Zhang
  2023-06-07  8:14   ` Miquel Raynal
  2023-06-14 22:43   ` Rob Herring
  2023-06-06 23:12 ` [PATCH 06/12] ARM: dts: broadcom: bcmbca: Add NAND controller node William Zhang
  2023-06-06 23:12 ` [PATCH 07/12] arm64: " William Zhang
  2 siblings, 2 replies; 13+ messages in thread
From: William Zhang @ 2023-06-06 23:12 UTC (permalink / raw)
  To: Broadcom Kernel List, Linux MTD List
  Cc: f.fainelli, rafal, kursad.oney, joel.peshkin, computersforpeace,
	anand.gore, dregan, kamal.dasu, tomer.yacoby, dan.beygelman,
	William Zhang, devicetree, linux-kernel, Krzysztof Kozlowski,
	Vignesh Raghavendra, Miquel Raynal, Richard Weinberger,
	Kamal Dasu, Rob Herring

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

Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
product. The old compatible string is still kept in the driver so old
dtb can still work.

Add brcm,nand-use-wp property to have an option for disabling this
feature on broadband board design that does not use write protection.
Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
broadband board designs because they do not specify ecc setting in dts
but rather using the strap setting.

Remove the requirement of interrupts and interrupt-names properties to
reflect the driver code.

This patch also includes a few minor fixes to the BCM63xx compatibles
and add myself to the list of maintainers.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
 1 file changed, 43 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index 1571024aa119..1fe1c166a9db 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
 maintainers:
   - Brian Norris <computersforpeace@gmail.com>
   - Kamal Dasu <kdasu.kdev@gmail.com>
+  - William Zhang <william.zhang@broadcom.com>
 
 description: |
   The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
@@ -18,9 +19,10 @@ description: |
   supports basic PROGRAM and READ functions, among other features.
 
   This controller was originally designed for STB SoCs (BCM7xxx) but is now
-  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
-  iProc/Cygnus. Its history includes several similar (but not fully register
-  compatible) versions.
+  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
+  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
+  Its history includes several similar (but not fully register compatible)
+  versions.
 
   -- Additional SoC-specific NAND controller properties --
 
@@ -53,9 +55,9 @@ properties:
               - brcm,brcmnand-v7.2
               - brcm,brcmnand-v7.3
           - const: brcm,brcmnand
-      - description: BCM63138 SoC-specific NAND controller
+      - description: BCMBCA SoC-specific NAND controller
         items:
-          - const: brcm,nand-bcm63138
+          - const: brcm,nand-bcmbca
           - enum:
               - brcm,brcmnand-v7.0
               - brcm,brcmnand-v7.1
@@ -65,11 +67,15 @@ properties:
           - const: brcm,nand-iproc
           - const: brcm,brcmnand-v6.1
           - const: brcm,brcmnand
-      - description: BCM63168 SoC-specific NAND controller
+      - description: BCM63xx SoC-specific NAND controller
         items:
-          - const: brcm,nand-bcm63168
-          - const: brcm,nand-bcm6368
-          - const: brcm,brcmnand-v4.0
+          - enum:
+              - brcm,nand-bcm63168
+              - brcm,nand-bcm6368
+          - enum:
+              - brcm,brcmnand-v2.1
+              - brcm,brcmnand-v2.2
+              - brcm,brcmnand-v4.0
           - const: brcm,brcmnand
 
   reg:
@@ -111,6 +117,19 @@ properties:
       earlier versions of this core that include WP
     type: boolean
 
+  brcm,nand-use-wp:
+    description:
+      Use this integer to indicate if board design uses
+      controller's write protection feature and connects its
+      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
+      use this feature when this property does not exist.
+      Set to 0 if WP pins are not connected and feature is not
+      used. Set to 1 if WP pins are connected and feature is used.
+      Set to 2 if WP pins are connected but disable this feature
+      through driver that sets controller to output high on NAND_WPb.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 1, 2]
+
 patternProperties:
   "^nand@[a-f0-9]$":
     type: object
@@ -136,13 +155,23 @@ patternProperties:
           layout.
         $ref: /schemas/types.yaml#/definitions/uint32
 
+      brcm,nand-ecc-use-strap:
+        description:
+          This flag is used by the driver to get the ecc strength and
+          spare area size from the SoC NAND boot strap setting. This
+          is commonly used by the BCMBCA SoC board design. If ecc
+          strength and spare area size are set by nand-ecc-strength
+          and brcm,nand-oob-sector-size in the dts, these settings
+          have precedence and override this flag.
+        $ref: /schemas/types.yaml#/definitions/flag
+
 allOf:
   - $ref: nand-controller.yaml#
   - if:
       properties:
         compatible:
           contains:
-            const: brcm,nand-bcm63138
+            const: brcm,nand-bcmbca
     then:
       properties:
         reg-names:
@@ -153,7 +182,9 @@ allOf:
       properties:
         compatible:
           contains:
-            const: brcm,nand-bcm6368
+            enum:
+              - brcm,nand-bcm63168
+              - brcm,nand-bcm6368
     then:
       properties:
         reg-names:
@@ -173,20 +204,12 @@ allOf:
             - const: nand
             - const: iproc-idm
             - const: iproc-ext
-  - if:
-      properties:
-        interrupts:
-          minItems: 2
-    then:
-      required:
-        - interrupt-names
 
 unevaluatedProperties: false
 
 required:
   - reg
   - reg-names
-  - interrupts
 
 examples:
   - |
@@ -215,8 +238,7 @@ examples:
     };
   - |
     nand-controller@10000200 {
-        compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
-                     "brcm,brcmnand-v4.0", "brcm,brcmnand";
+        compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand";
         reg = <0x10000200 0x180>,
               <0x100000b0 0x10>,
               <0x10000600 0x200>;
-- 
2.37.3


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* [PATCH 06/12] ARM: dts: broadcom: bcmbca: Add NAND controller node
  2023-06-06 23:12 [PATCH 00/12] mtd: rawnand: brcmnand: driver and doc updates William Zhang
  2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
@ 2023-06-06 23:12 ` William Zhang
  2023-06-06 23:12 ` [PATCH 07/12] arm64: " William Zhang
  2 siblings, 0 replies; 13+ messages in thread
From: William Zhang @ 2023-06-06 23:12 UTC (permalink / raw)
  To: Broadcom Kernel List, Linux MTD List
  Cc: f.fainelli, rafal, kursad.oney, joel.peshkin, computersforpeace,
	anand.gore, dregan, kamal.dasu, tomer.yacoby, dan.beygelman,
	William Zhang, devicetree, linux-kernel, Krzysztof Kozlowski,
	Rob Herring, linux-arm-kernel

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

Add support for Broadcom STB NAND controller in BCMBCA ARMv7 chip dts 
files.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 arch/arm/boot/dts/bcm47622.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm63138.dtsi    | 12 ++++++++++--
 arch/arm/boot/dts/bcm63148.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm63178.dtsi    | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm6756.dtsi     | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm6846.dtsi     | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm6855.dtsi     | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm6878.dtsi     | 17 +++++++++++++++++
 arch/arm/boot/dts/bcm947622.dts    |  4 ++++
 arch/arm/boot/dts/bcm963138.dts    |  4 ++++
 arch/arm/boot/dts/bcm963138dvt.dts | 12 +++++-------
 arch/arm/boot/dts/bcm963148.dts    |  4 ++++
 arch/arm/boot/dts/bcm963178.dts    |  4 ++++
 arch/arm/boot/dts/bcm96756.dts     |  4 ++++
 arch/arm/boot/dts/bcm96846.dts     |  4 ++++
 arch/arm/boot/dts/bcm96855.dts     |  4 ++++
 arch/arm/boot/dts/bcm96878.dts     |  4 ++++
 17 files changed, 166 insertions(+), 9 deletions(-)

diff --git a/arch/arm/boot/dts/bcm47622.dtsi b/arch/arm/boot/dts/bcm47622.dtsi
index cd25ed2757b7..401e1ce1da6d 100644
--- a/arch/arm/boot/dts/bcm47622.dtsi
+++ b/arch/arm/boot/dts/bcm47622.dtsi
@@ -137,6 +137,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm63138.dtsi b/arch/arm/boot/dts/bcm63138.dtsi
index 93281c47c9ba..2c9939e775fb 100644
--- a/arch/arm/boot/dts/bcm63138.dtsi
+++ b/arch/arm/boot/dts/bcm63138.dtsi
@@ -224,12 +224,20 @@ hsspi: spi@1000 {
 		nand_controller: nand-controller@2000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-			compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.0", "brcm,brcmnand";
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.0", "brcm,brcmnand";
 			reg = <0x2000 0x600>, <0xf0 0x10>;
 			reg-names = "nand", "nand-int-base";
 			status = "disabled";
 			interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
-			interrupt-names = "nand";
+			interrupt-names = "nand_ctlrdy";
+			brcm,nand-use-wp = <0>;
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
 		};
 
 		bootlut: bootlut@8000 {
diff --git a/arch/arm/boot/dts/bcm63148.dtsi b/arch/arm/boot/dts/bcm63148.dtsi
index ba7f265db121..de14d4564b14 100644
--- a/arch/arm/boot/dts/bcm63148.dtsi
+++ b/arch/arm/boot/dts/bcm63148.dtsi
@@ -118,5 +118,22 @@ hsspi: spi@1000 {
 			num-cs = <8>;
 			status = "disabled";
 		};
+
+		nand_controller: nand-controller@2000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x2000 0x600>, <0xf0 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/bcm63178.dtsi b/arch/arm/boot/dts/bcm63178.dtsi
index d8268a1e889b..ae205408c5cd 100644
--- a/arch/arm/boot/dts/bcm63178.dtsi
+++ b/arch/arm/boot/dts/bcm63178.dtsi
@@ -128,6 +128,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm6756.dtsi b/arch/arm/boot/dts/bcm6756.dtsi
index 49ecc1f0c18c..bbff47172dc1 100644
--- a/arch/arm/boot/dts/bcm6756.dtsi
+++ b/arch/arm/boot/dts/bcm6756.dtsi
@@ -138,6 +138,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm6846.dtsi b/arch/arm/boot/dts/bcm6846.dtsi
index fbc7d3a5dc5f..26a36a577b44 100644
--- a/arch/arm/boot/dts/bcm6846.dtsi
+++ b/arch/arm/boot/dts/bcm6846.dtsi
@@ -118,5 +118,22 @@ hsspi: spi@1000 {
 			num-cs = <8>;
 			status = "disabled";
 		};
+
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
 	};
 };
diff --git a/arch/arm/boot/dts/bcm6855.dtsi b/arch/arm/boot/dts/bcm6855.dtsi
index 5e0fe26530f1..0defcc10ca8a 100644
--- a/arch/arm/boot/dts/bcm6855.dtsi
+++ b/arch/arm/boot/dts/bcm6855.dtsi
@@ -128,6 +128,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm6878.dtsi b/arch/arm/boot/dts/bcm6878.dtsi
index 96529d3d4dc2..f6ae07fe1b44 100644
--- a/arch/arm/boot/dts/bcm6878.dtsi
+++ b/arch/arm/boot/dts/bcm6878.dtsi
@@ -119,6 +119,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/bcm947622.dts b/arch/arm/boot/dts/bcm947622.dts
index 93b8ce22678d..22e3c4508e1a 100644
--- a/arch/arm/boot/dts/bcm947622.dts
+++ b/arch/arm/boot/dts/bcm947622.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963138.dts b/arch/arm/boot/dts/bcm963138.dts
index 1b405c249213..450289d47dc7 100644
--- a/arch/arm/boot/dts/bcm963138.dts
+++ b/arch/arm/boot/dts/bcm963138.dts
@@ -29,3 +29,7 @@ &serial0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963138dvt.dts b/arch/arm/boot/dts/bcm963138dvt.dts
index b5af61853a07..f2140e512070 100644
--- a/arch/arm/boot/dts/bcm963138dvt.dts
+++ b/arch/arm/boot/dts/bcm963138dvt.dts
@@ -33,14 +33,12 @@ &serial1 {
 
 &nand_controller {
 	status = "okay";
+};
 
-	nand@0 {
-		compatible = "brcm,nandcs";
-		reg = <0>;
-		nand-ecc-strength = <4>;
-		nand-ecc-step-size = <512>;
-		brcm,nand-oob-sectors-size = <16>;
-	};
+&nandcs {
+	nand-ecc-strength = <4>;
+	nand-ecc-step-size = <512>;
+	brcm,nand-oob-sectors-size = <16>;
 };
 
 &ahci {
diff --git a/arch/arm/boot/dts/bcm963148.dts b/arch/arm/boot/dts/bcm963148.dts
index 1f5d6d783f09..aa08b473c7cd 100644
--- a/arch/arm/boot/dts/bcm963148.dts
+++ b/arch/arm/boot/dts/bcm963148.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm963178.dts b/arch/arm/boot/dts/bcm963178.dts
index d036e99dd8d1..c0f504ac43a4 100644
--- a/arch/arm/boot/dts/bcm963178.dts
+++ b/arch/arm/boot/dts/bcm963178.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96756.dts b/arch/arm/boot/dts/bcm96756.dts
index 8b104f3fb14a..2ce998f2b84f 100644
--- a/arch/arm/boot/dts/bcm96756.dts
+++ b/arch/arm/boot/dts/bcm96756.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96846.dts b/arch/arm/boot/dts/bcm96846.dts
index 55852c229608..f4b9a07370ee 100644
--- a/arch/arm/boot/dts/bcm96846.dts
+++ b/arch/arm/boot/dts/bcm96846.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96855.dts b/arch/arm/boot/dts/bcm96855.dts
index 2ad880af2104..5c94063bceaf 100644
--- a/arch/arm/boot/dts/bcm96855.dts
+++ b/arch/arm/boot/dts/bcm96855.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/bcm96878.dts b/arch/arm/boot/dts/bcm96878.dts
index b7af8ade7a9d..910f7e125bad 100644
--- a/arch/arm/boot/dts/bcm96878.dts
+++ b/arch/arm/boot/dts/bcm96878.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
-- 
2.37.3


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* [PATCH 07/12] arm64: dts: broadcom: bcmbca: Add NAND controller node
  2023-06-06 23:12 [PATCH 00/12] mtd: rawnand: brcmnand: driver and doc updates William Zhang
  2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
  2023-06-06 23:12 ` [PATCH 06/12] ARM: dts: broadcom: bcmbca: Add NAND controller node William Zhang
@ 2023-06-06 23:12 ` William Zhang
  2 siblings, 0 replies; 13+ messages in thread
From: William Zhang @ 2023-06-06 23:12 UTC (permalink / raw)
  To: Broadcom Kernel List, Linux MTD List
  Cc: f.fainelli, rafal, kursad.oney, joel.peshkin, computersforpeace,
	anand.gore, dregan, kamal.dasu, tomer.yacoby, dan.beygelman,
	William Zhang, devicetree, linux-kernel, Krzysztof Kozlowski,
	Rob Herring, linux-arm-kernel

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

Add support for Broadcom STB NAND controller in BCMBCA ARMv8 chip dts 
files.

Signed-off-by: William Zhang <william.zhang@broadcom.com>
---

 .../arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi |  5 ++++-
 .../arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi | 17 +++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm63146.dtsi      | 17 +++++++++++++++++
 .../boot/dts/broadcom/bcmbca/bcm63158.dtsi      | 17 +++++++++++++++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi | 17 +++++++++++++++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi | 17 +++++++++++++++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi | 17 +++++++++++++++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm94912.dts |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm963146.dts      |  4 ++++
 .../boot/dts/broadcom/bcmbca/bcm963158.dts      |  4 ++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm96813.dts |  4 ++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm96856.dts |  4 ++++
 .../arm64/boot/dts/broadcom/bcmbca/bcm96858.dts |  4 ++++
 13 files changed, 130 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
index 457805efb385..acfedff89d19 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
@@ -591,16 +591,19 @@ hsspi: spi@1000{
 		nand-controller@1800 {
 			#address-cells = <1>;
 			#size-cells = <0>;
-			compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
 			reg = <0x1800 0x600>, <0x2000 0x10>;
 			reg-names = "nand", "nand-int-base";
 			interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
 			interrupt-names = "nand_ctlrdy";
+			brcm,nand-use-wp = <0>;
 			status = "okay";
 
 			nandcs: nand@0 {
 				compatible = "brcm,nandcs";
 				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
 			};
 		};
 
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
index 46aa8c0b7971..7c611c1978ac 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
@@ -137,6 +137,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
index 7020f2e995e2..faf8b1198d8e 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
@@ -118,6 +118,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
index 6a0242cbea57..24c344ed5dba 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
@@ -136,6 +136,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
index 1a12905266ef..c3416146c946 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
@@ -137,6 +137,23 @@ hsspi: spi@1000 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		uart0: serial@12000 {
 			compatible = "arm,pl011", "arm,primecell";
 			reg = <0x12000 0x1000>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
index f41ebc30666f..ab6866ab6107 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
@@ -107,6 +107,23 @@ uart0: serial@640 {
 			status = "disabled";
 		};
 
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
+
 		hsspi: spi@1000 {
 			#address-cells = <1>;
 			#size-cells = <0>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
index fa2688f41f06..390d5da67a3b 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
@@ -155,5 +155,22 @@ hsspi: spi@1000 {
 			num-cs = <8>;
 			status = "disabled";
 		};
+
+		nand_controller: nand-controller@1800 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "brcm,nand-bcmbca", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+			reg = <0x1800 0x600>, <0x2000 0x10>;
+			reg-names = "nand", "nand-int-base";
+			brcm,nand-use-wp = <0>;
+			status = "disabled";
+
+			nandcs: nand@0 {
+				compatible = "brcm,nandcs";
+				reg = <0>;
+				nand-on-flash-bbt;
+				brcm,nand-ecc-use-strap;
+			};
+		};
 	};
 };
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
index e69cd683211a..4d1ea501e384 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
index db2c82d6dfd8..810b5a23da7b 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
index 25c12bc63545..3aaae5dbb568 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
index faba21f03120..6b167cc2af76 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
index 9808331eede2..d598cd618b57 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
index 1f561c8e13b0..e50ddbf6f58c 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
@@ -32,3 +32,7 @@ &uart0 {
 &hsspi {
 	status = "okay";
 };
+
+&nand_controller {
+	status = "okay";
+};
-- 
2.37.3


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
@ 2023-06-07  8:14   ` Miquel Raynal
  2023-06-07 20:01     ` William Zhang
  2023-06-14 22:46     ` Rob Herring
  2023-06-14 22:43   ` Rob Herring
  1 sibling, 2 replies; 13+ messages in thread
From: Miquel Raynal @ 2023-06-07  8:14 UTC (permalink / raw)
  To: William Zhang
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Richard Weinberger,
	Kamal Dasu, Rob Herring

Hi William,

william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:

> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
> product. The old compatible string is still kept in the driver so old
> dtb can still work.
> 
> Add brcm,nand-use-wp property to have an option for disabling this
> feature on broadband board design that does not use write protection.
> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
> broadband board designs because they do not specify ecc setting in dts
> but rather using the strap setting.
> 
> Remove the requirement of interrupts and interrupt-names properties to
> reflect the driver code.
> 
> This patch also includes a few minor fixes to the BCM63xx compatibles
> and add myself to the list of maintainers.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> ---
> 
>  .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
>  1 file changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> index 1571024aa119..1fe1c166a9db 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
>  maintainers:
>    - Brian Norris <computersforpeace@gmail.com>
>    - Kamal Dasu <kdasu.kdev@gmail.com>
> +  - William Zhang <william.zhang@broadcom.com>
>  
>  description: |
>    The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
> @@ -18,9 +19,10 @@ description: |
>    supports basic PROGRAM and READ functions, among other features.
>  
>    This controller was originally designed for STB SoCs (BCM7xxx) but is now
> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
> -  iProc/Cygnus. Its history includes several similar (but not fully register
> -  compatible) versions.
> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
> +  Its history includes several similar (but not fully register compatible)
> +  versions.
>  
>    -- Additional SoC-specific NAND controller properties --
>  
> @@ -53,9 +55,9 @@ properties:
>                - brcm,brcmnand-v7.2
>                - brcm,brcmnand-v7.3
>            - const: brcm,brcmnand
> -      - description: BCM63138 SoC-specific NAND controller
> +      - description: BCMBCA SoC-specific NAND controller
>          items:
> -          - const: brcm,nand-bcm63138
> +          - const: brcm,nand-bcmbca
>            - enum:
>                - brcm,brcmnand-v7.0
>                - brcm,brcmnand-v7.1
> @@ -65,11 +67,15 @@ properties:
>            - const: brcm,nand-iproc
>            - const: brcm,brcmnand-v6.1
>            - const: brcm,brcmnand
> -      - description: BCM63168 SoC-specific NAND controller
> +      - description: BCM63xx SoC-specific NAND controller
>          items:
> -          - const: brcm,nand-bcm63168
> -          - const: brcm,nand-bcm6368
> -          - const: brcm,brcmnand-v4.0
> +          - enum:
> +              - brcm,nand-bcm63168
> +              - brcm,nand-bcm6368
> +          - enum:
> +              - brcm,brcmnand-v2.1
> +              - brcm,brcmnand-v2.2
> +              - brcm,brcmnand-v4.0
>            - const: brcm,brcmnand
>  
>    reg:
> @@ -111,6 +117,19 @@ properties:
>        earlier versions of this core that include WP
>      type: boolean
>  
> +  brcm,nand-use-wp:
> +    description:
> +      Use this integer to indicate if board design uses
> +      controller's write protection feature and connects its
> +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
> +      use this feature when this property does not exist.
> +      Set to 0 if WP pins are not connected and feature is not
> +      used. Set to 1 if WP pins are connected and feature is used.
> +      Set to 2 if WP pins are connected but disable this feature
> +      through driver that sets controller to output high on NAND_WPb.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 1, 2]

Perhaps strings would be welcome. I'll let binding maintainers say what
they think of it.

> +
>  patternProperties:
>    "^nand@[a-f0-9]$":
>      type: object
> @@ -136,13 +155,23 @@ patternProperties:
>            layout.
>          $ref: /schemas/types.yaml#/definitions/uint32
>  
> +      brcm,nand-ecc-use-strap:
> +        description:
> +          This flag is used by the driver to get the ecc strength and
> +          spare area size from the SoC NAND boot strap setting. This
> +          is commonly used by the BCMBCA SoC board design. If ecc
> +          strength and spare area size are set by nand-ecc-strength
> +          and brcm,nand-oob-sector-size in the dts, these settings
> +          have precedence and override this flag.
> +        $ref: /schemas/types.yaml#/definitions/flag

How in practice do you access the strap value? Don't you need a phandle
over a specific area in the SoC?

> +
>  allOf:
>    - $ref: nand-controller.yaml#
>    - if:
>        properties:
>          compatible:
>            contains:
> -            const: brcm,nand-bcm63138
> +            const: brcm,nand-bcmbca
>      then:
>        properties:
>          reg-names:
> @@ -153,7 +182,9 @@ allOf:
>        properties:
>          compatible:
>            contains:
> -            const: brcm,nand-bcm6368
> +            enum:
> +              - brcm,nand-bcm63168
> +              - brcm,nand-bcm6368
>      then:
>        properties:
>          reg-names:
> @@ -173,20 +204,12 @@ allOf:
>              - const: nand
>              - const: iproc-idm
>              - const: iproc-ext
> -  - if:
> -      properties:
> -        interrupts:
> -          minItems: 2
> -    then:
> -      required:
> -        - interrupt-names

Why do you remove this? Removing "interrupts" from the required
properties is fine, but constraining the interrupts property when it is
relevant is still expected.

>  
>  unevaluatedProperties: false
>  
>  required:
>    - reg
>    - reg-names
> -  - interrupts

This should be done in a separate patch.

>  
>  examples:
>    - |
> @@ -215,8 +238,7 @@ examples:
>      };
>    - |
>      nand-controller@10000200 {
> -        compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
> -                     "brcm,brcmnand-v4.0", "brcm,brcmnand";
> +        compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand";
>          reg = <0x10000200 0x180>,
>                <0x100000b0 0x10>,
>                <0x10000600 0x200>;


Thanks,
Miquèl

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-07  8:14   ` Miquel Raynal
@ 2023-06-07 20:01     ` William Zhang
  2023-06-09  8:58       ` Miquel Raynal
  2023-06-14 22:46     ` Rob Herring
  1 sibling, 1 reply; 13+ messages in thread
From: William Zhang @ 2023-06-07 20:01 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Richard Weinberger,
	Kamal Dasu, Rob Herring

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

Hi Miquel,

On 06/07/2023 01:14 AM, Miquel Raynal wrote:
> Hi William,
> 
> william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:
> 
>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
>> product. The old compatible string is still kept in the driver so old
>> dtb can still work.
>>
>> Add brcm,nand-use-wp property to have an option for disabling this
>> feature on broadband board design that does not use write protection.
>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
>> broadband board designs because they do not specify ecc setting in dts
>> but rather using the strap setting.
>>
>> Remove the requirement of interrupts and interrupt-names properties to
>> reflect the driver code.
>>
>> This patch also includes a few minor fixes to the BCM63xx compatibles
>> and add myself to the list of maintainers.
>>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> ---
>>
>>   .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
>>   1 file changed, 43 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> index 1571024aa119..1fe1c166a9db 100644
>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
>>   maintainers:
>>     - Brian Norris <computersforpeace@gmail.com>
>>     - Kamal Dasu <kdasu.kdev@gmail.com>
>> +  - William Zhang <william.zhang@broadcom.com>
>>   
>>   description: |
>>     The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
>> @@ -18,9 +19,10 @@ description: |
>>     supports basic PROGRAM and READ functions, among other features.
>>   
>>     This controller was originally designed for STB SoCs (BCM7xxx) but is now
>> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
>> -  iProc/Cygnus. Its history includes several similar (but not fully register
>> -  compatible) versions.
>> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
>> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
>> +  Its history includes several similar (but not fully register compatible)
>> +  versions.
>>   
>>     -- Additional SoC-specific NAND controller properties --
>>   
>> @@ -53,9 +55,9 @@ properties:
>>                 - brcm,brcmnand-v7.2
>>                 - brcm,brcmnand-v7.3
>>             - const: brcm,brcmnand
>> -      - description: BCM63138 SoC-specific NAND controller
>> +      - description: BCMBCA SoC-specific NAND controller
>>           items:
>> -          - const: brcm,nand-bcm63138
>> +          - const: brcm,nand-bcmbca
>>             - enum:
>>                 - brcm,brcmnand-v7.0
>>                 - brcm,brcmnand-v7.1
>> @@ -65,11 +67,15 @@ properties:
>>             - const: brcm,nand-iproc
>>             - const: brcm,brcmnand-v6.1
>>             - const: brcm,brcmnand
>> -      - description: BCM63168 SoC-specific NAND controller
>> +      - description: BCM63xx SoC-specific NAND controller
>>           items:
>> -          - const: brcm,nand-bcm63168
>> -          - const: brcm,nand-bcm6368
>> -          - const: brcm,brcmnand-v4.0
>> +          - enum:
>> +              - brcm,nand-bcm63168
>> +              - brcm,nand-bcm6368
>> +          - enum:
>> +              - brcm,brcmnand-v2.1
>> +              - brcm,brcmnand-v2.2
>> +              - brcm,brcmnand-v4.0
>>             - const: brcm,brcmnand
>>   
>>     reg:
>> @@ -111,6 +117,19 @@ properties:
>>         earlier versions of this core that include WP
>>       type: boolean
>>   
>> +  brcm,nand-use-wp:
>> +    description:
>> +      Use this integer to indicate if board design uses
>> +      controller's write protection feature and connects its
>> +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
>> +      use this feature when this property does not exist.
>> +      Set to 0 if WP pins are not connected and feature is not
>> +      used. Set to 1 if WP pins are connected and feature is used.
>> +      Set to 2 if WP pins are connected but disable this feature
>> +      through driver that sets controller to output high on NAND_WPb.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 1, 2]
> 
> Perhaps strings would be welcome. I'll let binding maintainers say what
> they think of it.
> 
Practically there is really just use cases of 0 and 1. I could use a 
bool flag but to keep consistent with the driver code and in case there 
is any existing usage of 2.

>> +
>>   patternProperties:
>>     "^nand@[a-f0-9]$":
>>       type: object
>> @@ -136,13 +155,23 @@ patternProperties:
>>             layout.
>>           $ref: /schemas/types.yaml#/definitions/uint32
>>   
>> +      brcm,nand-ecc-use-strap:
>> +        description:
>> +          This flag is used by the driver to get the ecc strength and
>> +          spare area size from the SoC NAND boot strap setting. This
>> +          is commonly used by the BCMBCA SoC board design. If ecc
>> +          strength and spare area size are set by nand-ecc-strength
>> +          and brcm,nand-oob-sector-size in the dts, these settings
>> +          have precedence and override this flag.
>> +        $ref: /schemas/types.yaml#/definitions/flag
> 
> How in practice do you access the strap value? Don't you need a phandle
> over a specific area in the SoC?
> 
The strap value is latched and stored in the NAND controller register so 
there is no extra phandle needed.

>> +
>>   allOf:
>>     - $ref: nand-controller.yaml#
>>     - if:
>>         properties:
>>           compatible:
>>             contains:
>> -            const: brcm,nand-bcm63138
>> +            const: brcm,nand-bcmbca
>>       then:
>>         properties:
>>           reg-names:
>> @@ -153,7 +182,9 @@ allOf:
>>         properties:
>>           compatible:
>>             contains:
>> -            const: brcm,nand-bcm6368
>> +            enum:
>> +              - brcm,nand-bcm63168
>> +              - brcm,nand-bcm6368
>>       then:
>>         properties:
>>           reg-names:
>> @@ -173,20 +204,12 @@ allOf:
>>               - const: nand
>>               - const: iproc-idm
>>               - const: iproc-ext
>> -  - if:
>> -      properties:
>> -        interrupts:
>> -          minItems: 2
>> -    then:
>> -      required:
>> -        - interrupt-names
> 
> Why do you remove this? Removing "interrupts" from the required
> properties is fine, but constraining the interrupts property when it is
> relevant is still expected.
> 
There is no requirement for interrupt name even if it have two 
interrupts. Driver code does not use interrupt name but the interrupt 
index instead.

>>   
>>   unevaluatedProperties: false
>>   
>>   required:
>>     - reg
>>     - reg-names
>> -  - interrupts
> 
> This should be done in a separate patch.
> 
I thought this is also related to my update for bcmbca chips because 
they don't need to interrupt and interrupt name.

>>   
>>   examples:
>>     - |
>> @@ -215,8 +238,7 @@ examples:
>>       };
>>     - |
>>       nand-controller@10000200 {
>> -        compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
>> -                     "brcm,brcmnand-v4.0", "brcm,brcmnand";
>> +        compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand";
>>           reg = <0x10000200 0x180>,
>>                 <0x100000b0 0x10>,
>>                 <0x10000600 0x200>;
> 
> 
> Thanks,
> Miquèl
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-07 20:01     ` William Zhang
@ 2023-06-09  8:58       ` Miquel Raynal
  2023-06-09 19:05         ` William Zhang
  0 siblings, 1 reply; 13+ messages in thread
From: Miquel Raynal @ 2023-06-09  8:58 UTC (permalink / raw)
  To: William Zhang
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Richard Weinberger,
	Kamal Dasu, Rob Herring

Hi William,

william.zhang@broadcom.com wrote on Wed, 7 Jun 2023 13:01:56 -0700:

> Hi Miquel,
> 
> On 06/07/2023 01:14 AM, Miquel Raynal wrote:
> > Hi William,
> > 
> > william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:
> >   
> >> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
> >> product. The old compatible string is still kept in the driver so old
> >> dtb can still work.
> >>
> >> Add brcm,nand-use-wp property to have an option for disabling this
> >> feature on broadband board design that does not use write protection.
> >> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
> >> broadband board designs because they do not specify ecc setting in dts
> >> but rather using the strap setting.
> >>
> >> Remove the requirement of interrupts and interrupt-names properties to
> >> reflect the driver code.
> >>
> >> This patch also includes a few minor fixes to the BCM63xx compatibles
> >> and add myself to the list of maintainers.
> >>
> >> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> >> ---
> >>
> >>   .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
> >>   1 file changed, 43 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >> index 1571024aa119..1fe1c166a9db 100644
> >> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
> >>   maintainers:
> >>     - Brian Norris <computersforpeace@gmail.com>
> >>     - Kamal Dasu <kdasu.kdev@gmail.com>
> >> +  - William Zhang <william.zhang@broadcom.com>  
> >>   >>   description: |  
> >>     The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
> >> @@ -18,9 +19,10 @@ description: |
> >>     supports basic PROGRAM and READ functions, among other features.  
> >>   >>     This controller was originally designed for STB SoCs (BCM7xxx) but is now  
> >> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
> >> -  iProc/Cygnus. Its history includes several similar (but not fully register
> >> -  compatible) versions.
> >> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
> >> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
> >> +  Its history includes several similar (but not fully register compatible)
> >> +  versions.  
> >>   >>     -- Additional SoC-specific NAND controller properties --
> >>   >> @@ -53,9 +55,9 @@ properties:  
> >>                 - brcm,brcmnand-v7.2
> >>                 - brcm,brcmnand-v7.3
> >>             - const: brcm,brcmnand
> >> -      - description: BCM63138 SoC-specific NAND controller
> >> +      - description: BCMBCA SoC-specific NAND controller
> >>           items:
> >> -          - const: brcm,nand-bcm63138
> >> +          - const: brcm,nand-bcmbca
> >>             - enum:
> >>                 - brcm,brcmnand-v7.0
> >>                 - brcm,brcmnand-v7.1
> >> @@ -65,11 +67,15 @@ properties:
> >>             - const: brcm,nand-iproc
> >>             - const: brcm,brcmnand-v6.1
> >>             - const: brcm,brcmnand
> >> -      - description: BCM63168 SoC-specific NAND controller
> >> +      - description: BCM63xx SoC-specific NAND controller
> >>           items:
> >> -          - const: brcm,nand-bcm63168
> >> -          - const: brcm,nand-bcm6368
> >> -          - const: brcm,brcmnand-v4.0
> >> +          - enum:
> >> +              - brcm,nand-bcm63168
> >> +              - brcm,nand-bcm6368
> >> +          - enum:
> >> +              - brcm,brcmnand-v2.1
> >> +              - brcm,brcmnand-v2.2
> >> +              - brcm,brcmnand-v4.0
> >>             - const: brcm,brcmnand  
> >>   >>     reg:  
> >> @@ -111,6 +117,19 @@ properties:
> >>         earlier versions of this core that include WP
> >>       type: boolean  
> >>   >> +  brcm,nand-use-wp:  
> >> +    description:
> >> +      Use this integer to indicate if board design uses
> >> +      controller's write protection feature and connects its
> >> +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
> >> +      use this feature when this property does not exist.
> >> +      Set to 0 if WP pins are not connected and feature is not
> >> +      used. Set to 1 if WP pins are connected and feature is used.
> >> +      Set to 2 if WP pins are connected but disable this feature
> >> +      through driver that sets controller to output high on NAND_WPb.
> >> +    $ref: /schemas/types.yaml#/definitions/uint32
> >> +    enum: [0, 1, 2]  
> > 
> > Perhaps strings would be welcome. I'll let binding maintainers say what
> > they think of it.
> >   
> Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2.
> 
> >> +
> >>   patternProperties:
> >>     "^nand@[a-f0-9]$":
> >>       type: object
> >> @@ -136,13 +155,23 @@ patternProperties:
> >>             layout.
> >>           $ref: /schemas/types.yaml#/definitions/uint32  
> >>   >> +      brcm,nand-ecc-use-strap:  
> >> +        description:
> >> +          This flag is used by the driver to get the ecc strength and
> >> +          spare area size from the SoC NAND boot strap setting. This
> >> +          is commonly used by the BCMBCA SoC board design. If ecc
> >> +          strength and spare area size are set by nand-ecc-strength
> >> +          and brcm,nand-oob-sector-size in the dts, these settings
> >> +          have precedence and override this flag.
> >> +        $ref: /schemas/types.yaml#/definitions/flag  
> > 
> > How in practice do you access the strap value? Don't you need a phandle
> > over a specific area in the SoC?
> >   
> The strap value is latched and stored in the NAND controller register so there is no extra phandle needed.

Ok.

> 
> >> +
> >>   allOf:
> >>     - $ref: nand-controller.yaml#
> >>     - if:
> >>         properties:
> >>           compatible:
> >>             contains:
> >> -            const: brcm,nand-bcm63138
> >> +            const: brcm,nand-bcmbca
> >>       then:
> >>         properties:
> >>           reg-names:
> >> @@ -153,7 +182,9 @@ allOf:
> >>         properties:
> >>           compatible:
> >>             contains:
> >> -            const: brcm,nand-bcm6368
> >> +            enum:
> >> +              - brcm,nand-bcm63168
> >> +              - brcm,nand-bcm6368
> >>       then:
> >>         properties:
> >>           reg-names:
> >> @@ -173,20 +204,12 @@ allOf:
> >>               - const: nand
> >>               - const: iproc-idm
> >>               - const: iproc-ext
> >> -  - if:
> >> -      properties:
> >> -        interrupts:
> >> -          minItems: 2
> >> -    then:
> >> -      required:
> >> -        - interrupt-names  
> > 
> > Why do you remove this? Removing "interrupts" from the required
> > properties is fine, but constraining the interrupts property when it is
> > relevant is still expected.
> >   
> There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead.

It does not matter in this case how the driver uses the interrupts. If
names have been provided once in the bindings, you could expect another
project using the same bindings to use the interrupt names instead of
the order. So you must keep the names. Just don't mark them required it
they are.

> 
> >>   >>   unevaluatedProperties: false
> >>   >>   required:  
> >>     - reg
> >>     - reg-names
> >> -  - interrupts  
> > 
> > This should be done in a separate patch.
> >   
> I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name.

The fact that the driver does not use the interrupts does not mean they
should not be described. 

> 
> >>   >>   examples:  
> >>     - |
> >> @@ -215,8 +238,7 @@ examples:
> >>       };
> >>     - |
> >>       nand-controller@10000200 {
> >> -        compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
> >> -                     "brcm,brcmnand-v4.0", "brcm,brcmnand";
> >> +        compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand";
> >>           reg = <0x10000200 0x180>,
> >>                 <0x100000b0 0x10>,
> >>                 <0x10000600 0x200>;  
> > 
> > 
> > Thanks,
> > Miquèl
> >   


Thanks,
Miquèl

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-09  8:58       ` Miquel Raynal
@ 2023-06-09 19:05         ` William Zhang
  2023-06-12 17:43           ` Miquel Raynal
  0 siblings, 1 reply; 13+ messages in thread
From: William Zhang @ 2023-06-09 19:05 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Richard Weinberger,
	Kamal Dasu, Rob Herring

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

Hi Miquel,

On 06/09/2023 01:58 AM, Miquel Raynal wrote:
> Hi William,
> 
> william.zhang@broadcom.com wrote on Wed, 7 Jun 2023 13:01:56 -0700:
> 
>> Hi Miquel,
>>
>> On 06/07/2023 01:14 AM, Miquel Raynal wrote:
>>> Hi William,
>>>
>>> william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:
>>>    
>>>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
>>>> product. The old compatible string is still kept in the driver so old
>>>> dtb can still work.
>>>>
>>>> Add brcm,nand-use-wp property to have an option for disabling this
>>>> feature on broadband board design that does not use write protection.
>>>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
>>>> broadband board designs because they do not specify ecc setting in dts
>>>> but rather using the strap setting.
>>>>
>>>> Remove the requirement of interrupts and interrupt-names properties to
>>>> reflect the driver code.
>>>>
>>>> This patch also includes a few minor fixes to the BCM63xx compatibles
>>>> and add myself to the list of maintainers.
>>>>
>>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>>> ---
>>>>
>>>>    .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
>>>>    1 file changed, 43 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>>> index 1571024aa119..1fe1c166a9db 100644
>>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
>>>>    maintainers:
>>>>      - Brian Norris <computersforpeace@gmail.com>
>>>>      - Kamal Dasu <kdasu.kdev@gmail.com>
>>>> +  - William Zhang <william.zhang@broadcom.com>
>>>>    >>   description: |
>>>>      The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
>>>> @@ -18,9 +19,10 @@ description: |
>>>>      supports basic PROGRAM and READ functions, among other features.
>>>>    >>     This controller was originally designed for STB SoCs (BCM7xxx) but is now
>>>> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
>>>> -  iProc/Cygnus. Its history includes several similar (but not fully register
>>>> -  compatible) versions.
>>>> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
>>>> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
>>>> +  Its history includes several similar (but not fully register compatible)
>>>> +  versions.
>>>>    >>     -- Additional SoC-specific NAND controller properties --
>>>>    >> @@ -53,9 +55,9 @@ properties:
>>>>                  - brcm,brcmnand-v7.2
>>>>                  - brcm,brcmnand-v7.3
>>>>              - const: brcm,brcmnand
>>>> -      - description: BCM63138 SoC-specific NAND controller
>>>> +      - description: BCMBCA SoC-specific NAND controller
>>>>            items:
>>>> -          - const: brcm,nand-bcm63138
>>>> +          - const: brcm,nand-bcmbca
>>>>              - enum:
>>>>                  - brcm,brcmnand-v7.0
>>>>                  - brcm,brcmnand-v7.1
>>>> @@ -65,11 +67,15 @@ properties:
>>>>              - const: brcm,nand-iproc
>>>>              - const: brcm,brcmnand-v6.1
>>>>              - const: brcm,brcmnand
>>>> -      - description: BCM63168 SoC-specific NAND controller
>>>> +      - description: BCM63xx SoC-specific NAND controller
>>>>            items:
>>>> -          - const: brcm,nand-bcm63168
>>>> -          - const: brcm,nand-bcm6368
>>>> -          - const: brcm,brcmnand-v4.0
>>>> +          - enum:
>>>> +              - brcm,nand-bcm63168
>>>> +              - brcm,nand-bcm6368
>>>> +          - enum:
>>>> +              - brcm,brcmnand-v2.1
>>>> +              - brcm,brcmnand-v2.2
>>>> +              - brcm,brcmnand-v4.0
>>>>              - const: brcm,brcmnand
>>>>    >>     reg:
>>>> @@ -111,6 +117,19 @@ properties:
>>>>          earlier versions of this core that include WP
>>>>        type: boolean
>>>>    >> +  brcm,nand-use-wp:
>>>> +    description:
>>>> +      Use this integer to indicate if board design uses
>>>> +      controller's write protection feature and connects its
>>>> +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
>>>> +      use this feature when this property does not exist.
>>>> +      Set to 0 if WP pins are not connected and feature is not
>>>> +      used. Set to 1 if WP pins are connected and feature is used.
>>>> +      Set to 2 if WP pins are connected but disable this feature
>>>> +      through driver that sets controller to output high on NAND_WPb.
>>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>>> +    enum: [0, 1, 2]
>>>
>>> Perhaps strings would be welcome. I'll let binding maintainers say what
>>> they think of it.
>>>    
>> Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2.
>>
>>>> +
>>>>    patternProperties:
>>>>      "^nand@[a-f0-9]$":
>>>>        type: object
>>>> @@ -136,13 +155,23 @@ patternProperties:
>>>>              layout.
>>>>            $ref: /schemas/types.yaml#/definitions/uint32
>>>>    >> +      brcm,nand-ecc-use-strap:
>>>> +        description:
>>>> +          This flag is used by the driver to get the ecc strength and
>>>> +          spare area size from the SoC NAND boot strap setting. This
>>>> +          is commonly used by the BCMBCA SoC board design. If ecc
>>>> +          strength and spare area size are set by nand-ecc-strength
>>>> +          and brcm,nand-oob-sector-size in the dts, these settings
>>>> +          have precedence and override this flag.
>>>> +        $ref: /schemas/types.yaml#/definitions/flag
>>>
>>> How in practice do you access the strap value? Don't you need a phandle
>>> over a specific area in the SoC?
>>>    
>> The strap value is latched and stored in the NAND controller register so there is no extra phandle needed.
> 
> Ok.
> 
>>
>>>> +
>>>>    allOf:
>>>>      - $ref: nand-controller.yaml#
>>>>      - if:
>>>>          properties:
>>>>            compatible:
>>>>              contains:
>>>> -            const: brcm,nand-bcm63138
>>>> +            const: brcm,nand-bcmbca
>>>>        then:
>>>>          properties:
>>>>            reg-names:
>>>> @@ -153,7 +182,9 @@ allOf:
>>>>          properties:
>>>>            compatible:
>>>>              contains:
>>>> -            const: brcm,nand-bcm6368
>>>> +            enum:
>>>> +              - brcm,nand-bcm63168
>>>> +              - brcm,nand-bcm6368
>>>>        then:
>>>>          properties:
>>>>            reg-names:
>>>> @@ -173,20 +204,12 @@ allOf:
>>>>                - const: nand
>>>>                - const: iproc-idm
>>>>                - const: iproc-ext
>>>> -  - if:
>>>> -      properties:
>>>> -        interrupts:
>>>> -          minItems: 2
>>>> -    then:
>>>> -      required:
>>>> -        - interrupt-names
>>>
>>> Why do you remove this? Removing "interrupts" from the required
>>> properties is fine, but constraining the interrupts property when it is
>>> relevant is still expected.
>>>    
>> There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead.
> 
> It does not matter in this case how the driver uses the interrupts. If
> names have been provided once in the bindings, you could expect another
> project using the same bindings to use the interrupt names instead of
> the order. So you must keep the names. Just don't mark them required it
> they are.
> 
Certainly any dts file can still have interrupts names with one or two 
interrupts but to my understand it is not required to have the name when 
there are two interrupts. Anyway I will add this back so this constrain 
remains.

>>
>>>>    >>   unevaluatedProperties: false
>>>>    >>   required:
>>>>      - reg
>>>>      - reg-names
>>>> -  - interrupts
>>>
>>> This should be done in a separate patch.
>>>    
>> I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name.
> 
> The fact that the driver does not use the interrupts does not mean they
> should not be described.
> 
>>
>>>>    >>   examples:
>>>>      - |
>>>> @@ -215,8 +238,7 @@ examples:
>>>>        };
>>>>      - |
>>>>        nand-controller@10000200 {
>>>> -        compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
>>>> -                     "brcm,brcmnand-v4.0", "brcm,brcmnand";
>>>> +        compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand";
>>>>            reg = <0x10000200 0x180>,
>>>>                  <0x100000b0 0x10>,
>>>>                  <0x10000600 0x200>;
>>>
>>>
>>> Thanks,
>>> Miquèl
>>>    
> 
> 
> Thanks,
> Miquèl
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-09 19:05         ` William Zhang
@ 2023-06-12 17:43           ` Miquel Raynal
  0 siblings, 0 replies; 13+ messages in thread
From: Miquel Raynal @ 2023-06-12 17:43 UTC (permalink / raw)
  To: William Zhang
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Richard Weinberger,
	Kamal Dasu, Rob Herring

Hi William,

william.zhang@broadcom.com wrote on Fri, 9 Jun 2023 12:05:19 -0700:

> Hi Miquel,
> 
> On 06/09/2023 01:58 AM, Miquel Raynal wrote:
> > Hi William,
> > 
> > william.zhang@broadcom.com wrote on Wed, 7 Jun 2023 13:01:56 -0700:
> >   
> >> Hi Miquel,
> >>
> >> On 06/07/2023 01:14 AM, Miquel Raynal wrote:  
> >>> Hi William,
> >>>
> >>> william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:  
> >>>    >>>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband  
> >>>> product. The old compatible string is still kept in the driver so old
> >>>> dtb can still work.
> >>>>
> >>>> Add brcm,nand-use-wp property to have an option for disabling this
> >>>> feature on broadband board design that does not use write protection.
> >>>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
> >>>> broadband board designs because they do not specify ecc setting in dts
> >>>> but rather using the strap setting.
> >>>>
> >>>> Remove the requirement of interrupts and interrupt-names properties to
> >>>> reflect the driver code.
> >>>>
> >>>> This patch also includes a few minor fixes to the BCM63xx compatibles
> >>>> and add myself to the list of maintainers.
> >>>>
> >>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> >>>> ---
> >>>>
> >>>>    .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
> >>>>    1 file changed, 43 insertions(+), 21 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >>>> index 1571024aa119..1fe1c166a9db 100644
> >>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >>>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
> >>>>    maintainers:
> >>>>      - Brian Norris <computersforpeace@gmail.com>
> >>>>      - Kamal Dasu <kdasu.kdev@gmail.com>
> >>>> +  - William Zhang <william.zhang@broadcom.com>  
> >>>>    >>   description: |  
> >>>>      The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
> >>>> @@ -18,9 +19,10 @@ description: |
> >>>>      supports basic PROGRAM and READ functions, among other features.  
> >>>>    >>     This controller was originally designed for STB SoCs (BCM7xxx) but is now  
> >>>> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
> >>>> -  iProc/Cygnus. Its history includes several similar (but not fully register
> >>>> -  compatible) versions.
> >>>> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
> >>>> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
> >>>> +  Its history includes several similar (but not fully register compatible)
> >>>> +  versions.  
> >>>>    >>     -- Additional SoC-specific NAND controller properties --
> >>>>    >> @@ -53,9 +55,9 @@ properties:  
> >>>>                  - brcm,brcmnand-v7.2
> >>>>                  - brcm,brcmnand-v7.3
> >>>>              - const: brcm,brcmnand
> >>>> -      - description: BCM63138 SoC-specific NAND controller
> >>>> +      - description: BCMBCA SoC-specific NAND controller
> >>>>            items:
> >>>> -          - const: brcm,nand-bcm63138
> >>>> +          - const: brcm,nand-bcmbca
> >>>>              - enum:
> >>>>                  - brcm,brcmnand-v7.0
> >>>>                  - brcm,brcmnand-v7.1
> >>>> @@ -65,11 +67,15 @@ properties:
> >>>>              - const: brcm,nand-iproc
> >>>>              - const: brcm,brcmnand-v6.1
> >>>>              - const: brcm,brcmnand
> >>>> -      - description: BCM63168 SoC-specific NAND controller
> >>>> +      - description: BCM63xx SoC-specific NAND controller
> >>>>            items:
> >>>> -          - const: brcm,nand-bcm63168
> >>>> -          - const: brcm,nand-bcm6368
> >>>> -          - const: brcm,brcmnand-v4.0
> >>>> +          - enum:
> >>>> +              - brcm,nand-bcm63168
> >>>> +              - brcm,nand-bcm6368
> >>>> +          - enum:
> >>>> +              - brcm,brcmnand-v2.1
> >>>> +              - brcm,brcmnand-v2.2
> >>>> +              - brcm,brcmnand-v4.0
> >>>>              - const: brcm,brcmnand  
> >>>>    >>     reg:  
> >>>> @@ -111,6 +117,19 @@ properties:
> >>>>          earlier versions of this core that include WP
> >>>>        type: boolean  
> >>>>    >> +  brcm,nand-use-wp:  
> >>>> +    description:
> >>>> +      Use this integer to indicate if board design uses
> >>>> +      controller's write protection feature and connects its
> >>>> +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
> >>>> +      use this feature when this property does not exist.
> >>>> +      Set to 0 if WP pins are not connected and feature is not
> >>>> +      used. Set to 1 if WP pins are connected and feature is used.
> >>>> +      Set to 2 if WP pins are connected but disable this feature
> >>>> +      through driver that sets controller to output high on NAND_WPb.
> >>>> +    $ref: /schemas/types.yaml#/definitions/uint32
> >>>> +    enum: [0, 1, 2]  
> >>>
> >>> Perhaps strings would be welcome. I'll let binding maintainers say what
> >>> they think of it.  
> >>>    >> Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2.  
> >>  
> >>>> +
> >>>>    patternProperties:
> >>>>      "^nand@[a-f0-9]$":
> >>>>        type: object
> >>>> @@ -136,13 +155,23 @@ patternProperties:
> >>>>              layout.
> >>>>            $ref: /schemas/types.yaml#/definitions/uint32  
> >>>>    >> +      brcm,nand-ecc-use-strap:  
> >>>> +        description:
> >>>> +          This flag is used by the driver to get the ecc strength and
> >>>> +          spare area size from the SoC NAND boot strap setting. This
> >>>> +          is commonly used by the BCMBCA SoC board design. If ecc
> >>>> +          strength and spare area size are set by nand-ecc-strength
> >>>> +          and brcm,nand-oob-sector-size in the dts, these settings
> >>>> +          have precedence and override this flag.
> >>>> +        $ref: /schemas/types.yaml#/definitions/flag  
> >>>
> >>> How in practice do you access the strap value? Don't you need a phandle
> >>> over a specific area in the SoC?  
> >>>    >> The strap value is latched and stored in the NAND controller register so there is no extra phandle needed.  
> > 
> > Ok.
> >   
> >>  
> >>>> +
> >>>>    allOf:
> >>>>      - $ref: nand-controller.yaml#
> >>>>      - if:
> >>>>          properties:
> >>>>            compatible:
> >>>>              contains:
> >>>> -            const: brcm,nand-bcm63138
> >>>> +            const: brcm,nand-bcmbca
> >>>>        then:
> >>>>          properties:
> >>>>            reg-names:
> >>>> @@ -153,7 +182,9 @@ allOf:
> >>>>          properties:
> >>>>            compatible:
> >>>>              contains:
> >>>> -            const: brcm,nand-bcm6368
> >>>> +            enum:
> >>>> +              - brcm,nand-bcm63168
> >>>> +              - brcm,nand-bcm6368
> >>>>        then:
> >>>>          properties:
> >>>>            reg-names:
> >>>> @@ -173,20 +204,12 @@ allOf:
> >>>>                - const: nand
> >>>>                - const: iproc-idm
> >>>>                - const: iproc-ext
> >>>> -  - if:
> >>>> -      properties:
> >>>> -        interrupts:
> >>>> -          minItems: 2
> >>>> -    then:
> >>>> -      required:
> >>>> -        - interrupt-names  
> >>>
> >>> Why do you remove this? Removing "interrupts" from the required
> >>> properties is fine, but constraining the interrupts property when it is
> >>> relevant is still expected.  
> >>>    >> There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead.  
> > 
> > It does not matter in this case how the driver uses the interrupts. If
> > names have been provided once in the bindings, you could expect another
> > project using the same bindings to use the interrupt names instead of
> > the order. So you must keep the names. Just don't mark them required it
> > they are.
> >   
> Certainly any dts file can still have interrupts names with one or two interrupts but to my understand it is not required to have the name when there are two interrupts. Anyway I will add this back so this constrain remains.

That is true, until you enforce specific names. Then you are tied to
these names, otherwise you may break the bindings. It it is not
_really_ useful, then don't do it.

> 
> >>  
> >>>>    >>   unevaluatedProperties: false
> >>>>    >>   required:  
> >>>>      - reg
> >>>>      - reg-names
> >>>> -  - interrupts  
> >>>
> >>> This should be done in a separate patch.  
> >>>    >> I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name.  
> > 
> > The fact that the driver does not use the interrupts does not mean they
> > should not be described.
> >   
> >>  
> >>>>    >>   examples:  
> >>>>      - |
> >>>> @@ -215,8 +238,7 @@ examples:
> >>>>        };
> >>>>      - |
> >>>>        nand-controller@10000200 {
> >>>> -        compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368",
> >>>> -                     "brcm,brcmnand-v4.0", "brcm,brcmnand";
> >>>> +        compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand";
> >>>>            reg = <0x10000200 0x180>,
> >>>>                  <0x100000b0 0x10>,
> >>>>                  <0x10000600 0x200>;  
> >>>
> >>>
> >>> Thanks,
> >>> Miquèl  
> >>>    > >   
> > Thanks,
> > Miquèl
> >   


Thanks,
Miquèl

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
  2023-06-07  8:14   ` Miquel Raynal
@ 2023-06-14 22:43   ` Rob Herring
  2023-06-15  0:28     ` William Zhang
  1 sibling, 1 reply; 13+ messages in thread
From: Rob Herring @ 2023-06-14 22:43 UTC (permalink / raw)
  To: William Zhang
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Miquel Raynal,
	Richard Weinberger, Kamal Dasu

On Tue, Jun 06, 2023 at 04:12:45PM -0700, William Zhang wrote:
> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
> product. The old compatible string is still kept in the driver so old
> dtb can still work.

I don't understand why you need to change this. 


> Add brcm,nand-use-wp property to have an option for disabling this
> feature on broadband board design that does not use write protection.
> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
> broadband board designs because they do not specify ecc setting in dts
> but rather using the strap setting.
> 
> Remove the requirement of interrupts and interrupt-names properties to
> reflect the driver code.
> 
> This patch also includes a few minor fixes to the BCM63xx compatibles
> and add myself to the list of maintainers.
> 
> Signed-off-by: William Zhang <william.zhang@broadcom.com>
> ---
> 
>  .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
>  1 file changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> index 1571024aa119..1fe1c166a9db 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
>  maintainers:
>    - Brian Norris <computersforpeace@gmail.com>
>    - Kamal Dasu <kdasu.kdev@gmail.com>
> +  - William Zhang <william.zhang@broadcom.com>
>  
>  description: |
>    The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
> @@ -18,9 +19,10 @@ description: |
>    supports basic PROGRAM and READ functions, among other features.
>  
>    This controller was originally designed for STB SoCs (BCM7xxx) but is now
> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
> -  iProc/Cygnus. Its history includes several similar (but not fully register
> -  compatible) versions.
> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
> +  Its history includes several similar (but not fully register compatible)
> +  versions.
>  
>    -- Additional SoC-specific NAND controller properties --
>  
> @@ -53,9 +55,9 @@ properties:
>                - brcm,brcmnand-v7.2
>                - brcm,brcmnand-v7.3
>            - const: brcm,brcmnand
> -      - description: BCM63138 SoC-specific NAND controller
> +      - description: BCMBCA SoC-specific NAND controller
>          items:
> -          - const: brcm,nand-bcm63138
> +          - const: brcm,nand-bcmbca
>            - enum:
>                - brcm,brcmnand-v7.0
>                - brcm,brcmnand-v7.1
> @@ -65,11 +67,15 @@ properties:
>            - const: brcm,nand-iproc
>            - const: brcm,brcmnand-v6.1
>            - const: brcm,brcmnand
> -      - description: BCM63168 SoC-specific NAND controller
> +      - description: BCM63xx SoC-specific NAND controller
>          items:
> -          - const: brcm,nand-bcm63168
> -          - const: brcm,nand-bcm6368
> -          - const: brcm,brcmnand-v4.0
> +          - enum:
> +              - brcm,nand-bcm63168
> +              - brcm,nand-bcm6368
> +          - enum:
> +              - brcm,brcmnand-v2.1
> +              - brcm,brcmnand-v2.2
> +              - brcm,brcmnand-v4.0
>            - const: brcm,brcmnand

Completely changing what's valid here too. You're breaking the ABI.

Rob

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-07  8:14   ` Miquel Raynal
  2023-06-07 20:01     ` William Zhang
@ 2023-06-14 22:46     ` Rob Herring
  2023-06-15  0:40       ` William Zhang
  1 sibling, 1 reply; 13+ messages in thread
From: Rob Herring @ 2023-06-14 22:46 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: William Zhang, Broadcom Kernel List, Linux MTD List, f.fainelli,
	rafal, kursad.oney, joel.peshkin, computersforpeace, anand.gore,
	dregan, kamal.dasu, tomer.yacoby, dan.beygelman, devicetree,
	linux-kernel, Krzysztof Kozlowski, Vignesh Raghavendra,
	Richard Weinberger, Kamal Dasu

On Wed, Jun 07, 2023 at 10:14:52AM +0200, Miquel Raynal wrote:
> Hi William,
> 
> william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:
> 
> > Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
> > product. The old compatible string is still kept in the driver so old
> > dtb can still work.
> > 
> > Add brcm,nand-use-wp property to have an option for disabling this
> > feature on broadband board design that does not use write protection.
> > Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
> > broadband board designs because they do not specify ecc setting in dts
> > but rather using the strap setting.
> > 
> > Remove the requirement of interrupts and interrupt-names properties to
> > reflect the driver code.
> > 
> > This patch also includes a few minor fixes to the BCM63xx compatibles
> > and add myself to the list of maintainers.
> > 
> > Signed-off-by: William Zhang <william.zhang@broadcom.com>
> > ---
> > 
> >  .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
> >  1 file changed, 43 insertions(+), 21 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > index 1571024aa119..1fe1c166a9db 100644
> > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
> >  maintainers:
> >    - Brian Norris <computersforpeace@gmail.com>
> >    - Kamal Dasu <kdasu.kdev@gmail.com>
> > +  - William Zhang <william.zhang@broadcom.com>
> >  
> >  description: |
> >    The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
> > @@ -18,9 +19,10 @@ description: |
> >    supports basic PROGRAM and READ functions, among other features.
> >  
> >    This controller was originally designed for STB SoCs (BCM7xxx) but is now
> > -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
> > -  iProc/Cygnus. Its history includes several similar (but not fully register
> > -  compatible) versions.
> > +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
> > +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
> > +  Its history includes several similar (but not fully register compatible)
> > +  versions.
> >  
> >    -- Additional SoC-specific NAND controller properties --
> >  
> > @@ -53,9 +55,9 @@ properties:
> >                - brcm,brcmnand-v7.2
> >                - brcm,brcmnand-v7.3
> >            - const: brcm,brcmnand
> > -      - description: BCM63138 SoC-specific NAND controller
> > +      - description: BCMBCA SoC-specific NAND controller
> >          items:
> > -          - const: brcm,nand-bcm63138
> > +          - const: brcm,nand-bcmbca
> >            - enum:
> >                - brcm,brcmnand-v7.0
> >                - brcm,brcmnand-v7.1
> > @@ -65,11 +67,15 @@ properties:
> >            - const: brcm,nand-iproc
> >            - const: brcm,brcmnand-v6.1
> >            - const: brcm,brcmnand
> > -      - description: BCM63168 SoC-specific NAND controller
> > +      - description: BCM63xx SoC-specific NAND controller
> >          items:
> > -          - const: brcm,nand-bcm63168
> > -          - const: brcm,nand-bcm6368
> > -          - const: brcm,brcmnand-v4.0
> > +          - enum:
> > +              - brcm,nand-bcm63168
> > +              - brcm,nand-bcm6368
> > +          - enum:
> > +              - brcm,brcmnand-v2.1
> > +              - brcm,brcmnand-v2.2
> > +              - brcm,brcmnand-v4.0
> >            - const: brcm,brcmnand
> >  
> >    reg:
> > @@ -111,6 +117,19 @@ properties:
> >        earlier versions of this core that include WP
> >      type: boolean
> >  
> > +  brcm,nand-use-wp:
> > +    description:
> > +      Use this integer to indicate if board design uses
> > +      controller's write protection feature and connects its
> > +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
> > +      use this feature when this property does not exist.
> > +      Set to 0 if WP pins are not connected and feature is not
> > +      used. Set to 1 if WP pins are connected and feature is used.
> > +      Set to 2 if WP pins are connected but disable this feature
> > +      through driver that sets controller to output high on NAND_WPb.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    enum: [0, 1, 2]
> 
> Perhaps strings would be welcome. I'll let binding maintainers say what
> they think of it.

1 is redundant as it is the same as not present. Seems like a 
'brcm,disable-wp' boolean would suffice here.

Rob

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-14 22:43   ` Rob Herring
@ 2023-06-15  0:28     ` William Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: William Zhang @ 2023-06-15  0:28 UTC (permalink / raw)
  To: Rob Herring
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Miquel Raynal,
	Richard Weinberger, Kamal Dasu

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

Hi Rob,

On 06/14/2023 03:43 PM, Rob Herring wrote:
> On Tue, Jun 06, 2023 at 04:12:45PM -0700, William Zhang wrote:
>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
>> product. The old compatible string is still kept in the driver so old
>> dtb can still work.
> 
> I don't understand why you need to change this.
> 
> 
Same as the iProc SoC NAND controller flavor for our BCMBCA SoC 
controller. And the existing binding is wrong. It shows 
brcm,nand-bcm63138 can be backward compatible to either v7.0 and v7.1 
controller. But in fact 63138 only use v7.0.

I understand we had discussion before and chip specific name is the 
preferred method but the nand controller here is different and it has 
well defined and documented version control and our bcmbca SoCs 
integrate the controller exactly the same way so no chip specific 
compatible is needed.  In the unlikely event that a future bcmbca SoC 
has a different implementation,  we can always add new chip specific 
compatible.

If you are concerned about ABI breakage on the 63138 compatible, I can 
add it back, although I  find only a few 63138 and 4908 boards use it 
and I did keep it in the driver so they can still bind.

>> Add brcm,nand-use-wp property to have an option for disabling this
>> feature on broadband board design that does not use write protection.
>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
>> broadband board designs because they do not specify ecc setting in dts
>> but rather using the strap setting.
>>
>> Remove the requirement of interrupts and interrupt-names properties to
>> reflect the driver code.
>>
>> This patch also includes a few minor fixes to the BCM63xx compatibles
>> and add myself to the list of maintainers.
>>
>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>> ---
>>
>>   .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
>>   1 file changed, 43 insertions(+), 21 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> index 1571024aa119..1fe1c166a9db 100644
>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
>>   maintainers:
>>     - Brian Norris <computersforpeace@gmail.com>
>>     - Kamal Dasu <kdasu.kdev@gmail.com>
>> +  - William Zhang <william.zhang@broadcom.com>
>>   
>>   description: |
>>     The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
>> @@ -18,9 +19,10 @@ description: |
>>     supports basic PROGRAM and READ functions, among other features.
>>   
>>     This controller was originally designed for STB SoCs (BCM7xxx) but is now
>> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
>> -  iProc/Cygnus. Its history includes several similar (but not fully register
>> -  compatible) versions.
>> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
>> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
>> +  Its history includes several similar (but not fully register compatible)
>> +  versions.
>>   
>>     -- Additional SoC-specific NAND controller properties --
>>   
>> @@ -53,9 +55,9 @@ properties:
>>                 - brcm,brcmnand-v7.2
>>                 - brcm,brcmnand-v7.3
>>             - const: brcm,brcmnand
>> -      - description: BCM63138 SoC-specific NAND controller
>> +      - description: BCMBCA SoC-specific NAND controller
>>           items:
>> -          - const: brcm,nand-bcm63138
>> +          - const: brcm,nand-bcmbca
>>             - enum:
>>                 - brcm,brcmnand-v7.0
>>                 - brcm,brcmnand-v7.1
>> @@ -65,11 +67,15 @@ properties:
>>             - const: brcm,nand-iproc
>>             - const: brcm,brcmnand-v6.1
>>             - const: brcm,brcmnand
>> -      - description: BCM63168 SoC-specific NAND controller
>> +      - description: BCM63xx SoC-specific NAND controller
>>           items:
>> -          - const: brcm,nand-bcm63168
>> -          - const: brcm,nand-bcm6368
>> -          - const: brcm,brcmnand-v4.0
>> +          - enum:
>> +              - brcm,nand-bcm63168
>> +              - brcm,nand-bcm6368
>> +          - enum:
>> +              - brcm,brcmnand-v2.1
>> +              - brcm,brcmnand-v2.2
>> +              - brcm,brcmnand-v4.0
>>             - const: brcm,brcmnand
> 
> Completely changing what's valid here too. You're breaking the ABI.
> 
This binding is wrong, 63168 is not backward compatible to 6368. They 
should backward compatible to their specific controller version. If you 
looks arch/mips/boot/dts/brcm/bcm6328.dtsi, bcm6368.dtsi, bcm63268.dtsi 
and bcm6362.dtsi, their compatible list is correct but the yaml was not 
updated.

> Rob
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

* Re: [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
  2023-06-14 22:46     ` Rob Herring
@ 2023-06-15  0:40       ` William Zhang
  0 siblings, 0 replies; 13+ messages in thread
From: William Zhang @ 2023-06-15  0:40 UTC (permalink / raw)
  To: Rob Herring, Miquel Raynal
  Cc: Broadcom Kernel List, Linux MTD List, f.fainelli, rafal,
	kursad.oney, joel.peshkin, computersforpeace, anand.gore, dregan,
	kamal.dasu, tomer.yacoby, dan.beygelman, devicetree, linux-kernel,
	Krzysztof Kozlowski, Vignesh Raghavendra, Richard Weinberger,
	Kamal Dasu

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

Hi Rob,

On 06/14/2023 03:46 PM, Rob Herring wrote:
> On Wed, Jun 07, 2023 at 10:14:52AM +0200, Miquel Raynal wrote:
>> Hi William,
>>
>> william.zhang@broadcom.com wrote on Tue,  6 Jun 2023 16:12:45 -0700:
>>
>>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband
>>> product. The old compatible string is still kept in the driver so old
>>> dtb can still work.
>>>
>>> Add brcm,nand-use-wp property to have an option for disabling this
>>> feature on broadband board design that does not use write protection.
>>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for
>>> broadband board designs because they do not specify ecc setting in dts
>>> but rather using the strap setting.
>>>
>>> Remove the requirement of interrupts and interrupt-names properties to
>>> reflect the driver code.
>>>
>>> This patch also includes a few minor fixes to the BCM63xx compatibles
>>> and add myself to the list of maintainers.
>>>
>>> Signed-off-by: William Zhang <william.zhang@broadcom.com>
>>> ---
>>>
>>>   .../bindings/mtd/brcm,brcmnand.yaml           | 64 +++++++++++++------
>>>   1 file changed, 43 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>> index 1571024aa119..1fe1c166a9db 100644
>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller
>>>   maintainers:
>>>     - Brian Norris <computersforpeace@gmail.com>
>>>     - Kamal Dasu <kdasu.kdev@gmail.com>
>>> +  - William Zhang <william.zhang@broadcom.com>
>>>   
>>>   description: |
>>>     The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND
>>> @@ -18,9 +19,10 @@ description: |
>>>     supports basic PROGRAM and READ functions, among other features.
>>>   
>>>     This controller was originally designed for STB SoCs (BCM7xxx) but is now
>>> -  available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and
>>> -  iProc/Cygnus. Its history includes several similar (but not fully register
>>> -  compatible) versions.
>>> +  available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based
>>> +  Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus.
>>> +  Its history includes several similar (but not fully register compatible)
>>> +  versions.
>>>   
>>>     -- Additional SoC-specific NAND controller properties --
>>>   
>>> @@ -53,9 +55,9 @@ properties:
>>>                 - brcm,brcmnand-v7.2
>>>                 - brcm,brcmnand-v7.3
>>>             - const: brcm,brcmnand
>>> -      - description: BCM63138 SoC-specific NAND controller
>>> +      - description: BCMBCA SoC-specific NAND controller
>>>           items:
>>> -          - const: brcm,nand-bcm63138
>>> +          - const: brcm,nand-bcmbca
>>>             - enum:
>>>                 - brcm,brcmnand-v7.0
>>>                 - brcm,brcmnand-v7.1
>>> @@ -65,11 +67,15 @@ properties:
>>>             - const: brcm,nand-iproc
>>>             - const: brcm,brcmnand-v6.1
>>>             - const: brcm,brcmnand
>>> -      - description: BCM63168 SoC-specific NAND controller
>>> +      - description: BCM63xx SoC-specific NAND controller
>>>           items:
>>> -          - const: brcm,nand-bcm63168
>>> -          - const: brcm,nand-bcm6368
>>> -          - const: brcm,brcmnand-v4.0
>>> +          - enum:
>>> +              - brcm,nand-bcm63168
>>> +              - brcm,nand-bcm6368
>>> +          - enum:
>>> +              - brcm,brcmnand-v2.1
>>> +              - brcm,brcmnand-v2.2
>>> +              - brcm,brcmnand-v4.0
>>>             - const: brcm,brcmnand
>>>   
>>>     reg:
>>> @@ -111,6 +117,19 @@ properties:
>>>         earlier versions of this core that include WP
>>>       type: boolean
>>>   
>>> +  brcm,nand-use-wp:
>>> +    description:
>>> +      Use this integer to indicate if board design uses
>>> +      controller's write protection feature and connects its
>>> +      NAND_WPb pin to nand chip's WP_L pin. Driver defaults to
>>> +      use this feature when this property does not exist.
>>> +      Set to 0 if WP pins are not connected and feature is not
>>> +      used. Set to 1 if WP pins are connected and feature is used.
>>> +      Set to 2 if WP pins are connected but disable this feature
>>> +      through driver that sets controller to output high on NAND_WPb.
>>> +    $ref: /schemas/types.yaml#/definitions/uint32
>>> +    enum: [0, 1, 2]
>>
>> Perhaps strings would be welcome. I'll let binding maintainers say what
>> they think of it.
> 
> 1 is redundant as it is the same as not present. Seems like a
> 'brcm,disable-wp' boolean would suffice here.
> WP feature present is indicated by the existing flag brcm,nand-has-wp. 
1 is the opposite of 0 (use this feature vs not use this feature while 
chip has this feature). 2 is kind of strange (for feature disabled 
intentionally at run time while feature is used), maybe for testing 
purpose or for certainly nand chip that has issue with WP but the board 
design already connect the WP pin. I keep it here to match the driver 
code that have these three states of the wp usage.

> Rob
> 

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4212 bytes --]

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

end of thread, other threads:[~2023-06-15  0:41 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-06 23:12 [PATCH 00/12] mtd: rawnand: brcmnand: driver and doc updates William Zhang
2023-06-06 23:12 ` [PATCH 05/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs William Zhang
2023-06-07  8:14   ` Miquel Raynal
2023-06-07 20:01     ` William Zhang
2023-06-09  8:58       ` Miquel Raynal
2023-06-09 19:05         ` William Zhang
2023-06-12 17:43           ` Miquel Raynal
2023-06-14 22:46     ` Rob Herring
2023-06-15  0:40       ` William Zhang
2023-06-14 22:43   ` Rob Herring
2023-06-15  0:28     ` William Zhang
2023-06-06 23:12 ` [PATCH 06/12] ARM: dts: broadcom: bcmbca: Add NAND controller node William Zhang
2023-06-06 23:12 ` [PATCH 07/12] arm64: " William Zhang

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).