[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20121115131134.03E5F3E0B12@localhost>
Date: Thu, 15 Nov 2012 13:11:33 +0000
From: Grant Likely <grant.likely@...retlab.ca>
To: Srinivas KANDAGATLA <srinivas.kandagatla@...com>,
rob.herring@...xeda.com
Cc: srinivas.kandagatla@...com, devicetree-discuss@...ts.ozlabs.org,
linux-kernel@...r.kernel.org, arnd@...db.de
Subject: Re: [RFC PATCH 3.7.0-rc2] dt/platform: insert resources correctly into resource tree
On Fri, 2 Nov 2012 10:46:19 +0000, Srinivas KANDAGATLA <srinivas.kandagatla@...com> wrote:
> From: Srinivas Kandagatla <srinivas.kandagatla@...com>
>
> This patch add new code to correctly add resources into platform device.
> Issue with the existing code was the resources are added as flat entry
> without creating any tree, this is very much different to what non-dt
> platform code does.
>
> With this patch the resources appear correctly as tree in /proc/iomem,
> without this patch the resources in /proc/iomem appear as single entry.
>
> i2c example in /proc/iomem:
>
> With-patch:
>
> fed41000-fed4110f : /soc/i2c-stm@...41000
> fed41000-fed4110f : i2c
>
> Without patch:
> fed41000-fed4110f : i2c
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...com>
Yes, that is a problem that should be fixed.
Although it seems to me that some powerpc platforms will break due to
nodes with overlapping ranges. That will need to be tested.
I also don't like the duplication of code from platform_device_add().
Does something like this work for you instead? I might need to add an
exception to fallback onto of_device_add if the resource regions
overlay. Or modify platform_device_add() to complain about overlaps, but
not fail when on PowerPC.
g.
---
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 72c776f..2edf831 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -283,7 +283,7 @@ int platform_device_add(struct platform_device *pdev)
if (!pdev)
return -EINVAL;
- if (!pdev->dev.parent)
+ if (!pdev->dev.parent && !pdev->dev.of_node)
pdev->dev.parent = &platform_bus;
pdev->dev.bus = &platform_bus_type;
diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index b80891b..fb9cbad 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -214,16 +214,22 @@ struct platform_device *of_platform_device_create_pdata(
#if defined(CONFIG_MICROBLAZE)
dev->archdata.dma_mask = 0xffffffffUL;
#endif
+ dev->name = dev_name(&dev->dev);
dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
- dev->dev.bus = &platform_bus_type;
dev->dev.platform_data = platform_data;
+ dev->dev.id = PLATFORM_DEVID_NONE;
+ /* device_add will assume that this device is on the same node as
+ * the parent. If there is no parent defined, set the node
+ * explicitly */
+ if (!parent)
+ set_dev_node(&dev->dev, of_node_to_nid(np));
/* We do not fill the DMA ops for platform devices by default.
* This is currently the responsibility of the platform code
* to do such, possibly using a device notifier
*/
- if (of_device_add(dev) != 0) {
+ if (platform_device_add(dev)) {
platform_device_put(dev);
return NULL;
}
> ---
> Hi All,
> Recently I noticed that appearance of /proc/iomem ouput changed
> when I started using device trees and the reason for this was
> the of-plaform code was not adding resources in same way as
> non-dt platform code does.
>
> Do you have any reason for not doing it the same way as non-dt platform code?
>
> This patch is a fixup to that issue.
>
> Comment?
> thanks,
> srini
>
> drivers/of/platform.c | 23 +++++++++++++++++++++++
> 1 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index b80891b..f43922c 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -203,6 +203,7 @@ struct platform_device *of_platform_device_create_pdata(
> struct device *parent)
> {
> struct platform_device *dev;
> + int i;
>
> if (!of_device_is_available(np))
> return NULL;
> @@ -218,6 +219,28 @@ struct platform_device *of_platform_device_create_pdata(
> dev->dev.bus = &platform_bus_type;
> dev->dev.platform_data = platform_data;
>
> + for (i = 0; i < dev->num_resources; i++) {
> + struct resource *p, *r = &dev->resource[i];
> +
> + if (r->name == NULL)
> + r->name = dev_name(&dev->dev);
> +
> + p = r->parent;
> + if (!p) {
> + if (resource_type(r) == IORESOURCE_MEM)
> + p = &iomem_resource;
> + else if (resource_type(r) == IORESOURCE_IO)
> + p = &ioport_resource;
> + }
> +
> + if (p && insert_resource(p, r)) {
> + pr_err("%s: failed to claim resource %d\n",
> + dev_name(&dev->dev), i);
> + platform_device_put(dev);
> + return NULL;
> + }
> + }
> +
> /* We do not fill the DMA ops for platform devices by default.
> * This is currently the responsibility of the platform code
> * to do such, possibly using a device notifier
> --
> 1.7.0.4
>
--
Grant Likely, B.Sc, P.Eng.
Secret Lab Technologies, Ltd.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists