[RFC PATCH] SDHCI: S3C: Add support for retrieving memory and irq resource information from device tree.

[Grant Likely]

On Mon, Jan 31, 2011 at 9:28 AM,  <thomas.abraham at linaro.org> wrote:
> From: Thomas Abraham <thomas.abraham at 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 at 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 at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-dev
>



-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.