A usefdt boot-mode problem: openfirmware->fdt translation use vs. some existing powerpc64 or powerpc FreeBSD code, interrupt-parent examples shown

[Mark Millard]

[I added some debug output that shows odd mixes of openfirmware
vs. fdt node ids. This notes an example of what it shows.]

On 2019-Apr-13, at 19:44, Mark Millard <marklmi at yahoo.com> wrote:

> I'm going to use interrupt-parent as an example of the type of issue.
> 
> In the below ofwdump -ap diff extractions:
> 
> - is from without usefdt mode
> + is from with usefdt mode
> 
> -      Node 0xff963f60: interrupt-controller
> +      Node 0x2767c: interrupt-controller
> +        phandle:
> +          ff 96 3f 60 
> 
> Note that the Node id changes but a phandle is added hiolding the
> Node id that openfirmware originally had.
> 
> Picking an example where interrupt-parent is in use:
> 
> 
> -        Node 0xff964428: extint-gpio1
> +        Node 0x27800: extint-gpio1
> +          phandle:
> +            ff 96 44 28 
> . . .
>           interrupt-parent:
>             ff 96 3f 60 
> . . .
> 
> (openfirmware and fdt are no different for the value
> contained in the interrupt-parent property: it is the
> openfirmware node id, not the FDT one.)
> 
> So, for fdt use, comparison to the phandle property value is
> is how to find/match this interrupt-controller via a
> interrupt-parent. Direct use of 0xff963f60 as a node id
> is inappropriate for usefdt mode.
> 
> The above is very general and I did not try to match the
> specific nodes to ones the code below would be using, beyond
> initially sticking to interrupt-parent types of references.
> 
> I find there is example code around that makes no prevision
> for finding the interrupt-parent via a phandle comparison.
> 
> An example is in unin_chip_add_intr :
> 
>        if (OF_getprop(devnode, "interrupt-parent", &iparent, sizeof(iparent))
>            <= 0)
>                panic("Interrupt but no interrupt parent!\n");
> 
>        if (OF_searchprop(iparent, "#interrupt-cells", &icells, sizeof(icells))
>            <= 0)
>                icells = 1;
> 
> For usefdt mode iparent's value is an openfirmware node id, not a fdt one.
> It is the phandle property value that would match.
> 
> Another is in unin_chip_attach:
> 
>                        if (OF_getprop(child, "interrupt-parent", &iparent,
>                                       sizeof(iparent)) <= 0) {
> . . .
>                        }
>                        /* Add an interrupt number 0 to the parent. */
>                        irq = MAP_IRQ(iparent, 0);
> 
> (It appears MAP_IRQ uses powerpc_get_irq and I do not see a stage
> that can involve the phandle property for matching.)
> 
> Another may be macgpio_attach:
> 
>                        OF_searchencprop(child, "interrupt-parent", &iparent,
>                            sizeof(iparent));
>                        resource_list_add(&dinfo->mdi_resources, SYS_RES_IRQ,
>                            0, MAP_IRQ(iparent, irq[0]),
>                            MAP_IRQ(iparent, irq[0]), 1);
> 
> Other's besides interrupt-parent . . .
> 
> There are some gpio-parent properties but I did not find code using
> them. (I may have just missed such.) I quit looking for code use
> but did find more types of references in the fdt itself . . .
> 
> l2-cache in, say, a PowerPC,G4 for usefdt mode is another type of
> example.
> 
> platform-enableclock and platform-disableclock seem to be be
> examples (references back to uni-n in what I looked at, but
> having the openfirmwire node id, not the fdt one).
> 
> I may not have found all the example types of "needs phandle property
> value comparisons instead of direct node id use for usefdt mode".
> 
> This is all from a 2-socket/1-core-each G4 PowerMac3,6 example.
> 
> [Even any G5 example would be via 32-bit pweorpc FreeBSD. This is
> because (for a long time now) use of ofwdump -ap crashes powerpc64
> PowerMac's. But I've not gone through a G5 example (yet?).]

In powerpc_register_pic I added the printf shown below:

powerpc_register_pic(device_t dev, uint32_t node, u_int irqs, u_int ipis,
    u_int atpic)
. . .
        for (idx = 0; idx < npics; idx++) {
                p = &piclist[idx];
                printf("idx,npics: p->node, node, p->dev, dev: %u,%u: %x, %x, %x, %x\n",idx,npics,p->node,node,p->dev,dev);
                if (p->node != node)
                        continue;
                if (node != 0 || p->dev == dev)
                        break;
        }
        p = &piclist[idx];
        p->dev = dev;
        p->node = node;
        p->irqs = irqs;
        p->ipis = ipis;
        if (idx == npics) {
#ifdef DEV_ISA
                p->base = (atpic) ? 0 : nirqs;
#else
                p->base = nirqs;
#endif
                irq = p->base + irqs + ipis;
                nirqs = MAX(nirqs, irq);
                npics++;
        }


It shows for usefdt mode booting of a 2-socket/1-core-each G5 PowerMac11,2:

idx,npics: p->node, node, p->dev, dev: 0,1: ff981488, 4e88, 0, 1e95600

So piclist[0].node has a value for/from openfirmware instead of fdt.
But the caller supplied a fdt node value, not an openfirmware one.

The loop will not match the two, even if the fdt node has a phandle
property that lists the 0xff981488 value. The later code would then
add a separate entry in piclist with the fdt node id and increment
npics.

Looks to me like the intent for usefdt mode was likely to not have any
openfirmware id value in any piclist[?].node field. (True?)

This sort of thing appears to be involved in why usefdt mode crashes
on the example 2-socket/1-core-each G5 PowerMac7,2 .

The code directly putting the openfirmware value (that it was
given) into the piclist is:

powerpc_get_irq(uint32_t node, u_int pin)
. . .
        piclist[idx].dev = NULL;
        piclist[idx].node = node;
        piclist[idx].irqs = 124;
        piclist[idx].ipis = 4;
        piclist[idx].base = nirqs;
        nirqs += (1 << 25);
        npics++;
. . .

This goes back to MAP_IRQ use with openfirmware node id values.


===
Mark Millard
marklmi at yahoo.com
( dsl-only.net went
away in early 2018-Mar)