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

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

[Grant Likely]

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-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
CC: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
CC: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
CC: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
CC: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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;
 	}

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

[Benjamin Herrenschmidt]

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-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> CC: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> CC: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> CC: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> CC: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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;
>  	}
>  

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

[Grant Likely]

On Thu, Jun 10, 2010 at 12:44 AM, Benjamin Herrenschmidt
<benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.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-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
>> CC: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
>> CC: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
>> CC: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
>> CC: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

[Benjamin Herrenschmidt]

On Thu, 2010-06-10 at 08:28 -0600, Grant Likely wrote:
> On Thu, Jun 10, 2010 at 12:44 AM, Benjamin Herrenschmidt
> <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.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-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>
> >> CC: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
> >> CC: Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
> >> CC: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> >> CC: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.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;
> >>       }
> >>
> >
> >
> >
> 
> 
> 

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

[Segher Boessenkool]

>> 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

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

[Benjamin Herrenschmidt]

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.