Thread (1 message) 1 message, 1 author, 2015-03-28

Re: [PATCH V3 5/7] serial: earlycon: Set UPIO_MEM32BE based on DT properties

From: Peter Hurley <hidden>
Date: 2015-03-28 17:01:24
Also in: linux-devicetree, linux-serial

Possibly related (same subject, not in this thread)

Hi Grant,

On 03/27/2015 09:36 PM, Grant Likely wrote:
On Sun, 01 Mar 2015 17:23:11 -0500
, Peter Hurley [off-list ref]
 wrote:
quoted
Hi Kevin,

On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
quoted
If an earlycon (stdout-path) node is being used, check for "big-endian"
or "native-endian" properties and pass the appropriate iotype to the
driver.

Note that LE sets UPIO_MEM (8-bit) but BE sets UPIO_MEM32BE (32-bit).  The
big-endian property only really makes sense in the context of 32-bit
registers, since 8-bit accesses never require data swapping.

At some point, the of_earlycon code may want to pass in the reg-io-width,
reg-offset, and reg-shift parameters too.

Signed-off-by: Kevin Cernekee <cernekee@gmail.com>
---
 drivers/of/fdt.c              | 7 ++++++-
 drivers/tty/serial/earlycon.c | 4 ++--
 include/linux/serial_core.h   | 2 +-
 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index 658656f..9d21472 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -794,6 +794,8 @@ int __init early_init_dt_scan_chosen_serial(void)
 
 	while (match->compatible[0]) {
 		unsigned long addr;
+		unsigned char iotype = UPIO_MEM;
+
 		if (fdt_node_check_compatible(fdt, offset, match->compatible)) {
 			match++;
 			continue;
@@ -803,7 +805,10 @@ int __init early_init_dt_scan_chosen_serial(void)
 		if (!addr)
 			return -ENXIO;
 
-		of_setup_earlycon(addr, match->data);
+		if (of_fdt_is_big_endian(fdt, offset))
+			iotype = UPIO_MEM32BE;
+
+		of_setup_earlycon(addr, iotype, match->data);
I know these got ACKs already but as you point out in the commit log,
earlycon _will_ need reg-io-width, reg-offset and reg-shift. Since the
distinction between early_init_dt_scan_chosen_serial() and
of_setup_earlycon() is arbitrary, I'd rather see of_setup_earlycon()
taught to properly decode of_serial driver bindings instead of a
stack of parameters to of_setup_earlycon().

In fact, this patch allows a mis-defined devicetree to bring up a
functioning earlycon because the 'big-endian' property is directly
associated with UPIO_MEM32BE, which will create incompatibility problems
when DT earlycon is fixed to decode the of_serial DT bindings.
That's a good point. This hasn't been merged yet, so there isn't any
impact on addressing this. I would propose that for consistency, the
earlycon code should always default to 8-bit access. if big-endian
accesses are required, then reg-io-width + big-endian must be specified.

Something like the following would do it and would be future-proof. We
can add support for 16 or 64bit big or little endian access if it ever
became necessary.
I was planning on adding MEM32BE support to OF earlycon on top of my
patch series 'OF earlycon cleanup', which adds full support for the
of_serial driver DT properties (among other things).

Unfortunately, that series is waiting on two things:
1. libfdt upstream patch, which I submitted but was referred back to me
to add test cases. That was 3 weeks ago and I simply haven't had a free
day to burn to figure out how their test matrix is organized. I don't
think that's going to change anytime soon; I might just abandon that patch
and do the string manipulation on the stack.

ATM, earlycon is still broken if stdout-path options have been set.

2. Rob never got back to me on my query [1] to unify the OF_EARLYCON_DECLARE
macro with the EARLYCON_DECLARE macro so that all earlycon consoles
are named.

Regards,
Peter Hurley

[1] https://lkml.org/lkml/2015/3/6/571
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help