Re: [PATCH 2/3] fdtdump: Prettify output of properties

[Tom Rini <trini-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 4468 bytes --]

On Fri, Jun 16, 2017 at 12:01:35PM -0700, Frank Rowand wrote:
> On 06/16/17 08:40, Tom Rini wrote:
> > On Thu, Jun 15, 2017 at 06:17:20PM -0700, Frank Rowand wrote:
> >> Hi Tom,
> >>
> >> On 06/15/17 16:52, Tom Rini wrote:
> >>> On Wed, Jun 14, 2017 at 11:06:39PM +0800, David Gibson wrote:
> >>>> On Wed, Jun 14, 2017 at 05:53:49PM +0300, Pantelis Antoniou wrote:
> >>>>> Dumping files with large properties results in output with
> >>>>> arbitrary long lines.
> >>>>>
> >>>>> Original (manual line breaks inserted; it's a single long line):
> >>>>>
> >>>>> / {
> >>>>>     int = <0x00000001 0x00000024 0x00000004 0x00000000 \
> >>>>> 0x000502a4 0x000000df 0x00000003 0x13885783 0x13885783 \
> >>>>> 0x00000002 0x62797465 0x00000000 0x00000000 0x00000000 \
> >>>>> 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 \
> >>>>> 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000>;
> >>>>> };
> >>>>>
> >>>>> After prettification:
> >>>>>
> >>>>> / {
> >>>>>     int = <0x00000001 0x00000002 0x00000008 0x00000010 0x00000024 0x000000ab>,
> >>>>>           <0x00000001 0x00000017 0x00000004 0x00000038 0x00000007 0x00000009>,
> >>>>>           <0x00000000 0x00000068 0x00000214 0x0000b8d9 0x000502a4 0x00000001>,
> >>>>>           <0x00000004 0x0000002b 0x000000df 0x00000003 0x00000002 0x00000001>;
> >>>>> };
> >>>>>
> >>>>> There are two new options (-w/--width) and (-S/--shift).
> >>>>>
> >>>>> Width is the terminal width, shift is the amount of spaces each nest level
> >>>>> increases by.
> >>>>>
> >>>>> Width by default is set to 80, and shift to 4.
> >>>>
> >>>> Nack.
> >>>>
> >>>> fdtdump is supposed to be a trivial debug tool.   If you want to
> >>>> decompile dtbs "for real" use dtc -I dtb -O dts.
> >>>
> >>> There's been times, entirely unrelated to what Pantelis is doing, where
> >>> I've used fdtdump in production cases because I needed to whack a few
> >>> things around.  If it's just supposed to be a trivial debug tool, we've
> >>> likely moved well beyond the point where we need to keep trivial tools
> >>> around if they shouldn't be more widely used, IMHO.
> >>
> >> Let me paraphrase what I think that said:
> >>
> >>    If a trivial debug tool is used by a wide audience then we should get
> >>    rid of the tool.
> >>
> >> I suspect I misunderstood.  Can you clarify?
> > 
> > Sure.  Pantelis wants to improve a trivial debug tool to be slightly
> > more useful.  The maintainer says no, we shouldn't touch the tool, you
> > can use dtc -I dtb -O dts instead.  As that would also cover fdtdump
> > itself, it sounds like fdtdump is deprecated and should be removed, as
> > it's being used outside of the trivial debug use case.
> 
> fdtdump is useful for debugging and provides several features that aren't
> available in 'dtc -I dtb -O dts'.

Agreed.

> The maintainer wants to keep the debug tool simple.

Maintainers prerogative, yes.

> Another tool (dtc)
> already exists to do what the proposed patch would add.

I disagree, or at least don't see it in 'dtc -I dtb -O dts' as there
would need to be another argument for "linebreak the output and continue
to be valid".  I just rebuilt from master and dumped a dtb I had around
to confirm (and checked the help, too).

> Somehow Pantelis and you then jumped to the conclusion that fdtdump
> should be removed.  This is the part that I don't get.  It is a useful
> tool that has features that are not otherwise available.  Why would
> you get rid of it?

It's not Pantelis, it's my argument.  And for the record, I say it's
useful too.  I'm arguing that the logical end point of the maintainers
argument is that fdtdump shouldn't be used anywhere by anyone, it's a
trivial debug tool that no one should be shipping.  It's like enabling
various debug options in the kernel.  The right tool for most people of
"I need to read a dtb" is to use dtc -I dtb -O dts, and if you're
working on dtc, that's when maybe you need another tool at times, so you
can see the internal steps.

> > Or, can we talk about improving fdtdump as being a valid tool working
> > with dtb files when 'dtc -I dtb -O dts' is not desired?
> 
> That conflicts with the purpose (debug) and design goals (trivial)
> stated by the maintainer.

So what's the right tool for getting nice human readable output?  'dtc
-I dtb -O dts' will give you the arbitrarily long lines that this patch
fixes.

-- 
Tom

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]