[PATCH v2 3/6] Add fdtget utility to read property values from device tree
Simon Glass
sjg at chromium.org
Fri Sep 9 15:44:40 EST 2011
Hi David,
A bit rushed as late here but I wanted to respond before your weekend.
On Thu, Sep 8, 2011 at 9:49 PM, David Gibson
<david at gibson.dropbear.id.au> wrote:
> On Thu, Sep 08, 2011 at 05:47:34AM -0700, Simon Glass wrote:
>> Hi David,
>
> [snip]
>> >> Usage:
>> >> fdtget <options> <dt file> [<node> <property>]...
>> >> Options:
>> >> -t [<type>][<format>] Type of data
>> >> -h Print this help
>> >>
>> >> <type> One character (s=string, i=int, u=unsigned, b=byte)
>> >> <format> Optional format character (x=hex)
>> >
>> > It is not terribly clear how these are combined.
>>
>> A type character, then an optional format character. In fact the type
>> character is also optional at present since integer is assumed.
>>
>> Please can you suggest a suitable help message to convey this?
>
> Hrm, ok, so. I think part of the problem is that I'm finding what is
> covered by "type" and what is covered by "format" is only obvious once
> you've already understood what they mean. They're not fully
> independent either, since "format" makes no sense with type "s".
Yes I'm really just trying to explain a format similar to printf.
>
>> > Is there any way to make these more closely resemble printf() style
>> > format specifiers?
>>
>> The first patch series allowed printf specifiers but could segfault if
>> the user specified %s for something that was in fact not a string. I
>> have moved away from that as you suggested at the time.
>
> Yeah, I better understand your motivation for doing so now. But it's
> just not safe to do it that way.
>
>> I am not keen
>> to re-implement printf(), or manually decode a partial subset of
>> printf strings, to avoid that problem. So something simple like this
>> is what I have come up with.
>
> Hrm. The problem I have is that this sort-of-like printf() but not
> really like printf() may violate least surprise.
Well I mean that I don't want to reimplement it, but just using the
same syntax for a tiny subset would be fine.
>
>> We need to encode the data size in there also - byte or integer - so
>> it is not quite the same as what printf is trying to do. While
>> printf() has %c I don't think it is a good idea to use that, since we
>> are printing a byte as a number, not an ASCII character.
>
> Printing a byte value as a number would be %hhx or %hhd in printf.
Ick.
>
>> But it is fairly close to printf, in that you can use:
>>
>> -ts
>> -tx
>> -ti
>> -tu
>> -tix
>> -tbx
>>
>> We could combine -tbx into a single letter somehow but I'm not sure I
>> like that idea.
>>
>> I don't see any point in requiring a % since then users would expect
>> to be able to provide an arbitrary printf() string (see first patch
>> set).
>>
>> So, suggestions please.
>
> So, I think the first thing is I think you need to treat the "format"
> as the primary parameter, with the size (roughly what you're calling
> "type" at the moment) as a subordinate modifier to that. The trouble
> with making format subordinate to type, or supposedly independent
> parameters is that what sizes or "types" are acceptable depends on the
> format. A string format always accepts variable length input, and the
> integer formats always require a specified fixed length. Future
> extensions could conceivably have other constraints.
OK sounds good.
>
> With that basic principle, I basically see two reasonable ways to go.
>
> 1) Use a subset of printf() specifiers. Despite your balking at that,
> I don't really think it would be any harder than what you have now.
> Last character is always the non-optional "format" specifier, anything
> preceding it is modifiers. So for the integer formats "d", "x" and
> "o", valid modifiers are size, either "hh" (1 byte), "h" (2 byte), "l"
> (4 byte, default) or "ll" (8 byte). For the string format "s", no
> modifiers are valid.
OK, I like this option more. Particularly if you think I might get
away with also supporting 'b' as a shortcut for 'hh'.
>
> 2) Treat the size as a different parameter. So the -f option gives a
> single format char (or instead use -s, -x, -d, .. options). Then have
> a -s option for size (-s1 ... -s8) which is valid only with the
> integer format options.
>
More like what I had - have moved away from that and feel more
comfortable in my new space.
>
> Either way does leave a couple of unanswered questions:
>
> A) What to do if the format doesn't consume the whole parameter. I
> see two options:
> A1) Just print as much as the format says, then stop
> A2) Keep repeating the same format until the property is
> consumed (note that this makes sense for "s" as well, and would be an
> obvious way of handling the pretty common list-of-strings properties).
Well I think you have to keep printing. This is not a true format
string, but just a data type for each element. Again we have moved
away from being able to say -f "The %d item is %d and has a flag word
of %d".
So no need to use -tddd if there are three cells - just -td is enough.
>
> B) What to do when the property just doesn't fit into that format -
> either it's length is not a multiple of the fixed integer size, or
> it's not NUL-terminated for "s". Obviously some kind of error is in
> order, but in case A2 above, do we *just* error, or print what we can
> before giving up.
Prefer to error since any output might be confusing. But I will take a
look to make sure that is easy to do.
Thanks very much for your helpful comments. I will work on a new patch set.
Regards,
Simon
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>
More information about the devicetree-discuss
mailing list