| Message ID | 20230606231252.94838-6-william.zhang@broadcom.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Miquel Raynal |
| Headers | show |
| Series | mtd: rawnand: brcmnand: driver and doc updates | expand |
Hi William, william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: > Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband > product. The old compatible string is still kept in the driver so old > dtb can still work. > > Add brcm,nand-use-wp property to have an option for disabling this > feature on broadband board design that does not use write protection. > Add brcm,nand-ecc-use-strap to get ecc setting from board strap for > broadband board designs because they do not specify ecc setting in dts > but rather using the strap setting. > > Remove the requirement of interrupts and interrupt-names properties to > reflect the driver code. > > This patch also includes a few minor fixes to the BCM63xx compatibles > and add myself to the list of maintainers. > > Signed-off-by: William Zhang <william.zhang@broadcom.com> > --- > > .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ > 1 file changed, 43 insertions(+), 21 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > index 1571024aa119..1fe1c166a9db 100644 > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller > maintainers: > - Brian Norris <computersforpeace@gmail.com> > - Kamal Dasu <kdasu.kdev@gmail.com> > + - William Zhang <william.zhang@broadcom.com> > > description: | > The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND > @@ -18,9 +19,10 @@ description: | > supports basic PROGRAM and READ functions, among other features. > > This controller was originally designed for STB SoCs (BCM7xxx) but is now > - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and > - iProc/Cygnus. Its history includes several similar (but not fully register > - compatible) versions. > + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based > + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. > + Its history includes several similar (but not fully register compatible) > + versions. > > -- Additional SoC-specific NAND controller properties -- > > @@ -53,9 +55,9 @@ properties: > - brcm,brcmnand-v7.2 > - brcm,brcmnand-v7.3 > - const: brcm,brcmnand > - - description: BCM63138 SoC-specific NAND controller > + - description: BCMBCA SoC-specific NAND controller > items: > - - const: brcm,nand-bcm63138 > + - const: brcm,nand-bcmbca > - enum: > - brcm,brcmnand-v7.0 > - brcm,brcmnand-v7.1 > @@ -65,11 +67,15 @@ properties: > - const: brcm,nand-iproc > - const: brcm,brcmnand-v6.1 > - const: brcm,brcmnand > - - description: BCM63168 SoC-specific NAND controller > + - description: BCM63xx SoC-specific NAND controller > items: > - - const: brcm,nand-bcm63168 > - - const: brcm,nand-bcm6368 > - - const: brcm,brcmnand-v4.0 > + - enum: > + - brcm,nand-bcm63168 > + - brcm,nand-bcm6368 > + - enum: > + - brcm,brcmnand-v2.1 > + - brcm,brcmnand-v2.2 > + - brcm,brcmnand-v4.0 > - const: brcm,brcmnand > > reg: > @@ -111,6 +117,19 @@ properties: > earlier versions of this core that include WP > type: boolean > > + brcm,nand-use-wp: > + description: > + Use this integer to indicate if board design uses > + controller's write protection feature and connects its > + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to > + use this feature when this property does not exist. > + Set to 0 if WP pins are not connected and feature is not > + used. Set to 1 if WP pins are connected and feature is used. > + Set to 2 if WP pins are connected but disable this feature > + through driver that sets controller to output high on NAND_WPb. > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [0, 1, 2] Perhaps strings would be welcome. I'll let binding maintainers say what they think of it. > + > patternProperties: > "^nand@[a-f0-9]$": > type: object > @@ -136,13 +155,23 @@ patternProperties: > layout. > $ref: /schemas/types.yaml#/definitions/uint32 > > + brcm,nand-ecc-use-strap: > + description: > + This flag is used by the driver to get the ecc strength and > + spare area size from the SoC NAND boot strap setting. This > + is commonly used by the BCMBCA SoC board design. If ecc > + strength and spare area size are set by nand-ecc-strength > + and brcm,nand-oob-sector-size in the dts, these settings > + have precedence and override this flag. > + $ref: /schemas/types.yaml#/definitions/flag How in practice do you access the strap value? Don't you need a phandle over a specific area in the SoC? > + > allOf: > - $ref: nand-controller.yaml# > - if: > properties: > compatible: > contains: > - const: brcm,nand-bcm63138 > + const: brcm,nand-bcmbca > then: > properties: > reg-names: > @@ -153,7 +182,9 @@ allOf: > properties: > compatible: > contains: > - const: brcm,nand-bcm6368 > + enum: > + - brcm,nand-bcm63168 > + - brcm,nand-bcm6368 > then: > properties: > reg-names: > @@ -173,20 +204,12 @@ allOf: > - const: nand > - const: iproc-idm > - const: iproc-ext > - - if: > - properties: > - interrupts: > - minItems: 2 > - then: > - required: > - - interrupt-names Why do you remove this? Removing "interrupts" from the required properties is fine, but constraining the interrupts property when it is relevant is still expected. > > unevaluatedProperties: false > > required: > - reg > - reg-names > - - interrupts This should be done in a separate patch. > > examples: > - | > @@ -215,8 +238,7 @@ examples: > }; > - | > nand-controller@10000200 { > - compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368", > - "brcm,brcmnand-v4.0", "brcm,brcmnand"; > + compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand"; > reg = <0x10000200 0x180>, > <0x100000b0 0x10>, > <0x10000600 0x200>; Thanks, Miquèl
Hi Miquel, On 06/07/2023 01:14 AM, Miquel Raynal wrote: > Hi William, > > william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: > >> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband >> product. The old compatible string is still kept in the driver so old >> dtb can still work. >> >> Add brcm,nand-use-wp property to have an option for disabling this >> feature on broadband board design that does not use write protection. >> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for >> broadband board designs because they do not specify ecc setting in dts >> but rather using the strap setting. >> >> Remove the requirement of interrupts and interrupt-names properties to >> reflect the driver code. >> >> This patch also includes a few minor fixes to the BCM63xx compatibles >> and add myself to the list of maintainers. >> >> Signed-off-by: William Zhang <william.zhang@broadcom.com> >> --- >> >> .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ >> 1 file changed, 43 insertions(+), 21 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >> index 1571024aa119..1fe1c166a9db 100644 >> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller >> maintainers: >> - Brian Norris <computersforpeace@gmail.com> >> - Kamal Dasu <kdasu.kdev@gmail.com> >> + - William Zhang <william.zhang@broadcom.com> >> >> description: | >> The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND >> @@ -18,9 +19,10 @@ description: | >> supports basic PROGRAM and READ functions, among other features. >> >> This controller was originally designed for STB SoCs (BCM7xxx) but is now >> - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and >> - iProc/Cygnus. Its history includes several similar (but not fully register >> - compatible) versions. >> + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based >> + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. >> + Its history includes several similar (but not fully register compatible) >> + versions. >> >> -- Additional SoC-specific NAND controller properties -- >> >> @@ -53,9 +55,9 @@ properties: >> - brcm,brcmnand-v7.2 >> - brcm,brcmnand-v7.3 >> - const: brcm,brcmnand >> - - description: BCM63138 SoC-specific NAND controller >> + - description: BCMBCA SoC-specific NAND controller >> items: >> - - const: brcm,nand-bcm63138 >> + - const: brcm,nand-bcmbca >> - enum: >> - brcm,brcmnand-v7.0 >> - brcm,brcmnand-v7.1 >> @@ -65,11 +67,15 @@ properties: >> - const: brcm,nand-iproc >> - const: brcm,brcmnand-v6.1 >> - const: brcm,brcmnand >> - - description: BCM63168 SoC-specific NAND controller >> + - description: BCM63xx SoC-specific NAND controller >> items: >> - - const: brcm,nand-bcm63168 >> - - const: brcm,nand-bcm6368 >> - - const: brcm,brcmnand-v4.0 >> + - enum: >> + - brcm,nand-bcm63168 >> + - brcm,nand-bcm6368 >> + - enum: >> + - brcm,brcmnand-v2.1 >> + - brcm,brcmnand-v2.2 >> + - brcm,brcmnand-v4.0 >> - const: brcm,brcmnand >> >> reg: >> @@ -111,6 +117,19 @@ properties: >> earlier versions of this core that include WP >> type: boolean >> >> + brcm,nand-use-wp: >> + description: >> + Use this integer to indicate if board design uses >> + controller's write protection feature and connects its >> + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to >> + use this feature when this property does not exist. >> + Set to 0 if WP pins are not connected and feature is not >> + used. Set to 1 if WP pins are connected and feature is used. >> + Set to 2 if WP pins are connected but disable this feature >> + through driver that sets controller to output high on NAND_WPb. >> + $ref: /schemas/types.yaml#/definitions/uint32 >> + enum: [0, 1, 2] > > Perhaps strings would be welcome. I'll let binding maintainers say what > they think of it. > Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2. >> + >> patternProperties: >> "^nand@[a-f0-9]$": >> type: object >> @@ -136,13 +155,23 @@ patternProperties: >> layout. >> $ref: /schemas/types.yaml#/definitions/uint32 >> >> + brcm,nand-ecc-use-strap: >> + description: >> + This flag is used by the driver to get the ecc strength and >> + spare area size from the SoC NAND boot strap setting. This >> + is commonly used by the BCMBCA SoC board design. If ecc >> + strength and spare area size are set by nand-ecc-strength >> + and brcm,nand-oob-sector-size in the dts, these settings >> + have precedence and override this flag. >> + $ref: /schemas/types.yaml#/definitions/flag > > How in practice do you access the strap value? Don't you need a phandle > over a specific area in the SoC? > The strap value is latched and stored in the NAND controller register so there is no extra phandle needed. >> + >> allOf: >> - $ref: nand-controller.yaml# >> - if: >> properties: >> compatible: >> contains: >> - const: brcm,nand-bcm63138 >> + const: brcm,nand-bcmbca >> then: >> properties: >> reg-names: >> @@ -153,7 +182,9 @@ allOf: >> properties: >> compatible: >> contains: >> - const: brcm,nand-bcm6368 >> + enum: >> + - brcm,nand-bcm63168 >> + - brcm,nand-bcm6368 >> then: >> properties: >> reg-names: >> @@ -173,20 +204,12 @@ allOf: >> - const: nand >> - const: iproc-idm >> - const: iproc-ext >> - - if: >> - properties: >> - interrupts: >> - minItems: 2 >> - then: >> - required: >> - - interrupt-names > > Why do you remove this? Removing "interrupts" from the required > properties is fine, but constraining the interrupts property when it is > relevant is still expected. > There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead. >> >> unevaluatedProperties: false >> >> required: >> - reg >> - reg-names >> - - interrupts > > This should be done in a separate patch. > I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name. >> >> examples: >> - | >> @@ -215,8 +238,7 @@ examples: >> }; >> - | >> nand-controller@10000200 { >> - compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368", >> - "brcm,brcmnand-v4.0", "brcm,brcmnand"; >> + compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand"; >> reg = <0x10000200 0x180>, >> <0x100000b0 0x10>, >> <0x10000600 0x200>; > > > Thanks, > Miquèl >
Hi William, william.zhang@broadcom.com wrote on Wed, 7 Jun 2023 13:01:56 -0700: > Hi Miquel, > > On 06/07/2023 01:14 AM, Miquel Raynal wrote: > > Hi William, > > > > william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: > > > >> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband > >> product. The old compatible string is still kept in the driver so old > >> dtb can still work. > >> > >> Add brcm,nand-use-wp property to have an option for disabling this > >> feature on broadband board design that does not use write protection. > >> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for > >> broadband board designs because they do not specify ecc setting in dts > >> but rather using the strap setting. > >> > >> Remove the requirement of interrupts and interrupt-names properties to > >> reflect the driver code. > >> > >> This patch also includes a few minor fixes to the BCM63xx compatibles > >> and add myself to the list of maintainers. > >> > >> Signed-off-by: William Zhang <william.zhang@broadcom.com> > >> --- > >> > >> .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ > >> 1 file changed, 43 insertions(+), 21 deletions(-) > >> > >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > >> index 1571024aa119..1fe1c166a9db 100644 > >> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > >> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > >> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller > >> maintainers: > >> - Brian Norris <computersforpeace@gmail.com> > >> - Kamal Dasu <kdasu.kdev@gmail.com> > >> + - William Zhang <william.zhang@broadcom.com> > >> >> description: | > >> The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND > >> @@ -18,9 +19,10 @@ description: | > >> supports basic PROGRAM and READ functions, among other features. > >> >> This controller was originally designed for STB SoCs (BCM7xxx) but is now > >> - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and > >> - iProc/Cygnus. Its history includes several similar (but not fully register > >> - compatible) versions. > >> + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based > >> + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. > >> + Its history includes several similar (but not fully register compatible) > >> + versions. > >> >> -- Additional SoC-specific NAND controller properties -- > >> >> @@ -53,9 +55,9 @@ properties: > >> - brcm,brcmnand-v7.2 > >> - brcm,brcmnand-v7.3 > >> - const: brcm,brcmnand > >> - - description: BCM63138 SoC-specific NAND controller > >> + - description: BCMBCA SoC-specific NAND controller > >> items: > >> - - const: brcm,nand-bcm63138 > >> + - const: brcm,nand-bcmbca > >> - enum: > >> - brcm,brcmnand-v7.0 > >> - brcm,brcmnand-v7.1 > >> @@ -65,11 +67,15 @@ properties: > >> - const: brcm,nand-iproc > >> - const: brcm,brcmnand-v6.1 > >> - const: brcm,brcmnand > >> - - description: BCM63168 SoC-specific NAND controller > >> + - description: BCM63xx SoC-specific NAND controller > >> items: > >> - - const: brcm,nand-bcm63168 > >> - - const: brcm,nand-bcm6368 > >> - - const: brcm,brcmnand-v4.0 > >> + - enum: > >> + - brcm,nand-bcm63168 > >> + - brcm,nand-bcm6368 > >> + - enum: > >> + - brcm,brcmnand-v2.1 > >> + - brcm,brcmnand-v2.2 > >> + - brcm,brcmnand-v4.0 > >> - const: brcm,brcmnand > >> >> reg: > >> @@ -111,6 +117,19 @@ properties: > >> earlier versions of this core that include WP > >> type: boolean > >> >> + brcm,nand-use-wp: > >> + description: > >> + Use this integer to indicate if board design uses > >> + controller's write protection feature and connects its > >> + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to > >> + use this feature when this property does not exist. > >> + Set to 0 if WP pins are not connected and feature is not > >> + used. Set to 1 if WP pins are connected and feature is used. > >> + Set to 2 if WP pins are connected but disable this feature > >> + through driver that sets controller to output high on NAND_WPb. > >> + $ref: /schemas/types.yaml#/definitions/uint32 > >> + enum: [0, 1, 2] > > > > Perhaps strings would be welcome. I'll let binding maintainers say what > > they think of it. > > > Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2. > > >> + > >> patternProperties: > >> "^nand@[a-f0-9]$": > >> type: object > >> @@ -136,13 +155,23 @@ patternProperties: > >> layout. > >> $ref: /schemas/types.yaml#/definitions/uint32 > >> >> + brcm,nand-ecc-use-strap: > >> + description: > >> + This flag is used by the driver to get the ecc strength and > >> + spare area size from the SoC NAND boot strap setting. This > >> + is commonly used by the BCMBCA SoC board design. If ecc > >> + strength and spare area size are set by nand-ecc-strength > >> + and brcm,nand-oob-sector-size in the dts, these settings > >> + have precedence and override this flag. > >> + $ref: /schemas/types.yaml#/definitions/flag > > > > How in practice do you access the strap value? Don't you need a phandle > > over a specific area in the SoC? > > > The strap value is latched and stored in the NAND controller register so there is no extra phandle needed. Ok. > > >> + > >> allOf: > >> - $ref: nand-controller.yaml# > >> - if: > >> properties: > >> compatible: > >> contains: > >> - const: brcm,nand-bcm63138 > >> + const: brcm,nand-bcmbca > >> then: > >> properties: > >> reg-names: > >> @@ -153,7 +182,9 @@ allOf: > >> properties: > >> compatible: > >> contains: > >> - const: brcm,nand-bcm6368 > >> + enum: > >> + - brcm,nand-bcm63168 > >> + - brcm,nand-bcm6368 > >> then: > >> properties: > >> reg-names: > >> @@ -173,20 +204,12 @@ allOf: > >> - const: nand > >> - const: iproc-idm > >> - const: iproc-ext > >> - - if: > >> - properties: > >> - interrupts: > >> - minItems: 2 > >> - then: > >> - required: > >> - - interrupt-names > > > > Why do you remove this? Removing "interrupts" from the required > > properties is fine, but constraining the interrupts property when it is > > relevant is still expected. > > > There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead. It does not matter in this case how the driver uses the interrupts. If names have been provided once in the bindings, you could expect another project using the same bindings to use the interrupt names instead of the order. So you must keep the names. Just don't mark them required it they are. > > >> >> unevaluatedProperties: false > >> >> required: > >> - reg > >> - reg-names > >> - - interrupts > > > > This should be done in a separate patch. > > > I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name. The fact that the driver does not use the interrupts does not mean they should not be described. > > >> >> examples: > >> - | > >> @@ -215,8 +238,7 @@ examples: > >> }; > >> - | > >> nand-controller@10000200 { > >> - compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368", > >> - "brcm,brcmnand-v4.0", "brcm,brcmnand"; > >> + compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand"; > >> reg = <0x10000200 0x180>, > >> <0x100000b0 0x10>, > >> <0x10000600 0x200>; > > > > > > Thanks, > > Miquèl > > Thanks, Miquèl
Hi Miquel, On 06/09/2023 01:58 AM, Miquel Raynal wrote: > Hi William, > > william.zhang@broadcom.com wrote on Wed, 7 Jun 2023 13:01:56 -0700: > >> Hi Miquel, >> >> On 06/07/2023 01:14 AM, Miquel Raynal wrote: >>> Hi William, >>> >>> william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: >>> >>>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband >>>> product. The old compatible string is still kept in the driver so old >>>> dtb can still work. >>>> >>>> Add brcm,nand-use-wp property to have an option for disabling this >>>> feature on broadband board design that does not use write protection. >>>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for >>>> broadband board designs because they do not specify ecc setting in dts >>>> but rather using the strap setting. >>>> >>>> Remove the requirement of interrupts and interrupt-names properties to >>>> reflect the driver code. >>>> >>>> This patch also includes a few minor fixes to the BCM63xx compatibles >>>> and add myself to the list of maintainers. >>>> >>>> Signed-off-by: William Zhang <william.zhang@broadcom.com> >>>> --- >>>> >>>> .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ >>>> 1 file changed, 43 insertions(+), 21 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >>>> index 1571024aa119..1fe1c166a9db 100644 >>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >>>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller >>>> maintainers: >>>> - Brian Norris <computersforpeace@gmail.com> >>>> - Kamal Dasu <kdasu.kdev@gmail.com> >>>> + - William Zhang <william.zhang@broadcom.com> >>>> >> description: | >>>> The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND >>>> @@ -18,9 +19,10 @@ description: | >>>> supports basic PROGRAM and READ functions, among other features. >>>> >> This controller was originally designed for STB SoCs (BCM7xxx) but is now >>>> - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and >>>> - iProc/Cygnus. Its history includes several similar (but not fully register >>>> - compatible) versions. >>>> + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based >>>> + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. >>>> + Its history includes several similar (but not fully register compatible) >>>> + versions. >>>> >> -- Additional SoC-specific NAND controller properties -- >>>> >> @@ -53,9 +55,9 @@ properties: >>>> - brcm,brcmnand-v7.2 >>>> - brcm,brcmnand-v7.3 >>>> - const: brcm,brcmnand >>>> - - description: BCM63138 SoC-specific NAND controller >>>> + - description: BCMBCA SoC-specific NAND controller >>>> items: >>>> - - const: brcm,nand-bcm63138 >>>> + - const: brcm,nand-bcmbca >>>> - enum: >>>> - brcm,brcmnand-v7.0 >>>> - brcm,brcmnand-v7.1 >>>> @@ -65,11 +67,15 @@ properties: >>>> - const: brcm,nand-iproc >>>> - const: brcm,brcmnand-v6.1 >>>> - const: brcm,brcmnand >>>> - - description: BCM63168 SoC-specific NAND controller >>>> + - description: BCM63xx SoC-specific NAND controller >>>> items: >>>> - - const: brcm,nand-bcm63168 >>>> - - const: brcm,nand-bcm6368 >>>> - - const: brcm,brcmnand-v4.0 >>>> + - enum: >>>> + - brcm,nand-bcm63168 >>>> + - brcm,nand-bcm6368 >>>> + - enum: >>>> + - brcm,brcmnand-v2.1 >>>> + - brcm,brcmnand-v2.2 >>>> + - brcm,brcmnand-v4.0 >>>> - const: brcm,brcmnand >>>> >> reg: >>>> @@ -111,6 +117,19 @@ properties: >>>> earlier versions of this core that include WP >>>> type: boolean >>>> >> + brcm,nand-use-wp: >>>> + description: >>>> + Use this integer to indicate if board design uses >>>> + controller's write protection feature and connects its >>>> + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to >>>> + use this feature when this property does not exist. >>>> + Set to 0 if WP pins are not connected and feature is not >>>> + used. Set to 1 if WP pins are connected and feature is used. >>>> + Set to 2 if WP pins are connected but disable this feature >>>> + through driver that sets controller to output high on NAND_WPb. >>>> + $ref: /schemas/types.yaml#/definitions/uint32 >>>> + enum: [0, 1, 2] >>> >>> Perhaps strings would be welcome. I'll let binding maintainers say what >>> they think of it. >>> >> Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2. >> >>>> + >>>> patternProperties: >>>> "^nand@[a-f0-9]$": >>>> type: object >>>> @@ -136,13 +155,23 @@ patternProperties: >>>> layout. >>>> $ref: /schemas/types.yaml#/definitions/uint32 >>>> >> + brcm,nand-ecc-use-strap: >>>> + description: >>>> + This flag is used by the driver to get the ecc strength and >>>> + spare area size from the SoC NAND boot strap setting. This >>>> + is commonly used by the BCMBCA SoC board design. If ecc >>>> + strength and spare area size are set by nand-ecc-strength >>>> + and brcm,nand-oob-sector-size in the dts, these settings >>>> + have precedence and override this flag. >>>> + $ref: /schemas/types.yaml#/definitions/flag >>> >>> How in practice do you access the strap value? Don't you need a phandle >>> over a specific area in the SoC? >>> >> The strap value is latched and stored in the NAND controller register so there is no extra phandle needed. > > Ok. > >> >>>> + >>>> allOf: >>>> - $ref: nand-controller.yaml# >>>> - if: >>>> properties: >>>> compatible: >>>> contains: >>>> - const: brcm,nand-bcm63138 >>>> + const: brcm,nand-bcmbca >>>> then: >>>> properties: >>>> reg-names: >>>> @@ -153,7 +182,9 @@ allOf: >>>> properties: >>>> compatible: >>>> contains: >>>> - const: brcm,nand-bcm6368 >>>> + enum: >>>> + - brcm,nand-bcm63168 >>>> + - brcm,nand-bcm6368 >>>> then: >>>> properties: >>>> reg-names: >>>> @@ -173,20 +204,12 @@ allOf: >>>> - const: nand >>>> - const: iproc-idm >>>> - const: iproc-ext >>>> - - if: >>>> - properties: >>>> - interrupts: >>>> - minItems: 2 >>>> - then: >>>> - required: >>>> - - interrupt-names >>> >>> Why do you remove this? Removing "interrupts" from the required >>> properties is fine, but constraining the interrupts property when it is >>> relevant is still expected. >>> >> There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead. > > It does not matter in this case how the driver uses the interrupts. If > names have been provided once in the bindings, you could expect another > project using the same bindings to use the interrupt names instead of > the order. So you must keep the names. Just don't mark them required it > they are. > Certainly any dts file can still have interrupts names with one or two interrupts but to my understand it is not required to have the name when there are two interrupts. Anyway I will add this back so this constrain remains. >> >>>> >> unevaluatedProperties: false >>>> >> required: >>>> - reg >>>> - reg-names >>>> - - interrupts >>> >>> This should be done in a separate patch. >>> >> I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name. > > The fact that the driver does not use the interrupts does not mean they > should not be described. > >> >>>> >> examples: >>>> - | >>>> @@ -215,8 +238,7 @@ examples: >>>> }; >>>> - | >>>> nand-controller@10000200 { >>>> - compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368", >>>> - "brcm,brcmnand-v4.0", "brcm,brcmnand"; >>>> + compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand"; >>>> reg = <0x10000200 0x180>, >>>> <0x100000b0 0x10>, >>>> <0x10000600 0x200>; >>> >>> >>> Thanks, >>> Miquèl >>> > > > Thanks, > Miquèl >
Hi William, william.zhang@broadcom.com wrote on Fri, 9 Jun 2023 12:05:19 -0700: > Hi Miquel, > > On 06/09/2023 01:58 AM, Miquel Raynal wrote: > > Hi William, > > > > william.zhang@broadcom.com wrote on Wed, 7 Jun 2023 13:01:56 -0700: > > > >> Hi Miquel, > >> > >> On 06/07/2023 01:14 AM, Miquel Raynal wrote: > >>> Hi William, > >>> > >>> william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: > >>> >>>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband > >>>> product. The old compatible string is still kept in the driver so old > >>>> dtb can still work. > >>>> > >>>> Add brcm,nand-use-wp property to have an option for disabling this > >>>> feature on broadband board design that does not use write protection. > >>>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for > >>>> broadband board designs because they do not specify ecc setting in dts > >>>> but rather using the strap setting. > >>>> > >>>> Remove the requirement of interrupts and interrupt-names properties to > >>>> reflect the driver code. > >>>> > >>>> This patch also includes a few minor fixes to the BCM63xx compatibles > >>>> and add myself to the list of maintainers. > >>>> > >>>> Signed-off-by: William Zhang <william.zhang@broadcom.com> > >>>> --- > >>>> > >>>> .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ > >>>> 1 file changed, 43 insertions(+), 21 deletions(-) > >>>> > >>>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > >>>> index 1571024aa119..1fe1c166a9db 100644 > >>>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > >>>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > >>>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller > >>>> maintainers: > >>>> - Brian Norris <computersforpeace@gmail.com> > >>>> - Kamal Dasu <kdasu.kdev@gmail.com> > >>>> + - William Zhang <william.zhang@broadcom.com> > >>>> >> description: | > >>>> The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND > >>>> @@ -18,9 +19,10 @@ description: | > >>>> supports basic PROGRAM and READ functions, among other features. > >>>> >> This controller was originally designed for STB SoCs (BCM7xxx) but is now > >>>> - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and > >>>> - iProc/Cygnus. Its history includes several similar (but not fully register > >>>> - compatible) versions. > >>>> + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based > >>>> + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. > >>>> + Its history includes several similar (but not fully register compatible) > >>>> + versions. > >>>> >> -- Additional SoC-specific NAND controller properties -- > >>>> >> @@ -53,9 +55,9 @@ properties: > >>>> - brcm,brcmnand-v7.2 > >>>> - brcm,brcmnand-v7.3 > >>>> - const: brcm,brcmnand > >>>> - - description: BCM63138 SoC-specific NAND controller > >>>> + - description: BCMBCA SoC-specific NAND controller > >>>> items: > >>>> - - const: brcm,nand-bcm63138 > >>>> + - const: brcm,nand-bcmbca > >>>> - enum: > >>>> - brcm,brcmnand-v7.0 > >>>> - brcm,brcmnand-v7.1 > >>>> @@ -65,11 +67,15 @@ properties: > >>>> - const: brcm,nand-iproc > >>>> - const: brcm,brcmnand-v6.1 > >>>> - const: brcm,brcmnand > >>>> - - description: BCM63168 SoC-specific NAND controller > >>>> + - description: BCM63xx SoC-specific NAND controller > >>>> items: > >>>> - - const: brcm,nand-bcm63168 > >>>> - - const: brcm,nand-bcm6368 > >>>> - - const: brcm,brcmnand-v4.0 > >>>> + - enum: > >>>> + - brcm,nand-bcm63168 > >>>> + - brcm,nand-bcm6368 > >>>> + - enum: > >>>> + - brcm,brcmnand-v2.1 > >>>> + - brcm,brcmnand-v2.2 > >>>> + - brcm,brcmnand-v4.0 > >>>> - const: brcm,brcmnand > >>>> >> reg: > >>>> @@ -111,6 +117,19 @@ properties: > >>>> earlier versions of this core that include WP > >>>> type: boolean > >>>> >> + brcm,nand-use-wp: > >>>> + description: > >>>> + Use this integer to indicate if board design uses > >>>> + controller's write protection feature and connects its > >>>> + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to > >>>> + use this feature when this property does not exist. > >>>> + Set to 0 if WP pins are not connected and feature is not > >>>> + used. Set to 1 if WP pins are connected and feature is used. > >>>> + Set to 2 if WP pins are connected but disable this feature > >>>> + through driver that sets controller to output high on NAND_WPb. > >>>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>>> + enum: [0, 1, 2] > >>> > >>> Perhaps strings would be welcome. I'll let binding maintainers say what > >>> they think of it. > >>> >> Practically there is really just use cases of 0 and 1. I could use a bool flag but to keep consistent with the driver code and in case there is any existing usage of 2. > >> > >>>> + > >>>> patternProperties: > >>>> "^nand@[a-f0-9]$": > >>>> type: object > >>>> @@ -136,13 +155,23 @@ patternProperties: > >>>> layout. > >>>> $ref: /schemas/types.yaml#/definitions/uint32 > >>>> >> + brcm,nand-ecc-use-strap: > >>>> + description: > >>>> + This flag is used by the driver to get the ecc strength and > >>>> + spare area size from the SoC NAND boot strap setting. This > >>>> + is commonly used by the BCMBCA SoC board design. If ecc > >>>> + strength and spare area size are set by nand-ecc-strength > >>>> + and brcm,nand-oob-sector-size in the dts, these settings > >>>> + have precedence and override this flag. > >>>> + $ref: /schemas/types.yaml#/definitions/flag > >>> > >>> How in practice do you access the strap value? Don't you need a phandle > >>> over a specific area in the SoC? > >>> >> The strap value is latched and stored in the NAND controller register so there is no extra phandle needed. > > > > Ok. > > > >> > >>>> + > >>>> allOf: > >>>> - $ref: nand-controller.yaml# > >>>> - if: > >>>> properties: > >>>> compatible: > >>>> contains: > >>>> - const: brcm,nand-bcm63138 > >>>> + const: brcm,nand-bcmbca > >>>> then: > >>>> properties: > >>>> reg-names: > >>>> @@ -153,7 +182,9 @@ allOf: > >>>> properties: > >>>> compatible: > >>>> contains: > >>>> - const: brcm,nand-bcm6368 > >>>> + enum: > >>>> + - brcm,nand-bcm63168 > >>>> + - brcm,nand-bcm6368 > >>>> then: > >>>> properties: > >>>> reg-names: > >>>> @@ -173,20 +204,12 @@ allOf: > >>>> - const: nand > >>>> - const: iproc-idm > >>>> - const: iproc-ext > >>>> - - if: > >>>> - properties: > >>>> - interrupts: > >>>> - minItems: 2 > >>>> - then: > >>>> - required: > >>>> - - interrupt-names > >>> > >>> Why do you remove this? Removing "interrupts" from the required > >>> properties is fine, but constraining the interrupts property when it is > >>> relevant is still expected. > >>> >> There is no requirement for interrupt name even if it have two interrupts. Driver code does not use interrupt name but the interrupt index instead. > > > > It does not matter in this case how the driver uses the interrupts. If > > names have been provided once in the bindings, you could expect another > > project using the same bindings to use the interrupt names instead of > > the order. So you must keep the names. Just don't mark them required it > > they are. > > > Certainly any dts file can still have interrupts names with one or two interrupts but to my understand it is not required to have the name when there are two interrupts. Anyway I will add this back so this constrain remains. That is true, until you enforce specific names. Then you are tied to these names, otherwise you may break the bindings. It it is not _really_ useful, then don't do it. > > >> > >>>> >> unevaluatedProperties: false > >>>> >> required: > >>>> - reg > >>>> - reg-names > >>>> - - interrupts > >>> > >>> This should be done in a separate patch. > >>> >> I thought this is also related to my update for bcmbca chips because they don't need to interrupt and interrupt name. > > > > The fact that the driver does not use the interrupts does not mean they > > should not be described. > > > >> > >>>> >> examples: > >>>> - | > >>>> @@ -215,8 +238,7 @@ examples: > >>>> }; > >>>> - | > >>>> nand-controller@10000200 { > >>>> - compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368", > >>>> - "brcm,brcmnand-v4.0", "brcm,brcmnand"; > >>>> + compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand"; > >>>> reg = <0x10000200 0x180>, > >>>> <0x100000b0 0x10>, > >>>> <0x10000600 0x200>; > >>> > >>> > >>> Thanks, > >>> Miquèl > >>> > > > > Thanks, > > Miquèl > > Thanks, Miquèl
On Tue, Jun 06, 2023 at 04:12:45PM -0700, William Zhang wrote: > Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband > product. The old compatible string is still kept in the driver so old > dtb can still work. I don't understand why you need to change this. > Add brcm,nand-use-wp property to have an option for disabling this > feature on broadband board design that does not use write protection. > Add brcm,nand-ecc-use-strap to get ecc setting from board strap for > broadband board designs because they do not specify ecc setting in dts > but rather using the strap setting. > > Remove the requirement of interrupts and interrupt-names properties to > reflect the driver code. > > This patch also includes a few minor fixes to the BCM63xx compatibles > and add myself to the list of maintainers. > > Signed-off-by: William Zhang <william.zhang@broadcom.com> > --- > > .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ > 1 file changed, 43 insertions(+), 21 deletions(-) > > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > index 1571024aa119..1fe1c166a9db 100644 > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller > maintainers: > - Brian Norris <computersforpeace@gmail.com> > - Kamal Dasu <kdasu.kdev@gmail.com> > + - William Zhang <william.zhang@broadcom.com> > > description: | > The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND > @@ -18,9 +19,10 @@ description: | > supports basic PROGRAM and READ functions, among other features. > > This controller was originally designed for STB SoCs (BCM7xxx) but is now > - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and > - iProc/Cygnus. Its history includes several similar (but not fully register > - compatible) versions. > + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based > + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. > + Its history includes several similar (but not fully register compatible) > + versions. > > -- Additional SoC-specific NAND controller properties -- > > @@ -53,9 +55,9 @@ properties: > - brcm,brcmnand-v7.2 > - brcm,brcmnand-v7.3 > - const: brcm,brcmnand > - - description: BCM63138 SoC-specific NAND controller > + - description: BCMBCA SoC-specific NAND controller > items: > - - const: brcm,nand-bcm63138 > + - const: brcm,nand-bcmbca > - enum: > - brcm,brcmnand-v7.0 > - brcm,brcmnand-v7.1 > @@ -65,11 +67,15 @@ properties: > - const: brcm,nand-iproc > - const: brcm,brcmnand-v6.1 > - const: brcm,brcmnand > - - description: BCM63168 SoC-specific NAND controller > + - description: BCM63xx SoC-specific NAND controller > items: > - - const: brcm,nand-bcm63168 > - - const: brcm,nand-bcm6368 > - - const: brcm,brcmnand-v4.0 > + - enum: > + - brcm,nand-bcm63168 > + - brcm,nand-bcm6368 > + - enum: > + - brcm,brcmnand-v2.1 > + - brcm,brcmnand-v2.2 > + - brcm,brcmnand-v4.0 > - const: brcm,brcmnand Completely changing what's valid here too. You're breaking the ABI. Rob
On Wed, Jun 07, 2023 at 10:14:52AM +0200, Miquel Raynal wrote: > Hi William, > > william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: > > > Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband > > product. The old compatible string is still kept in the driver so old > > dtb can still work. > > > > Add brcm,nand-use-wp property to have an option for disabling this > > feature on broadband board design that does not use write protection. > > Add brcm,nand-ecc-use-strap to get ecc setting from board strap for > > broadband board designs because they do not specify ecc setting in dts > > but rather using the strap setting. > > > > Remove the requirement of interrupts and interrupt-names properties to > > reflect the driver code. > > > > This patch also includes a few minor fixes to the BCM63xx compatibles > > and add myself to the list of maintainers. > > > > Signed-off-by: William Zhang <william.zhang@broadcom.com> > > --- > > > > .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ > > 1 file changed, 43 insertions(+), 21 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > > index 1571024aa119..1fe1c166a9db 100644 > > --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > > +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml > > @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller > > maintainers: > > - Brian Norris <computersforpeace@gmail.com> > > - Kamal Dasu <kdasu.kdev@gmail.com> > > + - William Zhang <william.zhang@broadcom.com> > > > > description: | > > The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND > > @@ -18,9 +19,10 @@ description: | > > supports basic PROGRAM and READ functions, among other features. > > > > This controller was originally designed for STB SoCs (BCM7xxx) but is now > > - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and > > - iProc/Cygnus. Its history includes several similar (but not fully register > > - compatible) versions. > > + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based > > + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. > > + Its history includes several similar (but not fully register compatible) > > + versions. > > > > -- Additional SoC-specific NAND controller properties -- > > > > @@ -53,9 +55,9 @@ properties: > > - brcm,brcmnand-v7.2 > > - brcm,brcmnand-v7.3 > > - const: brcm,brcmnand > > - - description: BCM63138 SoC-specific NAND controller > > + - description: BCMBCA SoC-specific NAND controller > > items: > > - - const: brcm,nand-bcm63138 > > + - const: brcm,nand-bcmbca > > - enum: > > - brcm,brcmnand-v7.0 > > - brcm,brcmnand-v7.1 > > @@ -65,11 +67,15 @@ properties: > > - const: brcm,nand-iproc > > - const: brcm,brcmnand-v6.1 > > - const: brcm,brcmnand > > - - description: BCM63168 SoC-specific NAND controller > > + - description: BCM63xx SoC-specific NAND controller > > items: > > - - const: brcm,nand-bcm63168 > > - - const: brcm,nand-bcm6368 > > - - const: brcm,brcmnand-v4.0 > > + - enum: > > + - brcm,nand-bcm63168 > > + - brcm,nand-bcm6368 > > + - enum: > > + - brcm,brcmnand-v2.1 > > + - brcm,brcmnand-v2.2 > > + - brcm,brcmnand-v4.0 > > - const: brcm,brcmnand > > > > reg: > > @@ -111,6 +117,19 @@ properties: > > earlier versions of this core that include WP > > type: boolean > > > > + brcm,nand-use-wp: > > + description: > > + Use this integer to indicate if board design uses > > + controller's write protection feature and connects its > > + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to > > + use this feature when this property does not exist. > > + Set to 0 if WP pins are not connected and feature is not > > + used. Set to 1 if WP pins are connected and feature is used. > > + Set to 2 if WP pins are connected but disable this feature > > + through driver that sets controller to output high on NAND_WPb. > > + $ref: /schemas/types.yaml#/definitions/uint32 > > + enum: [0, 1, 2] > > Perhaps strings would be welcome. I'll let binding maintainers say what > they think of it. 1 is redundant as it is the same as not present. Seems like a 'brcm,disable-wp' boolean would suffice here. Rob
Hi Rob, On 06/14/2023 03:43 PM, Rob Herring wrote: > On Tue, Jun 06, 2023 at 04:12:45PM -0700, William Zhang wrote: >> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband >> product. The old compatible string is still kept in the driver so old >> dtb can still work. > > I don't understand why you need to change this. > > Same as the iProc SoC NAND controller flavor for our BCMBCA SoC controller. And the existing binding is wrong. It shows brcm,nand-bcm63138 can be backward compatible to either v7.0 and v7.1 controller. But in fact 63138 only use v7.0. I understand we had discussion before and chip specific name is the preferred method but the nand controller here is different and it has well defined and documented version control and our bcmbca SoCs integrate the controller exactly the same way so no chip specific compatible is needed. In the unlikely event that a future bcmbca SoC has a different implementation, we can always add new chip specific compatible. If you are concerned about ABI breakage on the 63138 compatible, I can add it back, although I find only a few 63138 and 4908 boards use it and I did keep it in the driver so they can still bind. >> Add brcm,nand-use-wp property to have an option for disabling this >> feature on broadband board design that does not use write protection. >> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for >> broadband board designs because they do not specify ecc setting in dts >> but rather using the strap setting. >> >> Remove the requirement of interrupts and interrupt-names properties to >> reflect the driver code. >> >> This patch also includes a few minor fixes to the BCM63xx compatibles >> and add myself to the list of maintainers. >> >> Signed-off-by: William Zhang <william.zhang@broadcom.com> >> --- >> >> .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ >> 1 file changed, 43 insertions(+), 21 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >> index 1571024aa119..1fe1c166a9db 100644 >> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller >> maintainers: >> - Brian Norris <computersforpeace@gmail.com> >> - Kamal Dasu <kdasu.kdev@gmail.com> >> + - William Zhang <william.zhang@broadcom.com> >> >> description: | >> The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND >> @@ -18,9 +19,10 @@ description: | >> supports basic PROGRAM and READ functions, among other features. >> >> This controller was originally designed for STB SoCs (BCM7xxx) but is now >> - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and >> - iProc/Cygnus. Its history includes several similar (but not fully register >> - compatible) versions. >> + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based >> + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. >> + Its history includes several similar (but not fully register compatible) >> + versions. >> >> -- Additional SoC-specific NAND controller properties -- >> >> @@ -53,9 +55,9 @@ properties: >> - brcm,brcmnand-v7.2 >> - brcm,brcmnand-v7.3 >> - const: brcm,brcmnand >> - - description: BCM63138 SoC-specific NAND controller >> + - description: BCMBCA SoC-specific NAND controller >> items: >> - - const: brcm,nand-bcm63138 >> + - const: brcm,nand-bcmbca >> - enum: >> - brcm,brcmnand-v7.0 >> - brcm,brcmnand-v7.1 >> @@ -65,11 +67,15 @@ properties: >> - const: brcm,nand-iproc >> - const: brcm,brcmnand-v6.1 >> - const: brcm,brcmnand >> - - description: BCM63168 SoC-specific NAND controller >> + - description: BCM63xx SoC-specific NAND controller >> items: >> - - const: brcm,nand-bcm63168 >> - - const: brcm,nand-bcm6368 >> - - const: brcm,brcmnand-v4.0 >> + - enum: >> + - brcm,nand-bcm63168 >> + - brcm,nand-bcm6368 >> + - enum: >> + - brcm,brcmnand-v2.1 >> + - brcm,brcmnand-v2.2 >> + - brcm,brcmnand-v4.0 >> - const: brcm,brcmnand > > Completely changing what's valid here too. You're breaking the ABI. > This binding is wrong, 63168 is not backward compatible to 6368. They should backward compatible to their specific controller version. If you looks arch/mips/boot/dts/brcm/bcm6328.dtsi, bcm6368.dtsi, bcm63268.dtsi and bcm6362.dtsi, their compatible list is correct but the yaml was not updated. > Rob >
Hi Rob, On 06/14/2023 03:46 PM, Rob Herring wrote: > On Wed, Jun 07, 2023 at 10:14:52AM +0200, Miquel Raynal wrote: >> Hi William, >> >> william.zhang@broadcom.com wrote on Tue, 6 Jun 2023 16:12:45 -0700: >> >>> Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband >>> product. The old compatible string is still kept in the driver so old >>> dtb can still work. >>> >>> Add brcm,nand-use-wp property to have an option for disabling this >>> feature on broadband board design that does not use write protection. >>> Add brcm,nand-ecc-use-strap to get ecc setting from board strap for >>> broadband board designs because they do not specify ecc setting in dts >>> but rather using the strap setting. >>> >>> Remove the requirement of interrupts and interrupt-names properties to >>> reflect the driver code. >>> >>> This patch also includes a few minor fixes to the BCM63xx compatibles >>> and add myself to the list of maintainers. >>> >>> Signed-off-by: William Zhang <william.zhang@broadcom.com> >>> --- >>> >>> .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ >>> 1 file changed, 43 insertions(+), 21 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >>> index 1571024aa119..1fe1c166a9db 100644 >>> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >>> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml >>> @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller >>> maintainers: >>> - Brian Norris <computersforpeace@gmail.com> >>> - Kamal Dasu <kdasu.kdev@gmail.com> >>> + - William Zhang <william.zhang@broadcom.com> >>> >>> description: | >>> The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND >>> @@ -18,9 +19,10 @@ description: | >>> supports basic PROGRAM and READ functions, among other features. >>> >>> This controller was originally designed for STB SoCs (BCM7xxx) but is now >>> - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and >>> - iProc/Cygnus. Its history includes several similar (but not fully register >>> - compatible) versions. >>> + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based >>> + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. >>> + Its history includes several similar (but not fully register compatible) >>> + versions. >>> >>> -- Additional SoC-specific NAND controller properties -- >>> >>> @@ -53,9 +55,9 @@ properties: >>> - brcm,brcmnand-v7.2 >>> - brcm,brcmnand-v7.3 >>> - const: brcm,brcmnand >>> - - description: BCM63138 SoC-specific NAND controller >>> + - description: BCMBCA SoC-specific NAND controller >>> items: >>> - - const: brcm,nand-bcm63138 >>> + - const: brcm,nand-bcmbca >>> - enum: >>> - brcm,brcmnand-v7.0 >>> - brcm,brcmnand-v7.1 >>> @@ -65,11 +67,15 @@ properties: >>> - const: brcm,nand-iproc >>> - const: brcm,brcmnand-v6.1 >>> - const: brcm,brcmnand >>> - - description: BCM63168 SoC-specific NAND controller >>> + - description: BCM63xx SoC-specific NAND controller >>> items: >>> - - const: brcm,nand-bcm63168 >>> - - const: brcm,nand-bcm6368 >>> - - const: brcm,brcmnand-v4.0 >>> + - enum: >>> + - brcm,nand-bcm63168 >>> + - brcm,nand-bcm6368 >>> + - enum: >>> + - brcm,brcmnand-v2.1 >>> + - brcm,brcmnand-v2.2 >>> + - brcm,brcmnand-v4.0 >>> - const: brcm,brcmnand >>> >>> reg: >>> @@ -111,6 +117,19 @@ properties: >>> earlier versions of this core that include WP >>> type: boolean >>> >>> + brcm,nand-use-wp: >>> + description: >>> + Use this integer to indicate if board design uses >>> + controller's write protection feature and connects its >>> + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to >>> + use this feature when this property does not exist. >>> + Set to 0 if WP pins are not connected and feature is not >>> + used. Set to 1 if WP pins are connected and feature is used. >>> + Set to 2 if WP pins are connected but disable this feature >>> + through driver that sets controller to output high on NAND_WPb. >>> + $ref: /schemas/types.yaml#/definitions/uint32 >>> + enum: [0, 1, 2] >> >> Perhaps strings would be welcome. I'll let binding maintainers say what >> they think of it. > > 1 is redundant as it is the same as not present. Seems like a > 'brcm,disable-wp' boolean would suffice here. > WP feature present is indicated by the existing flag brcm,nand-has-wp. 1 is the opposite of 0 (use this feature vs not use this feature while chip has this feature). 2 is kind of strange (for feature disabled intentionally at run time while feature is used), maybe for testing purpose or for certainly nand chip that has issue with WP but the board design already connect the WP pin. I keep it here to match the driver code that have these three states of the wp usage. > Rob >
diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml index 1571024aa119..1fe1c166a9db 100644 --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml @@ -9,6 +9,7 @@ title: Broadcom STB NAND Controller maintainers: - Brian Norris <computersforpeace@gmail.com> - Kamal Dasu <kdasu.kdev@gmail.com> + - William Zhang <william.zhang@broadcom.com> description: | The Broadcom Set-Top Box NAND controller supports low-level access to raw NAND @@ -18,9 +19,10 @@ description: | supports basic PROGRAM and READ functions, among other features. This controller was originally designed for STB SoCs (BCM7xxx) but is now - available on a variety of Broadcom SoCs, including some BCM3xxx, BCM63xx, and - iProc/Cygnus. Its history includes several similar (but not fully register - compatible) versions. + available on a variety of Broadcom SoCs, including some BCM3xxx, MIPS based + Broadband SoC (BCM63xx), ARM based Broadband SoC (BCMBCA) and iProc/Cygnus. + Its history includes several similar (but not fully register compatible) + versions. -- Additional SoC-specific NAND controller properties -- @@ -53,9 +55,9 @@ properties: - brcm,brcmnand-v7.2 - brcm,brcmnand-v7.3 - const: brcm,brcmnand - - description: BCM63138 SoC-specific NAND controller + - description: BCMBCA SoC-specific NAND controller items: - - const: brcm,nand-bcm63138 + - const: brcm,nand-bcmbca - enum: - brcm,brcmnand-v7.0 - brcm,brcmnand-v7.1 @@ -65,11 +67,15 @@ properties: - const: brcm,nand-iproc - const: brcm,brcmnand-v6.1 - const: brcm,brcmnand - - description: BCM63168 SoC-specific NAND controller + - description: BCM63xx SoC-specific NAND controller items: - - const: brcm,nand-bcm63168 - - const: brcm,nand-bcm6368 - - const: brcm,brcmnand-v4.0 + - enum: + - brcm,nand-bcm63168 + - brcm,nand-bcm6368 + - enum: + - brcm,brcmnand-v2.1 + - brcm,brcmnand-v2.2 + - brcm,brcmnand-v4.0 - const: brcm,brcmnand reg: @@ -111,6 +117,19 @@ properties: earlier versions of this core that include WP type: boolean + brcm,nand-use-wp: + description: + Use this integer to indicate if board design uses + controller's write protection feature and connects its + NAND_WPb pin to nand chip's WP_L pin. Driver defaults to + use this feature when this property does not exist. + Set to 0 if WP pins are not connected and feature is not + used. Set to 1 if WP pins are connected and feature is used. + Set to 2 if WP pins are connected but disable this feature + through driver that sets controller to output high on NAND_WPb. + $ref: /schemas/types.yaml#/definitions/uint32 + enum: [0, 1, 2] + patternProperties: "^nand@[a-f0-9]$": type: object @@ -136,13 +155,23 @@ patternProperties: layout. $ref: /schemas/types.yaml#/definitions/uint32 + brcm,nand-ecc-use-strap: + description: + This flag is used by the driver to get the ecc strength and + spare area size from the SoC NAND boot strap setting. This + is commonly used by the BCMBCA SoC board design. If ecc + strength and spare area size are set by nand-ecc-strength + and brcm,nand-oob-sector-size in the dts, these settings + have precedence and override this flag. + $ref: /schemas/types.yaml#/definitions/flag + allOf: - $ref: nand-controller.yaml# - if: properties: compatible: contains: - const: brcm,nand-bcm63138 + const: brcm,nand-bcmbca then: properties: reg-names: @@ -153,7 +182,9 @@ allOf: properties: compatible: contains: - const: brcm,nand-bcm6368 + enum: + - brcm,nand-bcm63168 + - brcm,nand-bcm6368 then: properties: reg-names: @@ -173,20 +204,12 @@ allOf: - const: nand - const: iproc-idm - const: iproc-ext - - if: - properties: - interrupts: - minItems: 2 - then: - required: - - interrupt-names unevaluatedProperties: false required: - reg - reg-names - - interrupts examples: - | @@ -215,8 +238,7 @@ examples: }; - | nand-controller@10000200 { - compatible = "brcm,nand-bcm63168", "brcm,nand-bcm6368", - "brcm,brcmnand-v4.0", "brcm,brcmnand"; + compatible = "brcm,nand-bcm6368", "brcm,brcmnand-v2.1", "brcm,brcmnand"; reg = <0x10000200 0x180>, <0x100000b0 0x10>, <0x10000600 0x200>;
Use new compatiable brcm,nand-bcmbca to support BCMBCA broadband product. The old compatible string is still kept in the driver so old dtb can still work. Add brcm,nand-use-wp property to have an option for disabling this feature on broadband board design that does not use write protection. Add brcm,nand-ecc-use-strap to get ecc setting from board strap for broadband board designs because they do not specify ecc setting in dts but rather using the strap setting. Remove the requirement of interrupts and interrupt-names properties to reflect the driver code. This patch also includes a few minor fixes to the BCM63xx compatibles and add myself to the list of maintainers. Signed-off-by: William Zhang <william.zhang@broadcom.com> --- .../bindings/mtd/brcm,brcmnand.yaml | 64 +++++++++++++------ 1 file changed, 43 insertions(+), 21 deletions(-)