Messages in this thread Patch in this message | | | From | Grant Likely <> | Subject | Re: [RFC PATCH 3.7.0-rc2] dt/platform: insert resources correctly into resource tree | Date | Thu, 15 Nov 2012 13:11:33 +0000 |
| |
On Fri, 2 Nov 2012 10:46:19 +0000, Srinivas KANDAGATLA <srinivas.kandagatla@st.com> wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@st.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@fed41000 > fed41000-fed4110f : i2c > > Without patch: > fed41000-fed4110f : i2c > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@st.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.
| |