On 01/24/2012 01:23 AM, Heiko Schocher wrote: > Hello Scott, > > Scott Wood wrote: >> On 01/23/2012 02:56 AM, Heiko Schocher wrote: >>> diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt >>> b/Documentation/devicetree/bindings/arm/davinci/nand.txt >>> new file mode 100644 >>> index 0000000..7e8d6db >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/arm/davinci/nand.txt >>> @@ -0,0 +1,72 @@ >>> +* Texas Instruments Davinci NAND >>> + >>> +This file provides information, what the device node for the >>> +davinci nand interface contain. >>> + >>> +Required properties: >>> +- compatible: "ti,davinci-nand"; >>> +- reg : contain 2 offset/length values: >>> + - offset and length for the access window >>> + - offset and length for accessing the aemif control registers >>> +- id: id of the controller >> >> What does "id of the controller" mean, specfically? From this I can't >> even tell if it's a number or a string, much less how to use it >> semantically. If it's just a "match what's in the manual" thing, >> perhaps an alias would be better here. Or, if it's a value with a >> specific meaning (e.g. that you need to program into a register) use a >> more specific name. > > Ok, fix this. Id means here, which chipselect the controller uses. > Maybe it is better to rename it to "chipselect" ?
Yes, or better "ti,chipselect" or "ti,davinci-chipselect". >>> +Recommended properties : >>> +- mask_ale: mask for ale >>> +- mask_cle: mask for cle >>> +- mask_chipsel: mask for chipselect >>> +- ecc_mode: ECC mode, see NAND_ECC_* defines >>> +- ecc_bits: used ECC bits >>> +- options: nand options, defined in >>> + include/linux/mtd/nand.h, grep for NAND_NO_AUTOINCR >>> +- bbt_options: NAND_BBT_* defines >> >> Binding-specific properties should have a vendor prefix. Dashes are >> preferred to underscores. > > You think something like that: > > davinci-mask-ale > davinci-mask-cle > ... "ti,davinci-mask-ale", etc. >> Don't specify Linux internals by reference -- they could change and >> invalidate existing device trees and non-Linux code that accepts them >> (e.g. U-Boot). If you want them to line up, copy the definition here, >> and if Linux changes, write glue code to convert. It would probably be >> better to define specific properties for anything that must be specified >> here (neither deteted dynamically nor defined by compatible = >> "ti,davinci-nand"). > > Ok, I add the defines here, and add also a comment in the dts. Which options actually need to come from the device tree? >> Do all of these properties really belong here? I can see providing some > > I think so, because this values come from existing platform code > (grep for struct davinci_nand_pdata) The standards are a bit stricter for the device tree, since it's a more stable interface across components -- at least that's how we've used it on a lot of powerpc targets. I'm not sure if that's the intent here, but I have seen U-Boot patches for ARM hardware using them as well. > Comment in arch/arm/mach-davinci/include/mach/nand.h says for > mask_ale and mask_cle: > > /* NOTE: boards don't need to use these address bits > * for ALE/CLE unless they support booting from NAND. > * They're used unless platform data overrides them. > */ > > It is used for addressing the ALE/CLE Signals through the address, > used on the arch/arm/mach-davinci/board-dm646x-evm.c and > arch/arm/mach-davinci/board-tnetv107x-evm.c board ... so I think, > this should be also be setupable through OF ... OK, if it's board logic that does the decoding, and the compatible is not board-specific, they belong here. -Scott _______________________________________________ devicetree-discuss mailing list [email protected] https://lists.ozlabs.org/listinfo/devicetree-discuss
