gianfar: dont conditionally alloc Rx/Err irq structs
authorPaul Gortmaker <paul.gortmaker@windriver.com>
Mon, 4 Feb 2013 09:49:42 +0000 (09:49 +0000)
committerDavid S. Miller <davem@davemloft.net>
Tue, 5 Feb 2013 02:08:01 +0000 (21:08 -0500)
Commit ee873fda3bec7c668407b837fc5519eb961fcd37

    "gianfar: Pack struct gfar_priv_grp into three cachelines"

causes the following null dereference at driver init on sbc8548:

   libphy: Freescale PowerQUICC MII Bus: probed
   Unable to handle kernel paging request for data at address 0x00000000
   Faulting instruction address: 0xc01d6a38
   Oops: Kernel access of bad area, sig: 11 [#1]
   [...]
   NIP [c01d6a38] gfar_parse_group+0x228/0x280
   LR [c01d6a34] gfar_parse_group+0x224/0x280
   Call Trace:
   [ef82dd60] [c01d6a34] gfar_parse_group+0x224/0x280 (unreliable)
   [ef82dd90] [c01d73a4] gfar_probe+0x284/0xfe0

The reason is that the commit also changed the allocation of the
Rx and error handling irq structs to be skipped for !MQ_MG_MODE.
In the !MQ_MG_MODE case, only the Tx irq struct is allocated.

Digging further, we see that MQ_MG_MODE is set only if we find
the OF compatible string "fsl,etsec2".

A quick grep in the dts directory shows lots of boards that support
Rx/Tx/Err, but without this specific compat string.  And hence they
go after the unallocated Rx/Error structs and cause the above oops.

Hence such a change can not be deployed until all the dts files
are updated and sufficiently deployed.  Further, the optimization
is of limited value, since the kmalloc'd struct in question has only
a single unsigned int, and an (IFNAMSIZ + 6) sized string.

Note that no changes to the freeing code are needed here, as it
already did an unconditional free of Rx/Tx/Error gfar_irqinfo.

Cc: Claudiu Manoil <claudiu.manoil@freescale.com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/ethernet/freescale/gianfar.c

index 19c54a0acb56938ad7ade3ac49e6bb769054b113..75734bfed10f345f9885ce449c81c2f4d90365d0 100644 (file)
@@ -580,19 +580,11 @@ static int gfar_parse_group(struct device_node *np,
        u32 *queue_mask;
        int i;
 
-       if (priv->mode == MQ_MG_MODE) {
-               for (i = 0; i < GFAR_NUM_IRQS; i++) {
-                       grp->irqinfo[i] = kzalloc(sizeof(struct gfar_irqinfo),
-                                                 GFP_KERNEL);
-                       if (!grp->irqinfo[i])
-                               return -ENOMEM;
-               }
-       } else {
-               grp->irqinfo[GFAR_TX] = kzalloc(sizeof(struct gfar_irqinfo),
-                                               GFP_KERNEL);
-               if (!grp->irqinfo[GFAR_TX])
+       for (i = 0; i < GFAR_NUM_IRQS; i++) {
+               grp->irqinfo[i] = kzalloc(sizeof(struct gfar_irqinfo),
+                                         GFP_KERNEL);
+               if (!grp->irqinfo[i])
                        return -ENOMEM;
-               grp->irqinfo[GFAR_RX] = grp->irqinfo[GFAR_ER] = NULL;
        }
 
        grp->regs = of_iomap(np, 0);