[coreboot] dt compiler patch
ron minnich
rminnich at gmail.com
Tue Nov 25 17:05:53 CET 2008
On Tue, Nov 25, 2008 at 6:28 AM, Myles Watson <mylesgw at gmail.com> wrote:
>
>
>> -----Original Message-----
>> From: coreboot-bounces at coreboot.org [mailto:coreboot-bounces at coreboot.org]
>> On Behalf Of ron minnich
>> Sent: Monday, November 24, 2008 11:06 AM
>> To: Coreboot
>> Subject: [coreboot] dt compiler patch
>>
>> This has been tested and builds a working boot-to-linux coreboot on
>> the dbe62 (old style dts) and builds a serengeti target (new dts, but
>> that new dts not submitted yet).
>>
>> ron
>
> Here's my first pass.
>
> Thanks,
> Myles
>
>> I took the opportunity to do some cleanup.
>>
>> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
>> Index: util/dtc/flattree.c
>> ===================================================================
>> --- util/dtc/flattree.c (revision 1046)
>> +++ util/dtc/flattree.c (working copy)
>> @@ -443,10 +449,7 @@
>> {
>> struct data d = p->val;
>> FILE *f = e;
>> - int i;
>> char *cleanname;
>> - int vallen = d.len > 4 ? 4 : d.len;
>> -
>
> Lets take out the commented code that uses these variables at the same time.
done
>
>> /* nothing to do? */
>> if (d.len == 0)
>> return;
>> @@ -865,7 +875,7 @@
>>
>> if (tree->config){
>> int i;
>> -// treename = clean(tree->label, 0);
>> +// treename = clean(tree->dtslabel, 0);
>
> Maybe just remove this line.
done
>
>> treename = toname(tree->config->label, "_config");
>> for(i = 0; i < emitted_names_count; i++)
>> if (!strcmp(treename, emitted_names[i]))
>> Index: util/dtc/livetree.c
>> ===================================================================
>> --- util/dtc/livetree.c (revision 1046)
>> +++ util/dtc/livetree.c (working copy)
>> + /* match */
>> + /* the chip properties are in c->config list
>> + * the dts properties are in the m->proplist.
>> + * So copy the mainboard properties to the chip
> proplist.
>> + */
>> + c->proplist = m->proplist;
>
> This is a replacement, shouldn't it be an append?
no, because at this point, c->proplist is empty. c->configlist has the
properties.
>
>> + c->children = m->children;
>
> Again, this is a replacement, shouldn't it be an append?
ah, there is a question. Yes, it should be. Fixed.
Let's see what it breaks for you :-)
>
>> + /* since we matched this node we have to delete it.
> */
>
> The delete function says you can't have children, so the children link needs
> to be NULL here.
Fixed.
>> +/* we should only ever delete simple nodes: nodes with no kids. */
>> +void del_node(struct node *node)
>> +{
>> + struct node *n;
>> + assert(! node->children);
>> + if (first_node == node) {
>> + first_node = node->next;
>> + free(node);
>> + return;
>> + }
>> + for_all_nodes(n) {
>> + if (n->next != node)
>> + continue;
>> + n->next = node->next;
>> + n->next_sibling = node->next_sibling;
>
> Will this always be true? It seems like you need to go through again to do
> the sibling links right.
I don't think so because, at this point, n is prev(node to be deleted).
so I am setting prev(node)->next and next_sibling to node->next etc.
Am I missing something?
>
> Do we guarantee that you will never be appending to a NULL list (first ==
> NULL), I didn't see that check.
Here's my thinking on this. The guarantee is that there is always a
root node -- we don't ever remove that.
New diff attached, with issue from your next email managed as well.
Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dts.diff
Type: application/octet-stream
Size: 18030 bytes
Desc: not available
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081125/bd860be4/attachment.obj>
More information about the coreboot
mailing list