Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1768699Ab2KOSl4 (ORCPT ); Thu, 15 Nov 2012 13:41:56 -0500 Received: from eu1sys200aog103.obsmtp.com ([207.126.144.115]:41786 "EHLO eu1sys200aog103.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1768466Ab2KOSlz (ORCPT ); Thu, 15 Nov 2012 13:41:55 -0500 Message-ID: <50A53722.4070000@st.com> Date: Thu, 15 Nov 2012 18:40:34 +0000 From: Srinivas KANDAGATLA Reply-To: srinivas.kandagatla@st.com Organization: STMicroelectronics User-Agent: Mozilla/5.0 (X11; Linux i686; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: Grant Likely Cc: rob.herring@calxeda.com, devicetree-discuss@lists.ozlabs.org, linux-kernel@vger.kernel.org, arnd@arndb.de Subject: Re: [RFC PATCH 3.7.0-rc2] dt/platform: insert resources correctly into resource tree References: <1351853179-27741-1-git-send-email-srinivas.kandagatla@st.com> <20121115131134.03E5F3E0B12@localhost> In-Reply-To: <20121115131134.03E5F3E0B12@localhost> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5146 Lines: 155 On 15/11/12 13:11, Grant Likely wrote: > On Fri, 2 Nov 2012 10:46:19 +0000, Srinivas KANDAGATLA wrote: >> From: Srinivas Kandagatla >> >> 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 > 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 > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/