[U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture

[Lee Jones <lee.jones@linaro.org>]

> > Let's try to move this forward.
> 
> Actually you don't.  You insist on not changing anything on your side,
> and ask others to adapt to it.

Not at all.

This current implementation is unacceptable to you and your counter-
suggestion is unacceptable to me (and all other kernel engineers).

I'm seeking a new avenue. 

> > Okay, to summarise so far:
> > 
> > 1. Bootloader and kernel mechanisms should be the same
> > 
> > So putting bootloader tracepoints in the bootlog and the kernel's
> > in an internal data structure is not acceptable, as it would add
> > too much extra overhead to link them together and parse.
> 
> OK.  But this appears a new requirement - your original implementation
> did not bother about this, using ATAGs here and somethign else
> there...

No, they're the same in the current implementation:

  struct tag_boottime {
         struct boottime_entry entry[BOOTTIME_MAX];
         u32 idle;  /* in us */
         u32 total; /* in us */
         u8 num;
  };

ATAGs is mearly a way to pass the data structure through.

Once the data is passed to though, the kernel then just populates
the structure with bootloader and kernel tracepoints alike. No
differentiation is made between the two.

> > 2. The kernel bootlog is not the correct place for tracepoints
> > 
> > If we were to adhere to point No1 and bootloader & kernel
> > entries would be placed into the bootlog; no self respecting
> > kernel engineer/maintainer will allow 100's of spurious
> > tracepoint entries in the kernel log, regardless of log level.
> 
> I wonder about the self-assuredness you speak for all of them.  Has
> this been dicussed in full context before?

It's just not logical. 

Putting 100's of tracepoints in the kernel log is insane. 

> > Actually, putting it in DT has lots of advantages. 1) DT works
> > cross-architecture and cross-platform, so your architecture
> > independent box is inherently ticked 2) DT already carries
> > non-hardware specific information such as the kernel cmdline.
> > I don't see how adding <10 (but would more likely to be 2-3)
> > tracepoint entries would completely break convention. We can
> > either get the kernel driver to scrub the entries if you'd be
> > that offended by keeping them in.
> 
> Hm... in accordance to No. 1 above your kernel code will add new
> entries to the device tree while the kernel is booting?  So instead of
> "adding <10" it now will be adding "100's of spurious tracepoint
> entries" to the DT?
>
> Are you sure this is a good idea?  [Not considering if it's actually
> possible.]
> 
> But then, if you drop item 1, then what's wrong with "<10 (but would
> more likely to be 2-3)" additional log messages?

Granted, I can understand the points above.

If it's feasible to put them in the buffer, parse it, then scrub
them so they don't appear in dmesg output, I guess that could be
doable.

Ideally I'd like to keep it all as data, as it will save lots of
text parsing code in the kernel. Surely there must be a call for
passing data structures from the bootloader to the kernel. After
all, that's why ATAGs were brought about wasn't it?

-- 
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog