Oliver Neukum [mailto:oneukum@xxxxxxx] > Sent: Friday, April 26, 2013 7:57 PM > To: Hayeswang > Cc: gregkh@xxxxxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; > linux-kernel@xxxxxxxxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; nic_swsd > Subject: Re: [PATCH net-next] net/usb: new driver for RTL8152 > [...] > > +static inline void set_ethernet_addr(struct r8152 *tp) > > +{ > > + struct net_device *dev = tp->netdev; > > + u8 node_id[8] = {0}; > > + > > + if (pla_ocp_read(tp, PLA_IDR, sizeof(node_id), node_id) < 0) > > DMA coherency rules. No buffers on the stack. Excuse me. I don't understand what you mean. I reference the rtl8150.c and it uses the same way. Besides, when I check the other drivers, I find the data pointer of the parameter of the usb_control_msg() may be a pointer of a local variable from the other functions. [...] > > +static void rtl_work_func_t(struct work_struct *work) > > +{ > > + struct r8152 *tp = container_of(work, struct r8152, > schedule.work); > > + > > + if (!netif_running(tp->netdev)) > > + goto out1; > > + > > + if (test_bit(RTL8152_UNPLUG, &tp->flags)) > > + goto out1; > > + > > + set_carrier(tp); > > + > > + if (test_bit(RTL8152_SET_RX_MODE, &tp->flags)) > > + _rtl8152_set_rx_mode(tp->netdev); > > + > > + schedule_delayed_work(&tp->schedule, HZ); > > Why does this need to run permanently? It is used to periodically check the speed, link status, and any other tasks which need to be finished. [...] > > +static int rtl8152_close(struct net_device *netdev) > > +{ > > + struct r8152 *tp = netdev_priv(netdev); > > + int res = 0; > > + > > + cancel_delayed_work_sync(&tp->schedule); > > Looks like a race. What makes sure the work isn't rescheduled? netif_running would be checked. Best Regards, Hayes -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
