Patchwork [5/5] of/address: restrict 'no-ranges' kludge to powerpc

login
register
mail settings
Submitter Grant Likely
Date June 8, 2010, 2:10 p.m.
Message ID <20100608141018.25879.44413.stgit@angua>
Download mbox | patch
Permalink /patch/54982/
State Not Applicable
Headers show

Comments

Grant Likely - June 8, 2010, 2:10 p.m.
Certain Apple machines don't use the ranges property correctly, but the
workaround should not be applied on other architectures.  This patch
disables the workaround for non-powerpc architectures.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
CC: Stephen Rothwell <sfr@canb.auug.org.au>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: linuxppc-dev@lists.ozlabs.org
CC: devicetree-discuss@lists.ozlabs.org
---
 drivers/of/address.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)
Benjamin Herrenschmidt - June 10, 2010, 6:44 a.m.
On Tue, 2010-06-08 at 08:10 -0600, Grant Likely wrote:
> Certain Apple machines don't use the ranges property correctly, but the
> workaround should not be applied on other architectures.  This patch
> disables the workaround for non-powerpc architectures.

I'm half tempted to add it to the quirk list (which should really be
made generic) so I can disable it on more 'modern' powerpc as well.

Cheers,
Ben.

> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> CC: Stephen Rothwell <sfr@canb.auug.org.au>
> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: linuxppc-dev@lists.ozlabs.org
> CC: devicetree-discuss@lists.ozlabs.org
> ---
>  drivers/of/address.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 0b04137..5c220c3 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -346,12 +346,21 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>  	 * a 1:1 translation at that level. It's up to the caller not to try
>  	 * to translate addresses that aren't supposed to be translated in
>  	 * the first place. --BenH.
> +	 *
> +	 * As far as we know, this damage only exists on Apple machines, so
> +	 * This code is only enabled on powerpc. --gcl
>  	 */
>  	ranges = of_get_property(parent, rprop, &rlen);
> +#if !defined(CONFIG_PPC)
> +	if (ranges == NULL) {
> +		pr_err("OF: no ranges; cannot translate\n");
> +		return 1;
> +	}
> +#endif /* !defined(CONFIG_PPC) */
>  	if (ranges == NULL || rlen == 0) {
>  		offset = of_read_number(addr, na);
>  		memset(addr, 0, pna * 4);
> -		pr_debug("OF: no ranges, 1:1 translation\n");
> +		pr_debug("OF: empty ranges; 1:1 translation\n");
>  		goto finish;
>  	}
>
Grant Likely - June 10, 2010, 2:28 p.m.
On Thu, Jun 10, 2010 at 12:44 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2010-06-08 at 08:10 -0600, Grant Likely wrote:
>> Certain Apple machines don't use the ranges property correctly, but the
>> workaround should not be applied on other architectures.  This patch
>> disables the workaround for non-powerpc architectures.
>
> I'm half tempted to add it to the quirk list (which should really be
> made generic) so I can disable it on more 'modern' powerpc as well.

In the mean time, are you okay with this version of the patch?

g.

>
> Cheers,
> Ben.
>
>> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>> CC: Stephen Rothwell <sfr@canb.auug.org.au>
>> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> CC: linuxppc-dev@lists.ozlabs.org
>> CC: devicetree-discuss@lists.ozlabs.org
>> ---
>>  drivers/of/address.c |   11 ++++++++++-
>>  1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/of/address.c b/drivers/of/address.c
>> index 0b04137..5c220c3 100644
>> --- a/drivers/of/address.c
>> +++ b/drivers/of/address.c
>> @@ -346,12 +346,21 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>>        * a 1:1 translation at that level. It's up to the caller not to try
>>        * to translate addresses that aren't supposed to be translated in
>>        * the first place. --BenH.
>> +      *
>> +      * As far as we know, this damage only exists on Apple machines, so
>> +      * This code is only enabled on powerpc. --gcl
>>        */
>>       ranges = of_get_property(parent, rprop, &rlen);
>> +#if !defined(CONFIG_PPC)
>> +     if (ranges == NULL) {
>> +             pr_err("OF: no ranges; cannot translate\n");
>> +             return 1;
>> +     }
>> +#endif /* !defined(CONFIG_PPC) */
>>       if (ranges == NULL || rlen == 0) {
>>               offset = of_read_number(addr, na);
>>               memset(addr, 0, pna * 4);
>> -             pr_debug("OF: no ranges, 1:1 translation\n");
>> +             pr_debug("OF: empty ranges; 1:1 translation\n");
>>               goto finish;
>>       }
>>
>
>
>
Benjamin Herrenschmidt - June 11, 2010, 1:12 a.m.
On Thu, 2010-06-10 at 08:28 -0600, Grant Likely wrote:
> On Thu, Jun 10, 2010 at 12:44 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > On Tue, 2010-06-08 at 08:10 -0600, Grant Likely wrote:
> >> Certain Apple machines don't use the ranges property correctly, but the
> >> workaround should not be applied on other architectures.  This patch
> >> disables the workaround for non-powerpc architectures.
> >
> > I'm half tempted to add it to the quirk list (which should really be
> > made generic) so I can disable it on more 'modern' powerpc as well.
> 
> In the mean time, are you okay with this version of the patch?

For the time being yes.

Cheers,
Ben.

> g.
> 
> >
> > Cheers,
> > Ben.
> >
> >> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> >> CC: Stephen Rothwell <sfr@canb.auug.org.au>
> >> CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> >> CC: linuxppc-dev@lists.ozlabs.org
> >> CC: devicetree-discuss@lists.ozlabs.org
> >> ---
> >>  drivers/of/address.c |   11 ++++++++++-
> >>  1 files changed, 10 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/of/address.c b/drivers/of/address.c
> >> index 0b04137..5c220c3 100644
> >> --- a/drivers/of/address.c
> >> +++ b/drivers/of/address.c
> >> @@ -346,12 +346,21 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> >>        * a 1:1 translation at that level. It's up to the caller not to try
> >>        * to translate addresses that aren't supposed to be translated in
> >>        * the first place. --BenH.
> >> +      *
> >> +      * As far as we know, this damage only exists on Apple machines, so
> >> +      * This code is only enabled on powerpc. --gcl
> >>        */
> >>       ranges = of_get_property(parent, rprop, &rlen);
> >> +#if !defined(CONFIG_PPC)
> >> +     if (ranges == NULL) {
> >> +             pr_err("OF: no ranges; cannot translate\n");
> >> +             return 1;
> >> +     }
> >> +#endif /* !defined(CONFIG_PPC) */
> >>       if (ranges == NULL || rlen == 0) {
> >>               offset = of_read_number(addr, na);
> >>               memset(addr, 0, pna * 4);
> >> -             pr_debug("OF: no ranges, 1:1 translation\n");
> >> +             pr_debug("OF: empty ranges; 1:1 translation\n");
> >>               goto finish;
> >>       }
> >>
> >
> >
> >
> 
> 
>
Segher Boessenkool - June 15, 2010, 4:23 p.m.
>> Certain Apple machines don't use the ranges property correctly,  
>> but the
>> workaround should not be applied on other architectures.  This patch
>> disables the workaround for non-powerpc architectures.
>
> I'm half tempted to add it to the quirk list (which should really be
> made generic) so I can disable it on more 'modern' powerpc as well.

Oh please oh please oh please yes do.

OTOH, it would be even better to just fix up the device tree in the
early platform code.  Quirks are for broken hardware; software, we
can fix.


Segher
Benjamin Herrenschmidt - June 16, 2010, 12:33 a.m.
On Tue, 2010-06-15 at 18:23 +0200, Segher Boessenkool wrote:
> >> Certain Apple machines don't use the ranges property correctly,  
> >> but the
> >> workaround should not be applied on other architectures.  This patch
> >> disables the workaround for non-powerpc architectures.
> >
> > I'm half tempted to add it to the quirk list (which should really be
> > made generic) so I can disable it on more 'modern' powerpc as well.
> 
> Oh please oh please oh please yes do.
> 
> OTOH, it would be even better to just fix up the device tree in the
> early platform code.  Quirks are for broken hardware; software, we
> can fix.

That would work if I could bloody remember which machines need what on
what nodes ... some of those are ancient and I don't have access to all
of them.

Cheers,
Ben.

Patch

diff --git a/drivers/of/address.c b/drivers/of/address.c
index 0b04137..5c220c3 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -346,12 +346,21 @@  static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 	 * a 1:1 translation at that level. It's up to the caller not to try
 	 * to translate addresses that aren't supposed to be translated in
 	 * the first place. --BenH.
+	 *
+	 * As far as we know, this damage only exists on Apple machines, so
+	 * This code is only enabled on powerpc. --gcl
 	 */
 	ranges = of_get_property(parent, rprop, &rlen);
+#if !defined(CONFIG_PPC)
+	if (ranges == NULL) {
+		pr_err("OF: no ranges; cannot translate\n");
+		return 1;
+	}
+#endif /* !defined(CONFIG_PPC) */
 	if (ranges == NULL || rlen == 0) {
 		offset = of_read_number(addr, na);
 		memset(addr, 0, pna * 4);
-		pr_debug("OF: no ranges, 1:1 translation\n");
+		pr_debug("OF: empty ranges; 1:1 translation\n");
 		goto finish;
 	}