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-kariche...@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
_______________________________________________
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source