PowerMac11,2 i2c-bus@0 duplicate dev-tree workaround
PATCH 1/1 PowerMac11,2 i2c-bus@0 duplicate dev-tree workaround
On a PowerMac11,2, there are two i2c-bus@0 nodes of which only the first is correct. This patch makes the device tree unflattening code ignore the second one on those machines.
---
I'm not sure this is the right way to do it. Maybe we should have some 'dev-tree quirks fixer' that makes a third pass through the device tree after the allnodes chain has been set up, and fixes it up. On the other hand, as long as there aren't too many workarounds, this works fine.
The patch looks longer than it is because some code changed indentation level.
arch/powerpc/kernel/prom.c +37 -10
@@ -624,6 +624,11 @@ static void *__init unflatten_dt_alloc(ureturn res; } +/* PowerMac11,2 has a bug in the device tree where +* an i2c-bus@0 shows up twice. We ignore the second +* one since it is the bogus one */ +static __initdata int powermac_i2c_bus_0_workaround =0 ; + static unsigned long __init unflatten_dt_node(unsigned long mem, unsigned long *p, struct device_node *dad,
@@ -695,17 +700,31 @@ static unsigned long __init unflatten_dtmemcpy (p, pathp, l); }else memcpy (np->full_name, pathp, l); +DBG ("node %s found\n" , np->full_name); prev_pp = &np->properties; - **allnextpp = np; - *allnextpp = &np->allnext; -if (dad !=NULL ) { - np->parent = dad; -/* we temporarily use the next field as `last_child'*/ -if (dad->next ==0 ) - dad->child = np; -else - dad->next->sibling = np; - dad->next = np; +/* so if the workaround is in effect, and we have the right +* node found, we increase the workaround count (we use the +* same variable) and then set allnextpp to NULL on the second +* one around so the node isn't added to the allnodes list */ +if (powermac_i2c_bus_0_workaround && +strcmp (np->full_name, +"/ht@0,f2000000/pci@8/mac-io@7/i2c@18000/i2c-bus@0" ) ==0 ) { + powermac_i2c_bus_0_workaround++; +if (powermac_i2c_bus_0_workaround ==3 ) + allnextpp =NULL ; + } +if (allnextpp) { + **allnextpp = np; + *allnextpp = &np->allnext; +if (dad !=NULL ) { + np->parent = dad; +/* we temporarily use the next field as `last_child'*/ +if (dad->next ==0 ) + dad->child = np; +else + dad->next->sibling = np; + dad->next = np; + } }kref_init (&np->kref); }
@@ -745,6 +764,14 @@ static unsigned long __init unflatten_dt }if (strcmp (pname,"ibm,phandle" ) ==0 ) np->linux_phandle = *((u32 *)*p); +/* we check the root node's compatible property +* to see if we need to start the powermac i2c +* workaround */ +if ((pathp[0 ] =='\0' ) && + (strcmp (pname,"compatible" ) ==0 ) && + (strncmp ((char *)*p,"PowerMac11,2" , sz) ==0 )) { + powermac_i2c_bus_0_workaround =1 ; + } pp->name = pname; pp->length = sz; pp->value = (void *)*p;
Quoted 17 lines
arch/powerpc/kernel/prom.c +16
-0+/* so if the workaround is in effect, and we have the right +* node found, we increase the workaround count (we use the +* same variable) and then set allnextpp to NULL on the second +* one around so the node isn't added to the allnodes list */ +if (powermac_i2c_bus_0_workaround && +strcmp (np->full_name, +"/ht@0,f2000000/pci@8/mac-io@7/i2c@18000/i2c-bus@0" ) ==0 ) { + powermac_i2c_bus_0_workaround++; +if (powermac_i2c_bus_0_workaround ==3 ) + allnextpp =NULL ; + } +if (allnextpp) { + **allnextpp = np; + *allnextpp = &np->allnext; +if (dad !=NULL ) { + np->parent = dad;
arch/powerpc/kernel/prom.c static unsigned long __init unflatten_dt · line 730
Quoted 10 lines
arch/powerpc/kernel/prom.c +7
-0+/* we temporarily use the next field as `last_child'*/ +if (dad->next ==0 ) + dad->child = np; +else + dad->next->sibling = np; + dad->next = np; + } }kref_init (&np->kref); }
Hrm... you set allnextpp to NULL ... won't that prevent any further node from being enqueued in the global node list ?
In fact, I think the workaround should be in prom_init.c when flattening... easier to skip a node there, and that's also where you are reasonably sure of getting the nodes in the right order, not when unflattening.
Also, you don't even need to test for PowerMac11,2 .. .prom_init.c already has a machine type, so just test that it's a mac and has this node duplicated, and if yes, remove the dup. In fact, you could probably even run a bit of forth with "interpret" to do so before the tree is even walked though :)
Ben.
Quoted 2 lines · Benjamin Herrenschmidt
Hrm... you set allnextpp to NULL ... won't that prevent any further node from being enqueued in the global node list ?
No, it only prevents all *child* nodes, since the function recurses so once you go out allnextpp is valid again. I'm not setting *allnextpp :)
Quoted 4 lines
In fact, I think the workaround should be in
prom_init.cwhen flattening... easier to skip a node there, and that's also where you are reasonably sure of getting the nodes in the right order, not when unflattening.
Ok. I'll look at that code then.
Quoted 5 lines
Also, you don't even need to test for PowerMac11,2 ..
.prom_init.calready has a machine type, so just test that it's a mac and has this node duplicated, and if yes, remove the dup. In fact, you could probably even run a bit of forth with "interpret" to do so before the tree is even walked though :)
Heh. Well, if you tell me what you prefer I'll try to do it. Meanwhile I'll use this patch so the sound stuff etc. doesn't get too confusing (it keeps telling me about two onyx codecs found...)
johannes
Quoted 6 lines · Johannes Berg
On a PowerMac11,2, there are two i2c-bus@0 nodes of which only the first is correct. This patch makes the device tree unflattening code ignore the second one on those machines.
Signed-off-by Johannes Berg <[email protected]>
Quoted 8 lines
---
I'm not sure this is the right way to do it. Maybe we should have some 'dev-tree quirks fixer' that makes a third pass through the device tree after the allnodes chain has been set up, and fixes it up. On the other hand, as long as there aren't too many workarounds, this works fine.The patch looks longer than it is because some code changed indentation level.
I don't understand why we need this patch? If we care about getting the "right" node why not check for that where we care?
cheers
Signature · 8 lines
Michael Ellerman IBM OzLabs wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Quoted 2 lines · Michael Ellerman
I don't understand why we need this patch? If we care about getting the "right" node why not check for that where we care?
It's not just a single node but also contains sub-nodes. In one of them, one of those sub nodes is there but contains bogus info, and in the other both are there and contain the right info. Some things that work off these bus nodes are detected twice if they're there, and they're used in at least two places that I know of.
johannes
Quoted 10 lines · Johannes Berg
Quoted 2 lines · Michael Ellerman
I don't understand why we need this patch? If we care about getting the "right" node why not check for that where we care?
It's not just a single node but also contains sub-nodes. In one of them, one of those sub nodes is there but contains bogus info, and in the other both are there and contain the right info. Some things that work off these bus nodes are detected twice if they're there, and they're used in at least two places that I know of.
OK, that sounds a little complicated. I'm just worried about having lots workarounds in the unflattening code, the code's hard enough to read as it is. I think it's preferable to do the workarounds in the code that needs the workaround, that way they're isolated in certain parts of the code rather than all in the unflattening.
cheers
Signature · 17 lines
Michael Ellerman IBM OzLabs wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person -- Michael Ellerman IBM OzLabs wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Quoted 3 lines · Michael Ellerman
OK, that sounds a little complicated. I'm just worried about having lots workarounds in the unflattening code, the code's hard enough to read as it is.
Yeah that's a good argument.
Quoted 3 lines
I think it's preferable to do the workarounds in the code that needs the workaround, that way they're isolated in certain parts of the code rather than all in the unflattening.
I'd really prefer to have these nodes vanish from the complete node chain so we don't have the workarounds cluttering up the rather high levels with machine dependent code. I'll find a better place to put it.
johannes