[coreboot] add array parsing to dts
Jordan Crouse
jordan.crouse at amd.com
Thu Apr 17 18:01:38 CEST 2008
On 12/04/08 22:11 -0400, Ward Vandewege wrote:
> On Thu, Apr 10, 2008 at 03:51:15AM +0200, Carl-Daniel Hailfinger wrote:
> > On 10.04.2008 02:51, Ward Vandewege wrote:
> > > With a lot of help from Stepan at the summit, this patch adds array parsing
> > > support to the dts, specifically for the 'unwanted_vpci' field.
> > >
> > > The code is a bit of a hack - it matches against the field name to change
> > > interpretation to array mode. Ideally we'd add some more type information
> > > after the parsing of the dts, and use that to write out statictree.c.
> > >
> > > The dts parsing/generation code needs some love, so I'm not too worried about
> > > the hackishness of this patch at this point. But if someone wants to improve
> > > it, be my guest :)
> > >
> >
> > It's great to have working code to demonstrate an idea.
> >
> > However, I strongly disagree with the hardcoding of the string
> > "unwanted_vpci" in several places in the code. Can't you use the
> > "must_be_cells" designation in the prop_checker_table in
> > util/dtc/livetree.c to handle this transparently?
>
> Actually, the proper way to do it is to look at what we're parsing in the dts
> files, store the type of the field in the 'data' struct, and write out the
> proper statictree.s and statictree.h files based on that.
>
> The attached patch does just that.
>
> Note that for 'byte' properties, it just printes 'UNIMPLEMENTED, FIXME',
> which will break the statictree output files. We don't use 'byte' properties
> right now, and I'm not sure what to write out. Suggestions?
>
> Thanks,
> Ward.
>
> --
> Ward Vandewege <ward at fsf.org>
> Free Software Foundation - Senior System Administrator
>
> Add generic array support to the coreboot dts output code.
>
> This is necessary for the 'unwanted_vpci' field on geode-based boards.
>
> Signed-off-by: Ward Vandewege <ward at gnu.org>
If this is the same patch as
http://www.coreboot.org/pipermail/coreboot/2008-April/033385.html
Then Acked-by: Jordan Crouse <jordan.crouse at amd.com>
If not, then I ack that patch. :)
> Index: flattree.c
> ===================================================================
> --- util/dtc/flattree.c (revision 656)
> +++ util/dtc/flattree.c (working copy)
> @@ -452,9 +452,24 @@
> return;
>
> cleanname = clean(p->name, 1);
> - fprintf(f, "\t.%s = ", cleanname);
> + if (d.type == 'S') {
> + // Standard property (scalar)
> + fprintf(f, "\t.%s = ", cleanname);
> + fprintf(f, "0x%lx,\n", strtoul((char *)d.val, 0, 0));
> + } else if (d.type == 'C') {
> + // 'Cell' property (array of 4-byte elements)
> + fprintf(f, "\t.%s = {\n", cleanname, d.len/4);
> + int i;
> + for (i = 0; (i < d.len) && (0 != *(u32 *)(d.val+i)); i = i+4) {
> + fprintf(f, "\t\t[%d] = 0x%08X,\n",i/4,*(u32 *)(d.val+i));
> + }
> + fprintf(f, "\t\t[%d] = 0x00000000,\n",i/4); // Make sure to end our array with a zero element
> + fprintf(f, "\t},\n");
> + } else if (d.type == 'B') {
> + fprintf(f, "\tUNIMPLEMENTED: FIXME\n");
> + }
> free(cleanname);
> - fprintf(f, "0x%lx,\n", strtoul((char *)d.val, 0, 0));
> +
> #if 0
> /* sorry, but right now, u32 is all you get */
> fprintf(f, "0");
> @@ -785,7 +800,16 @@
> if (streq(prop->name, "device_operations")) /* this is special */
> continue;
> cleanname = clean(prop->name, 0);
> - fprintf(f, "\tu32 %s;\n", cleanname);
> + if (prop->val.type == 'S') {
> + // Standard property, scalar
> + fprintf(f, "\tu32 %s;\n", cleanname);
> + } else if (prop->val.type == 'C') {
> + // 'Cell' property (array of 4-byte elements)
> + fprintf(f, "\tu32 %s[];\n", cleanname);
> + } else if (prop->val.type == 'B') {
> + // Byte property
> + fprintf(f, "\tUNIMPLEMENTED: FIXME\n");
> + }
> free(cleanname);
>
> }
> Index: data.c
> ===================================================================
> --- util/dtc/data.c (revision 656)
> +++ util/dtc/data.c (working copy)
> @@ -64,6 +64,7 @@
> nd.asize = newsize;
> nd.val = xrealloc(d.val, newsize);
> nd.len = d.len;
> + nd.type = d.type;
> nd.refs = d.refs;
>
> assert(nd.asize >= (d.len + xlen));
> @@ -199,7 +200,11 @@
>
> struct data data_append_cell(struct data d, cell_t word)
> {
> - cell_t beword = cpu_to_be32(word);
> + // Don't do system/network order byte translation. We don't do it for scalars either.
> + //cell_t beword = cpu_to_be32(word);
> + cell_t beword = word;
> + // Mark this property as being of the 'cell' type
> + d.type = 'C';
>
> return data_append_data(d, &beword, sizeof(beword));
> }
> @@ -223,6 +228,8 @@
>
> struct data data_append_byte(struct data d, uint8_t byte)
> {
> + // Mark this property as being of the 'byte' type
> + d.type = 'B';
> return data_append_data(d, &byte, 1);
> }
>
> Index: livetree.c
> ===================================================================
> --- util/dtc/livetree.c (revision 656)
> +++ util/dtc/livetree.c (working copy)
> @@ -32,6 +32,9 @@
>
> new->name = name;
> new->val = val;
> + if (new->val.type == NULL) {
> + new->val.type = 'S'; // Default to 'scalar' type; if this is a cell or byte value, type will already be set
> + }
>
> new->next = NULL;
>
> @@ -301,7 +304,9 @@
> } prop_checker_table[] = {
> {"name", must_be_string},
> {"name", name_prop_check},
> -/* we don't care about these things now -- we think */
> + /* unwanted_vpci must be a cells field (i.e. an array) */
> + {"unwanted_vpci", must_be_cells},
> + /* we don't care about these things now -- we think */
> {"linux,phandle", must_be_one_cell},
> {"#address-cells", must_be_one_cell},
> {"#size-cells", must_be_one_cell},
> Index: dtc.h
> ===================================================================
> --- util/dtc/dtc.h (revision 656)
> +++ util/dtc/dtc.h (working copy)
> @@ -105,6 +105,7 @@
> struct data {
> int len;
> unsigned char *val;
> + unsigned char type;
> int asize;
> struct fixup *refs;
> };
> --
> coreboot mailing list
> coreboot at coreboot.org
> http://www.coreboot.org/mailman/listinfo/coreboot
--
Jordan Crouse
Systems Software Development Engineer
Advanced Micro Devices, Inc.
More information about the coreboot
mailing list