-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Wolfgang Grandegger wrote: > This patch makes the private functions alloc_can_skb() and > alloc_can_err_skb() of the at91_can driver public and adapts all > drivers to use these. While making the patch I realized, that > the skb's are *not* setup consistently. The skb's are now setup > as shown: > > skb->dev = dev;
nitpick: we use "netdev_alloc_skb()", so "skb->dev = dev" isn't needed any more. The code is allright, just the commit message is misleading. > skb->protocol = __constant_htons(ETH_P_CAN); > skb->ip_summed = CHECKSUM_UNNECESSARY; > > Some drivers or library code used: > > skb->protocol = htons(ETH_P_CAN); or > skb->pkt_type = PACKET_BROADCAST; > > or did not set CHECKSUM_UNNECESSARY. > > Please check and comment. > > Marc, feel free to add your signed-off-by here. nitpick: as you did the patch, I can rather add my Acked-by: under your S-o-b. > Signed-off-by: Wolfgang Grandegger <[email protected]> > --- > kernel/2.6/drivers/net/can/at91_can.c | 32 ------------------ > kernel/2.6/drivers/net/can/cc770/cc770.c | 13 +------ > kernel/2.6/drivers/net/can/dev.c | 38 > ++++++++++++++++++++-- > kernel/2.6/drivers/net/can/esd_pci331.c | 14 +------- > kernel/2.6/drivers/net/can/mcp251x.c | 20 +---------- > kernel/2.6/drivers/net/can/mscan/mscan.c | 6 --- > kernel/2.6/drivers/net/can/sja1000/sja1000.c | 12 +----- > kernel/2.6/drivers/net/can/softing/softing_main.c | 8 +--- > kernel/2.6/drivers/net/can/usb/ems_usb.c | 16 +-------- > kernel/2.6/include/socketcan/can/dev.h | 4 ++ > 10 files changed, 54 insertions(+), 109 deletions(-) > > Index: trunk/kernel/2.6/drivers/net/can/dev.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/dev.c > +++ trunk/kernel/2.6/drivers/net/can/dev.c > @@ -318,8 +318,7 @@ void can_put_echo_skb(struct sk_buff *sk > skb->sk = srcsk; > > /* make settings for echo to reduce code in irq context */ > - skb->protocol = htons(ETH_P_CAN); > - skb->pkt_type = PACKET_BROADCAST; is it save to remove this flag? > + skb->protocol = __constant_htons(ETH_P_CAN); > skb->ip_summed = CHECKSUM_UNNECESSARY; > skb->dev = dev; > > @@ -403,7 +402,8 @@ void can_restart(unsigned long data) > goto restart; > } > skb->dev = dev; > - skb->protocol = htons(ETH_P_CAN); > + skb->protocol = __constant_htons(ETH_P_CAN); > + skb->ip_summed = CHECKSUM_UNNECESSARY; > cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > memset(cf, 0, sizeof(struct can_frame)); > cf->can_id = CAN_ERR_FLAG | CAN_ERR_RESTARTED; > @@ -496,6 +496,38 @@ static void can_setup(struct net_device > #endif > } > > +struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf) > +{ > + struct sk_buff *skb; > + > + skb = netdev_alloc_skb(dev, sizeof(struct can_frame)); > + if (unlikely(!skb)) > + return NULL; > + > + skb->protocol = __constant_htons(ETH_P_CAN); > + skb->ip_summed = CHECKSUM_UNNECESSARY; > + *cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > + > + return skb; > +} > +EXPORT_SYMBOL_GPL(alloc_can_skb); > +struct sk_buff *alloc_can_err_skb(struct net_device *dev, struct can_frame > **cf) > +{ > + struct sk_buff *skb; > + > + skb = alloc_can_skb(dev, cf); > + if (unlikely(!skb)) > + return NULL; > + > + memset(*cf, 0, sizeof(struct can_frame)); > + (*cf)->can_id = CAN_ERR_FLAG; > + (*cf)->can_dlc = CAN_ERR_DLC; > + > + return skb; > +} > +EXPORT_SYMBOL_GPL(alloc_can_err_skb); > + > /* > * Allocate and setup space for the CAN network device > */ > Index: trunk/kernel/2.6/drivers/net/can/at91_can.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/at91_can.c > +++ trunk/kernel/2.6/drivers/net/can/at91_can.c > @@ -221,38 +221,6 @@ static inline void set_mb_mode(const str > set_mb_mode_prio(priv, mb, mode, 0); > } > > -static struct sk_buff *alloc_can_skb(struct net_device *dev, > - struct can_frame **cf) > -{ > - struct sk_buff *skb; > - > - skb = netdev_alloc_skb(dev, sizeof(struct can_frame)); > - if (unlikely(!skb)) > - return NULL; > - > - skb->protocol = htons(ETH_P_CAN); > - skb->ip_summed = CHECKSUM_UNNECESSARY; > - *cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > - > - return skb; > -} > - > -static struct sk_buff *alloc_can_err_skb(struct net_device *dev, > - struct can_frame **cf) > -{ > - struct sk_buff *skb; > - > - skb = alloc_can_skb(dev, cf); > - if (unlikely(!skb)) > - return NULL; > - > - memset(*cf, 0, sizeof(struct can_frame)); > - (*cf)->can_id = CAN_ERR_FLAG; > - (*cf)->can_dlc = CAN_ERR_DLC; > - > - return skb; > -} > - > /* > * Swtich transceiver on or off > */ > Index: trunk/kernel/2.6/include/socketcan/can/dev.h > =================================================================== > --- trunk.orig/kernel/2.6/include/socketcan/can/dev.h > +++ trunk/kernel/2.6/include/socketcan/can/dev.h > @@ -88,4 +88,8 @@ void can_put_echo_skb(struct sk_buff *sk > void can_get_echo_skb(struct net_device *dev, unsigned int idx); > void can_free_echo_skb(struct net_device *dev, unsigned int idx); > > +struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf); > +struct sk_buff *alloc_can_err_skb(struct net_device *dev, > + struct can_frame **cf); > + > #endif /* CAN_DEV_H */ > Index: trunk/kernel/2.6/drivers/net/can/cc770/cc770.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/cc770/cc770.c > +++ trunk/kernel/2.6/drivers/net/can/cc770/cc770.c > @@ -517,13 +517,10 @@ static void cc770_rx(struct net_device * > u32 id; > int i; > > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_skb(dev, &cf); > if (skb == NULL) > return; > - skb->dev = dev; > - skb->protocol = htons(ETH_P_CAN); > > - cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > config = cc770_read_reg(priv, msgobj[mo].config); > > if (ctrl1 & RMTPND_SET) { > @@ -582,15 +579,9 @@ static int cc770_err(struct net_device * > > dev_dbg(ND2D(dev), "status interrupt (%#x)\n", status); > > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_err_skb(dev, &cf); > if (skb == NULL) > return -ENOMEM; > - skb->dev = dev; > - skb->protocol = htons(ETH_P_CAN); > - cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > - memset(cf, 0, sizeof(struct can_frame)); > - cf->can_id = CAN_ERR_FLAG; > - cf->can_dlc = CAN_ERR_DLC; > > if (status & STAT_BOFF) { > /* Disable interrupts */ > Index: trunk/kernel/2.6/drivers/net/can/esd_pci331.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/esd_pci331.c > +++ trunk/kernel/2.6/drivers/net/can/esd_pci331.c > @@ -439,7 +439,7 @@ static int esd331_create_err_frame(struc > struct can_frame *cf; > struct sk_buff *skb; > > - skb = dev_alloc_skb(sizeof(*cf)); > + skb = alloc_can_err_skb(dev, &cf); > if (unlikely(skb == NULL)) > return -ENOMEM; > > @@ -449,13 +449,7 @@ static int esd331_create_err_frame(struc > stats = &dev->stats; > #endif > > - skb->dev = dev; > - skb->protocol = htons(ETH_P_CAN); > - cf = (struct can_frame *)skb_put(skb, sizeof(*cf)); > - memset(cf, 0, sizeof(*cf)); > - > - cf->can_id = CAN_ERR_FLAG | idflags; > - cf->can_dlc = CAN_ERR_DLC; > + cf->can_id |= idflags; > cf->data[1] = d1; > > netif_rx(skb); > @@ -481,14 +475,12 @@ static void esd331_irq_rx(struct net_dev > struct sk_buff *skb; > int i; > > - skb = netdev_alloc_skb(dev, sizeof(*cfrm)); > + skb = alloc_can_skb(dev, &cfrm); > if (unlikely(skb == NULL)) { > stats->rx_dropped++; > return; > } > - skb->protocol = htons(ETH_P_CAN); > > - cfrm = (struct can_frame *)skb_put(skb, sizeof(*cfrm)); > memset(cfrm, 0, sizeof(*cfrm)); > > if (eff) { > Index: trunk/kernel/2.6/drivers/net/can/mcp251x.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/mcp251x.c > +++ trunk/kernel/2.6/drivers/net/can/mcp251x.c > @@ -357,15 +357,13 @@ static void mcp251x_hw_rx(struct spi_dev > struct sk_buff *skb; > struct can_frame *frame; > > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_skb(priv->net, &frame); > if (!skb) { > dev_err(&spi->dev, "%s: out of memory for Rx'd frame\n", > __func__); > priv->net->stats.rx_dropped++; > return; > } > - skb->dev = priv->net; > - frame = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > > if (pdata->model == CAN_MCP251X_MCP2510) { > int i; > @@ -457,9 +455,6 @@ static void mcp251x_hw_rx(struct spi_dev > priv->net->stats.rx_packets++; > priv->net->stats.rx_bytes += frame->can_dlc; > > - skb->protocol = __constant_htons(ETH_P_CAN); > - skb->pkt_type = PACKET_BROADCAST; > - skb->ip_summed = CHECKSUM_UNNECESSARY; > netif_rx(skb); > } > > @@ -808,19 +803,8 @@ static void mcp251x_irq_work_handler(str > u8 eflag = mcp251x_read_reg(spi, EFLG); > > /* create error frame */ > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_err_skb(net, &frame); > if (skb) { > - frame = (struct can_frame *) > - skb_put(skb, sizeof(struct can_frame)); > - *(unsigned long long *)&frame->data = 0ULL; > - frame->can_id = CAN_ERR_FLAG; > - frame->can_dlc = CAN_ERR_DLC; > - > - skb->dev = net; > - skb->protocol = __constant_htons(ETH_P_CAN); > - skb->pkt_type = PACKET_BROADCAST; > - skb->ip_summed = CHECKSUM_UNNECESSARY; > - > /* Set error frame flags based on bus state */ > if (eflag & EFLG_TXBO) { > frame->can_id |= CAN_ERR_BUSOFF; > Index: trunk/kernel/2.6/drivers/net/can/mscan/mscan.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/mscan/mscan.c > +++ trunk/kernel/2.6/drivers/net/can/mscan/mscan.c > @@ -364,7 +364,7 @@ static int mscan_rx_poll(struct net_devi > while (npackets < quota && ((canrflg = in_8(®s->canrflg)) & > (MSCAN_RXF | MSCAN_ERR_IF))) { > > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_skb(dev, &frame); > if (!skb) { > if (printk_ratelimit()) > dev_notice(ND2D(dev), "packet dropped\n"); > @@ -373,7 +373,6 @@ static int mscan_rx_poll(struct net_devi > continue; > } > > - frame = (struct can_frame *)skb_put(skb, sizeof(*frame)); > memset(frame, 0, sizeof(*frame)); > > if (canrflg & MSCAN_RXF) { > @@ -459,9 +458,6 @@ static int mscan_rx_poll(struct net_devi > } > > npackets++; > - skb->dev = dev; > - skb->protocol = __constant_htons(ETH_P_CAN); > - skb->ip_summed = CHECKSUM_UNNECESSARY; > netif_receive_skb(skb); > } > > Index: trunk/kernel/2.6/drivers/net/can/sja1000/sja1000.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/sja1000/sja1000.c > +++ trunk/kernel/2.6/drivers/net/can/sja1000/sja1000.c > @@ -308,11 +308,9 @@ static void sja1000_rx(struct net_device > uint8_t dlc; > int i; > > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_skb(dev, &cf); > if (skb == NULL) > return; > - skb->dev = dev; > - skb->protocol = htons(ETH_P_CAN); > > fi = priv->read_reg(priv, REG_FI); > dlc = fi & 0x0F; > @@ -370,15 +368,9 @@ static int sja1000_err(struct net_device > enum can_state state = priv->can.state; > uint8_t ecc, alc; > > - skb = dev_alloc_skb(sizeof(struct can_frame)); > + skb = alloc_can_err_skb(dev, &cf); > if (skb == NULL) > return -ENOMEM; > - skb->dev = dev; > - skb->protocol = htons(ETH_P_CAN); > - cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > - memset(cf, 0, sizeof(struct can_frame)); > - cf->can_id = CAN_ERR_FLAG; > - cf->can_dlc = CAN_ERR_DLC; > > if (isrc & IRQ_DOI) { > /* data overrun interrupt */ > Index: trunk/kernel/2.6/drivers/net/can/softing/softing_main.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/softing/softing_main.c > +++ trunk/kernel/2.6/drivers/net/can/softing/softing_main.c > @@ -143,16 +143,14 @@ int softing_rx(struct net_device *netdev > ktime_t ktime) > { > struct sk_buff *skb; > + struct can_frame *cf; > int ret; > struct net_device_stats *stats; > > - skb = dev_alloc_skb(sizeof(msg)); > + skb = alloc_can_skb(netdev, &cf); > if (!skb) > return -ENOMEM; > - skb->dev = netdev; > - skb->protocol = htons(ETH_P_CAN); > - skb->ip_summed = CHECKSUM_UNNECESSARY; > - memcpy(skb_put(skb, sizeof(*msg)), msg, sizeof(*msg)); > + memcpy(cf, msg, sizeof(*msg)); > skb->tstamp = ktime; > ret = netif_rx(skb); > if (ret == NET_RX_DROP) { > Index: trunk/kernel/2.6/drivers/net/can/usb/ems_usb.c > =================================================================== > --- trunk.orig/kernel/2.6/drivers/net/can/usb/ems_usb.c > +++ trunk/kernel/2.6/drivers/net/can/usb/ems_usb.c > @@ -311,14 +311,10 @@ static void ems_usb_rx_can_msg(struct em > int i; > struct net_device_stats *stats = &dev->netdev->stats; > > - skb = netdev_alloc_skb(dev->netdev, sizeof(struct can_frame)); > + skb = alloc_can_skb(dev->netdev, &cf); > if (skb == NULL) > return; > > - skb->protocol = htons(ETH_P_CAN); > - > - cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > - > cf->can_id = msg->msg.can_msg.id; > cf->can_dlc = min_t(u8, msg->msg.can_msg.length, 8); > > @@ -349,18 +345,10 @@ static void ems_usb_rx_err(struct ems_us > struct sk_buff *skb; > struct net_device_stats *stats = &dev->netdev->stats; > > - skb = netdev_alloc_skb(dev->netdev, sizeof(struct can_frame)); > + skb = alloc_can_err_skb(dev->netdev, &cf); > if (skb == NULL) > return; > > - skb->protocol = htons(ETH_P_CAN); > - > - cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > - memset(cf, 0, sizeof(struct can_frame)); > - > - cf->can_id = CAN_ERR_FLAG; > - cf->can_dlc = CAN_ERR_DLC; > - > if (msg->type == CPC_MSG_TYPE_CAN_STATE) { > u8 state = msg->msg.can_state; > > _______________________________________________ > Socketcan-core mailing list > [email protected] > https://lists.berlios.de/mailman/listinfo/socketcan-core - -- Pengutronix e.K. | Marc Kleine-Budde | Linux Solutions for Science and Industry | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkrLNyoACgkQjTAFq1RaXHOI1ACffyNfJaKrYXKYAKBEameIXI++ jUoAn1LtRYii74HXYDF0tUWYfoSQbXw/ =ANcx -----END PGP SIGNATURE----- _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
