[RFC PATCH] SDHCI: S3C: Add support for retrieving memory and irq resource information from device tree.
From: Thomas Abraham thomas.abraham@linaro.com
Add support for retrieving memory and irq resource information from device tree for Samsung's SDHCI controller driver.
Signed-off-by: Thomas Abraham thomas.abraham@linaro.org ---
The modification will be made more generic to support both DT and non-DT versions of the driver without the #ifdef's. For now, this patch is for review and to understand if the approach adopted to obtain resource information from the device tree is appropriate.
drivers/mmc/host/sdhci-s3c.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c index 1720358..f536061 100644 --- a/drivers/mmc/host/sdhci-s3c.c +++ b/drivers/mmc/host/sdhci-s3c.c @@ -19,6 +19,9 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/gpio.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_address.h>
#include <linux/mmc/host.h>
@@ -348,23 +351,52 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) struct sdhci_s3c *sc; struct resource *res; int ret, irq, ptr, clks; + struct device_node *np = NULL; +#ifdef CONFIG_OF + struct resource iores; +#endif
if (!pdata) { dev_err(dev, "no device data specified\n"); return -ENOENT; }
+#ifdef CONFIG_OF + for_each_compatible_node(np, NULL, "samsung,sdhci-s3c") { + const u32 *id = of_get_property(np, "cell-index", NULL); + if (be32_to_cpu(*id) == pdev->id) + break; + } + + if (!np) { + dev_err(dev, "no matching device node specified in device tree\n"); + return -ENOENT; + } +#endif + +#ifndef CONFIG_OF irq = platform_get_irq(pdev, 0); +#else + irq = of_irq_to_resource(np, 0, NULL); +#endif if (irq < 0) { dev_err(dev, "no irq specified\n"); return irq; }
+#ifndef CONFIG_OF res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { dev_err(dev, "no memory specified\n"); return -ENOENT; } +#else + if (of_address_to_resource(np, 0, &iores)) { + dev_err(dev, "no memory specified in device tree\n"); + return -ENOENT; + } + res = &iores; +#endif
host = sdhci_alloc_host(dev, sizeof(struct sdhci_s3c)); if (IS_ERR(host)) {
On Mon, Jan 31, 2011 at 9:28 AM, thomas.abraham@linaro.org wrote:
From: Thomas Abraham thomas.abraham@linaro.com
Add support for retrieving memory and irq resource information from device tree for Samsung's SDHCI controller driver.
Signed-off-by: Thomas Abraham thomas.abraham@linaro.org
The modification will be made more generic to support both DT and non-DT versions of the driver without the #ifdef's. For now, this patch is for review and to understand if the approach adopted to obtain resource information from the device tree is appropriate.
drivers/mmc/host/sdhci-s3c.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c index 1720358..f536061 100644 --- a/drivers/mmc/host/sdhci-s3c.c +++ b/drivers/mmc/host/sdhci-s3c.c @@ -19,6 +19,9 @@ #include <linux/clk.h> #include <linux/io.h> #include <linux/gpio.h> +#include <linux/of.h> +#include <linux/of_irq.h> +#include <linux/of_address.h>
#include <linux/mmc/host.h>
@@ -348,23 +351,52 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) struct sdhci_s3c *sc; struct resource *res; int ret, irq, ptr, clks;
- struct device_node *np = NULL;
+#ifdef CONFIG_OF
- struct resource iores;
+#endif
if (!pdata) { dev_err(dev, "no device data specified\n"); return -ENOENT; }
+#ifdef CONFIG_OF
- for_each_compatible_node(np, NULL, "samsung,sdhci-s3c") {
- const u32 *id = of_get_property(np, "cell-index", NULL);
- if (be32_to_cpu(*id) == pdev->id)
- break;
- }
- if (!np) {
- dev_err(dev, "no matching device node specified in device tree\n");
- return -ENOENT;
- }
I think I've got a better solution to this which I'll be posting today. I've got code that allows the dt support code to 'snoop' platform bus registrations and set the of_node pointer for matching nodes. Then all the normal platform_bus support will work just fine. Doing it this way prevents building a kernel that supports both dt and non-dt booting.
Also, relying on cell-index is generally considered a bad idea. When using the dt, let the kernel do the device enumeration instead of specifying it explicitly with cell-index.
+#endif
+#ifndef CONFIG_OF irq = platform_get_irq(pdev, 0); +#else
- irq = of_irq_to_resource(np, 0, NULL);
+#endif
When using the dt, platform_get_irq() should still work correctly. This shouldn't be necessary.
if (irq < 0) { dev_err(dev, "no irq specified\n"); return irq; }
+#ifndef CONFIG_OF res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { dev_err(dev, "no memory specified\n"); return -ENOENT; } +#else
- if (of_address_to_resource(np, 0, &iores)) {
- dev_err(dev, "no memory specified in device tree\n");
- return -ENOENT;
- }
- res = &iores;
+#endif
Ditto
host = sdhci_alloc_host(dev, sizeof(struct sdhci_s3c)); if (IS_ERR(host)) { -- 1.6.6.rc2
linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
Thomas,
On 01/31/2011 10:28 AM, thomas.abraham@linaro.org wrote:
From: Thomas Abrahamthomas.abraham@linaro.com
Add support for retrieving memory and irq resource information from device tree for Samsung's SDHCI controller driver.
Signed-off-by: Thomas Abrahamthomas.abraham@linaro.org
The modification will be made more generic to support both DT and non-DT versions of the driver without the #ifdef's. For now, this patch is for review and to understand if the approach adopted to obtain resource information from the device tree is appropriate.
There is already an OF SDHCI driver. Some fixes from me for ARM have gone into .38.
drivers/mmc/host/sdhci-s3c.c | 32 ++++++++++++++++++++++++++++++++ 1 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c index 1720358..f536061 100644 --- a/drivers/mmc/host/sdhci-s3c.c +++ b/drivers/mmc/host/sdhci-s3c.c @@ -19,6 +19,9 @@ #include<linux/clk.h> #include<linux/io.h> #include<linux/gpio.h> +#include<linux/of.h> +#include<linux/of_irq.h> +#include<linux/of_address.h>
#include<linux/mmc/host.h>
@@ -348,23 +351,52 @@ static int __devinit sdhci_s3c_probe(struct platform_device *pdev) struct sdhci_s3c *sc; struct resource *res; int ret, irq, ptr, clks;
- struct device_node *np = NULL;
+#ifdef CONFIG_OF
- struct resource iores;
+#endif
if (!pdata) { dev_err(dev, "no device data specified\n"); return -ENOENT; }
+#ifdef CONFIG_OF
- for_each_compatible_node(np, NULL, "samsung,sdhci-s3c") {
const u32 *id = of_get_property(np, "cell-index", NULL);
Per Grant, using cell-index should be avoided.
if (be32_to_cpu(*id) == pdev->id)
Any drivers that depend on pdev->id being an index will break for device tree.
break;- }
- if (!np) {
dev_err(dev, "no matching device node specified in device tree\n");return -ENOENT;- }
+#endif
+#ifndef CONFIG_OF irq = platform_get_irq(pdev, 0);
Using platform_get_irq works for OF drivers too.
+#else
- irq = of_irq_to_resource(np, 0, NULL);
+#endif if (irq< 0) { dev_err(dev, "no irq specified\n"); return irq; }
+#ifndef CONFIG_OF res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
Ditto
if (!res) { dev_err(dev, "no memory specified\n"); return -ENOENT; } +#else
- if (of_address_to_resource(np, 0,&iores)) {
dev_err(dev, "no memory specified in device tree\n");return -ENOENT;- }
- res =&iores;
+#endif
host = sdhci_alloc_host(dev, sizeof(struct sdhci_s3c)); if (IS_ERR(host)) {
You are missing an of_match_table. This patch could not work without it.
The primary things you need to do for OF support on a driver are add the match table and convert platform data to OF bindings.
Rob
On Mon, Jan 31, 2011 at 10:00 AM, Rob Herring robherring2@gmail.com wrote:
Thomas,
On 01/31/2011 10:28 AM, thomas.abraham@linaro.org wrote:
From: Thomas Abrahamthomas.abraham@linaro.com
Add support for retrieving memory and irq resource information from device tree for Samsung's SDHCI controller driver.
Signed-off-by: Thomas Abrahamthomas.abraham@linaro.org
The modification will be made more generic to support both DT and non-DT versions of the driver without the #ifdef's. For now, this patch is for review and to understand if the approach adopted to obtain resource information from the device tree is appropriate.
You are missing an of_match_table. This patch could not work without it.
Actually, the way he is implementing it the of_match_table would have absolutely no effect. However, you are right, it *should* be implemented so that it binds via the of_match_table. I suspect Thomas did it this way because he doesn't yet have an easy way to get his platform devices registered from the device tree.
g.
-
Grant Likely -
Rob Herring -
thomas.abraham＠linaro.org