Re: [RFC PATCH 1/2] memory: davinci - add aemif controller platform driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/02/2012 03:05 PM, Stephen Warren wrote:
On 11/02/2012 10:21 AM, 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 controller driver and driver for other async
devices such as NOR flash, ASRAM etc, the bus confuguration is
done by this driver at init time. A set of APIs (set/get) provided to
update the values based on specific driver usage.
+++ b/Documentation/devicetree/bindings/arm/davinci/aemif.txt
If the HW/binding is generic, I'd assume the documentation would be
place somewhere more like
Documentation/devicetree/bindings/memory/davinci-aemif.txt?
Thanks for your comments.

I think this is a generic driver that should be re-usable across multiple architectures such as arm, c6x and other new SoCs from TI that uses the AEMIF IP. AEMIF IP can be configured on a per chip select basis. There will be nand controller, NOR flash and other async devices on this bus. So cs bindings for each of this device will be a sub node under thie aemif device. AEMIF driver iterate over the cs sub nodes and configure the bus.

Let me post the complete bindings in the next revision so that this will be more clear and we can discuss it based on that. I will also move the nand documentation to a generic place and refer the bindings inside the aemif documentation. I will address some of the comments below in my next revision of the patch, but some of the comments discussion will have to be deferred to version v2 of the patch.

Murali
@@ -0,0 +1,62 @@
+* Texas Instruments Davinci AEMIF bus interface
+
+This file provides information for the davinci-emif chip select
+bindings.
Shouldn't the binding be for an IP block (the AEMIF bus controller I
assume), rather than for a particular chip-select generated by the
controller?
AEMIF has multiple functions such as it functions as a NAND controller, it provides interfaces to async devices. The bus is configured using the CS sub node inside the aemif device node (compatible ti, davinci-emif).

+This is a sub device node inside the davinci-emif device node
+to describe a async bus for a specific chip select. For NAND,
+CFI flash device bindings described inside an aemif node,
+etc, a cs sub node is defined to associate the bus parameter
+bindings used by the device.
Oh, this file only documents part of the controller's node? It seems
unusual to do that; the documentation for an entire node would usually
be in a single file, which seems to be
Documentation/devicetree/bindings/arm/davinci/nand.txt right now. Is
this "cs" sub-node really something that gets re-used across multiple
different memory controller IPs?

If so, I guess this file should be called something more like
davinci-aemif-cs.txt than davinci-aemif.txt. I'd suggest moving
arm/davinci/nand.txt into a common location too (and renaming it to
davici-nand.txt to better represent the compatible value it documents).

+Required properties:=
+- compatible: "ti,davinci-cs";
+- #address-cells = <1>;
+- #size-cells = <1>;
+- cs - cs used by the device (NAND, CFI flash etc. values in the range: 2-5
+
+Optional properties:-
+- asize - asynchronous data bus width (0 - 8bit, 1 - 16 bit)
+  All of the params below in nanoseconds
+
+- ta - Minimum turn around time
+- rhold - read hold width
+- rstobe - read strobe width
+- rsetup - read setup width
+- whold - write hold width
+- wstrobe - write strobe width
+- wsetup - write setup width
+- ss - enable/disable select strobe (0 - disable, 1 - enable)
+- ew - enable/disable extended wait cycles (0 - disable, 1 - enable)
I assume all of those are pretty custom to this binding, and not
something you'd expect to re-use across multiple vendors' bindings? If
so, shouldn't they have a "ti," vendor prefix?

+Example for davinci nand chip select
+
+aemif@60000000 {
+
+	compatible = "ti,davinci-aemif";
You need a reg property here.

+	#address-cells = <2>;
+	#size-cells = <1>;
+
+	nand_cs:cs2@70000000 {
+		compatible = "ti,davinci-cs";
You need a reg property here. The unit address "@70000000" in the node
name only has one address cell, whereas the parent node sets
#address-cells = <2>.

--
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


[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux