All of lore.kernel.org
 help / color / mirror / Atom feed
From: Przemyslaw Marczak <p.marczak@samsung.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] fdt: __of_translate_address(): check parent's 'ranges' before translate
Date: Thu, 07 Jan 2016 12:40:49 +0100	[thread overview]
Message-ID: <1452166849-24461-1-git-send-email-p.marczak@samsung.com> (raw)

The present implementation of __of_translate_address() taken
from the Linux, is designed for translate bus/child address
mappings by using 'ranges' property - and it doesn't allow
for checking an address for a device's node with zero size-cells.

The 'size-cells > 0' is required for bus/child address mapping,
but is not required for non-memory mapped address, e.g.: I2C chip.
Then when we need only raw 'reg' property's value.

Since the I2C device address goes to a single-cell reg property,
support for that case is welcome, but currently calling dev_get_addr()
for I2C device will return 'FDT_ADDR_T_NONE', and print the warning:

warning:
__of_translate_address: Bad cell count for 'some-dev'

The fix:
The proper result by this commit, is achieved by skipping
the condition check: 'size-cells > 0', when the parent doesn't
provide the 'ranges' property and allows to return 1:1 address
translation, if caller want's just the reg property's value.

No additional argument is needed, the 'ranges' property existence
decides, what type of translation is done when calling dev_get_addr().
And this should be, what the compatible driver expects.

Now, by this commit the function __of_translate_address() can be used
for the both reg property use-cases:

Case 1: (ranges)
----------------
some-bus {
	address-cells = <1>;
	size-cells = <1>;
	ranges = <0x0 0x10000000 0x1000>;
	reg = <0x10000000 0x1000>;

	child1 {
		reg = <0xa00 0x100>;
	};

	child2 {
		reg = <0xb00 0x100>;
	};
};

Return values (CONFIG_OF_TRANSLATE=y):
 - dev_get_addr(some-bus) - retrurns: 0x10000000 - correct
 - dev_get_addr(child1)   - retrurns: 0x10000a00 - correct
 - dev_get_addr(child2)   - retrurns: 0x10000b00 - correct
This works as previous - this commit have no impact on this case.

Case 2: (no ranges - e.g. I2C bus childs) - fixed
-------------------------------------------------
I2C-bus {
	address-cells = <1>;
	size-cells = <0>;
	reg = <0x10000000>;

	chip1 {
		reg = <0xa00>;
	};

	chip2 {
		reg = <0xb00>;
	};
};

Return values (CONFIG_OF_TRANSLATE=y):
 - dev_get_addr(I2C-bus) - retrurns: 0x10000000 - correct
 - dev_get_addr(chip1)   - retrurns: 0xa00      - correct
 - dev_get_addr(chip2)   - retrurns: 0xb00      - correct

This is fixed, since the previously returned value for chip1 and chip2
was: 0xffffffff, which means: 'FDT_ADDR_T_NONE'.

Signed-off-by: Przemyslaw Marczak <p.marczak@samsung.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Tom Rini <trini@konsulko.com>
Cc: Simon Glass <sjg@chromium.org>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
---
 common/fdt_support.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/common/fdt_support.c b/common/fdt_support.c
index 66464db..1f472ca 100644
--- a/common/fdt_support.c
+++ b/common/fdt_support.c
@@ -952,8 +952,9 @@ void fdt_del_node_and_alias(void *blob, const char *alias)
 /* Max address size we deal with */
 #define OF_MAX_ADDR_CELLS	4
 #define OF_BAD_ADDR	FDT_ADDR_T_NONE
-#define OF_CHECK_COUNTS(na, ns)	((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
-			(ns) > 0)
+#define OF_CHECK_COUNTS(na, ns, skip_ns) \
+			((na) > 0 && (na) <= OF_MAX_ADDR_CELLS && \
+			(skip_ns || (ns) > 0))
 
 /* Debug utility */
 #ifdef DEBUG
@@ -1104,11 +1105,12 @@ static int of_translate_one(void * blob, int parent, struct of_bus *bus,
 static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in_addr,
 				  const char *rprop)
 {
-	int parent;
-	struct of_bus *bus, *pbus;
+	const fdt32_t *pranges;
 	fdt32_t addr[OF_MAX_ADDR_CELLS];
-	int na, ns, pna, pns;
+	int na, ns, pna, pns, parent;
+	struct of_bus *bus, *pbus;
 	u64 result = OF_BAD_ADDR;
+	bool skip_ns_check;
 
 	debug("OF: ** translation for device %s **\n",
 		fdt_get_name(blob, node_offset, NULL));
@@ -1119,9 +1121,16 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in
 		goto bail;
 	bus = &of_busses[0];
 
-	/* Cound address cells & copy address locally */
+	/* Chek 'ns' only if parent provides 'ranges' property */
+	pranges = fdt_getprop(blob, parent, rprop, NULL);
+	if (pranges)
+		skip_ns_check = false;
+	else
+		skip_ns_check = true;
+
+	/* Count address cells & copy address locally */
 	bus->count_cells(blob, parent, &na, &ns);
-	if (!OF_CHECK_COUNTS(na, ns)) {
+	if (!OF_CHECK_COUNTS(na, ns, skip_ns_check)) {
 		printf("%s: Bad cell count for %s\n", __FUNCTION__,
 		       fdt_get_name(blob, node_offset, NULL));
 		goto bail;
@@ -1148,7 +1157,7 @@ static u64 __of_translate_address(void *blob, int node_offset, const fdt32_t *in
 		/* Get new parent bus and counts */
 		pbus = &of_busses[0];
 		pbus->count_cells(blob, parent, &pna, &pns);
-		if (!OF_CHECK_COUNTS(pna, pns)) {
+		if (!OF_CHECK_COUNTS(pna, pns, skip_ns_check)) {
 			printf("%s: Bad cell count for %s\n", __FUNCTION__,
 				fdt_get_name(blob, node_offset, NULL));
 			break;
-- 
1.9.1

             reply	other threads:[~2016-01-07 11:40 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-07 11:40 Przemyslaw Marczak [this message]
2016-01-07 13:03 ` [U-Boot] [PATCH] fdt: __of_translate_address(): check parent's 'ranges' before translate Lukasz Majewski
2016-01-07 18:25 ` Stephen Warren
2016-01-11 11:21   ` Przemyslaw Marczak
2016-01-11 16:47     ` Stephen Warren
2016-01-12 10:25       ` Przemyslaw Marczak
2016-01-12 13:57         ` Simon Glass
2016-01-12 14:22           ` Przemyslaw Marczak
2016-01-12 15:13             ` Simon Glass
2016-01-12 16:43         ` Stephen Warren
2016-01-13 11:10           ` Przemyslaw Marczak
2016-01-14 17:17             ` Simon Glass
2016-01-15 10:41               ` Przemyslaw Marczak
2016-01-15 16:35                 ` Stephen Warren
2016-01-29 18:23                   ` Simon Glass
2016-02-02  8:55                     ` Przemyslaw Marczak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1452166849-24461-1-git-send-email-p.marczak@samsung.com \
    --to=p.marczak@samsung.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.