From 194b9a4cb91713ddb60c9f98f7212f6d8cb8e05f Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Mon, 16 Jul 2012 12:58:31 +0200 Subject: [PATCH 1/9] can: mark bittiming_const pointer in struct can_priv as const This patch marks the bittiming_const pointer as in the struct can_pric as "const". This allows us to mark the struct can_bittiming_const in the CAN drivers as "const", too. Signed-off-by: Marc Kleine-Budde --- drivers/net/can/at91_can.c | 2 +- drivers/net/can/bfin_can.c | 2 +- drivers/net/can/c_can/c_can.c | 2 +- drivers/net/can/cc770/cc770.c | 2 +- drivers/net/can/flexcan.c | 2 +- drivers/net/can/janz-ican3.c | 2 +- drivers/net/can/mcp251x.c | 2 +- drivers/net/can/mscan/mscan.c | 2 +- drivers/net/can/pch_can.c | 2 +- drivers/net/can/sja1000/sja1000.c | 2 +- drivers/net/can/ti_hecc.c | 2 +- drivers/net/can/usb/ems_usb.c | 2 +- drivers/net/can/usb/esd_usb2.c | 2 +- drivers/net/can/usb/peak_usb/pcan_usb_core.h | 2 +- include/linux/can/dev.h | 2 +- 15 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c index 6ea905c2cf6d..fcff73a73b1d 100644 --- a/drivers/net/can/at91_can.c +++ b/drivers/net/can/at91_can.c @@ -170,7 +170,7 @@ static const struct at91_devtype_data at91_devtype_data[] __devinitconst = { }, }; -static struct can_bittiming_const at91_bittiming_const = { +static const struct can_bittiming_const at91_bittiming_const = { .name = KBUILD_MODNAME, .tseg1_min = 4, .tseg1_max = 16, diff --git a/drivers/net/can/bfin_can.c b/drivers/net/can/bfin_can.c index ea3143895e6d..f2d6d258a286 100644 --- a/drivers/net/can/bfin_can.c +++ b/drivers/net/can/bfin_can.c @@ -44,7 +44,7 @@ struct bfin_can_priv { /* * bfin can timing parameters */ -static struct can_bittiming_const bfin_can_bittiming_const = { +static const struct can_bittiming_const bfin_can_bittiming_const = { .name = DRV_NAME, .tseg1_min = 1, .tseg1_max = 16, diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c index eea660800a09..4c538e388655 100644 --- a/drivers/net/can/c_can/c_can.c +++ b/drivers/net/can/c_can/c_can.c @@ -189,7 +189,7 @@ enum c_can_bus_error_types { C_CAN_ERROR_PASSIVE, }; -static struct can_bittiming_const c_can_bittiming_const = { +static const struct can_bittiming_const c_can_bittiming_const = { .name = KBUILD_MODNAME, .tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */ .tseg1_max = 16, diff --git a/drivers/net/can/cc770/cc770.c b/drivers/net/can/cc770/cc770.c index a138db11cbf0..0f12abf6591c 100644 --- a/drivers/net/can/cc770/cc770.c +++ b/drivers/net/can/cc770/cc770.c @@ -90,7 +90,7 @@ static unsigned char cc770_obj_flags[CC770_OBJ_MAX] = { [CC770_OBJ_TX] = 0, }; -static struct can_bittiming_const cc770_bittiming_const = { +static const struct can_bittiming_const cc770_bittiming_const = { .name = KBUILD_MODNAME, .tseg1_min = 1, .tseg1_max = 16, diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index 1b6f5621ce89..c8a6fc72606d 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -203,7 +203,7 @@ static struct flexcan_devtype_data fsl_imx6q_devtype_data = { .hw_ver = 10, }; -static struct can_bittiming_const flexcan_bittiming_const = { +static const struct can_bittiming_const flexcan_bittiming_const = { .name = DRV_NAME, .tseg1_min = 4, .tseg1_max = 16, diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index 08c893cb7896..e7d1532d5f1b 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -1490,7 +1490,7 @@ static const struct net_device_ops ican3_netdev_ops = { */ /* This structure was stolen from drivers/net/can/sja1000/sja1000.c */ -static struct can_bittiming_const ican3_bittiming_const = { +static const struct can_bittiming_const ican3_bittiming_const = { .name = DRV_NAME, .tseg1_min = 1, .tseg1_max = 16, diff --git a/drivers/net/can/mcp251x.c b/drivers/net/can/mcp251x.c index 9120a36ec702..a580db29e503 100644 --- a/drivers/net/can/mcp251x.c +++ b/drivers/net/can/mcp251x.c @@ -214,7 +214,7 @@ static int mcp251x_enable_dma; /* Enable SPI DMA. Default: 0 (Off) */ module_param(mcp251x_enable_dma, int, S_IRUGO); MODULE_PARM_DESC(mcp251x_enable_dma, "Enable SPI DMA. Default: 0 (Off)"); -static struct can_bittiming_const mcp251x_bittiming_const = { +static const struct can_bittiming_const mcp251x_bittiming_const = { .name = DEVICE_NAME, .tseg1_min = 3, .tseg1_max = 16, diff --git a/drivers/net/can/mscan/mscan.c b/drivers/net/can/mscan/mscan.c index 41a2a2dda7ea..2b104d5f422c 100644 --- a/drivers/net/can/mscan/mscan.c +++ b/drivers/net/can/mscan/mscan.c @@ -34,7 +34,7 @@ #include "mscan.h" -static struct can_bittiming_const mscan_bittiming_const = { +static const struct can_bittiming_const mscan_bittiming_const = { .name = "mscan", .tseg1_min = 4, .tseg1_max = 16, diff --git a/drivers/net/can/pch_can.c b/drivers/net/can/pch_can.c index 1226297e7676..48b3d62b34cb 100644 --- a/drivers/net/can/pch_can.c +++ b/drivers/net/can/pch_can.c @@ -184,7 +184,7 @@ struct pch_can_priv { int use_msi; }; -static struct can_bittiming_const pch_can_bittiming_const = { +static const struct can_bittiming_const pch_can_bittiming_const = { .name = KBUILD_MODNAME, .tseg1_min = 2, .tseg1_max = 16, diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c index 5e10472371ed..4c4f33d482d2 100644 --- a/drivers/net/can/sja1000/sja1000.c +++ b/drivers/net/can/sja1000/sja1000.c @@ -69,7 +69,7 @@ MODULE_AUTHOR("Oliver Hartkopp "); MODULE_LICENSE("Dual BSD/GPL"); MODULE_DESCRIPTION(DRV_NAME "CAN netdevice driver"); -static struct can_bittiming_const sja1000_bittiming_const = { +static const struct can_bittiming_const sja1000_bittiming_const = { .name = DRV_NAME, .tseg1_min = 1, .tseg1_max = 16, diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c index 4accd7ec6954..527dbcf95335 100644 --- a/drivers/net/can/ti_hecc.c +++ b/drivers/net/can/ti_hecc.c @@ -196,7 +196,7 @@ MODULE_VERSION(HECC_MODULE_VERSION); #define HECC_CANGIM_SIL BIT(2) /* system interrupts to int line 1 */ /* CAN Bittiming constants as per HECC specs */ -static struct can_bittiming_const ti_hecc_bittiming_const = { +static const struct can_bittiming_const ti_hecc_bittiming_const = { .name = DRV_NAME, .tseg1_min = 1, .tseg1_max = 16, diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c index 7ae65fc80032..086fa321677a 100644 --- a/drivers/net/can/usb/ems_usb.c +++ b/drivers/net/can/usb/ems_usb.c @@ -889,7 +889,7 @@ static const struct net_device_ops ems_usb_netdev_ops = { .ndo_start_xmit = ems_usb_start_xmit, }; -static struct can_bittiming_const ems_usb_bittiming_const = { +static const struct can_bittiming_const ems_usb_bittiming_const = { .name = "ems_usb", .tseg1_min = 1, .tseg1_max = 16, diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c index 09b1da5bc512..bd36e5517173 100644 --- a/drivers/net/can/usb/esd_usb2.c +++ b/drivers/net/can/usb/esd_usb2.c @@ -871,7 +871,7 @@ static const struct net_device_ops esd_usb2_netdev_ops = { .ndo_start_xmit = esd_usb2_start_xmit, }; -static struct can_bittiming_const esd_usb2_bittiming_const = { +static const struct can_bittiming_const esd_usb2_bittiming_const = { .name = "esd_usb2", .tseg1_min = ESD_USB2_TSEG1_MIN, .tseg1_max = ESD_USB2_TSEG1_MAX, diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_core.h b/drivers/net/can/usb/peak_usb/pcan_usb_core.h index a948c5a89401..4c775b620be2 100644 --- a/drivers/net/can/usb/peak_usb/pcan_usb_core.h +++ b/drivers/net/can/usb/peak_usb/pcan_usb_core.h @@ -45,7 +45,7 @@ struct peak_usb_adapter { char *name; u32 device_id; struct can_clock clock; - struct can_bittiming_const bittiming_const; + const struct can_bittiming_const bittiming_const; unsigned int ctrl_count; int (*intf_probe)(struct usb_interface *intf); diff --git a/include/linux/can/dev.h b/include/linux/can/dev.h index ee5a771fb20d..2b2fc345afca 100644 --- a/include/linux/can/dev.h +++ b/include/linux/can/dev.h @@ -33,7 +33,7 @@ struct can_priv { struct can_device_stats can_stats; struct can_bittiming bittiming; - struct can_bittiming_const *bittiming_const; + const struct can_bittiming_const *bittiming_const; struct can_clock clock; enum can_state state; From 3d42a379b6fa5b46058e3302b1802b29f64865bb Mon Sep 17 00:00:00 2001 From: Steffen Trumtrar Date: Tue, 17 Jul 2012 16:14:34 +0200 Subject: [PATCH 2/9] can: flexcan: add 2nd clock to support imx53 and newer This patch adds support for a second clock to the flexcan driver. On modern freescale ARM cores like the imx53 and imx6q two clocks ("ipg" and "per") must be enabled in order to access the CAN core. In the original driver, the clock was requested without specifying the connection id, further all mainline ARM archs with flexcan support (imx28, imx25, imx35) register their flexcan clock without a connection id, too. This patch first renames the existing clk variable to clk_ipg and converts it to devm for easier error handling. The connection id "ipg" is added to the devm_clk_get() call. Then a second clock "per" is requested. As all archs don't specify a connection id, both clk_get return the same clock. This ensures compatibility to existing flexcan support and adds support for imx53 at the same time. After this patch hits mainline, the archs may give their existing flexcan clock the "ipg" connection id and implement a dummy "per" clock. This patch has been tested on imx28 (unmodified clk tree) and on imx53 with a seperate "ipg" and "per" clock. Cc: Sascha Hauer Cc: Shawn Guo Signed-off-by: Steffen Trumtrar Acked-by: Hui Wang Signed-off-by: Marc Kleine-Budde --- drivers/net/can/flexcan.c | 45 +++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index c8a6fc72606d..c5f143165f80 100644 --- a/drivers/net/can/flexcan.c +++ b/drivers/net/can/flexcan.c @@ -190,7 +190,8 @@ struct flexcan_priv { u32 reg_esr; u32 reg_ctrl_default; - struct clk *clk; + struct clk *clk_ipg; + struct clk *clk_per; struct flexcan_platform_data *pdata; const struct flexcan_devtype_data *devtype_data; }; @@ -828,7 +829,8 @@ static int flexcan_open(struct net_device *dev) struct flexcan_priv *priv = netdev_priv(dev); int err; - clk_prepare_enable(priv->clk); + clk_prepare_enable(priv->clk_ipg); + clk_prepare_enable(priv->clk_per); err = open_candev(dev); if (err) @@ -850,7 +852,8 @@ static int flexcan_open(struct net_device *dev) out_close: close_candev(dev); out: - clk_disable_unprepare(priv->clk); + clk_disable_unprepare(priv->clk_per); + clk_disable_unprepare(priv->clk_ipg); return err; } @@ -864,7 +867,8 @@ static int flexcan_close(struct net_device *dev) flexcan_chip_stop(dev); free_irq(dev->irq, dev); - clk_disable_unprepare(priv->clk); + clk_disable_unprepare(priv->clk_per); + clk_disable_unprepare(priv->clk_ipg); close_candev(dev); @@ -903,7 +907,8 @@ static int __devinit register_flexcandev(struct net_device *dev) struct flexcan_regs __iomem *regs = priv->base; u32 reg, err; - clk_prepare_enable(priv->clk); + clk_prepare_enable(priv->clk_ipg); + clk_prepare_enable(priv->clk_per); /* select "bus clock", chip must be disabled */ flexcan_chip_disable(priv); @@ -936,7 +941,8 @@ static int __devinit register_flexcandev(struct net_device *dev) out: /* disable core and turn off clocks */ flexcan_chip_disable(priv); - clk_disable_unprepare(priv->clk); + clk_disable_unprepare(priv->clk_per); + clk_disable_unprepare(priv->clk_ipg); return err; } @@ -964,7 +970,7 @@ static int __devinit flexcan_probe(struct platform_device *pdev) struct net_device *dev; struct flexcan_priv *priv; struct resource *mem; - struct clk *clk = NULL; + struct clk *clk_ipg = NULL, *clk_per = NULL; struct pinctrl *pinctrl; void __iomem *base; resource_size_t mem_size; @@ -980,13 +986,20 @@ static int __devinit flexcan_probe(struct platform_device *pdev) "clock-frequency", &clock_freq); if (!clock_freq) { - clk = clk_get(&pdev->dev, NULL); - if (IS_ERR(clk)) { - dev_err(&pdev->dev, "no clock defined\n"); - err = PTR_ERR(clk); + clk_ipg = devm_clk_get(&pdev->dev, "ipg"); + if (IS_ERR(clk_ipg)) { + dev_err(&pdev->dev, "no ipg clock defined\n"); + err = PTR_ERR(clk_ipg); + goto failed_clock; + } + clock_freq = clk_get_rate(clk_ipg); + + clk_per = devm_clk_get(&pdev->dev, "per"); + if (IS_ERR(clk_per)) { + dev_err(&pdev->dev, "no per clock defined\n"); + err = PTR_ERR(clk_per); goto failed_clock; } - clock_freq = clk_get_rate(clk); } mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); @@ -1039,7 +1052,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev) CAN_CTRLMODE_BERR_REPORTING; priv->base = base; priv->dev = dev; - priv->clk = clk; + priv->clk_ipg = clk_ipg; + priv->clk_per = clk_per; priv->pdata = pdev->dev.platform_data; priv->devtype_data = devtype_data; @@ -1067,8 +1081,6 @@ static int __devinit flexcan_probe(struct platform_device *pdev) failed_map: release_mem_region(mem->start, mem_size); failed_get: - if (clk) - clk_put(clk); failed_clock: return err; } @@ -1086,9 +1098,6 @@ static int __devexit flexcan_remove(struct platform_device *pdev) mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); release_mem_region(mem->start, resource_size(mem)); - if (priv->clk) - clk_put(priv->clk); - free_candev(dev); return 0; From 8456a9196f7696421eecffdbab5d785f852d95e8 Mon Sep 17 00:00:00 2001 From: "Ira W. Snyder" Date: Wed, 18 Jul 2012 15:33:13 -0700 Subject: [PATCH 3/9] can: janz-ican3: remove dead code The code which used this variable was removed during review, before the driver was added to mainline Linux. It is now dead code, and can be removed. Signed-off-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index e7d1532d5f1b..f41623d003e1 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -235,7 +235,6 @@ struct ican3_dev { /* fast host interface */ unsigned int fastrx_start; - unsigned int fastrx_int; unsigned int fastrx_num; unsigned int fasttx_start; unsigned int fasttx_num; @@ -454,7 +453,6 @@ static void __devinit ican3_init_fast_host_interface(struct ican3_dev *mod) /* save the start recv page */ mod->fastrx_start = mod->free_page; mod->fastrx_num = 0; - mod->fastrx_int = 0; /* build a single fast tohost queue descriptor */ memset(&desc, 0, sizeof(desc)); From 007890d726602c925077381500f0b633cfacd711 Mon Sep 17 00:00:00 2001 From: "Ira W. Snyder" Date: Wed, 18 Jul 2012 15:33:14 -0700 Subject: [PATCH 4/9] can: janz-ican3: drop invalid skbs The commit which added the janz-ican3 driver and commit 3ccd4c61 "can: Unify droping of invalid tx skbs and netdev stats" were committed into mainline Linux during the same merge window. Therefore, the addition of this code to the janz-ican3 driver was forgotten. This patch adds the expected code. Signed-off-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index f41623d003e1..754b803fa926 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -1420,6 +1420,9 @@ static int ican3_xmit(struct sk_buff *skb, struct net_device *ndev) void __iomem *desc_addr; unsigned long flags; + if (can_dropped_invalid_skb(ndev, skb)) + return NETDEV_TX_OK; + spin_lock_irqsave(&mod->lock, flags); /* check that we can actually transmit */ From 9e4d6909a273ada78cf48379e478855bc13ae0cb Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Thu, 21 Oct 2010 18:39:26 +0200 Subject: [PATCH 5/9] can: janz-ican3: cleanup of ican3_to_can_frame and can_frame_to_ican3 This patch cleans up the ICAN3 to Linux CAN frame and vice versa conversion functions: - RX: Use get_can_dlc() to limit the dlc value. - RX+TX: Don't copy the whole frame, only copy the amount of bytes specified in cf->can_dlc. Acked-by: Ira W. Snyder Tested-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index 754b803fa926..b19aca591c7a 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -811,10 +811,10 @@ static void ican3_to_can_frame(struct ican3_dev *mod, cf->can_id |= desc->data[0] << 3; cf->can_id |= (desc->data[1] & 0xe0) >> 5; - cf->can_dlc = desc->data[1] & ICAN3_CAN_DLC_MASK; - memcpy(cf->data, &desc->data[2], sizeof(cf->data)); + cf->can_dlc = get_can_dlc(desc->data[1] & ICAN3_CAN_DLC_MASK); + memcpy(cf->data, &desc->data[2], cf->can_dlc); } else { - cf->can_dlc = desc->data[0] & ICAN3_CAN_DLC_MASK; + cf->can_dlc = get_can_dlc(desc->data[0] & ICAN3_CAN_DLC_MASK); if (desc->data[0] & ICAN3_EFF_RTR) cf->can_id |= CAN_RTR_FLAG; @@ -829,7 +829,7 @@ static void ican3_to_can_frame(struct ican3_dev *mod, cf->can_id |= desc->data[3] >> 5; /* 2-0 */ } - memcpy(cf->data, &desc->data[6], sizeof(cf->data)); + memcpy(cf->data, &desc->data[6], cf->can_dlc); } } @@ -861,7 +861,7 @@ static void can_frame_to_ican3(struct ican3_dev *mod, } /* copy the data bits into the descriptor */ - memcpy(&desc->data[6], cf->data, sizeof(cf->data)); + memcpy(&desc->data[6], cf->data, cf->can_dlc); } /* From 88b587039c1ad4e7a981bea3269eeb02a1a2a14b Mon Sep 17 00:00:00 2001 From: "Ira W. Snyder" Date: Thu, 19 Jul 2012 08:54:18 -0700 Subject: [PATCH 6/9] can: janz-ican3: fix error and byte counters The error and byte counter statistics were being incremented incorrectly. For example, a TX error would be counted both in tx_errors and rx_errors. This corrects the problem so that tx_errors and rx_errors are only incremented for errors caused by packets sent to the bus. Error packets generated by the driver are not counted. The byte counters are only increased for packets which are actually transmitted or received from the bus. Error packets generated by the driver are not counted. Signed-off-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index b19aca591c7a..4a5a8fb53a2f 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -907,8 +907,8 @@ static void ican3_handle_msglost(struct ican3_dev *mod, struct ican3_msg *msg) if (skb) { cf->can_id |= CAN_ERR_CRTL; cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW; + stats->rx_over_errors++; stats->rx_errors++; - stats->rx_bytes += cf->can_dlc; netif_rx(skb); } } @@ -982,7 +982,6 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) dev_dbg(mod->dev, "bus error interrupt\n"); mod->can.can_stats.bus_error++; - stats->rx_errors++; cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; switch (ecc & ECC_MASK) { @@ -1001,8 +1000,12 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) break; } - if ((ecc & ECC_DIR) == 0) + if (!(ecc & ECC_DIR)) { cf->data[2] |= CAN_ERR_PROT_TX; + stats->tx_errors++; + } else { + stats->rx_errors++; + } cf->data[6] = txerr; cf->data[7] = rxerr; @@ -1028,8 +1031,6 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) } mod->can.state = state; - stats->rx_errors++; - stats->rx_bytes += cf->can_dlc; netif_rx(skb); return 0; } From 83702f69272e4591a91a27eb58eade1bcd361dae Mon Sep 17 00:00:00 2001 From: "Ira W. Snyder" Date: Thu, 19 Jul 2012 08:54:42 -0700 Subject: [PATCH 7/9] can: janz-ican3: fix support for CAN_RAW_RECV_OWN_MSGS The Janz VMOD-ICAN3 firmware does not support any sort of TX-done notification or interrupt. The driver previously used the hardware loopback to attempt to work around this deficiency, but this caused all sockets to receive all messages, even if CAN_RAW_RECV_OWN_MSGS is off. Using the new function ican3_cmp_echo_skb(), we can drop the loopback messages and return the original skbs. This fixes the issues with CAN_RAW_RECV_OWN_MSGS. A private skb queue is used to store the echo skbs. This avoids the need for any index management. Due to a lack of TX-error interrupts, bus errors are permanently enabled, and are used as a TX-error notification. This is used to drop an echo skb when transmission fails. Bus error packets are not generated if the user has not enabled bus error reporting. Signed-off-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 203 +++++++++++++++++++++++++++-------- 1 file changed, 157 insertions(+), 46 deletions(-) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index 4a5a8fb53a2f..47f8f6b4fef9 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -220,6 +220,9 @@ struct ican3_dev { /* old and new style host interface */ unsigned int iftype; + /* queue for echo packets */ + struct sk_buff_head echoq; + /* * Any function which changes the current DPM page must hold this * lock while it is performing data accesses. This ensures that the @@ -925,7 +928,7 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) struct net_device *dev = mod->ndev; struct net_device_stats *stats = &dev->stats; enum can_state state = mod->can.state; - u8 status, isrc, rxerr, txerr; + u8 isrc, ecc, status, rxerr, txerr; struct can_frame *cf; struct sk_buff *skb; @@ -941,15 +944,43 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) return -EINVAL; } - skb = alloc_can_err_skb(dev, &cf); - if (skb == NULL) - return -ENOMEM; - isrc = msg->data[0]; + ecc = msg->data[2]; status = msg->data[3]; rxerr = msg->data[4]; txerr = msg->data[5]; + /* + * This hardware lacks any support other than bus error messages to + * determine if packet transmission has failed. + * + * When TX errors happen, one echo skb needs to be dropped from the + * front of the queue. + * + * A small bit of code is duplicated here and below, to avoid error + * skb allocation when it will just be freed immediately. + */ + if (isrc == CEVTIND_BEI) { + int ret; + dev_dbg(mod->dev, "bus error interrupt\n"); + + /* TX error */ + if (!(ecc & ECC_DIR)) { + kfree_skb(skb_dequeue(&mod->echoq)); + stats->tx_errors++; + } else { + stats->rx_errors++; + } + + /* bus error reporting is off, return immediately */ + if (!(mod->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) + return 0; + } + + skb = alloc_can_err_skb(dev, &cf); + if (skb == NULL) + return -ENOMEM; + /* data overrun interrupt */ if (isrc == CEVTIND_DOI || isrc == CEVTIND_LOST) { dev_dbg(mod->dev, "data overrun interrupt\n"); @@ -978,9 +1009,6 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) /* bus error interrupt */ if (isrc == CEVTIND_BEI) { - u8 ecc = msg->data[2]; - - dev_dbg(mod->dev, "bus error interrupt\n"); mod->can.can_stats.bus_error++; cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; @@ -1000,12 +1028,8 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) break; } - if (!(ecc & ECC_DIR)) { + if (!(ecc & ECC_DIR)) cf->data[2] |= CAN_ERR_PROT_TX; - stats->tx_errors++; - } else { - stats->rx_errors++; - } cf->data[6] = txerr; cf->data[7] = rxerr; @@ -1089,6 +1113,88 @@ static void ican3_handle_message(struct ican3_dev *mod, struct ican3_msg *msg) } } +/* + * The ican3 needs to store all echo skbs, and therefore cannot + * use the generic infrastructure for this. + */ +static void ican3_put_echo_skb(struct ican3_dev *mod, struct sk_buff *skb) +{ + struct sock *srcsk = skb->sk; + + if (atomic_read(&skb->users) != 1) { + struct sk_buff *old_skb = skb; + + skb = skb_clone(old_skb, GFP_ATOMIC); + kfree_skb(old_skb); + if (!skb) + return; + } else { + skb_orphan(skb); + } + + skb->sk = srcsk; + + /* save this skb for tx interrupt echo handling */ + skb_queue_tail(&mod->echoq, skb); +} + +static unsigned int ican3_get_echo_skb(struct ican3_dev *mod) +{ + struct sk_buff *skb = skb_dequeue(&mod->echoq); + struct can_frame *cf; + u8 dlc; + + /* this should never trigger unless there is a driver bug */ + if (!skb) { + netdev_err(mod->ndev, "BUG: echo skb not occupied\n"); + return 0; + } + + cf = (struct can_frame *)skb->data; + dlc = cf->can_dlc; + + /* check flag whether this packet has to be looped back */ + if (skb->pkt_type != PACKET_LOOPBACK) { + kfree_skb(skb); + return dlc; + } + + skb->protocol = htons(ETH_P_CAN); + skb->pkt_type = PACKET_BROADCAST; + skb->ip_summed = CHECKSUM_UNNECESSARY; + skb->dev = mod->ndev; + netif_receive_skb(skb); + return dlc; +} + +/* + * Compare an skb with an existing echo skb + * + * This function will be used on devices which have a hardware loopback. + * On these devices, this function can be used to compare a received skb + * with the saved echo skbs so that the hardware echo skb can be dropped. + * + * Returns true if the skb's are identical, false otherwise. + */ +static bool ican3_echo_skb_matches(struct ican3_dev *mod, struct sk_buff *skb) +{ + struct can_frame *cf = (struct can_frame *)skb->data; + struct sk_buff *echo_skb = skb_peek(&mod->echoq); + struct can_frame *echo_cf; + + if (!echo_skb) + return false; + + echo_cf = (struct can_frame *)echo_skb->data; + if (cf->can_id != echo_cf->can_id) + return false; + + if (cf->can_dlc != echo_cf->can_dlc) + return false; + + return memcmp(cf->data, echo_cf->data, cf->can_dlc) == 0; +} + /* * Check that there is room in the TX ring to transmit another skb * @@ -1099,6 +1205,10 @@ static bool ican3_txok(struct ican3_dev *mod) struct ican3_fast_desc __iomem *desc; u8 control; + /* check that we have echo queue space */ + if (skb_queue_len(&mod->echoq) >= ICAN3_TX_BUFFERS) + return false; + /* copy the control bits of the descriptor */ ican3_set_page(mod, mod->fasttx_start + (mod->fasttx_num / 16)); desc = mod->dpm + ((mod->fasttx_num % 16) * sizeof(*desc)); @@ -1149,10 +1259,27 @@ static int ican3_recv_skb(struct ican3_dev *mod) /* convert the ICAN3 frame into Linux CAN format */ ican3_to_can_frame(mod, &desc, cf); - /* receive the skb, update statistics */ - netif_receive_skb(skb); + /* + * If this is an ECHO frame received from the hardware loopback + * feature, use the skb saved in the ECHO stack instead. This allows + * the Linux CAN core to support CAN_RAW_RECV_OWN_MSGS correctly. + * + * Since this is a confirmation of a successfully transmitted packet + * sent from this host, update the transmit statistics. + * + * Also, the netdevice queue needs to be allowed to send packets again. + */ + if (ican3_echo_skb_matches(mod, skb)) { + stats->tx_packets++; + stats->tx_bytes += ican3_get_echo_skb(mod); + kfree_skb(skb); + goto err_noalloc; + } + + /* update statistics, receive the skb */ stats->rx_packets++; stats->rx_bytes += cf->can_dlc; + netif_receive_skb(skb); err_noalloc: /* toggle the valid bit and return the descriptor to the ring */ @@ -1175,13 +1302,13 @@ err_noalloc: static int ican3_napi(struct napi_struct *napi, int budget) { struct ican3_dev *mod = container_of(napi, struct ican3_dev, napi); - struct ican3_msg msg; unsigned long flags; int received = 0; int ret; /* process all communication messages */ while (true) { + struct ican3_msg msg; ret = ican3_recv_msg(mod, &msg); if (ret) break; @@ -1353,7 +1480,6 @@ static int __devinit ican3_startup_module(struct ican3_dev *mod) static int ican3_open(struct net_device *ndev) { struct ican3_dev *mod = netdev_priv(ndev); - u8 quota; int ret; /* open the CAN layer */ @@ -1363,19 +1489,6 @@ static int ican3_open(struct net_device *ndev) return ret; } - /* set the bus error generation state appropriately */ - if (mod->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING) - quota = ICAN3_BUSERR_QUOTA_MAX; - else - quota = 0; - - ret = ican3_set_buserror(mod, quota); - if (ret) { - dev_err(mod->dev, "unable to set bus-error\n"); - close_candev(ndev); - return ret; - } - /* bring the bus online */ ret = ican3_set_bus_state(mod, true); if (ret) { @@ -1407,6 +1520,9 @@ static int ican3_stop(struct net_device *ndev) return ret; } + /* drop all outstanding echo skbs */ + skb_queue_purge(&mod->echoq); + /* close the CAN layer */ close_candev(ndev); return 0; @@ -1415,7 +1531,6 @@ static int ican3_stop(struct net_device *ndev) static int ican3_xmit(struct sk_buff *skb, struct net_device *ndev) { struct ican3_dev *mod = netdev_priv(ndev); - struct net_device_stats *stats = &ndev->stats; struct can_frame *cf = (struct can_frame *)skb->data; struct ican3_fast_desc desc; void __iomem *desc_addr; @@ -1428,8 +1543,7 @@ static int ican3_xmit(struct sk_buff *skb, struct net_device *ndev) /* check that we can actually transmit */ if (!ican3_txok(mod)) { - dev_err(mod->dev, "no free descriptors, stopping queue\n"); - netif_stop_queue(ndev); + dev_err(mod->dev, "BUG: no free descriptors\n"); spin_unlock_irqrestore(&mod->lock, flags); return NETDEV_TX_BUSY; } @@ -1443,6 +1557,14 @@ static int ican3_xmit(struct sk_buff *skb, struct net_device *ndev) /* convert the Linux CAN frame into ICAN3 format */ can_frame_to_ican3(mod, cf, &desc); + /* + * This hardware doesn't have TX-done notifications, so we'll try and + * emulate it the best we can using ECHO skbs. Add the skb to the ECHO + * stack. Upon packet reception, check if the ECHO skb and received + * skb match, and use that to wake the queue. + */ + ican3_put_echo_skb(mod, skb); + /* * the programming manual says that you must set the IVALID bit, then * interrupt, then set the valid bit. Quite weird, but it seems to be @@ -1461,19 +1583,7 @@ static int ican3_xmit(struct sk_buff *skb, struct net_device *ndev) mod->fasttx_num = (desc.control & DESC_WRAP) ? 0 : (mod->fasttx_num + 1); - /* update statistics */ - stats->tx_packets++; - stats->tx_bytes += cf->can_dlc; - kfree_skb(skb); - - /* - * This hardware doesn't have TX-done notifications, so we'll try and - * emulate it the best we can using ECHO skbs. Get the next TX - * descriptor, and see if we have room to send. If not, stop the queue. - * It will be woken when the ECHO skb for the current packet is recv'd. - */ - - /* copy the control bits of the descriptor */ + /* if there is no free descriptor space, stop the transmit queue */ if (!ican3_txok(mod)) netif_stop_queue(ndev); @@ -1669,6 +1779,7 @@ static int __devinit ican3_probe(struct platform_device *pdev) mod->dev = &pdev->dev; mod->num = pdata->modno; netif_napi_add(ndev, &mod->napi, ican3_napi, ICAN3_RX_BUFFERS); + skb_queue_head_init(&mod->echoq); spin_lock_init(&mod->lock); init_completion(&mod->termination_comp); init_completion(&mod->buserror_comp); From 30df5888e4a244093c1b403b55ef889c97824f7b Mon Sep 17 00:00:00 2001 From: "Ira W. Snyder" Date: Wed, 18 Jul 2012 15:33:17 -0700 Subject: [PATCH 8/9] can: janz-ican3: avoid firmware lockup caused by infinite bus error quota If the bus error quota is set to infinite and the host CPU cannot keep up, the Janz VMOD-ICAN3 firmware will stop responding to control messages until the controller is reset. The firmware will automatically stop sending bus error messages when the quota is reached, and will only resume sending bus error messages when the quota is re-set to a positive value. This limitation is worked around by setting the bus error quota to one message, and then re-setting the quota to one message every time a bus error message is received. By doing this, the firmware never stops responding to control messages. The CAN bus can be reset without a hard-reset of the controller card. Signed-off-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index 47f8f6b4fef9..e06ec403521a 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -972,6 +972,16 @@ static int ican3_handle_cevtind(struct ican3_dev *mod, struct ican3_msg *msg) stats->rx_errors++; } + /* + * The controller automatically disables bus-error interrupts + * and therefore we must re-enable them. + */ + ret = ican3_set_buserror(mod, 1); + if (ret) { + dev_err(mod->dev, "unable to re-enable bus-error\n"); + return ret; + } + /* bus error reporting is off, return immediately */ if (!(mod->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)) return 0; @@ -1451,7 +1461,7 @@ static int __devinit ican3_startup_module(struct ican3_dev *mod) } /* default to "bus errors enabled" */ - ret = ican3_set_buserror(mod, ICAN3_BUSERR_QUOTA_MAX); + ret = ican3_set_buserror(mod, 1); if (ret) { dev_err(mod->dev, "unable to set bus-error\n"); return ret; From 3b5c6b9e49f78f07ebcd34b38c1185e57a0fd9eb Mon Sep 17 00:00:00 2001 From: "Ira W. Snyder" Date: Wed, 18 Jul 2012 15:33:18 -0700 Subject: [PATCH 9/9] can: janz-ican3: add support for one shot mode The Janz VMOD-ICAN3 hardware has support for one shot packet transmission. This means that a packet will be attempted to be sent once, with no automatic retries. The SocketCAN core has a controller-wide setting for this mode: CAN_CTRLMODE_ONE_SHOT. The Janz VMOD-ICAN3 hardware supports this flag on a per-packet level, but the SocketCAN core does not. Signed-off-by: Ira W. Snyder Signed-off-by: Marc Kleine-Budde --- drivers/net/can/janz-ican3.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c index e06ec403521a..98ee43819911 100644 --- a/drivers/net/can/janz-ican3.c +++ b/drivers/net/can/janz-ican3.c @@ -116,6 +116,7 @@ #define ICAN3_BUSERR_QUOTA_MAX 255 /* Janz ICAN3 CAN Frame Conversion */ +#define ICAN3_SNGL 0x02 #define ICAN3_ECHO 0x10 #define ICAN3_EFF_RTR 0x40 #define ICAN3_SFF_RTR 0x10 @@ -848,6 +849,10 @@ static void can_frame_to_ican3(struct ican3_dev *mod, desc->data[0] |= cf->can_dlc; desc->data[1] |= ICAN3_ECHO; + /* support single transmission (no retries) mode */ + if (mod->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT) + desc->data[1] |= ICAN3_SNGL; + if (cf->can_id & CAN_RTR_FLAG) desc->data[0] |= ICAN3_EFF_RTR; @@ -1810,7 +1815,8 @@ static int __devinit ican3_probe(struct platform_device *pdev) mod->can.do_set_mode = ican3_set_mode; mod->can.do_get_berr_counter = ican3_get_berr_counter; mod->can.ctrlmode_supported = CAN_CTRLMODE_3_SAMPLES - | CAN_CTRLMODE_BERR_REPORTING; + | CAN_CTRLMODE_BERR_REPORTING + | CAN_CTRLMODE_ONE_SHOT; /* find our IRQ number */ mod->irq = platform_get_irq(pdev, 0);