On 11/06/2012 02:47 PM, Murali Karicheri wrote:
> This is a platform driver for asynchronous external memory interface
> available on TI SoCs. This driver was previously located inside the
> mach-davinci folder. As this DaVinci IP is re-used across multiple
> family of devices such as c6x, keystone etc, the driver is moved to drivers.
> The driver configures async bus parameters associated with a particular
> chip select. For DaVinci NAND controller driver and driver for other async
> devices such as NOR flash, ASRAM etc, the bus confuguration is
> done by this driver at probe. A set of APIs (set/get) are provided to
> update the values based on specific driver usage.
> diff --git a/Documentation/devicetree/bindings/memory/davinci-aemif.txt b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> +* Texas Instruments Davinci AEMIF bus interface
> +
> +This file provides information for the davinci-emif device and
> +async bus bindings.
> +
> +Required properties:=
> +- compatible: "ti,davinci-aemif";
> +- #address-cells : Should be either two or three. The first cell is the
> + chipselect number, and the remaining cells are the
> + offset into the chipselect.
> +- #size-cells : Either one or two, depending on how large each chipselect
> + can be.
What about "how large each chipselect can be" determines 2-vs-3
(address) or 1-vs-2 (size) cells? I assume it's 32-vs-64-bit bus? It
might be best to make that explicit.
> +- reg : contains offset/length value for AEMIF control registers space
> +- ranges : Each range corresponds to a single chipselect, and cover
add "s" at the end of that line.
> + the entire access window as configured.
> +
> +Child device nodes describe the devices connected to IFC such as NOR (e.g.
> +cfi-flash) and NAND (ti,davinci-nand, see Documentation/devicetree/bindings/
> +mtd/davinci-nand.txt). There might be board specific devices like FPGAs.
> +
> +In addition, optional child sub nodes contains bindings for the async bus
> +interface for a given chip select.
Hmmm. That's two different kinds of children, which appear to use
different addressing schemes given the examples below; more on that below.
> +Optional cs node properties:-
> +- compatible: "ti,davinci-cs"
> +
> + All of the params below in nanoseconds and are optional
> +
> +- ti,davinci-cs-asize - asynchronous data bus width (0 - 8bit, 1 - 16 bit)
Perhaps s/asize/bus-width/? Also, directly using values 8 and 16 would
be a lot more obvious to read than 0 and 1.
> +- ti,davinci-cs-ta - Minimum turn around time
> +- ti,davinci-cs-rhold - read hold width
> +- ti,davinci-cs-rstobe - read strobe width
> +- ti,davinci-cs-rsetup - read setup width
> +- ti,davinci-cs-whold - write hold width
> +- ti,davinci-cs-wstrobe - write strobe width
> +- ti,davinci-cs-wsetup - write setup width
The comments in the examples indicate these are in nS. This should be
mentioned here instead I think.
Does there need to be a specification of bus clock rate? How does the
driver convert nS into number-of-clock-cycles, which I assume is what
the HW would be programmed with?
> +- ti,davinci-cs-ss - enable/disable select strobe (0 - disable, 1 - enable)
> +- ti,davinci-cs-ew - enable/disable extended wait cycles (0 - disable, 1 - enable)
Boolean properties are usually represented as present (on/enabled/1) or
missing (off/disabled/0). Shouldn't these two properties work that way?
I see the comment below:
> +if any of the above parameters are absent, hardware register default or that
> +set by a boot loader are used.
implied they're really tri-state (explicitly off or on, or just not
programmed). However, I think it'd be better if the DT always
represented the complete configuration, since the DT and binding should
be a full description of the HW rather than just the data you expect a
particular Linux driver to need after a particular boot loader has
executed first.
> +Example for aemif, davinci nand and nor flash chip select shown below.
> +
> +aemif@60000000 {
> + compatible = "ti,davinci-aemif";
> + #address-cells = <2>;
> + #size-cells = <1>;
> + reg = <0x68000000 0x80000>;
> + ranges = <2 0 0x60000000 0x02000000
> + 3 0 0x62000000 0x02000000
> + 4 0 0x64000000 0x02000000
> + 5 0 0x66000000 0x02000000
> + 6 0 0x68000000 0x02000000>;
> +
> + nand_cs:cs2@60000000 {
This node has no reg property, but it needs one if you use a unit
address ("@60000000") in the node name.
The numbering scheme unit address above doesn't seem to match the
addresses provided to child nodes by the ranges property. Perhaps the
node layout you want is:
aemif {
chip-selects {
nand_cs:cs2@60000000 {
};
};
devices {
nand@3,0 {
}
};
};
so that the chip-selects and devices nodes can both set appropriate and
different #address-cells and #size-cells?
That does feel a bit wierd though, and I imagine writing the ranges
property in the aemif node would be hard.
Perhaps the chip-select timings should just be written as properties in
the aemif controller, rather than child nodes?
ti,cs-timings =
< ... > /* 0 */
< ... > /* 1 */
< ... > /* 2 */
;
with each <...> being a list of n cells in a specific order?
> + compatible = "ti,davinci-cs";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + /* all timings in nanoseconds */
> + ti,davinci-cs-ta = <0>;
> + ti,davinci-cs-rhold = <7>;
> + ti,davinci-cs-rstrobe = <42>;
> + ti,davinci-cs-rsetup = <14>;
> + ti,davinci-cs-whold = <7>;
> + ti,davinci-cs-wstrobe = <42>;
> + ti,davinci-cs-wsetup = <14>;
> + };
...
> + nand@3,0 {
> + compatible = "ti,davinci-nand";
> + reg = <3 0x0 0x807ff
> + 6 0x0 0x8000>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + .. See Documentation/devicetree/bindings/mtd/davinci-nand.txt
> + };
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
