Skip to contentSkip to conversation

libfdt: add address translation support

PATCH In review Rob Herring 7 participants 22 messages Apr 1, 2014 – May 14, 2014 9 messages not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 libfdt: add address translation support

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Cc Grant Likely <[email protected]>
Cc Scott Wood <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
---
 libfdt/fdt_ro.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |   4 ++
 2 files changed, 207 insertions(+)
libfdt/fdt_ro.c +203 -0
index 50007f6..3c1bde1 100644
@@ -571,3 +571,206 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
 	return offset; /* error from fdt_next_node() */
 }
+
+/* Max address size we deal with */
+#define OF_MAX_ADDR_CELLS	4
+#define OF_CHECK_COUNTS(na, ns)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
+			(ns) > 0)
+
+/* Helper to read a big number; size is in cells (not bytes) */
+static uint64_t fdt_read_number(const fdt32_t *cell, int size)
+{
+	uint64_t r = 0;
+	while (size--)
+		r = (r << 32) | fdt32_to_cpu(*(cell++));
+	return r;
+}
+
+/* Callbacks for bus specific translators */
+struct of_bus {
+	void (*count_cells)(void *blob, int parentoffset,
+			    int *addrc, int *sizec);
+	uint64_t (*map)(fdt32_t *addr, const fdt32_t *range,
+				int na, int ns, int pna);
+	int (*translate)(fdt32_t *addr, uint64_t offset, int na);
+};
+
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+				 	int *addrc, int *sizec)
+{
+	const fdt32_t *prop;
+
+	if (addrc) {
+		prop = fdt_getprop(blob, parentoffset, "#address-cells", NULL);
+		if (prop)
+			*addrc = fdt32_to_cpu(*prop);
+		else
+			*addrc = 2;
+	}
+
+	if (sizec) {
+		prop = fdt_getprop(blob, parentoffset, "#size-cells", NULL);
+		if (prop)
+			*sizec = fdt32_to_cpu(*prop);
+		else
+			*sizec = 2;
+	}
+}
+
+static uint64_t fdt_bus_default_map(fdt32_t *addr, const fdt32_t *range,
+				    int na, int ns, int pna)
+{
+	uint64_t cp, s, da;
+
+	cp = fdt_read_number(range, na);
+	s  = fdt_read_number(range + na + pna, ns);
+	da = fdt_read_number(addr, na);
+
+	if (da < cp || da >= (cp + s))
+		return FDT_BAD_ADDR;
+	return da - cp;
+}
+
+static int fdt_bus_default_translate(fdt32_t *addr, uint64_t offset, int na)
+{
+	uint64_t a = fdt_read_number(addr, na);
+	memset(addr, 0, na * 4);
+	a += offset;
+	if (na > 1)
+		addr[na - 2] = cpu_to_fdt32(a >> 32);
+	addr[na - 1] = cpu_to_fdt32(a & 0xffffffffu);
+
+	return 0;
+}
+
+/* Array of bus specific translators */
+static const struct of_bus of_busses[] = {
+	/* Default */
+	{
+		.count_cells = fdt_bus_default_count_cells,
+		.map = fdt_bus_default_map,
+		.translate = fdt_bus_default_translate,
+	},
+};
+
+static int fdt_translate_one(void * blob, int parent,
+			     const struct of_bus *bus,
+			     const struct of_bus *pbus, fdt32_t *addr,
+			     int na, int ns, int pna, const char *rprop)
+{
+	const fdt32_t *ranges;
+	int rlen;
+	int rone;
+	uint64_t offset = FDT_BAD_ADDR;
+
+	/* Normally, an absence of a "ranges" property means we are
+	 * crossing a non-translatable boundary, and thus the addresses
+	 * below the current not cannot be converted to CPU physical ones.
+	 * Unfortunately, while this is very clear in the spec, it's not
+	 * what Apple understood, and they do have things like /uni-n or
+	 * /ht nodes with no "ranges" property and a lot of perfectly
+	 * useable mapped devices below them. Thus we treat the absence of
+	 * "ranges" as equivalent to an empty "ranges" property which means
+	 * 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.
+	 */
+	ranges = fdt_getprop(blob, parent, rprop, &rlen);
+	if (ranges == NULL || rlen == 0) {
+		offset = fdt_read_number(addr, na);
+		memset(addr, 0, pna * 4);
+		goto finish;
+	}
+
+	/* Now walk through the ranges */
+	rlen /= 4;
+	rone = na + pna + ns;
+	for (; rlen >= rone; rlen -= rone, ranges += rone) {
+		offset = bus->map(addr, ranges, na, ns, pna);
+		if (offset != FDT_BAD_ADDR)
+			break;
+	}
+	if (offset == FDT_BAD_ADDR)
+		return -FDT_ERR_NOTFOUND;
+
+	memcpy(addr, ranges + na, 4 * pna);
+
+ finish:
+	/* Translate it into parent bus space */
+	return pbus->translate(addr, offset, pna);
+}
+
+static uint64_t __fdt_translate_address(void *blob, int node_offset,
+					const char *rprop)
+{
+	int parent, len;
+	const struct of_bus *bus, *pbus;
+	const fdt32_t *reg;
+	fdt32_t addr[OF_MAX_ADDR_CELLS];
+	int na, ns, pna, pns;
+	uint64_t result = FDT_BAD_ADDR;
+
+	reg = fdt_getprop(blob, node_offset, "reg", &len);
+	if (!reg)
+		goto bail;
+
+	/* Get parent & match bus type */
+	parent = fdt_parent_offset(blob, node_offset);
+	if (parent < 0)
+		goto bail;
+	bus = &of_busses[0];
+
+	/* Cound address cells & copy address locally */
+	bus->count_cells(blob, parent, &na, &ns);
+	if (!OF_CHECK_COUNTS(na, ns))
+		goto bail;
+
+	memcpy(addr, reg, na * 4);
+
+	/* Translate */
+	for (;;) {
+		/* Switch to parent bus */
+		node_offset = parent;
+		parent = fdt_parent_offset(blob, node_offset);
+
+		/* If root, we have finished */
+		if (parent < 0) {
+			result = fdt_read_number(addr, na);
+			break;
+		}
+
+		/* Get new parent bus and counts */
+		pbus = &of_busses[0];
+		pbus->count_cells(blob, parent, &pna, &pns);
+		if (!OF_CHECK_COUNTS(pna, pns))
+			break;
+
+		/* Apply bus translation */
+		if (fdt_translate_one(blob, node_offset, bus, pbus,
+					addr, na, ns, pna, "ranges"))
+			break;
+
+		/* Complete the move up one level */
+		na = pna;
+		ns = pns;
+		bus = pbus;
+	}
+ bail:
+	return result;
+}
+
+/*
+ * Translate an address from the device-tree into a CPU physical address,
+ * this walks up the tree and applies the various bus mappings on the
+ * way.
+ *
+ * Note: We consider that crossing any level with #size-cells == 0 to mean
+ * that translation is impossible (that is we are not dealing with a value
+ * that can be mapped to a cpu physical address). This is not really specified
+ * that way, but this is traditionally the way IBM at least do things
+ */
+uint64_t fdt_translate_address(void *fdt, int node_offset)
+{
+	return __fdt_translate_address(fdt, node_offset, "ranges");
+}
libfdt/libfdt.h +4 -0
index c4d5a91..afbea65 100644
@@ -118,6 +118,8 @@
 
 #define FDT_ERR_MAX		13
 
+#define FDT_BAD_ADDR		((uint64_t)-1)
+
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
 /**********************************************************************/
@@ -852,6 +854,8 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
  */
 int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
 
+uint64_t fdt_translate_address(void *blob, int node_offset);
+
 /**********************************************************************/
 /* Write-in-place functions                                           */
 /**********************************************************************/
Signature · 6 lines
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1396371463-7516-1-git-send-email-robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

#2 lore raw
Quoted 2 lines · Rob Herring

Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

In the absence of a formal, written conveyance of the Copyright, it belongs to the author(s) of that file.

Warner

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 4073722A-7162-4041-B1A2-8A992871FEC0-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org

#4 lore raw
Quoted 8 lines · Warner Losh
Quoted 2 lines · Rob Herring

Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

In the absence of a formal, written conveyance of the Copyright, it belongs to the author(s) of that file.

It might depend on jurisdiction, but I don't believe presence or absence of a copyright notice actually changes who holds the copyright. It just makes it easier or harder to figure out who htat is.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

A message that is not in the archive (sent off-list or not yet received): [email protected]

#6 lore raw
Quoted 7 lines · David Gibson
Quoted 6 lines · Warner Losh
Quoted 2 lines · Rob Herring

Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Quoted 7 lines
Quoted 2 lines

In the absence of a formal, written conveyance of the Copyright, it belongs to the author(s) of that file.

It might depend on jurisdiction, but I don't believe presence or absence of a copyright notice actually changes who holds the copyright. It just makes it easier or harder to figure out who htat is.

Pedantically correct. In the US copyright can only be transferred in writing. Most other countries are similar, but there is still variation despite the attempts to make it completely uniform. But the important bit is that there’s no “default” author in copyright law if one fails to document the author. It doesn’t go to Torvalds or The Linux Foundation or the Easter Bunny. The original author is the person who wrote it, or if it was a work for hire or such a contract exists then the employer of the person who wrote it. Just to add more wrinkles to the mix.

Warner

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): AB306DF0-D50C-476A-8314-CFCE5C57BB2E-uzTCJ5RojNnQT0dZR+AlfA@public.gmane.org

#8 lore raw
Quoted 18 lines · Warner Losh
Quoted 7 lines · David Gibson
Quoted 6 lines · Warner Losh
Quoted 2 lines · Rob Herring

Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Quoted 7 lines
Quoted 2 lines

In the absence of a formal, written conveyance of the Copyright, it belongs to the author(s) of that file.

It might depend on jurisdiction, but I don't believe presence or absence of a copyright notice actually changes who holds the copyright. It just makes it easier or harder to figure out who htat is.

Quoted 8 lines

Pedantically correct. In the US copyright can only be transferred in writing. Most other countries are similar, but there is still variation despite the attempts to make it completely uniform. But the important bit is that there’s no “default” author in copyright law if one fails to document the author. It doesn’t go to Torvalds or The Linux Foundation or the Easter Bunny. The original author is the person who wrote it, or if it was a work for hire or such a contract exists then the employer of the person who wrote it. Just to add more wrinkles to the mix.

Right, we are in agreement.

I'm sorry, I misread your initial comment as referring to a copyright notice, rather than a separate conveyance of copyright.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
#9 lore raw
In reply to a message not in the archive
Quoted 12 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

Quoted 9 lines

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

ACK for the Freescale contribution (on copyright issues, not technical -- see below).

FWIW, if you have trouble tracking everyone down for this code, there's address translation code in arch/powerpc/boot/devtree.c that has a cleaner history (I wrote it from scratch, and there's been only minor modifications since then, by Mark Greer).

Quoted 55 lines
Cc Grant Likely <[email protected]>
Cc Scott Wood <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
---
 libfdt/fdt_ro.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |   4 ++
 2 files changed, 207 insertions(+)
libfdt/fdt_ro.c +36 -0
index 50007f6..3c1bde1 100644
@@ -571,3 +571,206 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
 	return offset; /* error from fdt_next_node() */
 }
+
+/* Max address size we deal with */
+#define OF_MAX_ADDR_CELLS	4
+#define OF_CHECK_COUNTS(na, ns)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
+			(ns) > 0)
+
+/* Helper to read a big number; size is in cells (not bytes) */
+static uint64_t fdt_read_number(const fdt32_t *cell, int size)
+{
+	uint64_t r = 0;
+	while (size--)
+		r = (r << 32) | fdt32_to_cpu(*(cell++));
+	return r;
+}
+
+/* Callbacks for bus specific translators */
+struct of_bus {
+	void (*count_cells)(void *blob, int parentoffset,
+			    int *addrc, int *sizec);
+	uint64_t (*map)(fdt32_t *addr, const fdt32_t *range,
+				int na, int ns, int pna);
+	int (*translate)(fdt32_t *addr, uint64_t offset, int na);
+};
+
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+				 	int *addrc, int *sizec)
+{
+	const fdt32_t *prop;
+
+	if (addrc) {
+		prop = fdt_getprop(blob, parentoffset, "#address-cells", NULL);
+		if (prop)
+			*addrc = fdt32_to_cpu(*prop);
+		else
+			*addrc = 2;

libfdt/fdt_ro.c int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 619

Quoted 10 lines
libfdt/fdt_ro.c +10 -0
+	}
+
+	if (sizec) {
+		prop = fdt_getprop(blob, parentoffset, "#size-cells", NULL);
+		if (prop)
+			*sizec = fdt32_to_cpu(*prop);
+		else
+			*sizec = 2;
+	}
+}

#size-cells should default to 1 as per IEEE1275 (ePAPR agrees, though it calls a missing #size-cells non-compliant). This is also what kernel code currently seems to do (see OF_ROOT_NODE_SIZE_CELLS_DEFAULT).

Quoted 48 lines
libfdt/fdt_ro.c +48 -0
+static uint64_t fdt_bus_default_map(fdt32_t *addr, const fdt32_t *range,
+				    int na, int ns, int pna)
+{
+	uint64_t cp, s, da;
+
+	cp = fdt_read_number(range, na);
+	s  = fdt_read_number(range + na + pna, ns);
+	da = fdt_read_number(addr, na);
+
+	if (da < cp || da >= (cp + s))
+		return FDT_BAD_ADDR;
+	return da - cp;
+}
+
+static int fdt_bus_default_translate(fdt32_t *addr, uint64_t offset, int na)
+{
+	uint64_t a = fdt_read_number(addr, na);
+	memset(addr, 0, na * 4);
+	a += offset;
+	if (na > 1)
+		addr[na - 2] = cpu_to_fdt32(a >> 32);
+	addr[na - 1] = cpu_to_fdt32(a & 0xffffffffu);
+
+	return 0;
+}
+
+/* Array of bus specific translators */
+static const struct of_bus of_busses[] = {
+	/* Default */
+	{
+		.count_cells = fdt_bus_default_count_cells,
+		.map = fdt_bus_default_map,
+		.translate = fdt_bus_default_translate,
+	},
+};
+
+static int fdt_translate_one(void * blob, int parent,
+			     const struct of_bus *bus,
+			     const struct of_bus *pbus, fdt32_t *addr,
+			     int na, int ns, int pna, const char *rprop)
+{
+	const fdt32_t *ranges;
+	int rlen;
+	int rone;
+	uint64_t offset = FDT_BAD_ADDR;
+
+	/* Normally, an absence of a "ranges" property means we are
+	 * crossing a non-translatable boundary, and thus the addresses

libfdt/fdt_ro.c int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 678

Quoted 10 lines
libfdt/fdt_ro.c +10 -0
+	 * below the current not cannot be converted to CPU physical ones.
+	 * Unfortunately, while this is very clear in the spec, it's not
+	 * what Apple understood, and they do have things like /uni-n or
+	 * /ht nodes with no "ranges" property and a lot of perfectly
+	 * useable mapped devices below them. Thus we treat the absence of
+	 * "ranges" as equivalent to an empty "ranges" property which means
+	 * 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.
+	 */

I don't think libfdt should be copying a non-compliant hack that was meant to deal with broken Apple firmware that doesn't use flat trees.

-Scott

Quoted 112 lines
libfdt/fdt_ro.c +98 -0
+	ranges = fdt_getprop(blob, parent, rprop, &rlen);
+	if (ranges == NULL || rlen == 0) {
+		offset = fdt_read_number(addr, na);
+		memset(addr, 0, pna * 4);
+		goto finish;
+	}
+
+	/* Now walk through the ranges */
+	rlen /= 4;
+	rone = na + pna + ns;
+	for (; rlen >= rone; rlen -= rone, ranges += rone) {
+		offset = bus->map(addr, ranges, na, ns, pna);
+		if (offset != FDT_BAD_ADDR)
+			break;
+	}
+	if (offset == FDT_BAD_ADDR)
+		return -FDT_ERR_NOTFOUND;
+
+	memcpy(addr, ranges + na, 4 * pna);
+
+ finish:
+	/* Translate it into parent bus space */
+	return pbus->translate(addr, offset, pna);
+}
+
+static uint64_t __fdt_translate_address(void *blob, int node_offset,
+					const char *rprop)
+{
+	int parent, len;
+	const struct of_bus *bus, *pbus;
+	const fdt32_t *reg;
+	fdt32_t addr[OF_MAX_ADDR_CELLS];
+	int na, ns, pna, pns;
+	uint64_t result = FDT_BAD_ADDR;
+
+	reg = fdt_getprop(blob, node_offset, "reg", &len);
+	if (!reg)
+		goto bail;
+
+	/* Get parent & match bus type */
+	parent = fdt_parent_offset(blob, node_offset);
+	if (parent < 0)
+		goto bail;
+	bus = &of_busses[0];
+
+	/* Cound address cells & copy address locally */
+	bus->count_cells(blob, parent, &na, &ns);
+	if (!OF_CHECK_COUNTS(na, ns))
+		goto bail;
+
+	memcpy(addr, reg, na * 4);
+
+	/* Translate */
+	for (;;) {
+		/* Switch to parent bus */
+		node_offset = parent;
+		parent = fdt_parent_offset(blob, node_offset);
+
+		/* If root, we have finished */
+		if (parent < 0) {
+			result = fdt_read_number(addr, na);
+			break;
+		}
+
+		/* Get new parent bus and counts */
+		pbus = &of_busses[0];
+		pbus->count_cells(blob, parent, &pna, &pns);
+		if (!OF_CHECK_COUNTS(pna, pns))
+			break;
+
+		/* Apply bus translation */
+		if (fdt_translate_one(blob, node_offset, bus, pbus,
+					addr, na, ns, pna, "ranges"))
+			break;
+
+		/* Complete the move up one level */
+		na = pna;
+		ns = pns;
+		bus = pbus;
+	}
+ bail:
+	return result;
+}
+
+/*
+ * Translate an address from the device-tree into a CPU physical address,
+ * this walks up the tree and applies the various bus mappings on the
+ * way.
+ *
+ * Note: We consider that crossing any level with #size-cells == 0 to mean
+ * that translation is impossible (that is we are not dealing with a value
+ * that can be mapped to a cpu physical address). This is not really specified
+ * that way, but this is traditionally the way IBM at least do things
+ */
+uint64_t fdt_translate_address(void *fdt, int node_offset)
+{
+	return __fdt_translate_address(fdt, node_offset, "ranges");
+}
libfdt/libfdt.h +2 -0
index c4d5a91..afbea65 100644
@@ -118,6 +118,8 @@
 
 #define FDT_ERR_MAX		13
 
+#define FDT_BAD_ADDR		((uint64_t)-1)
+
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
 /**********************************************************************/

libfdt/libfdt.h int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 861

Quoted 8 lines
libfdt/libfdt.h +2 -0
@@ -852,6 +854,8 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
  */
 int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
 
+uint64_t fdt_translate_address(void *blob, int node_offset);
+
 /**********************************************************************/
 /* Write-in-place functions                                           */
 /**********************************************************************/

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 1396393358.32034.40.camel-88ow+0ZRuxG2UiBs7uKeOtHuzzzSOjJt@public.gmane.org

#11 lore raw
Quoted 23 lines · On Tue, 1 Apr 2014 18:02:38 -0500 Scott Wood…
Quoted 12 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

Quoted 9 lines

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Quoted 1 line

ACK for the Freescale contribution (on copyright issues, not technical -- see below).

FWIW, ack from me too.

libfdt/fdt_ro.c int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 600

Quoted 3 lines
libfdt/fdt_ro.c +3 -0
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+				 	int *addrc, int *sizec)

space before tab in indent (git am).

Also, thanks for not introducing new sparse warnings :)

Kim

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20140402142403.9d3316f14520252a4a3474e0-KZfg59tc24xl57MIdRCFDg@public.gmane.org

#13 lore raw
Quoted 26 lines · Kim Phillips
Quoted 23 lines · On Tue, 1 Apr 2014 18:02:38 -0500 Scott Wood…
Quoted 21 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Quoted 3 lines
Quoted 1 line

ACK for the Freescale contribution (on copyright issues, not technical -- see below).

FWIW, ack from me too.

Thanks.

Quoted 7 lines

libfdt/fdt_ro.c int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 600

Quoted 3 lines
libfdt/fdt_ro.c +3 -0
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+                                     int *addrc, int *sizec)

space before tab in indent (git am).

Also, thanks for not introducing new sparse warnings :)

I believe that's mainly because someone already did on the u-boot source. :)

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#14 lore raw
In reply to a message not in the archive

Ben,

Quoted 23 lines · Scott Wood
Quoted 12 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

Quoted 9 lines

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Quoted 1 line

ACK for the Freescale contribution (on copyright issues, not technical -- see below).

Can you please comment on the re-licensing of this code.

Rob

Quoted 254 lines

FWIW, if you have trouble tracking everyone down for this code, there's address translation code in arch/powerpc/boot/devtree.c that has a cleaner history (I wrote it from scratch, and there's been only minor modifications since then, by Mark Greer).

Quoted 55 lines
Cc Grant Likely <[email protected]>
Cc Scott Wood <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
---
 libfdt/fdt_ro.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |   4 ++
 2 files changed, 207 insertions(+)
libfdt/fdt_ro.c +36 -0
index 50007f6..3c1bde1 100644
@@ -571,3 +571,206 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
      return offset; /* error from fdt_next_node() */
 }
+
+/* Max address size we deal with */
+#define OF_MAX_ADDR_CELLS    4
+#define OF_CHECK_COUNTS(na, ns)      ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
+                     (ns) > 0)
+
+/* Helper to read a big number; size is in cells (not bytes) */
+static uint64_t fdt_read_number(const fdt32_t *cell, int size)
+{
+     uint64_t r = 0;
+     while (size--)
+             r = (r << 32) | fdt32_to_cpu(*(cell++));
+     return r;
+}
+
+/* Callbacks for bus specific translators */
+struct of_bus {
+     void (*count_cells)(void *blob, int parentoffset,
+                         int *addrc, int *sizec);
+     uint64_t (*map)(fdt32_t *addr, const fdt32_t *range,
+                             int na, int ns, int pna);
+     int (*translate)(fdt32_t *addr, uint64_t offset, int na);
+};
+
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+                                     int *addrc, int *sizec)
+{
+     const fdt32_t *prop;
+
+     if (addrc) {
+             prop = fdt_getprop(blob, parentoffset, "#address-cells", NULL);
+             if (prop)
+                     *addrc = fdt32_to_cpu(*prop);
+             else
+                     *addrc = 2;

libfdt/fdt_ro.c int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 619

Quoted 10 lines
libfdt/fdt_ro.c +10 -0
+     }
+
+     if (sizec) {
+             prop = fdt_getprop(blob, parentoffset, "#size-cells", NULL);
+             if (prop)
+                     *sizec = fdt32_to_cpu(*prop);
+             else
+                     *sizec = 2;
+     }
+}

#size-cells should default to 1 as per IEEE1275 (ePAPR agrees, though it calls a missing #size-cells non-compliant). This is also what kernel code currently seems to do (see OF_ROOT_NODE_SIZE_CELLS_DEFAULT).

Quoted 48 lines
libfdt/fdt_ro.c +48 -0
+static uint64_t fdt_bus_default_map(fdt32_t *addr, const fdt32_t *range,
+                                 int na, int ns, int pna)
+{
+     uint64_t cp, s, da;
+
+     cp = fdt_read_number(range, na);
+     s  = fdt_read_number(range + na + pna, ns);
+     da = fdt_read_number(addr, na);
+
+     if (da < cp || da >= (cp + s))
+             return FDT_BAD_ADDR;
+     return da - cp;
+}
+
+static int fdt_bus_default_translate(fdt32_t *addr, uint64_t offset, int na)
+{
+     uint64_t a = fdt_read_number(addr, na);
+     memset(addr, 0, na * 4);
+     a += offset;
+     if (na > 1)
+             addr[na - 2] = cpu_to_fdt32(a >> 32);
+     addr[na - 1] = cpu_to_fdt32(a & 0xffffffffu);
+
+     return 0;
+}
+
+/* Array of bus specific translators */
+static const struct of_bus of_busses[] = {
+     /* Default */
+     {
+             .count_cells = fdt_bus_default_count_cells,
+             .map = fdt_bus_default_map,
+             .translate = fdt_bus_default_translate,
+     },
+};
+
+static int fdt_translate_one(void * blob, int parent,
+                          const struct of_bus *bus,
+                          const struct of_bus *pbus, fdt32_t *addr,
+                          int na, int ns, int pna, const char *rprop)
+{
+     const fdt32_t *ranges;
+     int rlen;
+     int rone;
+     uint64_t offset = FDT_BAD_ADDR;
+
+     /* Normally, an absence of a "ranges" property means we are
+      * crossing a non-translatable boundary, and thus the addresses

libfdt/fdt_ro.c int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 678

Quoted 10 lines
libfdt/fdt_ro.c +10 -0
+      * below the current not cannot be converted to CPU physical ones.
+      * Unfortunately, while this is very clear in the spec, it's not
+      * what Apple understood, and they do have things like /uni-n or
+      * /ht nodes with no "ranges" property and a lot of perfectly
+      * useable mapped devices below them. Thus we treat the absence of
+      * "ranges" as equivalent to an empty "ranges" property which means
+      * 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.
+      */

I don't think libfdt should be copying a non-compliant hack that was meant to deal with broken Apple firmware that doesn't use flat trees.

-Scott

Quoted 115 lines
libfdt/fdt_ro.c +98 -0
+     ranges = fdt_getprop(blob, parent, rprop, &rlen);
+     if (ranges == NULL || rlen == 0) {
+             offset = fdt_read_number(addr, na);
+             memset(addr, 0, pna * 4);
+             goto finish;
+     }
+
+     /* Now walk through the ranges */
+     rlen /= 4;
+     rone = na + pna + ns;
+     for (; rlen >= rone; rlen -= rone, ranges += rone) {
+             offset = bus->map(addr, ranges, na, ns, pna);
+             if (offset != FDT_BAD_ADDR)
+                     break;
+     }
+     if (offset == FDT_BAD_ADDR)
+             return -FDT_ERR_NOTFOUND;
+
+     memcpy(addr, ranges + na, 4 * pna);
+
+ finish:
+     /* Translate it into parent bus space */
+     return pbus->translate(addr, offset, pna);
+}
+
+static uint64_t __fdt_translate_address(void *blob, int node_offset,
+                                     const char *rprop)
+{
+     int parent, len;
+     const struct of_bus *bus, *pbus;
+     const fdt32_t *reg;
+     fdt32_t addr[OF_MAX_ADDR_CELLS];
+     int na, ns, pna, pns;
+     uint64_t result = FDT_BAD_ADDR;
+
+     reg = fdt_getprop(blob, node_offset, "reg", &len);
+     if (!reg)
+             goto bail;
+
+     /* Get parent & match bus type */
+     parent = fdt_parent_offset(blob, node_offset);
+     if (parent < 0)
+             goto bail;
+     bus = &of_busses[0];
+
+     /* Cound address cells & copy address locally */
+     bus->count_cells(blob, parent, &na, &ns);
+     if (!OF_CHECK_COUNTS(na, ns))
+             goto bail;
+
+     memcpy(addr, reg, na * 4);
+
+     /* Translate */
+     for (;;) {
+             /* Switch to parent bus */
+             node_offset = parent;
+             parent = fdt_parent_offset(blob, node_offset);
+
+             /* If root, we have finished */
+             if (parent < 0) {
+                     result = fdt_read_number(addr, na);
+                     break;
+             }
+
+             /* Get new parent bus and counts */
+             pbus = &of_busses[0];
+             pbus->count_cells(blob, parent, &pna, &pns);
+             if (!OF_CHECK_COUNTS(pna, pns))
+                     break;
+
+             /* Apply bus translation */
+             if (fdt_translate_one(blob, node_offset, bus, pbus,
+                                     addr, na, ns, pna, "ranges"))
+                     break;
+
+             /* Complete the move up one level */
+             na = pna;
+             ns = pns;
+             bus = pbus;
+     }
+ bail:
+     return result;
+}
+
+/*
+ * Translate an address from the device-tree into a CPU physical address,
+ * this walks up the tree and applies the various bus mappings on the
+ * way.
+ *
+ * Note: We consider that crossing any level with #size-cells == 0 to mean
+ * that translation is impossible (that is we are not dealing with a value
+ * that can be mapped to a cpu physical address). This is not really specified
+ * that way, but this is traditionally the way IBM at least do things
+ */
+uint64_t fdt_translate_address(void *fdt, int node_offset)
+{
+     return __fdt_translate_address(fdt, node_offset, "ranges");
+}
libfdt/libfdt.h +2 -0
index c4d5a91..afbea65 100644
@@ -118,6 +118,8 @@
 
 #define FDT_ERR_MAX          13
 
+#define FDT_BAD_ADDR         ((uint64_t)-1)
+
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
 /**********************************************************************/
@@ -852,6 +854,8 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
  */
 int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
 
Quoted 8 lines

libfdt/libfdt.h int fdt_node_offset_by_compatible(const void *fdt, int startoffset, · line 861

Quoted 5 lines
libfdt/libfdt.h +2 -0
+uint64_t fdt_translate_address(void *blob, int node_offset);
+
 /**********************************************************************/
 /* Write-in-place functions                                           */
 /**********************************************************************/

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): CAL_JsqKMUHiPeU7iQDHZ=E6GzMxvXBB4L8Arg+m0JrfQsRkhtg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#16 lore raw
Quoted 26 lines · Rob Herring

Ben,

Quoted 23 lines · Scott Wood
Quoted 21 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt. Who is the copyright holder on a file in the kernel with no copyright? Does this default to the author or Linus or nobody?

Quoted 4 lines
Quoted 1 line

ACK for the Freescale contribution (on copyright issues, not technical -- see below).

Can you please comment on the re-licensing of this code.

Ping.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#17 lore raw
In reply to a message not in the archive
Quoted 12 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

Quoted 8 lines

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt.

Okay, so this makes things a bit more complicated. You need to compose a list of every commit that has touched this file in *both* the u-boot and kernel trees, and identify all of the copyright holders based on who submitted the patches. Once you have that list, then you need to get an Ack from every single one of them. If anyone does not respond, or does not give agreement, then we need to look at their change and determine whether or not we consider it to be a significant enough change that it would warrant copyright protection.

g.

Quoted 248 lines

Who is the copyright holder on a file in the kernel
with no copyright? Does this default to the author or Linus or nobody?

Cc Grant Likely <[email protected]>
Cc Scott Wood <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
---
 libfdt/fdt_ro.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |   4 ++
 2 files changed, 207 insertions(+)
libfdt/fdt_ro.c +203 -0
index 50007f6..3c1bde1 100644
@@ -571,3 +571,206 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
        return offset; /* error from fdt_next_node() */
 }
+
+/* Max address size we deal with */
+#define OF_MAX_ADDR_CELLS      4
+#define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
+                       (ns) > 0)
+
+/* Helper to read a big number; size is in cells (not bytes) */
+static uint64_t fdt_read_number(const fdt32_t *cell, int size)
+{
+       uint64_t r = 0;
+       while (size--)
+               r = (r << 32) | fdt32_to_cpu(*(cell++));
+       return r;
+}
+
+/* Callbacks for bus specific translators */
+struct of_bus {
+       void (*count_cells)(void *blob, int parentoffset,
+                           int *addrc, int *sizec);
+       uint64_t (*map)(fdt32_t *addr, const fdt32_t *range,
+                               int na, int ns, int pna);
+       int (*translate)(fdt32_t *addr, uint64_t offset, int na);
+};
+
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+                                       int *addrc, int *sizec)
+{
+       const fdt32_t *prop;
+
+       if (addrc) {
+               prop = fdt_getprop(blob, parentoffset, "#address-cells", NULL);
+               if (prop)
+                       *addrc = fdt32_to_cpu(*prop);
+               else
+                       *addrc = 2;
+       }
+
+       if (sizec) {
+               prop = fdt_getprop(blob, parentoffset, "#size-cells", NULL);
+               if (prop)
+                       *sizec = fdt32_to_cpu(*prop);
+               else
+                       *sizec = 2;
+       }
+}
+
+static uint64_t fdt_bus_default_map(fdt32_t *addr, const fdt32_t *range,
+                                   int na, int ns, int pna)
+{
+       uint64_t cp, s, da;
+
+       cp = fdt_read_number(range, na);
+       s  = fdt_read_number(range + na + pna, ns);
+       da = fdt_read_number(addr, na);
+
+       if (da < cp || da >= (cp + s))
+               return FDT_BAD_ADDR;
+       return da - cp;
+}
+
+static int fdt_bus_default_translate(fdt32_t *addr, uint64_t offset, int na)
+{
+       uint64_t a = fdt_read_number(addr, na);
+       memset(addr, 0, na * 4);
+       a += offset;
+       if (na > 1)
+               addr[na - 2] = cpu_to_fdt32(a >> 32);
+       addr[na - 1] = cpu_to_fdt32(a & 0xffffffffu);
+
+       return 0;
+}
+
+/* Array of bus specific translators */
+static const struct of_bus of_busses[] = {
+       /* Default */
+       {
+               .count_cells = fdt_bus_default_count_cells,
+               .map = fdt_bus_default_map,
+               .translate = fdt_bus_default_translate,
+       },
+};
+
+static int fdt_translate_one(void * blob, int parent,
+                            const struct of_bus *bus,
+                            const struct of_bus *pbus, fdt32_t *addr,
+                            int na, int ns, int pna, const char *rprop)
+{
+       const fdt32_t *ranges;
+       int rlen;
+       int rone;
+       uint64_t offset = FDT_BAD_ADDR;
+
+       /* Normally, an absence of a "ranges" property means we are
+        * crossing a non-translatable boundary, and thus the addresses
+        * below the current not cannot be converted to CPU physical ones.
+        * Unfortunately, while this is very clear in the spec, it's not
+        * what Apple understood, and they do have things like /uni-n or
+        * /ht nodes with no "ranges" property and a lot of perfectly
+        * useable mapped devices below them. Thus we treat the absence of
+        * "ranges" as equivalent to an empty "ranges" property which means
+        * 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.
+        */
+       ranges = fdt_getprop(blob, parent, rprop, &rlen);
+       if (ranges == NULL || rlen == 0) {
+               offset = fdt_read_number(addr, na);
+               memset(addr, 0, pna * 4);
+               goto finish;
+       }
+
+       /* Now walk through the ranges */
+       rlen /= 4;
+       rone = na + pna + ns;
+       for (; rlen >= rone; rlen -= rone, ranges += rone) {
+               offset = bus->map(addr, ranges, na, ns, pna);
+               if (offset != FDT_BAD_ADDR)
+                       break;
+       }
+       if (offset == FDT_BAD_ADDR)
+               return -FDT_ERR_NOTFOUND;
+
+       memcpy(addr, ranges + na, 4 * pna);
+
+ finish:
+       /* Translate it into parent bus space */
+       return pbus->translate(addr, offset, pna);
+}
+
+static uint64_t __fdt_translate_address(void *blob, int node_offset,
+                                       const char *rprop)
+{
+       int parent, len;
+       const struct of_bus *bus, *pbus;
+       const fdt32_t *reg;
+       fdt32_t addr[OF_MAX_ADDR_CELLS];
+       int na, ns, pna, pns;
+       uint64_t result = FDT_BAD_ADDR;
+
+       reg = fdt_getprop(blob, node_offset, "reg", &len);
+       if (!reg)
+               goto bail;
+
+       /* Get parent & match bus type */
+       parent = fdt_parent_offset(blob, node_offset);
+       if (parent < 0)
+               goto bail;
+       bus = &of_busses[0];
+
+       /* Cound address cells & copy address locally */
+       bus->count_cells(blob, parent, &na, &ns);
+       if (!OF_CHECK_COUNTS(na, ns))
+               goto bail;
+
+       memcpy(addr, reg, na * 4);
+
+       /* Translate */
+       for (;;) {
+               /* Switch to parent bus */
+               node_offset = parent;
+               parent = fdt_parent_offset(blob, node_offset);
+
+               /* If root, we have finished */
+               if (parent < 0) {
+                       result = fdt_read_number(addr, na);
+                       break;
+               }
+
+               /* Get new parent bus and counts */
+               pbus = &of_busses[0];
+               pbus->count_cells(blob, parent, &pna, &pns);
+               if (!OF_CHECK_COUNTS(pna, pns))
+                       break;
+
+               /* Apply bus translation */
+               if (fdt_translate_one(blob, node_offset, bus, pbus,
+                                       addr, na, ns, pna, "ranges"))
+                       break;
+
+               /* Complete the move up one level */
+               na = pna;
+               ns = pns;
+               bus = pbus;
+       }
+ bail:
+       return result;
+}
+
+/*
+ * Translate an address from the device-tree into a CPU physical address,
+ * this walks up the tree and applies the various bus mappings on the
+ * way.
+ *
+ * Note: We consider that crossing any level with #size-cells == 0 to mean
+ * that translation is impossible (that is we are not dealing with a value
+ * that can be mapped to a cpu physical address). This is not really specified
+ * that way, but this is traditionally the way IBM at least do things
+ */
+uint64_t fdt_translate_address(void *fdt, int node_offset)
+{
+       return __fdt_translate_address(fdt, node_offset, "ranges");
+}
libfdt/libfdt.h +4 -1
index c4d5a91..afbea65 100644
@@ -118,6 +118,8 @@
 
 #define FDT_ERR_MAX            13
 
+#define FDT_BAD_ADDR           ((uint64_t)-1)
+
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
 /**********************************************************************/
@@ -852,6 +854,8 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
  */
 int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
 
+uint64_t fdt_translate_address(void *blob, int node_offset);
+
 /**********************************************************************/
 /* Write-in-place functions                                           */
 /**********************************************************************/
--
Quoted 2 lines

1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): CACxGe6uPerPT_cT+p1zvL20ERwuguDJWDHasxz5gAUe5kPQHNg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#19 lore raw
Quoted 22 lines · Grant Likely
Quoted 12 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

Quoted 8 lines

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt.

Quoted 8 lines

Okay, so this makes things a bit more complicated. You need to compose a list of every commit that has touched this file in *both* the u-boot and kernel trees, and identify all of the copyright holders based on who submitted the patches. Once you have that list, then you need to get an Ack from every single one of them. If anyone does not respond, or does not give agreement, then we need to look at their change and determine whether or not we consider it to be a significant enough change that it would warrant copyright protection.

Fortunately, it is not that complicated. The u-boot version was done almost exactly the same time as the PPC version was moved by you (June 2010). The next commit that was not you was Oct 2010. Therefore, any changes to drivers/of/address.c can be ignored (I'm assuming you will ack this :)). So we only need to look at the powerpc version in arch/powerpc/kernel/prom_parse.c. Ignoring lines other than the default address translation code, there are only 3 committers: Ben, Jeremy Kerr, and Stephen Rothwell. The overwhelming majority of the code is from the original commit by Ben (hence why I only considered him to begin with).

Here are the commits from Jeremy and Stephen. I would characterize as trivial fixes, but I've copied both of them here.

Stephen:
e2eb639 [POWERPC] Rename get_property to of_get_property: arch/powerpc 9213fee [POWERPC] Rename prom_n_size_cells to of_n_size_cells a8bda5d [POWERPC] Rename prom_n_addr_cells to of_n_addr_cells

Jeremy:
a7f67bd [POWERPC] Constify & voidify get_property() b5a1a9a [POWERPC] Use const qualifiers for prom parsing utilites

Here is the git blame I used for the analysis:
git blame dbbdee9^ arch/powerpc/kernel/prom_parse.c

For the u-boot side, I don't think the situation is any different from we originally discussed. I've verified the copyrights are in agreement with the history. The file has relevant copyrights (although missing source attribution), and all changes of the relevant code are/were employees of the copyright holder Freescale.

Rob

Quoted 246 lines

g.

Quoted 242 lines

Who is the copyright holder on a file in the kernel
with no copyright? Does this default to the author or Linus or nobody?

Cc Grant Likely <[email protected]>
Cc Scott Wood <[email protected]>
Cc Kim Phillips <[email protected]>
Cc Kumar Gala <[email protected]>
Cc Benjamin Herrenschmidt <[email protected]>
---
 libfdt/fdt_ro.c | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 libfdt/libfdt.h |   4 ++
 2 files changed, 207 insertions(+)
libfdt/fdt_ro.c +203 -0
index 50007f6..3c1bde1 100644
@@ -571,3 +571,206 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
 
        return offset; /* error from fdt_next_node() */
 }
+
+/* Max address size we deal with */
+#define OF_MAX_ADDR_CELLS      4
+#define OF_CHECK_COUNTS(na, ns)        ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
+                       (ns) > 0)
+
+/* Helper to read a big number; size is in cells (not bytes) */
+static uint64_t fdt_read_number(const fdt32_t *cell, int size)
+{
+       uint64_t r = 0;
+       while (size--)
+               r = (r << 32) | fdt32_to_cpu(*(cell++));
+       return r;
+}
+
+/* Callbacks for bus specific translators */
+struct of_bus {
+       void (*count_cells)(void *blob, int parentoffset,
+                           int *addrc, int *sizec);
+       uint64_t (*map)(fdt32_t *addr, const fdt32_t *range,
+                               int na, int ns, int pna);
+       int (*translate)(fdt32_t *addr, uint64_t offset, int na);
+};
+
+/* Default translator (generic bus) */
+static void fdt_bus_default_count_cells(void *blob, int parentoffset,
+                                       int *addrc, int *sizec)
+{
+       const fdt32_t *prop;
+
+       if (addrc) {
+               prop = fdt_getprop(blob, parentoffset, "#address-cells", NULL);
+               if (prop)
+                       *addrc = fdt32_to_cpu(*prop);
+               else
+                       *addrc = 2;
+       }
+
+       if (sizec) {
+               prop = fdt_getprop(blob, parentoffset, "#size-cells", NULL);
+               if (prop)
+                       *sizec = fdt32_to_cpu(*prop);
+               else
+                       *sizec = 2;
+       }
+}
+
+static uint64_t fdt_bus_default_map(fdt32_t *addr, const fdt32_t *range,
+                                   int na, int ns, int pna)
+{
+       uint64_t cp, s, da;
+
+       cp = fdt_read_number(range, na);
+       s  = fdt_read_number(range + na + pna, ns);
+       da = fdt_read_number(addr, na);
+
+       if (da < cp || da >= (cp + s))
+               return FDT_BAD_ADDR;
+       return da - cp;
+}
+
+static int fdt_bus_default_translate(fdt32_t *addr, uint64_t offset, int na)
+{
+       uint64_t a = fdt_read_number(addr, na);
+       memset(addr, 0, na * 4);
+       a += offset;
+       if (na > 1)
+               addr[na - 2] = cpu_to_fdt32(a >> 32);
+       addr[na - 1] = cpu_to_fdt32(a & 0xffffffffu);
+
+       return 0;
+}
+
+/* Array of bus specific translators */
+static const struct of_bus of_busses[] = {
+       /* Default */
+       {
+               .count_cells = fdt_bus_default_count_cells,
+               .map = fdt_bus_default_map,
+               .translate = fdt_bus_default_translate,
+       },
+};
+
+static int fdt_translate_one(void * blob, int parent,
+                            const struct of_bus *bus,
+                            const struct of_bus *pbus, fdt32_t *addr,
+                            int na, int ns, int pna, const char *rprop)
+{
+       const fdt32_t *ranges;
+       int rlen;
+       int rone;
+       uint64_t offset = FDT_BAD_ADDR;
+
+       /* Normally, an absence of a "ranges" property means we are
+        * crossing a non-translatable boundary, and thus the addresses
+        * below the current not cannot be converted to CPU physical ones.
+        * Unfortunately, while this is very clear in the spec, it's not
+        * what Apple understood, and they do have things like /uni-n or
+        * /ht nodes with no "ranges" property and a lot of perfectly
+        * useable mapped devices below them. Thus we treat the absence of
+        * "ranges" as equivalent to an empty "ranges" property which means
+        * 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.
+        */
+       ranges = fdt_getprop(blob, parent, rprop, &rlen);
+       if (ranges == NULL || rlen == 0) {
+               offset = fdt_read_number(addr, na);
+               memset(addr, 0, pna * 4);
+               goto finish;
+       }
+
+       /* Now walk through the ranges */
+       rlen /= 4;
+       rone = na + pna + ns;
+       for (; rlen >= rone; rlen -= rone, ranges += rone) {
+               offset = bus->map(addr, ranges, na, ns, pna);
+               if (offset != FDT_BAD_ADDR)
+                       break;
+       }
+       if (offset == FDT_BAD_ADDR)
+               return -FDT_ERR_NOTFOUND;
+
+       memcpy(addr, ranges + na, 4 * pna);
+
+ finish:
+       /* Translate it into parent bus space */
+       return pbus->translate(addr, offset, pna);
+}
+
+static uint64_t __fdt_translate_address(void *blob, int node_offset,
+                                       const char *rprop)
+{
+       int parent, len;
+       const struct of_bus *bus, *pbus;
+       const fdt32_t *reg;
+       fdt32_t addr[OF_MAX_ADDR_CELLS];
+       int na, ns, pna, pns;
+       uint64_t result = FDT_BAD_ADDR;
+
+       reg = fdt_getprop(blob, node_offset, "reg", &len);
+       if (!reg)
+               goto bail;
+
+       /* Get parent & match bus type */
+       parent = fdt_parent_offset(blob, node_offset);
+       if (parent < 0)
+               goto bail;
+       bus = &of_busses[0];
+
+       /* Cound address cells & copy address locally */
+       bus->count_cells(blob, parent, &na, &ns);
+       if (!OF_CHECK_COUNTS(na, ns))
+               goto bail;
+
+       memcpy(addr, reg, na * 4);
+
+       /* Translate */
+       for (;;) {
+               /* Switch to parent bus */
+               node_offset = parent;
+               parent = fdt_parent_offset(blob, node_offset);
+
+               /* If root, we have finished */
+               if (parent < 0) {
+                       result = fdt_read_number(addr, na);
+                       break;
+               }
+
+               /* Get new parent bus and counts */
+               pbus = &of_busses[0];
+               pbus->count_cells(blob, parent, &pna, &pns);
+               if (!OF_CHECK_COUNTS(pna, pns))
+                       break;
+
+               /* Apply bus translation */
+               if (fdt_translate_one(blob, node_offset, bus, pbus,
+                                       addr, na, ns, pna, "ranges"))
+                       break;
+
+               /* Complete the move up one level */
+               na = pna;
+               ns = pns;
+               bus = pbus;
+       }
+ bail:
+       return result;
+}
+
+/*
+ * Translate an address from the device-tree into a CPU physical address,
+ * this walks up the tree and applies the various bus mappings on the
+ * way.
+ *
+ * Note: We consider that crossing any level with #size-cells == 0 to mean
+ * that translation is impossible (that is we are not dealing with a value
+ * that can be mapped to a cpu physical address). This is not really specified
+ * that way, but this is traditionally the way IBM at least do things
+ */
+uint64_t fdt_translate_address(void *fdt, int node_offset)
+{
+       return __fdt_translate_address(fdt, node_offset, "ranges");
+}
libfdt/libfdt.h +2 -0
index c4d5a91..afbea65 100644
@@ -118,6 +118,8 @@
 
 #define FDT_ERR_MAX            13
 
+#define FDT_BAD_ADDR           ((uint64_t)-1)
+
 /**********************************************************************/
 /* Low-level functions (you probably don't need these)                */
 /**********************************************************************/
@@ -852,6 +854,8 @@ int fdt_node_offset_by_compatible(const void *fdt, int startoffset,
  */
 int fdt_stringlist_contains(const char *strlist, int listlen, const char *str);
 
Quoted 8 lines
Quoted 8 lines
libfdt/libfdt.h +2 -1
+uint64_t fdt_translate_address(void *blob, int node_offset);
+
 /**********************************************************************/
 /* Write-in-place functions                                           */
 /**********************************************************************/
--

1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#20 lore raw
In reply to a message not in the archive
Quoted 22 lines · Grant Likely
Quoted 12 lines · Rob Herring

From: Rob Herring <[email protected]>

Add FDT based address translation. Currently, only simple bus translations are supported, but the framework allows adding other buses like PCI. Adding this to libfdt allows the address translation code to be shared amongst u-boot, Linux kernel and other projects.

This code is based on GPL only licensed code. It must first be re-licensed for dual GPL/BSD to add to libfdt which requires approval from the copyright holders.

Quoted 8 lines

This code is copied from u-boot common/fdt_support.c. The portion used here was originally added to u-boot by Kumar Gala in 2010 and Freescale is the copyright holder. Later changes have also only been done by Freescale authors. The u-boot code appears to have been copied from the Linux kernel's address translation code in drivers/of/address.c which has no copyright. This code was moved by Grant Likely in 2010 and originated from arch/powerpc/kernel/prom_parse.c which was written by Ben Herrenschmidt.

Quoted 8 lines

Okay, so this makes things a bit more complicated. You need to compose a list of every commit that has touched this file in *both* the u-boot and kernel trees, and identify all of the copyright holders based on who submitted the patches. Once you have that list, then you need to get an Ack from every single one of them. If anyone does not respond, or does not give agreement, then we need to look at their change and determine whether or not we consider it to be a significant enough change that it would warrant copyright protection.

Actually, don't bother.

I'm not all that impressed by this translation code - in particular the per-bus-type hooks don't make sense to me. AFAIK the interpretation of ranges is not bus specific. I think it will also fail in some cases with #address-cells > 2, which is unfortunate.

I'm about to post my own implementation of address translation.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
#21 lore raw
Quoted 6 lines · David Gibson

I'm not all that impressed by this translation code - in particular the per-bus-type hooks don't make sense to me. AFAIK the interpretation of ranges is not bus specific. I think it will also fail in some cases with #address-cells > 2, which is unfortunate.

I'm about to post my own implementation of address translation.

Well, I came up with the original code which did per-bus type hooks. There are cases where it is needed.

Take PCI. The top word can contain completely unrelated stuff such as the "prefetchable" attribute.

It's perfectly legal to put a prefetchable BAR under a non-prefetchable bridge window.

However a translation scheme that doesn't know to know that bit will fail.

It's not far fetched, it happens on our machines today.

And that's just one of the problems I had back then...

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#22 lore raw
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 6 lines · David Gibson

I'm not all that impressed by this translation code - in particular the per-bus-type hooks don't make sense to me. AFAIK the interpretation of ranges is not bus specific. I think it will also fail in some cases with #address-cells > 2, which is unfortunate.

I'm about to post my own implementation of address translation.

Well, I came up with the original code which did per-bus type hooks. There are cases where it is needed.

Quoted 8 lines

Take PCI. The top word can contain completely unrelated stuff such as the "prefetchable" attribute.

It's perfectly legal to put a prefetchable BAR under a non-prefetchable bridge window.

However a translation scheme that doesn't know to know that bit will fail.

Oh and the other way around with IO vs. memory btw.

Quoted 3 lines

It's not far fetched, it happens on our machines today.

And that's just one of the problems I had back then...

Note that I'm just stating the problem :-) There may be a better way to address it. For example, for the one above specifically, well, we could have a per-bus-type list of masks to apply before translation (bits to ignore).

I wish in fact OF had done that to begin with like we have interrupt-map-mask, maybe we should introduce that concept to limit the damage to existing well known cases like PCI and have anything new just tell us what to use for the compare.

In fact, thinking more, the above mask trick might not fully work for PCI, we have to check the exact values, because I think the same field somewhat use an enumeration for 32-bit MEM, 64-bit MEM and IO ... oh well.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#23 lore raw
Quoted 37 lines · Benjamin Herrenschmidt
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 6 lines · David Gibson

I'm not all that impressed by this translation code - in particular the per-bus-type hooks don't make sense to me. AFAIK the interpretation of ranges is not bus specific. I think it will also fail in some cases with #address-cells > 2, which is unfortunate.

I'm about to post my own implementation of address translation.

Well, I came up with the original code which did per-bus type hooks. There are cases where it is needed.

Quoted 8 lines

Take PCI. The top word can contain completely unrelated stuff such as the "prefetchable" attribute.

It's perfectly legal to put a prefetchable BAR under a non-prefetchable bridge window.

However a translation scheme that doesn't know to know that bit will fail.

Oh and the other way around with IO vs. memory btw.

Quoted 3 lines

It's not far fetched, it happens on our machines today.

And that's just one of the problems I had back then...

Note that I'm just stating the problem :-) There may be a better way to address it. For example, for the one above specifically, well, we could have a per-bus-type list of masks to apply before translation (bits to ignore).

I wish in fact OF had done that to begin with like we have interrupt-map-mask, maybe we should introduce that concept to limit the damage to existing well known cases like PCI and have anything new just tell us what to use for the compare.

Quoted 4 lines

In fact, thinking more, the above mask trick might not fully work for PCI, we have to check the exact values, because I think the same field somewhat use an enumeration for 32-bit MEM, 64-bit MEM and IO ... oh well.

Hm, yeah. At this point I'm thinking a per-bus "quirks" mechanism around the basic universal translation logic.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
#24 lore raw
In reply to Benjamin Herrenschmidt
Quoted 17 lines · Benjamin Herrenschmidt
Quoted 6 lines · David Gibson

I'm not all that impressed by this translation code - in particular the per-bus-type hooks don't make sense to me. AFAIK the interpretation of ranges is not bus specific. I think it will also fail in some cases with #address-cells > 2, which is unfortunate.

I'm about to post my own implementation of address translation.

Well, I came up with the original code which did per-bus type hooks. There are cases where it is needed.

Take PCI. The top word can contain completely unrelated stuff such as the "prefetchable" attribute.

It's perfectly legal to put a prefetchable BAR under a non-prefetchable bridge window.

Quoted 6 lines

However a translation scheme that doesn't know to know that bit will fail.

It's not far fetched, it happens on our machines today.

And that's just one of the problems I had back then...

But what you wrote was for unflattened tree. For early FDT uses, do we really need to worry about PCI or other special cases? The current FDT address translation code in arch/powerpc/boot/devtree.c (yes, now we have 3 implementations) does not for example.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): CAL_Jsq+zi72PAjQ+RGvUddLtHocgphPQQ9x2X0S0dYGOKz+Hgg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#26 lore raw
Quoted 4 lines · Rob Herring

But what you wrote was for unflattened tree. For early FDT uses, do we really need to worry about PCI or other special cases? The current FDT address translation code in arch/powerpc/boot/devtree.c (yes, now we have 3 implementations) does not for example.

This is specific to a bootloader case yes and it's ... fishy.

PCI on ARM is here now. PCI is everywhere (well, pseudo-PCI but that's the basic issue here) on Intel embedded.

Some embedded systems will probably want to enumerate from the firmware and Linux will need to translate. For example because the UART or some critical GPIO that needs to be handled early on is behind a pseudo-PCI interface of some sort.

I think we need to handle these cases rather than fail in obscure ways.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#27 lore raw
Quoted 11 lines · Benjamin Herrenschmidt
Quoted 4 lines · Rob Herring

But what you wrote was for unflattened tree. For early FDT uses, do we really need to worry about PCI or other special cases? The current FDT address translation code in arch/powerpc/boot/devtree.c (yes, now we have 3 implementations) does not for example.

This is specific to a bootloader case yes and it's ... fishy.

PCI on ARM is here now. PCI is everywhere (well, pseudo-PCI but that's the basic issue here) on Intel embedded.

Quoted 6 lines

Some embedded systems will probably want to enumerate from the firmware and Linux will need to translate. For example because the UART or some critical GPIO that needs to be handled early on is behind a pseudo-PCI interface of some sort.

I think we need to handle these cases rather than fail in obscure ways.

Yeah, I think PCI is way to common to just ignore.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
#28 lore raw
In reply to Benjamin Herrenschmidt
Quoted 17 lines · Benjamin Herrenschmidt
Quoted 6 lines · David Gibson

I'm not all that impressed by this translation code - in particular the per-bus-type hooks don't make sense to me. AFAIK the interpretation of ranges is not bus specific. I think it will also fail in some cases with #address-cells > 2, which is unfortunate.

I'm about to post my own implementation of address translation.

Well, I came up with the original code which did per-bus type hooks. There are cases where it is needed.

Take PCI. The top word can contain completely unrelated stuff such as the "prefetchable" attribute.

It's perfectly legal to put a prefetchable BAR under a non-prefetchable bridge window.

Quoted 6 lines

However a translation scheme that doesn't know to know that bit will fail.

It's not far fetched, it happens on our machines today.

And that's just one of the problems I had back then...

Ah. Bother.

I supposed it's arguable that a PCI ranges property really should have windows for both prefetchable and non-prefetchable areas, but since that's not done in practice, it's pretty much moot.

Ok, I'll need to rethink.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
#29 lore raw
Quoted 3 lines · David Gibson

I supposed it's arguable that a PCI ranges property really should have windows for both prefetchable and non-prefetchable areas, but since that's not done in practice, it's pretty much moot.

They don't have to, only if there's a relevant difference, ie it's perfectly legit for a firmware to assign prefetchable BARs in a non-prefetchable region.

Now granted, those are somewhat special cases so maybe we should do just that ... special case PCI and try to not continue that mess with new bindings.

Cheers,
Ben.

--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

#30 lore raw
Quoted 8 lines · Benjamin Herrenschmidt
Quoted 3 lines · David Gibson

I supposed it's arguable that a PCI ranges property really should have windows for both prefetchable and non-prefetchable areas, but since that's not done in practice, it's pretty much moot.

They don't have to, only if there's a relevant difference, ie it's perfectly legit for a firmware to assign prefetchable BARs in a non-prefetchable region.

In what sense is the window region non-prefetchable? Isn't that a property only of the final destination of the bus cycle?

Quoted 3 lines

Now granted, those are somewhat special cases so maybe we should do just that ... special case PCI and try to not continue that mess with new bindings.

Yeah, I think that's the way to go.

Signature · 4 lines
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson