[RFC v2 PATCH 0/2]Move AEMIF driver out of DaVinci machine to memory subsystem

[RFC v2 PATCH 0/2]Move AEMIF driver out of DaVinci machine to memory subsystem

[Murali Karicheri]

Some of the comments addressed in this patch vs initial version are
 1. moved the bindings documentation to bindings/memory/davinci-aemif.txt
 2. moved the bindings documentation for nand to mtd/ folder
 3. renamed cs subnode fields with a ti,davinci-cs prefix.
 4. davinci-aemif.txt now has full picture of aemif async bus bindings
    and that of all of the sub device nodes and sub nodes
 5. Made cs node optional so that on platforms that sets the cs
    parameters in bootloader can ignore the cs binding nodes so that
    hardware values set by boot loader is used.

The DaVinci AEMIF (asynchronous external memory interface) is used on other
TI SoCs that are not DaVinci based. So the AEMIF driver is to be moved
outside mach-davinci to the drivers folder so that it can be re-used on other
TI SoCs. Also migrate the DaVinci NAND driver to use the new aemif API.

Some of these code has been borrowed from intial patch from Heiko Schocher
 <hs@denx.de>. So I have added his name in the Copyright for davinci-aemif.c
This is an RFC to get the intial response so that all the platforms can
be migrated to use this driver.

Murali Karicheri (2):
  memory: davinci - add aemif controller platform driver
  mtd: davinci - remove DaVinci architecture depedency

 .../devicetree/bindings/arm/davinci/nand.txt       |   59 ---
 .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
 .../devicetree/bindings/mtd/davinci-nand.txt       |   59 +++
 drivers/memory/Kconfig                             |   10 +
 drivers/memory/Makefile                            |    1 +
 drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
 drivers/mtd/nand/Kconfig                           |    6 +-
 drivers/mtd/nand/davinci_nand.c                    |   40 +-
 include/linux/platform_data/davinci-aemif.h        |   47 ++
 include/linux/platform_data/davinci-nand.h         |   87 ++++
 10 files changed, 806 insertions(+), 85 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
 create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
 create mode 100644 drivers/memory/davinci-aemif.c
 create mode 100644 include/linux/platform_data/davinci-aemif.h
 create mode 100644 include/linux/platform_data/davinci-nand.h

-- 
1.7.9.5

[RFC v2 PATCH 0/2]Move AEMIF driver out of DaVinci machine to memory subsystem

[Murali Karicheri]

Some of the comments addressed in this patch vs initial version are
 1. moved the bindings documentation to bindings/memory/davinci-aemif.txt
 2. moved the bindings documentation for nand to mtd/ folder
 3. renamed cs subnode fields with a ti,davinci-cs prefix.
 4. davinci-aemif.txt now has full picture of aemif async bus bindings
    and that of all of the sub device nodes and sub nodes
 5. Made cs node optional so that on platforms that sets the cs
    parameters in bootloader can ignore the cs binding nodes so that
    hardware values set by boot loader is used.

The DaVinci AEMIF (asynchronous external memory interface) is used on other
TI SoCs that are not DaVinci based. So the AEMIF driver is to be moved
outside mach-davinci to the drivers folder so that it can be re-used on other
TI SoCs. Also migrate the DaVinci NAND driver to use the new aemif API.

Some of these code has been borrowed from intial patch from Heiko Schocher
 <hs-ynQEQJNshbs@public.gmane.org>. So I have added his name in the Copyright for davinci-aemif.c
This is an RFC to get the intial response so that all the platforms can
be migrated to use this driver.

Murali Karicheri (2):
  memory: davinci - add aemif controller platform driver
  mtd: davinci - remove DaVinci architecture depedency

 .../devicetree/bindings/arm/davinci/nand.txt       |   59 ---
 .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
 .../devicetree/bindings/mtd/davinci-nand.txt       |   59 +++
 drivers/memory/Kconfig                             |   10 +
 drivers/memory/Makefile                            |    1 +
 drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
 drivers/mtd/nand/Kconfig                           |    6 +-
 drivers/mtd/nand/davinci_nand.c                    |   40 +-
 include/linux/platform_data/davinci-aemif.h        |   47 ++
 include/linux/platform_data/davinci-nand.h         |   87 ++++
 10 files changed, 806 insertions(+), 85 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
 create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
 create mode 100644 drivers/memory/davinci-aemif.c
 create mode 100644 include/linux/platform_data/davinci-aemif.h
 create mode 100644 include/linux/platform_data/davinci-nand.h

-- 
1.7.9.5

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

[Murali Karicheri]

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.

This supports configuration of the bus either through platform_data or
through DT bindings.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
 drivers/memory/Kconfig                             |   10 +
 drivers/memory/Makefile                            |    1 +
 drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
 include/linux/platform_data/davinci-aemif.h        |   47 ++
 5 files changed, 640 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
 create mode 100644 drivers/memory/davinci-aemif.c
 create mode 100644 include/linux/platform_data/davinci-aemif.h

diff --git a/Documentation/devicetree/bindings/memory/davinci-aemif.txt b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
new file mode 100644
index 0000000..a79b3ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
@@ -0,0 +1,103 @@
+* 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.
+- reg : contains offset/length value for AEMIF control registers space
+- ranges : Each range corresponds to a single chipselect, and cover
+           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.
+
+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)
+- 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
+- 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)
+
+if any of the above parameters are absent, hardware register default or that
+set by a boot loader are used.
+
+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 {
+		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>;
+	};
+
+	nor_cs:cs3@62000000 {
+		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>;
+		ti,davinci-cs-asize = <1>;
+	};
+
+	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
+	};
+
+	flash@2,0 {
+		compatible = "cfi-flash";
+		reg = <2 0x0 0x400000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		bank-width = <2>;
+		device-width = <2>;
+	};
+};
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 067f311..2636a95 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -40,4 +40,14 @@ config TEGRA30_MC
 	  analysis, especially for IOMMU/SMMU(System Memory Management
 	  Unit) module.
 
+config TI_DAVINCI_AEMIF
+	bool "Texas Instruments DaVinci AEMIF driver"
+	help
+	  This driver is for the AEMIF module available in Texas Instruments
+	  SoCs. AEMIF stands for Asynchronous External Memory Interface and
+	  is intended to provide a glue-less interface to a variety of
+	  asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
+	  of 256M bytes of any of these memories can be accessed at a given
+	  time via four chip selects with 64M byte access per chip select.
+
 endif
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 42b3ce9..246aa61 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -5,3 +5,4 @@
 obj-$(CONFIG_TI_EMIF)		+= emif.o
 obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
 obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
+obj-$(CONFIG_TI_DAVINCI_AEMIF)	+= davinci-aemif.o
diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
new file mode 100644
index 0000000..27a6995
--- /dev/null
+++ b/drivers/memory/davinci-aemif.c
@@ -0,0 +1,479 @@
+/*
+ * AEMIF support for DaVinci SoCs
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.com/
+ * Copyright (C) Heiko Schocher <hs@denx.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_data/davinci-aemif.h>
+#include <linux/platform_device.h>
+#include <linux/time.h>
+
+#define TA_SHIFT	2
+#define RHOLD_SHIFT	4
+#define RSTROBE_SHIFT	7
+#define RSETUP_SHIFT	13
+#define WHOLD_SHIFT	17
+#define WSTROBE_SHIFT	20
+#define WSETUP_SHIFT	26
+#define EW_SHIFT	30
+#define SS_SHIFT	31
+
+#define TA(x)		((x) << TA_SHIFT)
+#define RHOLD(x)	((x) << RHOLD_SHIFT)
+#define RSTROBE(x)	((x) << RSTROBE_SHIFT)
+#define RSETUP(x)	((x) << RSETUP_SHIFT)
+#define WHOLD(x)	((x) << WHOLD_SHIFT)
+#define WSTROBE(x)	((x) << WSTROBE_SHIFT)
+#define WSETUP(x)	((x) << WSETUP_SHIFT)
+#define EW(x)		((x) << EW_SHIFT)
+#define SS(x)		((x) << SS_SHIFT)
+
+#define ASIZE_MAX	0x1
+#define TA_MAX		0x3
+#define RHOLD_MAX	0x7
+#define RSTROBE_MAX	0x3f
+#define RSETUP_MAX	0xf
+#define WHOLD_MAX	0x7
+#define WSTROBE_MAX	0x3f
+#define WSETUP_MAX	0xf
+#define EW_MAX		0x1
+#define SS_MAX		0x1
+#define NUM_CS		4
+
+#define TA_VAL(x)	(((x) & TA(TA_MAX)) >> TA_SHIFT)
+#define RHOLD_VAL(x)	(((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
+#define RSTROBE_VAL(x)	(((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
+#define RSETUP_VAL(x)	(((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
+#define WHOLD_VAL(x)	(((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
+#define WSTROBE_VAL(x)	(((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
+#define WSETUP_VAL(x)	(((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
+#define EW_VAL(x)	(((x) & EW(EW_MAX)) >> EW_SHIFT)
+#define SS_VAL(x)	(((x) & SS(SS_MAX)) >> SS_SHIFT)
+
+
+#define CONFIG_MASK	(TA(TA_MAX) | \
+				RHOLD(RHOLD_MAX) | \
+				RSTROBE(RSTROBE_MAX) |	\
+				RSETUP(RSETUP_MAX) | \
+				WHOLD(WHOLD_MAX) | \
+				WSTROBE(WSTROBE_MAX) | \
+				WSETUP(WSETUP_MAX) | \
+				EW(EW_MAX) | SS(SS_MAX) | \
+				ASIZE_MAX)
+
+#define DRV_NAME "davinci-aemif"
+
+struct aemif_device {
+	struct davinci_aemif_pdata *cfg;
+	void __iomem *base;
+	struct clk *clk;
+	/* clock rate in KHz */
+	unsigned long clk_rate;
+};
+
+static struct aemif_device *aemif;
+/**
+ * aemif_calc_rate - calculate timing data.
+ * @wanted: The cycle time needed in nanoseconds.
+ * @clk: The input clock rate in kHz.
+ * @max: The maximum divider value that can be programmed.
+ *
+ * On success, returns the calculated timing value minus 1 for easy
+ * programming into AEMIF timing registers, else negative errno.
+ */
+static int aemif_calc_rate(int wanted, unsigned long clk, int max)
+{
+	int result;
+
+	result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
+
+	pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, wanted);
+
+	/* It is generally OK to have a more relaxed timing than requested... */
+	if (result < 0)
+		result = 0;
+
+	/* ... But configuring tighter timings is not an option. */
+	else if (result > max)
+		result = -EINVAL;
+
+	return result;
+}
+
+/**
+ * davinci_aemif_config_abus - configure async bus parameters given
+ * AEMIF interface
+ * @cs: chip-select to program the timing values for
+ * @data: aemif chip select configuration
+ * @base: aemif io mapped configuration base
+ *
+ * This function programs the given timing values (in real clock) into the
+ * AEMIF registers taking the AEMIF clock into account.
+ *
+ * This function does not use any locking while programming the AEMIF
+ * because it is expected that there is only one user of a given
+ * chip-select.
+ *
+ * Returns 0 on success, else negative errno.
+ */
+static int davinci_aemif_config_abus(unsigned int cs,
+				void __iomem *base,
+				struct davinci_aemif_cs_data *data)
+{
+	int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
+	unsigned offset = A1CR_OFFSET + cs * 4;
+	u32 set, val;
+
+	if (!data)
+		return -EINVAL;
+
+	ta	= aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
+	rhold	= aemif_calc_rate(data->rhold, aemif->clk_rate, RHOLD_MAX);
+	rstrobe	= aemif_calc_rate(data->rstrobe, aemif->clk_rate, RSTROBE_MAX);
+	rsetup	= aemif_calc_rate(data->rsetup, aemif->clk_rate, RSETUP_MAX);
+	whold	= aemif_calc_rate(data->whold, aemif->clk_rate, WHOLD_MAX);
+	wstrobe	= aemif_calc_rate(data->wstrobe, aemif->clk_rate, WSTROBE_MAX);
+	wsetup	= aemif_calc_rate(data->wsetup, aemif->clk_rate, WSETUP_MAX);
+
+	if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
+			whold < 0 || wstrobe < 0 || wsetup < 0) {
+		pr_err("%s: cannot get suitable timings\n", __func__);
+		return -EINVAL;
+	}
+
+	set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
+		WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
+
+	set |= (data->asize & ACR_ASIZE_MASK);
+	if (data->enable_ew)
+		set |= ACR_EW_MASK;
+	if (data->enable_ss)
+		set |= ACR_SS_MASK;
+
+	val = readl(aemif->base + offset);
+	val &= ~CONFIG_MASK;
+	val |= set;
+	writel(val, aemif->base + offset);
+
+	return 0;
+}
+
+inline int aemif_cycles_to_nsec(int val)
+{
+	return (val * NSEC_PER_MSEC) / aemif->clk_rate;
+}
+
+/**
+ * davinci_aemif_get_hw_params - function to read hw register values
+ * @cs: chip select
+ * @data: ptr to cs data
+ *
+ * This function reads the defaults from the registers and update
+ * the timing values. Required for get/set commands and also for
+ * the case when driver needs to use defaults in hardware.
+ */
+static void davinci_aemif_get_hw_params(int cs,
+		struct davinci_aemif_cs_data *data)
+{
+	u32 val, offset = A1CR_OFFSET + cs * 4;
+
+	val = readl(aemif->base + offset);
+	data->ta = aemif_cycles_to_nsec(TA_VAL(val));
+	data->rhold = aemif_cycles_to_nsec(RHOLD_VAL(val));
+	data->rstrobe = aemif_cycles_to_nsec(RSTROBE_VAL(val));
+	data->rsetup = aemif_cycles_to_nsec(RSETUP_VAL(val));
+	data->whold = aemif_cycles_to_nsec(WHOLD_VAL(val));
+	data->wstrobe = aemif_cycles_to_nsec(WSTROBE_VAL(val));
+	data->wsetup = aemif_cycles_to_nsec(WSETUP_VAL(val));
+	data->enable_ew = EW_VAL(val);
+	data->enable_ss = SS_VAL(val);
+	data->asize = val & ASIZE_MAX;
+}
+
+/**
+ * get_cs_data - helper function to get bus configuration data for a given cs
+ * @cs: chip-select, values 2-5
+ */
+static struct davinci_aemif_cs_data *get_cs_data(int cs)
+{
+	int i;
+
+	for (i = 0; i < aemif->cfg->num_cs; i++) {
+		if (cs == aemif->cfg->cs_data[i].cs)
+			break;
+	}
+
+	if (i == aemif->cfg->num_cs)
+		return NULL;
+
+	return &aemif->cfg->cs_data[i];
+}
+
+/**
+ * davinci_aemif_set_abus_params - Set bus configuration data for a given cs
+ * @cs: chip-select, values 2-5
+ * @data: ptr to a struct to hold the configuration data to be set
+ *
+ * This function is called to configure emif bus parameters for a given cs.
+ * Users call this function after calling davinci_aemif_get_abus_params()
+ * to get current parameters, modify and call this function
+ */
+int davinci_aemif_set_abus_params(unsigned int cs,
+			struct davinci_aemif_cs_data *data)
+{
+	struct davinci_aemif_cs_data *curr_cs_data;
+	int ret = -EINVAL, chip_cs;
+
+	if (data == NULL)
+		return ret;
+
+	if (aemif->base == NULL)
+		return ret;
+
+	/* translate to chip CS which starts at 2 */
+	chip_cs = cs + 2;
+
+	curr_cs_data = get_cs_data(chip_cs);
+	if (curr_cs_data) {
+		/* for configuration we use cs since it is used to index ACR */
+		ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
+		if (!ret) {
+			*curr_cs_data = *data;
+			return 0;
+		}
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(davinci_aemif_set_abus_params);
+
+/**
+ * davinci_aemif_get_abus_params - Get bus configuration data for a given cs
+ * @cs: chip-select, values 2-5
+ * returns: ptr to a struct having the current configuration data
+ */
+struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs)
+{
+	/* translate to chip CS which starts at 2 */
+	return get_cs_data(cs + 2);
+}
+EXPORT_SYMBOL(davinci_aemif_get_abus_params);
+
+#if defined(CONFIG_OF)
+/**
+ * dv_get_value - helper function to read a attribute valye
+ * @np: device node ptr
+ * @name: name of the attribute
+ * returns: value of the attribute
+ */
+static int dv_get_value(struct device_node *np, const char *name)
+{
+	u32 data;
+	int ret = -EINVAL;
+
+	ret = of_property_read_u32(np, name, &data);
+	if (ret != 0)
+		return ret;
+
+	return data;
+}
+
+/**
+ * of_davinci_aemif_parse_abus_config - parse bus config data from a cs node
+ * @np: device node ptr
+ *
+ * This function update the emif async bus configuration based on the values
+ * configured in a cs device binding node.
+ */
+static int of_davinci_aemif_parse_abus_config(struct device_node *np)
+{
+	struct davinci_aemif_cs_data *data;
+	unsigned long cs;
+	int val;
+
+	if (kstrtoul(np->name + 2, 10, &cs) < 0)
+		return -EINVAL;
+
+	if (cs < 2 || cs >= NUM_CS)
+		return -EINVAL;
+
+	if (aemif->cfg->num_cs >= NUM_CS)
+		return -EINVAL;
+
+	data = &aemif->cfg->cs_data[aemif->cfg->num_cs++];
+	data->cs = cs;
+
+	/* read the current value in the hw register */
+	davinci_aemif_get_hw_params(cs - 2, data);
+
+	/* override the values from device node */
+	val = dv_get_value(np, "davinci-cs-ta");
+	if (val >= 0)
+		data->ta = val;
+	val = dv_get_value(np, "davinci-cs-rhold");
+	if (val >= 0)
+		data->rhold	= val;
+	val = dv_get_value(np, "davinci-cs-rstrobe");
+	if (val >= 0)
+		data->rstrobe = val;
+	val = dv_get_value(np, "davinci-cs-rsetup");
+	if (val >= 0)
+		data->rsetup = val;
+	val = dv_get_value(np, "davinci-cs-whold");
+	if (val >= 0)
+		data->whold = val;
+	val = dv_get_value(np, "davinci-cs-wstrobe");
+	if (val >= 0)
+		data->wstrobe = val;
+	val = dv_get_value(np, "davinci-cs-wsetup");
+	if (val >= 0)
+		data->wsetup = val;
+	val = dv_get_value(np, "davinci-cs-asize");
+	if (val >= 0)
+		data->asize = val;
+	val = dv_get_value(np, "davinci-cs-ew");
+	if (val >= 0)
+		data->enable_ew	= val;
+	val = dv_get_value(np, "davinci-cs-ss");
+	if (val >= 0)
+		data->enable_ss	= val;
+	return 0;
+}
+#endif
+
+static const struct of_device_id davinci_aemif_of_match[] __devinitconst = {
+	{ .compatible = "ti,davinci-aemif", },
+	{},
+};
+
+static const struct of_device_id davinci_cs_of_match[] __devinitconst = {
+	{ .compatible = "ti,davinci-cs", },
+	{},
+};
+
+/**
+ * of_davinci_aemif_cs_init - init cs data based on cs device nodes
+ * @np: device node ptr
+ *
+ * For every controller device node, there is a cs device node that
+ * describe the bus configuration parameters. This functions iterate
+ * over these nodes and update the cs data array.
+ */
+static int of_davinci_aemif_cs_init(struct device_node *aemif_np)
+{
+	struct device_node *np = aemif_np;
+
+	/* cs nodes are optional. So just return success */
+	if (!np)
+		return 0;
+
+	for_each_matching_node(np, davinci_cs_of_match)
+		of_davinci_aemif_parse_abus_config(np);
+	return 0;
+}
+
+static int __devinit davinci_aemif_probe(struct platform_device *pdev)
+{
+	struct davinci_aemif_pdata *cfg;
+	int ret  = -ENODEV, i;
+	struct resource *res;
+
+	aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
+
+	if (!aemif)
+		return -ENOMEM;
+
+	aemif->clk = clk_get(NULL, "aemif");
+	if (IS_ERR(aemif->clk))
+		return PTR_ERR(aemif->clk);
+
+	clk_prepare_enable(aemif->clk);
+	aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		pr_err("No IO memory address defined\n");
+		goto error;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	aemif->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!aemif->base) {
+		ret = -EBUSY;
+		pr_err("ioremap failed\n");
+		goto error;
+	}
+
+	if (pdev->dev.platform_data == NULL) {
+		/* Not platform data, we get the cs data from the cs nodes */
+		cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
+		if (cfg == NULL)
+			return -ENOMEM;
+
+		aemif->cfg = cfg;
+		if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
+			pr_err("No platform data or cs of node present\n");
+			goto error;
+		}
+	} else {
+		cfg = pdev->dev.platform_data;
+		aemif->cfg = cfg;
+	}
+
+	for (i = 0; i < cfg->num_cs; i++) {
+		/* cs is from 2-5. Internally we use cs-2 to access ACR */
+		ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
+				aemif->base, &cfg->cs_data[i]);
+		if (ret < 0) {
+			pr_err("Error configuring chip select %d\n",
+				cfg->cs_data[i].cs);
+			goto error;
+		}
+	}
+	return 0;
+error:
+	clk_disable_unprepare(aemif->clk);
+	clk_put(aemif->clk);
+	return ret;
+}
+
+static struct platform_driver davinci_aemif_driver = {
+	.probe = davinci_aemif_probe,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = davinci_aemif_of_match,
+	},
+};
+
+static int __init davinci_aemif_init(void)
+{
+	return platform_driver_register(&davinci_aemif_driver);
+}
+subsys_initcall(davinci_aemif_init);
+
+static void __exit davinci_aemif_exit(void)
+{
+	clk_disable_unprepare(aemif->clk);
+	clk_put(aemif->clk);
+	platform_driver_unregister(&davinci_aemif_driver);
+}
+module_exit(davinci_aemif_exit);
+
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
+MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/platform_data/davinci-aemif.h b/include/linux/platform_data/davinci-aemif.h
new file mode 100644
index 0000000..03f3ad0
--- /dev/null
+++ b/include/linux/platform_data/davinci-aemif.h
@@ -0,0 +1,47 @@
+/*
+ * TI DaVinci AEMIF support
+ *
+ * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+#ifndef _MACH_DAVINCI_AEMIF_H
+#define _MACH_DAVINCI_AEMIF_H
+
+#define NRCSR_OFFSET		0x00
+#define AWCCR_OFFSET		0x04
+#define A1CR_OFFSET		0x10
+
+#define ACR_ASIZE_MASK		0x3
+#define ACR_EW_MASK		BIT(30)
+#define ACR_SS_MASK		BIT(31)
+#define ASIZE_16BIT		1
+
+struct davinci_aemif_cs_data {
+	u8	cs;
+	u16	wstrobe;
+	u16	rstrobe;
+	u8	wsetup;
+	u8	whold;
+	u8	rsetup;
+	u8	rhold;
+	u8	ta;
+	u8	enable_ss;
+	u8	enable_ew;
+	u8	asize;
+};
+
+struct davinci_aemif_pdata {
+	u8	num_cs;
+	struct davinci_aemif_cs_data cs_data[4];
+};
+
+/* API to Get current Asynchrnous emif bus parameters */
+struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs);
+
+/* API to Set current Asynchrnous emif bus parameters */
+int davinci_aemif_set_abus_params(unsigned int cs,
+			struct davinci_aemif_cs_data *data);
+#endif
-- 
1.7.9.5

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

[Murali Karicheri]

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.

This supports configuration of the bus either through platform_data or
through DT bindings.

Signed-off-by: Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>
---
 .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
 drivers/memory/Kconfig                             |   10 +
 drivers/memory/Makefile                            |    1 +
 drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
 include/linux/platform_data/davinci-aemif.h        |   47 ++
 5 files changed, 640 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
 create mode 100644 drivers/memory/davinci-aemif.c
 create mode 100644 include/linux/platform_data/davinci-aemif.h

diff --git a/Documentation/devicetree/bindings/memory/davinci-aemif.txt b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
new file mode 100644
index 0000000..a79b3ed
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
@@ -0,0 +1,103 @@
+* 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.
+- reg : contains offset/length value for AEMIF control registers space
+- ranges : Each range corresponds to a single chipselect, and cover
+           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.
+
+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)
+- 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
+- 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)
+
+if any of the above parameters are absent, hardware register default or that
+set by a boot loader are used.
+
+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 {
+		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>;
+	};
+
+	nor_cs:cs3@62000000 {
+		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>;
+		ti,davinci-cs-asize = <1>;
+	};
+
+	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
+	};
+
+	flash@2,0 {
+		compatible = "cfi-flash";
+		reg = <2 0x0 0x400000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		bank-width = <2>;
+		device-width = <2>;
+	};
+};
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 067f311..2636a95 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -40,4 +40,14 @@ config TEGRA30_MC
 	  analysis, especially for IOMMU/SMMU(System Memory Management
 	  Unit) module.
 
+config TI_DAVINCI_AEMIF
+	bool "Texas Instruments DaVinci AEMIF driver"
+	help
+	  This driver is for the AEMIF module available in Texas Instruments
+	  SoCs. AEMIF stands for Asynchronous External Memory Interface and
+	  is intended to provide a glue-less interface to a variety of
+	  asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
+	  of 256M bytes of any of these memories can be accessed at a given
+	  time via four chip selects with 64M byte access per chip select.
+
 endif
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 42b3ce9..246aa61 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -5,3 +5,4 @@
 obj-$(CONFIG_TI_EMIF)		+= emif.o
 obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
 obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
+obj-$(CONFIG_TI_DAVINCI_AEMIF)	+= davinci-aemif.o
diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
new file mode 100644
index 0000000..27a6995
--- /dev/null
+++ b/drivers/memory/davinci-aemif.c
@@ -0,0 +1,479 @@
+/*
+ * AEMIF support for DaVinci SoCs
+ *
+ * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.com/
+ * Copyright (C) Heiko Schocher <hs-ynQEQJNshbs@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_data/davinci-aemif.h>
+#include <linux/platform_device.h>
+#include <linux/time.h>
+
+#define TA_SHIFT	2
+#define RHOLD_SHIFT	4
+#define RSTROBE_SHIFT	7
+#define RSETUP_SHIFT	13
+#define WHOLD_SHIFT	17
+#define WSTROBE_SHIFT	20
+#define WSETUP_SHIFT	26
+#define EW_SHIFT	30
+#define SS_SHIFT	31
+
+#define TA(x)		((x) << TA_SHIFT)
+#define RHOLD(x)	((x) << RHOLD_SHIFT)
+#define RSTROBE(x)	((x) << RSTROBE_SHIFT)
+#define RSETUP(x)	((x) << RSETUP_SHIFT)
+#define WHOLD(x)	((x) << WHOLD_SHIFT)
+#define WSTROBE(x)	((x) << WSTROBE_SHIFT)
+#define WSETUP(x)	((x) << WSETUP_SHIFT)
+#define EW(x)		((x) << EW_SHIFT)
+#define SS(x)		((x) << SS_SHIFT)
+
+#define ASIZE_MAX	0x1
+#define TA_MAX		0x3
+#define RHOLD_MAX	0x7
+#define RSTROBE_MAX	0x3f
+#define RSETUP_MAX	0xf
+#define WHOLD_MAX	0x7
+#define WSTROBE_MAX	0x3f
+#define WSETUP_MAX	0xf
+#define EW_MAX		0x1
+#define SS_MAX		0x1
+#define NUM_CS		4
+
+#define TA_VAL(x)	(((x) & TA(TA_MAX)) >> TA_SHIFT)
+#define RHOLD_VAL(x)	(((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
+#define RSTROBE_VAL(x)	(((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
+#define RSETUP_VAL(x)	(((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
+#define WHOLD_VAL(x)	(((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
+#define WSTROBE_VAL(x)	(((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
+#define WSETUP_VAL(x)	(((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
+#define EW_VAL(x)	(((x) & EW(EW_MAX)) >> EW_SHIFT)
+#define SS_VAL(x)	(((x) & SS(SS_MAX)) >> SS_SHIFT)
+
+
+#define CONFIG_MASK	(TA(TA_MAX) | \
+				RHOLD(RHOLD_MAX) | \
+				RSTROBE(RSTROBE_MAX) |	\
+				RSETUP(RSETUP_MAX) | \
+				WHOLD(WHOLD_MAX) | \
+				WSTROBE(WSTROBE_MAX) | \
+				WSETUP(WSETUP_MAX) | \
+				EW(EW_MAX) | SS(SS_MAX) | \
+				ASIZE_MAX)
+
+#define DRV_NAME "davinci-aemif"
+
+struct aemif_device {
+	struct davinci_aemif_pdata *cfg;
+	void __iomem *base;
+	struct clk *clk;
+	/* clock rate in KHz */
+	unsigned long clk_rate;
+};
+
+static struct aemif_device *aemif;
+/**
+ * aemif_calc_rate - calculate timing data.
+ * @wanted: The cycle time needed in nanoseconds.
+ * @clk: The input clock rate in kHz.
+ * @max: The maximum divider value that can be programmed.
+ *
+ * On success, returns the calculated timing value minus 1 for easy
+ * programming into AEMIF timing registers, else negative errno.
+ */
+static int aemif_calc_rate(int wanted, unsigned long clk, int max)
+{
+	int result;
+
+	result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
+
+	pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, wanted);
+
+	/* It is generally OK to have a more relaxed timing than requested... */
+	if (result < 0)
+		result = 0;
+
+	/* ... But configuring tighter timings is not an option. */
+	else if (result > max)
+		result = -EINVAL;
+
+	return result;
+}
+
+/**
+ * davinci_aemif_config_abus - configure async bus parameters given
+ * AEMIF interface
+ * @cs: chip-select to program the timing values for
+ * @data: aemif chip select configuration
+ * @base: aemif io mapped configuration base
+ *
+ * This function programs the given timing values (in real clock) into the
+ * AEMIF registers taking the AEMIF clock into account.
+ *
+ * This function does not use any locking while programming the AEMIF
+ * because it is expected that there is only one user of a given
+ * chip-select.
+ *
+ * Returns 0 on success, else negative errno.
+ */
+static int davinci_aemif_config_abus(unsigned int cs,
+				void __iomem *base,
+				struct davinci_aemif_cs_data *data)
+{
+	int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
+	unsigned offset = A1CR_OFFSET + cs * 4;
+	u32 set, val;
+
+	if (!data)
+		return -EINVAL;
+
+	ta	= aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
+	rhold	= aemif_calc_rate(data->rhold, aemif->clk_rate, RHOLD_MAX);
+	rstrobe	= aemif_calc_rate(data->rstrobe, aemif->clk_rate, RSTROBE_MAX);
+	rsetup	= aemif_calc_rate(data->rsetup, aemif->clk_rate, RSETUP_MAX);
+	whold	= aemif_calc_rate(data->whold, aemif->clk_rate, WHOLD_MAX);
+	wstrobe	= aemif_calc_rate(data->wstrobe, aemif->clk_rate, WSTROBE_MAX);
+	wsetup	= aemif_calc_rate(data->wsetup, aemif->clk_rate, WSETUP_MAX);
+
+	if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
+			whold < 0 || wstrobe < 0 || wsetup < 0) {
+		pr_err("%s: cannot get suitable timings\n", __func__);
+		return -EINVAL;
+	}
+
+	set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
+		WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
+
+	set |= (data->asize & ACR_ASIZE_MASK);
+	if (data->enable_ew)
+		set |= ACR_EW_MASK;
+	if (data->enable_ss)
+		set |= ACR_SS_MASK;
+
+	val = readl(aemif->base + offset);
+	val &= ~CONFIG_MASK;
+	val |= set;
+	writel(val, aemif->base + offset);
+
+	return 0;
+}
+
+inline int aemif_cycles_to_nsec(int val)
+{
+	return (val * NSEC_PER_MSEC) / aemif->clk_rate;
+}
+
+/**
+ * davinci_aemif_get_hw_params - function to read hw register values
+ * @cs: chip select
+ * @data: ptr to cs data
+ *
+ * This function reads the defaults from the registers and update
+ * the timing values. Required for get/set commands and also for
+ * the case when driver needs to use defaults in hardware.
+ */
+static void davinci_aemif_get_hw_params(int cs,
+		struct davinci_aemif_cs_data *data)
+{
+	u32 val, offset = A1CR_OFFSET + cs * 4;
+
+	val = readl(aemif->base + offset);
+	data->ta = aemif_cycles_to_nsec(TA_VAL(val));
+	data->rhold = aemif_cycles_to_nsec(RHOLD_VAL(val));
+	data->rstrobe = aemif_cycles_to_nsec(RSTROBE_VAL(val));
+	data->rsetup = aemif_cycles_to_nsec(RSETUP_VAL(val));
+	data->whold = aemif_cycles_to_nsec(WHOLD_VAL(val));
+	data->wstrobe = aemif_cycles_to_nsec(WSTROBE_VAL(val));
+	data->wsetup = aemif_cycles_to_nsec(WSETUP_VAL(val));
+	data->enable_ew = EW_VAL(val);
+	data->enable_ss = SS_VAL(val);
+	data->asize = val & ASIZE_MAX;
+}
+
+/**
+ * get_cs_data - helper function to get bus configuration data for a given cs
+ * @cs: chip-select, values 2-5
+ */
+static struct davinci_aemif_cs_data *get_cs_data(int cs)
+{
+	int i;
+
+	for (i = 0; i < aemif->cfg->num_cs; i++) {
+		if (cs == aemif->cfg->cs_data[i].cs)
+			break;
+	}
+
+	if (i == aemif->cfg->num_cs)
+		return NULL;
+
+	return &aemif->cfg->cs_data[i];
+}
+
+/**
+ * davinci_aemif_set_abus_params - Set bus configuration data for a given cs
+ * @cs: chip-select, values 2-5
+ * @data: ptr to a struct to hold the configuration data to be set
+ *
+ * This function is called to configure emif bus parameters for a given cs.
+ * Users call this function after calling davinci_aemif_get_abus_params()
+ * to get current parameters, modify and call this function
+ */
+int davinci_aemif_set_abus_params(unsigned int cs,
+			struct davinci_aemif_cs_data *data)
+{
+	struct davinci_aemif_cs_data *curr_cs_data;
+	int ret = -EINVAL, chip_cs;
+
+	if (data == NULL)
+		return ret;
+
+	if (aemif->base == NULL)
+		return ret;
+
+	/* translate to chip CS which starts at 2 */
+	chip_cs = cs + 2;
+
+	curr_cs_data = get_cs_data(chip_cs);
+	if (curr_cs_data) {
+		/* for configuration we use cs since it is used to index ACR */
+		ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
+		if (!ret) {
+			*curr_cs_data = *data;
+			return 0;
+		}
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(davinci_aemif_set_abus_params);
+
+/**
+ * davinci_aemif_get_abus_params - Get bus configuration data for a given cs
+ * @cs: chip-select, values 2-5
+ * returns: ptr to a struct having the current configuration data
+ */
+struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs)
+{
+	/* translate to chip CS which starts at 2 */
+	return get_cs_data(cs + 2);
+}
+EXPORT_SYMBOL(davinci_aemif_get_abus_params);
+
+#if defined(CONFIG_OF)
+/**
+ * dv_get_value - helper function to read a attribute valye
+ * @np: device node ptr
+ * @name: name of the attribute
+ * returns: value of the attribute
+ */
+static int dv_get_value(struct device_node *np, const char *name)
+{
+	u32 data;
+	int ret = -EINVAL;
+
+	ret = of_property_read_u32(np, name, &data);
+	if (ret != 0)
+		return ret;
+
+	return data;
+}
+
+/**
+ * of_davinci_aemif_parse_abus_config - parse bus config data from a cs node
+ * @np: device node ptr
+ *
+ * This function update the emif async bus configuration based on the values
+ * configured in a cs device binding node.
+ */
+static int of_davinci_aemif_parse_abus_config(struct device_node *np)
+{
+	struct davinci_aemif_cs_data *data;
+	unsigned long cs;
+	int val;
+
+	if (kstrtoul(np->name + 2, 10, &cs) < 0)
+		return -EINVAL;
+
+	if (cs < 2 || cs >= NUM_CS)
+		return -EINVAL;
+
+	if (aemif->cfg->num_cs >= NUM_CS)
+		return -EINVAL;
+
+	data = &aemif->cfg->cs_data[aemif->cfg->num_cs++];
+	data->cs = cs;
+
+	/* read the current value in the hw register */
+	davinci_aemif_get_hw_params(cs - 2, data);
+
+	/* override the values from device node */
+	val = dv_get_value(np, "davinci-cs-ta");
+	if (val >= 0)
+		data->ta = val;
+	val = dv_get_value(np, "davinci-cs-rhold");
+	if (val >= 0)
+		data->rhold	= val;
+	val = dv_get_value(np, "davinci-cs-rstrobe");
+	if (val >= 0)
+		data->rstrobe = val;
+	val = dv_get_value(np, "davinci-cs-rsetup");
+	if (val >= 0)
+		data->rsetup = val;
+	val = dv_get_value(np, "davinci-cs-whold");
+	if (val >= 0)
+		data->whold = val;
+	val = dv_get_value(np, "davinci-cs-wstrobe");
+	if (val >= 0)
+		data->wstrobe = val;
+	val = dv_get_value(np, "davinci-cs-wsetup");
+	if (val >= 0)
+		data->wsetup = val;
+	val = dv_get_value(np, "davinci-cs-asize");
+	if (val >= 0)
+		data->asize = val;
+	val = dv_get_value(np, "davinci-cs-ew");
+	if (val >= 0)
+		data->enable_ew	= val;
+	val = dv_get_value(np, "davinci-cs-ss");
+	if (val >= 0)
+		data->enable_ss	= val;
+	return 0;
+}
+#endif
+
+static const struct of_device_id davinci_aemif_of_match[] __devinitconst = {
+	{ .compatible = "ti,davinci-aemif", },
+	{},
+};
+
+static const struct of_device_id davinci_cs_of_match[] __devinitconst = {
+	{ .compatible = "ti,davinci-cs", },
+	{},
+};
+
+/**
+ * of_davinci_aemif_cs_init - init cs data based on cs device nodes
+ * @np: device node ptr
+ *
+ * For every controller device node, there is a cs device node that
+ * describe the bus configuration parameters. This functions iterate
+ * over these nodes and update the cs data array.
+ */
+static int of_davinci_aemif_cs_init(struct device_node *aemif_np)
+{
+	struct device_node *np = aemif_np;
+
+	/* cs nodes are optional. So just return success */
+	if (!np)
+		return 0;
+
+	for_each_matching_node(np, davinci_cs_of_match)
+		of_davinci_aemif_parse_abus_config(np);
+	return 0;
+}
+
+static int __devinit davinci_aemif_probe(struct platform_device *pdev)
+{
+	struct davinci_aemif_pdata *cfg;
+	int ret  = -ENODEV, i;
+	struct resource *res;
+
+	aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
+
+	if (!aemif)
+		return -ENOMEM;
+
+	aemif->clk = clk_get(NULL, "aemif");
+	if (IS_ERR(aemif->clk))
+		return PTR_ERR(aemif->clk);
+
+	clk_prepare_enable(aemif->clk);
+	aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		pr_err("No IO memory address defined\n");
+		goto error;
+	}
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	aemif->base = devm_request_and_ioremap(&pdev->dev, res);
+	if (!aemif->base) {
+		ret = -EBUSY;
+		pr_err("ioremap failed\n");
+		goto error;
+	}
+
+	if (pdev->dev.platform_data == NULL) {
+		/* Not platform data, we get the cs data from the cs nodes */
+		cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
+		if (cfg == NULL)
+			return -ENOMEM;
+
+		aemif->cfg = cfg;
+		if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
+			pr_err("No platform data or cs of node present\n");
+			goto error;
+		}
+	} else {
+		cfg = pdev->dev.platform_data;
+		aemif->cfg = cfg;
+	}
+
+	for (i = 0; i < cfg->num_cs; i++) {
+		/* cs is from 2-5. Internally we use cs-2 to access ACR */
+		ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
+				aemif->base, &cfg->cs_data[i]);
+		if (ret < 0) {
+			pr_err("Error configuring chip select %d\n",
+				cfg->cs_data[i].cs);
+			goto error;
+		}
+	}
+	return 0;
+error:
+	clk_disable_unprepare(aemif->clk);
+	clk_put(aemif->clk);
+	return ret;
+}
+
+static struct platform_driver davinci_aemif_driver = {
+	.probe = davinci_aemif_probe,
+	.driver = {
+		.name = DRV_NAME,
+		.owner = THIS_MODULE,
+		.of_match_table = davinci_aemif_of_match,
+	},
+};
+
+static int __init davinci_aemif_init(void)
+{
+	return platform_driver_register(&davinci_aemif_driver);
+}
+subsys_initcall(davinci_aemif_init);
+
+static void __exit davinci_aemif_exit(void)
+{
+	clk_disable_unprepare(aemif->clk);
+	clk_put(aemif->clk);
+	platform_driver_unregister(&davinci_aemif_driver);
+}
+module_exit(davinci_aemif_exit);
+
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2-l0cyMroinI0@public.gmane.org>");
+MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/include/linux/platform_data/davinci-aemif.h b/include/linux/platform_data/davinci-aemif.h
new file mode 100644
index 0000000..03f3ad0
--- /dev/null
+++ b/include/linux/platform_data/davinci-aemif.h
@@ -0,0 +1,47 @@
+/*
+ * TI DaVinci AEMIF support
+ *
+ * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2. This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+#ifndef _MACH_DAVINCI_AEMIF_H
+#define _MACH_DAVINCI_AEMIF_H
+
+#define NRCSR_OFFSET		0x00
+#define AWCCR_OFFSET		0x04
+#define A1CR_OFFSET		0x10
+
+#define ACR_ASIZE_MASK		0x3
+#define ACR_EW_MASK		BIT(30)
+#define ACR_SS_MASK		BIT(31)
+#define ASIZE_16BIT		1
+
+struct davinci_aemif_cs_data {
+	u8	cs;
+	u16	wstrobe;
+	u16	rstrobe;
+	u8	wsetup;
+	u8	whold;
+	u8	rsetup;
+	u8	rhold;
+	u8	ta;
+	u8	enable_ss;
+	u8	enable_ew;
+	u8	asize;
+};
+
+struct davinci_aemif_pdata {
+	u8	num_cs;
+	struct davinci_aemif_cs_data cs_data[4];
+};
+
+/* API to Get current Asynchrnous emif bus parameters */
+struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs);
+
+/* API to Set current Asynchrnous emif bus parameters */
+int davinci_aemif_set_abus_params(unsigned int cs,
+			struct davinci_aemif_cs_data *data);
+#endif
-- 
1.7.9.5

[RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Murali Karicheri]

DaVinci NAND driver is a controller driver based on the AEMIF hardware
IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
patch removes the driver dependency on DaVinci architecture so that it
can be used on other architectures such as c6x, keystone etc.

Also migrate the driver to use the new AEMIF platform driver API and
moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
as this is expected to be used outside of arm/davinci.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 .../devicetree/bindings/arm/davinci/nand.txt       |   59 -------------
 .../devicetree/bindings/mtd/davinci-nand.txt       |   59 +++++++++++++
 drivers/mtd/nand/Kconfig                           |    6 +-
 drivers/mtd/nand/davinci_nand.c                    |   40 ++++-----
 include/linux/platform_data/davinci-nand.h         |   87 ++++++++++++++++++++
 5 files changed, 166 insertions(+), 85 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
 create mode 100644 include/linux/platform_data/davinci-nand.h

diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
deleted file mode 100644
index 4746452..0000000
--- a/Documentation/devicetree/bindings/arm/davinci/nand.txt
+++ /dev/null
@@ -1,59 +0,0 @@
-* 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
-- ti,davinci-chipselect: Indicates on the davinci_nand driver which
-                         chipselect is used for accessing the nand.
-
-Recommended properties :
-- ti,davinci-mask-ale: mask for ale
-- ti,davinci-mask-cle: mask for cle
-- ti,davinci-mask-chipsel: mask for chipselect
-- ti,davinci-ecc-mode: ECC mode valid values for davinci driver:
-		- "none"
-		- "soft"
-		- "hw"
-- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
-- ti,davinci-nand-buswidth: buswidth 8 or 16
-- ti,davinci-nand-use-bbt: use flash based bad block table support.
-
-nand device bindings may contain additional sub-nodes describing
-partitions of the address space. See partition.txt for more detail.
-
-Example (enbw_cmc board):
-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@3,0 {
-		compatible = "ti,davinci-nand";
-		reg = <3 0x0 0x807ff
-			6 0x0 0x8000>;
-		#address-cells = <1>;
-		#size-cells = <1>;
-		ti,davinci-chipselect = <1>;
-		ti,davinci-mask-ale = <0>;
-		ti,davinci-mask-cle = <0>;
-		ti,davinci-mask-chipsel = <0>;
-		ti,davinci-ecc-mode = "hw";
-		ti,davinci-ecc-bits = <4>;
-		ti,davinci-nand-use-bbt;
-
-		partition@180000 {
-			label = "ubifs";
-			reg = <0x180000 0x7e80000>;
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
new file mode 100644
index 0000000..4746452
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
@@ -0,0 +1,59 @@
+* 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
+- ti,davinci-chipselect: Indicates on the davinci_nand driver which
+                         chipselect is used for accessing the nand.
+
+Recommended properties :
+- ti,davinci-mask-ale: mask for ale
+- ti,davinci-mask-cle: mask for cle
+- ti,davinci-mask-chipsel: mask for chipselect
+- ti,davinci-ecc-mode: ECC mode valid values for davinci driver:
+		- "none"
+		- "soft"
+		- "hw"
+- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
+- ti,davinci-nand-buswidth: buswidth 8 or 16
+- ti,davinci-nand-use-bbt: use flash based bad block table support.
+
+nand device bindings may contain additional sub-nodes describing
+partitions of the address space. See partition.txt for more detail.
+
+Example (enbw_cmc board):
+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@3,0 {
+		compatible = "ti,davinci-nand";
+		reg = <3 0x0 0x807ff
+			6 0x0 0x8000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ti,davinci-chipselect = <1>;
+		ti,davinci-mask-ale = <0>;
+		ti,davinci-mask-cle = <0>;
+		ti,davinci-mask-chipsel = <0>;
+		ti,davinci-ecc-mode = "hw";
+		ti,davinci-ecc-bits = <4>;
+		ti,davinci-nand-use-bbt;
+
+		partition@180000 {
+			label = "ubifs";
+			reg = <0x180000 0x7e80000>;
+		};
+	};
+};
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 8ca4176..390cc95 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -569,11 +569,11 @@ config MTD_NAND_SH_FLCTL
 	  for NAND Flash using FLCTL.
 
 config MTD_NAND_DAVINCI
-        tristate "Support NAND on DaVinci SoC"
-        depends on ARCH_DAVINCI
+        tristate "Support NAND on SoCs with AEMIF"
+	select TI_DAVINCI_AEMIF
         help
 	  Enable the driver for NAND flash chips on Texas Instruments
-	  DaVinci processors.
+	  SoCs that has Asynchronous External Memory Interface (AEMIF).
 
 config MTD_NAND_TXX9NDFMC
 	tristate "NAND Flash support for TXx9 SoC"
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 321b053..306959e 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -35,8 +35,8 @@
 #include <linux/slab.h>
 #include <linux/of_device.h>
 
-#include <mach/nand.h>
-#include <mach/aemif.h>
+#include <linux/platform_data/davinci-nand.h>
+#include <linux/platform_data/davinci-aemif.h>
 
 /*
  * This is a device driver for the NAND flash controller found on the
@@ -73,7 +73,7 @@ struct davinci_nand_info {
 
 	uint32_t		core_chipsel;
 
-	struct davinci_aemif_timing	*timing;
+	struct davinci_aemif_cs_data	*cs_data;
 };
 
 static DEFINE_SPINLOCK(davinci_nand_lock);
@@ -652,7 +652,6 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 	info->chip.options	= pdata->options;
 	info->chip.bbt_td	= pdata->bbt_td;
 	info->chip.bbt_md	= pdata->bbt_md;
-	info->timing		= pdata->timing;
 
 	info->ioaddr		= (uint32_t __force) vaddr;
 
@@ -731,26 +730,21 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		goto err_clk_enable;
 	}
 
-	/*
-	 * Setup Async configuration register in case we did not boot from
-	 * NAND and so bootloader did not bother to set it up.
-	 */
-	val = davinci_nand_readl(info, A1CR_OFFSET + info->core_chipsel * 4);
-
-	/* Extended Wait is not valid and Select Strobe mode is not used */
-	val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
-	if (info->chip.options & NAND_BUSWIDTH_16)
-		val |= 0x1;
+	if (info->chip.options & NAND_BUSWIDTH_16) {
+		info->cs_data =
+			davinci_aemif_get_abus_params(info->core_chipsel);
+		if (info->cs_data == NULL)
+			goto err_bus_config;
 
-	davinci_nand_writel(info, A1CR_OFFSET + info->core_chipsel * 4, val);
+		/* asize = 1 for 16bit bus */
+		info->cs_data->asize = 1;
+		ret = davinci_aemif_set_abus_params(info->core_chipsel,
+						info->cs_data);
 
-	ret = 0;
-	if (info->timing)
-		ret = davinci_aemif_setup_timing(info->timing, info->base,
-							info->core_chipsel);
-	if (ret < 0) {
-		dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
-		goto err_timing;
+		if (ret < 0) {
+			dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
+			goto err_bus_config;
+		}
 	}
 
 	spin_lock_irq(&davinci_nand_lock);
@@ -841,7 +835,7 @@ syndrome_done:
 	return 0;
 
 err_scan:
-err_timing:
+err_bus_config:
 	clk_disable_unprepare(info->clk);
 
 err_clk_enable:
diff --git a/include/linux/platform_data/davinci-nand.h b/include/linux/platform_data/davinci-nand.h
new file mode 100644
index 0000000..df1fc66
--- /dev/null
+++ b/include/linux/platform_data/davinci-nand.h
@@ -0,0 +1,87 @@
+/*
+ * mach-davinci/nand.h
+ *
+ * Copyright © 2006 Texas Instruments.
+ *
+ * Ported to 2.6.23 Copyright © 2008 by
+ *   Sander Huijsen <Shuijsen@optelecom-nkf.com>
+ *   Troy Kisky <troy.kisky@boundarydevices.com>
+ *   Dirk Behme <Dirk.Behme@gmail.com>
+ *
+ * --------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef __ARCH_ARM_DAVINCI_NAND_H
+#define __ARCH_ARM_DAVINCI_NAND_H
+
+#include <linux/mtd/nand.h>
+
+#define NANDFCR_OFFSET		0x60
+#define NANDFSR_OFFSET		0x64
+#define NANDF1ECC_OFFSET	0x70
+
+/* 4-bit ECC syndrome registers */
+#define NAND_4BIT_ECC_LOAD_OFFSET	0xbc
+#define NAND_4BIT_ECC1_OFFSET		0xc0
+#define NAND_4BIT_ECC2_OFFSET		0xc4
+#define NAND_4BIT_ECC3_OFFSET		0xc8
+#define NAND_4BIT_ECC4_OFFSET		0xcc
+#define NAND_ERR_ADD1_OFFSET		0xd0
+#define NAND_ERR_ADD2_OFFSET		0xd4
+#define NAND_ERR_ERRVAL1_OFFSET		0xd8
+#define NAND_ERR_ERRVAL2_OFFSET		0xdc
+
+/* 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.
+ */
+#define	MASK_ALE		0x08
+#define	MASK_CLE		0x10
+
+struct davinci_nand_pdata {		/* platform_data */
+	uint32_t		mask_ale;
+	uint32_t		mask_cle;
+
+	/* for packages using two chipselects */
+	uint32_t		mask_chipsel;
+
+	/* board's default static partition info */
+	struct mtd_partition	*parts;
+	unsigned		nr_parts;
+
+	/* none  == NAND_ECC_NONE (strongly *not* advised!!)
+	 * soft  == NAND_ECC_SOFT
+	 * else  == NAND_ECC_HW, according to ecc_bits
+	 *
+	 * All DaVinci-family chips support 1-bit hardware ECC.
+	 * Newer ones also support 4-bit ECC, but are awkward
+	 * using it with large page chips.
+	 */
+	nand_ecc_modes_t	ecc_mode;
+	u8			ecc_bits;
+
+	/* e.g. NAND_BUSWIDTH_16 */
+	unsigned		options;
+	/* e.g. NAND_BBT_USE_FLASH */
+	unsigned		bbt_options;
+
+	/* Main and mirror bbt descriptor overrides */
+	struct nand_bbt_descr	*bbt_td;
+	struct nand_bbt_descr	*bbt_md;
+};
+
+#endif	/* __ARCH_ARM_DAVINCI_NAND_H */
-- 
1.7.9.5

[RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Murali Karicheri]

DaVinci NAND driver is a controller driver based on the AEMIF hardware
IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
patch removes the driver dependency on DaVinci architecture so that it
can be used on other architectures such as c6x, keystone etc.

Also migrate the driver to use the new AEMIF platform driver API and
moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
as this is expected to be used outside of arm/davinci.

Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
 .../devicetree/bindings/arm/davinci/nand.txt       |   59 -------------
 .../devicetree/bindings/mtd/davinci-nand.txt       |   59 +++++++++++++
 drivers/mtd/nand/Kconfig                           |    6 +-
 drivers/mtd/nand/davinci_nand.c                    |   40 ++++-----
 include/linux/platform_data/davinci-nand.h         |   87 ++++++++++++++++++++
 5 files changed, 166 insertions(+), 85 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
 create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
 create mode 100644 include/linux/platform_data/davinci-nand.h

diff --git a/Documentation/devicetree/bindings/arm/davinci/nand.txt b/Documentation/devicetree/bindings/arm/davinci/nand.txt
deleted file mode 100644
index 4746452..0000000
--- a/Documentation/devicetree/bindings/arm/davinci/nand.txt
+++ /dev/null
@@ -1,59 +0,0 @@
-* 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
-- ti,davinci-chipselect: Indicates on the davinci_nand driver which
-                         chipselect is used for accessing the nand.
-
-Recommended properties :
-- ti,davinci-mask-ale: mask for ale
-- ti,davinci-mask-cle: mask for cle
-- ti,davinci-mask-chipsel: mask for chipselect
-- ti,davinci-ecc-mode: ECC mode valid values for davinci driver:
-		- "none"
-		- "soft"
-		- "hw"
-- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
-- ti,davinci-nand-buswidth: buswidth 8 or 16
-- ti,davinci-nand-use-bbt: use flash based bad block table support.
-
-nand device bindings may contain additional sub-nodes describing
-partitions of the address space. See partition.txt for more detail.
-
-Example (enbw_cmc board):
-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@3,0 {
-		compatible = "ti,davinci-nand";
-		reg = <3 0x0 0x807ff
-			6 0x0 0x8000>;
-		#address-cells = <1>;
-		#size-cells = <1>;
-		ti,davinci-chipselect = <1>;
-		ti,davinci-mask-ale = <0>;
-		ti,davinci-mask-cle = <0>;
-		ti,davinci-mask-chipsel = <0>;
-		ti,davinci-ecc-mode = "hw";
-		ti,davinci-ecc-bits = <4>;
-		ti,davinci-nand-use-bbt;
-
-		partition@180000 {
-			label = "ubifs";
-			reg = <0x180000 0x7e80000>;
-		};
-	};
-};
diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
new file mode 100644
index 0000000..4746452
--- /dev/null
+++ b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
@@ -0,0 +1,59 @@
+* 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
+- ti,davinci-chipselect: Indicates on the davinci_nand driver which
+                         chipselect is used for accessing the nand.
+
+Recommended properties :
+- ti,davinci-mask-ale: mask for ale
+- ti,davinci-mask-cle: mask for cle
+- ti,davinci-mask-chipsel: mask for chipselect
+- ti,davinci-ecc-mode: ECC mode valid values for davinci driver:
+		- "none"
+		- "soft"
+		- "hw"
+- ti,davinci-ecc-bits: used ECC bits, currently supported 1 or 4.
+- ti,davinci-nand-buswidth: buswidth 8 or 16
+- ti,davinci-nand-use-bbt: use flash based bad block table support.
+
+nand device bindings may contain additional sub-nodes describing
+partitions of the address space. See partition.txt for more detail.
+
+Example (enbw_cmc board):
+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@3,0 {
+		compatible = "ti,davinci-nand";
+		reg = <3 0x0 0x807ff
+			6 0x0 0x8000>;
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ti,davinci-chipselect = <1>;
+		ti,davinci-mask-ale = <0>;
+		ti,davinci-mask-cle = <0>;
+		ti,davinci-mask-chipsel = <0>;
+		ti,davinci-ecc-mode = "hw";
+		ti,davinci-ecc-bits = <4>;
+		ti,davinci-nand-use-bbt;
+
+		partition@180000 {
+			label = "ubifs";
+			reg = <0x180000 0x7e80000>;
+		};
+	};
+};
diff --git a/drivers/mtd/nand/Kconfig b/drivers/mtd/nand/Kconfig
index 8ca4176..390cc95 100644
--- a/drivers/mtd/nand/Kconfig
+++ b/drivers/mtd/nand/Kconfig
@@ -569,11 +569,11 @@ config MTD_NAND_SH_FLCTL
 	  for NAND Flash using FLCTL.
 
 config MTD_NAND_DAVINCI
-        tristate "Support NAND on DaVinci SoC"
-        depends on ARCH_DAVINCI
+        tristate "Support NAND on SoCs with AEMIF"
+	select TI_DAVINCI_AEMIF
         help
 	  Enable the driver for NAND flash chips on Texas Instruments
-	  DaVinci processors.
+	  SoCs that has Asynchronous External Memory Interface (AEMIF).
 
 config MTD_NAND_TXX9NDFMC
 	tristate "NAND Flash support for TXx9 SoC"
diff --git a/drivers/mtd/nand/davinci_nand.c b/drivers/mtd/nand/davinci_nand.c
index 321b053..306959e 100644
--- a/drivers/mtd/nand/davinci_nand.c
+++ b/drivers/mtd/nand/davinci_nand.c
@@ -35,8 +35,8 @@
 #include <linux/slab.h>
 #include <linux/of_device.h>
 
-#include <mach/nand.h>
-#include <mach/aemif.h>
+#include <linux/platform_data/davinci-nand.h>
+#include <linux/platform_data/davinci-aemif.h>
 
 /*
  * This is a device driver for the NAND flash controller found on the
@@ -73,7 +73,7 @@ struct davinci_nand_info {
 
 	uint32_t		core_chipsel;
 
-	struct davinci_aemif_timing	*timing;
+	struct davinci_aemif_cs_data	*cs_data;
 };
 
 static DEFINE_SPINLOCK(davinci_nand_lock);
@@ -652,7 +652,6 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 	info->chip.options	= pdata->options;
 	info->chip.bbt_td	= pdata->bbt_td;
 	info->chip.bbt_md	= pdata->bbt_md;
-	info->timing		= pdata->timing;
 
 	info->ioaddr		= (uint32_t __force) vaddr;
 
@@ -731,26 +730,21 @@ static int __init nand_davinci_probe(struct platform_device *pdev)
 		goto err_clk_enable;
 	}
 
-	/*
-	 * Setup Async configuration register in case we did not boot from
-	 * NAND and so bootloader did not bother to set it up.
-	 */
-	val = davinci_nand_readl(info, A1CR_OFFSET + info->core_chipsel * 4);
-
-	/* Extended Wait is not valid and Select Strobe mode is not used */
-	val &= ~(ACR_ASIZE_MASK | ACR_EW_MASK | ACR_SS_MASK);
-	if (info->chip.options & NAND_BUSWIDTH_16)
-		val |= 0x1;
+	if (info->chip.options & NAND_BUSWIDTH_16) {
+		info->cs_data =
+			davinci_aemif_get_abus_params(info->core_chipsel);
+		if (info->cs_data == NULL)
+			goto err_bus_config;
 
-	davinci_nand_writel(info, A1CR_OFFSET + info->core_chipsel * 4, val);
+		/* asize = 1 for 16bit bus */
+		info->cs_data->asize = 1;
+		ret = davinci_aemif_set_abus_params(info->core_chipsel,
+						info->cs_data);
 
-	ret = 0;
-	if (info->timing)
-		ret = davinci_aemif_setup_timing(info->timing, info->base,
-							info->core_chipsel);
-	if (ret < 0) {
-		dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
-		goto err_timing;
+		if (ret < 0) {
+			dev_dbg(&pdev->dev, "NAND timing values setup fail\n");
+			goto err_bus_config;
+		}
 	}
 
 	spin_lock_irq(&davinci_nand_lock);
@@ -841,7 +835,7 @@ syndrome_done:
 	return 0;
 
 err_scan:
-err_timing:
+err_bus_config:
 	clk_disable_unprepare(info->clk);
 
 err_clk_enable:
diff --git a/include/linux/platform_data/davinci-nand.h b/include/linux/platform_data/davinci-nand.h
new file mode 100644
index 0000000..df1fc66
--- /dev/null
+++ b/include/linux/platform_data/davinci-nand.h
@@ -0,0 +1,87 @@
+/*
+ * mach-davinci/nand.h
+ *
+ * Copyright © 2006 Texas Instruments.
+ *
+ * Ported to 2.6.23 Copyright © 2008 by
+ *   Sander Huijsen <Shuijsen@optelecom-nkf.com>
+ *   Troy Kisky <troy.kisky@boundarydevices.com>
+ *   Dirk Behme <Dirk.Behme@gmail.com>
+ *
+ * --------------------------------------------------------------------------
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#ifndef __ARCH_ARM_DAVINCI_NAND_H
+#define __ARCH_ARM_DAVINCI_NAND_H
+
+#include <linux/mtd/nand.h>
+
+#define NANDFCR_OFFSET		0x60
+#define NANDFSR_OFFSET		0x64
+#define NANDF1ECC_OFFSET	0x70
+
+/* 4-bit ECC syndrome registers */
+#define NAND_4BIT_ECC_LOAD_OFFSET	0xbc
+#define NAND_4BIT_ECC1_OFFSET		0xc0
+#define NAND_4BIT_ECC2_OFFSET		0xc4
+#define NAND_4BIT_ECC3_OFFSET		0xc8
+#define NAND_4BIT_ECC4_OFFSET		0xcc
+#define NAND_ERR_ADD1_OFFSET		0xd0
+#define NAND_ERR_ADD2_OFFSET		0xd4
+#define NAND_ERR_ERRVAL1_OFFSET		0xd8
+#define NAND_ERR_ERRVAL2_OFFSET		0xdc
+
+/* 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.
+ */
+#define	MASK_ALE		0x08
+#define	MASK_CLE		0x10
+
+struct davinci_nand_pdata {		/* platform_data */
+	uint32_t		mask_ale;
+	uint32_t		mask_cle;
+
+	/* for packages using two chipselects */
+	uint32_t		mask_chipsel;
+
+	/* board's default static partition info */
+	struct mtd_partition	*parts;
+	unsigned		nr_parts;
+
+	/* none  == NAND_ECC_NONE (strongly *not* advised!!)
+	 * soft  == NAND_ECC_SOFT
+	 * else  == NAND_ECC_HW, according to ecc_bits
+	 *
+	 * All DaVinci-family chips support 1-bit hardware ECC.
+	 * Newer ones also support 4-bit ECC, but are awkward
+	 * using it with large page chips.
+	 */
+	nand_ecc_modes_t	ecc_mode;
+	u8			ecc_bits;
+
+	/* e.g. NAND_BUSWIDTH_16 */
+	unsigned		options;
+	/* e.g. NAND_BBT_USE_FLASH */
+	unsigned		bbt_options;
+
+	/* Main and mirror bbt descriptor overrides */
+	struct nand_bbt_descr	*bbt_td;
+	struct nand_bbt_descr	*bbt_md;
+};
+
+#endif	/* __ARCH_ARM_DAVINCI_NAND_H */
-- 
1.7.9.5

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

[Santosh Shilimkar]

On Tuesday 06 November 2012 03: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.
>
> This supports configuration of the bus either through platform_data or
> through DT bindings.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>   .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
>   drivers/memory/Kconfig                             |   10 +
>   drivers/memory/Makefile                            |    1 +
>   drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
>   include/linux/platform_data/davinci-aemif.h        |   47 ++
>   5 files changed, 640 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
>   create mode 100644 drivers/memory/davinci-aemif.c
>   create mode 100644 include/linux/platform_data/davinci-aemif.h
>
Please split the DT and drivers/memory/* changes in two seperate
patches.

> diff --git a/Documentation/devicetree/bindings/memory/davinci-aemif.txt b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> new file mode 100644
> index 0000000..a79b3ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> @@ -0,0 +1,103 @@
> +* 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.
> +- reg : contains offset/length value for AEMIF control registers space
> +- ranges : Each range corresponds to a single chipselect, and cover
> +           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.
> +
> +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)
> +- 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
> +- 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)
> +
> +if any of the above parameters are absent, hardware register default or that
> +set by a boot loader are used.
> +
> +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 {
> +		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>;
> +	};
> +
> +	nor_cs:cs3@62000000 {
> +		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>;
> +		ti,davinci-cs-asize = <1>;
> +	};
> +
> +	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
> +	};
> +
> +	flash@2,0 {
> +		compatible = "cfi-flash";
> +		reg = <2 0x0 0x400000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		bank-width = <2>;
> +		device-width = <2>;
> +	};
> +};
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 067f311..2636a95 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -40,4 +40,14 @@ config TEGRA30_MC
>   	  analysis, especially for IOMMU/SMMU(System Memory Management
>   	  Unit) module.
>
> +config TI_DAVINCI_AEMIF
> +	bool "Texas Instruments DaVinci AEMIF driver"
Add some default depends on ARCH here otherwise, this driver
will get build for all the generic builds and might show
build errors.

> +	help
> +	  This driver is for the AEMIF module available in Texas Instruments
> +	  SoCs. AEMIF stands for Asynchronous External Memory Interface and
> +	  is intended to provide a glue-less interface to a variety of
> +	  asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
> +	  of 256M bytes of any of these memories can be accessed at a given
> +	  time via four chip selects with 64M byte access per chip select.
> +
>   endif
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 42b3ce9..246aa61 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -5,3 +5,4 @@
>   obj-$(CONFIG_TI_EMIF)		+= emif.o
>   obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
>   obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
> +obj-$(CONFIG_TI_DAVINCI_AEMIF)	+= davinci-aemif.o
> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
> new file mode 100644
> index 0000000..27a6995
> --- /dev/null
> +++ b/drivers/memory/davinci-aemif.c
> @@ -0,0 +1,479 @@
> +/*
> + * AEMIF support for DaVinci SoCs
> + *
> + * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.com/
s/2010/2012
> + * Copyright (C) Heiko Schocher <hs@denx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_data/davinci-aemif.h>
> +#include <linux/platform_device.h>
> +#include <linux/time.h>
> +
> +#define TA_SHIFT	2
> +#define RHOLD_SHIFT	4
> +#define RSTROBE_SHIFT	7
> +#define RSETUP_SHIFT	13
> +#define WHOLD_SHIFT	17
> +#define WSTROBE_SHIFT	20
> +#define WSETUP_SHIFT	26
> +#define EW_SHIFT	30
> +#define SS_SHIFT	31
> +
> +#define TA(x)		((x) << TA_SHIFT)
> +#define RHOLD(x)	((x) << RHOLD_SHIFT)
> +#define RSTROBE(x)	((x) << RSTROBE_SHIFT)
> +#define RSETUP(x)	((x) << RSETUP_SHIFT)
> +#define WHOLD(x)	((x) << WHOLD_SHIFT)
> +#define WSTROBE(x)	((x) << WSTROBE_SHIFT)
> +#define WSETUP(x)	((x) << WSETUP_SHIFT)
> +#define EW(x)		((x) << EW_SHIFT)
> +#define SS(x)		((x) << SS_SHIFT)
> +
Can you not use BIT(*) directly instead above magics

> +#define ASIZE_MAX	0x1
> +#define TA_MAX		0x3
> +#define RHOLD_MAX	0x7
> +#define RSTROBE_MAX	0x3f
> +#define RSETUP_MAX	0xf
> +#define WHOLD_MAX	0x7
> +#define WSTROBE_MAX	0x3f
> +#define WSETUP_MAX	0xf
> +#define EW_MAX		0x1
> +#define SS_MAX		0x1
> +#define NUM_CS		4
> +
> +#define TA_VAL(x)	(((x) & TA(TA_MAX)) >> TA_SHIFT)
> +#define RHOLD_VAL(x)	(((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
> +#define RSTROBE_VAL(x)	(((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
> +#define RSETUP_VAL(x)	(((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
> +#define WHOLD_VAL(x)	(((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
> +#define WSTROBE_VAL(x)	(((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
> +#define WSETUP_VAL(x)	(((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
> +#define EW_VAL(x)	(((x) & EW(EW_MAX)) >> EW_SHIFT)
> +#define SS_VAL(x)	(((x) & SS(SS_MAX)) >> SS_SHIFT)
> +
Same here..

> +
> +#define CONFIG_MASK	(TA(TA_MAX) | \
> +				RHOLD(RHOLD_MAX) | \
> +				RSTROBE(RSTROBE_MAX) |	\
> +				RSETUP(RSETUP_MAX) | \
> +				WHOLD(WHOLD_MAX) | \
> +				WSTROBE(WSTROBE_MAX) | \
> +				WSETUP(WSETUP_MAX) | \
> +				EW(EW_MAX) | SS(SS_MAX) | \
> +				ASIZE_MAX)
> +
This mask is next to impossible if reviewer needs to decode it.

> +#define DRV_NAME "davinci-aemif"
> +
> +struct aemif_device {
> +	struct davinci_aemif_pdata *cfg;
> +	void __iomem *base;
> +	struct clk *clk;
> +	/* clock rate in KHz */
> +	unsigned long clk_rate;
> +};
> +
> +static struct aemif_device *aemif;
> +/**
> + * aemif_calc_rate - calculate timing data.
> + * @wanted: The cycle time needed in nanoseconds.
> + * @clk: The input clock rate in kHz.
> + * @max: The maximum divider value that can be programmed.
> + *
> + * On success, returns the calculated timing value minus 1 for easy
> + * programming into AEMIF timing registers, else negative errno.
> + */
> +static int aemif_calc_rate(int wanted, unsigned long clk, int max)
> +{
> +	int result;
> +
> +	result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
> +
> +	pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, wanted);
> +
> +	/* It is generally OK to have a more relaxed timing than requested... */
> +	if (result < 0)
> +		result = 0;
> +
don't break if--else with un-ncessary new line here.

> +	/* ... But configuring tighter timings is not an option. */
> +	else if (result > max)
> +		result = -EINVAL;
> +
> +	return result;
> +}
> +
> +/**
> + * davinci_aemif_config_abus - configure async bus parameters given
> + * AEMIF interface
> + * @cs: chip-select to program the timing values for
> + * @data: aemif chip select configuration
> + * @base: aemif io mapped configuration base
> + *
> + * This function programs the given timing values (in real clock) into the
> + * AEMIF registers taking the AEMIF clock into account.
> + *
> + * This function does not use any locking while programming the AEMIF
> + * because it is expected that there is only one user of a given
> + * chip-select.
> + *
> + * Returns 0 on success, else negative errno.
> + */
> +static int davinci_aemif_config_abus(unsigned int cs,
> +				void __iomem *base,
> +				struct davinci_aemif_cs_data *data)
> +{
> +	int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
> +	unsigned offset = A1CR_OFFSET + cs * 4;
> +	u32 set, val;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	ta	= aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
> +	rhold	= aemif_calc_rate(data->rhold, aemif->clk_rate, RHOLD_MAX);
> +	rstrobe	= aemif_calc_rate(data->rstrobe, aemif->clk_rate, RSTROBE_MAX);
> +	rsetup	= aemif_calc_rate(data->rsetup, aemif->clk_rate, RSETUP_MAX);
> +	whold	= aemif_calc_rate(data->whold, aemif->clk_rate, WHOLD_MAX);
> +	wstrobe	= aemif_calc_rate(data->wstrobe, aemif->clk_rate, WSTROBE_MAX);
> +	wsetup	= aemif_calc_rate(data->wsetup, aemif->clk_rate, WSETUP_MAX);
> +
> +	if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
> +			whold < 0 || wstrobe < 0 || wsetup < 0) {
> +		pr_err("%s: cannot get suitable timings\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
> +		WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
> +
> +	set |= (data->asize & ACR_ASIZE_MASK);
> +	if (data->enable_ew)
> +		set |= ACR_EW_MASK;
> +	if (data->enable_ss)
> +		set |= ACR_SS_MASK;
> +
> +	val = readl(aemif->base + offset);
> +	val &= ~CONFIG_MASK;
> +	val |= set;
> +	writel(val, aemif->base + offset);
> +
> +	return 0;
> +}
> +
> +inline int aemif_cycles_to_nsec(int val)
> +{
> +	return (val * NSEC_PER_MSEC) / aemif->clk_rate;
> +}
> +
> +/**
> + * davinci_aemif_get_hw_params - function to read hw register values
> + * @cs: chip select
> + * @data: ptr to cs data
> + *
> + * This function reads the defaults from the registers and update
> + * the timing values. Required for get/set commands and also for
> + * the case when driver needs to use defaults in hardware.
> + */
> +static void davinci_aemif_get_hw_params(int cs,
> +		struct davinci_aemif_cs_data *data)
> +{
> +	u32 val, offset = A1CR_OFFSET + cs * 4;
> +
> +	val = readl(aemif->base + offset);
> +	data->ta = aemif_cycles_to_nsec(TA_VAL(val));
> +	data->rhold = aemif_cycles_to_nsec(RHOLD_VAL(val));
> +	data->rstrobe = aemif_cycles_to_nsec(RSTROBE_VAL(val));
> +	data->rsetup = aemif_cycles_to_nsec(RSETUP_VAL(val));
> +	data->whold = aemif_cycles_to_nsec(WHOLD_VAL(val));
> +	data->wstrobe = aemif_cycles_to_nsec(WSTROBE_VAL(val));
> +	data->wsetup = aemif_cycles_to_nsec(WSETUP_VAL(val));
> +	data->enable_ew = EW_VAL(val);
> +	data->enable_ss = SS_VAL(val);
> +	data->asize = val & ASIZE_MAX;
> +}
> +
> +/**
> + * get_cs_data - helper function to get bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + */
> +static struct davinci_aemif_cs_data *get_cs_data(int cs)
> +{
> +	int i;
> +
> +	for (i = 0; i < aemif->cfg->num_cs; i++) {
> +		if (cs == aemif->cfg->cs_data[i].cs)
> +			break;
> +	}
> +
> +	if (i == aemif->cfg->num_cs)
> +		return NULL;
> +
> +	return &aemif->cfg->cs_data[i];
> +}
> +
> +/**
> + * davinci_aemif_set_abus_params - Set bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + * @data: ptr to a struct to hold the configuration data to be set
> + *
> + * This function is called to configure emif bus parameters for a given cs.
> + * Users call this function after calling davinci_aemif_get_abus_params()
> + * to get current parameters, modify and call this function
> + */
> +int davinci_aemif_set_abus_params(unsigned int cs,
> +			struct davinci_aemif_cs_data *data)
> +{
> +	struct davinci_aemif_cs_data *curr_cs_data;
> +	int ret = -EINVAL, chip_cs;
> +
> +	if (data == NULL)
> +		return ret;
> +
> +	if (aemif->base == NULL)
> +		return ret;
> +
> +	/* translate to chip CS which starts at 2 */
what is chip CS ? you can either use CS or chip select..
> +	chip_cs = cs + 2;
Use a macro like AEMIF_CS_START instead of hardcoding it.

> +
> +	curr_cs_data = get_cs_data(chip_cs);
> +	if (curr_cs_data) {
> +		/* for configuration we use cs since it is used to index ACR */
> +		ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
> +		if (!ret) {
> +			*curr_cs_data = *data;
> +			return 0;
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(davinci_aemif_set_abus_params);
> +
Who is the user of this EXPORT ?

> +/**
> + * davinci_aemif_get_abus_params - Get bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + * returns: ptr to a struct having the current configuration data
> + */
> +struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs)
> +{
> +	/* translate to chip CS which starts at 2 */
> +	return get_cs_data(cs + 2);
> +}
> +EXPORT_SYMBOL(davinci_aemif_get_abus_params);
> +
This one too.

[...]

> +static int __devinit davinci_aemif_probe(struct platform_device *pdev)
> +{
> +	struct davinci_aemif_pdata *cfg;
> +	int ret  = -ENODEV, i;
> +	struct resource *res;
> +
> +	aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
> +
> +	if (!aemif)
> +		return -ENOMEM;
> +
> +	aemif->clk = clk_get(NULL, "aemif");
Please use dev attributes. Above usage of clk_get isn't recommonded
in drivers. You might have to add alias entries in your clock data for
it to work though.

> +	if (IS_ERR(aemif->clk))
> +		return PTR_ERR(aemif->clk);
> +
> +	clk_prepare_enable(aemif->clk);
> +	aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
> +
/1000 for what ? Converting it into KHz ?

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		pr_err("No IO memory address defined\n");
> +		goto error;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	aemif->base = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!aemif->base) {
> +		ret = -EBUSY;
> +		pr_err("ioremap failed\n");
> +		goto error;
> +	}
> +
> +	if (pdev->dev.platform_data == NULL) {
> +		/* Not platform data, we get the cs data from the cs nodes */
> +		cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
> +		if (cfg == NULL)
> +			return -ENOMEM;
> +
> +		aemif->cfg = cfg;
> +		if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
> +			pr_err("No platform data or cs of node present\n");
> +			goto error;
> +		}
> +	} else {
> +		cfg = pdev->dev.platform_data;
> +		aemif->cfg = cfg;
> +	}
> +
> +	for (i = 0; i < cfg->num_cs; i++) {
> +		/* cs is from 2-5. Internally we use cs-2 to access ACR */
> +		ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
> +				aemif->base, &cfg->cs_data[i]);
> +		if (ret < 0) {
> +			pr_err("Error configuring chip select %d\n",
> +				cfg->cs_data[i].cs);
> +			goto error;
> +		}
> +	}
> +	return 0;
> +error:
> +	clk_disable_unprepare(aemif->clk);
> +	clk_put(aemif->clk);
Alos unmap 'aemif->base' here..

> +	return ret;
> +}
> +
> +static struct platform_driver davinci_aemif_driver = {
> +	.probe = davinci_aemif_probe,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = davinci_aemif_of_match,
> +	},
> +};
> +
> +static int __init davinci_aemif_init(void)
> +{
> +	return platform_driver_register(&davinci_aemif_driver);
> +}
> +subsys_initcall(davinci_aemif_init);
> +
> +static void __exit davinci_aemif_exit(void)
> +{
> +	clk_disable_unprepare(aemif->clk);
> +	clk_put(aemif->clk);
> +	platform_driver_unregister(&davinci_aemif_driver);
> +}
> +module_exit(davinci_aemif_exit);
> +
> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
> +MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRV_NAME);
> diff --git a/include/linux/platform_data/davinci-aemif.h b/include/linux/platform_data/davinci-aemif.h
> new file mode 100644
> index 0000000..03f3ad0
> --- /dev/null
> +++ b/include/linux/platform_data/davinci-aemif.h
> @@ -0,0 +1,47 @@
> +/*
> + * TI DaVinci AEMIF support
> + *
> + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
> + *
s/2010/2012
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +#ifndef _MACH_DAVINCI_AEMIF_H
> +#define _MACH_DAVINCI_AEMIF_H
Fix the header guard as per new directory

Regards
Santosh

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

[Santosh Shilimkar]

On Tuesday 06 November 2012 03: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.
>
> This supports configuration of the bus either through platform_data or
> through DT bindings.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>   .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
>   drivers/memory/Kconfig                             |   10 +
>   drivers/memory/Makefile                            |    1 +
>   drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
>   include/linux/platform_data/davinci-aemif.h        |   47 ++
>   5 files changed, 640 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
>   create mode 100644 drivers/memory/davinci-aemif.c
>   create mode 100644 include/linux/platform_data/davinci-aemif.h
>
Please split the DT and drivers/memory/* changes in two seperate
patches.

> diff --git a/Documentation/devicetree/bindings/memory/davinci-aemif.txt b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> new file mode 100644
> index 0000000..a79b3ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> @@ -0,0 +1,103 @@
> +* 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.
> +- reg : contains offset/length value for AEMIF control registers space
> +- ranges : Each range corresponds to a single chipselect, and cover
> +           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.
> +
> +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)
> +- 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
> +- 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)
> +
> +if any of the above parameters are absent, hardware register default or that
> +set by a boot loader are used.
> +
> +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 {
> +		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>;
> +	};
> +
> +	nor_cs:cs3@62000000 {
> +		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>;
> +		ti,davinci-cs-asize = <1>;
> +	};
> +
> +	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
> +	};
> +
> +	flash@2,0 {
> +		compatible = "cfi-flash";
> +		reg = <2 0x0 0x400000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		bank-width = <2>;
> +		device-width = <2>;
> +	};
> +};
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 067f311..2636a95 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -40,4 +40,14 @@ config TEGRA30_MC
>   	  analysis, especially for IOMMU/SMMU(System Memory Management
>   	  Unit) module.
>
> +config TI_DAVINCI_AEMIF
> +	bool "Texas Instruments DaVinci AEMIF driver"
Add some default depends on ARCH here otherwise, this driver
will get build for all the generic builds and might show
build errors.

> +	help
> +	  This driver is for the AEMIF module available in Texas Instruments
> +	  SoCs. AEMIF stands for Asynchronous External Memory Interface and
> +	  is intended to provide a glue-less interface to a variety of
> +	  asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
> +	  of 256M bytes of any of these memories can be accessed at a given
> +	  time via four chip selects with 64M byte access per chip select.
> +
>   endif
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 42b3ce9..246aa61 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -5,3 +5,4 @@
>   obj-$(CONFIG_TI_EMIF)		+= emif.o
>   obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
>   obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
> +obj-$(CONFIG_TI_DAVINCI_AEMIF)	+= davinci-aemif.o
> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
> new file mode 100644
> index 0000000..27a6995
> --- /dev/null
> +++ b/drivers/memory/davinci-aemif.c
> @@ -0,0 +1,479 @@
> +/*
> + * AEMIF support for DaVinci SoCs
> + *
> + * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.com/
s/2010/2012
> + * Copyright (C) Heiko Schocher <hs@denx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_data/davinci-aemif.h>
> +#include <linux/platform_device.h>
> +#include <linux/time.h>
> +
> +#define TA_SHIFT	2
> +#define RHOLD_SHIFT	4
> +#define RSTROBE_SHIFT	7
> +#define RSETUP_SHIFT	13
> +#define WHOLD_SHIFT	17
> +#define WSTROBE_SHIFT	20
> +#define WSETUP_SHIFT	26
> +#define EW_SHIFT	30
> +#define SS_SHIFT	31
> +
> +#define TA(x)		((x) << TA_SHIFT)
> +#define RHOLD(x)	((x) << RHOLD_SHIFT)
> +#define RSTROBE(x)	((x) << RSTROBE_SHIFT)
> +#define RSETUP(x)	((x) << RSETUP_SHIFT)
> +#define WHOLD(x)	((x) << WHOLD_SHIFT)
> +#define WSTROBE(x)	((x) << WSTROBE_SHIFT)
> +#define WSETUP(x)	((x) << WSETUP_SHIFT)
> +#define EW(x)		((x) << EW_SHIFT)
> +#define SS(x)		((x) << SS_SHIFT)
> +
Can you not use BIT(*) directly instead above magics

> +#define ASIZE_MAX	0x1
> +#define TA_MAX		0x3
> +#define RHOLD_MAX	0x7
> +#define RSTROBE_MAX	0x3f
> +#define RSETUP_MAX	0xf
> +#define WHOLD_MAX	0x7
> +#define WSTROBE_MAX	0x3f
> +#define WSETUP_MAX	0xf
> +#define EW_MAX		0x1
> +#define SS_MAX		0x1
> +#define NUM_CS		4
> +
> +#define TA_VAL(x)	(((x) & TA(TA_MAX)) >> TA_SHIFT)
> +#define RHOLD_VAL(x)	(((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
> +#define RSTROBE_VAL(x)	(((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
> +#define RSETUP_VAL(x)	(((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
> +#define WHOLD_VAL(x)	(((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
> +#define WSTROBE_VAL(x)	(((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
> +#define WSETUP_VAL(x)	(((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
> +#define EW_VAL(x)	(((x) & EW(EW_MAX)) >> EW_SHIFT)
> +#define SS_VAL(x)	(((x) & SS(SS_MAX)) >> SS_SHIFT)
> +
Same here..

> +
> +#define CONFIG_MASK	(TA(TA_MAX) | \
> +				RHOLD(RHOLD_MAX) | \
> +				RSTROBE(RSTROBE_MAX) |	\
> +				RSETUP(RSETUP_MAX) | \
> +				WHOLD(WHOLD_MAX) | \
> +				WSTROBE(WSTROBE_MAX) | \
> +				WSETUP(WSETUP_MAX) | \
> +				EW(EW_MAX) | SS(SS_MAX) | \
> +				ASIZE_MAX)
> +
This mask is next to impossible if reviewer needs to decode it.

> +#define DRV_NAME "davinci-aemif"
> +
> +struct aemif_device {
> +	struct davinci_aemif_pdata *cfg;
> +	void __iomem *base;
> +	struct clk *clk;
> +	/* clock rate in KHz */
> +	unsigned long clk_rate;
> +};
> +
> +static struct aemif_device *aemif;
> +/**
> + * aemif_calc_rate - calculate timing data.
> + * @wanted: The cycle time needed in nanoseconds.
> + * @clk: The input clock rate in kHz.
> + * @max: The maximum divider value that can be programmed.
> + *
> + * On success, returns the calculated timing value minus 1 for easy
> + * programming into AEMIF timing registers, else negative errno.
> + */
> +static int aemif_calc_rate(int wanted, unsigned long clk, int max)
> +{
> +	int result;
> +
> +	result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
> +
> +	pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, wanted);
> +
> +	/* It is generally OK to have a more relaxed timing than requested... */
> +	if (result < 0)
> +		result = 0;
> +
don't break if--else with un-ncessary new line here.

> +	/* ... But configuring tighter timings is not an option. */
> +	else if (result > max)
> +		result = -EINVAL;
> +
> +	return result;
> +}
> +
> +/**
> + * davinci_aemif_config_abus - configure async bus parameters given
> + * AEMIF interface
> + * @cs: chip-select to program the timing values for
> + * @data: aemif chip select configuration
> + * @base: aemif io mapped configuration base
> + *
> + * This function programs the given timing values (in real clock) into the
> + * AEMIF registers taking the AEMIF clock into account.
> + *
> + * This function does not use any locking while programming the AEMIF
> + * because it is expected that there is only one user of a given
> + * chip-select.
> + *
> + * Returns 0 on success, else negative errno.
> + */
> +static int davinci_aemif_config_abus(unsigned int cs,
> +				void __iomem *base,
> +				struct davinci_aemif_cs_data *data)
> +{
> +	int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
> +	unsigned offset = A1CR_OFFSET + cs * 4;
> +	u32 set, val;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	ta	= aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
> +	rhold	= aemif_calc_rate(data->rhold, aemif->clk_rate, RHOLD_MAX);
> +	rstrobe	= aemif_calc_rate(data->rstrobe, aemif->clk_rate, RSTROBE_MAX);
> +	rsetup	= aemif_calc_rate(data->rsetup, aemif->clk_rate, RSETUP_MAX);
> +	whold	= aemif_calc_rate(data->whold, aemif->clk_rate, WHOLD_MAX);
> +	wstrobe	= aemif_calc_rate(data->wstrobe, aemif->clk_rate, WSTROBE_MAX);
> +	wsetup	= aemif_calc_rate(data->wsetup, aemif->clk_rate, WSETUP_MAX);
> +
> +	if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
> +			whold < 0 || wstrobe < 0 || wsetup < 0) {
> +		pr_err("%s: cannot get suitable timings\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
> +		WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
> +
> +	set |= (data->asize & ACR_ASIZE_MASK);
> +	if (data->enable_ew)
> +		set |= ACR_EW_MASK;
> +	if (data->enable_ss)
> +		set |= ACR_SS_MASK;
> +
> +	val = readl(aemif->base + offset);
> +	val &= ~CONFIG_MASK;
> +	val |= set;
> +	writel(val, aemif->base + offset);
> +
> +	return 0;
> +}
> +
> +inline int aemif_cycles_to_nsec(int val)
> +{
> +	return (val * NSEC_PER_MSEC) / aemif->clk_rate;
> +}
> +
> +/**
> + * davinci_aemif_get_hw_params - function to read hw register values
> + * @cs: chip select
> + * @data: ptr to cs data
> + *
> + * This function reads the defaults from the registers and update
> + * the timing values. Required for get/set commands and also for
> + * the case when driver needs to use defaults in hardware.
> + */
> +static void davinci_aemif_get_hw_params(int cs,
> +		struct davinci_aemif_cs_data *data)
> +{
> +	u32 val, offset = A1CR_OFFSET + cs * 4;
> +
> +	val = readl(aemif->base + offset);
> +	data->ta = aemif_cycles_to_nsec(TA_VAL(val));
> +	data->rhold = aemif_cycles_to_nsec(RHOLD_VAL(val));
> +	data->rstrobe = aemif_cycles_to_nsec(RSTROBE_VAL(val));
> +	data->rsetup = aemif_cycles_to_nsec(RSETUP_VAL(val));
> +	data->whold = aemif_cycles_to_nsec(WHOLD_VAL(val));
> +	data->wstrobe = aemif_cycles_to_nsec(WSTROBE_VAL(val));
> +	data->wsetup = aemif_cycles_to_nsec(WSETUP_VAL(val));
> +	data->enable_ew = EW_VAL(val);
> +	data->enable_ss = SS_VAL(val);
> +	data->asize = val & ASIZE_MAX;
> +}
> +
> +/**
> + * get_cs_data - helper function to get bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + */
> +static struct davinci_aemif_cs_data *get_cs_data(int cs)
> +{
> +	int i;
> +
> +	for (i = 0; i < aemif->cfg->num_cs; i++) {
> +		if (cs == aemif->cfg->cs_data[i].cs)
> +			break;
> +	}
> +
> +	if (i == aemif->cfg->num_cs)
> +		return NULL;
> +
> +	return &aemif->cfg->cs_data[i];
> +}
> +
> +/**
> + * davinci_aemif_set_abus_params - Set bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + * @data: ptr to a struct to hold the configuration data to be set
> + *
> + * This function is called to configure emif bus parameters for a given cs.
> + * Users call this function after calling davinci_aemif_get_abus_params()
> + * to get current parameters, modify and call this function
> + */
> +int davinci_aemif_set_abus_params(unsigned int cs,
> +			struct davinci_aemif_cs_data *data)
> +{
> +	struct davinci_aemif_cs_data *curr_cs_data;
> +	int ret = -EINVAL, chip_cs;
> +
> +	if (data == NULL)
> +		return ret;
> +
> +	if (aemif->base == NULL)
> +		return ret;
> +
> +	/* translate to chip CS which starts at 2 */
what is chip CS ? you can either use CS or chip select..
> +	chip_cs = cs + 2;
Use a macro like AEMIF_CS_START instead of hardcoding it.

> +
> +	curr_cs_data = get_cs_data(chip_cs);
> +	if (curr_cs_data) {
> +		/* for configuration we use cs since it is used to index ACR */
> +		ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
> +		if (!ret) {
> +			*curr_cs_data = *data;
> +			return 0;
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(davinci_aemif_set_abus_params);
> +
Who is the user of this EXPORT ?

> +/**
> + * davinci_aemif_get_abus_params - Get bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + * returns: ptr to a struct having the current configuration data
> + */
> +struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs)
> +{
> +	/* translate to chip CS which starts at 2 */
> +	return get_cs_data(cs + 2);
> +}
> +EXPORT_SYMBOL(davinci_aemif_get_abus_params);
> +
This one too.

[...]

> +static int __devinit davinci_aemif_probe(struct platform_device *pdev)
> +{
> +	struct davinci_aemif_pdata *cfg;
> +	int ret  = -ENODEV, i;
> +	struct resource *res;
> +
> +	aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
> +
> +	if (!aemif)
> +		return -ENOMEM;
> +
> +	aemif->clk = clk_get(NULL, "aemif");
Please use dev attributes. Above usage of clk_get isn't recommonded
in drivers. You might have to add alias entries in your clock data for
it to work though.

> +	if (IS_ERR(aemif->clk))
> +		return PTR_ERR(aemif->clk);
> +
> +	clk_prepare_enable(aemif->clk);
> +	aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
> +
/1000 for what ? Converting it into KHz ?

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		pr_err("No IO memory address defined\n");
> +		goto error;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	aemif->base = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!aemif->base) {
> +		ret = -EBUSY;
> +		pr_err("ioremap failed\n");
> +		goto error;
> +	}
> +
> +	if (pdev->dev.platform_data == NULL) {
> +		/* Not platform data, we get the cs data from the cs nodes */
> +		cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
> +		if (cfg == NULL)
> +			return -ENOMEM;
> +
> +		aemif->cfg = cfg;
> +		if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
> +			pr_err("No platform data or cs of node present\n");
> +			goto error;
> +		}
> +	} else {
> +		cfg = pdev->dev.platform_data;
> +		aemif->cfg = cfg;
> +	}
> +
> +	for (i = 0; i < cfg->num_cs; i++) {
> +		/* cs is from 2-5. Internally we use cs-2 to access ACR */
> +		ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
> +				aemif->base, &cfg->cs_data[i]);
> +		if (ret < 0) {
> +			pr_err("Error configuring chip select %d\n",
> +				cfg->cs_data[i].cs);
> +			goto error;
> +		}
> +	}
> +	return 0;
> +error:
> +	clk_disable_unprepare(aemif->clk);
> +	clk_put(aemif->clk);
Alos unmap 'aemif->base' here..

> +	return ret;
> +}
> +
> +static struct platform_driver davinci_aemif_driver = {
> +	.probe = davinci_aemif_probe,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = davinci_aemif_of_match,
> +	},
> +};
> +
> +static int __init davinci_aemif_init(void)
> +{
> +	return platform_driver_register(&davinci_aemif_driver);
> +}
> +subsys_initcall(davinci_aemif_init);
> +
> +static void __exit davinci_aemif_exit(void)
> +{
> +	clk_disable_unprepare(aemif->clk);
> +	clk_put(aemif->clk);
> +	platform_driver_unregister(&davinci_aemif_driver);
> +}
> +module_exit(davinci_aemif_exit);
> +
> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
> +MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRV_NAME);
> diff --git a/include/linux/platform_data/davinci-aemif.h b/include/linux/platform_data/davinci-aemif.h
> new file mode 100644
> index 0000000..03f3ad0
> --- /dev/null
> +++ b/include/linux/platform_data/davinci-aemif.h
> @@ -0,0 +1,47 @@
> +/*
> + * TI DaVinci AEMIF support
> + *
> + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
> + *
s/2010/2012
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +#ifndef _MACH_DAVINCI_AEMIF_H
> +#define _MACH_DAVINCI_AEMIF_H
Fix the header guard as per new directory

Regards
Santosh

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

[Santosh Shilimkar]

On Tuesday 06 November 2012 03: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.
>
> This supports configuration of the bus either through platform_data or
> through DT bindings.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
> ---
>   .../devicetree/bindings/memory/davinci-aemif.txt   |  103 +++++
>   drivers/memory/Kconfig                             |   10 +
>   drivers/memory/Makefile                            |    1 +
>   drivers/memory/davinci-aemif.c                     |  479 ++++++++++++++++++++
>   include/linux/platform_data/davinci-aemif.h        |   47 ++
>   5 files changed, 640 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/memory/davinci-aemif.txt
>   create mode 100644 drivers/memory/davinci-aemif.c
>   create mode 100644 include/linux/platform_data/davinci-aemif.h
>
Please split the DT and drivers/memory/* changes in two seperate
patches.

> diff --git a/Documentation/devicetree/bindings/memory/davinci-aemif.txt b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> new file mode 100644
> index 0000000..a79b3ed
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory/davinci-aemif.txt
> @@ -0,0 +1,103 @@
> +* 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.
> +- reg : contains offset/length value for AEMIF control registers space
> +- ranges : Each range corresponds to a single chipselect, and cover
> +           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.
> +
> +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)
> +- 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
> +- 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)
> +
> +if any of the above parameters are absent, hardware register default or that
> +set by a boot loader are used.
> +
> +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 {
> +		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>;
> +	};
> +
> +	nor_cs:cs3@62000000 {
> +		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>;
> +		ti,davinci-cs-asize = <1>;
> +	};
> +
> +	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
> +	};
> +
> +	flash@2,0 {
> +		compatible = "cfi-flash";
> +		reg = <2 0x0 0x400000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		bank-width = <2>;
> +		device-width = <2>;
> +	};
> +};
> diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
> index 067f311..2636a95 100644
> --- a/drivers/memory/Kconfig
> +++ b/drivers/memory/Kconfig
> @@ -40,4 +40,14 @@ config TEGRA30_MC
>   	  analysis, especially for IOMMU/SMMU(System Memory Management
>   	  Unit) module.
>
> +config TI_DAVINCI_AEMIF
> +	bool "Texas Instruments DaVinci AEMIF driver"
Add some default depends on ARCH here otherwise, this driver
will get build for all the generic builds and might show
build errors.

> +	help
> +	  This driver is for the AEMIF module available in Texas Instruments
> +	  SoCs. AEMIF stands for Asynchronous External Memory Interface and
> +	  is intended to provide a glue-less interface to a variety of
> +	  asynchronuous memory devices like ASRAM, NOR and NAND memory. A total
> +	  of 256M bytes of any of these memories can be accessed at a given
> +	  time via four chip selects with 64M byte access per chip select.
> +
>   endif
> diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
> index 42b3ce9..246aa61 100644
> --- a/drivers/memory/Makefile
> +++ b/drivers/memory/Makefile
> @@ -5,3 +5,4 @@
>   obj-$(CONFIG_TI_EMIF)		+= emif.o
>   obj-$(CONFIG_TEGRA20_MC)	+= tegra20-mc.o
>   obj-$(CONFIG_TEGRA30_MC)	+= tegra30-mc.o
> +obj-$(CONFIG_TI_DAVINCI_AEMIF)	+= davinci-aemif.o
> diff --git a/drivers/memory/davinci-aemif.c b/drivers/memory/davinci-aemif.c
> new file mode 100644
> index 0000000..27a6995
> --- /dev/null
> +++ b/drivers/memory/davinci-aemif.c
> @@ -0,0 +1,479 @@
> +/*
> + * AEMIF support for DaVinci SoCs
> + *
> + * Copyright (C) 2010 Texas Instruments Incorporated. http://www.ti.com/
s/2010/2012
> + * Copyright (C) Heiko Schocher <hs@denx.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/platform_data/davinci-aemif.h>
> +#include <linux/platform_device.h>
> +#include <linux/time.h>
> +
> +#define TA_SHIFT	2
> +#define RHOLD_SHIFT	4
> +#define RSTROBE_SHIFT	7
> +#define RSETUP_SHIFT	13
> +#define WHOLD_SHIFT	17
> +#define WSTROBE_SHIFT	20
> +#define WSETUP_SHIFT	26
> +#define EW_SHIFT	30
> +#define SS_SHIFT	31
> +
> +#define TA(x)		((x) << TA_SHIFT)
> +#define RHOLD(x)	((x) << RHOLD_SHIFT)
> +#define RSTROBE(x)	((x) << RSTROBE_SHIFT)
> +#define RSETUP(x)	((x) << RSETUP_SHIFT)
> +#define WHOLD(x)	((x) << WHOLD_SHIFT)
> +#define WSTROBE(x)	((x) << WSTROBE_SHIFT)
> +#define WSETUP(x)	((x) << WSETUP_SHIFT)
> +#define EW(x)		((x) << EW_SHIFT)
> +#define SS(x)		((x) << SS_SHIFT)
> +
Can you not use BIT(*) directly instead above magics

> +#define ASIZE_MAX	0x1
> +#define TA_MAX		0x3
> +#define RHOLD_MAX	0x7
> +#define RSTROBE_MAX	0x3f
> +#define RSETUP_MAX	0xf
> +#define WHOLD_MAX	0x7
> +#define WSTROBE_MAX	0x3f
> +#define WSETUP_MAX	0xf
> +#define EW_MAX		0x1
> +#define SS_MAX		0x1
> +#define NUM_CS		4
> +
> +#define TA_VAL(x)	(((x) & TA(TA_MAX)) >> TA_SHIFT)
> +#define RHOLD_VAL(x)	(((x) & RHOLD(RHOLD_MAX)) >> RHOLD_SHIFT)
> +#define RSTROBE_VAL(x)	(((x) & RSTROBE(RSTROBE_MAX)) >> RSTROBE_SHIFT)
> +#define RSETUP_VAL(x)	(((x) & RSETUP(RSETUP_MAX)) >> RSETUP_SHIFT)
> +#define WHOLD_VAL(x)	(((x) & WHOLD(WHOLD_MAX)) >> WHOLD_SHIFT)
> +#define WSTROBE_VAL(x)	(((x) & WSTROBE(WSTROBE_MAX)) >> WSTROBE_SHIFT)
> +#define WSETUP_VAL(x)	(((x) & WSETUP(WSETUP_MAX)) >> WSETUP_SHIFT)
> +#define EW_VAL(x)	(((x) & EW(EW_MAX)) >> EW_SHIFT)
> +#define SS_VAL(x)	(((x) & SS(SS_MAX)) >> SS_SHIFT)
> +
Same here..

> +
> +#define CONFIG_MASK	(TA(TA_MAX) | \
> +				RHOLD(RHOLD_MAX) | \
> +				RSTROBE(RSTROBE_MAX) |	\
> +				RSETUP(RSETUP_MAX) | \
> +				WHOLD(WHOLD_MAX) | \
> +				WSTROBE(WSTROBE_MAX) | \
> +				WSETUP(WSETUP_MAX) | \
> +				EW(EW_MAX) | SS(SS_MAX) | \
> +				ASIZE_MAX)
> +
This mask is next to impossible if reviewer needs to decode it.

> +#define DRV_NAME "davinci-aemif"
> +
> +struct aemif_device {
> +	struct davinci_aemif_pdata *cfg;
> +	void __iomem *base;
> +	struct clk *clk;
> +	/* clock rate in KHz */
> +	unsigned long clk_rate;
> +};
> +
> +static struct aemif_device *aemif;
> +/**
> + * aemif_calc_rate - calculate timing data.
> + * @wanted: The cycle time needed in nanoseconds.
> + * @clk: The input clock rate in kHz.
> + * @max: The maximum divider value that can be programmed.
> + *
> + * On success, returns the calculated timing value minus 1 for easy
> + * programming into AEMIF timing registers, else negative errno.
> + */
> +static int aemif_calc_rate(int wanted, unsigned long clk, int max)
> +{
> +	int result;
> +
> +	result = DIV_ROUND_UP((wanted * clk), NSEC_PER_MSEC) - 1;
> +
> +	pr_debug("%s: result %d from %ld, %d\n", __func__, result, clk, wanted);
> +
> +	/* It is generally OK to have a more relaxed timing than requested... */
> +	if (result < 0)
> +		result = 0;
> +
don't break if--else with un-ncessary new line here.

> +	/* ... But configuring tighter timings is not an option. */
> +	else if (result > max)
> +		result = -EINVAL;
> +
> +	return result;
> +}
> +
> +/**
> + * davinci_aemif_config_abus - configure async bus parameters given
> + * AEMIF interface
> + * @cs: chip-select to program the timing values for
> + * @data: aemif chip select configuration
> + * @base: aemif io mapped configuration base
> + *
> + * This function programs the given timing values (in real clock) into the
> + * AEMIF registers taking the AEMIF clock into account.
> + *
> + * This function does not use any locking while programming the AEMIF
> + * because it is expected that there is only one user of a given
> + * chip-select.
> + *
> + * Returns 0 on success, else negative errno.
> + */
> +static int davinci_aemif_config_abus(unsigned int cs,
> +				void __iomem *base,
> +				struct davinci_aemif_cs_data *data)
> +{
> +	int ta, rhold, rstrobe, rsetup, whold, wstrobe, wsetup;
> +	unsigned offset = A1CR_OFFSET + cs * 4;
> +	u32 set, val;
> +
> +	if (!data)
> +		return -EINVAL;
> +
> +	ta	= aemif_calc_rate(data->ta, aemif->clk_rate, TA_MAX);
> +	rhold	= aemif_calc_rate(data->rhold, aemif->clk_rate, RHOLD_MAX);
> +	rstrobe	= aemif_calc_rate(data->rstrobe, aemif->clk_rate, RSTROBE_MAX);
> +	rsetup	= aemif_calc_rate(data->rsetup, aemif->clk_rate, RSETUP_MAX);
> +	whold	= aemif_calc_rate(data->whold, aemif->clk_rate, WHOLD_MAX);
> +	wstrobe	= aemif_calc_rate(data->wstrobe, aemif->clk_rate, WSTROBE_MAX);
> +	wsetup	= aemif_calc_rate(data->wsetup, aemif->clk_rate, WSETUP_MAX);
> +
> +	if (ta < 0 || rhold < 0 || rstrobe < 0 || rsetup < 0 ||
> +			whold < 0 || wstrobe < 0 || wsetup < 0) {
> +		pr_err("%s: cannot get suitable timings\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	set = TA(ta) | RHOLD(rhold) | RSTROBE(rstrobe) | RSETUP(rsetup) |
> +		WHOLD(whold) | WSTROBE(wstrobe) | WSETUP(wsetup);
> +
> +	set |= (data->asize & ACR_ASIZE_MASK);
> +	if (data->enable_ew)
> +		set |= ACR_EW_MASK;
> +	if (data->enable_ss)
> +		set |= ACR_SS_MASK;
> +
> +	val = readl(aemif->base + offset);
> +	val &= ~CONFIG_MASK;
> +	val |= set;
> +	writel(val, aemif->base + offset);
> +
> +	return 0;
> +}
> +
> +inline int aemif_cycles_to_nsec(int val)
> +{
> +	return (val * NSEC_PER_MSEC) / aemif->clk_rate;
> +}
> +
> +/**
> + * davinci_aemif_get_hw_params - function to read hw register values
> + * @cs: chip select
> + * @data: ptr to cs data
> + *
> + * This function reads the defaults from the registers and update
> + * the timing values. Required for get/set commands and also for
> + * the case when driver needs to use defaults in hardware.
> + */
> +static void davinci_aemif_get_hw_params(int cs,
> +		struct davinci_aemif_cs_data *data)
> +{
> +	u32 val, offset = A1CR_OFFSET + cs * 4;
> +
> +	val = readl(aemif->base + offset);
> +	data->ta = aemif_cycles_to_nsec(TA_VAL(val));
> +	data->rhold = aemif_cycles_to_nsec(RHOLD_VAL(val));
> +	data->rstrobe = aemif_cycles_to_nsec(RSTROBE_VAL(val));
> +	data->rsetup = aemif_cycles_to_nsec(RSETUP_VAL(val));
> +	data->whold = aemif_cycles_to_nsec(WHOLD_VAL(val));
> +	data->wstrobe = aemif_cycles_to_nsec(WSTROBE_VAL(val));
> +	data->wsetup = aemif_cycles_to_nsec(WSETUP_VAL(val));
> +	data->enable_ew = EW_VAL(val);
> +	data->enable_ss = SS_VAL(val);
> +	data->asize = val & ASIZE_MAX;
> +}
> +
> +/**
> + * get_cs_data - helper function to get bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + */
> +static struct davinci_aemif_cs_data *get_cs_data(int cs)
> +{
> +	int i;
> +
> +	for (i = 0; i < aemif->cfg->num_cs; i++) {
> +		if (cs == aemif->cfg->cs_data[i].cs)
> +			break;
> +	}
> +
> +	if (i == aemif->cfg->num_cs)
> +		return NULL;
> +
> +	return &aemif->cfg->cs_data[i];
> +}
> +
> +/**
> + * davinci_aemif_set_abus_params - Set bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + * @data: ptr to a struct to hold the configuration data to be set
> + *
> + * This function is called to configure emif bus parameters for a given cs.
> + * Users call this function after calling davinci_aemif_get_abus_params()
> + * to get current parameters, modify and call this function
> + */
> +int davinci_aemif_set_abus_params(unsigned int cs,
> +			struct davinci_aemif_cs_data *data)
> +{
> +	struct davinci_aemif_cs_data *curr_cs_data;
> +	int ret = -EINVAL, chip_cs;
> +
> +	if (data == NULL)
> +		return ret;
> +
> +	if (aemif->base == NULL)
> +		return ret;
> +
> +	/* translate to chip CS which starts at 2 */
what is chip CS ? you can either use CS or chip select..
> +	chip_cs = cs + 2;
Use a macro like AEMIF_CS_START instead of hardcoding it.

> +
> +	curr_cs_data = get_cs_data(chip_cs);
> +	if (curr_cs_data) {
> +		/* for configuration we use cs since it is used to index ACR */
> +		ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
> +		if (!ret) {
> +			*curr_cs_data = *data;
> +			return 0;
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(davinci_aemif_set_abus_params);
> +
Who is the user of this EXPORT ?

> +/**
> + * davinci_aemif_get_abus_params - Get bus configuration data for a given cs
> + * @cs: chip-select, values 2-5
> + * returns: ptr to a struct having the current configuration data
> + */
> +struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned int cs)
> +{
> +	/* translate to chip CS which starts at 2 */
> +	return get_cs_data(cs + 2);
> +}
> +EXPORT_SYMBOL(davinci_aemif_get_abus_params);
> +
This one too.

[...]

> +static int __devinit davinci_aemif_probe(struct platform_device *pdev)
> +{
> +	struct davinci_aemif_pdata *cfg;
> +	int ret  = -ENODEV, i;
> +	struct resource *res;
> +
> +	aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
> +
> +	if (!aemif)
> +		return -ENOMEM;
> +
> +	aemif->clk = clk_get(NULL, "aemif");
Please use dev attributes. Above usage of clk_get isn't recommonded
in drivers. You might have to add alias entries in your clock data for
it to work though.

> +	if (IS_ERR(aemif->clk))
> +		return PTR_ERR(aemif->clk);
> +
> +	clk_prepare_enable(aemif->clk);
> +	aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
> +
/1000 for what ? Converting it into KHz ?

> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!res) {
> +		pr_err("No IO memory address defined\n");
> +		goto error;
> +	}
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	aemif->base = devm_request_and_ioremap(&pdev->dev, res);
> +	if (!aemif->base) {
> +		ret = -EBUSY;
> +		pr_err("ioremap failed\n");
> +		goto error;
> +	}
> +
> +	if (pdev->dev.platform_data == NULL) {
> +		/* Not platform data, we get the cs data from the cs nodes */
> +		cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
> +		if (cfg == NULL)
> +			return -ENOMEM;
> +
> +		aemif->cfg = cfg;
> +		if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
> +			pr_err("No platform data or cs of node present\n");
> +			goto error;
> +		}
> +	} else {
> +		cfg = pdev->dev.platform_data;
> +		aemif->cfg = cfg;
> +	}
> +
> +	for (i = 0; i < cfg->num_cs; i++) {
> +		/* cs is from 2-5. Internally we use cs-2 to access ACR */
> +		ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
> +				aemif->base, &cfg->cs_data[i]);
> +		if (ret < 0) {
> +			pr_err("Error configuring chip select %d\n",
> +				cfg->cs_data[i].cs);
> +			goto error;
> +		}
> +	}
> +	return 0;
> +error:
> +	clk_disable_unprepare(aemif->clk);
> +	clk_put(aemif->clk);
Alos unmap 'aemif->base' here..

> +	return ret;
> +}
> +
> +static struct platform_driver davinci_aemif_driver = {
> +	.probe = davinci_aemif_probe,
> +	.driver = {
> +		.name = DRV_NAME,
> +		.owner = THIS_MODULE,
> +		.of_match_table = davinci_aemif_of_match,
> +	},
> +};
> +
> +static int __init davinci_aemif_init(void)
> +{
> +	return platform_driver_register(&davinci_aemif_driver);
> +}
> +subsys_initcall(davinci_aemif_init);
> +
> +static void __exit davinci_aemif_exit(void)
> +{
> +	clk_disable_unprepare(aemif->clk);
> +	clk_put(aemif->clk);
> +	platform_driver_unregister(&davinci_aemif_driver);
> +}
> +module_exit(davinci_aemif_exit);
> +
> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
> +MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" DRV_NAME);
> diff --git a/include/linux/platform_data/davinci-aemif.h b/include/linux/platform_data/davinci-aemif.h
> new file mode 100644
> index 0000000..03f3ad0
> --- /dev/null
> +++ b/include/linux/platform_data/davinci-aemif.h
> @@ -0,0 +1,47 @@
> +/*
> + * TI DaVinci AEMIF support
> + *
> + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
> + *
s/2010/2012
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2. This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +#ifndef _MACH_DAVINCI_AEMIF_H
> +#define _MACH_DAVINCI_AEMIF_H
Fix the header guard as per new directory

Regards
Santosh

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

[Murali Karicheri]

Santhosh,

Thanks for the review. Some of the comments below applies to the 
original code as this driver is moved from mach-davinci to memory. I 
will fix them though. See below my response.
>> +
>> +    curr_cs_data = get_cs_data(chip_cs);
>> +    if (curr_cs_data) {
>> +        /* for configuration we use cs since it is used to index ACR */
>> +        ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
>> +        if (!ret) {
>> +            *curr_cs_data = *data;
>> +            return 0;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_set_abus_params);
>> +
> Who is the user of this EXPORT ?
Mostly not needed, but there is a use case for connecting FPGA that 
needs to do the configuration dynamically I guess. Want to hear from 
others if this is needed? Same for below comment. May be we can remove 
this now and add it later if needed.
>
>> +/**
>> + * davinci_aemif_get_abus_params - Get bus configuration data for a 
>> given cs
>> + * @cs: chip-select, values 2-5
>> + * returns: ptr to a struct having the current configuration data
>> + */
>> +struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned 
>> int cs)
>> +{
>> +    /* translate to chip CS which starts at 2 */
>> +    return get_cs_data(cs + 2);
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_get_abus_params);
>> +
> This one too.
>
> [...]
>
>> +static int __devinit davinci_aemif_probe(struct platform_device *pdev)
>> +{
>> +    struct davinci_aemif_pdata *cfg;
>> +    int ret  = -ENODEV, i;
>> +    struct resource *res;
>> +
>> +    aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
>> +
>> +    if (!aemif)
>> +        return -ENOMEM;
>> +
>> +    aemif->clk = clk_get(NULL, "aemif");
> Please use dev attributes. Above usage of clk_get isn't recommonded
> in drivers. You might have to add alias entries in your clock data for
> it to work though.
>
Need to investigate.
>> +    if (IS_ERR(aemif->clk))
>> +        return PTR_ERR(aemif->clk);
>> +
>> +    clk_prepare_enable(aemif->clk);
>> +    aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
>> +
> /1000 for what ? Converting it into KHz ?
>
Yes. WIll add a comment.
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    if (!res) {
>> +        pr_err("No IO memory address defined\n");
>> +        goto error;
>> +    }
>> +
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +    aemif->base = devm_request_and_ioremap(&pdev->dev, res);
>> +    if (!aemif->base) {
>> +        ret = -EBUSY;
>> +        pr_err("ioremap failed\n");
>> +        goto error;
>> +    }
>> +
>> +    if (pdev->dev.platform_data == NULL) {
>> +        /* Not platform data, we get the cs data from the cs nodes */
>> +        cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
>> +        if (cfg == NULL)
>> +            return -ENOMEM;
>> +
>> +        aemif->cfg = cfg;
>> +        if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
>> +            pr_err("No platform data or cs of node present\n");
>> +            goto error;
>> +        }
>> +    } else {
>> +        cfg = pdev->dev.platform_data;
>> +        aemif->cfg = cfg;
>> +    }
>> +
>> +    for (i = 0; i < cfg->num_cs; i++) {
>> +        /* cs is from 2-5. Internally we use cs-2 to access ACR */
>> +        ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
>> +                aemif->base, &cfg->cs_data[i]);
>> +        if (ret < 0) {
>> +            pr_err("Error configuring chip select %d\n",
>> +                cfg->cs_data[i].cs);
>> +            goto error;
>> +        }
>> +    }
>> +    return 0;
>> +error:
>> +    clk_disable_unprepare(aemif->clk);
>> +    clk_put(aemif->clk);
> Alos unmap 'aemif->base' here..
>
I thought devm_ API do this automatically. I will check.
>> +    return ret;
>> +}
>> +
>> +static struct platform_driver davinci_aemif_driver = {
>> +    .probe = davinci_aemif_probe,
>> +    .driver = {
>> +        .name = DRV_NAME,
>> +        .owner = THIS_MODULE,
>> +        .of_match_table = davinci_aemif_of_match,
>> +    },
>> +};
>> +
>> +static int __init davinci_aemif_init(void)
>> +{
>> +    return platform_driver_register(&davinci_aemif_driver);
>> +}
>> +subsys_initcall(davinci_aemif_init);
>> +
>> +static void __exit davinci_aemif_exit(void)
>> +{
>> +    clk_disable_unprepare(aemif->clk);
>> +    clk_put(aemif->clk);
>> +    platform_driver_unregister(&davinci_aemif_driver);
>> +}
>> +module_exit(davinci_aemif_exit);
>> +
>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>> +MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:" DRV_NAME);
>> diff --git a/include/linux/platform_data/davinci-aemif.h 
>> b/include/linux/platform_data/davinci-aemif.h
>> new file mode 100644
>> index 0000000..03f3ad0
>> --- /dev/null
>> +++ b/include/linux/platform_data/davinci-aemif.h
>> @@ -0,0 +1,47 @@
>> +/*
>> + * TI DaVinci AEMIF support
>> + *
>> + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
>> + *
> s/2010/2012
>> + * This file is licensed under the terms of the GNU General Public 
>> License
>> + * version 2. This program is licensed "as is" without any warranty 
>> of any
>> + * kind, whether express or implied.
>> + */
>> +#ifndef _MACH_DAVINCI_AEMIF_H
>> +#define _MACH_DAVINCI_AEMIF_H
> Fix the header guard as per new directory
>
Ok.
> Regards
> Santosh
>
>

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

[Murali Karicheri]

Santhosh,

Thanks for the review. Some of the comments below applies to the 
original code as this driver is moved from mach-davinci to memory. I 
will fix them though. See below my response.
>> +
>> +    curr_cs_data = get_cs_data(chip_cs);
>> +    if (curr_cs_data) {
>> +        /* for configuration we use cs since it is used to index ACR */
>> +        ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
>> +        if (!ret) {
>> +            *curr_cs_data = *data;
>> +            return 0;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_set_abus_params);
>> +
> Who is the user of this EXPORT ?
Mostly not needed, but there is a use case for connecting FPGA that 
needs to do the configuration dynamically I guess. Want to hear from 
others if this is needed? Same for below comment. May be we can remove 
this now and add it later if needed.
>
>> +/**
>> + * davinci_aemif_get_abus_params - Get bus configuration data for a 
>> given cs
>> + * @cs: chip-select, values 2-5
>> + * returns: ptr to a struct having the current configuration data
>> + */
>> +struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned 
>> int cs)
>> +{
>> +    /* translate to chip CS which starts at 2 */
>> +    return get_cs_data(cs + 2);
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_get_abus_params);
>> +
> This one too.
>
> [...]
>
>> +static int __devinit davinci_aemif_probe(struct platform_device *pdev)
>> +{
>> +    struct davinci_aemif_pdata *cfg;
>> +    int ret  = -ENODEV, i;
>> +    struct resource *res;
>> +
>> +    aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
>> +
>> +    if (!aemif)
>> +        return -ENOMEM;
>> +
>> +    aemif->clk = clk_get(NULL, "aemif");
> Please use dev attributes. Above usage of clk_get isn't recommonded
> in drivers. You might have to add alias entries in your clock data for
> it to work though.
>
Need to investigate.
>> +    if (IS_ERR(aemif->clk))
>> +        return PTR_ERR(aemif->clk);
>> +
>> +    clk_prepare_enable(aemif->clk);
>> +    aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
>> +
> /1000 for what ? Converting it into KHz ?
>
Yes. WIll add a comment.
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    if (!res) {
>> +        pr_err("No IO memory address defined\n");
>> +        goto error;
>> +    }
>> +
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +    aemif->base = devm_request_and_ioremap(&pdev->dev, res);
>> +    if (!aemif->base) {
>> +        ret = -EBUSY;
>> +        pr_err("ioremap failed\n");
>> +        goto error;
>> +    }
>> +
>> +    if (pdev->dev.platform_data == NULL) {
>> +        /* Not platform data, we get the cs data from the cs nodes */
>> +        cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
>> +        if (cfg == NULL)
>> +            return -ENOMEM;
>> +
>> +        aemif->cfg = cfg;
>> +        if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
>> +            pr_err("No platform data or cs of node present\n");
>> +            goto error;
>> +        }
>> +    } else {
>> +        cfg = pdev->dev.platform_data;
>> +        aemif->cfg = cfg;
>> +    }
>> +
>> +    for (i = 0; i < cfg->num_cs; i++) {
>> +        /* cs is from 2-5. Internally we use cs-2 to access ACR */
>> +        ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
>> +                aemif->base, &cfg->cs_data[i]);
>> +        if (ret < 0) {
>> +            pr_err("Error configuring chip select %d\n",
>> +                cfg->cs_data[i].cs);
>> +            goto error;
>> +        }
>> +    }
>> +    return 0;
>> +error:
>> +    clk_disable_unprepare(aemif->clk);
>> +    clk_put(aemif->clk);
> Alos unmap 'aemif->base' here..
>
I thought devm_ API do this automatically. I will check.
>> +    return ret;
>> +}
>> +
>> +static struct platform_driver davinci_aemif_driver = {
>> +    .probe = davinci_aemif_probe,
>> +    .driver = {
>> +        .name = DRV_NAME,
>> +        .owner = THIS_MODULE,
>> +        .of_match_table = davinci_aemif_of_match,
>> +    },
>> +};
>> +
>> +static int __init davinci_aemif_init(void)
>> +{
>> +    return platform_driver_register(&davinci_aemif_driver);
>> +}
>> +subsys_initcall(davinci_aemif_init);
>> +
>> +static void __exit davinci_aemif_exit(void)
>> +{
>> +    clk_disable_unprepare(aemif->clk);
>> +    clk_put(aemif->clk);
>> +    platform_driver_unregister(&davinci_aemif_driver);
>> +}
>> +module_exit(davinci_aemif_exit);
>> +
>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>> +MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:" DRV_NAME);
>> diff --git a/include/linux/platform_data/davinci-aemif.h 
>> b/include/linux/platform_data/davinci-aemif.h
>> new file mode 100644
>> index 0000000..03f3ad0
>> --- /dev/null
>> +++ b/include/linux/platform_data/davinci-aemif.h
>> @@ -0,0 +1,47 @@
>> +/*
>> + * TI DaVinci AEMIF support
>> + *
>> + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
>> + *
> s/2010/2012
>> + * This file is licensed under the terms of the GNU General Public 
>> License
>> + * version 2. This program is licensed "as is" without any warranty 
>> of any
>> + * kind, whether express or implied.
>> + */
>> +#ifndef _MACH_DAVINCI_AEMIF_H
>> +#define _MACH_DAVINCI_AEMIF_H
> Fix the header guard as per new directory
>
Ok.
> Regards
> Santosh
>
>

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

[Murali Karicheri]

Santhosh,

Thanks for the review. Some of the comments below applies to the 
original code as this driver is moved from mach-davinci to memory. I 
will fix them though. See below my response.
>> +
>> +    curr_cs_data = get_cs_data(chip_cs);
>> +    if (curr_cs_data) {
>> +        /* for configuration we use cs since it is used to index ACR */
>> +        ret = davinci_aemif_config_abus(chip_cs, aemif->base, data);
>> +        if (!ret) {
>> +            *curr_cs_data = *data;
>> +            return 0;
>> +        }
>> +    }
>> +
>> +    return ret;
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_set_abus_params);
>> +
> Who is the user of this EXPORT ?
Mostly not needed, but there is a use case for connecting FPGA that 
needs to do the configuration dynamically I guess. Want to hear from 
others if this is needed? Same for below comment. May be we can remove 
this now and add it later if needed.
>
>> +/**
>> + * davinci_aemif_get_abus_params - Get bus configuration data for a 
>> given cs
>> + * @cs: chip-select, values 2-5
>> + * returns: ptr to a struct having the current configuration data
>> + */
>> +struct davinci_aemif_cs_data *davinci_aemif_get_abus_params(unsigned 
>> int cs)
>> +{
>> +    /* translate to chip CS which starts at 2 */
>> +    return get_cs_data(cs + 2);
>> +}
>> +EXPORT_SYMBOL(davinci_aemif_get_abus_params);
>> +
> This one too.
>
> [...]
>
>> +static int __devinit davinci_aemif_probe(struct platform_device *pdev)
>> +{
>> +    struct davinci_aemif_pdata *cfg;
>> +    int ret  = -ENODEV, i;
>> +    struct resource *res;
>> +
>> +    aemif = devm_kzalloc(&pdev->dev, sizeof(*aemif), GFP_KERNEL);
>> +
>> +    if (!aemif)
>> +        return -ENOMEM;
>> +
>> +    aemif->clk = clk_get(NULL, "aemif");
> Please use dev attributes. Above usage of clk_get isn't recommonded
> in drivers. You might have to add alias entries in your clock data for
> it to work though.
>
Need to investigate.
>> +    if (IS_ERR(aemif->clk))
>> +        return PTR_ERR(aemif->clk);
>> +
>> +    clk_prepare_enable(aemif->clk);
>> +    aemif->clk_rate = clk_get_rate(aemif->clk) / 1000;
>> +
> /1000 for what ? Converting it into KHz ?
>
Yes. WIll add a comment.
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +    if (!res) {
>> +        pr_err("No IO memory address defined\n");
>> +        goto error;
>> +    }
>> +
>> +    res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +
>> +    aemif->base = devm_request_and_ioremap(&pdev->dev, res);
>> +    if (!aemif->base) {
>> +        ret = -EBUSY;
>> +        pr_err("ioremap failed\n");
>> +        goto error;
>> +    }
>> +
>> +    if (pdev->dev.platform_data == NULL) {
>> +        /* Not platform data, we get the cs data from the cs nodes */
>> +        cfg = devm_kzalloc(&pdev->dev, sizeof(*cfg), GFP_KERNEL);
>> +        if (cfg == NULL)
>> +            return -ENOMEM;
>> +
>> +        aemif->cfg = cfg;
>> +        if (of_davinci_aemif_cs_init(pdev->dev.of_node) < 0) {
>> +            pr_err("No platform data or cs of node present\n");
>> +            goto error;
>> +        }
>> +    } else {
>> +        cfg = pdev->dev.platform_data;
>> +        aemif->cfg = cfg;
>> +    }
>> +
>> +    for (i = 0; i < cfg->num_cs; i++) {
>> +        /* cs is from 2-5. Internally we use cs-2 to access ACR */
>> +        ret = davinci_aemif_config_abus(cfg->cs_data[i].cs - 2,
>> +                aemif->base, &cfg->cs_data[i]);
>> +        if (ret < 0) {
>> +            pr_err("Error configuring chip select %d\n",
>> +                cfg->cs_data[i].cs);
>> +            goto error;
>> +        }
>> +    }
>> +    return 0;
>> +error:
>> +    clk_disable_unprepare(aemif->clk);
>> +    clk_put(aemif->clk);
> Alos unmap 'aemif->base' here..
>
I thought devm_ API do this automatically. I will check.
>> +    return ret;
>> +}
>> +
>> +static struct platform_driver davinci_aemif_driver = {
>> +    .probe = davinci_aemif_probe,
>> +    .driver = {
>> +        .name = DRV_NAME,
>> +        .owner = THIS_MODULE,
>> +        .of_match_table = davinci_aemif_of_match,
>> +    },
>> +};
>> +
>> +static int __init davinci_aemif_init(void)
>> +{
>> +    return platform_driver_register(&davinci_aemif_driver);
>> +}
>> +subsys_initcall(davinci_aemif_init);
>> +
>> +static void __exit davinci_aemif_exit(void)
>> +{
>> +    clk_disable_unprepare(aemif->clk);
>> +    clk_put(aemif->clk);
>> +    platform_driver_unregister(&davinci_aemif_driver);
>> +}
>> +module_exit(davinci_aemif_exit);
>> +
>> +MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
>> +MODULE_DESCRIPTION("Texas Instruments AEMIF driver");
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_ALIAS("platform:" DRV_NAME);
>> diff --git a/include/linux/platform_data/davinci-aemif.h 
>> b/include/linux/platform_data/davinci-aemif.h
>> new file mode 100644
>> index 0000000..03f3ad0
>> --- /dev/null
>> +++ b/include/linux/platform_data/davinci-aemif.h
>> @@ -0,0 +1,47 @@
>> +/*
>> + * TI DaVinci AEMIF support
>> + *
>> + * Copyright 2010 (C) Texas Instruments, Inc. http://www.ti.com/
>> + *
> s/2010/2012
>> + * This file is licensed under the terms of the GNU General Public 
>> License
>> + * version 2. This program is licensed "as is" without any warranty 
>> of any
>> + * kind, whether express or implied.
>> + */
>> +#ifndef _MACH_DAVINCI_AEMIF_H
>> +#define _MACH_DAVINCI_AEMIF_H
> Fix the header guard as per new directory
>
Ok.
> Regards
> Santosh
>
>

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

[Stephen Warren]

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
> +	};

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

[Stephen Warren]

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
> +	};

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Stephen Warren]

On 11/06/2012 02:47 PM, Murali Karicheri wrote:
> DaVinci NAND driver is a controller driver based on the AEMIF hardware
> IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
> patch removes the driver dependency on DaVinci architecture so that it
> can be used on other architectures such as c6x, keystone etc.
> 
> Also migrate the driver to use the new AEMIF platform driver API and
> moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
> as this is expected to be used outside of arm/davinci.

>  delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
>  create mode 100644 include/linux/platform_data/davinci-nand.h

Using "git format-patch -M" might show this as a file move/rename rather
than a delete/add, which would be useful to highlight any changes you
made at the same time.

> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt

> +Example (enbw_cmc board):
> +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@3,0 {

Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND accesses?

> +		compatible = "ti,davinci-nand";
> +		reg = <3 0x0 0x807ff
> +			6 0x0 0x8000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ti,davinci-chipselect = <1>;

So I don't understand why that chipselect property is needed, or has a
different value. Is this muxing the AEMIF output chip-selects onto
different SoC package pins or something? Seems like a job for pinctrl
perhaps?

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Stephen Warren]

On 11/06/2012 02:47 PM, Murali Karicheri wrote:
> DaVinci NAND driver is a controller driver based on the AEMIF hardware
> IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
> patch removes the driver dependency on DaVinci architecture so that it
> can be used on other architectures such as c6x, keystone etc.
> 
> Also migrate the driver to use the new AEMIF platform driver API and
> moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
> as this is expected to be used outside of arm/davinci.

>  delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
>  create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
>  create mode 100644 include/linux/platform_data/davinci-nand.h

Using "git format-patch -M" might show this as a file move/rename rather
than a delete/add, which would be useful to highlight any changes you
made at the same time.

> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt

> +Example (enbw_cmc board):
> +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@3,0 {

Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND accesses?

> +		compatible = "ti,davinci-nand";
> +		reg = <3 0x0 0x807ff
> +			6 0x0 0x8000>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ti,davinci-chipselect = <1>;

So I don't understand why that chipselect property is needed, or has a
different value. Is this muxing the AEMIF output chip-selects onto
different SoC package pins or something? Seems like a job for pinctrl
perhaps?

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

[Murali Karicheri]

Stephen,

Thanks for reviewing this. See my responses below
On 11/07/2012 03:05 PM, Stephen Warren wrote:
> 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.
I have re-used the bindings from another patch and don't know why the 
above description is used.
I am using a value of 2 for address cells and 1 for size-cells which I 
believe is how these will be used as shown below.

                         ranges = <2 0 0x70000000 0x08000000
                                 3 0 0x74000000 0x04000000
                                 4 0 0x78000000 0x04000000
                                 5 0 0x7C000000 0x04000000
                                 6 0 0x20c00000 0x100>;

So I will change the description as below.

- compatible: must be "ti,davinci-aemif"
- reg: AEMIF control registers base and size
- #address-cells: must be 2 (chip-select + offset)
- #size-cells: must be 1
- ranges: mapping from EMIF space to parent space. Each range 
corresponds to a single chip select, and covers the entire access window 
as configured.
>> +- 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.
Ok
>> +           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.
Ok. Will fix,
>
>> +- 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.
Ok.
>
> 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?
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.
>> +- 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.
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-enable-ss - "on", "off"

As cs node is optional, If not present, hw values will be used. I don't 
think it make sense to make it tri-state based on my explanation above.
>> +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.
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.
> 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?
>
  To make this simple, I will drop the unit address from cs node and 
represent it as

nand_cs:cs2 {

};

nor_cs:cs3 {

};


+		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
>> +	};
>
>
>

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

[Murali Karicheri]

Stephen,

Thanks for reviewing this. See my responses below
On 11/07/2012 03:05 PM, Stephen Warren wrote:
> 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.
I have re-used the bindings from another patch and don't know why the 
above description is used.
I am using a value of 2 for address cells and 1 for size-cells which I 
believe is how these will be used as shown below.

                         ranges = <2 0 0x70000000 0x08000000
                                 3 0 0x74000000 0x04000000
                                 4 0 0x78000000 0x04000000
                                 5 0 0x7C000000 0x04000000
                                 6 0 0x20c00000 0x100>;

So I will change the description as below.

- compatible: must be "ti,davinci-aemif"
- reg: AEMIF control registers base and size
- #address-cells: must be 2 (chip-select + offset)
- #size-cells: must be 1
- ranges: mapping from EMIF space to parent space. Each range 
corresponds to a single chip select, and covers the entire access window 
as configured.
>> +- 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.
Ok
>> +           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.
Ok. Will fix,
>
>> +- 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.
Ok.
>
> 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?
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.
>> +- 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.
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-enable-ss - "on", "off"

As cs node is optional, If not present, hw values will be used. I don't 
think it make sense to make it tri-state based on my explanation above.
>> +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.
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.
> 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?
>
  To make this simple, I will drop the unit address from cs node and 
represent it as

nand_cs:cs2 {

};

nor_cs:cs3 {

};


+		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
>> +	};
>
>
>

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

[Murali Karicheri]

Stephen,

Thanks for reviewing this. See my responses below
On 11/07/2012 03:05 PM, Stephen Warren wrote:
> 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.
I have re-used the bindings from another patch and don't know why the 
above description is used.
I am using a value of 2 for address cells and 1 for size-cells which I 
believe is how these will be used as shown below.

                         ranges = <2 0 0x70000000 0x08000000
                                 3 0 0x74000000 0x04000000
                                 4 0 0x78000000 0x04000000
                                 5 0 0x7C000000 0x04000000
                                 6 0 0x20c00000 0x100>;

So I will change the description as below.

- compatible: must be "ti,davinci-aemif"
- reg: AEMIF control registers base and size
- #address-cells: must be 2 (chip-select + offset)
- #size-cells: must be 1
- ranges: mapping from EMIF space to parent space. Each range 
corresponds to a single chip select, and covers the entire access window 
as configured.
>> +- 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.
Ok
>> +           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.
Ok. Will fix,
>
>> +- 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.
Ok.
>
> 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?
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.
>> +- 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.
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-enable-ss - "on", "off"

As cs node is optional, If not present, hw values will be used. I don't 
think it make sense to make it tri-state based on my explanation above.
>> +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.
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.
> 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?
>
  To make this simple, I will drop the unit address from cs node and 
represent it as

nand_cs:cs2 {

};

nor_cs:cs3 {

};


+		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
>> +	};
>
>
>

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Murali Karicheri]

On 11/07/2012 03:08 PM, Stephen Warren wrote:
> On 11/06/2012 02:47 PM, Murali Karicheri wrote:
>> DaVinci NAND driver is a controller driver based on the AEMIF hardware
>> IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
>> patch removes the driver dependency on DaVinci architecture so that it
>> can be used on other architectures such as c6x, keystone etc.
>>
>> Also migrate the driver to use the new AEMIF platform driver API and
>> moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> as this is expected to be used outside of arm/davinci.
>>   delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
>>   create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>   create mode 100644 include/linux/platform_data/davinci-nand.h
> Using "git format-patch -M" might show this as a file move/rename rather
> than a delete/add, which would be useful to highlight any changes you
> made at the same time.
>
>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> +Example (enbw_cmc board):
>> +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@3,0 {
> Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND accesses?
>
Yes.
>> +		compatible = "ti,davinci-nand";
>> +		reg = <3 0x0 0x807ff
>> +			6 0x0 0x8000>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ti,davinci-chipselect = <1>;
> So I don't understand why that chipselect property is needed, or has a
> different value. Is this muxing the AEMIF output chip-selects onto
> different SoC package pins or something? Seems like a job for pinctrl
> perhaps?
Actually this was added by somebody else. Do you know what 0 in 3,0 
stands for? Is there a way I can retrieve the chip-select id so that I 
can remove the davinci-chipselect property. The driver uses a cs index 
of 0-3 and the hardware documentation refers CS2-5. Actually cs2 is CE0 
signal. So internally driver
translates to 2-5 to 0-3. pinmux is currently done in platform specific 
init code and probably need to migrate to use pictrl later.

Murali
>
>

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Murali Karicheri]

On 11/07/2012 03:08 PM, Stephen Warren wrote:
> On 11/06/2012 02:47 PM, Murali Karicheri wrote:
>> DaVinci NAND driver is a controller driver based on the AEMIF hardware
>> IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
>> patch removes the driver dependency on DaVinci architecture so that it
>> can be used on other architectures such as c6x, keystone etc.
>>
>> Also migrate the driver to use the new AEMIF platform driver API and
>> moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> as this is expected to be used outside of arm/davinci.
>>   delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
>>   create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>   create mode 100644 include/linux/platform_data/davinci-nand.h
> Using "git format-patch -M" might show this as a file move/rename rather
> than a delete/add, which would be useful to highlight any changes you
> made at the same time.
>
>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> +Example (enbw_cmc board):
>> +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@3,0 {
> Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND accesses?
>
Yes.
>> +		compatible = "ti,davinci-nand";
>> +		reg = <3 0x0 0x807ff
>> +			6 0x0 0x8000>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ti,davinci-chipselect = <1>;
> So I don't understand why that chipselect property is needed, or has a
> different value. Is this muxing the AEMIF output chip-selects onto
> different SoC package pins or something? Seems like a job for pinctrl
> perhaps?
Actually this was added by somebody else. Do you know what 0 in 3,0 
stands for? Is there a way I can retrieve the chip-select id so that I 
can remove the davinci-chipselect property. The driver uses a cs index 
of 0-3 and the hardware documentation refers CS2-5. Actually cs2 is CE0 
signal. So internally driver
translates to 2-5 to 0-3. pinmux is currently done in platform specific 
init code and probably need to migrate to use pictrl later.

Murali
>
>

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Murali Karicheri]

On 11/07/2012 03:08 PM, Stephen Warren wrote:
> On 11/06/2012 02:47 PM, Murali Karicheri wrote:
>> DaVinci NAND driver is a controller driver based on the AEMIF hardware
>> IP found on TI SoCs. It is also used on SoCs that are not DaVinci based. This
>> patch removes the driver dependency on DaVinci architecture so that it
>> can be used on other architectures such as c6x, keystone etc.
>>
>> Also migrate the driver to use the new AEMIF platform driver API and
>> moving Documentation to Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> as this is expected to be used outside of arm/davinci.
>>   delete mode 100644 Documentation/devicetree/bindings/arm/davinci/nand.txt
>>   create mode 100644 Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>   create mode 100644 include/linux/platform_data/davinci-nand.h
> Using "git format-patch -M" might show this as a file move/rename rather
> than a delete/add, which would be useful to highlight any changes you
> made at the same time.
>
>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>> +Example (enbw_cmc board):
>> +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@3,0 {
> Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND accesses?
>
Yes.
>> +		compatible = "ti,davinci-nand";
>> +		reg = <3 0x0 0x807ff
>> +			6 0x0 0x8000>;
>> +		#address-cells = <1>;
>> +		#size-cells = <1>;
>> +		ti,davinci-chipselect = <1>;
> So I don't understand why that chipselect property is needed, or has a
> different value. Is this muxing the AEMIF output chip-selects onto
> different SoC package pins or something? Seems like a job for pinctrl
> perhaps?
Actually this was added by somebody else. Do you know what 0 in 3,0 
stands for? Is there a way I can retrieve the chip-select id so that I 
can remove the davinci-chipselect property. The driver uses a cs index 
of 0-3 and the hardware documentation refers CS2-5. Actually cs2 is CE0 
signal. So internally driver
translates to 2-5 to 0-3. pinmux is currently done in platform specific 
init code and probably need to migrate to use pictrl later.

Murali
>
>

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Stephen Warren]

On 11/08/2012 08:57 AM, Murali Karicheri wrote:
> On 11/07/2012 03:08 PM, Stephen Warren wrote:
>> On 11/06/2012 02:47 PM, Murali Karicheri wrote:
>>> DaVinci NAND driver is a controller driver based on the AEMIF hardware
>>> IP found on TI SoCs. It is also used on SoCs that are not DaVinci
>>> based. This
>>> patch removes the driver dependency on DaVinci architecture so that it
>>> can be used on other architectures such as c6x, keystone etc.
>>>
>>> Also migrate the driver to use the new AEMIF platform driver API and
>>> moving Documentation to
>>> Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> as this is expected to be used outside of arm/davinci.
>>>   delete mode 100644
>>> Documentation/devicetree/bindings/arm/davinci/nand.txt
>>>   create mode 100644
>>> Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>>   create mode 100644 include/linux/platform_data/davinci-nand.h
>> Using "git format-patch -M" might show this as a file move/rename rather
>> than a delete/add, which would be useful to highlight any changes you
>> made at the same time.
>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> +Example (enbw_cmc board):
>>> +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@3,0 {
>> Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND
>> accesses?
>>
> Yes.
>>> +        compatible = "ti,davinci-nand";
>>> +        reg = <3 0x0 0x807ff
>>> +            6 0x0 0x8000>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +        ti,davinci-chipselect = <1>;
>> So I don't understand why that chipselect property is needed, or has a
>> different value. Is this muxing the AEMIF output chip-selects onto
>> different SoC package pins or something? Seems like a job for pinctrl
>> perhaps?
>
> Actually this was added by somebody else. Do you know what 0 in 3,0
> stands for? Is there a way I can retrieve the chip-select id so that I
> can remove the davinci-chipselect property. The driver uses a cs index
> of 0-3 and the hardware documentation refers CS2-5. Actually cs2 is CE0
> signal. So internally driver
> translates to 2-5 to 0-3. pinmux is currently done in platform specific
> init code and probably need to migrate to use pictrl later.

for a node named "nand@3,0", the "3,0" is the address value from the
first entry in the reg property "reg = <3 0x0 0x807ff ...". Given your
previous email, that means chip-select 3 offset 0, I believe. Presumably
you can read the reg property directly to find these numbers, or perhaps
there are already some helper functions for this. I have no idea why
there's a "3" in the node name and reg property, but
"ti,davinci-chipselect = <1>" not "= <3>".

Re: [RFC v2 PATCH 2/2] mtd: davinci - remove DaVinci architecture depedency

[Stephen Warren]

On 11/08/2012 08:57 AM, Murali Karicheri wrote:
> On 11/07/2012 03:08 PM, Stephen Warren wrote:
>> On 11/06/2012 02:47 PM, Murali Karicheri wrote:
>>> DaVinci NAND driver is a controller driver based on the AEMIF hardware
>>> IP found on TI SoCs. It is also used on SoCs that are not DaVinci
>>> based. This
>>> patch removes the driver dependency on DaVinci architecture so that it
>>> can be used on other architectures such as c6x, keystone etc.
>>>
>>> Also migrate the driver to use the new AEMIF platform driver API and
>>> moving Documentation to
>>> Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> as this is expected to be used outside of arm/davinci.
>>>   delete mode 100644
>>> Documentation/devicetree/bindings/arm/davinci/nand.txt
>>>   create mode 100644
>>> Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>>   create mode 100644 include/linux/platform_data/davinci-nand.h
>> Using "git format-patch -M" might show this as a file move/rename rather
>> than a delete/add, which would be useful to highlight any changes you
>> made at the same time.
>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> b/Documentation/devicetree/bindings/mtd/davinci-nand.txt
>>> +Example (enbw_cmc board):
>>> +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@3,0 {
>> Here, isn't 3,0 the aemif chip-select ID that is decoding the NAND
>> accesses?
>>
> Yes.
>>> +        compatible = "ti,davinci-nand";
>>> +        reg = <3 0x0 0x807ff
>>> +            6 0x0 0x8000>;
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +        ti,davinci-chipselect = <1>;
>> So I don't understand why that chipselect property is needed, or has a
>> different value. Is this muxing the AEMIF output chip-selects onto
>> different SoC package pins or something? Seems like a job for pinctrl
>> perhaps?
>
> Actually this was added by somebody else. Do you know what 0 in 3,0
> stands for? Is there a way I can retrieve the chip-select id so that I
> can remove the davinci-chipselect property. The driver uses a cs index
> of 0-3 and the hardware documentation refers CS2-5. Actually cs2 is CE0
> signal. So internally driver
> translates to 2-5 to 0-3. pinmux is currently done in platform specific
> init code and probably need to migrate to use pictrl later.

for a node named "nand@3,0", the "3,0" is the address value from the
first entry in the reg property "reg = <3 0x0 0x807ff ...". Given your
previous email, that means chip-select 3 offset 0, I believe. Presumably
you can read the reg property directly to find these numbers, or perhaps
there are already some helper functions for this. I have no idea why
there's a "3" in the node name and reg property, but
"ti,davinci-chipselect = <1>" not "= <3>".