Hi Anant, here is my quick review.
Anant Gole wrote: > TI HECC (High End CAN Controller) module is found on many TI devices. It > has 32 hardware mailboxes with full implemtation of CAN protocol 2.0B > with bus speeds up to 1Mbps. Specifications of the module are available > at TI web <http://www.ti.com> > > Signed-off-by: Anant Gole <[email protected]> > --- > drivers/net/can/Kconfig | 7 + > drivers/net/can/Makefile | 1 + > drivers/net/can/ti_hecc.c | 1002 > ++++++++++++++++++++++++++++++++++ > include/linux/can/platform/ti_hecc.h | 40 ++ > 4 files changed, 1050 insertions(+), 0 deletions(-) > create mode 100644 drivers/net/can/ti_hecc.c > create mode 100644 include/linux/can/platform/ti_hecc.h > > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig > index 0900743..77523af 100644 > --- a/drivers/net/can/Kconfig > +++ b/drivers/net/can/Kconfig > @@ -82,6 +82,13 @@ config CAN_KVASER_PCI > This driver is for the the PCIcanx and PCIcan cards (1, 2 or > 4 channel) from Kvaser (http://www.kvaser.com). > > +config CAN_TI_HECC > + depends on CAN_DEV > + tristate "TI High End CAN Controller" > + ---help--- > + Driver for TI HECC (High End CAN Controller) module found on many > + TI devices. The device specifications are available from www.ti.com > + > config CAN_DEBUG_DEVICES > bool "CAN devices debugging messages" > depends on CAN > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile > index 523a941..45d42dc 100644 > --- a/drivers/net/can/Makefile > +++ b/drivers/net/can/Makefile > @@ -8,5 +8,6 @@ obj-$(CONFIG_CAN_DEV) += can-dev.o > can-dev-y := dev.o > > obj-$(CONFIG_CAN_SJA1000) += sja1000/ > +obj-$(CONFIG_CAN_TI_HECC) += ti_hecc.o Die Patch will fail because two more drivers have been accepted for net-next-2.6. They should show up soon. > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG > diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c > new file mode 100644 > index 0000000..0d3cbe1 > --- /dev/null > +++ b/drivers/net/can/ti_hecc.c > @@ -0,0 +1,1002 @@ > +/* > + * TI HECC (CAN) device driver > + * > + * This driver supports TI's HECC (High End CAN Controller module) and the > + * specs for the same is available at <http://www.ti.com> > + * > + * Copyright (C) 2009 Texas Instruments Incorporated - http://www.ti.com/ > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed as is WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + */ > + > +/* > + * Your platform definitions should specify module ram offsets and interrupt > + * number to use as follows: > + * > + * static struct ti_hecc_platform_data omap3517_evm_hecc_pdata = { > + * .scc_hecc_offset = 0, > + * .scc_ram_offset = 0x3000, > + * .hecc_ram_offset = 0x3000, > + * .mbox_offset = 0x2000, > + * .int_line = 0, > + * .revision = 1, > + * }; > + * > + * Please see include/can/platform/ti_hecc.h for description of above fields > + * > + */ > + > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/kernel.h> > +#include <linux/sched.h> > +#include <linux/types.h> > +#include <linux/fcntl.h> > +#include <linux/interrupt.h> > +#include <linux/ptrace.h> > +#include <linux/string.h> > +#include <linux/errno.h> > +#include <linux/netdevice.h> > +#include <linux/if_arp.h> > +#include <linux/if_ether.h> > +#include <linux/skbuff.h> > +#include <linux/delay.h> > +#include <linux/platform_device.h> > +#include <linux/clk.h> > +#include <linux/io.h> > +#include <linux/debugfs.h> Do you still use debugfs? Maybe some more #include's could be removed, e.g. ptrace.h. > +#include <linux/can.h> > +#include <linux/can/dev.h> > +#include <linux/can/error.h> > +#include <linux/can/platform/ti_hecc.h> > + > +#define DRV_NAME "ti_hecc" > +#define HECC_MODULE_VERSION "0.4" > +MODULE_VERSION(HECC_MODULE_VERSION); > +#define DRV_DESC "TI High End CAN Controller Driver " HECC_MODULE_VERSION > + > +/* TX / RX Mailbox Configuration */ > +#define HECC_MAX_MAILBOXES 32 /* hardware mboxes - do not change */ > +#define MAX_TX_PRIO 0x3F /* hardware value - do not change */ > + > +#define HECC_MAX_TX_MBOX 4 > +#define HECC_MAX_RX_MBOX (HECC_MAX_MAILBOXES - HECC_MAX_TX_MBOX) > + > +#if (HECC_MAX_TX_MBOX > CAN_ECHO_SKB_MAX) > +#error "HECC: MAX TX mailboxes should be equal or less than CAN_ECHO_SKB_MAX" > +#endif > + > +/* Important Note: TX mailbox configuration Please write: /* * Important Note: TX mailbox configuration > + * TX mailboxes should be restricted to the number of SKB buffers to avoid > + * maintaining SKB buffers separately. TX mailboxes should be a power of 2 > + * for the mailbox logic to work. Top mailbox numbers are reserved for RX > + * and lower mailboxes for TX. > + * > + * HECC_MAX_TX_MBOX HECC_MB_TX_SHIFT > + * 4 (default) 2 > + * 8 3 > + * 16 4 > + */ > +#define HECC_MB_TX_SHIFT 2 /* as per table above */ > + > +#define HECC_TX_PRIO_SHIFT (HECC_MB_TX_SHIFT) > +#define HECC_TX_PRIO_MASK (0x3f << HECC_MB_TX_SHIFT) > +#define HECC_TX_MB_MASK (HECC_MAX_TX_MBOX - 1) > +#define HECC_TX_MASK ((HECC_MAX_TX_MBOX - 1) | HECC_TX_PRIO_MASK) Would be nice to aligned the line above as well. > +#define HECC_TX_MBOX_MASK (~((1 << HECC_MAX_TX_MBOX) - 1)) > +#define HECC_DEF_NAPI_WEIGHT HECC_MAX_RX_MBOX > + > +/* Important Note: RX mailboxes are further logically split into two - main > + * and buffer mailboxes. The goal is to get all packets into main mailboxes > + * as driven by mailbox number and receive priority (higher to lower) and > + * buffer mailboxes are used to receive pkts while main mailboxes are being > + * processed. This ensures in-order packet reception. > + * > + * Here are the recommended values for buffer mailbox. Note that RX mailboxes > + * start after TX mailboxes: > + * > + * HECC_MAX_RX_MBOX HECC_RX_BUFFER_MBOX No of buffer mailboxes > + * 28 12 8 > + * 16 20 4 > + */ > +#define HECC_RX_BUFFER_MBOX 12 /* as per table above */ > + > +#define HECC_RX_FIRST_MBOX (HECC_MAX_MAILBOXES - 1) > +#define HECC_RX_HIGH_MBOX_MASK (~((1 << HECC_RX_BUFFER_MBOX) - 1)) > + > +/* TI HECC module registers */ > +#define HECC_CANME 0x0 /* Mailbox enable */ > +#define HECC_CANMD 0x4 /* Mailbox direction */ > +#define HECC_CANTRS 0x8 /* Transmit request set */ > +#define HECC_CANTRR 0xC /* Transmit request */ > +#define HECC_CANTA 0x10 /* Transmission acknowledge */ > +#define HECC_CANAA 0x14 /* Abort acknowledge */ > +#define HECC_CANRMP 0x18 /* Receive message pending */ > +#define HECC_CANRML 0x1C /* Remote message lost */ > +#define HECC_CANRFP 0x20 /* Remote frame pending */ > +#define HECC_CANGAM 0x24 /* SECC only:Global acceptance mask */ > +#define HECC_CANMC 0x28 /* Master control */ > +#define HECC_CANBTC 0x2C /* Bit timing configuration */ > +#define HECC_CANES 0x30 /* Error and status */ > +#define HECC_CANTEC 0x34 /* Transmit error counter */ > +#define HECC_CANREC 0x38 /* Receive error counter */ > +#define HECC_CANGIF0 0x3C /* Global interrupt flag 0 */ > +#define HECC_CANGIM 0x40 /* Global interrupt mask */ > +#define HECC_CANGIF1 0x44 /* Global interrupt flag 1 */ > +#define HECC_CANMIM 0x48 /* Mailbox interrupt mask */ > +#define HECC_CANMIL 0x4C /* Mailbox interrupt level */ > +#define HECC_CANOPC 0x50 /* Overwrite protection control */ > +#define HECC_CANTIOC 0x54 /* Transmit I/O control */ > +#define HECC_CANRIOC 0x58 /* Receive I/O control */ > +#define HECC_CANLNT 0x5C /* HECC only: Local network time */ > +#define HECC_CANTOC 0x60 /* HECC only: Time-out control */ > +#define HECC_CANTOS 0x64 /* HECC only: Time-out status */ > +#define HECC_CANTIOCE 0x68 /* SCC only:Enhanced TX I/O > control */ > +#define HECC_CANRIOCE 0x6C /* SCC only:Enhanced RX I/O > control */ > + > +/* SCC only:Local acceptance registers */ > +#define HECC_CANLAM0 (priv->scc_ram_offset + 0x0) > +#define HECC_CANLAM3 (priv->scc_ram_offset + 0xC) > + > +/* HECC only */ > +#define HECC_CANLAM(mbxno) (priv->hecc_ram_offset + ((mbxno) * 4)) > +#define HECC_CANMOTS(mbxno) (priv->hecc_ram_offset + ((mbxno) * 4) + 0x80) > +#define HECC_CANMOTO(mbxno) (priv->hecc_ram_offset + ((mbxno) * 4) + 0x100) > + > +/* Mailbox registers */ > +#define HECC_CANMID(mbxno) (priv->mbox_offset + ((mbxno) * 0x10)) > +#define HECC_CANMCF(mbxno) (priv->mbox_offset + ((mbxno) * 0x10) + 0x4) > +#define HECC_CANMDL(mbxno) (priv->mbox_offset + ((mbxno) * 0x10) + 0x8) > +#define HECC_CANMDH(mbxno) (priv->mbox_offset + ((mbxno) * 0x10) + 0xC) The above macros require that the name "priv" is declared. They are used as shown below, hecc_write(priv, HECC_CANMDH(mbxno), val); which is not really transparent. Either add the argument "priv" to the macro definition, or even better, introduce inline function for read/write of mailbox registers, e.g. hecc_mbox_read(). > + > +#define HECC_SET_REG 0xFFFFFFFF > +#define HECC_CANID_MASK 0x3FF /* 18 bits mask for extended > id's */ > +#define HECC_CCE_WAIT_COUNT 100 /* Wait for ~1 sec for CCE bit */ > + > +#define HECC_CANMC_SCM BIT(13) /* SCC compat mode */ > +#define HECC_CANMC_CCR BIT(12) /* Change config request */ > +#define HECC_CANMC_PDR BIT(11) /* Local Power down - for sleep > mode */ > +#define HECC_CANMC_ABO BIT(7) /* Auto Bus On */ > +#define HECC_CANMC_STM BIT(6) /* Self test mode - loopback */ > +#define HECC_CANMC_SRES BIT(5) /* Software reset */ > + > +#define HECC_CANTIOC_EN BIT(3) /* Enable CAN TX I/O pin */ > +#define HECC_CANRIOC_EN BIT(3) /* Enable CAN RX I/O pin */ > + > +#define HECC_CANMID_IDE BIT(31) /* Extended frame format */ > +#define HECC_CANMID_AME BIT(30) /* Acceptance mask enable */ > +#define HECC_CANMID_AAM BIT(29) /* Auto answer mode */ > + > +#define HECC_CANES_FE BIT(24) /* form error */ > +#define HECC_CANES_BE BIT(23) /* bit error */ > +#define HECC_CANES_SA1 BIT(22) /* stuck at dominant error */ > +#define HECC_CANES_CRCE BIT(21) /* CRC error */ > +#define HECC_CANES_SE BIT(20) /* stuff bit error */ > +#define HECC_CANES_ACKE BIT(19) /* ack error */ > +#define HECC_CANES_BO BIT(18) /* Bus off status */ > +#define HECC_CANES_EP BIT(17) /* Error passive status */ > +#define HECC_CANES_EW BIT(16) /* Error warning status */ > +#define HECC_CANES_SMA BIT(5) /* suspend mode ack */ > +#define HECC_CANES_CCE BIT(4) /* Change config enabled */ > +#define HECC_CANES_PDA BIT(3) /* Power down mode ack */ > + > +#define HECC_CANBTC_SAM BIT(7) /* sample points */ > + > +#define HECC_BUS_ERROR (HECC_CANES_FE | HECC_CANES_BE |\ > + HECC_CANES_CRCE | HECC_CANES_SE |\ > + HECC_CANES_ACKE) > + > +#define HECC_CANMCF_RTR BIT(4) /* Remote xmit request */ > + > +#define HECC_CANGIF_MAIF BIT(17) /* Message alarm interrupt */ > +#define HECC_CANGIF_TCOIF BIT(16) /* Timer counter overflow int */ > +#define HECC_CANGIF_GMIF BIT(15) /* Global mailbox interrupt */ > +#define HECC_CANGIF_AAIF BIT(14) /* Abort ack interrupt */ > +#define HECC_CANGIF_WDIF BIT(13) /* Write denied interrupt */ > +#define HECC_CANGIF_WUIF BIT(12) /* Wake up interrupt */ > +#define HECC_CANGIF_RMLIF BIT(11) /* Receive message lost interrupt */ > +#define HECC_CANGIF_BOIF BIT(10) /* Bus off interrupt */ > +#define HECC_CANGIF_EPIF BIT(9) /* Error passive interrupt */ > +#define HECC_CANGIF_WLIF BIT(8) /* Warning level interrupt */ > +#define HECC_CANGIF_MBOX_MASK 0x1F /* Mailbox number mask */ > +#define HECC_CANGIM_I1EN BIT(1) /* Int line 1 enable */ > +#define HECC_CANGIM_I0EN BIT(0) /* Int line 0 enable */ > +#define HECC_CANGIM_DEF_MASK 0x700 /* only busoff/warning/passive */ > +#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 = { > + .name = DRV_NAME, > + .tseg1_min = 1, > + .tseg1_max = 16, > + .tseg2_min = 1, > + .tseg2_max = 8, > + .sjw_max = 4, > + .brp_min = 1, > + .brp_max = 256, > + .brp_inc = 1, > +}; > + > +struct ti_hecc_priv { > + struct can_priv can; /* MUST be first member/field */ > + struct napi_struct napi; > + struct net_device *ndev; > + struct clk *clk; > + void __iomem *base; > + u32 scc_ram_offset; > + u32 hecc_ram_offset; > + u32 mbox_offset; > + u32 int_line; > + spinlock_t mbox_lock; /* CANME register needs protection */ > + u32 tx_head; > + u32 tx_tail; > + u32 rx_next; > +}; > + > +static inline int get_tx_head_mb(struct ti_hecc_priv *priv) > +{ > + return priv->tx_head & HECC_TX_MB_MASK; > +} > + > +static inline int get_tx_tail_mb(struct ti_hecc_priv *priv) > +{ > + return priv->tx_tail & HECC_TX_MB_MASK; > +} > + > +static inline int get_tx_head_prio(struct ti_hecc_priv *priv) > +{ > + return (priv->tx_head >> HECC_TX_PRIO_SHIFT) & 0x3F; > +} > + > +static inline > +void hecc_write(struct ti_hecc_priv *priv, int reg, u32 val) > +{ > + __raw_writel(val, priv->base + reg); > +} > + > +static inline u32 hecc_read(struct ti_hecc_priv *priv, int reg) > +{ > + return __raw_readl(priv->base + reg); > +} > + > +static inline > +void hecc_set_bit(struct ti_hecc_priv *priv, int reg, unsigned bit) > +{ > + hecc_write(priv, reg, (hecc_read(priv, reg) | bit)); > +} I would expect the argument bit to be the bit number, but actually it's a mask which is applied. > +static inline > +void hecc_clear_bit(struct ti_hecc_priv *priv, int reg, unsigned bit) > +{ > + hecc_write(priv, reg, (hecc_read(priv, reg) & ~bit)); > +} > + > +static inline > +u32 hecc_get_bit(struct ti_hecc_priv *priv, int reg, int bit) Please use a consistant style for breaking the lines of function declarations... > +{ > + return (hecc_read(priv, reg) & bit) ? 1 : 0; > +} > + > +static int ti_hecc_get_state(const struct net_device *ndev, > + enum can_state *state) ... I personally prefer this one. > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + *state = priv->can.state; > + return 0; > +} > + > +static int ti_hecc_set_btc(struct ti_hecc_priv *priv) > +{ > + struct can_bittiming *bit_timing = &priv->can.bittiming; > + u32 can_btc = 0; I think there is no need to pre-set the variable above. > + > + can_btc = ((bit_timing->phase_seg2 - 1) & 0x7); > + can_btc |= (((bit_timing->phase_seg1 + bit_timing->prop_seg - 1) > + & 0xF) << 3); > + if ((bit_timing->brp > 4) && > + (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)) > + can_btc |= HECC_CANBTC_SAM; It would be nice to print a warning if triple sampling could not be set due to hardware restrictions. > + can_btc |= (((bit_timing->sjw - 1) & 0x3) << 8); > + can_btc |= (((bit_timing->brp - 1) & 0xFF) << 16); > + > + /* ERM being set to 0 by default meaning resync at falling edge */ > + > + hecc_write(priv, HECC_CANBTC, can_btc); > + dev_info(priv->ndev->dev.parent, "setting CANBTC=%#x\n", can_btc); > + > + return 0; > +} > + > +static void ti_hecc_reset(struct net_device *ndev) > +{ > + u32 cnt; > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + > + dev_dbg(ndev->dev.parent, "resetting hecc ...\n"); > + hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_SRES); > + > + /* Set change control request and wait till enabled */ > + hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_CCR); > + > + /* INFO: It has been observed that at times CCE bit may not be > + * set and hw seems to be ok even if this bit is not set so > + * timing out with a timing of 1ms to respect the specs > + */ > + cnt = HECC_CCE_WAIT_COUNT; > + while (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE) && (0 != cnt)) { > + --cnt; > + udelay(10); > + } > + > + /* Note: On HECC, BTC can be programmed only in initialization mode, so > + * it is expected that the can bittiming parameters are set via ip > + * utility before the device is opened > + */ > + ti_hecc_set_btc(priv); > + > + /* Clear CCR (and CANMC register) and wait for CCE = 0 enable */ > + hecc_write(priv, HECC_CANMC, 0); > + > + /* INFO: CAN net stack handles bus off and hence disabling auto-bus-on > + * hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_ABO); > + */ > + > + /* INFO: It has been observed that at times CCE bit may not be > + * set and hw seems to be ok even if this bit is not set so > + */ > + cnt = HECC_CCE_WAIT_COUNT; > + while (hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE) && (0 != cnt)) { > + --cnt; > + udelay(10); > + } > + > + /* Enable TX and RX I/O Control pins */ > + hecc_write(priv, HECC_CANTIOC, HECC_CANTIOC_EN); > + hecc_write(priv, HECC_CANRIOC, HECC_CANRIOC_EN); > + > + /* Clear registers for clean operation */ > + hecc_write(priv, HECC_CANTA, HECC_SET_REG); > + hecc_write(priv, HECC_CANRMP, HECC_SET_REG); > + hecc_write(priv, HECC_CANGIF0, HECC_SET_REG); > + hecc_write(priv, HECC_CANGIF1, HECC_SET_REG); > + hecc_write(priv, HECC_CANME, 0); > + hecc_write(priv, HECC_CANMD, 0); > + > + /* SCC compat mode NOT supported (and not needed too) */ > + hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_SCM); > +} > + > +static void ti_hecc_start(struct net_device *ndev) > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + int cnt, mbxno; > + > + /* put HECC in initialization mode and set btc */ > + ti_hecc_reset(ndev); > + > + priv->tx_head = priv->tx_tail = HECC_TX_MASK; > + priv->rx_next = HECC_RX_FIRST_MBOX; > + > + /* Enable local and global acceptance mask registers */ > + hecc_write(priv, HECC_CANGAM, HECC_SET_REG); > + hecc_write(priv, HECC_CANLAM0, HECC_SET_REG); > + hecc_write(priv, HECC_CANLAM3, HECC_SET_REG); > + > + /* Prepare configured mailboxes to receive messages */ > + for (cnt = 0; cnt < HECC_MAX_RX_MBOX; cnt++) { > + mbxno = (HECC_MAX_MAILBOXES - 1 - cnt); > + hecc_clear_bit(priv, HECC_CANME, (1 << mbxno)); > + hecc_write(priv, HECC_CANMID(mbxno), HECC_CANMID_AME); > + hecc_write(priv, HECC_CANLAM(mbxno), HECC_SET_REG); > + hecc_set_bit(priv, HECC_CANMD, (1 << mbxno)); > + hecc_set_bit(priv, HECC_CANME, (1 << mbxno)); > + hecc_set_bit(priv, HECC_CANMIM, (1 << mbxno)); > + } > + > + /* Prevent message over-write & Enable interrupts */ > + hecc_write(priv, HECC_CANOPC, HECC_SET_REG); > + if (priv->int_line) { > + hecc_write(priv, HECC_CANMIL, HECC_SET_REG); > + hecc_write(priv, HECC_CANGIM, (HECC_CANGIM_DEF_MASK | > + HECC_CANGIM_I1EN | HECC_CANGIM_SIL)); > + } else { > + hecc_write(priv, HECC_CANMIL, 0); > + hecc_write(priv, HECC_CANGIM, > + (HECC_CANGIM_DEF_MASK | HECC_CANGIM_I0EN)); > + } > + priv->can.state = CAN_STATE_ERROR_ACTIVE; > +} > + > +static void ti_hecc_stop(struct net_device *ndev) > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + > + /* Disable interrupts and disable mailboxes */ > + hecc_write(priv, HECC_CANGIM, 0); > + hecc_write(priv, HECC_CANMIM, 0); > + hecc_write(priv, HECC_CANME, 0); > + priv->can.state = CAN_STATE_STOPPED; > +} > + > +static int ti_hecc_do_set_mode(struct net_device *ndev, enum can_mode mode) > +{ > + int ret = 0; > + > + switch (mode) { > + case CAN_MODE_START: > + ti_hecc_start(ndev); > + netif_wake_queue(ndev); > + break; > + default: > + ret = -EOPNOTSUPP; > + break; > + } > + > + return ret; > +} > + > +/** /* ? > + * ti_hecc_xmit: HECC Transmit > + * > + * The transmit mailboxes start from 0 to HECC_MAX_TX_MBOX. In HECC the > + * priority of the mailbox for tranmission is dependent upon priority setting > + * field in mailbox registers. The mailbox with highest value in priority > field > + * is transmitted first. Only when two mailboxes have the same value in > + * priority field the highest numbered mailbox is transmitted first. > + * > + * To utilize the HECC priority feature as described above we start with the > + * highest numbered mailbox with highest priority level and move on to the > next > + * mailbox with the same priority level and so on. Once we loop through all > the > + * transmit mailboxes we choose the next priority level (lower) and so on > + * until we reach the lowest priority level on the lowest numbered mailbox > + * when we stop transmission until all mailboxes are transmitted and then > + * restart at highest numbered mailbox with highest priority. > + * > + * Two counters (head and tail) are used to track the next mailbox to > transmit > + * and to track the echo buffer for already transmitted mailbox. The queue > + * is stopped when all the mailboxes are busy or when there is a priority > + * value roll-over happens. > + */ > +static netdev_tx_t ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev) > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + struct can_frame *cf = (struct can_frame *)skb->data; > + u32 mbxno = 0; > + u32 data; > + unsigned long flags; > + > + mbxno = get_tx_head_mb(priv); > + spin_lock_irqsave(&priv->mbox_lock, flags); > + if (unlikely(hecc_read(priv, HECC_CANME) & (1 << mbxno))) { > + spin_unlock_irqrestore(&priv->mbox_lock, flags); > + netif_stop_queue(ndev); > + dev_err(priv->ndev->dev.parent, > + "BUG: TX mbox not ready tx_head=%08X, tx_tail=%08X\n", > + priv->tx_head, priv->tx_tail); > + return NETDEV_TX_BUSY; Can this happen? Will the hardware recover from the error? > + } > + spin_unlock_irqrestore(&priv->mbox_lock, flags); > + > + /* Prepare mailbox for transmission */ > + data = cf->can_dlc & 0xF; > + if (cf->can_id & CAN_RTR_FLAG) /* Remote transmission request */ > + data |= HECC_CANMCF_RTR; > + data |= (get_tx_head_prio(priv) << 8); > + hecc_write(priv, HECC_CANMCF(mbxno), data); > + > + if (cf->can_id & CAN_EFF_FLAG) /* Extended frame format */ > + data = ((cf->can_id & CAN_EFF_MASK) | HECC_CANMID_IDE); > + else /* Standard frame format */ > + data = ((cf->can_id & CAN_SFF_MASK) << 18); > + hecc_write(priv, HECC_CANMID(mbxno), data); > + hecc_write(priv, HECC_CANMDL(mbxno), > + be32_to_cpu(*(u32 *)(cf->data + 0))); Please remove "+ 0". > + if (cf->can_dlc > 4) { > + hecc_write(priv, HECC_CANMDH(mbxno), > + be32_to_cpu(*(u32 *)(cf->data + 4))); > + } else { > + *(u32 *)(cf->data + 4) = 0; > + } > + can_put_echo_skb(skb, ndev, mbxno); > + > + spin_lock_irqsave(&priv->mbox_lock, flags); > + --priv->tx_head; > + if ((hecc_read(priv, HECC_CANME) & (1 << get_tx_head_mb(priv))) || > + (priv->tx_head & HECC_TX_MASK) == HECC_TX_MASK) { > + netif_stop_queue(ndev); > + } > + hecc_set_bit(priv, HECC_CANME, (1 << mbxno)); > + spin_unlock_irqrestore(&priv->mbox_lock, flags); > + > + hecc_clear_bit(priv, HECC_CANMD, (1 << mbxno)); > + hecc_set_bit(priv, HECC_CANMIM, (1 << mbxno)); > + hecc_write(priv, HECC_CANTRS, (1 << mbxno)); > + > + return NETDEV_TX_OK; > +} > + > +static int ti_hecc_rx_pkt(struct ti_hecc_priv *priv, int mbxno) > +{ > + struct net_device_stats *stats = &priv->ndev->stats; > + struct can_frame *cf; > + struct sk_buff *skb; > + u32 data; > + unsigned long flags; > + > + skb = dev_alloc_skb(sizeof(struct can_frame)); Please use netdev_alloc_skb(priv->ndev, ....) making ... > + if (!skb) { > + if (printk_ratelimit()) > + dev_err(priv->ndev->dev.parent, > + "dev_alloc_skb() failed\n"); > + return -ENOMEM; > + } > + skb->dev = priv->ndev; ... the line above obsolete. > + skb->protocol = htons(ETH_P_CAN); > + skb->ip_summed = CHECKSUM_UNNECESSARY; > + > + cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame)); > + data = hecc_read(priv, HECC_CANMID(mbxno)); > + if (data & HECC_CANMID_IDE) > + cf->can_id = (data & CAN_EFF_MASK) | CAN_EFF_FLAG; > + else > + cf->can_id = ((data >> 18) & CAN_SFF_MASK); > + data = hecc_read(priv, HECC_CANMCF(mbxno)); > + if (data & HECC_CANMCF_RTR) > + cf->can_id |= CAN_RTR_FLAG; > + cf->can_dlc = data & 0xF; > + data = hecc_read(priv, HECC_CANMDL(mbxno)); > + *(u32 *)(cf->data + 0) = cpu_to_be32(data); Please remove "+ 0". > + if (cf->can_dlc > 4) { > + data = hecc_read(priv, HECC_CANMDH(mbxno)); > + *(u32 *)(cf->data + 4) = cpu_to_be32(data); > + } else { > + *(u32 *)(cf->data + 4) = 0; > + } > + spin_lock_irqsave(&priv->mbox_lock, flags); > + hecc_clear_bit(priv, HECC_CANME, (1 << mbxno)); > + hecc_write(priv, HECC_CANRMP, (1 << mbxno)); > + /* enable mailbox only if it is part of rx buffer mailboxes */ > + if (priv->rx_next < HECC_RX_BUFFER_MBOX) > + hecc_set_bit(priv, HECC_CANME, (1 << mbxno)); > + spin_unlock_irqrestore(&priv->mbox_lock, flags); > + > + stats->rx_bytes += cf->can_dlc; > + netif_receive_skb(skb); > + stats->rx_packets++; > + > + return 0; > +} > + > +/** /* ? > + * ti_hecc_rx_poll - HECC receive pkts > + * > + * The receive mailboxes start from highest numbered mailbox till last xmit > + * mailbox. On CAN frame reception the hardware places the data into highest > + * numbered mailbox that matches the CAN ID filter. Since all receive > mailboxes > + * have same filtering (ALL CAN frames) packets will arrive in the highest > + * available RX mailbox and we need to ensure in-order packet reception. > + * > + * To ensure the packets are received in the right order we logically divide > + * the RX mailboxes into main and buffer mailboxes. Packets are received as > per > + * mailbox priotity (higher to lower) in the main bank and once it is full we > + * disable further reception into main mailboxes. While the main mailboxes > are > + * processed in NAPI, further packets are received in buffer mailboxes. > + * > + * We maintain a RX next mailbox counter to process packets and once all main > + * mailboxe packets are passed to the upper stack we enable all of them but > + * continue to process packets received in buffer mailboxes. With each packet > + * received from buffer mailbox we enable it immediately so as to handle the > + * overflow from higher mailboxes. > + */ > +static int ti_hecc_rx_poll(struct napi_struct *napi, int quota) > +{ > + struct net_device *ndev = napi->dev; > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + u32 num_pkts = 0; > + u32 mbxno, mbx_mask; > + unsigned long pending_pkts, flags; > + > + if (!netif_running(ndev)) > + return 0; > + > + while ((pending_pkts = hecc_read(priv, HECC_CANRMP)) && > + (num_pkts < quota)) { > + mbx_mask = BIT(priv->rx_next); /* next rx mailbox to process */ > + if (mbx_mask & pending_pkts) { > + if (ti_hecc_rx_pkt(priv, priv->rx_next) < 0) > + return num_pkts; > + ++num_pkts; > + } else if (priv->rx_next > HECC_RX_BUFFER_MBOX) { > + break; /* pkt not received yet */ > + } > + --priv->rx_next; > + if (priv->rx_next == HECC_RX_BUFFER_MBOX) { > + /* enable high bank mailboxes */ > + spin_lock_irqsave(&priv->mbox_lock, flags); > + mbx_mask = hecc_read(priv, HECC_CANME); > + mbx_mask |= HECC_RX_HIGH_MBOX_MASK; > + hecc_write(priv, HECC_CANME, mbx_mask); > + spin_unlock_irqrestore(&priv->mbox_lock, flags); > + } else if (priv->rx_next == (HECC_MAX_TX_MBOX - 1)) { > + priv->rx_next = HECC_RX_FIRST_MBOX; > + break; > + } > + } > + > + /* Enable packet interrupt if all pkts are handled */ > + if (0 == hecc_read(priv, HECC_CANRMP)) { You seem to be a friend of this style. I personally find if (hecc_read(priv, HECC_CANRMP) == 0) or if (!hecc_read(priv, HECC_CANRMP)) more readable. But it's not a real requirement, AFAIK. > + napi_complete(napi); > + /* Re-enable RX mailbox interrupts */ > + mbxno = hecc_read(priv, HECC_CANMIM); > + mbxno |= HECC_TX_MBOX_MASK; > + hecc_write(priv, HECC_CANMIM, mbxno); > + } > + > + return num_pkts; > +} > + > +static int > +ti_hecc_error(struct net_device *ndev, int int_status, int err_status) > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &ndev->stats; > + struct can_frame *cf; > + struct sk_buff *skb; > + > + /* propogate the error condition to the can stack */ > + skb = dev_alloc_skb(sizeof(struct can_frame)); See above. > + if (!skb) { > + if (printk_ratelimit()) > + dev_err(priv->ndev->dev.parent, > + "dev_alloc_skb() failed in err processing\n"); > + return -ENOMEM; > + } > + skb->dev = ndev; > + skb->protocol = htons(ETH_P_CAN); Please add: 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; > + cf->can_dlc = CAN_ERR_DLC; > + > + if (int_status & HECC_CANGIF_WLIF) { /* warning level int */ > + if (0 == (int_status & HECC_CANGIF_BOIF)) { > + priv->can.state = CAN_STATE_ERROR_WARNING; > + ++priv->can.can_stats.error_warning; > + cf->can_id |= CAN_ERR_CRTL; > + if (hecc_read(priv, HECC_CANTEC) > 96) > + cf->data[1] |= CAN_ERR_CRTL_TX_WARNING; > + if (hecc_read(priv, HECC_CANREC) > 96) > + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING; > + } > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_EW); > + dev_dbg(priv->ndev->dev.parent, "Error Warning interrupt\n"); > + hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR); > + } > + > + if (int_status & HECC_CANGIF_EPIF) { /* error passive int */ > + if (0 == (int_status & HECC_CANGIF_BOIF)) { > + priv->can.state = CAN_STATE_ERROR_PASSIVE; > + ++priv->can.can_stats.error_passive; > + cf->can_id |= CAN_ERR_CRTL; > + if (hecc_read(priv, HECC_CANTEC) > 127) > + cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE; > + if (hecc_read(priv, HECC_CANREC) > 127) > + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE; > + } > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_EP); > + dev_dbg(priv->ndev->dev.parent, "Error passive interrupt\n"); > + hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR); > + } > + > + /* Need to check busoff condition in error status register too to > + * ensure warning interrupts don't hog the system > + */ > + if ((int_status & HECC_CANGIF_BOIF) || (err_status & HECC_CANES_BO)) { > + priv->can.state = CAN_STATE_BUS_OFF; > + cf->can_id |= CAN_ERR_BUSOFF; > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_BO); > + hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_CCR); > + can_bus_off(ndev); > + /* Disable all interrupts in bus-off to avoid int hog */ > + hecc_write(priv, HECC_CANGIM, 0); The interrupts should problable be disabled before can_bus_off() is called (which may start a timer to schedule recovery). > + } > + > + if (err_status & HECC_BUS_ERROR) { > + ++priv->can.can_stats.bus_error; > + cf->can_id |= (CAN_ERR_BUSERROR | CAN_ERR_PROT); > + cf->data[2] |= CAN_ERR_PROT_UNSPEC; > + if (err_status & HECC_CANES_FE) { > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_FE); > + cf->data[2] |= CAN_ERR_PROT_FORM; > + } > + if (err_status & HECC_CANES_BE) { > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_BE); > + cf->data[2] |= CAN_ERR_PROT_BIT; > + } > + if (err_status & HECC_CANES_SE) { > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_SE); > + cf->data[2] |= CAN_ERR_PROT_STUFF; > + } > + if (err_status & HECC_CANES_CRCE) { > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_CRCE); > + cf->data[2] |= (CAN_ERR_PROT_LOC_CRC_SEQ | > + CAN_ERR_PROT_LOC_CRC_DEL); > + } > + if (err_status & HECC_CANES_ACKE) { > + hecc_set_bit(priv, HECC_CANES, HECC_CANES_ACKE); > + cf->data[2] |= (CAN_ERR_PROT_LOC_ACK | > + CAN_ERR_PROT_LOC_ACK_DEL); > + } > + } > + > + netif_receive_skb(skb); > + ndev->last_rx = jiffies; Please remove the line above. It's not needed any more. > + stats->rx_packets++; > + stats->rx_bytes += cf->can_dlc; > + return 0; > +} > + > +static irqreturn_t ti_hecc_interrupt(int irq, void *dev_id) > +{ > + struct net_device *ndev = (struct net_device *)dev_id; > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &ndev->stats; > + u32 mbxno, int_status, err_status; > + unsigned long ack, flags; > + > + int_status = hecc_read(priv, > + ((priv->int_line) ? HECC_CANGIF1 : HECC_CANGIF0)); > + > + if (0 == int_status) > + return IRQ_NONE; > + > + err_status = hecc_read(priv, HECC_CANES); > + if (err_status & (HECC_BUS_ERROR | HECC_CANES_BO | > + HECC_CANES_EP | HECC_CANES_EW)) > + ti_hecc_error(ndev, int_status, err_status); > + > + if (int_status & HECC_CANGIF_GMIF) { > + while ((priv->tx_tail - priv->tx_head) > 0) { > + mbxno = get_tx_tail_mb(priv); > + if (!((1 << mbxno) & hecc_read(priv, HECC_CANTA))) > + break; > + hecc_clear_bit(priv, HECC_CANMIM, (1 << mbxno)); > + hecc_write(priv, HECC_CANTA, (1 << mbxno)); > + spin_lock_irqsave(&priv->mbox_lock, flags); > + hecc_clear_bit(priv, HECC_CANME, (1 << mbxno)); > + spin_unlock_irqrestore(&priv->mbox_lock, flags); > + stats->tx_bytes += (hecc_read(priv, HECC_CANMCF(mbxno)) > + & 0xF); > + stats->tx_packets++; > + can_get_echo_skb(ndev, mbxno); > + --priv->tx_tail; > + } > + > + /* restart queue if wrap-up or if queue stalled on last pkt */ > + if (((priv->tx_head == priv->tx_tail) && > + ((priv->tx_head & HECC_TX_MASK) != HECC_TX_MASK)) || > + (((priv->tx_tail & HECC_TX_MASK) == HECC_TX_MASK) && > + ((priv->tx_head & HECC_TX_MASK) == HECC_TX_MASK))) > + netif_wake_queue(ndev); > + > + /* Disable RX mailbox interrupts and let NAPI reenable them */ > + if (hecc_read(priv, HECC_CANRMP)) { > + ack = hecc_read(priv, HECC_CANMIM); > + ack &= ((1 << HECC_MAX_TX_MBOX) - 1); > + hecc_write(priv, HECC_CANMIM, ack); > + napi_schedule(&priv->napi); > + } > + } > + > + /* clear all interrupt conditions - read back to avoid spurious ints */ > + if (priv->int_line) { > + hecc_write(priv, HECC_CANGIF1, HECC_SET_REG); > + int_status = hecc_read(priv, HECC_CANGIF1); > + } else { > + hecc_write(priv, HECC_CANGIF0, HECC_SET_REG); > + int_status = hecc_read(priv, HECC_CANGIF0); > + } > + > + return IRQ_HANDLED; > +} > + > +static int ti_hecc_open(struct net_device *ndev) > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + int err; > + > + err = request_irq(ndev->irq, ti_hecc_interrupt, IRQF_DISABLED, > + ndev->name, ndev); > + if (err) { > + dev_err(ndev->dev.parent, "error requesting interrupt\n"); > + return err; > + } > + > + /* Open common can device */ > + err = open_candev(ndev); > + if (err) { > + dev_err(ndev->dev.parent, "open_candev() failed %08X\n", err); > + free_irq(ndev->irq, ndev); > + return err; > + } > + > + clk_enable(priv->clk); > + ti_hecc_start(ndev); > + napi_enable(&priv->napi); > + netif_start_queue(ndev); > + > + return 0; > +} > + > +static int ti_hecc_close(struct net_device *ndev) > +{ > + struct ti_hecc_priv *priv = netdev_priv(ndev); > + > + netif_stop_queue(ndev); > + napi_disable(&priv->napi); > + ti_hecc_stop(ndev); > + free_irq(ndev->irq, ndev); > + clk_disable(priv->clk); > + close_candev(ndev); > + > + return 0; > +} > + > +static const struct net_device_ops ti_hecc_netdev_ops = { > + .ndo_open = ti_hecc_open, > + .ndo_stop = ti_hecc_close, > + .ndo_start_xmit = ti_hecc_xmit, > +}; > + > +static int ti_hecc_probe(struct platform_device *pdev) > +{ > + struct net_device *ndev = (struct net_device *)0; > + struct ti_hecc_priv *priv; > + struct ti_hecc_platform_data *pdata; > + struct resource *mem, *irq; > + void __iomem *addr; > + int err; > + > + pdata = pdev->dev.platform_data; > + if (!pdata) { > + dev_err(&pdev->dev, "No platform data - exiting\n"); s/- exiting// > + return -ENODEV; Here you use "return", and below "goto probe_exit"; > + } > + > + mem = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + if (!mem) { > + dev_err(&pdev->dev, "no mem resources???\n"); Please remove "???" > + err = -ENODEV; Pre-setting err would also save some lines. > + goto probe_exit; > + } > + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); > + if (!irq) { > + dev_err(&pdev->dev, "no irq resourc???\n"); Ditto. > + err = -ENODEV; > + goto probe_exit; > + } > + if (!request_mem_region(mem->start, resource_size(mem), pdev->name)) { > + dev_err(&pdev->dev, "HECC region already claimed\n"); > + err = -EBUSY; > + goto probe_exit; > + } > + addr = ioremap(mem->start, resource_size(mem)); > + if (!addr) { > + dev_err(&pdev->dev, "ioremap failed\n"); > + err = -ENOMEM; > + goto probe_exit_free_region; > + } > + > + ndev = alloc_candev(sizeof(struct ti_hecc_priv)); > + if (!ndev) { > + dev_err(&pdev->dev, "alloc_candev failed\n"); > + err = -ENOMEM; > + goto probe_exit_iounmap; > + } > + > + priv = netdev_priv(ndev); > + priv->ndev = ndev; > + priv->base = addr; > + priv->scc_ram_offset = pdata->scc_ram_offset; > + priv->hecc_ram_offset = pdata->hecc_ram_offset; > + priv->mbox_offset = pdata->mbox_offset; > + priv->int_line = pdata->int_line; > + > + priv->can.bittiming_const = &ti_hecc_bittiming_const; > + priv->can.do_set_mode = ti_hecc_do_set_mode; > + priv->can.do_get_state = ti_hecc_get_state; > + > + ndev->irq = irq->start; > + ndev->flags |= IFF_ECHO; > + platform_set_drvdata(pdev, ndev); > + SET_NETDEV_DEV(ndev, &pdev->dev); > + ndev->netdev_ops = &ti_hecc_netdev_ops; > + > + /* Note: clk name would change using hecc_vbusp_ck temporarily */ > + priv->clk = clk_get(&pdev->dev, "hecc_vbusp_ck"); > + if (IS_ERR(priv->clk)) { > + dev_err(&pdev->dev, "no clock available\n"); > + err = PTR_ERR(priv->clk); > + priv->clk = NULL; > + goto probe_exit_candev; > + } > + priv->can.clock.freq = clk_get_rate(priv->clk); > + netif_napi_add(ndev, &priv->napi, ti_hecc_rx_poll, > + HECC_DEF_NAPI_WEIGHT); > + > + err = register_candev(ndev); > + if (err) { > + dev_err(&pdev->dev, "register_candev() failed\n"); > + err = -ENODEV; > + goto probe_exit_clk; > + } > + dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%u)\n", > + priv->base, (u32) ndev->irq); > + > + return 0; > + > +probe_exit_clk: > + clk_put(priv->clk); > +probe_exit_candev: > + free_candev(ndev); > +probe_exit_iounmap: > + iounmap(addr); > +probe_exit_free_region: > + release_mem_region(mem->start, resource_size(mem)); > +probe_exit: > + return err; > +} [snip] Wolfgang. _______________________________________________ Socketcan-core mailing list [email protected] https://lists.berlios.de/mailman/listinfo/socketcan-core
