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