On Sat, Jan 30, 2010 at 8:49 PM, Wolfgang Grandegger <[email protected]> wrote: > Hi Christian, >
Hi, > > Could you please truncate the lines to the usual 72 (or 80) characters > per line? > ack, just reconfigured nano! >> +static irqreturn_t mcp251x_can_ist(int irq, void *dev_id); >> +static void mcp251x_restart_work_handler(struct work_struct *ws); >> +static void mcp251x_tx_work_handler(struct work_struct *ws); > > Any chance to get rid of these forward declarations (by reordering them)? > ack, I was trying to minimize patch size so the reason for not moving code. I fixed it. >> + * This message is worrisome (because it points out >> + * something wrong with locking logic) if seen when >> + * there is no bus-off recovery going on. >> + */ > > Before it calls netif_carrier_on, it calls the drivers "do_set_mode" > callback. See: > > http://lxr.linux.no/#linux+v2.6.32/drivers/net/can/dev.c#L383 > > Is there a way to clear the TX logic in the "do_set_mode" callback? > ack, sorry saw this just now >> + dev_info(&net->dev, >> + "cannot allocate error skb\n"); > > dev_err? > ack >> if (priv->can.state == CAN_STATE_BUS_OFF) { >> mcp251x_clean(net); >> netif_wake_queue(net); >> - return; >> + goto restart_work_unlock; >> } > > } else { ? To get rid of the label. > ack. I realized that netif_wake_queue here is wrong too or at least meaningless: the carrier of the can network interface is declared down so we shouldn't reschedule the tx queue anyway. I took it out and retested without it. -- Christian Pellegrin, see http://www.evolware.org/chri/ "Real Programmers don't play tennis, or any other sport which requires you to change clothes. Mountain climbing is OK, and Real Programmers wear their climbing boots to work in case a mountain should suddenly spring up in the middle of the computer room." _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
