linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH]  of/base: Fix PowerPC address parsing hack
@ 2014-11-12  5:51 Benjamin Herrenschmidt
  2014-11-12 14:39 ` Rob Herring
  2014-11-12 22:10 ` Stephen Rothwell
  0 siblings, 2 replies; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-11-12  5:51 UTC (permalink / raw)
  To: Rob Herring, Grant Likely
  Cc: devicetree, linux-kernel, Arnd Bergmann, Olof Johansson, linuxppc-dev

We have a historical hack that treats missing ranges properties as the
equivalent of an empty one. This is needed for ancient PowerMac "bad"
device-trees, and shouldn't be enabled for any other PowerPC platform,
otherwise we get some nasty layout of devices in sysfs or even
duplication when a set of otherwise identically named devices is
created multiple times under a different parent node with no ranges
property.

This fix is needed for the PowerNV i2c busses to be exposed properly
and will fix a number of other embedded cases.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
--- 

diff --git a/drivers/of/address.c b/drivers/of/address.c
index e371825..e37f017 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -7,6 +7,10 @@
 #include <linux/pci_regs.h>
 #include <linux/string.h>
 
+#ifdef CONFIG_PPC
+#include <asm/machdep.h>
+#endif
+
 /* Max address size we deal with */
 #define OF_MAX_ADDR_CELLS	4
 #define OF_CHECK_ADDR_COUNT(na)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
@@ -428,12 +432,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 	 * This code is only enabled on powerpc. --gcl
 	 */
 	ranges = of_get_property(parent, rprop, &rlen);
-#if !defined(CONFIG_PPC)
+#if defined(CONFIG_PPC)
+	if (!machine_is(powermac))
+#endif /* defined(CONFIG_PPC) */
 	if (ranges == NULL) {
-		pr_err("OF: no ranges; cannot translate\n");
+		pr_debug("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);


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] of/base: Fix PowerPC address parsing hack
  2014-11-12  5:51 [PATCH] of/base: Fix PowerPC address parsing hack Benjamin Herrenschmidt
@ 2014-11-12 14:39 ` Rob Herring
  2014-11-12 17:02   ` Grant Likely
  2014-11-12 19:55   ` Benjamin Herrenschmidt
  2014-11-12 22:10 ` Stephen Rothwell
  1 sibling, 2 replies; 12+ messages in thread
From: Rob Herring @ 2014-11-12 14:39 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Grant Likely, devicetree, linux-kernel, Arnd Bergmann,
	Olof Johansson, linuxppc-dev

On Tue, Nov 11, 2014 at 11:51 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> We have a historical hack that treats missing ranges properties as the
> equivalent of an empty one. This is needed for ancient PowerMac "bad"
> device-trees, and shouldn't be enabled for any other PowerPC platform,
> otherwise we get some nasty layout of devices in sysfs or even
> duplication when a set of otherwise identically named devices is
> created multiple times under a different parent node with no ranges
> property.
>
> This fix is needed for the PowerNV i2c busses to be exposed properly
> and will fix a number of other embedded cases.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>
> ---
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index e371825..e37f017 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -7,6 +7,10 @@
>  #include <linux/pci_regs.h>
>  #include <linux/string.h>
>
> +#ifdef CONFIG_PPC
> +#include <asm/machdep.h>
> +#endif
> +
>  /* Max address size we deal with */
>  #define OF_MAX_ADDR_CELLS      4
>  #define OF_CHECK_ADDR_COUNT(na)        ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
> @@ -428,12 +432,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>          * This code is only enabled on powerpc. --gcl
>          */
>         ranges = of_get_property(parent, rprop, &rlen);
> -#if !defined(CONFIG_PPC)
> +#if defined(CONFIG_PPC)
> +       if (!machine_is(powermac))

Can we use a machine compatible here or something not PPC specific?
Then we can use IS_ENABLED(CONFIG_PPC) instead of ifdefs.

Rob

> +#endif /* defined(CONFIG_PPC) */
>         if (ranges == NULL) {
> -               pr_err("OF: no ranges; cannot translate\n");
> +               pr_debug("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);
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] of/base: Fix PowerPC address parsing hack
  2014-11-12 14:39 ` Rob Herring
@ 2014-11-12 17:02   ` Grant Likely
  2014-11-12 19:55   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 12+ messages in thread
From: Grant Likely @ 2014-11-12 17:02 UTC (permalink / raw)
  To: Rob Herring, Benjamin Herrenschmidt
  Cc: devicetree, linux-kernel, Arnd Bergmann, Olof Johansson, linuxppc-dev

On Wed, 12 Nov 2014 08:39:32 -0600
, Rob Herring <robherring2@gmail.com>
 wrote:
> On Tue, Nov 11, 2014 at 11:51 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > We have a historical hack that treats missing ranges properties as the
> > equivalent of an empty one. This is needed for ancient PowerMac "bad"
> > device-trees, and shouldn't be enabled for any other PowerPC platform,
> > otherwise we get some nasty layout of devices in sysfs or even
> > duplication when a set of otherwise identically named devices is
> > created multiple times under a different parent node with no ranges
> > property.
> >
> > This fix is needed for the PowerNV i2c busses to be exposed properly
> > and will fix a number of other embedded cases.
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: <stable@vger.kernel.org>
> > ---
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index e371825..e37f017 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -7,6 +7,10 @@
> >  #include <linux/pci_regs.h>
> >  #include <linux/string.h>
> >
> > +#ifdef CONFIG_PPC
> > +#include <asm/machdep.h>
> > +#endif
> > +
> >  /* Max address size we deal with */
> >  #define OF_MAX_ADDR_CELLS      4
> >  #define OF_CHECK_ADDR_COUNT(na)        ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
> > @@ -428,12 +432,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> >          * This code is only enabled on powerpc. --gcl
> >          */
> >         ranges = of_get_property(parent, rprop, &rlen);
> > -#if !defined(CONFIG_PPC)
> > +#if defined(CONFIG_PPC)
> > +       if (!machine_is(powermac))
> 
> Can we use a machine compatible here or something not PPC specific?
> Then we can use IS_ENABLED(CONFIG_PPC) instead of ifdefs.

Yeah, that's kind of nasty!

g.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] of/base: Fix PowerPC address parsing hack
  2014-11-12 14:39 ` Rob Herring
  2014-11-12 17:02   ` Grant Likely
@ 2014-11-12 19:55   ` Benjamin Herrenschmidt
  1 sibling, 0 replies; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-11-12 19:55 UTC (permalink / raw)
  To: Rob Herring
  Cc: Grant Likely, devicetree, linux-kernel, Arnd Bergmann,
	Olof Johansson, linuxppc-dev

On Wed, 2014-11-12 at 08:39 -0600, Rob Herring wrote:
> On Tue, Nov 11, 2014 at 11:51 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > We have a historical hack that treats missing ranges properties as the
> > equivalent of an empty one. This is needed for ancient PowerMac "bad"
> > device-trees, and shouldn't be enabled for any other PowerPC platform,
> > otherwise we get some nasty layout of devices in sysfs or even
> > duplication when a set of otherwise identically named devices is
> > created multiple times under a different parent node with no ranges
> > property.
> >
> > This fix is needed for the PowerNV i2c busses to be exposed properly
> > and will fix a number of other embedded cases.
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: <stable@vger.kernel.org>
> > ---
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index e371825..e37f017 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -7,6 +7,10 @@
> >  #include <linux/pci_regs.h>
> >  #include <linux/string.h>
> >
> > +#ifdef CONFIG_PPC
> > +#include <asm/machdep.h>
> > +#endif
> > +
> >  /* Max address size we deal with */
> >  #define OF_MAX_ADDR_CELLS      4
> >  #define OF_CHECK_ADDR_COUNT(na)        ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
> > @@ -428,12 +432,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> >          * This code is only enabled on powerpc. --gcl
> >          */
> >         ranges = of_get_property(parent, rprop, &rlen);
> > -#if !defined(CONFIG_PPC)
> > +#if defined(CONFIG_PPC)
> > +       if (!machine_is(powermac))
> 
> Can we use a machine compatible here or something not PPC specific?
> Then we can use IS_ENABLED(CONFIG_PPC) instead of ifdefs.

We could, we'd have to use a pair of machine compatible, I'll spin a new
patch later today.

Ben.
 
> Rob
> 
> > +#endif /* defined(CONFIG_PPC) */
> >         if (ranges == NULL) {
> > -               pr_err("OF: no ranges; cannot translate\n");
> > +               pr_debug("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);
> >
> --
> 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/



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]  of/base: Fix PowerPC address parsing hack
  2014-11-12  5:51 [PATCH] of/base: Fix PowerPC address parsing hack Benjamin Herrenschmidt
  2014-11-12 14:39 ` Rob Herring
@ 2014-11-12 22:10 ` Stephen Rothwell
  2014-11-12 23:08   ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 12+ messages in thread
From: Stephen Rothwell @ 2014-11-12 22:10 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Rob Herring, Grant Likely, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]

Hi Ben,

Urk!  :-)

How about:

On Wed, 12 Nov 2014 16:51:01 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index e371825..e37f017 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -7,6 +7,10 @@
>  #include <linux/pci_regs.h>
>  #include <linux/string.h>
>  
> +#ifdef CONFIG_PPC
> +#include <asm/machdep.h>

#define IS_PMAC	machine_is(pmac)
#else
#define IS_PMAC	(0)

> +#endif
> +
>  /* Max address size we deal with */
>  #define OF_MAX_ADDR_CELLS	4
>  #define OF_CHECK_ADDR_COUNT(na)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
> @@ -428,12 +432,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>  	 * This code is only enabled on powerpc. --gcl
>  	 */
>  	ranges = of_get_property(parent, rprop, &rlen);
> -#if !defined(CONFIG_PPC)
> +#if defined(CONFIG_PPC)
> +	if (!machine_is(powermac))
> +#endif /* defined(CONFIG_PPC) */
>  	if (ranges == NULL) {

if ((!IS_PMAC) && (ranges == NULL)) {

> -		pr_err("OF: no ranges; cannot translate\n");
> +		pr_debug("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);

There might be a better identifier than IS_PMAC ...

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]  of/base: Fix PowerPC address parsing hack
  2014-11-12 22:10 ` Stephen Rothwell
@ 2014-11-12 23:08   ` Benjamin Herrenschmidt
  2014-11-13  0:45     ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-11-12 23:08 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Rob Herring, Grant Likely, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

On Thu, 2014-11-13 at 09:10 +1100, Stephen Rothwell wrote:
> Hi Ben,
> 
> Urk!  :-)
> 
> How about:
> 
> On Wed, 12 Nov 2014 16:51:01 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index e371825..e37f017 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -7,6 +7,10 @@
> >  #include <linux/pci_regs.h>
> >  #include <linux/string.h>
> >  
> > +#ifdef CONFIG_PPC
> > +#include <asm/machdep.h>
> 
> #define IS_PMAC	machine_is(pmac)
> #else
> #define IS_PMAC	(0)

I'll just do an of machine compatible check instead, so there's no ifdef
at all, I'll send a new patch later today.

> > +#endif
> > +
> >  /* Max address size we deal with */
> >  #define OF_MAX_ADDR_CELLS	4
> >  #define OF_CHECK_ADDR_COUNT(na)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
> > @@ -428,12 +432,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> >  	 * This code is only enabled on powerpc. --gcl
> >  	 */
> >  	ranges = of_get_property(parent, rprop, &rlen);
> > -#if !defined(CONFIG_PPC)
> > +#if defined(CONFIG_PPC)
> > +	if (!machine_is(powermac))
> > +#endif /* defined(CONFIG_PPC) */
> >  	if (ranges == NULL) {
> 
> if ((!IS_PMAC) && (ranges == NULL)) {
> 
> > -		pr_err("OF: no ranges; cannot translate\n");
> > +		pr_debug("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);
> 
> There might be a better identifier than IS_PMAC ...
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH]  of/base: Fix PowerPC address parsing hack
  2014-11-12 23:08   ` Benjamin Herrenschmidt
@ 2014-11-13  0:45     ` Benjamin Herrenschmidt
  2014-11-13  1:02       ` Stephen Rothwell
  2014-11-13 12:44       ` Grant Likely
  0 siblings, 2 replies; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-11-13  0:45 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Rob Herring, Grant Likely, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

What about this one instead ? I want to cache it because that function
can be called quite a while and doing two additional property lookup
and string compares every time might hurt some platforms.

----

We have a historical hack that treats missing ranges properties as the
equivalent of an empty one. This is needed for ancient PowerMac "bad"
device-trees, and shouldn't be enabled for any other PowerPC platform,
otherwise we get some nasty layout of devices in sysfs or even
duplication when a set of otherwise identically named devices is
created multiple times under a different parent node with no ranges
property.

This fix is needed for the PowerNV i2c busses to be exposed properly
and will fix a number of other embedded cases.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>

diff --git a/drivers/of/address.c b/drivers/of/address.c
index e371825..5eae0cd 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -403,6 +403,17 @@ static struct of_bus *of_match_bus(struct device_node *np)
 	return NULL;
 }
 
+static int of_empty_ranges_quirk(void)
+{
+	/* To save cycles, we cache the result */
+	static int quirk_state = -1;
+
+	if (quirk_state < 0)
+		quirk_state = of_machine_is_compatible("Power Macintosh") ||
+			of_machine_is_compatible("MacRISC");
+	return quirk_state;
+}
+
 static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 			    struct of_bus *pbus, __be32 *addr,
 			    int na, int ns, int pna, const char *rprop)
@@ -428,12 +439,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 	 * This code is only enabled on powerpc. --gcl
 	 */
 	ranges = of_get_property(parent, rprop, &rlen);
-#if !defined(CONFIG_PPC)
-	if (ranges == NULL) {
+	if (ranges == NULL && !of_empty_ranges_quirk()) {
 		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);



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH]  of/base: Fix PowerPC address parsing hack
  2014-11-13  0:45     ` Benjamin Herrenschmidt
@ 2014-11-13  1:02       ` Stephen Rothwell
  2014-11-13 12:44       ` Grant Likely
  1 sibling, 0 replies; 12+ messages in thread
From: Stephen Rothwell @ 2014-11-13  1:02 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Rob Herring, Grant Likely, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

Hi Ben,

On Thu, 13 Nov 2014 11:45:22 +1100 Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> What about this one instead ? I want to cache it because that function
> can be called quite a while and doing two additional property lookup
> and string compares every time might hurt some platforms.

Looks good to me.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] of/base: Fix PowerPC address parsing hack
  2014-11-13  0:45     ` Benjamin Herrenschmidt
  2014-11-13  1:02       ` Stephen Rothwell
@ 2014-11-13 12:44       ` Grant Likely
  2014-11-13 12:53         ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 12+ messages in thread
From: Grant Likely @ 2014-11-13 12:44 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Stephen Rothwell, Rob Herring, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

On Thu, Nov 13, 2014 at 12:45 AM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> What about this one instead ? I want to cache it because that function
> can be called quite a while and doing two additional property lookup
> and string compares every time might hurt some platforms.
>
> ----
>
> We have a historical hack that treats missing ranges properties as the
> equivalent of an empty one. This is needed for ancient PowerMac "bad"
> device-trees, and shouldn't be enabled for any other PowerPC platform,
> otherwise we get some nasty layout of devices in sysfs or even
> duplication when a set of otherwise identically named devices is
> created multiple times under a different parent node with no ranges
> property.
>
> This fix is needed for the PowerNV i2c busses to be exposed properly
> and will fix a number of other embedded cases.
>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>

How far back does this need to go? I assume I need to get this in for v3.18.

>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index e371825..5eae0cd 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -403,6 +403,17 @@ static struct of_bus *of_match_bus(struct device_node *np)
>         return NULL;
>  }
>
> +static int of_empty_ranges_quirk(void)
> +{
> +       /* To save cycles, we cache the result */
> +       static int quirk_state = -1;
> +

if (IS_ENABLED(CONFIG_POWERPC)) {

> +       if (quirk_state < 0)
> +               quirk_state = of_machine_is_compatible("Power Macintosh") ||
> +                       of_machine_is_compatible("MacRISC");
> +       return quirk_state;

}
return 0;

So it gets compiled out for non powerpc.

> +}
> +
>  static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>                             struct of_bus *pbus, __be32 *addr,
>                             int na, int ns, int pna, const char *rprop)
> @@ -428,12 +439,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>          * This code is only enabled on powerpc. --gcl
>          */
>         ranges = of_get_property(parent, rprop, &rlen);
> -#if !defined(CONFIG_PPC)
> -       if (ranges == NULL) {
> +       if (ranges == NULL && !of_empty_ranges_quirk()) {
>                 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);
>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] of/base: Fix PowerPC address parsing hack
  2014-11-13 12:44       ` Grant Likely
@ 2014-11-13 12:53         ` Benjamin Herrenschmidt
  2014-11-14  6:55           ` [PATCH v3] " Benjamin Herrenschmidt
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-11-13 12:53 UTC (permalink / raw)
  To: Grant Likely
  Cc: Stephen Rothwell, Rob Herring, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

On Thu, 2014-11-13 at 12:44 +0000, Grant Likely wrote:
> On Thu, Nov 13, 2014 at 12:45 AM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > What about this one instead ? I want to cache it because that function
> > can be called quite a while and doing two additional property lookup
> > and string compares every time might hurt some platforms.
> >
> > ----
> >
> > We have a historical hack that treats missing ranges properties as the
> > equivalent of an empty one. This is needed for ancient PowerMac "bad"
> > device-trees, and shouldn't be enabled for any other PowerPC platform,
> > otherwise we get some nasty layout of devices in sysfs or even
> > duplication when a set of otherwise identically named devices is
> > created multiple times under a different parent node with no ranges
> > property.
> >
> > This fix is needed for the PowerNV i2c busses to be exposed properly
> > and will fix a number of other embedded cases.
> >
> > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > CC: <stable@vger.kernel.org>
> 
> How far back does this need to go? I assume I need to get this in for v3.18.

I'd like some distro to pick it up in 3.16

> > diff --git a/drivers/of/address.c b/drivers/of/address.c
> > index e371825..5eae0cd 100644
> > --- a/drivers/of/address.c
> > +++ b/drivers/of/address.c
> > @@ -403,6 +403,17 @@ static struct of_bus *of_match_bus(struct device_node *np)
> >         return NULL;
> >  }
> >
> > +static int of_empty_ranges_quirk(void)
> > +{
> > +       /* To save cycles, we cache the result */
> > +       static int quirk_state = -1;
> > +
> 
> if (IS_ENABLED(CONFIG_POWERPC)) {
> 
> > +       if (quirk_state < 0)
> > +               quirk_state = of_machine_is_compatible("Power Macintosh") ||
> > +                       of_machine_is_compatible("MacRISC");
> > +       return quirk_state;
> 
> }
> return 0;
> 
> So it gets compiled out for non powerpc.

Yeah, I'm set in my ways, keep forgetting about all the shiny new stuff.

> > +}
> > +
> >  static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> >                             struct of_bus *pbus, __be32 *addr,
> >                             int na, int ns, int pna, const char *rprop)
> > @@ -428,12 +439,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
> >          * This code is only enabled on powerpc. --gcl
> >          */
> >         ranges = of_get_property(parent, rprop, &rlen);
> > -#if !defined(CONFIG_PPC)
> > -       if (ranges == NULL) {
> > +       if (ranges == NULL && !of_empty_ranges_quirk()) {
> >                 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);
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] of/base: Fix PowerPC address parsing hack
  2014-11-13 12:53         ` Benjamin Herrenschmidt
@ 2014-11-14  6:55           ` Benjamin Herrenschmidt
  2014-11-18 16:52             ` Grant Likely
  0 siblings, 1 reply; 12+ messages in thread
From: Benjamin Herrenschmidt @ 2014-11-14  6:55 UTC (permalink / raw)
  To: Grant Likely
  Cc: Stephen Rothwell, Rob Herring, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

We have a historical hack that treats missing ranges properties as the
equivalent of an empty one. This is needed for ancient PowerMac "bad"
device-trees, and shouldn't be enabled for any other PowerPC platform,
otherwise we get some nasty layout of devices in sysfs or even
duplication when a set of otherwise identically named devices is
created multiple times under a different parent node with no ranges
property.

This fix is needed for the PowerNV i2c busses to be exposed properly
and will fix a number of other embedded cases.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: <stable@vger.kernel.org>
---

V2: Make it less horrendously ugly

V3: use IS_ENABLED()

 drivers/of/address.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/of/address.c b/drivers/of/address.c
index e371825..42e416a 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -403,6 +403,21 @@ static struct of_bus *of_match_bus(struct device_node *np)
 	return NULL;
 }
 
+static int of_empty_ranges_quirk(void)
+{
+	if (IS_ENABLED(CONFIG_PPC)) {
+		/* To save cycles, we cache the result */
+		static int quirk_state = -1;
+
+		if (quirk_state < 0)
+			quirk_state =
+				of_machine_is_compatible("Power Macintosh") ||
+				of_machine_is_compatible("MacRISC");
+		return quirk_state;
+	}
+	return false;
+}
+
 static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 			    struct of_bus *pbus, __be32 *addr,
 			    int na, int ns, int pna, const char *rprop)
@@ -428,12 +443,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
 	 * This code is only enabled on powerpc. --gcl
 	 */
 	ranges = of_get_property(parent, rprop, &rlen);
-#if !defined(CONFIG_PPC)
-	if (ranges == NULL) {
+	if (ranges == NULL && !of_empty_ranges_quirk()) {
 		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);




^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] of/base: Fix PowerPC address parsing hack
  2014-11-14  6:55           ` [PATCH v3] " Benjamin Herrenschmidt
@ 2014-11-18 16:52             ` Grant Likely
  0 siblings, 0 replies; 12+ messages in thread
From: Grant Likely @ 2014-11-18 16:52 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Stephen Rothwell, Rob Herring, Olof Johansson, devicetree,
	linux-kernel, Arnd Bergmann, linuxppc-dev

On Fri, 14 Nov 2014 17:55:03 +1100
, Benjamin Herrenschmidt <benh@kernel.crashing.org>
 wrote:
> We have a historical hack that treats missing ranges properties as the
> equivalent of an empty one. This is needed for ancient PowerMac "bad"
> device-trees, and shouldn't be enabled for any other PowerPC platform,
> otherwise we get some nasty layout of devices in sysfs or even
> duplication when a set of otherwise identically named devices is
> created multiple times under a different parent node with no ranges
> property.
> 
> This fix is needed for the PowerNV i2c busses to be exposed properly
> and will fix a number of other embedded cases.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> CC: <stable@vger.kernel.org>

Acked-by: Grant Likely <grant.likely@linaro.org>

Rob will pick up this patch and send it to Linus in his fixups tree for
v3.18

g.

> ---
> 
> V2: Make it less horrendously ugly
> 
> V3: use IS_ENABLED()
> 
>  drivers/of/address.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index e371825..42e416a 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -403,6 +403,21 @@ static struct of_bus *of_match_bus(struct device_node *np)
>  	return NULL;
>  }
>  
> +static int of_empty_ranges_quirk(void)
> +{
> +	if (IS_ENABLED(CONFIG_PPC)) {
> +		/* To save cycles, we cache the result */
> +		static int quirk_state = -1;
> +
> +		if (quirk_state < 0)
> +			quirk_state =
> +				of_machine_is_compatible("Power Macintosh") ||
> +				of_machine_is_compatible("MacRISC");
> +		return quirk_state;
> +	}
> +	return false;
> +}
> +
>  static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>  			    struct of_bus *pbus, __be32 *addr,
>  			    int na, int ns, int pna, const char *rprop)
> @@ -428,12 +443,10 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus,
>  	 * This code is only enabled on powerpc. --gcl
>  	 */
>  	ranges = of_get_property(parent, rprop, &rlen);
> -#if !defined(CONFIG_PPC)
> -	if (ranges == NULL) {
> +	if (ranges == NULL && !of_empty_ranges_quirk()) {
>  		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);
> 
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2014-11-18 17:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-12  5:51 [PATCH] of/base: Fix PowerPC address parsing hack Benjamin Herrenschmidt
2014-11-12 14:39 ` Rob Herring
2014-11-12 17:02   ` Grant Likely
2014-11-12 19:55   ` Benjamin Herrenschmidt
2014-11-12 22:10 ` Stephen Rothwell
2014-11-12 23:08   ` Benjamin Herrenschmidt
2014-11-13  0:45     ` Benjamin Herrenschmidt
2014-11-13  1:02       ` Stephen Rothwell
2014-11-13 12:44       ` Grant Likely
2014-11-13 12:53         ` Benjamin Herrenschmidt
2014-11-14  6:55           ` [PATCH v3] " Benjamin Herrenschmidt
2014-11-18 16:52             ` Grant Likely

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).