2024-02-03 00:29:12

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 00/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs

This patch series is an update from the previous version [1] after
exex_op support and fixes (patch 1 to 4 from the previous version.)

It updates all the BCMBCA SoC to support the nand controller and add
functions to handle BCMBCA specific needs on ECC and Write Protection
usage. The device tree document is also updated accordingly with the new
properties needed by the driver.

In addition there is a bug fix for exec_op helper functions and log
level adjustment on uncorrectable ECC error.

[1] https://lore.kernel.org/lkml/[email protected]/

Changes in v4:
- Split the yaml changes into three patches.
- Move the WP pin property to a new patch and change it to boolean type.
- Move ecc strap property to a new patch and remove some non-binding
related text from the description
- Add a new patch for bcm4908 based router board dts update
- Move the board related dts setting from SoC dtsi to board dts
- Update the comments for ecc setting selection
- Use the new brcm,wp-not-connected property based on the dts binding
change
- Fix the commit id in the fixes tag
- Revert the log level change for correctable ecc error

Changes in v3:
- Update brcm,nand-use-wp description
- Revert the description change to BCM63168 SoC-specific NAND controller
- Updated bcmbca_read_data_bus comment

Changes in v2:
- Revert the new compatible string nand-bcmbca
- Drop the BCM63168 compatible fix to avoid any potential ABI
Incompatibility issue
- Simplify the explanation for brcm,nand-use-wp
- Keep the interrupt name requirement when interrupt number is specified
- Add nand controller node label for 4908 so it is consistent with other
SoC's and can be referenced by board dts file
- Drop the is_param argument to the read data bus function now that we
have the exec_op API to read the parameter page and ONFI data
- Minor cosmetic fixes
- Added patches 8, 9, 10 to patch series

David Regan (2):
mtd: rawnand: brcmnand: exec_op helper functions return type fixes
mtd: rawnand: brcmnand: update log level messages

William Zhang (10):
dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs
dt-bindings: mtd: brcmnand: Add WP pin connection property
dt-bindings: mtd: brcmnand: Add ecc strap property
ARM: dts: broadcom: bcmbca: Add NAND controller node
arm64: dts: broadcom: bcmbca: Add NAND controller node
arm64: dts: broadcom: bcmbca: Update router boards
mtd: rawnand: brcmnand: Rename bcm63138 nand driver
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 | 29 +++-
arch/arm/boot/dts/broadcom/bcm47622.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm63138.dtsi | 7 +-
arch/arm/boot/dts/broadcom/bcm63148.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm63178.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm6756.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm6846.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm6855.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm6878.dtsi | 14 ++
arch/arm/boot/dts/broadcom/bcm947622.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm963138.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm963138dvt.dts | 14 +-
arch/arm/boot/dts/broadcom/bcm963148.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm963178.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm96756.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm96846.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm96855.dts | 10 ++
arch/arm/boot/dts/broadcom/bcm96878.dts | 10 ++
.../bcmbca/bcm4906-netgear-r8000p.dts | 5 +
.../bcmbca/bcm4906-tplink-archer-c2300-v1.dts | 5 +
.../bcmbca/bcm4908-asus-gt-ac5300.dts | 6 +-
.../boot/dts/broadcom/bcmbca/bcm4908.dtsi | 4 +-
.../boot/dts/broadcom/bcmbca/bcm4912.dtsi | 14 ++
.../boot/dts/broadcom/bcmbca/bcm63146.dtsi | 14 ++
.../boot/dts/broadcom/bcmbca/bcm63158.dtsi | 14 ++
.../boot/dts/broadcom/bcmbca/bcm6813.dtsi | 14 ++
.../boot/dts/broadcom/bcmbca/bcm6856.dtsi | 14 ++
.../boot/dts/broadcom/bcmbca/bcm6858.dtsi | 14 ++
.../boot/dts/broadcom/bcmbca/bcm94908.dts | 10 ++
.../boot/dts/broadcom/bcmbca/bcm94912.dts | 10 ++
.../boot/dts/broadcom/bcmbca/bcm963146.dts | 10 ++
.../boot/dts/broadcom/bcmbca/bcm963158.dts | 10 ++
.../boot/dts/broadcom/bcmbca/bcm96813.dts | 10 ++
.../boot/dts/broadcom/bcmbca/bcm96856.dts | 10 ++
.../boot/dts/broadcom/bcmbca/bcm96858.dts | 10 ++
drivers/mtd/nand/raw/brcmnand/Makefile | 2 +-
drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c | 99 --------------
drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c | 126 ++++++++++++++++++
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 126 +++++++++++++++---
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 2 +
40 files changed, 623 insertions(+), 134 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



2024-02-03 00:29:31

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 01/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs

Update the descriptions to reflect different families of broadband SoC and
use the general name bcmbca for ARM based SoC.

Remove the requirement of interrupts property to reflect the driver
code and only require interrupt-names when interrupts property present.

Also add myself to the list of maintainers.

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>

---

Changes in v4:
- Split the yaml changes into three patches. This is the first one.

Changes in v3:
- Update brcm,nand-use-wp description
- Revert the description change to BCM63168 SoC-specific NAND controller

Changes in v2:
- Revert the new compatible string nand-bcmbca
- Drop the BCM63168 compatible fix to avoid any potential ABI
incompatibility issue
- Simplify the explanation for brcm,nand-use-wp
- Keep the interrupt name requirement when interrupt number is specified

.../devicetree/bindings/mtd/brcm,brcmnand.yaml | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index f57e96374e67..e54ca08a798a 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 <[email protected]>
- Kamal Dasu <[email protected]>
+ - William Zhang <[email protected]>

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,7 +55,7 @@ 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
- enum:
@@ -177,6 +179,8 @@ allOf:
- const: iproc-idm
- const: iproc-ext
- if:
+ required:
+ - interrupts
properties:
interrupts:
minItems: 2
@@ -189,7 +193,6 @@ unevaluatedProperties: false
required:
- reg
- reg-names
- - interrupts

examples:
- |
--
2.37.3


2024-02-03 00:29:57

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property

Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
board boot strap for broadband board designs because they do not specify
ecc setting in dts but rather using the strap setting.

Signed-off-by: William Zhang <[email protected]>

---

Changes in v4:
- Move ecc strap property to this separate patch and remove some
non-binding related text from the description

Changes in v3: None
Changes in v2: None

Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index d0168d55c73e..2599d902ec3a 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -147,6 +147,14 @@ patternProperties:
layout.
$ref: /schemas/types.yaml#/definitions/uint32

+ brcm,nand-ecc-use-strap:
+ description:
+ This flag indicates the ecc strength and spare area size should
+ be retrieved from the SoC NAND boot strap setting instead of
+ nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.
+ This is commonly used by the BCMBCA SoC board design.
+ $ref: /schemas/types.yaml#/definitions/flag
+
unevaluatedProperties: false

allOf:
--
2.37.3


2024-02-03 00:30:27

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 04/12] ARM: dts: broadcom: bcmbca: Add NAND controller node

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

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>

---

Changes in v4:
- Move the board related dts setting from SoC dtsi to board dts

Changes in v3: None
Changes in v2: None

arch/arm/boot/dts/broadcom/bcm47622.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm63138.dtsi | 7 ++++++-
arch/arm/boot/dts/broadcom/bcm63148.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm63178.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm6756.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm6846.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm6855.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm6878.dtsi | 14 ++++++++++++++
arch/arm/boot/dts/broadcom/bcm947622.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm963138.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm963138dvt.dts | 14 +++++++-------
arch/arm/boot/dts/broadcom/bcm963148.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm963178.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm96756.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm96846.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm96855.dts | 10 ++++++++++
arch/arm/boot/dts/broadcom/bcm96878.dts | 10 ++++++++++
17 files changed, 191 insertions(+), 8 deletions(-)

diff --git a/arch/arm/boot/dts/broadcom/bcm47622.dtsi b/arch/arm/boot/dts/broadcom/bcm47622.dtsi
index 7cd38de118c3..485863f9c420 100644
--- a/arch/arm/boot/dts/broadcom/bcm47622.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm47622.dtsi
@@ -138,6 +138,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
uart0: serial@12000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/broadcom/bcm63138.dtsi b/arch/arm/boot/dts/broadcom/bcm63138.dtsi
index 4ef02283612b..e74ba6bf370d 100644
--- a/arch/arm/boot/dts/broadcom/bcm63138.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm63138.dtsi
@@ -229,7 +229,12 @@ nand_controller: nand-controller@2000 {
reg-names = "nand", "nand-int-base";
status = "disabled";
interrupts = <GIC_SPI 38 IRQ_TYPE_LEVEL_HIGH>;
- interrupt-names = "nand";
+ interrupt-names = "nand_ctlrdy";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
};

serial@4400 {
diff --git a/arch/arm/boot/dts/broadcom/bcm63148.dtsi b/arch/arm/boot/dts/broadcom/bcm63148.dtsi
index 24431de1810e..53703827ee3f 100644
--- a/arch/arm/boot/dts/broadcom/bcm63148.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm63148.dtsi
@@ -119,5 +119,19 @@ hsspi: spi@1000 {
num-cs = <8>;
status = "disabled";
};
+
+ nand_controller: nand-controller@2000 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x2000 0x600>, <0xf0 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
};
};
diff --git a/arch/arm/boot/dts/broadcom/bcm63178.dtsi b/arch/arm/boot/dts/broadcom/bcm63178.dtsi
index 3f9aed96babf..6d8d33498983 100644
--- a/arch/arm/boot/dts/broadcom/bcm63178.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm63178.dtsi
@@ -129,6 +129,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
uart0: serial@12000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/broadcom/bcm6756.dtsi b/arch/arm/boot/dts/broadcom/bcm6756.dtsi
index 1d8d957d65dd..6433f8fa5eff 100644
--- a/arch/arm/boot/dts/broadcom/bcm6756.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm6756.dtsi
@@ -139,6 +139,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
uart0: serial@12000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/broadcom/bcm6846.dtsi b/arch/arm/boot/dts/broadcom/bcm6846.dtsi
index cf92cf8c4693..ee361cb00b7c 100644
--- a/arch/arm/boot/dts/broadcom/bcm6846.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm6846.dtsi
@@ -119,5 +119,19 @@ hsspi: spi@1000 {
num-cs = <8>;
status = "disabled";
};
+
+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
};
};
diff --git a/arch/arm/boot/dts/broadcom/bcm6855.dtsi b/arch/arm/boot/dts/broadcom/bcm6855.dtsi
index 52d6bc89f9f8..52915ec6f339 100644
--- a/arch/arm/boot/dts/broadcom/bcm6855.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm6855.dtsi
@@ -129,6 +129,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
uart0: serial@12000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/broadcom/bcm6878.dtsi b/arch/arm/boot/dts/broadcom/bcm6878.dtsi
index 2c5d706bac7e..70cf23a65fdb 100644
--- a/arch/arm/boot/dts/broadcom/bcm6878.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm6878.dtsi
@@ -120,6 +120,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
uart0: serial@12000 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x12000 0x1000>;
diff --git a/arch/arm/boot/dts/broadcom/bcm947622.dts b/arch/arm/boot/dts/broadcom/bcm947622.dts
index 93b8ce22678d..6241485408d3 100644
--- a/arch/arm/boot/dts/broadcom/bcm947622.dts
+++ b/arch/arm/boot/dts/broadcom/bcm947622.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm963138.dts b/arch/arm/boot/dts/broadcom/bcm963138.dts
index 1b405c249213..7fd87e05ec20 100644
--- a/arch/arm/boot/dts/broadcom/bcm963138.dts
+++ b/arch/arm/boot/dts/broadcom/bcm963138.dts
@@ -29,3 +29,13 @@ &serial0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm963138dvt.dts b/arch/arm/boot/dts/broadcom/bcm963138dvt.dts
index b5af61853a07..f60d09908ab9 100644
--- a/arch/arm/boot/dts/broadcom/bcm963138dvt.dts
+++ b/arch/arm/boot/dts/broadcom/bcm963138dvt.dts
@@ -32,15 +32,15 @@ &serial1 {
};

&nand_controller {
+ brcm,wp-not-connected;
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-sector-size = <16>;
+ nand-on-flash-bbt;
};

&ahci {
diff --git a/arch/arm/boot/dts/broadcom/bcm963148.dts b/arch/arm/boot/dts/broadcom/bcm963148.dts
index 1f5d6d783f09..44bca063a327 100644
--- a/arch/arm/boot/dts/broadcom/bcm963148.dts
+++ b/arch/arm/boot/dts/broadcom/bcm963148.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm963178.dts b/arch/arm/boot/dts/broadcom/bcm963178.dts
index d036e99dd8d1..098a222cd71a 100644
--- a/arch/arm/boot/dts/broadcom/bcm963178.dts
+++ b/arch/arm/boot/dts/broadcom/bcm963178.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm96756.dts b/arch/arm/boot/dts/broadcom/bcm96756.dts
index 8b104f3fb14a..402038d3cd0c 100644
--- a/arch/arm/boot/dts/broadcom/bcm96756.dts
+++ b/arch/arm/boot/dts/broadcom/bcm96756.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm96846.dts b/arch/arm/boot/dts/broadcom/bcm96846.dts
index 55852c229608..943896afb7cc 100644
--- a/arch/arm/boot/dts/broadcom/bcm96846.dts
+++ b/arch/arm/boot/dts/broadcom/bcm96846.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm96855.dts b/arch/arm/boot/dts/broadcom/bcm96855.dts
index 2ad880af2104..571663d9a1ea 100644
--- a/arch/arm/boot/dts/broadcom/bcm96855.dts
+++ b/arch/arm/boot/dts/broadcom/bcm96855.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm/boot/dts/broadcom/bcm96878.dts b/arch/arm/boot/dts/broadcom/bcm96878.dts
index b7af8ade7a9d..8d6eddd54c6e 100644
--- a/arch/arm/boot/dts/broadcom/bcm96878.dts
+++ b/arch/arm/boot/dts/broadcom/bcm96878.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
--
2.37.3


2024-02-03 00:31:00

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 07/12] mtd: rawnand: brcmnand: Rename bcm63138 nand driver

In preparing to support multiple BCMBCA SoCs, rename bcm63138 to bcmbca
in the driver code and driver file name.

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>
Acked-by: Florian Fainelli <[email protected]>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

drivers/mtd/nand/raw/brcmnand/Makefile | 2 +-
drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c | 99 -------------------
drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c | 99 +++++++++++++++++++
3 files changed, 100 insertions(+), 100 deletions(-)
delete mode 100644 drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c
create mode 100644 drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c

diff --git a/drivers/mtd/nand/raw/brcmnand/Makefile b/drivers/mtd/nand/raw/brcmnand/Makefile
index 9907e3ec4bb2..0536568c6467 100644
--- a/drivers/mtd/nand/raw/brcmnand/Makefile
+++ b/drivers/mtd/nand/raw/brcmnand/Makefile
@@ -2,7 +2,7 @@
# link order matters; don't link the more generic brcmstb_nand.o before the
# more specific iproc_nand.o, for instance
obj-$(CONFIG_MTD_NAND_BRCMNAND_IPROC) += iproc_nand.o
-obj-$(CONFIG_MTD_NAND_BRCMNAND_BCMBCA) += bcm63138_nand.o
+obj-$(CONFIG_MTD_NAND_BRCMNAND_BCMBCA) += bcmbca_nand.o
obj-$(CONFIG_MTD_NAND_BRCMNAND_BCM63XX) += bcm6368_nand.o
obj-$(CONFIG_MTD_NAND_BRCMNAND_BRCMSTB) += brcmstb_nand.o
obj-$(CONFIG_MTD_NAND_BRCMNAND) += brcmnand.o
diff --git a/drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c b/drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c
deleted file mode 100644
index 968c5b674b08..000000000000
--- a/drivers/mtd/nand/raw/brcmnand/bcm63138_nand.c
+++ /dev/null
@@ -1,99 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright © 2015 Broadcom Corporation
- */
-
-#include <linux/device.h>
-#include <linux/io.h>
-#include <linux/ioport.h>
-#include <linux/module.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <linux/platform_device.h>
-#include <linux/slab.h>
-
-#include "brcmnand.h"
-
-struct bcm63138_nand_soc {
- struct brcmnand_soc soc;
- void __iomem *base;
-};
-
-#define BCM63138_NAND_INT_STATUS 0x00
-#define BCM63138_NAND_INT_EN 0x04
-
-enum {
- BCM63138_CTLRDY = BIT(4),
-};
-
-static bool bcm63138_nand_intc_ack(struct brcmnand_soc *soc)
-{
- struct bcm63138_nand_soc *priv =
- container_of(soc, struct bcm63138_nand_soc, soc);
- void __iomem *mmio = priv->base + BCM63138_NAND_INT_STATUS;
- u32 val = brcmnand_readl(mmio);
-
- if (val & BCM63138_CTLRDY) {
- brcmnand_writel(val & ~BCM63138_CTLRDY, mmio);
- return true;
- }
-
- return false;
-}
-
-static void bcm63138_nand_intc_set(struct brcmnand_soc *soc, bool en)
-{
- struct bcm63138_nand_soc *priv =
- container_of(soc, struct bcm63138_nand_soc, soc);
- void __iomem *mmio = priv->base + BCM63138_NAND_INT_EN;
- u32 val = brcmnand_readl(mmio);
-
- if (en)
- val |= BCM63138_CTLRDY;
- else
- val &= ~BCM63138_CTLRDY;
-
- brcmnand_writel(val, mmio);
-}
-
-static int bcm63138_nand_probe(struct platform_device *pdev)
-{
- struct device *dev = &pdev->dev;
- struct bcm63138_nand_soc *priv;
- struct brcmnand_soc *soc;
-
- priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
- return -ENOMEM;
- soc = &priv->soc;
-
- priv->base = devm_platform_ioremap_resource_byname(pdev, "nand-int-base");
- if (IS_ERR(priv->base))
- return PTR_ERR(priv->base);
-
- soc->ctlrdy_ack = bcm63138_nand_intc_ack;
- soc->ctlrdy_set_enabled = bcm63138_nand_intc_set;
-
- return brcmnand_probe(pdev, soc);
-}
-
-static const struct of_device_id bcm63138_nand_of_match[] = {
- { .compatible = "brcm,nand-bcm63138" },
- {},
-};
-MODULE_DEVICE_TABLE(of, bcm63138_nand_of_match);
-
-static struct platform_driver bcm63138_nand_driver = {
- .probe = bcm63138_nand_probe,
- .remove_new = brcmnand_remove,
- .driver = {
- .name = "bcm63138_nand",
- .pm = &brcmnand_pm_ops,
- .of_match_table = bcm63138_nand_of_match,
- }
-};
-module_platform_driver(bcm63138_nand_driver);
-
-MODULE_LICENSE("GPL v2");
-MODULE_AUTHOR("Brian Norris");
-MODULE_DESCRIPTION("NAND driver for BCM63138");
diff --git a/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c b/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c
new file mode 100644
index 000000000000..3e2f3b79788d
--- /dev/null
+++ b/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c
@@ -0,0 +1,99 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright © 2015 Broadcom Corporation
+ */
+
+#include <linux/device.h>
+#include <linux/io.h>
+#include <linux/ioport.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#include "brcmnand.h"
+
+struct bcmbca_nand_soc {
+ struct brcmnand_soc soc;
+ void __iomem *base;
+};
+
+#define BCMBCA_NAND_INT_STATUS 0x00
+#define BCMBCA_NAND_INT_EN 0x04
+
+enum {
+ BCMBCA_CTLRDY = BIT(4),
+};
+
+static bool bcmbca_nand_intc_ack(struct brcmnand_soc *soc)
+{
+ struct bcmbca_nand_soc *priv =
+ container_of(soc, struct bcmbca_nand_soc, soc);
+ void __iomem *mmio = priv->base + BCMBCA_NAND_INT_STATUS;
+ u32 val = brcmnand_readl(mmio);
+
+ if (val & BCMBCA_CTLRDY) {
+ brcmnand_writel(val & ~BCMBCA_CTLRDY, mmio);
+ return true;
+ }
+
+ return false;
+}
+
+static void bcmbca_nand_intc_set(struct brcmnand_soc *soc, bool en)
+{
+ struct bcmbca_nand_soc *priv =
+ container_of(soc, struct bcmbca_nand_soc, soc);
+ void __iomem *mmio = priv->base + BCMBCA_NAND_INT_EN;
+ u32 val = brcmnand_readl(mmio);
+
+ if (en)
+ val |= BCMBCA_CTLRDY;
+ else
+ val &= ~BCMBCA_CTLRDY;
+
+ brcmnand_writel(val, mmio);
+}
+
+static int bcmbca_nand_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct bcmbca_nand_soc *priv;
+ struct brcmnand_soc *soc;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+ soc = &priv->soc;
+
+ priv->base = devm_platform_ioremap_resource_byname(pdev, "nand-int-base");
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ soc->ctlrdy_ack = bcmbca_nand_intc_ack;
+ soc->ctlrdy_set_enabled = bcmbca_nand_intc_set;
+
+ return brcmnand_probe(pdev, soc);
+}
+
+static const struct of_device_id bcmbca_nand_of_match[] = {
+ { .compatible = "brcm,nand-bcm63138" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, bcmbca_nand_of_match);
+
+static struct platform_driver bcmbca_nand_driver = {
+ .probe = bcmbca_nand_probe,
+ .remove_new = brcmnand_remove,
+ .driver = {
+ .name = "bcmbca_nand",
+ .pm = &brcmnand_pm_ops,
+ .of_match_table = bcmbca_nand_of_match,
+ }
+};
+module_platform_driver(bcmbca_nand_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Brian Norris");
+MODULE_DESCRIPTION("NAND driver for BCMBCA");
--
2.37.3


2024-02-03 00:31:17

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 09/12] mtd: rawnand: brcmnand: Add support for getting ecc setting from strap

BCMBCA broadband SoC based board design does not specify ecc setting in
dts but rather use the SoC NAND strap info to obtain the ecc strength
and spare area size setting. Add brcm,nand-ecc-use-strap dts propety for
this purpose and update driver to support this option.

The generic nand ecc settings still take precedence over this flag. For
example, if nand-ecc-strength is set in the dts, the driver ignores the
strap setting and falls back to original behavior. This makes sure that
the existing BCMBCA board dts still works the old way even the strap
flag is set in the BCMBCA chip dtsi.

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>

---

Changes in v4:
- Update the comments for ecc setting selection

Changes in v3: None
Changes in v2:
- Minor cosmetic fixes

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 83 ++++++++++++++++++++++--
1 file changed, 76 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 73fdf7ce21aa..efeee9e80213 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1038,6 +1038,19 @@ static inline int brcmnand_sector_1k_shift(struct brcmnand_controller *ctrl)
return -1;
}

+static int brcmnand_get_sector_size_1k(struct brcmnand_host *host)
+{
+ struct brcmnand_controller *ctrl = host->ctrl;
+ int shift = brcmnand_sector_1k_shift(ctrl);
+ u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+ BRCMNAND_CS_ACC_CONTROL);
+
+ if (shift < 0)
+ return 0;
+
+ return (nand_readreg(ctrl, acc_control_offs) >> shift) & 0x1;
+}
+
static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
{
struct brcmnand_controller *ctrl = host->ctrl;
@@ -1055,6 +1068,38 @@ static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
nand_writereg(ctrl, acc_control_offs, tmp);
}

+static int brcmnand_get_spare_size(struct brcmnand_host *host)
+{
+ struct brcmnand_controller *ctrl = host->ctrl;
+ u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+ BRCMNAND_CS_ACC_CONTROL);
+ u32 acc = nand_readreg(ctrl, acc_control_offs);
+
+ return (acc & brcmnand_spare_area_mask(ctrl));
+}
+
+static int brcmnand_get_ecc_strength(struct brcmnand_host *host)
+{
+ struct brcmnand_controller *ctrl = host->ctrl;
+ u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+ BRCMNAND_CS_ACC_CONTROL);
+ int sector_size_1k = brcmnand_get_sector_size_1k(host);
+ int spare_area_size, ecc_level, ecc_strength;
+ u32 acc;
+
+ spare_area_size = brcmnand_get_spare_size(host);
+ acc = nand_readreg(ctrl, acc_control_offs);
+ ecc_level = (acc & brcmnand_ecc_level_mask(ctrl)) >> ctrl->ecc_level_shift;
+ if (sector_size_1k)
+ ecc_strength = ecc_level * 2;
+ else if (spare_area_size == 16 && ecc_level == 15)
+ ecc_strength = 1; /* hamming */
+ else
+ ecc_strength = ecc_level;
+
+ return ecc_strength;
+}
+
/***********************************************************************
* CS_NAND_SELECT
***********************************************************************/
@@ -2622,19 +2667,43 @@ static int brcmnand_setup_dev(struct brcmnand_host *host)
nanddev_get_memorg(&chip->base);
struct brcmnand_controller *ctrl = host->ctrl;
struct brcmnand_cfg *cfg = &host->hwcfg;
- char msg[128];
+ struct device_node *np = nand_get_flash_node(chip);
u32 offs, tmp, oob_sector;
- int ret;
+ int ret, sector_size_1k = 0;
+ bool use_strap = false;
+ char msg[128];

memset(cfg, 0, sizeof(*cfg));
+ use_strap = of_property_read_bool(np, "brcm,nand-ecc-use-strap");

- ret = of_property_read_u32(nand_get_flash_node(chip),
- "brcm,nand-oob-sector-size",
+ /*
+ * Set ECC size and strength based on hw configuration from strap
+ * if brcm,nand-ecc-use-strap is set. However if nand-ecc-strength
+ * is set, its value will be used and ignore the strap setting.
+ */
+ if (chip->ecc.strength)
+ use_strap = 0;
+
+ if (use_strap) {
+ chip->ecc.strength = brcmnand_get_ecc_strength(host);
+ sector_size_1k = brcmnand_get_sector_size_1k(host);
+ if (chip->ecc.size == 0) {
+ if (sector_size_1k < 0)
+ chip->ecc.size = 512;
+ else
+ chip->ecc.size = 512 << sector_size_1k;
+ }
+ }
+
+ ret = of_property_read_u32(np, "brcm,nand-oob-sector-size",
&oob_sector);
if (ret) {
- /* Use detected size */
- cfg->spare_area_size = mtd->oobsize /
- (mtd->writesize >> FC_SHIFT);
+ if (use_strap)
+ cfg->spare_area_size = brcmnand_get_spare_size(host);
+ else
+ /* Use detected size */
+ cfg->spare_area_size = mtd->oobsize /
+ (mtd->writesize >> FC_SHIFT);
} else {
cfg->spare_area_size = oob_sector;
}
--
2.37.3


2024-02-03 00:31:52

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 11/12] mtd: rawnand: brcmnand: exec_op helper functions return type fixes

From: David Regan <[email protected]>

fix return type for exec_op reset and status detect helper functions

Reported-by: Dan Carpenter <[email protected]>
Closes: http://lists.infradead.org/pipermail/linux-mtd/2023-December/102423.html
Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
Signed-off-by: David Regan <[email protected]>
Signed-off-by: William Zhang <[email protected]>
Reviewed-by: William Zhang <[email protected]>

---

Changes in v4:
- Fix the commit id in the fixes tag

Changes in v3: None
Changes in v2:
- Added to patch series

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 8c1489ff7bd6..7ce2b267676f 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -625,7 +625,7 @@ enum {
/* Only for v7.2 */
#define ACC_CONTROL_ECC_EXT_SHIFT 13

-static u8 brcmnand_status(struct brcmnand_host *host);
+static int brcmnand_status(struct brcmnand_host *host);

static inline bool brcmnand_non_mmio_ops(struct brcmnand_controller *ctrl)
{
@@ -1749,7 +1749,7 @@ static int brcmnand_waitfunc(struct nand_chip *chip)
INTFC_FLASH_STATUS;
}

-static u8 brcmnand_status(struct brcmnand_host *host)
+static int brcmnand_status(struct brcmnand_host *host)
{
struct nand_chip *chip = &host->chip;
struct mtd_info *mtd = nand_to_mtd(chip);
@@ -1760,7 +1760,7 @@ static u8 brcmnand_status(struct brcmnand_host *host)
return brcmnand_waitfunc(chip);
}

-static u8 brcmnand_reset(struct brcmnand_host *host)
+static int brcmnand_reset(struct brcmnand_host *host)
{
struct nand_chip *chip = &host->chip;

@@ -2492,11 +2492,14 @@ static int brcmnand_exec_op(struct nand_chip *chip,

if (brcmnand_op_is_status(op)) {
status = op->instrs[1].ctx.data.buf.in;
- *status = brcmnand_status(host);
+ ret = brcmnand_status(host);
+ if (ret < 0)
+ return ret;
+
+ *status = ret & 0xFF;

return 0;
- }
- else if (brcmnand_op_is_reset(op)) {
+ } else if (brcmnand_op_is_reset(op)) {
ret = brcmnand_reset(host);
if (ret < 0)
return ret;
--
2.37.3


2024-02-03 01:33:01

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 08/12] mtd: rawnand: brcmnand: Add BCMBCA read data bus interface

The BCMBCA broadband SoC integrates the NAND controller differently than
STB, iProc and other SoCs. It has different endianness for NAND cache
data.

Add a SoC read data bus shim for BCMBCA to meet the specific SoC need
and performance improvement using the optimized memcpy function on NAND
cache memory.

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>

---

Changes in v4: None
Changes in v3:
- Updated bcmbca_read_data_bus comment

Changes in v2:
- Drop the is_param argument to the read data bus function now that we
have the exec_op API to read the parameter page and ONFI data
- Remove be32_to_cpu from brcmnand_read_data_bus

drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c | 27 +++++++++++++++++++++
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 20 ++++++++++++---
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 2 ++
3 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c b/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c
index 3e2f3b79788d..7ad3e7a98f97 100644
--- a/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c
+++ b/drivers/mtd/nand/raw/brcmnand/bcmbca_nand.c
@@ -26,6 +26,18 @@ enum {
BCMBCA_CTLRDY = BIT(4),
};

+#if defined(CONFIG_ARM64)
+#define ALIGN_REQ 8
+#else
+#define ALIGN_REQ 4
+#endif
+
+static inline bool bcmbca_nand_is_buf_aligned(void *flash_cache, void *buffer)
+{
+ return IS_ALIGNED((uintptr_t)buffer, ALIGN_REQ) &&
+ IS_ALIGNED((uintptr_t)flash_cache, ALIGN_REQ);
+}
+
static bool bcmbca_nand_intc_ack(struct brcmnand_soc *soc)
{
struct bcmbca_nand_soc *priv =
@@ -56,6 +68,20 @@ static void bcmbca_nand_intc_set(struct brcmnand_soc *soc, bool en)
brcmnand_writel(val, mmio);
}

+static void bcmbca_read_data_bus(struct brcmnand_soc *soc,
+ void __iomem *flash_cache, u32 *buffer, int fc_words)
+{
+ /*
+ * memcpy can do unaligned aligned access depending on source
+ * and dest address, which is incompatible with nand cache. Fallback
+ * to the memcpy_fromio in such case
+ */
+ if (bcmbca_nand_is_buf_aligned((void *)flash_cache, buffer))
+ memcpy((void *)buffer, (void *)flash_cache, fc_words * 4);
+ else
+ memcpy_fromio((void *)buffer, flash_cache, fc_words * 4);
+}
+
static int bcmbca_nand_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
@@ -73,6 +99,7 @@ static int bcmbca_nand_probe(struct platform_device *pdev)

soc->ctlrdy_ack = bcmbca_nand_intc_ack;
soc->ctlrdy_set_enabled = bcmbca_nand_intc_set;
+ soc->read_data_bus = bcmbca_read_data_bus;

return brcmnand_probe(pdev, soc);
}
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 8faca43ae1ff..73fdf7ce21aa 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -851,6 +851,20 @@ static inline u32 edu_readl(struct brcmnand_controller *ctrl,
return brcmnand_readl(ctrl->edu_base + offs);
}

+static inline void brcmnand_read_data_bus(struct brcmnand_controller *ctrl,
+ void __iomem *flash_cache, u32 *buffer, int fc_words)
+{
+ struct brcmnand_soc *soc = ctrl->soc;
+ int i;
+
+ if (soc->read_data_bus) {
+ soc->read_data_bus(soc, flash_cache, buffer, fc_words);
+ } else {
+ for (i = 0; i < fc_words; i++)
+ buffer[i] = brcmnand_read_fc(ctrl, i);
+ }
+}
+
static void brcmnand_clear_ecc_addr(struct brcmnand_controller *ctrl)
{

@@ -1975,7 +1989,7 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
{
struct brcmnand_host *host = nand_get_controller_data(chip);
struct brcmnand_controller *ctrl = host->ctrl;
- int i, j, ret = 0;
+ int i, ret = 0;

brcmnand_clear_ecc_addr(ctrl);

@@ -1988,8 +2002,8 @@ static int brcmnand_read_by_pio(struct mtd_info *mtd, struct nand_chip *chip,
if (likely(buf)) {
brcmnand_soc_data_bus_prepare(ctrl->soc, false);

- for (j = 0; j < FC_WORDS; j++, buf++)
- *buf = brcmnand_read_fc(ctrl, j);
+ brcmnand_read_data_bus(ctrl, ctrl->nand_fc, buf, FC_WORDS);
+ buf += FC_WORDS;

brcmnand_soc_data_bus_unprepare(ctrl->soc, false);
}
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.h b/drivers/mtd/nand/raw/brcmnand/brcmnand.h
index 928114c0be5e..7261a69989fe 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.h
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.h
@@ -24,6 +24,8 @@ struct brcmnand_soc {
void (*ctlrdy_set_enabled)(struct brcmnand_soc *soc, bool en);
void (*prepare_data_bus)(struct brcmnand_soc *soc, bool prepare,
bool is_param);
+ void (*read_data_bus)(struct brcmnand_soc *soc, void __iomem *flash_cache,
+ u32 *buffer, int fc_words);
const struct brcmnand_io_ops *ops;
};

--
2.37.3


2024-02-03 01:34:15

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 12/12] mtd: rawnand: brcmnand: update log level messages

From: David Regan <[email protected]>

Update log level messages so that more critical messages
can be seen.

Signed-off-by: David Regan <[email protected]>
Signed-off-by: William Zhang <[email protected]>
Reviewed-by: William Zhang <[email protected]>

---

Changes in v4:
- Revert the log level change for correctable ecc error

Changes in v3: None
Changes in v2:
- Added to patch series

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 7ce2b267676f..e50582b45182 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1143,7 +1143,7 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
if ((val & mask) == expected_val)
return 0;

- dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
+ dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
expected_val, val & mask);

return -ETIMEDOUT;
@@ -2196,7 +2196,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
return err;
}

- dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
+ dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
(unsigned long long)err_addr);
mtd->ecc_stats.failed++;
/* NAND layer expects zero on ECC errors */
--
2.37.3


2024-02-03 01:51:21

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 05/12] arm64: dts: broadcom: bcmbca: Add NAND controller node

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

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>

---

Changes in v4:
- Move the board related dts setting from SoC dtsi to board dts

Changes in v3: None
Changes in v2:
- Add nand controller node label for 4908 so it is consistent with other
SoCs and can be referenced by board dts file

arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi | 4 ++--
arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi | 14 ++++++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi | 14 ++++++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi | 14 ++++++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi | 14 ++++++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi | 14 ++++++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi | 14 ++++++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts | 10 ++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts | 10 ++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts | 10 ++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts | 10 ++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts | 10 ++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts | 10 ++++++++++
arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts | 10 ++++++++++
14 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
index 2f124b027bbf..336016e334d9 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908.dtsi
@@ -589,7 +589,7 @@ hsspi: spi@1000 {
status = "disabled";
};

- nand-controller@1800 {
+ nand_controller: nand-controller@1800 {
#address-cells = <1>;
#size-cells = <0>;
compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
@@ -597,7 +597,7 @@ nand-controller@1800 {
reg-names = "nand", "nand-int-base";
interrupts = <GIC_SPI 37 IRQ_TYPE_LEVEL_HIGH>;
interrupt-names = "nand_ctlrdy";
- status = "okay";
+ status = "disabled";

nandcs: nand@0 {
compatible = "brcm,nandcs";
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
index d658c81f7285..14b2adfb817c 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4912.dtsi
@@ -138,6 +138,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
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 4f474d47022e..589b8a1efc72 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63146.dtsi
@@ -119,6 +119,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
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 909f254dc47d..48d618e75866 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm63158.dtsi
@@ -137,6 +137,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
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 685ae32951c9..1d1303cf90f3 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6813.dtsi
@@ -138,6 +138,20 @@ hsspi: spi@1000 {
status = "disabled";
};

+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
+
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 820553ce541b..00c62c1e5df0 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6856.dtsi
@@ -119,5 +119,19 @@ hsspi: spi@1000 {
num-cs = <8>;
status = "disabled";
};
+
+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
};
};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
index 0eb93c298297..caeaf428dc15 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm6858.dtsi
@@ -156,5 +156,19 @@ hsspi: spi@1000 {
num-cs = <8>;
status = "disabled";
};
+
+ nand_controller: nand-controller@1800 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ compatible = "brcm,nand-bcm63138", "brcm,brcmnand-v7.1", "brcm,brcmnand";
+ reg = <0x1800 0x600>, <0x2000 0x10>;
+ reg-names = "nand", "nand-int-base";
+ status = "disabled";
+
+ nandcs: nand@0 {
+ compatible = "brcm,nandcs";
+ reg = <0>;
+ };
+ };
};
};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts
index c4e6e71f6310..030ffa5364fb 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94908.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ 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..4b779e6c22e1 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm94912.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
index db2c82d6dfd8..2851e8e41bf4 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963146.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
index 25c12bc63545..17dc594fe83f 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm963158.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
index faba21f03120..34832a734734 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96813.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
index 9808331eede2..e1396b5544b7 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96856.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
index 1f561c8e13b0..30bbf6f2917e 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm96858.dts
@@ -32,3 +32,13 @@ &uart0 {
&hsspi {
status = "okay";
};
+
+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
+&nandcs {
+ nand-on-flash-bbt;
+ brcm,nand-ecc-use-strap;
+};
--
2.37.3


2024-02-03 02:57:30

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 02/12] dt-bindings: mtd: brcmnand: Add WP pin connection property

Add brcm,wp-not-connected property to have an option for disabling this
feature on broadband board design that does not connect WP pin.

Signed-off-by: William Zhang <[email protected]>

---

Changes in v4:
- Move the WP pin property to this separate patch and change it to
boolean type.

Changes in v3: None
Changes in v2: None

Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index e54ca08a798a..d0168d55c73e 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -113,6 +113,14 @@ properties:
earlier versions of this core that include WP
type: boolean

+ brcm,wp-not-connected:
+ description:
+ Use this property when board design does not connect controller's
+ NAND_WPb pin to NAND chip's WP_L pin and disable the write
+ protection feature. By default, controller assumes the pin is
+ connected and feature is used.
+ $ref: /schemas/types.yaml#/definitions/flag
+
patternProperties:
"^nand@[a-f0-9]$":
type: object
--
2.37.3


2024-02-03 03:23:49

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 10/12] mtd: rawnand: brcmnand: Support write protection setting from dts

The write protection feature is controlled by the module parameter wp_on
with default set to enabled. But not all the board use this feature
especially in BCMBCA broadband board. And module parameter is not
sufficient as different board can have different option. Add a device
tree property and allow this feature to be configured through the board
dts on per board basis.

Signed-off-by: William Zhang <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
Reviewed-by: Kamal Dasu <[email protected]>
Reviewed-by: David Regan <[email protected]>

---

Changes in v4:
- Use the new brcm,wp-not-connected property based on the dts binding
change

Changes in v3: None
Changes in v2: None

drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index efeee9e80213..8c1489ff7bd6 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -3218,6 +3218,10 @@ int brcmnand_probe(struct platform_device *pdev, struct brcmnand_soc *soc)
/* Disable XOR addressing */
brcmnand_rmw_reg(ctrl, BRCMNAND_CS_XOR, 0xff, 0, 0);

+ /* Check if the board connects the WP pin */
+ if (of_property_read_bool(dn, "brcm,wp-not-connected"))
+ wp_on = 0;
+
if (ctrl->features & BRCMNAND_HAS_WP) {
/* Permanently disable write protection */
if (wp_on == 2)
--
2.37.3


2024-02-03 04:31:33

by William Zhang

[permalink] [raw]
Subject: [PATCH v4 06/12] arm64: dts: broadcom: bcmbca: Update router boards

Enable the nand controller and add WP pin connection property in actual
board dts as they are board level properties now that they are disabled
and moved out from SoC dtsi.

Also remove the unnecessary brcm,nand-has-wp property from AC5300 board.
This property is only needed for some old controller that this board
does not apply.

Signed-off-by: William Zhang <[email protected]>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

.../boot/dts/broadcom/bcmbca/bcm4906-netgear-r8000p.dts | 5 +++++
.../dts/broadcom/bcmbca/bcm4906-tplink-archer-c2300-v1.dts | 5 +++++
.../boot/dts/broadcom/bcmbca/bcm4908-asus-gt-ac5300.dts | 6 +++++-
3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-netgear-r8000p.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-netgear-r8000p.dts
index 78204d71ecd2..999d93730240 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-netgear-r8000p.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-netgear-r8000p.dts
@@ -125,6 +125,11 @@ port@7 {
};
};

+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
&nandcs {
nand-ecc-strength = <4>;
nand-ecc-step-size = <512>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-tplink-archer-c2300-v1.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-tplink-archer-c2300-v1.dts
index fcf092c81b59..19fc03ef47a0 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-tplink-archer-c2300-v1.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4906-tplink-archer-c2300-v1.dts
@@ -155,6 +155,11 @@ port@7 {
};
};

+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
&nandcs {
nand-ecc-strength = <4>;
nand-ecc-step-size = <512>;
diff --git a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908-asus-gt-ac5300.dts b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908-asus-gt-ac5300.dts
index d94a53d68320..52f928dbfa3c 100644
--- a/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908-asus-gt-ac5300.dts
+++ b/arch/arm64/boot/dts/broadcom/bcmbca/bcm4908-asus-gt-ac5300.dts
@@ -166,11 +166,15 @@ led@19 {
};
};

+&nand_controller {
+ brcm,wp-not-connected;
+ status = "okay";
+};
+
&nandcs {
nand-ecc-strength = <4>;
nand-ecc-step-size = <512>;
nand-on-flash-bbt;
- brcm,nand-has-wp;

#address-cells = <1>;
#size-cells = <0>;
--
2.37.3


2024-02-03 14:50:03

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property

On Fri, Feb 02, 2024 at 04:28:24PM -0800, William Zhang wrote:
> Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
> board boot strap for broadband board designs because they do not specify
> ecc setting in dts but rather using the strap setting.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v4:
> - Move ecc strap property to this separate patch and remove some
> non-binding related text from the description
>
> Changes in v3: None
> Changes in v2: None
>
> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> index d0168d55c73e..2599d902ec3a 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> @@ -147,6 +147,14 @@ patternProperties:
> layout.
> $ref: /schemas/types.yaml#/definitions/uint32
>
> + brcm,nand-ecc-use-strap:
> + description:
> + This flag indicates the ecc strength and spare area size should
> + be retrieved from the SoC NAND boot strap setting instead of
> + nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.

I'm still on the fence about this being overly prescriptive about the
operating systems behaviour. I think it would be good to say why the
strap values are better than those explicitly provided in DT rather than
just saying "these strap values should be used".

> + This is commonly used by the BCMBCA SoC board design.
> + $ref: /schemas/types.yaml#/definitions/flag
> +
> unevaluatedProperties: false
>
> allOf:
> --
> 2.37.3
>


Attachments:
(No filename) (1.81 kB)
signature.asc (235.00 B)
Download all attachments

2024-02-03 14:51:16

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH v4 02/12] dt-bindings: mtd: brcmnand: Add WP pin connection property

On Fri, Feb 02, 2024 at 04:28:23PM -0800, William Zhang wrote:
> Add brcm,wp-not-connected property to have an option for disabling this
> feature on broadband board design that does not connect WP pin.
>
> Signed-off-by: William Zhang <[email protected]>

Reviewed-by: Conor Dooley <[email protected]>

Thanks,
Conor.


Attachments:
(No filename) (348.00 B)
signature.asc (235.00 B)
Download all attachments

2024-02-04 21:57:13

by William Zhang

[permalink] [raw]
Subject: Re: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property



On 2/3/24 06:49, Conor Dooley wrote:
> On Fri, Feb 02, 2024 at 04:28:24PM -0800, William Zhang wrote:
>> Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
>> board boot strap for broadband board designs because they do not specify
>> ecc setting in dts but rather using the strap setting.
>>
>> Signed-off-by: William Zhang <[email protected]>
>>
>> ---
>>
>> Changes in v4:
>> - Move ecc strap property to this separate patch and remove some
>> non-binding related text from the description
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> index d0168d55c73e..2599d902ec3a 100644
>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> @@ -147,6 +147,14 @@ patternProperties:
>> layout.
>> $ref: /schemas/types.yaml#/definitions/uint32
>>
>> + brcm,nand-ecc-use-strap:
>> + description:
>> + This flag indicates the ecc strength and spare area size should
>> + be retrieved from the SoC NAND boot strap setting instead of
>> + nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.
>
> I'm still on the fence about this being overly prescriptive about the
> operating systems behaviour. I think it would be good to say why the
> strap values are better than those explicitly provided in DT rather than
> just saying "these strap values should be used".
>
This is a board/SoC design choice. I wouldn't advise it as better
choice as other board/SoC may not have that option. But definitively for
BCMBCA SoC board design, it is better and much easier and convenient
option than explicit dt setting. How about: This property provides a
choice for retrieving ecc strength and spare area size from the SoC NAND
boot strap setting. It is commonly used by the BCMBCA SoC board design.

>> + This is commonly used by the BCMBCA SoC board design.
>> + $ref: /schemas/types.yaml#/definitions/flag
>> +
>> unevaluatedProperties: false
>>
>> allOf:
>> --
>> 2.37.3
>>


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2024-02-05 13:26:59

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property

Hi,

[email protected] wrote on Sat, 3 Feb 2024 14:49:50 +0000:

> On Fri, Feb 02, 2024 at 04:28:24PM -0800, William Zhang wrote:
> > Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
> > board boot strap for broadband board designs because they do not specify
> > ecc setting in dts but rather using the strap setting.
> >
> > Signed-off-by: William Zhang <[email protected]>
> >
> > ---
> >
> > Changes in v4:
> > - Move ecc strap property to this separate patch and remove some
> > non-binding related text from the description
> >
> > Changes in v3: None
> > Changes in v2: None
> >
> > Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > index d0168d55c73e..2599d902ec3a 100644
> > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> > @@ -147,6 +147,14 @@ patternProperties:
> > layout.
> > $ref: /schemas/types.yaml#/definitions/uint32
> >
> > + brcm,nand-ecc-use-strap:
> > + description:
> > + This flag indicates the ecc strength and spare area size should
> > + be retrieved from the SoC NAND boot strap setting instead of
> > + nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.
>
> I'm still on the fence about this being overly prescriptive about the
> operating systems behaviour. I think it would be good to say why the
> strap values are better than those explicitly provided in DT rather than
> just saying "these strap values should be used".

I don't know if there is a point is saying why they would be better, as
they are not. It is a -questionable- design choice. However I would
just get rid of any mention to other properties. Just say one should
expect the strap value to be read and applied by the system when this
property is present.

> > + This is commonly used by the BCMBCA SoC board design.
> > + $ref: /schemas/types.yaml#/definitions/flag
> > +
> > unevaluatedProperties: false
> >
> > allOf:
> > --
> > 2.37.3
> >


Thanks,
Miquèl

2024-02-05 13:32:59

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 02/12] dt-bindings: mtd: brcmnand: Add WP pin connection property

Hi William,

[email protected] wrote on Fri, 2 Feb 2024 16:28:23 -0800:

> Add brcm,wp-not-connected property to have an option for disabling this
> feature on broadband board design that does not connect WP pin.
>
> Signed-off-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v4:
> - Move the WP pin property to this separate patch and change it to
> boolean type.
>
> Changes in v3: None
> Changes in v2: None
>
> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> index e54ca08a798a..d0168d55c73e 100644
> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> @@ -113,6 +113,14 @@ properties:
> earlier versions of this core that include WP
> type: boolean
>
> + brcm,wp-not-connected:
> + description:
> + Use this property when board design does not connect controller's
> + NAND_WPb pin to NAND chip's WP_L pin and disable the write
> + protection feature. By default, controller assumes the pin is
> + connected and feature is used.

I would rephrase slightly. What about:

WP pin is not physically wired to the NAND chip. Write
protection feature cannot be used.

> + $ref: /schemas/types.yaml#/definitions/flag
> +
> patternProperties:
> "^nand@[a-f0-9]$":
> type: object


Thanks,
Miquèl

2024-02-05 13:38:35

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 12/12] mtd: rawnand: brcmnand: update log level messages

Hi William,

[email protected] wrote on Fri, 2 Feb 2024 16:28:33 -0800:

> From: David Regan <[email protected]>
>
> Update log level messages so that more critical messages
> can be seen.

This commit log does not convince me. Warning messages are visible,
they are in dmesg. If you want them on your console, lower your default
console level by 1 and they will appear. I'm fine increasing the log
level on error messages, but the justification cannot be specific to
your own setup.

>
> Signed-off-by: David Regan <[email protected]>
> Signed-off-by: William Zhang <[email protected]>
> Reviewed-by: William Zhang <[email protected]>
>
> ---
>
> Changes in v4:
> - Revert the log level change for correctable ecc error
>
> Changes in v3: None
> Changes in v2:
> - Added to patch series
>
> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index 7ce2b267676f..e50582b45182 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1143,7 +1143,7 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
> if ((val & mask) == expected_val)
> return 0;
>
> - dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
> + dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
> expected_val, val & mask);
>
> return -ETIMEDOUT;
> @@ -2196,7 +2196,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
> return err;
> }
>
> - dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
> + dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
> (unsigned long long)err_addr);
> mtd->ecc_stats.failed++;
> /* NAND layer expects zero on ECC errors */


Thanks,
Miquèl

2024-02-05 18:05:49

by William Zhang

[permalink] [raw]
Subject: Re: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property

Hi Miquel,

On 2/5/24 05:26, Miquel Raynal wrote:
> Hi,
>
> [email protected] wrote on Sat, 3 Feb 2024 14:49:50 +0000:
>
>> On Fri, Feb 02, 2024 at 04:28:24PM -0800, William Zhang wrote:
>>> Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
>>> board boot strap for broadband board designs because they do not specify
>>> ecc setting in dts but rather using the strap setting.
>>>
>>> Signed-off-by: William Zhang <[email protected]>
>>>
>>> ---
>>>
>>> Changes in v4:
>>> - Move ecc strap property to this separate patch and remove some
>>> non-binding related text from the description
>>>
>>> Changes in v3: None
>>> Changes in v2: None
>>>
>>> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
>>> 1 file changed, 8 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>> index d0168d55c73e..2599d902ec3a 100644
>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>> @@ -147,6 +147,14 @@ patternProperties:
>>> layout.
>>> $ref: /schemas/types.yaml#/definitions/uint32
>>>
>>> + brcm,nand-ecc-use-strap:
>>> + description:
>>> + This flag indicates the ecc strength and spare area size should
>>> + be retrieved from the SoC NAND boot strap setting instead of
>>> + nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.
>>
>> I'm still on the fence about this being overly prescriptive about the
>> operating systems behaviour. I think it would be good to say why the
>> strap values are better than those explicitly provided in DT rather than
>> just saying "these strap values should be used".
>
> I don't know if there is a point is saying why they would be better, as
> they are not. It is a -questionable- design choice. However I would
> just get rid of any mention to other properties. Just say one should
> expect the strap value to be read and applied by the system when this
> property is present.
>
Agree we don't need to say which is better as it is just a design
choice. And it is used by all BCMBCA internal ref boards and customer
designs. But if we just say strap value is read and applied, it is vague
and nobody would know what is applied. I replied this email yesterday
and suggest the followings:

This property provides a choice for retrieving ecc strength and spare
area size from the SoC NAND boot strap setting. It is commonly used by
the BCMBCA SoC board design.

Hope this make everyone happy and we can move forward.


>>> + This is commonly used by the BCMBCA SoC board design.
>>> + $ref: /schemas/types.yaml#/definitions/flag
>>> +
>>> unevaluatedProperties: false
>>>
>>> allOf:
>>> --
>>> 2.37.3
>>>
>
>
> Thanks,
> Miquèl


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2024-02-05 18:06:38

by William Zhang

[permalink] [raw]
Subject: Re: [PATCH v4 02/12] dt-bindings: mtd: brcmnand: Add WP pin connection property



On 2/5/24 05:32, Miquel Raynal wrote:
> Hi William,
>
> [email protected] wrote on Fri, 2 Feb 2024 16:28:23 -0800:
>
>> Add brcm,wp-not-connected property to have an option for disabling this
>> feature on broadband board design that does not connect WP pin.
>>
>> Signed-off-by: William Zhang <[email protected]>
>>
>> ---
>>
>> Changes in v4:
>> - Move the WP pin property to this separate patch and change it to
>> boolean type.
>>
>> Changes in v3: None
>> Changes in v2: None
>>
>> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
>> 1 file changed, 8 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> index e54ca08a798a..d0168d55c73e 100644
>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>> @@ -113,6 +113,14 @@ properties:
>> earlier versions of this core that include WP
>> type: boolean
>>
>> + brcm,wp-not-connected:
>> + description:
>> + Use this property when board design does not connect controller's
>> + NAND_WPb pin to NAND chip's WP_L pin and disable the write
>> + protection feature. By default, controller assumes the pin is
>> + connected and feature is used.
>
> I would rephrase slightly. What about:
>
> WP pin is not physically wired to the NAND chip. Write
> protection feature cannot be used.
>
That's fine with me. Will update.

>> + $ref: /schemas/types.yaml#/definitions/flag
>> +
>> patternProperties:
>> "^nand@[a-f0-9]$":
>> type: object
>
>
> Thanks,
> Miquèl


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2024-02-05 18:20:35

by William Zhang

[permalink] [raw]
Subject: Re: [PATCH v4 12/12] mtd: rawnand: brcmnand: update log level messages



On 2/5/24 05:37, Miquel Raynal wrote:
> Hi William,
>
> [email protected] wrote on Fri, 2 Feb 2024 16:28:33 -0800:
>
>> From: David Regan <[email protected]>
>>
>> Update log level messages so that more critical messages
>> can be seen.
>
> This commit log does not convince me. Warning messages are visible,
> they are in dmesg. If you want them on your console, lower your default
> console level by 1 and they will appear. I'm fine increasing the log
> level on error messages, but the justification cannot be specific to
> your own setup.
>
It is mainly for troubleshooting our customer devices. Understand we can
get the log through dmesg but when we deal with system hang issue,
console log is very important. And also sometimes customer has rebooted
the device due to error condition and we lost the dmesg.

How about we update the commit message to:
Update log level messages so that more critical messages
can be logged to console and help the troubleshooting with customer
devices.

>>
>> Signed-off-by: David Regan <[email protected]>
>> Signed-off-by: William Zhang <[email protected]>
>> Reviewed-by: William Zhang <[email protected]>
>>
>> ---
>>
>> Changes in v4:
>> - Revert the log level change for correctable ecc error
>>
>> Changes in v3: None
>> Changes in v2:
>> - Added to patch series
>>
>> drivers/mtd/nand/raw/brcmnand/brcmnand.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> index 7ce2b267676f..e50582b45182 100644
>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> @@ -1143,7 +1143,7 @@ static int bcmnand_ctrl_poll_status(struct brcmnand_host *host,
>> if ((val & mask) == expected_val)
>> return 0;
>>
>> - dev_warn(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
>> + dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
>> expected_val, val & mask);
>>
>> return -ETIMEDOUT;
>> @@ -2196,7 +2196,7 @@ static int brcmnand_read(struct mtd_info *mtd, struct nand_chip *chip,
>> return err;
>> }
>>
>> - dev_dbg(ctrl->dev, "uncorrectable error at 0x%llx\n",
>> + dev_err(ctrl->dev, "uncorrectable error at 0x%llx\n",
>> (unsigned long long)err_addr);
>> mtd->ecc_stats.failed++;
>> /* NAND layer expects zero on ECC errors */
>
>
> Thanks,
> Miquèl


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature

2024-02-05 19:01:19

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH v4 01/12] dt-bindings: mtd: brcmnand: Updates for bcmbca SoCs

On Fri, Feb 02, 2024 at 04:28:22PM -0800, William Zhang wrote:
> Update the descriptions to reflect different families of broadband SoC and
> use the general name bcmbca for ARM based SoC.
>
> Remove the requirement of interrupts property to reflect the driver
> code and only require interrupt-names when interrupts property present.
>
> Also add myself to the list of maintainers.
>
> Signed-off-by: William Zhang <[email protected]>
> Reviewed-by: David Regan <[email protected]>
>
> ---
>
> Changes in v4:
> - Split the yaml changes into three patches. This is the first one.
>
> Changes in v3:
> - Update brcm,nand-use-wp description
> - Revert the description change to BCM63168 SoC-specific NAND controller
>
> Changes in v2:
> - Revert the new compatible string nand-bcmbca
> - Drop the BCM63168 compatible fix to avoid any potential ABI
> incompatibility issue
> - Simplify the explanation for brcm,nand-use-wp
> - Keep the interrupt name requirement when interrupt number is specified
>
> .../devicetree/bindings/mtd/brcm,brcmnand.yaml | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)

Reviewed-by: Rob Herring <[email protected]>

2024-02-06 09:35:22

by Miquel Raynal

[permalink] [raw]
Subject: Re: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property

Hi William,

[email protected] wrote on Mon, 5 Feb 2024 10:05:14 -0800:

> Hi Miquel,
>
> On 2/5/24 05:26, Miquel Raynal wrote:
> > Hi,
> >
> > [email protected] wrote on Sat, 3 Feb 2024 14:49:50 +0000:
> >
> >> On Fri, Feb 02, 2024 at 04:28:24PM -0800, William Zhang wrote:
> >>> Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
> >>> board boot strap for broadband board designs because they do not specify
> >>> ecc setting in dts but rather using the strap setting.
> >>>
> >>> Signed-off-by: William Zhang <[email protected]>
> >>>
> >>> ---
> >>>
> >>> Changes in v4:
> >>> - Move ecc strap property to this separate patch and remove some
> >>> non-binding related text from the description
> >>>
> >>> Changes in v3: None
> >>> Changes in v2: None
> >>>
> >>> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
> >>> 1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >>> index d0168d55c73e..2599d902ec3a 100644
> >>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >>> @@ -147,6 +147,14 @@ patternProperties:
> >>> layout.
> >>> $ref: /schemas/types.yaml#/definitions/uint32
> >>> >>> + brcm,nand-ecc-use-strap:
> >>> + description:
> >>> + This flag indicates the ecc strength and spare area size should
> >>> + be retrieved from the SoC NAND boot strap setting instead of
> >>> + nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.
> >>
> >> I'm still on the fence about this being overly prescriptive about the
> >> operating systems behaviour. I think it would be good to say why the
> >> strap values are better than those explicitly provided in DT rather than
> >> just saying "these strap values should be used".
> >
> > I don't know if there is a point is saying why they would be better, as
> > they are not. It is a -questionable- design choice. However I would
> > just get rid of any mention to other properties. Just say one should
> > expect the strap value to be read and applied by the system when this
> > property is present.
> >
> Agree we don't need to say which is better as it is just a design choice. And it is used by all BCMBCA internal ref boards and customer designs. But if we just say strap value is read and applied, it is vague and nobody would know what is applied. I replied this email yesterday and suggest the followings:
>
> This property provides a choice for retrieving ecc strength and spare area size from
the SoC NAND boot strap setting. It is commonly used by the BCMBCA SoC board design.

What about:

This property requires the host system to get the ECC strength and step
size from the SoC NAND boot strap setting. This is a common hardware
design on BCMBCA based boards.

I would also like to constrain this more by adding an exclusive use wrt
the nand-ecc-* properties. So either you put this property, or you put
the generic nand-ecc-* properties, or you put nothing (which, again, is
by far the best solution), but in no case you can have a mix.

>
> Hope this make everyone happy and we can move forward.

I was advising to avoid mentioning too specific DT properties, but
mentioning the kind of impact it has (on the choice for the ECC
strength and size) is fine.

>
>
> >>> + This is commonly used by the BCMBCA SoC board design.
> >>> + $ref: /schemas/types.yaml#/definitions/flag
> >>> +
> >>> unevaluatedProperties: false
> >>> >>> allOf:
> >>> -- >>> 2.37.3
> >>> > >
> > Thanks,
> > Miquèl


Thanks,
Miquèl

2024-02-07 02:22:07

by William Zhang

[permalink] [raw]
Subject: Re: [PATCH v4 03/12] dt-bindings: mtd: brcmnand: Add ecc strap property



On 2/6/24 01:34, Miquel Raynal wrote:
> Hi William,
>
> [email protected] wrote on Mon, 5 Feb 2024 10:05:14 -0800:
>
>> Hi Miquel,
>>
>> On 2/5/24 05:26, Miquel Raynal wrote:
>>> Hi,
>>>
>>> [email protected] wrote on Sat, 3 Feb 2024 14:49:50 +0000:
>>>
>>>> On Fri, Feb 02, 2024 at 04:28:24PM -0800, William Zhang wrote:
>>>>> Add brcm,nand-ecc-use-strap to get ecc and spare area size settings from
>>>>> board boot strap for broadband board designs because they do not specify
>>>>> ecc setting in dts but rather using the strap setting.
>>>>>
>>>>> Signed-off-by: William Zhang <[email protected]>
>>>>>
>>>>> ---
>>>>>
>>>>> Changes in v4:
>>>>> - Move ecc strap property to this separate patch and remove some
>>>>> non-binding related text from the description
>>>>>
>>>>> Changes in v3: None
>>>>> Changes in v2: None
>>>>>
>>>>> Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml | 8 ++++++++
>>>>> 1 file changed, 8 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>>>> index d0168d55c73e..2599d902ec3a 100644
>>>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
>>>>> @@ -147,6 +147,14 @@ patternProperties:
>>>>> layout.
>>>>> $ref: /schemas/types.yaml#/definitions/uint32
>>>>> >>> + brcm,nand-ecc-use-strap:
>>>>> + description:
>>>>> + This flag indicates the ecc strength and spare area size should
>>>>> + be retrieved from the SoC NAND boot strap setting instead of
>>>>> + nand-ecc-strength and brcm,nand-oob-sector-size or auto detection.
>>>>
>>>> I'm still on the fence about this being overly prescriptive about the
>>>> operating systems behaviour. I think it would be good to say why the
>>>> strap values are better than those explicitly provided in DT rather than
>>>> just saying "these strap values should be used".
>>>
>>> I don't know if there is a point is saying why they would be better, as
>>> they are not. It is a -questionable- design choice. However I would
>>> just get rid of any mention to other properties. Just say one should
>>> expect the strap value to be read and applied by the system when this
>>> property is present.
>>>
>> Agree we don't need to say which is better as it is just a design choice. And it is used by all BCMBCA internal ref boards and customer designs. But if we just say strap value is read and applied, it is vague and nobody would know what is applied. I replied this email yesterday and suggest the followings:
>>
>> This property provides a choice for retrieving ecc strength and spare area size from
> the SoC NAND boot strap setting. It is commonly used by the BCMBCA SoC board design.
>
> What about:
>
> This property requires the host system to get the ECC strength and step
> size from the SoC NAND boot strap setting. This is a common hardware
> design on BCMBCA based boards.
>
Sounds good. Will update.

> I would also like to constrain this more by adding an exclusive use wrt
> the nand-ecc-* properties. So either you put this property, or you put
> the generic nand-ecc-* properties, or you put nothing (which, again, is
> by far the best solution), but in no case you can have a mix.
>
Right, nobody should have a mix but in case it happens, the driver code
handles that well by taking nand-ecc-* property. So not sure if the
exclusion check is really important. Anyway I will add a condition check
in the yaml on them as you requested.

>>
>> Hope this make everyone happy and we can move forward.
>
> I was advising to avoid mentioning too specific DT properties, but
> mentioning the kind of impact it has (on the choice for the ECC
> strength and size) is fine.
>
>>
>>
>>>>> + This is commonly used by the BCMBCA SoC board design.
>>>>> + $ref: /schemas/types.yaml#/definitions/flag
>>>>> +
>>>>> unevaluatedProperties: false
>>>>> >>> allOf:
>>>>> -- >>> 2.37.3
>>>>> > >
>>> Thanks,
>>> Miquèl
>
>
> Thanks,
> Miquèl


Attachments:
smime.p7s (4.11 kB)
S/MIME Cryptographic Signature