Re: [PATCH v4 1/2] memory: ti-aemif: introduce AEMIF driver
From: Ivan Khoronzhuk
Date: Wed Feb 19 2014 - 05:33:36 EST
On 02/18/2014 10:30 PM, Greg KH wrote:
On Wed, Feb 05, 2014 at 09:46:13PM +0200, Ivan Khoronzhuk wrote:
Add new AEMIF driver for EMIF16 Texas Instruments controller.
The EMIF16 module is intended to provide a glue-less interface to
a variety of asynchronous memory devices like ASRA M, NOR and NAND
memory. A total of 256M bytes of any of these memories can be
accessed at any given time via 4 chip selects with 64M byte access
per chip select.
Synchronous memories such as DDR1 SD RAM, SDR SDRAM and Mobile SDR
are not supported.
This controller is used on SoCs like Davinci, Keysone2
Acked-by: Santosh Shilimkar <santosh.shilimkar@xxxxxx
Signed-off-by: [initial author] Murali Karicheri <m-karicheri2@xxxxxx>
What's this [] stuff?
If Murali wrote this, that name needs to be in a "From:" line in the
patch to properly attribute it, and drop the [] here.
Ok, I'll delete [] stuff.
Signed-off-by: Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxx>
---
drivers/memory/Kconfig | 11 ++
drivers/memory/Makefile | 1 +
drivers/memory/ti-aemif.c | 429 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 441 insertions(+)
create mode 100644 drivers/memory/ti-aemif.c
diff --git a/drivers/memory/Kconfig b/drivers/memory/Kconfig
index 29a11db..7bc3982 100644
--- a/drivers/memory/Kconfig
+++ b/drivers/memory/Kconfig
@@ -7,6 +7,17 @@ menuconfig MEMORY
if MEMORY
+config TI_AEMIF
+ tristate "Texas Instruments AEMIF driver"
+ depends on (ARCH_DAVINCI || ARCH_KEYSTONE) && OF
+ 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.
+
config TI_EMIF
tristate "Texas Instruments EMIF driver"
depends on ARCH_OMAP2PLUS
diff --git a/drivers/memory/Makefile b/drivers/memory/Makefile
index 969d923..d4e150c 100644
--- a/drivers/memory/Makefile
+++ b/drivers/memory/Makefile
@@ -5,6 +5,7 @@
ifeq ($(CONFIG_DDR),y)
obj-$(CONFIG_OF) += of_memory.o
endif
+obj-$(CONFIG_TI_AEMIF) += ti-aemif.o
obj-$(CONFIG_TI_EMIF) += emif.o
obj-$(CONFIG_MVEBU_DEVBUS) += mvebu-devbus.o
obj-$(CONFIG_TEGRA20_MC) += tegra20-mc.o
diff --git a/drivers/memory/ti-aemif.c b/drivers/memory/ti-aemif.c
new file mode 100644
index 0000000..8d15d87
--- /dev/null
+++ b/drivers/memory/ti-aemif.c
@@ -0,0 +1,429 @@
+/*
+ * TI AEMIF driver
+ *
+ * Copyright (C) 2010 - 2013 Texas Instruments Incorporated. http://www.ti.com/
+ *
+ * Authors:
+ * Murali Karicheri <m-karicheri2@xxxxxx>
+ * Ivan Khoronzhuk <ivan.khoronzhuk@xxxxxx>
+ *
+ * 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_platform.h>
+#include <linux/platform_device.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 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
+
+#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 "ti-aemif"
What's wrong with KBUILD_MODNAME?
Yes, I'll drop DRV_NAME in favour of KBUILD_MODNAME
+static int aemif_probe(struct platform_device *pdev)
+{
+ int ret = -ENODEV, i;
Extra spaces?
Split this into two lines?
Ok.
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+ struct device_node *child_np;
+ struct aemif_device *aemif;
+
+ if (np == NULL)
+ return 0;
+
+ aemif = devm_kzalloc(dev, sizeof(*aemif), GFP_KERNEL);
+ if (!aemif) {
+ dev_err(dev, "cannot allocate memory for aemif\n");
Not needed, you'll get a message from kzalloc about this.\
Ok.
+ return -ENOMEM;
+ }
+
+ platform_set_drvdata(pdev, aemif);
+
+ aemif->clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(aemif->clk)) {
+ dev_err(dev, "cannot get clock 'aemif'\n");
+ return PTR_ERR(aemif->clk);
No freeing memory?
There is no need to free memory explicitly.
devm_kzalloc is used instead of kzalloc.
+ }
+
+ clk_prepare_enable(aemif->clk);
+ aemif->clk_rate = clk_get_rate(aemif->clk) / MSEC_PER_SEC;
+
+ if (of_device_is_compatible(np, "ti,da850-aemif"))
+ aemif->cs_offset = 2;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ aemif->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(aemif->base)) {
+ ret = PTR_ERR(aemif->base);
+ goto error;
+ }
+
+ /*
+ * 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.
+ */
+ for_each_available_child_of_node(np, child_np) {
+ ret = of_aemif_parse_abus_config(pdev, child_np);
+ if (ret < 0)
+ goto error;
+ }
+
+ for (i = 0; i < aemif->num_cs; i++) {
+ ret = aemif_config_abus(pdev, i);
+ if (ret < 0) {
+ dev_err(dev, "Error configuring chip select %d\n",
+ aemif->cs_data[i].cs);
+ goto error;
+ }
+ }
+
+ /*
+ * Create a child devices explicitly from here to
+ * guarantee that the child will be probed after the AEMIF timing
+ * parameters are set.
+ */
+ for_each_available_child_of_node(np, child_np) {
+ ret = of_platform_populate(child_np, NULL, NULL, dev);
+ if (ret < 0)
+ goto error;
+ }
+
+ return 0;
+error:
+ clk_disable_unprepare(aemif->clk);
+ return ret;
No freeing memory?
devm_* allocations were used, so no need.
+}
+
+static int aemif_remove(struct platform_device *pdev)
+{
+ struct aemif_device *aemif = platform_get_drvdata(pdev);
+ clk_disable_unprepare(aemif->clk);
Extra line needed.
Ok
Thanks!
--
Regards,
Ivan Khoronzhuk
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/