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:
signature.asc
Description: Digital signature
