On 11/06/2012 02:47 PM, Murali Karicheri wrote:I have re-used the bindings from another patch and don't know why the above description is used.This is a platform driver for asynchronous external memory interfaceWhat about "how large each chipselect can be" determines 2-vs-3
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.
(address) or 1-vs-2 (size) cells? I assume it's 32-vs-64-bit bus? It
might be best to make that explicit.
Ok+- reg : contains offset/length value for AEMIF control registers spaceadd "s" at the end of that line.
+- ranges : Each range corresponds to a single chipselect, and cover
Ok. Will fix,+ the entire access window as configured.Hmmm. That's two different kinds of children, which appear to use
+
+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.
different addressing schemes given the examples below; more on that below.
+Optional cs node properties:-Perhaps s/asize/bus-width/? Also, directly using values 8 and 16 would
+- 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)
be a lot more obvious to read than 0 and 1.
Ok.
+- ti,davinci-cs-ta - Minimum turn around timeThe comments in the examples indicate these are in nS. This should be
+- 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
mentioned here instead I think.
aemif driver clk is either specified as a clk device node or as a device bindings. Aemif driver gets the clk rate and do the conversion of ns to emif clk cycles internally in the driver.
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?
Actually this driver has to work on various platforms some of them set values in boot loader, others explicitly specify it in bindings or platform data. If set in boot loader, these bindings are not required (so that it is compatible with old implementation) to be configured in Linux kernel. So I believe I should be using something like+- ti,davinci-cs-ss - enable/disable select strobe (0 - disable, 1 - enable)Boolean properties are usually represented as present (on/enabled/1) or
+- ti,davinci-cs-ew - enable/disable extended wait cycles (0 - disable, 1 - enable)
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 thatimplied they're really tri-state (explicitly off or on, or just not
+set by a boot loader are used.
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.
So let me drop the unit address as it is not needed for the cs node. The AEMIF control register address is specified by the parent reg property and is common to all chip selects.+Example for aemif, davinci nand and nor flash chip select shown below.This node has no reg property, but it needs one if you use a unit
+
+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 {
address ("@60000000") in the node name.
The numbering scheme unit address above doesn't seem to match theTo make this simple, I will drop the unit address from cs node and represent it as
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?
...+ 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
+ };