Re: [PATCH 1/3] dtc: Keep type information from DTS

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Tue, Jan 21, 2014 at 12:06:06PM +0000, Grant Likely wrote:
> On Mon, 16 Dec 2013 11:08:33 -0700, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> > On 12/13/2013 09:49 AM, Tomasz Figa wrote:
> > > Currently dtc loses information about data types when parsing DTS,
> > > because it flattens all the parsed property data into a flat stream of
> > > bytes. The only saved metadata is for references to other nodes inside
> > > cell arrays. This makes it impossible to do any checks on data types on
> > > livetree representation.
> > > 
> > > This patch makes dtc store type information inside data struct by using
> > > marker infrastructure. A new type of marker is introduced that holds
> > > type enum as its ref member. Such markers are then inserted wherever
> > > data of given type starts, so information about type of each property
> > > data section is preserved.
> > 
> > > diff --git a/dtc-parser.y b/dtc-parser.y
> > 
> > >  	| propdataprefix DT_REF
> > >  		{
> > > -			$$ = data_add_marker($1, REF_PATH, $2);
> > > +			struct data d;
> > > +			d = data_add_marker($1, TYPE, (char *)TYPE_STRING);
> > > +			$$ = data_add_marker(d, REF_PATH, $2);
> > >  		}
> > 
> > I guess here, the lexer does give us a string that's the target of the
> > reference, so this is correct. However, I wonder if semantically we
> > shouldn't call this a TYPE_REFERENCE, so we can distinguish between
> > references and regular strings?
> > 
> > > diff --git a/livetree.c b/livetree.c
> > 
> > > @@ -530,16 +530,24 @@ cell_t get_node_phandle(struct node *root, struct node *node)
> > >  	node->phandle = phandle;
> > >  
> > >  	if (!get_property(node, "linux,phandle")
> > > -	    && (phandle_format & PHANDLE_LEGACY))
> > > +	    && (phandle_format & PHANDLE_LEGACY)) {
> > > +		struct data d;
> > > +
> > > +		d = data_add_marker(empty_data, TYPE, (char *)TYPE_ARRAY_INT32);
> > 
> > Similarly here, can we encode as e.g. TYPE_PHANDLE or something like
> > that, so we keep the semantic information?
> 
> I think both those are reasonable and useful extensions also. On the
> whole I think the patch is the right thing to do. Feel free to add my
> acked by:
> 
> Acked-by: Grant Likely <grant.likely-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Still a NACK from me, I think this is fundamentally the wrong
approach.

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

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