On 02/12/15 19:05, Brian Norris wrote: > + Broadcom list + Kamal > > On Tue, Nov 24, 2015 at 08:19:37PM -0000, Simon Arlott wrote: >> Add device tree binding for NAND on the BCM63268. >> >> The BCM63268 has a NAND interrupt register with combined status and enable >> registers. >> >> Signed-off-by: Simon Arlott <simon@xxxxxxxxxxx> >> --- >> .../devicetree/bindings/mtd/brcm,brcmnand.txt | 35 ++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> index 4ff7128..f2a71c8 100644 >> --- a/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> +++ b/Documentation/devicetree/bindings/mtd/brcm,brcmnand.txt >> @@ -72,6 +72,14 @@ we define additional 'compatible' properties and associated register resources w >> and enable registers >> - reg-names: (required) "nand-int-base" >> >> + * "brcm,nand-bcm63268" >> + - compatible: should contain "brcm,nand-bcm<soc>", "brcm,nand-bcm63268" > > Looks like you're aiming to support bcm63168? Is bcm63268 the first > chip to include this style of register then? The numbering seems > backwards, but that may just be reality. Yes, I have a BCM963168VX (BCM63168) but all the Broadcom code refers to this SoC as BCM63268. This is the only one with these registers in the source code of similar MIPS that I have. https://github.com/lp0/bcm963xx_4.12L.06B_consumer/blob/master/bcmdrivers/opensource/char/board/bcm963xx/impl1/board.c#L6595 int kerSysGetChipId() { ... /* Force BCM63168, BCM63169, and BCM63269 to be BCM63268) */ if( ( (r & 0xffffe) == 0x63168 ) || ( (r & 0xffffe) == 0x63268 )) r = 0x63268; >> + - reg: (required) the 'NAND_INTR_BASE' register range, with combined status >> + and enable registers, and boot address registers >> + - reg-names: (required) "nand-intr-base" >> + - clock: (required) reference to the clock for the NAND controller >> + - clock-names: (required) "nand" >> + >> * "brcm,nand-iproc" >> - reg: (required) the "IDM" register range, for interrupt enable and APB >> bus access endianness configuration, and the "EXT" register range, >> @@ -148,3 +156,30 @@ nand@f0442800 { >> }; >> }; >> }; >> + >> +nand@10000200 { >> + compatible = "brcm,nand-bcm63168", "brcm,nand-bcm63268", >> + "brcm,brcmnand-v4.0", "brcm,brcmnand"; >> + reg = <0x10000200 0x180>, >> + <0x10000600 0x200>, >> + <0x100000b0 0x10>; >> + reg-names = "nand", "nand-cache", "nand-intr-base"; >> + interrupt-parent = <&periph_intc>; >> + interrupts = <50>; >> + clocks = <&periph_clk 20>; >> + clock-names = "nand"; >> + >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + nand0: nandcs@0 { >> + compatible = "brcm,nandcs"; >> + reg = <0>; >> + nand-on-flash-bbt; >> + nand-ecc-strength = <1>; >> + nand-ecc-step-size = <512>; >> + >> + #address-cells = <0>; >> + #size-cells = <0>; > > What are these {address,size}-cells for? If you need them for > partitioning, then those are wrong -- they shouldn't be zero. Maybe just > drop them? (I can cut them out when applying, if that's the only change > to make.) This is the correct way to do partitioning, there would be a "partitions" block with no address/size that contains the partitions as child nodes. [Documentation/devicetree/bindings/mtd/partition.txt] I think they're also implicit so you can just remove those two lines. I've created a bcm963268part driver so there won't need to be any partitions in DT for bcm63268. >> + }; >> +}; > > Brian > -- Simon Arlott -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html