On Mon, 22 Jan 2024 at 18:34, William Zhang <william.zhang@xxxxxxxxxxxx> wrote:
>
> Hi,
>
> On 1/22/24 03:48, Jonas Gorski wrote:
> > Hi,
> >
> > On Thu, 18 Jan 2024 at 20:56, <dregan@xxxxxxxxxxxx> wrote:
> >>
> >> From: William Zhang <william.zhang@xxxxxxxxxxxx>
> >>
> >> Update the descriptions to reflect different families of broadband SoC and
> >> use the general name bcmbca for ARM based SoC.
> >>
> >> 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 boot 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 property to reflect the driver
> >> code. Also add myself to the list of maintainers.
> >>
> >> Signed-off-by: William Zhang <william.zhang@xxxxxxxxxxxx>
> >> Reviewed-by: David Regan <dregan@xxxxxxxxxxxx>
> >> ---
> >> 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
> >> ---
> >> .../bindings/mtd/brcm,brcmnand.yaml | 36 +++++++++++++++----
> >> 1 file changed, 30 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.yaml
> >> index f57e96374e67..56176ec1a992 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@xxxxxxxxx>
> >> - Kamal Dasu <kdasu.kdev@xxxxxxxxx>
> >> + - William Zhang <william.zhang@xxxxxxxxxxxx>
> >>
> >> 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:
> >> @@ -65,7 +67,7 @@ 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
> >
> > Only the BCM63268 family has a v4.0 NAND controller with support for
> > ONFI and raw access; BCM6368 has a v2.1, and BCM6328 and BCM6362 have
> > a v2.2.
> >
> > So claiming this is a generic binding is wrong; you would need to add
> > the appropriate variants first. Or add another one for the BCM6368
> > NAND v2.x controllers, which is missing. You can find them used in
> > arch/mips/boot/dts/brcm/bcm63{28,62,68}.dtsi.
> >
> I am not changing binding here but jsut update the description to
> identify these MIPS based chip as bcm63xx family. This convention is
> used in other IP blocks too. And yes this binding is not correct and I
> noticed the same v2.x usage in the dtsi files you pointed out. So I
> actually updated this binding in my v1 here
> https://lore.kernel.org/lkml/20230606231252.94838-6-william.zhang@xxxxxxxxxxxx/
> but there was some concern as it could possibly break the ABI in that
> thread's discussion.
That change did change the ABI that would have (AFAICT) broken it for BCM63168.
My point is: This binding in its current state is valid *only* for the
BCM63168 NAND controller, and not for any other BCM63xx NAND
controllers.
So changing the description to BCM63xx is wrong, unless you extend it
to also match the BCM6328/6362/6368 NAND controller bindings as used.
I can send a patch adding the missing binding parts, I actually have a
WIP one lying around as I started trying to address dts issues in the
MIPS bcm63xx dts(i) files a while ago.
Best Regards,
Jonas
