On 15/11/12 13:11, Grant Likely wrote:
> On Fri, 2 Nov 2012 10:46:19 +0000, Srinivas KANDAGATLA
> <srinivas.kandaga...@st.com> wrote:
>> From: Srinivas Kandagatla <srinivas.kandaga...@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.kandaga...@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().
I agree, I don't like the duplication too.
> Does something like this work for you instead?
Yes, it works for me and I have tested your patch.
Tested-by: Srinivas Kandagatla<srinivas.kandaga...@st.com>
> 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
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
