Hi Grant,
On Friday 09 March 2012 11:07 AM, Grant Likely wrote:
On Thu, 8 Mar 2012 22:03:57 +0530, Aneesh V<aneesh@xxxxxx> wrote:
Cc: Rajendra Nayak<rnayak@xxxxxx>
Cc: Benoit Cousson<b-cousson@xxxxxx>
Signed-off-by: Aneesh V<aneesh@xxxxxx>
---
Changes since RFC v4:
- Rebased to the latest version of EMIF series
- Replace kzalloc()/kfree() with devm_* variants
---
drivers/misc/emif.c | 289 ++++++++++++++++++++++++++++++++++++++++++++++++++-
1 files changed, 288 insertions(+), 1 deletions(-)
diff --git a/drivers/misc/emif.c b/drivers/misc/emif.c
index 79fb161..0aaa61e 100644
--- a/drivers/misc/emif.c
+++ b/drivers/misc/emif.c
@@ -18,6 +18,7 @@
#include<linux/platform_device.h>
#include<linux/interrupt.h>
#include<linux/slab.h>
+#include<linux/of.h>
#include<linux/debugfs.h>
#include<linux/seq_file.h>
#include<linux/module.h>
@@ -49,6 +50,7 @@
* frequency in effect at the moment)
* @plat_data: Pointer to saved platform data.
* @debugfs_root: dentry to the root folder for EMIF in debugfs
+ * @np_ddr: Pointer to ddr device tree node
*/
struct emif_data {
u8 duplicate;
@@ -63,6 +65,9 @@ struct emif_data {
struct emif_regs *curr_regs;
struct emif_platform_data *plat_data;
struct dentry *debugfs_root;
+#if defined(CONFIG_OF)
+ struct device_node *np_ddr;
+#endif
Don't bother with the #if/#endif wrapper here.
Ok.
};
static struct emif_data *emif1;
@@ -1147,6 +1152,270 @@ static int is_custom_config_valid(struct emif_custom_configs *cust_cfgs,
return valid;
}
+#if defined(CONFIG_OF)
+static void __init of_get_custom_configs(struct device_node *np_emif,
__devinit. Same through the rest of the file.
I am not sure if we need __devinit. I see that __devinit will not have
any effect if CONFIG_HOTPLUG is enabled and CONFIG_HOTPLUG is always
enabled in our configuration. EMIF devices are not hot-pluggable
devices and are always added at boot-time from the board file. They are
on-chip IP modules and dynamic discovery and addition doesn't make
sense for them. So, can't we save some memory by making them __init.
AFAIU, __init doesn't have any effect in a module. However, I can make
that more explicit by using '__init_or_module'. Is that ok?
[...]
+ return NULL;
+out:
+ return emif;
+}
+#endif
+
static struct emif_data * __init get_device_details(
This function also must be __devinit
struct platform_device *pdev)
{
@@ -1266,7 +1535,13 @@ static int __init emif_probe(struct platform_device *pdev)
struct resource *res;
int irq;
- emif = get_device_details(pdev);
+#if defined(CONFIG_OF)
+ if (pdev->dev.of_node)
+ emif = of_get_device_details(pdev->dev.of_node,&pdev->dev);
+ else
+#endif
+ emif = get_device_details(pdev);
+
if (!emif) {
pr_err("%s: error getting device data\n", __func__);
goto error;
@@ -1643,11 +1918,23 @@ static void __attribute__((unused)) freq_post_notify_handling(void)
spin_unlock_irqrestore(&emif_lock, irq_state);
}
+#if defined(CONFIG_OF)
+static const struct of_device_id emif_of_match[] = {
+ { .compatible = "ti,emif-4d" },
+ { .compatible = "ti,emif-4d5" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, emif_of_match);
+#endif
+
static struct platform_driver emif_driver = {
.remove = __exit_p(emif_remove),
Not part of this patch I realize, but this is a bug. .remove must
be in the __devexit section and the __devexit_p() wrapper must
be used. Similarly, emif_probe must be in the __devinit section.
Again, in our case remove() is not needed unless the driver is built as
a module because the devices will never be un-registered. When it is
built as a module the effect is same for both:
#if defined(MODULE) || defined(CONFIG_HOTPLUG)
#define __devexit_p(x) x
#else
#define __devexit_p(x) NULL
#endif
#ifdef MODULE
#define __exit_p(x) x
#else
#define __exit_p(x) NULL
#endif
.shutdown = emif_shutdown,
.driver = {
.name = "emif",
+#if defined(CONFIG_OF)
+ .of_match_table = of_match_ptr(emif_of_match),
+#endif
The of_match_ptr() macro makes the #if/#endif wrapper redundant.
Ok. I will remove it.
Otherwise, on brief review this patch looks right.
Thanks for the review. Are you ok with the bindings too?
br,
Aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html