[PATCH v4 2/3] dtc: Support character literals in cell lists

[Anton Staaf]

On Mon, Sep 19, 2011 at 8:34 PM, David Gibson
<david at gibson.dropbear.id.au> wrote:
> On Mon, Sep 19, 2011 at 07:54:09PM -0700, Anton Staaf wrote:
>> On Mon, Sep 19, 2011 at 5:59 PM, David Gibson
>> <david at gibson.dropbear.id.au> wrote:
>> > On Mon, Sep 19, 2011 at 09:45:34AM -0700, Anton Staaf wrote:
>> >> On Sun, Sep 18, 2011 at 7:00 PM, David Gibson
>> >> <david at gibson.dropbear.id.au> wrote:
>> >> > On Sat, Sep 17, 2011 at 11:49:21AM -0500, Jon Loeliger wrote:
>> >> >> > On Fri, Sep 09, 2011 at 12:16:30PM -0700, Anton Staaf wrote:
>> >> >> > > With this patch the following property assignment:
>> >> >> > >
>> >> >> > >     property = <0x12345678 'a' '\r' 100>;
>> >> >> > >
>> >> >> > > is equivalent to:
>> >> >> > >
>> >> >> > >     property = <0x12345678 0x00000061 0x0000000D 0x00000064>
>> >> >> > >
>> >> >> > > Signed-off-by: Anton Staaf <robotboy at chromium.org>
>> >> >> >
>> >> >> > Acked-by: David Gibson <david at gibson.dropbear.id.au>
>> >> >>
>> >> >> So, I *think* we want to wait until the question of size
>> >> >> is resolved some more, right?  Or, take this in any event
>> >> >> as "without a type indicator they are all 32-bit values"?
>> >> >
>> >> > No this patch is fine to take without changing the cell size
>> >> > semantics.  It's just that it becomes a lot more useful when we do get
>> >> > those.
>> >>
>> >> Yup, I'm working on a size patch by the way.  Any comments on my
>> >> previous post about it would be helpful.  But in the mean time I'm
>> >> going ahead with a solution where the current cell size is stored in
>> >> the "struct data" type references are not allowed in cell lists of
>> >> size other than 32 bits.
>> >
>> > Ah, sorry, I meant to give comments on that earlier but got
>> > sidetracked.
>> >
>> > Storing the cell size in struct data doesn't really work - a single
>> > property could be assembled from several cell lists of different
>> > sizes.  By the time the reference substitution happens, they will have
>> > been all merged into a single struct data.
>> >
>> > I think prohibiting cell references anywhere but 32-bit cell lists is
>> > the right approach, but we need to work out a way to do the check
>> > during the parse phase.
>>
>> Yes absolutely.  My intent with storing the current cell size in the
>> data struct was to use that to do the parse time rejection of
>> references in all but 32-bit cell lists.  That is the current cell
>> size would not be used past parsing, for exactly the reason you
>> mention.  My thought was that every creation of a cell list would set
>> the current cell size to 32 or the value defined by /size/ and the
>> closing '>' would set it back to 0.  The empty_data initializer would
>> set the current cell size to 0, even though it should never be used
>> outside of the <...> context.  Then when a reference or literal are to
>> be appended to the current data struct we can check for the value 32
>> if it's a reference, and otherwise use the current cell size to
>> validate (ensure it fits in the current cell size) and pad (add
>> leading zero's) the literal.  Padding won't actually be required if
>> the literal parsing routine always returns a 64-bit value.
>
> Oh, I see.  I guess that would work, but it's a really nasty misuse of
> a long-term data structure to store some short-term information.  No,
> I'm pretty sure we can rearrange the grammer to handle this more
> cleanly.
>
> Maybe something like this:
>
> cellarrayprefix = DT_SIZE DT_LITERAL '<'
>                | cellarrayprefix cellval
>                | cellarrayprefix DT_REF
>                | cellarrayprefix DT_LABEL
>
> The semantic type of cellarray prefix would need to contain both a
> struct data and a int for the cell size, which is a bit fiddly, but
> should be doable.  The DT_REF case can check the cell size part of $1
> and error if it's no 32-bit.

I like this more than my solution.  I've written it up along with some
tests.  I'm now working on the dts format doc.

Should I submit this assuming that the character literal support in
cell lists will be accepted shortly?  Or should I remove the use of
character literals from my test case?  It would be simpler for me to
leave the patches based on top of the character literal in cell list
patch that has been acked but not submitted.  I won't base them on top
of the character literals in bytestring patch since that is not going
in without more discussion.  Also, would you prefer that I squash the
test cases into the commit with the variable sized cell support and
documentation changes?

Thanks,
    Anton

> --
> 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
>