of/base: Fix PowerPC address parsing hack

PATCH Superseded Benjamin Herrenschmidt 4 participants 12 messages Nov 12, 2014 – Nov 18, 2014 Device tree

Share & tools

Benjamin Herrenschmidt

author

2014-11-12 05:51 UTC #0 lore raw

PATCH 1/1 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]>

---

drivers/of/address.c +8 -3

index e371825..e37f017 100644
@@ -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);

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

Rob Herring

2014-11-12 14:39 UTC #1 lore raw

Quoted 31 lines · Benjamin Herrenschmidt

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]>
---
drivers/of/address.c +4 -0
index e371825..e37f017 100644
@@ -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)

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 436

Quoted 6 lines

drivers/of/address.c +2 -1

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

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 444

Quoted 11 lines

drivers/of/address.c +2 -2

+#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 devicetree" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Grant Likely

2014-11-12 17:02 UTC #2 lore raw

Quoted 34 lines · Rob Herring

Quoted 32 lines · Benjamin Herrenschmidt
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]>
---
drivers/of/address.c +4 -0
index e371825..e37f017 100644
@@ -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

Quoted 8 lines

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 436
Quoted 5 lines
drivers/of/address.c +2 -1
         */
        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!

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

Benjamin Herrenschmidt

author

2014-11-12 19:55 UTC #3 lore raw

In reply to Rob Herring

Quoted 34 lines · Rob Herring

Quoted 32 lines · Benjamin Herrenschmidt
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]>
---
drivers/of/address.c +4 -0
index e371825..e37f017 100644
@@ -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

Quoted 8 lines

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 436
Quoted 5 lines
drivers/of/address.c +2 -1
         */
        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.

Quoted 18 lines

Rob
drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 444
Quoted 11 lines
drivers/of/address.c +2 -2
+#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/

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

Stephen Rothwell

2014-11-12 22:10 UTC #4 lore raw

In reply to Benjamin Herrenschmidt

Hi Ben,

Urk! :-)

How about:

drivers/of/address.c line 11

Quoted 11 lines · On Wed, 12 Nov 2014 16:51:01 +1100 Benjamin Herrenschmidt…

drivers/of/address.c +2 -0

index e371825..e37f017 100644
@@ -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)

Quoted 6 lines

drivers/of/address.c +2 -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)

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 438

Quoted 8 lines

drivers/of/address.c +3 -1

@@ -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)) {

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 444

Quoted 8 lines

drivers/of/address.c +1 -2

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

Signature · 2 lines

Cheers,
Stephen Rothwell                    [email protected]

Benjamin Herrenschmidt

author

2014-11-12 23:08 UTC #5 lore raw

Quoted 16 lines · Stephen Rothwell

Hi Ben,
Urk! :-)
How about:
Quoted 9 lines · On Wed, 12 Nov 2014 16:51:01 +1100 Benjamin Herrenschmidt…
drivers/of/address.c +0 -0
index e371825..e37f017 100644
@@ -7,6 +7,10 @@
 #include <linux/pci_regs.h>
 #include <linux/string.h>
 

Quoted 6 lines

drivers/of/address.c line 11
Quoted 2 lines
drivers/of/address.c +2 -0
+#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.

Quoted 17 lines

Quoted 6 lines

drivers/of/address.c +2 -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)

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 438

Quoted 8 lines

drivers/of/address.c +3 -1

@@ -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)) {

Quoted 11 lines

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 444
Quoted 8 lines
drivers/of/address.c +1 -2
-		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 ...

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

Benjamin Herrenschmidt

author

2014-11-13 00:45 UTC #6 lore raw

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.

----

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

drivers/of/address.c +12 -3

index e371825..5eae0cd 100644
@@ -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);

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

Stephen Rothwell

2014-11-13 01:02 UTC #7 lore raw

Hi Ben,

Quoted 4 lines · On Thu, 13 Nov 2014 11:45:22 +1100 Benjamin Herrenschmidt…

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.

Signature · 2 lines

Cheers,
Stephen Rothwell                    [email protected]

Grant Likely

2014-11-13 12:44 UTC #8 lore raw

In reply to Benjamin Herrenschmidt

Quoted 14 lines · Benjamin Herrenschmidt

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.

Quoted 5 lines

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.

Quoted 6 lines

drivers/of/address.c +0 -0
index e371825..5eae0cd 100644

drivers/of/address.c static struct of_bus *of_match_bus(struct device_node *np) · line 410

Quoted 8 lines

drivers/of/address.c +5 -0

@@ -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)) {

drivers/of/address.c static struct of_bus *of_match_bus(struct device_node *np) · line 414

Quoted 4 lines

drivers/of/address.c +4 -0

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

Quoted 11 lines

drivers/of/address.c +2 -0

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

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 448

Quoted 10 lines

drivers/of/address.c +1 -3

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

Benjamin Herrenschmidt

author

2014-11-13 12:53 UTC #9 lore raw

Quoted 16 lines · Grant Likely

Quoted 14 lines · Benjamin Herrenschmidt
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.

Quoted 7 lines

Quoted 5 lines
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

Quoted 21 lines

drivers/of/address.c static struct of_bus *of_match_bus(struct device_node *np) · line 410
Quoted 13 lines
drivers/of/address.c +5 -0
index e371825..5eae0cd 100644
@@ -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)) {
drivers/of/address.c static struct of_bus *of_match_bus(struct device_node *np) · line 414
Quoted 4 lines
drivers/of/address.c +4 -0
+       if (quirk_state < 0)
+               quirk_state = of_machine_is_compatible("Power Macintosh") ||
+                       of_machine_is_compatible("MacRISC");
+       return quirk_state;

Quoted 4 lines

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

Quoted 25 lines

Quoted 11 lines

drivers/of/address.c +2 -0

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

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 448

Quoted 10 lines

drivers/of/address.c +1 -3

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

Benjamin Herrenschmidt

author

2014-11-14 06:55 UTC #10 lore raw

PATCH v3 1/1 of/base: Fix PowerPC address parsing hack

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

drivers/of/address.c +16 -3

index e371825..42e416a 100644
@@ -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);

Grant Likely

Acked-by on v3 1/1

2014-11-18 16:52 UTC #11 lore raw

Quoted 8 lines · Benjamin Herrenschmidt

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.

Quoted 5 lines

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

Quoted 42 lines

---

V2: Make it less horrendously ugly

V3: use IS_ENABLED()

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

drivers/of/address.c +15 -0

index e371825..42e416a 100644
@@ -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);

drivers/of/address.c static int of_translate_one(struct device_node *parent, struct of_bus *bus, · line 452

Quoted 10 lines

drivers/of/address.c +1 -3

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