[PATCH] driver core: simplify platform_get_resource()

[PATCH] driver core: simplify platform_get_resource()

[Mike Frysinger]

The platform_get_resource() function takes a number for the desired index
into the resource array, but then does a for loop on the array to get to
that index.  Considering the array is linear, the loop overhead is just
that -- overhead.  So unless I missed something, convert it into an index
check and access the desired resource directly.  Resulting code makes a
lot more sense considering its purpose.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 drivers/base/platform.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 81cb01b..dc8c943 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -37,14 +37,12 @@ EXPORT_SYMBOL_GPL(platform_bus);
 struct resource *platform_get_resource(struct platform_device *dev,
 				       unsigned int type, unsigned int num)
 {
-	int i;
-
-	for (i = 0; i < dev->num_resources; i++) {
-		struct resource *r = &dev->resource[i];
-
-		if (type == resource_type(r) && num-- == 0)
+	if (num >= 0 && num < dev->num_resources) {
+		struct resource *r = &dev->resource[num];
+		if (type == resource_type(r))
 			return r;
 	}
+
 	return NULL;
 }
 EXPORT_SYMBOL_GPL(platform_get_resource);
-- 
1.6.3.3

Re: [PATCH] driver core: simplify platform_get_resource()

[Mark Brown]

On Mon, Jul 27, 2009 at 02:20:56PM -0400, Mike Frysinger wrote:
> The platform_get_resource() function takes a number for the desired index
> into the resource array, but then does a for loop on the array to get to
> that index.  Considering the array is linear, the loop overhead is just
> that -- overhead.  So unless I missed something, convert it into an index
> check and access the desired resource directly.  Resulting code makes a
> lot more sense considering its purpose.

This showed up in linux-next for me today and is causing breakage on at
least S3C64xx platforms since it changes the resource numbering when
there's more than one resource type for a device:

> -	for (i = 0; i < dev->num_resources; i++) {
> -		struct resource *r = &dev->resource[i];
> -
> -		if (type == resource_type(r) && num-- == 0)
> +	if (num >= 0 && num < dev->num_resources) {
> +		struct resource *r = &dev->resource[num];
> +		if (type == resource_type(r))

Previously the resources were indexed within their type (so you'd get
I/O resources 0, 1, ..., IRQ resources 0, 1, ... and so on) but now the
index treats all the resources for a device as a single array, causing
them to be renumbered for callers.

This causes drivers doing lookups by number to fail to find their
resources and not probe, causing widespread breakage.  Reverting the
patch fixes the problem.

Re: [PATCH] driver core: simplify platform_get_resource()

[Greg KH]

On Tue, Jul 28, 2009 at 11:59:49AM +0100, Mark Brown wrote:
> On Mon, Jul 27, 2009 at 02:20:56PM -0400, Mike Frysinger wrote:
> > The platform_get_resource() function takes a number for the desired index
> > into the resource array, but then does a for loop on the array to get to
> > that index.  Considering the array is linear, the loop overhead is just
> > that -- overhead.  So unless I missed something, convert it into an index
> > check and access the desired resource directly.  Resulting code makes a
> > lot more sense considering its purpose.
> 
> This showed up in linux-next for me today and is causing breakage on at
> least S3C64xx platforms since it changes the resource numbering when
> there's more than one resource type for a device:
> 
> > -	for (i = 0; i < dev->num_resources; i++) {
> > -		struct resource *r = &dev->resource[i];
> > -
> > -		if (type == resource_type(r) && num-- == 0)
> > +	if (num >= 0 && num < dev->num_resources) {
> > +		struct resource *r = &dev->resource[num];
> > +		if (type == resource_type(r))
> 
> Previously the resources were indexed within their type (so you'd get
> I/O resources 0, 1, ..., IRQ resources 0, 1, ... and so on) but now the
> index treats all the resources for a device as a single array, causing
> them to be renumbered for callers.
> 
> This causes drivers doing lookups by number to fail to find their
> resources and not probe, causing widespread breakage.  Reverting the
> patch fixes the problem.

I've now dropped this patch.

thanks,

greg k-h