2014-11-12 05:51:39

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: [PATCH] of/base: Fix PowerPC address parsing hack

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 <[email protected]>
CC: <[email protected]>
---

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


2014-11-12 14:39:56

by Rob Herring

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

On Tue, Nov 11, 2014 at 11:51 PM, Benjamin Herrenschmidt
<[email protected]> 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 <[email protected]>
> CC: <[email protected]>
> ---
>
> 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);
>

2014-11-12 19:55:35

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

On Wed, 2014-11-12 at 08:39 -0600, Rob Herring wrote:
> On Tue, Nov 11, 2014 at 11:51 PM, Benjamin Herrenschmidt
> <[email protected]> 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 <[email protected]>
> > CC: <[email protected]>
> > ---
> >
> > 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

2014-11-12 21:20:54

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

On Wed, 12 Nov 2014 08:39:32 -0600
, Rob Herring <[email protected]>
wrote:
> On Tue, Nov 11, 2014 at 11:51 PM, Benjamin Herrenschmidt
> <[email protected]> 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 <[email protected]>
> > CC: <[email protected]>
> > ---
> >
> > 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.

2014-11-12 22:10:45

by Stephen Rothwell

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

Hi Ben,

Urk! :-)

How about:

On Wed, 12 Nov 2014 16:51:01 +1100 Benjamin Herrenschmidt <[email protected]> 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 [email protected]


Attachments:
(No filename) (819.00 B)
OpenPGP digital signature

2014-11-12 23:09:16

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

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 <[email protected]> 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 ...
>

2014-11-13 00:45:53

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

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 <[email protected]>
CC: <[email protected]>

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

2014-11-13 01:02:49

by Stephen Rothwell

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

Hi Ben,

On Thu, 13 Nov 2014 11:45:22 +1100 Benjamin Herrenschmidt <[email protected]> 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 [email protected]


Attachments:
(No filename) (819.00 B)
OpenPGP digital signature

2014-11-13 12:44:42

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

On Thu, Nov 13, 2014 at 12:45 AM, Benjamin Herrenschmidt
<[email protected]> 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 <[email protected]>
> CC: <[email protected]>

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

2014-11-13 12:54:28

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] of/base: Fix PowerPC address parsing hack

On Thu, 2014-11-13 at 12:44 +0000, Grant Likely wrote:
> On Thu, Nov 13, 2014 at 12:45 AM, Benjamin Herrenschmidt
> <[email protected]> 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 <[email protected]>
> > CC: <[email protected]>
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-11-14 06:55:41

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: [PATCH v3] of/base: Fix PowerPC address parsing hack

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 <[email protected]>
CC: <[email protected]>
---

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


2014-11-18 17:00:12

by Grant Likely

[permalink] [raw]
Subject: Re: [PATCH v3] of/base: Fix PowerPC address parsing hack

On Fri, 14 Nov 2014 17:55:03 +1100
, Benjamin Herrenschmidt <[email protected]>
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 <[email protected]>
> CC: <[email protected]>

Acked-by: Grant Likely <[email protected]>

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