On Sat, 28 Mar 2015 13:01:24 -0400
, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
wrote:
> Hi Grant,
>
> On 03/27/2015 09:36 PM, Grant Likely wrote:
> > On Sun, 01 Mar 2015 17:23:11 -0500
> > , Peter Hurley <peter@xxxxxxxxxxxxxxxxxx>
> > wrote:
> >> Hi Kevin,
> >>
> >> On 11/24/2014 06:36 PM, Kevin Cernekee wrote:
> >>> 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@xxxxxxxxx>
> >>> ---
> >>> 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.
I don't seem to have that patch. Can you send it to me please?
I do have a thought though. Would it be better to teach
fdt_path_offset() to recognize the ':' delimiter? It's never a valid
character for a path.
The unittests are easy. "make check" builds and runs them. Adding a test
is as simple as editing tests/parent_offset.c. main() calls check_path()
several times to test calls to fdt_path_offset(). The tests can be added
directly to that file.
g.
