Re: Kernel 2.6.39-2-amiga crashes with cirrusfb

[Geert Uytterhoeven <geert@linux-m68k.org>]

2011/8/5 Geert Uytterhoeven <geert@linux-m68k.org>:
> 2011/7/25 Ingo Jürgensmann <ij@2011.bluespice.org>:
>> [    1.020000] Unable to handle kernel NULL pointer dereference at virtual address 000000f8
>> [    1.040000] Oops: 00000000
>> [    1.050000] Modules linked in:
>> [    1.060000] PC: [<00260f04>] cirrusfb_zorro_register+0x1ac/0x430
>> [    1.070000] SR: 2000  SP: 07c1fd64  a2: 07c199b0
>> [    1.080000] d0: 00000005    d1: 00000000    d2: 00000003    d3: 00200000
>> [    1.090000] d4: 00200000    d5: 00200000    a0: 00197390    a1: 00000000
>
> static int __devinit cirrusfb_zorro_register(struct zorro_dev *z,
>                                             const struct zorro_device_id *ent)
> {
>        ...
>        struct zorro_dev *z2 = NULL;
>        ...
>        btype = ent->driver_data;
>        if (cirrusfb_zorro_table2[btype].id2)
>                z2 = zorro_find_device(cirrusfb_zorro_table2[btype].id2, NULL);
>
> The entry for Picasso II in cirrusfb_zorro_table2[] has:
>
>        [BT_PICASSO] = {
>                .id2    = ZORRO_PROD_VILLAGE_TRONIC_PICASSO_II_II_PLUS_REG,
>                .size   = 0x200000
>        },
>
> id2 is the Zorro ID for the second Zorro device for this physical board.
>
> Later:
>
>        if (btype == BT_PICASSO4) {
>                ...
>        } else {
>                dev_info(info->device, " REG at $%lx\n",
>                         (unsigned long) z2->resource.start);
>
> Bang! z2 seems to be zero!
>
> Hence the call to zorro_find_device() above must have returned NULL, i.e. it
> couldn't find the second Zorro device. Why not?
>
> I guess this is due to the mixing of new-style struct zorro_driver and old-style
> zorro_find() in cirrusfb.
>
> static int __init amiga_zorro_probe(struct platform_device *pdev)
> {
>        ...
>        for (i = 0; i < zorro_num_autocon; i++) {
>                z = &zorro_autocon[i];
>                z->id = (z->rom.er_Manufacturer<<16) | (z->rom.er_Product<<8);
>                if (z->id == ZORRO_PROD_GVP_EPC_BASE) {
>                        /* GVP quirk */
>                        unsigned long magic = zorro_resource_start(z)+0x8000;
>                        z->id |= *(u16 *)ZTWO_VADDR(magic) & GVP_PRODMASK;
>                }
>                ...
>                error = device_register(&z->dev);
>
> This causes device i to be registered and started.
> However, at this point, zorro_autocon[j].id hasn't been initialized
> yet for any j > i,
> so zorro_find_device() cannot find any devices beyond i.
>
> Does this (whitespace-challenged --- thank you gmail) patch help? It
> delays registration
> of devices until all devices has been set up.
>
> diff --git a/drivers/zorro/zorro.c b/drivers/zorro/zorro.c
> index e0c2807..068c3c1 100644
> --- a/drivers/zorro/zorro.c
> +++ b/drivers/zorro/zorro.c
> @@ -172,6 +172,10 @@ static int __init amiga_zorro_probe(struct
> platform_device *pdev)
>                dev_set_name(&z->dev, "%02x", i);
>                z->dev.parent = &bus->dev;
>                z->dev.bus = &zorro_bus_type;
> +       }
> +
> +       for (i = 0; i < zorro_num_autocon; i++) {
> +               z = &zorro_autocon[i];
>                error = device_register(&z->dev);
>                if (error) {
>                        dev_err(&bus->dev, "Error registering device %s\n",

I set up a fake Zorro bus with a fake Picasso II under User Mode Linux, and
could confirm the above theory, and that the patch fixed this.

Note that cirrusfb needs some hardening anyway, as there can be other
reasons id2
is not found (e.g. ZORRO_NUM_AUTO too low, or not enough Zorro II space
for all your devices).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds