diff mbox

of: address: Don't fail a lookup just because a node has no reg property

Message ID 1341498754-30445-1-git-send-email-lee.jones@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Lee Jones July 5, 2012, 2:32 p.m. UTC
Sometimes it doesn't make any sense for a node to have an address.
In this case device lookup will always be unsuccessful because we
currently assume every node will have a reg property. This patch
changes the semantics so that the resource address and the lookup
address will only be compared if one exists.

Things like AUXDATA() rely on of_dev_lookup to return the lookup
entry of a particular device in order to do things like apply
platform_data to a device. However, this is currently broken for
nodes which do not have a reg property, meaning that platform_data
can not be passed in those cases.

Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/of/platform.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

Comments

Linus Walleij July 5, 2012, 2:38 p.m. UTC | #1
On Thu, Jul 5, 2012 at 4:32 PM, Lee Jones <lee.jones@linaro.org> wrote:

> Sometimes it doesn't make any sense for a node to have an address.
> In this case device lookup will always be unsuccessful because we
> currently assume every node will have a reg property. This patch
> changes the semantics so that the resource address and the lookup
> address will only be compared if one exists.
>
> Things like AUXDATA() rely on of_dev_lookup to return the lookup
> entry of a particular device in order to do things like apply
> platform_data to a device. However, this is currently broken for
> nodes which do not have a reg property, meaning that platform_data
> can not be passed in those cases.
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>

I sure see the problem!
Acked-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Rob Herring July 5, 2012, 2:57 p.m. UTC | #2
On 07/05/2012 09:32 AM, Lee Jones wrote:
> Sometimes it doesn't make any sense for a node to have an address.
> In this case device lookup will always be unsuccessful because we
> currently assume every node will have a reg property. This patch
> changes the semantics so that the resource address and the lookup
> address will only be compared if one exists.
> 
> Things like AUXDATA() rely on of_dev_lookup to return the lookup
> entry of a particular device in order to do things like apply
> platform_data to a device. However, this is currently broken for
> nodes which do not have a reg property, meaning that platform_data
> can not be passed in those cases.
> 
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---

Acked-by: Rob Herring <rob.herring@calxeda.com>

Do you have something dependent on this or want me to apply?

Rob

>  drivers/of/platform.c |    7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/of/platform.c b/drivers/of/platform.c
> index 343ad29..9600480 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -317,10 +317,9 @@ static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l
>  	for(; lookup->compatible != NULL; lookup++) {
>  		if (!of_device_is_compatible(np, lookup->compatible))
>  			continue;
> -		if (of_address_to_resource(np, 0, &res))
> -			continue;
> -		if (res.start != lookup->phys_addr)
> -			continue;
> +		if (!of_address_to_resource(np, 0, &res))
> +			if (res.start != lookup->phys_addr)
> +				continue;
>  		pr_debug("%s: devname=%s\n", np->full_name, lookup->name);
>  		return lookup;
>  	}
>
Lee Jones July 5, 2012, 3:17 p.m. UTC | #3
On 05/07/12 15:57, Rob Herring wrote:
> On 07/05/2012 09:32 AM, Lee Jones wrote:
>> Sometimes it doesn't make any sense for a node to have an address.
>> In this case device lookup will always be unsuccessful because we
>> currently assume every node will have a reg property. This patch
>> changes the semantics so that the resource address and the lookup
>> address will only be compared if one exists.
>>
>> Things like AUXDATA() rely on of_dev_lookup to return the lookup
>> entry of a particular device in order to do things like apply
>> platform_data to a device. However, this is currently broken for
>> nodes which do not have a reg property, meaning that platform_data
>> can not be passed in those cases.
>>
>> Acked-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Lee Jones <lee.jones@linaro.org>
>> ---
>
> Acked-by: Rob Herring <rob.herring@calxeda.com>
>
> Do you have something dependent on this or want me to apply?

I have things that require this patch yes. Please apply.

Thanks Rob.
Arnd Bergmann July 5, 2012, 3:38 p.m. UTC | #4
On Thursday 05 July 2012, Lee Jones wrote:
> >
> > Acked-by: Rob Herring <rob.herring@calxeda.com>
> >
> > Do you have something dependent on this or want me to apply?
> 
> I have things that require this patch yes. Please apply.

I guess Rob's question was whether you want to keep this together
with the patch that depends on it rather than him applying just
the one patch.

	Arnd
Lee Jones July 5, 2012, 3:50 p.m. UTC | #5
On 05/07/12 16:38, Arnd Bergmann wrote:
> On Thursday 05 July 2012, Lee Jones wrote:
>>>
>>> Acked-by: Rob Herring <rob.herring@calxeda.com>
>>>
>>> Do you have something dependent on this or want me to apply?
>>
>> I have things that require this patch yes. Please apply.
>
> I guess Rob's question was whether you want to keep this together
> with the patch that depends on it rather than him applying just
> the one patch.

No, it's not important.

I'll send you the other patch as part of my DT pull-request.

I'm just waiting on Linus' Ack on a few patches and it'll be ready.
diff mbox

Patch

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 343ad29..9600480 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -317,10 +317,9 @@  static const struct of_dev_auxdata *of_dev_lookup(const struct of_dev_auxdata *l
 	for(; lookup->compatible != NULL; lookup++) {
 		if (!of_device_is_compatible(np, lookup->compatible))
 			continue;
-		if (of_address_to_resource(np, 0, &res))
-			continue;
-		if (res.start != lookup->phys_addr)
-			continue;
+		if (!of_address_to_resource(np, 0, &res))
+			if (res.start != lookup->phys_addr)
+				continue;
 		pr_debug("%s: devname=%s\n", np->full_name, lookup->name);
 		return lookup;
 	}