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, log level
adjustment on uncorrectable ECC error and some coding style fixes.
[1] https://lore.kernel.org/lkml/[email protected]/
Changes in v6:
- Moved style fixes to a separate patch
- Fix style issue
- Add reviewed-by tags
- Add other nand ecc properties to the exclude check list
- Update the brcm,nand-ecc-use-strap property description
- Combine the ecc step size and ecc strength into one get function
- Treat it as error condition if both brcm,nand-ecc-use-strap and nand
ecc dts properties are set
- Add intermediate steps to get the sector size bitfield
Changes in v5:
- Update the commit message
- Add reviewed-by tag
- Update the description of this new property
- Update the description for this ecc strap property
- Add check to make sure brcm,nand-ecc-use-strap and
nand-ecc-strength/brcm,nand-oob-sector-size can not be used at the
same time
Changes in v4:
- Fix the commit id in the fixes tag
- Revert the log level change for correctable ecc error
- Split the yaml changes into three patches. This is the first one
- Move the WP pin property to this separate patch and change it to
boolean type.
- Move ecc strap property to this separate patch and remove some
non-binding related text from the description
- Move the board related dts setting from SoC dtsi to board dts
- 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
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:
- Added to patch series
- Added to patch series
- 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
SoCs 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
- Remove be32_to_cpu from brcmnand_read_data_bus
- Minor cosmetic fixes
David Regan (2):
mtd: rawnand: brcmnand: exec_op helper functions return type fixes
mtd: rawnand: brcmnand: update log level messages
William Zhang (11):
mtd: rawnand: brcmnand: fix style issues
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 | 44 +++++-
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 | 148 ++++++++++++++----
drivers/mtd/nand/raw/brcmnand/brcmnand.h | 2 +
40 files changed, 650 insertions(+), 144 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
Fix various style issues.
Signed-off-by: David Regan <[email protected]>
Signed-off-by: William Zhang <[email protected]>
Reviewed-by: William Zhang <[email protected]>
---
Changes in v6:
- Added to patch series
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
drivers/mtd/nand/raw/brcmnand/brcmnand.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index b8e70fc64348..5f34e5a51d25 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -2339,7 +2339,7 @@ static int brcmnand_write_oob_raw(struct nand_chip *chip, int page)
}
static int brcmnand_exec_instr(struct brcmnand_host *host, int i,
- const struct nand_operation *op)
+ const struct nand_operation *op)
{
const struct nand_op_instr *instr = &op->instrs[i];
struct brcmnand_controller *ctrl = host->ctrl;
@@ -2353,7 +2353,7 @@ static int brcmnand_exec_instr(struct brcmnand_host *host, int i,
* (WAITRDY excepted).
*/
last_op = ((i == (op->ninstrs - 1)) && (instr->type != NAND_OP_WAITRDY_INSTR)) ||
- ((i == (op->ninstrs - 2)) && (op->instrs[i+1].type == NAND_OP_WAITRDY_INSTR));
+ ((i == (op->ninstrs - 2)) && (op->instrs[i + 1].type == NAND_OP_WAITRDY_INSTR));
switch (instr->type) {
case NAND_OP_CMD_INSTR:
@@ -2398,10 +2398,10 @@ static int brcmnand_exec_instr(struct brcmnand_host *host, int i,
static int brcmnand_op_is_status(const struct nand_operation *op)
{
- if ((op->ninstrs == 2) &&
- (op->instrs[0].type == NAND_OP_CMD_INSTR) &&
- (op->instrs[0].ctx.cmd.opcode == NAND_CMD_STATUS) &&
- (op->instrs[1].type == NAND_OP_DATA_IN_INSTR))
+ if (op->ninstrs == 2 &&
+ op->instrs[0].type == NAND_OP_CMD_INSTR &&
+ op->instrs[0].ctx.cmd.opcode == NAND_CMD_STATUS &&
+ op->instrs[1].type == NAND_OP_DATA_IN_INSTR)
return 1;
return 0;
@@ -2409,10 +2409,10 @@ static int brcmnand_op_is_status(const struct nand_operation *op)
static int brcmnand_op_is_reset(const struct nand_operation *op)
{
- if ((op->ninstrs == 2) &&
- (op->instrs[0].type == NAND_OP_CMD_INSTR) &&
- (op->instrs[0].ctx.cmd.opcode == NAND_CMD_RESET) &&
- (op->instrs[1].type == NAND_OP_WAITRDY_INSTR))
+ if (op->ninstrs == 2 &&
+ op->instrs[0].type == NAND_OP_CMD_INSTR &&
+ op->instrs[0].ctx.cmd.opcode == NAND_CMD_RESET &&
+ op->instrs[1].type == NAND_OP_WAITRDY_INSTR)
return 1;
return 0;
@@ -2440,8 +2440,7 @@ static int brcmnand_exec_op(struct nand_chip *chip,
*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
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]>
Reviewed-by: Rob Herring <[email protected]>
Reviewed-by: Florian Fainelli <[email protected]>
---
Changes in v6:
- Add reviewed-by tag
Changes in v5:
- Add reviewed-by tag
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
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 v6:
- Add other nand ecc properties to the exclude check list
- Update the brcm,nand-ecc-use-strap property description
Changes in v5:
- Update the description for this ecc strap property
- Add check to make sure brcm,nand-ecc-use-strap and
nand-ecc-strength/brcm,nand-oob-sector-size can not be used at the
same time
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
.../bindings/mtd/brcm,brcmnand.yaml | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index 6a717bcedfd3..064e840aeaa1 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -146,6 +146,15 @@ patternProperties:
layout.
$ref: /schemas/types.yaml#/definitions/uint32
+ brcm,nand-ecc-use-strap:
+ description:
+ This property requires the host system to get the ECC related
+ settings from the SoC NAND boot strap configuration instead of
+ the generic NAND ECC settings. This is a common hardware design
+ on BCMBCA based boards. This strap ECC option and generic NAND
+ ECC option can not be specified at the same time.
+ $ref: /schemas/types.yaml#/definitions/flag
+
unevaluatedProperties: false
allOf:
@@ -195,6 +204,21 @@ allOf:
required:
- interrupt-names
+ - if:
+ patternProperties:
+ "^nand@[a-f0-9]$":
+ required:
+ - brcm,nand-ecc-use-strap
+ then:
+ patternProperties:
+ "^nand@[a-f0-9]$":
+ properties:
+ nand-ecc-strength: false
+ nand-ecc-step-size: false
+ nand-ecc-maximize: false
+ nand-ecc-algo: false
+ brcm,nand-oob-sector-size: false
+
unevaluatedProperties: false
required:
--
2.37.3
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]>
Reviewed-by: Florian Fainelli <[email protected]>
---
Changes in v6:
- Add reviewed-by tags
Changes in v5:
- Update the description of this new property
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 | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
index e54ca08a798a..6a717bcedfd3 100644
--- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
+++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
@@ -113,6 +113,13 @@ properties:
earlier versions of this core that include WP
type: boolean
+ brcm,wp-not-connected:
+ description:
+ Use this property when WP pin is not physically wired to the NAND chip.
+ Write protection feature cannot be used. 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
From: David Regan <[email protected]>
Update log level messages so that more critical messages can be logged
to console and help the troubleshooting with field devices.
Signed-off-by: David Regan <[email protected]>
Signed-off-by: William Zhang <[email protected]>
Reviewed-by: William Zhang <[email protected]>
---
Changes in v6:
- Fix style issue
Changes in v5:
- Update the commit message
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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 5f34e5a51d25..f1f0de50b5f7 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1084,8 +1084,8 @@ 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",
- expected_val, val & mask);
+ dev_err(ctrl->dev, "timeout on status poll (expected %x got %x)\n",
+ expected_val, val & mask);
return -ETIMEDOUT;
}
@@ -2137,7 +2137,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
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 v6: None
Changes in v5: None
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
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. However these two
options can not be used at the same time.
Signed-off-by: William Zhang <[email protected]>
Reviewed-by: David Regan <[email protected]>
---
Changes in v6:
- Combine the ecc step size and ecc strength into one get function
- Treat it as error condition if both brcm,nand-ecc-use-strap and nand
ecc dts properties are set
- Add intermediate steps to get the sector size bitfield
Changes in v5: None
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, 77 insertions(+), 6 deletions(-)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index ef7d340475be..e8ffc283b365 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -1038,6 +1038,22 @@ 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 sector_size_bit = brcmnand_sector_1k_shift(ctrl);
+ u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
+ BRCMNAND_CS_ACC_CONTROL);
+ u32 acc_control;
+
+ if (sector_size_bit < 0)
+ return 0;
+
+ acc_control = nand_readreg(ctrl, acc_control_offs);
+
+ return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
+}
+
static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
{
struct brcmnand_controller *ctrl = host->ctrl;
@@ -1055,6 +1071,43 @@ 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 void brcmnand_get_ecc_settings(struct brcmnand_host *host, struct nand_chip *chip)
+{
+ 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;
+ 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)
+ chip->ecc.strength = ecc_level * 2;
+ else if (spare_area_size == 16 && ecc_level == 15)
+ chip->ecc.strength = 1; /* hamming */
+ else
+ chip->ecc.strength = ecc_level;
+
+ if (chip->ecc.size == 0) {
+ if (sector_size_1k < 0)
+ chip->ecc.size = 512;
+ else
+ chip->ecc.size = 512 << sector_size_1k;
+ }
+}
+
/***********************************************************************
* CS_NAND_SELECT
***********************************************************************/
@@ -2625,19 +2678,37 @@ 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;
+ bool use_strap = false;
+ char msg[128];
int ret;
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",
+ /*
+ * Either nand-ecc-xxx or brcm,nand-ecc-use-strap can be set. Error out
+ * if both exist.
+ */
+ if (chip->ecc.strength && use_strap) {
+ dev_err(ctrl->dev,
+ "nand ecc and strap ecc settings can't be set at the same time\n");
+ return -EINVAL;
+ }
+
+ if (use_strap)
+ brcmnand_get_ecc_settings(host, chip);
+
+ 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
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 v6: None
Changes in v5: None
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 e8ffc283b365..4810345d0d8a 100644
--- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
+++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
@@ -3223,6 +3223,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
From: David Regan <[email protected]>
Fix return types for exec_op reset and status 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 v6:
- Moved style fixes to a separate patch
Changes in v5: None
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 | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
index 8faca43ae1ff..b8e70fc64348 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)
{
@@ -1690,7 +1690,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);
@@ -1701,7 +1701,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;
@@ -2433,7 +2433,11 @@ 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;
}
--
2.37.3
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 v6: None
Changes in v5: None
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
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 v6:
- Fix style issue
Changes in v5: None
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 f1f0de50b5f7..ef7d340475be 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..9f171252a2ae 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
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 v6: None
Changes in v5: None
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
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 v6: None
Changes in v5: None
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
Hi William,
[email protected] wrote on Thu, 22 Feb 2024 19:47:57 -0800:
> 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. However these two
> options can not be used at the same time.
>
> Signed-off-by: William Zhang <[email protected]>
> Reviewed-by: David Regan <[email protected]>
>
FYI I did not receive patches 7, 8, 9, which makes the series numbering
very odd.
> ---
>
> Changes in v6:
> - Combine the ecc step size and ecc strength into one get function
> - Treat it as error condition if both brcm,nand-ecc-use-strap and nand
> ecc dts properties are set
> - Add intermediate steps to get the sector size bitfield
>
> Changes in v5: None
> 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, 77 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> index ef7d340475be..e8ffc283b365 100644
> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> @@ -1038,6 +1038,22 @@ 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 sector_size_bit = brcmnand_sector_1k_shift(ctrl);
> + u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
> + BRCMNAND_CS_ACC_CONTROL);
> + u32 acc_control;
> +
> + if (sector_size_bit < 0)
> + return 0;
> +
> + acc_control = nand_readreg(ctrl, acc_control_offs);
> +
> + return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
FIELD_PREP, FIELD_GET, *please*.
> +}
> +
> static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
> {
> struct brcmnand_controller *ctrl = host->ctrl;
> @@ -1055,6 +1071,43 @@ 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 void brcmnand_get_ecc_settings(struct brcmnand_host *host, struct nand_chip *chip)
> +{
> + 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;
> + 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;
ditto
> + if (sector_size_1k)
> + chip->ecc.strength = ecc_level * 2;
> + else if (spare_area_size == 16 && ecc_level == 15)
> + chip->ecc.strength = 1; /* hamming */
> + else
> + chip->ecc.strength = ecc_level;
> +
> + if (chip->ecc.size == 0) {
> + if (sector_size_1k < 0)
Should be <= 0 I guess
> + chip->ecc.size = 512;
> + else
> + chip->ecc.size = 512 << sector_size_1k;
What is this? Are you expecting sector_size_1k to be 0 or 1
and thus multiply 512 by two?
Please just use:
chip->ecc.size = SZ_1K;
> + }
> +}
> +
> /***********************************************************************
> * CS_NAND_SELECT
> ***********************************************************************/
> @@ -2625,19 +2678,37 @@ 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;
> + bool use_strap = false;
> + char msg[128];
> int ret;
>
> 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",
> + /*
> + * Either nand-ecc-xxx or brcm,nand-ecc-use-strap can be set. Error out
> + * if both exist.
> + */
Thanks for the comment but I think the error string is clear enough.
> + if (chip->ecc.strength && use_strap) {
> + dev_err(ctrl->dev,
> + "nand ecc and strap ecc settings can't be set at the same time\n");
Can we change to
"ECC strap and DT ECC configuration properties are mutually exclusive"
> + return -EINVAL;
> + }
> +
> + if (use_strap)
> + brcmnand_get_ecc_settings(host, chip);
> +
> + 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;
> }
The rest of the series looks good to me.
Thanks,
Miquèl
Hi Miquel,
On 2/23/24 01:18, Miquel Raynal wrote:
> Hi William,
>
> [email protected] wrote on Thu, 22 Feb 2024 19:47:57 -0800:
>
>> 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. However these two
>> options can not be used at the same time.
>>
>> Signed-off-by: William Zhang <[email protected]>
>> Reviewed-by: David Regan <[email protected]>
>>
>
> FYI I did not receive patches 7, 8, 9, which makes the series numbering
> very odd.
>
I was using the get maintainer script mainly and it sends to the linux
MTD list. I will add your email directly next time.
>> ---
>>
>> Changes in v6:
>> - Combine the ecc step size and ecc strength into one get function
>> - Treat it as error condition if both brcm,nand-ecc-use-strap and nand
>> ecc dts properties are set
>> - Add intermediate steps to get the sector size bitfield
>>
>> Changes in v5: None
>> 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, 77 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> index ef7d340475be..e8ffc283b365 100644
>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>> @@ -1038,6 +1038,22 @@ 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 sector_size_bit = brcmnand_sector_1k_shift(ctrl);
>> + u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
>> + BRCMNAND_CS_ACC_CONTROL);
>> + u32 acc_control;
>> +
>> + if (sector_size_bit < 0)
>> + return 0;
>> +
>> + acc_control = nand_readreg(ctrl, acc_control_offs);
>> +
>> + return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
>
> FIELD_PREP, FIELD_GET, *please*.
You probably missed my reply to your comments on the same patch in v5.
Here is the link for the post in case it lost in your email:
https://lore.kernel.org/lkml/[email protected]/T/#m1d911d2f119f3bd345c575a81b60bc2bd8c461eb
The mask is not constant here and cause build errors.
>
>> +}
>> +
>> static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
>> {
>> struct brcmnand_controller *ctrl = host->ctrl;
>> @@ -1055,6 +1071,43 @@ 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 void brcmnand_get_ecc_settings(struct brcmnand_host *host, struct nand_chip *chip)
>> +{
>> + 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;
>> + 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;
>
> ditto
>
>> + if (sector_size_1k)
>> + chip->ecc.strength = ecc_level * 2;
>> + else if (spare_area_size == 16 && ecc_level == 15)
>> + chip->ecc.strength = 1; /* hamming */
>> + else
>> + chip->ecc.strength = ecc_level;
>> +
>> + if (chip->ecc.size == 0) {
>> + if (sector_size_1k < 0)
>
> Should be <= 0 I guess
>
>> + chip->ecc.size = 512;
>> + else
>> + chip->ecc.size = 512 << sector_size_1k;
>
> What is this? Are you expecting sector_size_1k to be 0 or 1
> and thus multiply 512 by two?
>
Explained in the same post above. Sector_size_1k can be negative number
for error condition where we default to 512 step size. Otherwise 0 for
512 and 1 for 1K which the above shift takes care of.
> Please just use:
> chip->ecc.size = SZ_1K;
>
>> + }
>> +}
>> +
>> /***********************************************************************
>> * CS_NAND_SELECT
>> ***********************************************************************/
>> @@ -2625,19 +2678,37 @@ 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;
>> + bool use_strap = false;
>> + char msg[128];
>> int ret;
>>
>> 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",
>> + /*
>> + * Either nand-ecc-xxx or brcm,nand-ecc-use-strap can be set. Error out
>> + * if both exist.
>> + */
>
> Thanks for the comment but I think the error string is clear enough.
>
>> + if (chip->ecc.strength && use_strap) {
>> + dev_err(ctrl->dev,
>> + "nand ecc and strap ecc settings can't be set at the same time\n");
>
> Can we change to
> "ECC strap and DT ECC configuration properties are mutually exclusive"
>
Will do.
>> + return -EINVAL;
>> + }
>> +
>> + if (use_strap)
>> + brcmnand_get_ecc_settings(host, chip);
>> +
>> + 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;
>> }
>
> The rest of the series looks good to me.
>
> Thanks,
> Miquèl
On Thu, Feb 22, 2024 at 07:47:51PM -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]>
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
Hi William,
[email protected] wrote on Fri, 23 Feb 2024 09:25:09 -0800:
> Hi Miquel,
>
> On 2/23/24 01:18, Miquel Raynal wrote:
> > Hi William,
> >
> > [email protected] wrote on Thu, 22 Feb 2024 19:47:57 -0800:
> >
> >> 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. However these two
> >> options can not be used at the same time.
> >>
> >> Signed-off-by: William Zhang <[email protected]>
> >> Reviewed-by: David Regan <[email protected]>
> >>
> >
> > FYI I did not receive patches 7, 8, 9, which makes the series numbering
> > very odd.
> >
> I was using the get maintainer script mainly and it sends to the linux MTD list. I will add your email directly next time.
Yes, I prefer to be in Cc of the whole series, please.
> >> ---
> >>
> >> Changes in v6:
> >> - Combine the ecc step size and ecc strength into one get function
> >> - Treat it as error condition if both brcm,nand-ecc-use-strap and nand
> >> ecc dts properties are set
> >> - Add intermediate steps to get the sector size bitfield
> >>
> >> Changes in v5: None
> >> 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, 77 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> index ef7d340475be..e8ffc283b365 100644
> >> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
> >> @@ -1038,6 +1038,22 @@ 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 sector_size_bit = brcmnand_sector_1k_shift(ctrl);
> >> + u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
> >> + BRCMNAND_CS_ACC_CONTROL);
> >> + u32 acc_control;
> >> +
> >> + if (sector_size_bit < 0)
> >> + return 0;
> >> +
> >> + acc_control = nand_readreg(ctrl, acc_control_offs);
> >> +
> >> + return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
> >
> > FIELD_PREP, FIELD_GET, *please*.
> You probably missed my reply to your comments on the same patch in v5. Here is the link for the post in case it lost in your email:
> https://lore.kernel.org/lkml/[email protected]/T/#m1d911d2f119f3bd345c575a81b60bc2bd8c461eb
I didn't miss it, but the reason does not sound legitimate to me.
Please work on it, it will be so much cleaner.
> The mask is not constant here and cause build errors.
> >
> >> +}
> >> +
> >> static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
> >> {
> >> struct brcmnand_controller *ctrl = host->ctrl;
> >> @@ -1055,6 +1071,43 @@ 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 void brcmnand_get_ecc_settings(struct brcmnand_host *host, struct nand_chip *chip)
> >> +{
> >> + 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;
> >> + 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;
> >
> > ditto
> >
> >> + if (sector_size_1k)
> >> + chip->ecc.strength = ecc_level * 2;
> >> + else if (spare_area_size == 16 && ecc_level == 15)
> >> + chip->ecc.strength = 1; /* hamming */
> >> + else
> >> + chip->ecc.strength = ecc_level;
> >> +
> >> + if (chip->ecc.size == 0) {
> >> + if (sector_size_1k < 0)
> >
> > Should be <= 0 I guess
> >
> >> + chip->ecc.size = 512;
> >> + else
> >> + chip->ecc.size = 512 << sector_size_1k;
> >
> > What is this? Are you expecting sector_size_1k to be 0 or 1
> > and thus multiply 512 by two?
> >
> Explained in the same post above. Sector_size_1k can be negative number for error condition where we default to 512 step size. Otherwise 0 for 512 and 1 for 1K which the above shift takes care of.
The logic is unclear, unnatural. Please simplify. You have the
possibility to change all the driver, so please simplify and clarify
the logic.
> > Please just use:
> > chip->ecc.size = SZ_1K;
> >
Thanks,
Miquèl
On Fri, 2024-02-23 at 03:47:58 UTC, William Zhang wrote:
> 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]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:56 UTC, William Zhang wrote:
> 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]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:55 UTC, William Zhang wrote:
> 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]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:53 UTC, William Zhang wrote:
> 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]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:51 UTC, 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]>
> Reviewed-by: Conor Dooley <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:50 UTC, 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]>
> Reviewed-by: Florian Fainelli <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:49 UTC, 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]>
> Reviewed-by: Rob Herring <[email protected]>
> Reviewed-by: Florian Fainelli <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:48 UTC, William Zhang wrote:
> From: David Regan <[email protected]>
>
> Update log level messages so that more critical messages can be logged
> to console and help the troubleshooting with field devices.
>
> Signed-off-by: David Regan <[email protected]>
> Signed-off-by: William Zhang <[email protected]>
> Reviewed-by: William Zhang <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:47 UTC, William Zhang wrote:
> Fix various style issues.
>
> Signed-off-by: David Regan <[email protected]>
> Signed-off-by: William Zhang <[email protected]>
> Reviewed-by: William Zhang <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:46 UTC, William Zhang wrote:
> From: David Regan <[email protected]>
>
> Fix return types for exec_op reset and status 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]>
> Reviewed-by: Florian Fainelli <[email protected]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:54 UTC, William Zhang wrote:
> 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]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On Fri, 2024-02-23 at 03:47:52 UTC, William Zhang wrote:
> 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]>
Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Miquel
On 2/26/24 00:36, Miquel Raynal wrote:
> Hi William,
>
> [email protected] wrote on Fri, 23 Feb 2024 09:25:09 -0800:
>
>> Hi Miquel,
>>
>> On 2/23/24 01:18, Miquel Raynal wrote:
>>> Hi William,
>>>
>>> [email protected] wrote on Thu, 22 Feb 2024 19:47:57 -0800:
>>>
>>>> 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. However these two
>>>> options can not be used at the same time.
>>>>
>>>> Signed-off-by: William Zhang <[email protected]>
>>>> Reviewed-by: David Regan <[email protected]>
>>>>
>>>
>>> FYI I did not receive patches 7, 8, 9, which makes the series numbering
>>> very odd.
>>>
>> I was using the get maintainer script mainly and it sends to the linux MTD list. I will add your email directly next time.
>
> Yes, I prefer to be in Cc of the whole series, please.
>
Sure. And thanks for applying other patches. Do you want me to just
send a new single patch for the update?
>>>> ---
>>>>
>>>> Changes in v6:
>>>> - Combine the ecc step size and ecc strength into one get function
>>>> - Treat it as error condition if both brcm,nand-ecc-use-strap and nand
>>>> ecc dts properties are set
>>>> - Add intermediate steps to get the sector size bitfield
>>>>
>>>> Changes in v5: None
>>>> 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, 77 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/mtd/nand/raw/brcmnand/brcmnand.c b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>>> index ef7d340475be..e8ffc283b365 100644
>>>> --- a/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>>> +++ b/drivers/mtd/nand/raw/brcmnand/brcmnand.c
>>>> @@ -1038,6 +1038,22 @@ 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 sector_size_bit = brcmnand_sector_1k_shift(ctrl);
>>>> + u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
>>>> + BRCMNAND_CS_ACC_CONTROL);
>>>> + u32 acc_control;
>>>> +
>>>> + if (sector_size_bit < 0)
>>>> + return 0;
>>>> +
>>>> + acc_control = nand_readreg(ctrl, acc_control_offs);
>>>> +
>>>> + return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
>>>
>>> FIELD_PREP, FIELD_GET, *please*.
>> You probably missed my reply to your comments on the same patch in v5. Here is the link for the post in case it lost in your email:
>> https://lore.kernel.org/lkml/[email protected]/T/#m1d911d2f119f3bd345c575a81b60bc2bd8c461eb
>
> I didn't miss it, but the reason does not sound legitimate to me.
> Please work on it, it will be so much cleaner.
>
I understand FIELD_PREP/GET is the preferred way of linux accessing the
register fields but it requires a constant MASK value and does not apply
to our case as we have different versions of the register and have
different mask. There is way to workaround it. i.e defining the
multiple constants directly and using these macros with if/else based on
reg version. But it is not clean and since we already have helper
functions that handle and return different shift/mask value, I see this
is a perfect way for our situation and can adapt to future reg version
change easily and cleanly.
>> The mask is not constant here and cause build errors.
>>>
>>>> +}
>>>> +
>>>> static void brcmnand_set_sector_size_1k(struct brcmnand_host *host, int val)
>>>> {
>>>> struct brcmnand_controller *ctrl = host->ctrl;
>>>> @@ -1055,6 +1071,43 @@ 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 void brcmnand_get_ecc_settings(struct brcmnand_host *host, struct nand_chip *chip)
>>>> +{
>>>> + 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;
>>>> + 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;
>>>
>>> ditto
>>>
>>>> + if (sector_size_1k)
>>>> + chip->ecc.strength = ecc_level * 2;
>>>> + else if (spare_area_size == 16 && ecc_level == 15)
>>>> + chip->ecc.strength = 1; /* hamming */
>>>> + else
>>>> + chip->ecc.strength = ecc_level;
>>>> +
>>>> + if (chip->ecc.size == 0) {
>>>> + if (sector_size_1k < 0)
>>>
>>> Should be <= 0 I guess
>>>
>>>> + chip->ecc.size = 512;
>>>> + else
>>>> + chip->ecc.size = 512 << sector_size_1k;
>>>
>>> What is this? Are you expecting sector_size_1k to be 0 or 1
>>> and thus multiply 512 by two?
>>>
>> Explained in the same post above. Sector_size_1k can be negative number for error condition where we default to 512 step size. Otherwise 0 for 512 and 1 for 1K which the above shift takes care of.
>
> The logic is unclear, unnatural. Please simplify. You have the
> possibility to change all the driver, so please simplify and clarify
> the logic.
>
Will update.
>>> Please just use:
>>> chip->ecc.size = SZ_1K;
>>>
>
> Thanks,
> Miquèl
On 2/22/24 19:47, William Zhang wrote:
> 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, log level
> adjustment on uncorrectable ECC error and some coding style fixes.
>
> [1] https://lore.kernel.org/lkml/[email protected]/
Miquel, thanks for having applied the patches, we should have discussed
ahead of time whether you should take the SoC/board-level DTS changes
through your tree or mine, but it's fine either way and should not lead
to conflicts in Linus' tree.
--
Florian
Hi Florian,
[email protected] wrote on Mon, 26 Feb 2024 09:36:02 -0800:
> On 2/22/24 19:47, William Zhang wrote:
> > 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, log level
> > adjustment on uncorrectable ECC error and some coding style fixes.
> >
> > [1] https://lore.kernel.org/lkml/[email protected]/
>
> Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.
I'm sorry for not thinking about this ahead of time, I was also not
Cced on the other patches, I noticed it (told Willliam) and just forgot
about this when I applied the series.
It is currently living in -next so if there is any problem I can still
act.
However for this kind of change I usually apply the bindings and .c
changes independently from the DT patches. I believe there is no
problem having one or the other being merged first, or do I overlook
something?
Cheers,
Miquèl
Hi William,
[email protected] wrote on Mon, 26 Feb 2024 12:05:18 -0800:
> On 2/26/24 00:36, Miquel Raynal wrote:
> > Hi William,
> >
> > [email protected] wrote on Fri, 23 Feb 2024 09:25:09 -0800:
> >
> >> Hi Miquel,
> >>
> >> On 2/23/24 01:18, Miquel Raynal wrote:
> >>> Hi William,
> >>>
> >>> [email protected] wrote on Thu, 22 Feb 2024 19:47:57 -0800:
> >>> >>>> 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. However these two
> >>>> options can not be used at the same time.
> >>>>
> >>>> Signed-off-by: William Zhang <[email protected]>
> >>>> Reviewed-by: David Regan <[email protected]>
> >>>> >>>
> >>> FYI I did not receive patches 7, 8, 9, which makes the series numbering
> >>> very odd.
> >>> >> I was using the get maintainer script mainly and it sends to the linux MTD list. I will add your email directly next time.
> >
> > Yes, I prefer to be in Cc of the whole series, please.
> >
> Sure. And thanks for applying other patches. Do you want me to just send a new single patch for the update?
Yes just the missing patch.
> >>>> >> +static int brcmnand_get_sector_size_1k(struct brcmnand_host *host)
> >>>> +{
> >>>> + struct brcmnand_controller *ctrl = host->ctrl;
> >>>> + int sector_size_bit = brcmnand_sector_1k_shift(ctrl);
> >>>> + u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
> >>>> + BRCMNAND_CS_ACC_CONTROL);
> >>>> + u32 acc_control;
> >>>> +
> >>>> + if (sector_size_bit < 0)
> >>>> + return 0;
> >>>> +
> >>>> + acc_control = nand_readreg(ctrl, acc_control_offs);
> >>>> +
> >>>> + return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
> >>>
> >>> FIELD_PREP, FIELD_GET, *please*.
> >> You probably missed my reply to your comments on the same patch in v5. Here is the link for the post in case it lost in your email:
> >> https://lore.kernel.org/lkml/[email protected]/T/#m1d911d2f119f3bd345c575a81b60bc2bd8c461eb
> >
> > I didn't miss it, but the reason does not sound legitimate to me.
> > Please work on it, it will be so much cleaner.
> >
> I understand FIELD_PREP/GET is the preferred way of linux accessing the register fields but it requires a constant MASK value and does not apply to our case as we have different versions of the register and have different mask. There is way to workaround it. i.e defining the multiple constants directly and using these macros with if/else based on reg version. But it is not clean and since we already have helper functions that handle and return different shift/mask value, I see this is a perfect way for our situation and can adapt to future reg version change easily and cleanly.
>
> >> The mask is not constant here and cause build errors.
Which errors?
+ acc_control = nand_readreg(ctrl, acc_control_offs);
+ return FIELD_GET(BIT(sector_size_bit), acc_control);
Does not return any error here.
Thanks,
Miquèl
Hi Miquel,
On 2/29/24 01:11, Miquel Raynal wrote:
> Hi Florian,
>
> [email protected] wrote on Mon, 26 Feb 2024 09:36:02 -0800:
>
>> On 2/22/24 19:47, William Zhang wrote:
>>> 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, log level
>>> adjustment on uncorrectable ECC error and some coding style fixes.
>>>
>>> [1] https://lore.kernel.org/lkml/[email protected]/
>>
>> Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.
>
> I'm sorry for not thinking about this ahead of time, I was also not
> Cced on the other patches, I noticed it (told Willliam) and just forgot
> about this when I applied the series.
Not a problem.
>
> It is currently living in -next so if there is any problem I can still
> act.
>
> However for this kind of change I usually apply the bindings and .c
> changes independently from the DT patches. I believe there is no
> problem having one or the other being merged first, or do I overlook
> something?
That is totally fine my concern was more with you also applying the DTS
changes which could easily conflict with changes queued up in my ARM SoC
tree, and also did not have my Signed-off-by/Acked-by tag on them (I was
waiting on the bindings patch to be Acked-by before giving my own).
Anyway, let's not make this more complicated than it needs to be, and
thanks for working with William on these changes!
--
Florian
Hi Minquel,
On 2/29/24 02:31, Miquel Raynal wrote:
> Hi William,
>
> [email protected] wrote on Mon, 26 Feb 2024 12:05:18 -0800:
>
>> On 2/26/24 00:36, Miquel Raynal wrote:
>>> Hi William,
>>>
>>> [email protected] wrote on Fri, 23 Feb 2024 09:25:09 -0800:
>>>
>>>> Hi Miquel,
>>>>
>>>> On 2/23/24 01:18, Miquel Raynal wrote:
>>>>> Hi William,
>>>>>
>>>>> [email protected] wrote on Thu, 22 Feb 2024 19:47:57 -0800:
>>>>> >>>> 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. However these two
>>>>>> options can not be used at the same time.
>>>>>>
>>>>>> Signed-off-by: William Zhang <[email protected]>
>>>>>> Reviewed-by: David Regan <[email protected]>
>>>>>> >>>
>>>>> FYI I did not receive patches 7, 8, 9, which makes the series numbering
>>>>> very odd.
>>>>> >> I was using the get maintainer script mainly and it sends to the linux MTD list. I will add your email directly next time.
>>>
>>> Yes, I prefer to be in Cc of the whole series, please.
>>>
>> Sure. And thanks for applying other patches. Do you want me to just send a new single patch for the update?
>
> Yes just the missing patch.
>
v7 of this patch was sent early
>>>>>> >> +static int brcmnand_get_sector_size_1k(struct brcmnand_host *host)
>>>>>> +{
>>>>>> + struct brcmnand_controller *ctrl = host->ctrl;
>>>>>> + int sector_size_bit = brcmnand_sector_1k_shift(ctrl);
>>>>>> + u16 acc_control_offs = brcmnand_cs_offset(ctrl, host->cs,
>>>>>> + BRCMNAND_CS_ACC_CONTROL);
>>>>>> + u32 acc_control;
>>>>>> +
>>>>>> + if (sector_size_bit < 0)
>>>>>> + return 0;
>>>>>> +
>>>>>> + acc_control = nand_readreg(ctrl, acc_control_offs);
>>>>>> +
>>>>>> + return (acc_control & BIT(sector_size_bit)) >> sector_size_bit;
>>>>>
>>>>> FIELD_PREP, FIELD_GET, *please*.
>>>> You probably missed my reply to your comments on the same patch in v5. Here is the link for the post in case it lost in your email:
>>>> https://lore.kernel.org/lkml/[email protected]/T/#m1d911d2f119f3bd345c575a81b60bc2bd8c461eb
>>>
>>> I didn't miss it, but the reason does not sound legitimate to me.
>>> Please work on it, it will be so much cleaner.
>>>
>> I understand FIELD_PREP/GET is the preferred way of linux accessing the register fields but it requires a constant MASK value and does not apply to our case as we have different versions of the register and have different mask. There is way to workaround it. i.e defining the multiple constants directly and using these macros with if/else based on reg version. But it is not clean and since we already have helper functions that handle and return different shift/mask value, I see this is a perfect way for our situation and can adapt to future reg version change easily and cleanly.
>>
>>>> The mask is not constant here and cause build errors.
>
> Which errors?
>
If I use the code below In function brcmnand_get_sector_size_1k, inlined
from brcmnand_get_ecc_settings at
drivers/mtd/nand/raw/brcmnand/brcmnand.c:1089:24,
inlined from brcmnand_setup_dev at
drivers/mtd/nand/raw/brcmnand/brcmnand.c:2701:3:
/./include/linux/compiler_types.h:442:38: error: call to
compiletime_assert_254 declared with attribute error: FIELD_GET: mask is
not constant
> + acc_control = nand_readreg(ctrl, acc_control_offs);
> + return FIELD_GET(BIT(sector_size_bit), acc_control);
>
> Does not return any error here.
>
Right, this function does not return error. brcmnand_sector_1k_shift
does. I didn't make that clear enough. This function is updated and
logic that calls this function is simplified in v7 based on your feedbacks.
> Thanks,
> Miquèl
Hi Florian,
[email protected] wrote on Thu, 29 Feb 2024 10:11:01 +0100:
> Hi Florian,
>
> [email protected] wrote on Mon, 26 Feb 2024 09:36:02 -0800:
>
> > On 2/22/24 19:47, William Zhang wrote:
> > > 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, log level
> > > adjustment on uncorrectable ECC error and some coding style fixes.
> > >
> > > [1] https://lore.kernel.org/lkml/[email protected]/
> >
> > Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.
>
> I'm sorry for not thinking about this ahead of time, I was also not
> Cced on the other patches, I noticed it (told Willliam) and just forgot
> about this when I applied the series.
>
> It is currently living in -next so if there is any problem I can still
> act.
>
> However for this kind of change I usually apply the bindings and .c
> changes independently from the DT patches. I believe there is no
> problem having one or the other being merged first, or do I overlook
> something?
What the heck /o\ I just understand now my mistake, I am very truly
sorry for that...
You were telling me I should sync with you before taking DT changes,
and I was so convinced I _did_not_ take the DT, when I looked at the
branch I did not understand your point. But I am totally sorry I
actually did take the DTs by mistake and I truly did not notice it.
Confirmation bias I suppose. My very sincere apologies.
As mentioned previously, I was not CC'ed on the DT patches, but I
believe the linux-mtd list was, so the patches didn't appear in my
inbox, and once I was happy with the binding/driver changes I applied
it all without noticing the DT changes had sneaked in.
I'm finally preparing the PR for Linus and I see it now...
I believe the SoC tree is closed now so it's up to you what I should do
with them. Let me know if you want me to keep them in my tree and
forward them to Linus or if I should drop them and you'll take them for
the next cycle. Also, if I keep them, shall I add some tag of yours on
these 3 patches? For the record I did not review them.
Thanks and again, I'm confused. I never apply DT patches like that,
your initial remark was more than legitimate.
Cheers,
Miquèl
On 3/14/24 15:04, Miquel Raynal wrote:
> Hi Florian,
>
> [email protected] wrote on Thu, 29 Feb 2024 10:11:01 +0100:
>
>> Hi Florian,
>>
>> [email protected] wrote on Mon, 26 Feb 2024 09:36:02 -0800:
>>
>>> On 2/22/24 19:47, William Zhang wrote:
>>>> 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, log level
>>>> adjustment on uncorrectable ECC error and some coding style fixes.
>>>>
>>>> [1] https://lore.kernel.org/lkml/[email protected]/
>>>
>>> Miquel, thanks for having applied the patches, we should have discussed ahead of time whether you should take the SoC/board-level DTS changes through your tree or mine, but it's fine either way and should not lead to conflicts in Linus' tree.
>>
>> I'm sorry for not thinking about this ahead of time, I was also not
>> Cced on the other patches, I noticed it (told Willliam) and just forgot
>> about this when I applied the series.
>>
>> It is currently living in -next so if there is any problem I can still
>> act.
>>
>> However for this kind of change I usually apply the bindings and .c
>> changes independently from the DT patches. I believe there is no
>> problem having one or the other being merged first, or do I overlook
>> something?
>
> What the heck /o\ I just understand now my mistake, I am very truly
> sorry for that...
>
> You were telling me I should sync with you before taking DT changes,
> and I was so convinced I _did_not_ take the DT, when I looked at the
> branch I did not understand your point. But I am totally sorry I
> actually did take the DTs by mistake and I truly did not notice it.
> Confirmation bias I suppose. My very sincere apologies.
>
> As mentioned previously, I was not CC'ed on the DT patches, but I
> believe the linux-mtd list was, so the patches didn't appear in my
> inbox, and once I was happy with the binding/driver changes I applied
> it all without noticing the DT changes had sneaked in.
>
> I'm finally preparing the PR for Linus and I see it now...
>
> I believe the SoC tree is closed now so it's up to you what I should do
> with them. Let me know if you want me to keep them in my tree and
> forward them to Linus or if I should drop them and you'll take them for
> the next cycle. Also, if I keep them, shall I add some tag of yours on
> these 3 patches? For the record I did not review them.
Yes please add my:
Acked-by: Florian Fainelli <[email protected]>
tag, and it's fine I don't expect that we will get conflicts for those
files.
>
> Thanks and again, I'm confused. I never apply DT patches like that,
> your initial remark was more than legitimate.
Not a problem!
--
Florian
> >> I'm sorry for not thinking about this ahead of time, I was also not
> >> Cced on the other patches, I noticed it (told Willliam) and just forgot
> >> about this when I applied the series.
> >>
> >> It is currently living in -next so if there is any problem I can still
> >> act.
> >>
> >> However for this kind of change I usually apply the bindings and .c
> >> changes independently from the DT patches. I believe there is no
> >> problem having one or the other being merged first, or do I overlook
> >> something?
> >
> > What the heck /o\ I just understand now my mistake, I am very truly
> > sorry for that...
> >
> > You were telling me I should sync with you before taking DT changes,
> > and I was so convinced I _did_not_ take the DT, when I looked at the
> > branch I did not understand your point. But I am totally sorry I
> > actually did take the DTs by mistake and I truly did not notice it.
> > Confirmation bias I suppose. My very sincere apologies.
> >
> > As mentioned previously, I was not CC'ed on the DT patches, but I
> > believe the linux-mtd list was, so the patches didn't appear in my
> > inbox, and once I was happy with the binding/driver changes I applied
> > it all without noticing the DT changes had sneaked in.
> >
> > I'm finally preparing the PR for Linus and I see it now...
> >
> > I believe the SoC tree is closed now so it's up to you what I should do
> > with them. Let me know if you want me to keep them in my tree and
> > forward them to Linus or if I should drop them and you'll take them for
> > the next cycle. Also, if I keep them, shall I add some tag of yours on
> > these 3 patches? For the record I did not review them.
>
> Yes please add my:
>
> Acked-by: Florian Fainelli <[email protected]>
>
> tag, and it's fine I don't expect that we will get conflicts for those files.
>
> >
> > Thanks and again, I'm confused. I never apply DT patches like that,
> > your initial remark was more than legitimate.
>
> Not a problem!
Thanks :-)
Miquèl