netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] net-next:can: add TI CAN (HECC) driver
@ 2009-08-28 11:18 Anant Gole
  2009-08-28 12:44 ` Oliver Hartkopp
  2009-09-01  9:04 ` Wolfram Sang
  0 siblings, 2 replies; 13+ messages in thread
From: Anant Gole @ 2009-08-28 11:18 UTC (permalink / raw)
  To: socketcan-core; +Cc: netdev, anantgole

TI HECC (High End CAN Controller) module is found on many TI devices. It has
32 harwdare mailboxes with full implementation of CAN protocol version 2.0B
and bus speeds up to 1Mbps. The module specifications are available at TI web
<http://www.ti.com>.

This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.

Signed-off-by: Anant Gole <anantgole@ti.com>
---
 drivers/net/can/Kconfig                       |    9 +
 drivers/net/can/Makefile                      |    2 +
 drivers/net/can/ti_hecc.c                     | 1352 +++++++++++++++++++++++++
 include/linux/can/platform/ti_hecc_platform.h |   40 +
 4 files changed, 1403 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/ti_hecc.c
 create mode 100644 include/linux/can/platform/ti_hecc_platform.h

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 30ae55d..fae62df 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -75,6 +75,15 @@ 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 (HECC)"
+        default N
+        ---help---
+	  This driver adds support for TI High End CAN Controller module
+	  found on many TI devices. The specifications of this module are
+	  are available from TI web (http://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..d923512 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -9,4 +9,6 @@ can-dev-y			:= dev.o
 
 obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
 
+obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
+
 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..4741b4a
--- /dev/null
+++ b/drivers/net/can/ti_hecc.c
@@ -0,0 +1,1352 @@
+/*
+ * 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.
+ *
+ */
+
+#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>
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/can/platform/ti_hecc_platform.h>
+
+#define DRV_NAME "TI HECC"
+#define HECC_MODULE_VERSION     "0.2"
+MODULE_VERSION(HECC_MODULE_VERSION);
+#define DRV_DESC "TI High End CAN Controller Driver " HECC_MODULE_VERSION
+
+#define HECC_MAX_MAILBOXES	32	/* hardware mboxes - do not change */
+
+#if (CAN_ECHO_SKB_MAX > 16)
+#define TI_HECC_MAX_TX_MBOX	16
+#else
+#define TI_HECC_MAX_TX_MBOX	CAN_ECHO_SKB_MAX
+#endif
+#define TI_HECC_MAX_RX_MBOX	(HECC_MAX_MAILBOXES - TI_HECC_MAX_TX_MBOX)
+
+#define TI_HECC_DEF_NAPI_WEIGHT	TI_HECC_MAX_RX_MBOX
+
+/* 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)
+
+#define HECC_SET_REG		0xFFFFFFFF
+#define HECC_CANID_MASK		0x3FF	/* 18 bits mask for extended id's */
+
+#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	0xFF00	/* all except maif and tcoif */
+#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;
+	struct napi_struct napi;
+	struct net_device *ndev;
+	struct clk *clk;
+	void __iomem *base;
+	unsigned int scc_ram_offset;
+	unsigned int hecc_ram_offset;
+	unsigned int mbox_offset;
+	unsigned int int_line;
+	DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
+	spinlock_t tx_lock;
+
+	/* Statistics */
+	unsigned out_of_tx_mbox;
+	unsigned write_denied_cnt;
+	unsigned message_lost_cnt;
+	unsigned wake_up_cnt;
+	unsigned message_alarm_cnt;
+	unsigned timer_overflow_cnt;
+};
+
+static inline
+void hecc_write(struct ti_hecc_priv *priv, int reg, unsigned int val)
+{
+	__raw_writel(val, priv->base + reg);
+}
+
+static inline unsigned int 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));
+}
+
+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
+unsigned int hecc_get_bit(struct ti_hecc_priv *priv, int reg, int bit)
+{
+	return (hecc_read(priv, reg) & bit) ? 1 : 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+
+static struct ti_hecc_priv *debug_priv;
+
+#define PRINTMBOXREG(r) seq_printf(s, "%d\t%08X %08X %08X %08X %08X\n", r,\
+	hecc_read(debug_priv, HECC_CANMID(r)),\
+	hecc_read(debug_priv, HECC_CANMCF(r)),\
+	hecc_read(debug_priv, HECC_CANMDH(r)),\
+	hecc_read(debug_priv, HECC_CANMDL(r)),\
+	hecc_read(debug_priv, HECC_CANLAM(r)))
+
+/* Print mailbox data */
+static void hecc_print_mbox_regs(struct seq_file *s)
+{
+	int cnt = 0;
+	static struct ti_hecc_priv *priv;
+	priv = debug_priv;
+	seq_printf(s, "\n--- %s %s - mailbox regs ---\n\n",
+		DRV_NAME, HECC_MODULE_VERSION);
+	seq_printf(s, "MbxNo\tMID\t MCF\t  MDH\t   MDL\t   LAM\n");
+	seq_printf(s, "-----------------------------------------------\n");
+	for (cnt = 0; cnt < HECC_MAX_MAILBOXES; cnt++)
+		PRINTMBOXREG(cnt);
+}
+
+#define PRINTREG(d, r) seq_printf(s, "%s\t%08x\n", d, hecc_read(debug_priv, r))
+/* Print HECC registers */
+static void hecc_print_regs(struct seq_file *s)
+{
+	static struct ti_hecc_priv *priv;
+	priv = debug_priv;
+
+	seq_printf(s, "\n--- %s %s - module regs ---\n\n",
+		DRV_NAME, HECC_MODULE_VERSION);
+	PRINTREG("HECC_CANME\tMailbox Enable\t", HECC_CANME);
+	PRINTREG("HECC_CANMD\tMailbox Direction", HECC_CANMD);
+	PRINTREG("HECC_CANTRS\tTransmit request set", HECC_CANTRS);
+	PRINTREG("HECC_CANTRR\tTransmit request reset", HECC_CANTRR);
+	PRINTREG("HECC_CANTA\tTransmission ack", HECC_CANTA);
+	PRINTREG("HECC_CANAA\tAbort acknowledge", HECC_CANAA);
+	PRINTREG("HECC_CANRMP\tReceive message pending", HECC_CANRMP);
+	PRINTREG("HECC_CANRML\tRemote message lost", HECC_CANRML);
+	PRINTREG("HECC_CANRFP\tRemote frame pending", HECC_CANRFP);
+	PRINTREG("HECC_CANGAM (SCC) Global accept mask", HECC_CANGAM);
+	PRINTREG("HECC_CANMC\tMaster control\t", HECC_CANMC);
+	PRINTREG("HECC_CANBTC\tBit timing config", HECC_CANBTC);
+	PRINTREG("HECC_CANES\tError and status", HECC_CANES);
+	PRINTREG("HECC_CANTEC\tTransmit error counter", HECC_CANTEC);
+	PRINTREG("HECC_CANREC\tReceive error counter", HECC_CANREC);
+	PRINTREG("HECC_CANGIF0\tGlobal interrupt flag 0", HECC_CANGIF0);
+	PRINTREG("HECC_CANGIM\tGlobal interrupt mask", HECC_CANGIM);
+	PRINTREG("HECC_CANGIF1\tGlobal interrupt flag 1", HECC_CANGIF1);
+	PRINTREG("HECC_CANMIM\tMailbox interrupt mask", HECC_CANMIM);
+	PRINTREG("HECC_CANMIL\tMailbox interrupt level", HECC_CANMIL);
+	PRINTREG("HECC_CANOPC\tOverwrite prot control", HECC_CANOPC);
+	PRINTREG("HECC_CANTIOC\tTransmit I/O control", HECC_CANTIOC);
+	PRINTREG("HECC_CANRIOC\tReceive I/O control", HECC_CANRIOC);
+	PRINTREG("HECC_CANLNT (HECC) Local network time", HECC_CANLNT);
+	PRINTREG("HECC_CANTOC (HECC) Time-out control", HECC_CANTOC);
+	PRINTREG("HECC_CANTOS (HECC) Time-out status", HECC_CANTOS);
+	PRINTREG("HECC_CANTIOCE (SCC) Enh TX I/O ctrl", HECC_CANTIOCE);
+	PRINTREG("HECC_CANRIOCE (SCC) Enh RX I/O ctrl", HECC_CANRIOCE);
+	seq_printf(s, "HECC_CANLAM0 (SCC) \t\t\t%08X\n",
+		hecc_read(priv, HECC_CANLAM0));
+	seq_printf(s, "HECC_CANLAM3 (SCC) \t\t\t%08X\n",
+		hecc_read(priv, HECC_CANLAM3));
+	seq_printf(s, "\n");
+}
+
+static char *hecc_debug_can_state[] = {
+	"CAN_STATE_ERROR_ACTIVE",
+	"CAN_STATE_ERROR_WARNING",
+	"CAN_STATE_ERROR_PASSIVE",
+	"CAN_STATE_BUS_OFF",
+	"CAN_STATE_STOPPED",
+	"CAN_STATE_SLEEPING",
+	"CAN_STATE_MAX"
+};
+
+/* Print status and statistics */
+static void hecc_print_status(struct seq_file *s)
+{
+	seq_printf(s, "\n--- %s %s - status ---\n\n",
+		DRV_NAME, HECC_MODULE_VERSION);
+	seq_printf(s, "\n--- ti_hecc status ---\n\n");
+	seq_printf(s, "CAN state \t\t= %s\n",
+		hecc_debug_can_state[debug_priv->can.state]);
+	seq_printf(s, "CAN restart_ms \t\t= %u\n", debug_priv->can.restart_ms);
+	seq_printf(s, "CAN input clock \t= %u\n", debug_priv->can.clock.freq);
+	seq_printf(s, "CAN Bittiming\n");
+	seq_printf(s, "\tbitrate \t= %u\n",
+			debug_priv->can.bittiming.bitrate);
+	seq_printf(s, "\tsample_point \t= %u\n",
+			debug_priv->can.bittiming.sample_point);
+	seq_printf(s, "\ttq \t\t= %u\n",
+			debug_priv->can.bittiming.tq);
+	seq_printf(s, "\tprop_seg \t= %u\n",
+			debug_priv->can.bittiming.prop_seg);
+	seq_printf(s, "\tphase_seg1 \t= %u\n",
+			debug_priv->can.bittiming.phase_seg1);
+	seq_printf(s, "\tphase_seg2 \t= %u\n",
+			debug_priv->can.bittiming.phase_seg2);
+	seq_printf(s, "\tsjw \t\t= %u\n",
+			debug_priv->can.bittiming.sjw);
+	seq_printf(s, "\tbrp \t\t= %u\n",
+			debug_priv->can.bittiming.brp);
+	seq_printf(s, "CAN Bittiming Constants\n");
+	seq_printf(s, "\ttseg1 min-max \t= %u-%u\n",
+			debug_priv->can.bittiming_const->tseg1_min,
+			debug_priv->can.bittiming_const->tseg1_max);
+	seq_printf(s, "\ttseg2 min-max \t= %u-%u\n",
+			debug_priv->can.bittiming_const->tseg2_min,
+			debug_priv->can.bittiming_const->tseg2_max);
+	seq_printf(s, "\tsjw_max \t= %u\n",
+			debug_priv->can.bittiming_const->sjw_max);
+	seq_printf(s, "\tbrp min-max \t= %u-%u\n",
+			debug_priv->can.bittiming_const->brp_min,
+			debug_priv->can.bittiming_const->brp_max);
+	seq_printf(s, "\tbrp_inc \t= %u\n",
+			debug_priv->can.bittiming_const->brp_inc);
+	seq_printf(s, "CAN out of TX mbox\t= %d\n",
+		debug_priv->out_of_tx_mbox);
+	seq_printf(s, "CAN write denied cnt \t= %d\n",
+		debug_priv->write_denied_cnt);
+	seq_printf(s, "CAN message lost cnt \t= %d\n",
+		debug_priv->message_lost_cnt);
+	seq_printf(s, "CAN wake up cnt \t= %d\n", debug_priv->wake_up_cnt);
+	seq_printf(s, "CAN message alarm cnt \t= %d\n",
+		debug_priv->message_alarm_cnt);
+	seq_printf(s, "CAN timer overflow cnt\t= %d\n",
+		debug_priv->timer_overflow_cnt);
+	/* CAN statistics */
+	seq_printf(s, "CAN stats bus error \t= %d\n",
+		debug_priv->can.can_stats.bus_error);
+	seq_printf(s, "CAN stats error warning\t= %d\n",
+		debug_priv->can.can_stats.error_warning);
+	seq_printf(s, "CAN stats error passive\t= %d\n",
+		debug_priv->can.can_stats.error_passive);
+	seq_printf(s, "CAN stats bus off\t= %d\n",
+		debug_priv->can.can_stats.bus_off);
+	seq_printf(s, "CAN stats arbitration lost= %d\n",
+		debug_priv->can.can_stats.arbitration_lost);
+	seq_printf(s, "CAN stats restarts\t= %d\n",
+		debug_priv->can.can_stats.restarts);
+}
+
+/** Toggle HECC Self-Test i.e loopback bit
+ * INFO: Reading this debug variable toggles the loopback status on the device.
+ * This is done to simplify the debug function to set looback
+ */
+static int hecc_debug_loopback(struct seq_file *s)
+{
+	static int toggle;
+
+	/* Put module in self test mode i.e. loopback */
+	if (toggle) {
+		seq_printf(s, "In Self Test Mode (loopback)\n");
+		hecc_set_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
+		toggle = 0;
+	} else {
+		seq_printf(s, "Out of Self Test Mode (NO loopback)\n");
+		hecc_clear_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
+		toggle = 1;
+	}
+
+	return 0;
+}
+
+static int hecc_debug_show(struct seq_file *s, void *unused)
+{
+	void (*func)(struct seq_file *) = s->private;
+	func(s);
+	return 0;
+}
+
+static int hecc_debug_open(struct inode *inode, struct file *file)
+{
+	return single_open(file, hecc_debug_show, inode->i_private);
+}
+
+/* HECC debug operations */
+static const struct file_operations hecc_debug_fops = {
+	.open		= hecc_debug_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= single_release,
+};
+
+static struct dentry *hecc_dir;
+static unsigned int hecc_debug_state;
+
+static int hecc_debug_init(struct ti_hecc_priv *priv)
+{
+	debug_priv = priv;
+
+	hecc_dir = debugfs_create_dir("ti_hecc", NULL);
+	if (IS_ERR(hecc_dir)) {
+		int err = PTR_ERR(hecc_dir);
+		hecc_dir = NULL;
+		return err;
+	}
+
+	debugfs_create_file("ti_hecc_regs", S_IRUGO,
+			hecc_dir, &hecc_print_regs, &hecc_debug_fops);
+	debugfs_create_file("ti_hecc_mbox_regs", S_IRUGO,
+			hecc_dir, &hecc_print_mbox_regs, &hecc_debug_fops);
+	debugfs_create_file("ti_hecc_status", S_IRUGO,
+			hecc_dir, &hecc_print_status, &hecc_debug_fops);
+	debugfs_create_file("ti_hecc_loopback", S_IRUGO,
+			hecc_dir, &hecc_debug_loopback, &hecc_debug_fops);
+	debugfs_create_u32("ti_hecc_debug", S_IWUGO,
+			hecc_dir, &hecc_debug_state);
+	debugfs_create_u32("ti_hecc_bittiming", S_IWUGO,
+			hecc_dir, &debug_priv->can.bittiming.bitrate);
+	debugfs_create_u32("ti_hecc_restart_ms", S_IWUGO,
+			hecc_dir, &debug_priv->can.restart_ms);
+	debugfs_create_u32("prop_seg", S_IWUGO,
+			hecc_dir, &debug_priv->can.bittiming.prop_seg);
+	debugfs_create_u32("phase_seg2", S_IWUGO,
+			hecc_dir, &debug_priv->can.bittiming.phase_seg2);
+	debugfs_create_u32("phase_seg1", S_IWUGO,
+			hecc_dir, &debug_priv->can.bittiming.phase_seg1);
+	debugfs_create_u32("brp", S_IWUGO,
+			hecc_dir, &debug_priv->can.bittiming.brp);
+	debugfs_create_u32("sjw", S_IWUGO,
+			hecc_dir, &debug_priv->can.bittiming.sjw);
+
+	return 0;
+}
+
+static void hecc_debug_exit(void)
+{
+	if (hecc_dir)
+		debugfs_remove_recursive(hecc_dir);
+}
+
+#endif /* CONFIG_DEBUG_FS */
+
+
+static int ti_hecc_get_state(const struct net_device *ndev,
+	enum can_state *state)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+	*state = priv->can.state;
+	return 0;
+}
+
+static int ti_hecc_set_bittiming(struct net_device *ndev)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+	struct can_bittiming *bit_timing = &priv->can.bittiming;
+	unsigned int can_btc = 0;
+
+	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)
+		can_btc |= HECC_CANBTC_SAM;
+	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);
+
+	return 0;
+}
+
+/**
+ * ti_hecc_reset: Reset HECC module and set bit timings
+ *
+ * Resets HECC by writing to change config request bit and then sets
+ * bit-timing registers in the module to enable the module for operation
+ */
+static void ti_hecc_reset(struct net_device *ndev)
+{
+#define HECC_CCE_WAIT_COUNT	1000
+	unsigned int 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);
+
+	/* if change control request not enabled */
+	if (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
+		/* 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 large counter to respect the specs
+		 */
+		cnt = HECC_CCE_WAIT_COUNT;
+		while (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
+			--cnt;
+			if (0 == cnt) {
+				dev_info(ndev->dev.parent,
+					"timing out on CCE after reset\n");
+				break;
+			}
+			if (printk_ratelimit())
+				dev_dbg(ndev->dev.parent,
+					"waiting CCE after reset\n");
+		}
+	}
+
+	/* Set bit timing on the device */
+	ti_hecc_set_bittiming(priv->ndev);
+
+	/* 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);
+	*/
+
+	/* Wait till CCE bit clears */
+	/* 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 large counter to respect the specs
+	 */
+	cnt = HECC_CCE_WAIT_COUNT;
+	while (hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
+		--cnt;
+		if (0 == cnt) {
+			dev_info(ndev->dev.parent,
+				"timing out on CCE after bittiming\n");
+			break;
+		}
+		if (printk_ratelimit())
+			dev_dbg(ndev->dev.parent,
+				"waiting CCE after bittiming\n");
+	}
+
+	/* 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);
+}
+
+/**
+ * ti_hecc_start: Starts HECC module
+ *
+ * If CAN state is not stopped, reset the module, init bit timings
+ * and start the device for operation
+ */
+static void ti_hecc_start(struct net_device *ndev)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+	int cnt, mbxno;
+
+	ti_hecc_reset(ndev);
+
+	bitmap_zero(priv->tx_free_mbx, TI_HECC_MAX_TX_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 < TI_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_CANTRS, 0);
+	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)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+	int ret = 0;
+
+	switch (mode) {
+	case CAN_MODE_SLEEP:
+		dev_info(priv->ndev->dev.parent, "device going to sleep\n");
+		if (netif_running(ndev)) {
+			netif_stop_queue(ndev);
+			netif_device_detach(ndev);
+			/* Put HECC in low power mode */
+			hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
+		}
+		priv->can.state = CAN_STATE_SLEEPING;
+		break;
+	case CAN_MODE_STOP:
+		dev_info(priv->ndev->dev.parent, "device stopping\n");
+		ti_hecc_stop(ndev);
+		break;
+	case CAN_MODE_START:
+		dev_info(priv->ndev->dev.parent, "device (re)starting\n");
+		++priv->can.can_stats.restarts;
+		ti_hecc_start(ndev);
+		if (netif_queue_stopped(ndev))
+			netif_wake_queue(ndev);
+		break;
+	default:
+		ret = -EOPNOTSUPP;
+		break;
+	}
+
+	return ret;
+}
+
+static int ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+	struct net_device_stats *stats = &ndev->stats;
+	struct can_frame *cf = (struct can_frame *)skb->data;
+	u32 mbxno = 0;
+	u32 data;
+	unsigned long flags;
+
+	/* Find the first mailbox that is free for xmit */
+	spin_lock_irqsave(&priv->tx_lock, flags);
+	mbxno = find_first_zero_bit(priv->tx_free_mbx, TI_HECC_MAX_TX_MBOX);
+	if (mbxno == TI_HECC_MAX_TX_MBOX) {
+		netif_stop_queue(ndev);
+		if (printk_ratelimit())
+			dev_err(priv->ndev->dev.parent,
+				"Out of TX buffers ...\n");
+		spin_unlock_irqrestore(&priv->tx_lock, flags);
+		return NETDEV_TX_BUSY;
+
+	}
+	set_bit(mbxno, priv->tx_free_mbx);
+	spin_unlock_irqrestore(&priv->tx_lock, flags);
+
+	/* Prepare mailbox for transmission */
+	hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
+	data = cf->can_dlc & 0xF;
+	if (cf->can_id & CAN_RTR_FLAG) /* Remote transmission request */
+		data |= HECC_CANMCF_RTR;
+	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);
+	data = (cf->data[0] << 24) | (cf->data[1] << 16) |
+			(cf->data[2] << 8) | cf->data[3];
+	hecc_write(priv, HECC_CANMDL(mbxno), data);
+	if (cf->can_dlc > 4) {
+		data = (cf->data[4] << 24) | (cf->data[5] << 16) |
+			(cf->data[6] << 8) | cf->data[7];
+		hecc_write(priv, HECC_CANMDH(mbxno), data);
+	}
+
+#ifdef CONFIG_DEBUG_FS
+	if (hecc_debug_state) {
+		printk(KERN_ERR "Mbxno=%d, CANMID=%08X, CANMCF=%08X," \
+			" CANMDH=%08X, CANMDL=%08X\n", mbxno,
+			hecc_read(priv, HECC_CANMID(mbxno)),
+			hecc_read(priv, HECC_CANMCF(mbxno)),
+			hecc_read(priv, HECC_CANMDH(mbxno)),
+			hecc_read(priv, HECC_CANMDL(mbxno)));
+		printk(KERN_INFO "HECC_TX: %02X, %02X, %02X, %02X, %02X," \
+			" %02X, %02X, %02X\n",
+			cf->data[0], cf->data[1], cf->data[2], cf->data[3],
+			cf->data[4], cf->data[5], cf->data[6], cf->data[7]);
+	}
+#endif /* CONFIG_DEBUG_FS */
+
+	/* Enable interrupt for mbox and start transmission */
+	hecc_clear_bit(priv, HECC_CANMD, (1 << mbxno));
+	hecc_set_bit(priv, HECC_CANME, (1 << mbxno));
+	hecc_set_bit(priv, HECC_CANMIM, (1 << mbxno));
+	hecc_set_bit(priv, HECC_CANTRS, (1 << mbxno));
+
+	stats->tx_bytes += cf->can_dlc;
+	ndev->trans_start = jiffies;
+	can_put_echo_skb(skb, ndev, 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;
+
+	skb = dev_alloc_skb(sizeof(struct can_frame));
+	if (!skb) {
+		if (printk_ratelimit())
+			dev_err(priv->ndev->dev.parent,
+				"dev_alloc_skb() failed\n");
+		return -ENOMEM;
+	}
+	skb->dev = priv->ndev;
+	skb->protocol = 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));
+	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));
+	/* The below statements are for readability sake */
+	cf->data[0] = ((data & 0xFF000000) >> 24);
+	cf->data[1] = ((data & 0xFF0000) >> 16);
+	cf->data[2] = ((data & 0xFF00) >> 8);
+	cf->data[3] = (data & 0xFF);
+	if (cf->can_dlc > 4) {
+		data = hecc_read(priv, HECC_CANMDH(mbxno));
+		cf->data[4] = ((data & 0xFF000000) >> 24);
+		cf->data[5] = ((data & 0xFF0000) >> 16);
+		cf->data[6] = ((data & 0xFF00) >> 8);
+		cf->data[7] = (data & 0xFF);
+	}
+
+#ifdef CONFIG_DEBUG_FS
+	if (hecc_debug_state) {
+		printk(KERN_ERR "Mbxno=%d, CANMID=%08X, CANMCF=%08X," \
+			" CANMDH=%08X, CANMDL=%08X\n", mbxno,
+			hecc_read(priv, HECC_CANMID(mbxno)),
+			hecc_read(priv, HECC_CANMCF(mbxno)),
+			hecc_read(priv, HECC_CANMDH(mbxno)),
+			hecc_read(priv, HECC_CANMDL(mbxno)));
+		printk(KERN_INFO "HECC_RX: %02X, %02X, %02X, %02X, %02X,"\
+			" %02X, %02X, %02X\n",
+			cf->data[0], cf->data[1], cf->data[2], cf->data[3],
+			cf->data[4], cf->data[5], cf->data[6], cf->data[7]);
+	}
+#endif /* CONFIG_DEBUG_FS */
+
+	/* prepare mailbox for next receive */
+	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));
+
+	stats->rx_bytes += cf->can_dlc;
+	netif_rx(skb);
+	stats->rx_packets++;
+	priv->ndev->last_rx = jiffies;
+
+	return 0;
+}
+
+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);
+	int num_pkts = 0;
+	unsigned long pending_pkts;
+	int mbxno;
+
+	if (!netif_running(ndev))
+		return 0;
+
+	pending_pkts = hecc_read(priv, HECC_CANRMP);
+	while (pending_pkts && (num_pkts < quota)) {
+		mbxno = find_first_bit(&pending_pkts, HECC_MAX_MAILBOXES);
+		if (mbxno == HECC_MAX_MAILBOXES) {
+			dev_info(priv->ndev->dev.parent,
+				"Reached max mailboxes. No rx pkts\n");
+			return num_pkts;
+		}
+
+		if (ti_hecc_rx_pkt(priv, mbxno) < 0)
+			return num_pkts;
+
+		clear_bit(mbxno, &pending_pkts);
+		hecc_set_bit(priv, HECC_CANRMP, (1 << mbxno));
+		++num_pkts;
+	}
+
+	/* Enable packet interrupt if all pkts are handled */
+	if (0 == hecc_read(priv, HECC_CANRMP)) {
+		napi_complete(napi);
+		/* Re-enable RX mailbox interrupts */
+		mbxno = hecc_read(priv, HECC_CANMIM);
+		mbxno |= (~((1 << TI_HECC_MAX_TX_MBOX) - 1));
+		hecc_write(priv, HECC_CANMIM, mbxno);
+	}
+
+	return num_pkts;
+}
+
+/**
+ * ti_hecc_error: TI HECC error routine
+ *
+ * Handles HECC error - handles error condition and send a packet up the stack
+ */
+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;
+	int data;
+
+	/* propogate the error condition to the can stack */
+	skb = dev_alloc_skb(sizeof(struct can_frame));
+	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);
+	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_RMLIF) { /* Message lost interrupt */
+		data = hecc_read(priv, HECC_CANRML);
+		hecc_write(priv, HECC_CANRML, data);
+		++priv->message_lost_cnt;
+		cf->can_id |= CAN_ERR_CRTL;
+		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
+		stats->rx_over_errors++;
+		stats->rx_errors++;
+		dev_info(priv->ndev->dev.parent, "Message lost interrupt\n");
+	}
+
+	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_info(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_info(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) {
+		priv->can.state = CAN_STATE_BUS_OFF;
+		cf->can_id |= CAN_ERR_BUSOFF;
+		hecc_set_bit(priv, HECC_CANES, HECC_CANES_BO);
+		dev_info(priv->ndev->dev.parent, "Bus Off interrupt\n");
+		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);
+	}
+
+	if (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);
+		dev_info(priv->ndev->dev.parent, "Bus Off condition\n");
+		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);
+	}
+
+	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;
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+	return 0;
+}
+
+
+/**
+ * ti_hecc_interrupt: TI HECC interrupt routine
+ *
+ * Handles HECC interrupts - disables interrupt if receive pkts that will
+ * be enabled when rx pkts are compelte (napi_complete is done)
+ */
+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;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	unsigned int int_status;
+	unsigned long ack;
+	int mbxno;
+	unsigned long flags;
+
+	if (priv->int_line)
+		int_status = hecc_read(priv, HECC_CANGIF1);
+	else
+		int_status = hecc_read(priv, HECC_CANGIF0);
+
+	if (0 == int_status)
+		return IRQ_NONE;
+
+	/* Handle message alarm interrupt */
+	if (int_status & HECC_CANGIF_MAIF) {
+		++priv->message_alarm_cnt;
+		dev_info(priv->ndev->dev.parent, "Message alarm interrupt\n");
+	}
+
+	/* Handle local network timer counter overflow interrupt */
+	if (int_status & HECC_CANGIF_TCOIF) {
+		++priv->timer_overflow_cnt;
+		dev_info(priv->ndev->dev.parent,
+			"Local network timer counter overflow interrupt\n");
+	}
+
+	/* Handle write denied interrupt */
+	if (int_status & HECC_CANGIF_WDIF) {
+		++priv->write_denied_cnt;
+		dev_info(priv->ndev->dev.parent, "Write denied interrupt\n");
+	}
+
+	/* Handle wake up interrupt */
+	if (int_status & HECC_CANGIF_WUIF) {
+		++priv->wake_up_cnt;
+		dev_info(priv->ndev->dev.parent, "Wake up interrupt\n");
+	}
+
+	ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));
+
+	/* Handle Abort acknowledge interrupt */
+	if (int_status & HECC_CANGIF_AAIF) {
+		ack = hecc_read(priv, HECC_CANAA);
+		while (ack) {
+			mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
+			if (mbxno == HECC_MAX_MAILBOXES) {
+				break;
+			} else {
+				clear_bit(mbxno, &ack);
+				/* release echo pkt & update counters */
+				hecc_set_bit(priv, HECC_CANAA, (1 << mbxno));
+				hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
+				/* FIXME: since net-next tree's dev.h does not
+				 * include can_free_echo_skb() doing equivalent
+				 * of can_free_echo_skb(ndev, mbxno);
+				 */
+				if (priv->can.echo_skb[mbxno]) {
+					kfree_skb(priv->can.echo_skb[mbxno]);
+					priv->can.echo_skb[mbxno] = NULL;
+				}
+				if (netif_queue_stopped(ndev))
+					netif_wake_queue(ndev);
+				spin_lock_irqsave(&priv->tx_lock, flags);
+				clear_bit(mbxno, priv->tx_free_mbx);
+				spin_unlock_irqrestore(&priv->tx_lock, flags);
+			}
+		}
+	}
+
+	/* Handle mailbox interrupt */
+	if (int_status & HECC_CANGIF_GMIF) {
+		ack = hecc_read(priv, HECC_CANTA);
+		while (ack) {
+			mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
+			if (mbxno == HECC_MAX_MAILBOXES) {
+				break;
+			} else {
+				clear_bit(mbxno, &ack);
+				hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
+				hecc_set_bit(priv, HECC_CANTA, (1 << mbxno));
+				skb = priv->can.echo_skb[mbxno];
+				cf = (struct can_frame *) (skb->data);
+				can_get_echo_skb(ndev, mbxno);
+				stats->tx_bytes += cf->can_dlc;
+				spin_lock_irqsave(&priv->tx_lock, flags);
+				clear_bit(mbxno, priv->tx_free_mbx);
+				spin_unlock_irqrestore(&priv->tx_lock, flags);
+				stats->tx_packets++;
+			}
+		}
+		if (netif_queue_stopped(ndev))
+			netif_wake_queue(ndev);
+
+		/* Disable RX mailbox interrupts and let NAPI reenable them */
+		ack = hecc_read(priv, HECC_CANMIM);
+		ack &= ((1 << TI_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;
+}
+
+/* NOTE: yet to test suspend/resume */
+static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+
+	if (netif_running(ndev)) {
+		netif_stop_queue(ndev);
+		netif_device_detach(ndev);
+	}
+
+	hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
+	priv->can.state = CAN_STATE_SLEEPING;
+	clk_disable(priv->clk);
+
+	return 0;
+}
+
+/* NOTE: yet to test suspend/resume */
+static int ti_hecc_resume(struct platform_device *pdev)
+{
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+
+	clk_enable(priv->clk);
+	hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+	if (netif_running(ndev)) {
+		netif_device_attach(ndev);
+		netif_start_queue(ndev);
+	}
+
+	return 0;
+}
+
+static int ti_hecc_open(struct net_device *ndev)
+{
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+	int err;
+
+	dev_info(ndev->dev.parent, "opening device\n");
+
+	if (request_irq(ndev->irq, ti_hecc_interrupt, IRQF_DISABLED,
+				ndev->name, ndev)) {
+		dev_err(ndev->dev.parent, "error requesting interrupt\n");
+		return -EAGAIN;
+	}
+
+	/* Open common can device */
+	err = open_candev(ndev);
+	if (err) {
+		dev_err(ndev->dev.parent, "open_candev() failed %08X\n", err);
+		return err;
+	}
+
+	/* Initialize device and start net queue */
+	spin_lock_init(&priv->tx_lock);
+
+	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);
+
+	dev_info(ndev->dev.parent, "closing device\n");
+	napi_disable(&priv->napi);
+	netif_stop_queue(ndev);
+	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;
+
+	printk(KERN_INFO DRV_NAME " probing devices...\n");
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		printk(KERN_ERR "No platform data available - exiting\n");
+		return -ENODEV;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		printk(KERN_ERR "no mem resource?\n");
+		err = -ENODEV;
+		goto probe_exit;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!irq) {
+		printk(KERN_ERR "no irq resource?\n");
+		err = -ENODEV;
+		goto probe_exit;
+	}
+	if (!request_mem_region(mem->start, (mem->end - mem->start) + 1,
+		pdev->name)) {
+		printk(KERN_ERR "HECC region already claimed\n");
+		err = -EBUSY;
+		goto probe_exit;
+	}
+	addr = ioremap(mem->start, mem->end - mem->start + 1);
+	if (!addr) {
+		printk(KERN_ERR "ioremap failed\n");
+		err = -ENOMEM;
+		goto probe_exit_free_region;
+	}
+
+	ndev = alloc_candev(sizeof(struct ti_hecc_priv));
+	if (!ndev) {
+		printk(KERN_ERR "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_bittiming	= ti_hecc_set_bittiming;
+	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(ndev->dev.parent, "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,
+			TI_HECC_DEF_NAPI_WEIGHT);
+
+	err = register_candev(ndev);
+	if (err) {
+		dev_err(ndev->dev.parent, "register_candev() failed\n");
+		err = -ENODEV;
+		goto probe_exit_clk;
+	}
+	dev_info(ndev->dev.parent, "regs=%p, irq=%d\n",
+		priv->base, (unsigned int) ndev->irq);
+
+#ifdef CONFIG_DEBUG_FS
+	hecc_debug_init(priv);
+#endif
+	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, mem->end - mem->start + 1);
+probe_exit:
+	dev_err(ndev->dev.parent, "probe error = %08X\n", err);
+	return err;
+}
+
+static int __devexit ti_hecc_remove(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+
+#ifdef CONFIG_DEBUG_FS
+	hecc_debug_exit();
+#endif /* CONFIG_DEBUG_FS */
+
+	clk_put(priv->clk);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	iounmap(priv->base);
+	release_mem_region(res->start, res->end - res->start + 1);
+	unregister_candev(ndev);
+	free_candev(ndev);
+	platform_set_drvdata(pdev, NULL);
+	dev_info(ndev->dev.parent, "driver removed\n");
+
+	return 0;
+}
+
+/* TI HECC netdevice driver: platform driver structure */
+static struct platform_driver ti_hecc_driver = {
+	.driver = {
+		.name    = "ti_hecc",
+		.owner   = THIS_MODULE,
+	},
+	.probe = ti_hecc_probe,
+	.remove = __devexit_p(ti_hecc_remove),
+	.suspend = ti_hecc_suspend,
+	.resume = ti_hecc_resume,
+};
+
+static int __init ti_hecc_init_driver(void)
+{
+	printk(KERN_INFO DRV_DESC "\n");
+	return platform_driver_register(&ti_hecc_driver);
+}
+module_init(ti_hecc_init_driver);
+
+static void __exit ti_hecc_exit_driver(void)
+{
+	printk(KERN_INFO DRV_DESC " :Exit\n");
+	platform_driver_unregister(&ti_hecc_driver);
+}
+module_exit(ti_hecc_exit_driver);
+
+MODULE_AUTHOR("Anant Gole <anantgole@ti.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION(DRV_DESC);
diff --git a/include/linux/can/platform/ti_hecc_platform.h b/include/linux/can/platform/ti_hecc_platform.h
new file mode 100644
index 0000000..4a57daf
--- /dev/null
+++ b/include/linux/can/platform/ti_hecc_platform.h
@@ -0,0 +1,40 @@
+/*
+ * TI HECC (High End CAN Controller) driver platform header
+ *
+ * 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.
+ *
+ */
+
+/**
+ * struct hecc_platform_data - HECC Platform Data
+ *
+ * @scc_hecc_offset:	mostly 0 - should really never change
+ * @scc_ram_offset:	SCC RAM offset
+ * @hecc_ram_offset:	HECC RAM offset
+ * @mbox_offset:	Mailbox RAM offset
+ * @int_line:		Interrupt line to use - 0 or 1
+ * @version:		version for future use
+ *
+ * Platform data structure to get all platform specific settings.
+ * this structure also accounts the fact that the IP may have different
+ * RAM and mailbox offsets for different SOC's
+ */
+struct ti_hecc_platform_data {
+	unsigned int scc_hecc_offset;
+	unsigned int scc_ram_offset;
+	unsigned int hecc_ram_offset;
+	unsigned int mbox_offset;
+	unsigned int int_line;
+	unsigned int version;
+};
+
+
-- 
1.6.2.4


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-28 11:18 Anant Gole
@ 2009-08-28 12:44 ` Oliver Hartkopp
  2009-08-28 13:13   ` Gole, Anant
  2009-09-01  9:04 ` Wolfram Sang
  1 sibling, 1 reply; 13+ messages in thread
From: Oliver Hartkopp @ 2009-08-28 12:44 UTC (permalink / raw)
  To: Anant Gole; +Cc: socketcan-core, netdev, Wolfgang Grandegger

Anant Gole wrote:
> TI HECC (High End CAN Controller) module is found on many TI devices. It has
> 32 harwdare mailboxes with full implementation of CAN protocol version 2.0B
> and bus speeds up to 1Mbps. The module specifications are available at TI web
> <http://www.ti.com>.
> 
> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
> 

Hello Anant,

some nitpicking first:

> +#include <linux/can/platform/ti_hecc_platform.h>

Please use

linux/can/platform/ti_hecc.c

following the other drivers.

> +
> +#define DRV_NAME "TI HECC"

DRV_NAME "ti_hecc"

like your module is called in various places in the Kernel.

This could be used later in

> +static struct platform_driver ti_hecc_driver = {
> +	.driver = {
> +		.name    = "ti_hecc",
> +		.owner   = THIS_MODULE,
> +	},

also.

And it's better for this:


> +/* CAN Bittiming constants as per HECC specs */
> +static struct can_bittiming_const ti_hecc_bittiming_const = {
> +	.name = DRV_NAME,
> +	.tseg1_min = 1,
> (..)


> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static struct ti_hecc_priv *debug_priv;
> +
> +#define PRINTMBOXREG(r) seq_printf(s, "%d\t%08X %08X %08X %08X %08X\n", r,\
> +	hecc_read(debug_priv, HECC_CANMID(r)),\
> +	hecc_read(debug_priv, HECC_CANMCF(r)),\
> +	hecc_read(debug_priv, HECC_CANMDH(r)),\
> +	hecc_read(debug_priv, HECC_CANMDL(r)),\
> +	hecc_read(debug_priv, HECC_CANLAM(r)))
> +
> +/* Print mailbox data */
> +static void hecc_print_mbox_regs(struct seq_file *s)
> +{
> +	int cnt = 0;
> +	static struct ti_hecc_priv *priv;
> +	priv = debug_priv;
> +	seq_printf(s, "\n--- %s %s - mailbox regs ---\n\n",
> +		DRV_NAME, HECC_MODULE_VERSION);
> +	seq_printf(s, "MbxNo\tMID\t MCF\t  MDH\t   MDL\t   LAM\n");
> +	seq_printf(s, "-----------------------------------------------\n");
> +	for (cnt = 0; cnt < HECC_MAX_MAILBOXES; cnt++)
> +		PRINTMBOXREG(cnt);
> +}
> +
> +#define PRINTREG(d, r) seq_printf(s, "%s\t%08x\n", d, hecc_read(debug_priv, r))
> +/* Print HECC registers */
> +static void hecc_print_regs(struct seq_file *s)
> +{

I discovered lot's of debug code (>20%).

Is this really needed?



> +static char *hecc_debug_can_state[] = {
> +	"CAN_STATE_ERROR_ACTIVE",
> +	"CAN_STATE_ERROR_WARNING",
> +	"CAN_STATE_ERROR_PASSIVE",
> +	"CAN_STATE_BUS_OFF",
> +	"CAN_STATE_STOPPED",
> +	"CAN_STATE_SLEEPING",
> +	"CAN_STATE_MAX"
> +};

Hm - defining this in a driver looks like a bad idea.

Maybe we could move this to the CAN driver interface depending on

CONFIG_CAN_DEBUG_DEVICES

?!?


> +
> +/* Print status and statistics */
> +static void hecc_print_status(struct seq_file *s)
> +{
> +	seq_printf(s, "\n--- %s %s - status ---\n\n",
> +		DRV_NAME, HECC_MODULE_VERSION);
> +	seq_printf(s, "\n--- ti_hecc status ---\n\n");
> +	seq_printf(s, "CAN state \t\t= %s\n",
> +		hecc_debug_can_state[debug_priv->can.state]);
> +	seq_printf(s, "CAN restart_ms \t\t= %u\n", debug_priv->can.restart_ms);
> +	seq_printf(s, "CAN input clock \t= %u\n", debug_priv->can.clock.freq);
> +	seq_printf(s, "CAN Bittiming\n");
> +	seq_printf(s, "\tbitrate \t= %u\n",
> +			debug_priv->can.bittiming.bitrate);
> +	seq_printf(s, "\tsample_point \t= %u\n",
> +			debug_priv->can.bittiming.sample_point);
> +	seq_printf(s, "\ttq \t\t= %u\n",
> +			debug_priv->can.bittiming.tq);
> +	seq_printf(s, "\tprop_seg \t= %u\n",
> +			debug_priv->can.bittiming.prop_seg);
> +	seq_printf(s, "\tphase_seg1 \t= %u\n",
> +			debug_priv->can.bittiming.phase_seg1);
> +	seq_printf(s, "\tphase_seg2 \t= %u\n",
> +			debug_priv->can.bittiming.phase_seg2);
> +	seq_printf(s, "\tsjw \t\t= %u\n",
> +			debug_priv->can.bittiming.sjw);
> +	seq_printf(s, "\tbrp \t\t= %u\n",
> +			debug_priv->can.bittiming.brp);
> +	seq_printf(s, "CAN Bittiming Constants\n");
> +	seq_printf(s, "\ttseg1 min-max \t= %u-%u\n",
> +			debug_priv->can.bittiming_const->tseg1_min,
> +			debug_priv->can.bittiming_const->tseg1_max);
> +	seq_printf(s, "\ttseg2 min-max \t= %u-%u\n",
> +			debug_priv->can.bittiming_const->tseg2_min,
> +			debug_priv->can.bittiming_const->tseg2_max);
> +	seq_printf(s, "\tsjw_max \t= %u\n",
> +			debug_priv->can.bittiming_const->sjw_max);
> +	seq_printf(s, "\tbrp min-max \t= %u-%u\n",
> +			debug_priv->can.bittiming_const->brp_min,
> +			debug_priv->can.bittiming_const->brp_max);
> +	seq_printf(s, "\tbrp_inc \t= %u\n",
> +			debug_priv->can.bittiming_const->brp_inc);

(..)

And this could be also a candidate to be in the CAN driver interface.

@Wolfgang: Any preferences to this idea?


> +
> +/** Toggle HECC Self-Test i.e loopback bit
> + * INFO: Reading this debug variable toggles the loopback status on the device.
> + * This is done to simplify the debug function to set looback
> + */
> +static int hecc_debug_loopback(struct seq_file *s)
> +{
> +	static int toggle;
> +
> +	/* Put module in self test mode i.e. loopback */
> +	if (toggle) {
> +		seq_printf(s, "In Self Test Mode (loopback)\n");
> +		hecc_set_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
> +		toggle = 0;
> +	} else {
> +		seq_printf(s, "Out of Self Test Mode (NO loopback)\n");
> +		hecc_clear_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
> +		toggle = 1;
> +	}
> +
> +	return 0;
> +}
> +

Ugh!

No. This should definitely be done by netlink.

I did not take a closer look into the device access and error handling right
now. So this was just my first impression :-)

Thanks,
Oliver

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-28 12:44 ` Oliver Hartkopp
@ 2009-08-28 13:13   ` Gole, Anant
  2009-08-28 13:27     ` Oliver Hartkopp
  0 siblings, 1 reply; 13+ messages in thread
From: Gole, Anant @ 2009-08-28 13:13 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: socketcan-core@lists.berlios.de, netdev@vger.kernel.org,
	Wolfgang Grandegger


>-----Original Message-----
>From: Oliver Hartkopp [mailto:socketcan@hartkopp.net]
>Sent: Friday, August 28, 2009 6:15 PM
>To: Gole, Anant
>Cc: socketcan-core@lists.berlios.de; netdev@vger.kernel.org; Wolfgang
>Grandegger
>Subject: Re: [PATCH] net-next:can: add TI CAN (HECC) driver
>
>Anant Gole wrote:
>> TI HECC (High End CAN Controller) module is found on many TI devices. It
>has
>> 32 harwdare mailboxes with full implementation of CAN protocol version
>2.0B
>> and bus speeds up to 1Mbps. The module specifications are available at TI
>web
>> <http://www.ti.com>.
>>
>> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
>>
>
>Hello Anant,
>
>some nitpicking first:

No issues at all. The driver code will only get better when more people will review it and that is good.

>
>> +#include <linux/can/platform/ti_hecc_platform.h>
>
>Please use
>
>linux/can/platform/ti_hecc.c

are you suggesting to keep the platform file name as ti_hecc.h?

>
>following the other drivers.
>
>> +
>> +#define DRV_NAME "TI HECC"
>
>DRV_NAME "ti_hecc"

Agreed. I will change it.

>
>> +/* Print HECC registers */
>> +static void hecc_print_regs(struct seq_file *s)
>> +{
>
>I discovered lot's of debug code (>20%).
>
>Is this really needed?

Not really. I started with lots of debug code as I thought I would really need it and it was indeed helpful initially - I intended on removing it later but I thought keeping more debug code may not be a bad idea though I don't have any strong preference on this. Occasionally it is helpful to look at all registers and hence I coded it under CONFIG_DEBUG_FS.

>
>
>
>> +static char *hecc_debug_can_state[] = {
>> +	"CAN_STATE_ERROR_ACTIVE",
>> +	"CAN_STATE_ERROR_WARNING",
>> +	"CAN_STATE_ERROR_PASSIVE",
>> +	"CAN_STATE_BUS_OFF",
>> +	"CAN_STATE_STOPPED",
>> +	"CAN_STATE_SLEEPING",
>> +	"CAN_STATE_MAX"
>> +};
>
>Hm - defining this in a driver looks like a bad idea.
>
>Maybe we could move this to the CAN driver interface depending on
>
>CONFIG_CAN_DEBUG_DEVICES
>
>?!?

Agreed - this was again just for my debug. It would help as you mentioned for debug and can potentially go under can netlink.h.

>> +
>> +/** Toggle HECC Self-Test i.e loopback bit
>> + * INFO: Reading this debug variable toggles the loopback status on the
>device.
>> + * This is done to simplify the debug function to set looback
>> + */
>> +static int hecc_debug_loopback(struct seq_file *s)
>> +{
>> +	static int toggle;
>> +
>> +	/* Put module in self test mode i.e. loopback */
>> +	if (toggle) {
>> +		seq_printf(s, "In Self Test Mode (loopback)\n");
>> +		hecc_set_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
>> +		toggle = 0;
>> +	} else {
>> +		seq_printf(s, "Out of Self Test Mode (NO loopback)\n");
>> +		hecc_clear_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
>> +		toggle = 1;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>
>Ugh!
>
>No. This should definitely be done by netlink.

yes completely agreed - for some reason I did not get to compile iproute2 utils and hence used this for quick/easy debug of my driver. I will remove it.

>
>I did not take a closer look into the device access and error handling
>right
>now. So this was just my first impression :-)
>

Thanks for the quick feedback. Appreciate it. I will wait for more from you and others too.

>Thanks,
>Oliver


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-28 13:13   ` Gole, Anant
@ 2009-08-28 13:27     ` Oliver Hartkopp
  0 siblings, 0 replies; 13+ messages in thread
From: Oliver Hartkopp @ 2009-08-28 13:27 UTC (permalink / raw)
  To: Gole, Anant
  Cc: socketcan-core@lists.berlios.de, netdev@vger.kernel.org,
	Wolfgang Grandegger

Gole, Anant wrote:

>>> +#include <linux/can/platform/ti_hecc_platform.h>
>> Please use
>>
>> linux/can/platform/ti_hecc.c
> 
> are you suggesting to keep the platform file name as ti_hecc.h?
> 

Yep!

We currently have

linux/can/platform/sja1000.h
linux/can/platform/mcp251x.h

there. And as you need the full path for the include anyway, we decided to put
these CAN specific platform includes in linux/can/platform.

Else we would end with tons of <drivername>_platform.h in linux/can where we
have all the common stuff for CAN networking layer, etc.

Regards,
Oliver

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
       [not found] <2A3DCF3DA181AD40BDE86A3150B27B6B02F621106B@dbde02.ent.ti.com>
@ 2009-08-29  7:06 ` Wolfgang Grandegger
  2009-08-31  7:22   ` Gole, Anant
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Grandegger @ 2009-08-29  7:06 UTC (permalink / raw)
  To: Gole, Anant; +Cc: Linux Netdev List, Socketcan-core

> TI HECC (High End CAN Controller) module is found on many TI devices. It has
> 32 harwdare mailboxes with full implementation of CAN protocol version 2.0B
> and bus speeds up to 1Mbps. The module specifications are available at TI web
> <http://www.ti.com>.
> 
> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.

Nice driver, even using the NAPI interface. First some general comments.
Please remove debugging code mainly useful for development, e.g. the
CONFIG_DEBUG_FS block, many dev_info calls and special statistics
counters. Also use dev_dbg for the remaining debug messages useful for
the normal user, like state changes. More comments inline.

> 
> Signed-off-by: Anant Gole <anantgole@ti.com>
> ---
>  drivers/net/can/Kconfig                       |    9 +
>  drivers/net/can/Makefile                      |    2 +
>  drivers/net/can/ti_hecc.c                     | 1352 +++++++++++++++++++++++++
>  include/linux/can/platform/ti_hecc_platform.h |   40 +
>  4 files changed, 1403 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/can/ti_hecc.c
>  create mode 100644 include/linux/can/platform/ti_hecc_platform.h
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 30ae55d..fae62df 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -75,6 +75,15 @@ 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 (HECC)"
> +        default N
> +        ---help---
> +         This driver adds support for TI High End CAN Controller module
> +         found on many TI devices. The specifications of this module are
> +         are available from TI web (http://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..d923512 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -9,4 +9,6 @@ can-dev-y                       := dev.o
> 
>  obj-$(CONFIG_CAN_SJA1000)      += sja1000/
> 
> +obj-$(CONFIG_CAN_TI_HECC)      += ti_hecc.o
> +
>  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..4741b4a
> --- /dev/null
> +++ b/drivers/net/can/ti_hecc.c
> @@ -0,0 +1,1352 @@
> +/*
> + * 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.
> + *

It would be nice if you could mention the related platform data here,
similar to the mcp251x.c driver.

> + */
> +
> +#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>
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/can/platform/ti_hecc_platform.h>
> +
> +#define DRV_NAME "TI HECC"

#define DRV_NAME "ti_hecc"

Other drivers also use the file name here.

> +#define HECC_MODULE_VERSION     "0.2"

A version number is will usually not maintained. May drivers have it but
it's never changed.

> +MODULE_VERSION(HECC_MODULE_VERSION);
> +#define DRV_DESC "TI High End CAN Controller Driver " HECC_MODULE_VERSION
> +
> +#define HECC_MAX_MAILBOXES     32      /* hardware mboxes - do not change */
> +
> +#if (CAN_ECHO_SKB_MAX > 16)
> +#define TI_HECC_MAX_TX_MBOX    16
> +#else
> +#define TI_HECC_MAX_TX_MBOX    CAN_ECHO_SKB_MAX
> +#endif
> +#define TI_HECC_MAX_RX_MBOX    (HECC_MAX_MAILBOXES - TI_HECC_MAX_TX_MBOX)
> +
> +#define TI_HECC_DEF_NAPI_WEIGHT        TI_HECC_MAX_RX_MBOX
> +
> +/* 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)
> +
> +#define HECC_SET_REG           0xFFFFFFFF
> +#define HECC_CANID_MASK                0x3FF   /* 18 bits mask for extended id's */
> +
> +#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   0xFF00  /* all except maif and tcoif */
> +#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,

Please check if tseg2_min is a valid value. Usually it's larger.

> +       .tseg2_max = 8,
> +       .sjw_max = 4,
> +       .brp_min = 1,
> +       .brp_max = 256,
> +       .brp_inc = 1,
> +};
> +
> +struct ti_hecc_priv {
> +       struct can_priv can;

Please add "must be the first member/field".

> +       struct napi_struct napi;
> +       struct net_device *ndev;
> +       struct clk *clk;
> +       void __iomem *base;
> +       unsigned int scc_ram_offset;
> +       unsigned int hecc_ram_offset;
> +       unsigned int mbox_offset;
> +       unsigned int int_line;
> +       DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
> +       spinlock_t tx_lock;

Please document the spinlock tx_lock. What is it used for.

> +
> +       /* Statistics */
> +       unsigned out_of_tx_mbox;
> +       unsigned write_denied_cnt;
> +       unsigned message_lost_cnt;
> +       unsigned wake_up_cnt;
> +       unsigned message_alarm_cnt;
> +       unsigned timer_overflow_cnt;#

Debugging!? Should be removed. There is no interface to list these fields.

> +};
> +
> +static inline
> +void hecc_write(struct ti_hecc_priv *priv, int reg, unsigned int val)
> +{
> +       __raw_writel(val, priv->base + reg);
> +}
> +
> +static inline unsigned int 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));
> +}
> +
> +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
> +unsigned int hecc_get_bit(struct ti_hecc_priv *priv, int reg, int bit)
> +{
> +       return (hecc_read(priv, reg) & bit) ? 1 : 0;
> +}
> +
[snip]
> +
> +static int ti_hecc_get_state(const struct net_device *ndev,
> +       enum can_state *state)
> +{
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +       *state = priv->can.state;
> +       return 0;
> +}
> +
> +static int ti_hecc_set_bittiming(struct net_device *ndev)
> +{
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +       struct can_bittiming *bit_timing = &priv->can.bittiming;
> +       unsigned int can_btc = 0;
> +
> +       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)
> +               can_btc |= HECC_CANBTC_SAM;

Please use:

	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
	          can_btc |= HECC_CANBTC_SAM;

CAN controller modes can be set via "ip" utility. Note that also
loopback and listen-only is supported.

> +       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);

Other drivers use dev_info here:

	dev_info(ND2D(dev), "setting CANBTC=%#x\n", can_btc);


> +       return 0;
> +}
> +
> +/**
> + * ti_hecc_reset: Reset HECC module and set bit timings
> + *
> + * Resets HECC by writing to change config request bit and then sets
> + * bit-timing registers in the module to enable the module for operation
> + */
> +static void ti_hecc_reset(struct net_device *ndev)
> +{
> +#define HECC_CCE_WAIT_COUNT    1000
> +       unsigned int 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);
> +
> +       /* if change control request not enabled */
> +       if (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
> +               /* 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 large counter to respect the specs
> +                */
> +               cnt = HECC_CCE_WAIT_COUNT;
> +               while (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
> +                       --cnt;
> +                       if (0 == cnt) {
> +                               dev_info(ndev->dev.parent,
> +                                       "timing out on CCE after reset\n");
> +                               break;
> +                       }
> +                       if (printk_ratelimit())
> +                               dev_dbg(ndev->dev.parent,
> +                                       "waiting CCE after reset\n");

I think you don't need the ratelimit if you check after the while loop.

> +               }

The while loop above does not use any defined delay and therefore the
timing depends on the processor speed. Adding udelay(1|10) would solve
the issue.

> +       }
> +



> +       /* Set bit timing on the device */
> +       ti_hecc_set_bittiming(priv->ndev);

Hm, you re-set the bittiming here!? It is alread done via netlink
interface before the device is opened/started.

> +
> +       /* 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);
> +       */

Right, automatic bus-off recovery should be disabled. To make that clear:

           hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_ABO);


> +       /* Wait till CCE bit clears */
> +       /* 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 large counter to respect the specs
> +        */
> +       cnt = HECC_CCE_WAIT_COUNT;
> +       while (hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
> +               --cnt;
> +               if (0 == cnt) {
> +                       dev_info(ndev->dev.parent,
> +                               "timing out on CCE after bittiming\n");
> +                       break;
> +               }
> +               if (printk_ratelimit())
> +                       dev_dbg(ndev->dev.parent,
> +                               "waiting CCE after bittiming\n");
> +       }

Consider adding a udelay(1|10) as mentioned above.

> +       /* 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);
> +}
> +
> +/**
> + * ti_hecc_start: Starts HECC module
> + *
> + * If CAN state is not stopped, reset the module, init bit timings
> + * and start the device for operation
> + */
> +static void ti_hecc_start(struct net_device *ndev)
> +{
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +       int cnt, mbxno;
> +
> +       ti_hecc_reset(ndev);
> +
> +       bitmap_zero(priv->tx_free_mbx, TI_HECC_MAX_TX_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 < TI_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_CANTRS, 0);
> +       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)
> +{
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +       int ret = 0;
> +
> +       switch (mode) {
> +       case CAN_MODE_SLEEP:
> +               dev_info(priv->ndev->dev.parent, "device going to sleep\n");
> +               if (netif_running(ndev)) {
> +                       netif_stop_queue(ndev);
> +                       netif_device_detach(ndev);
> +                       /* Put HECC in low power mode */
> +                       hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> +               }
> +               priv->can.state = CAN_STATE_SLEEPING;
> +               break;

Has sleeping been tested? Actually, we do not have an interface yet to
enter sleep mode. Please remove. If there is a need for it, we should
work togehter to refine the interface and to provide a solution.

> +       case CAN_MODE_STOP:
> +               dev_info(priv->ndev->dev.parent, "device stopping\n");
> +               ti_hecc_stop(ndev);
> +               break;

Only CAN_MODE_START is used by the CAN devicde interface for bus-off
recovery.

> +       case CAN_MODE_START:
> +               dev_info(priv->ndev->dev.parent, "device (re)starting\n");
> +               ++priv->can.can_stats.restarts;
> +               ti_hecc_start(ndev);
> +               if (netif_queue_stopped(ndev))
> +                       netif_wake_queue(ndev);
> +               break;
> +       default:
> +               ret = -EOPNOTSUPP;
> +               break;
> +       }
> +
> +       return ret;
> +}
> +
> +static int ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +       struct net_device_stats *stats = &ndev->stats;
> +       struct can_frame *cf = (struct can_frame *)skb->data;
> +       u32 mbxno = 0;
> +       u32 data;
> +       unsigned long flags;
> +
> +       /* Find the first mailbox that is free for xmit */
> +       spin_lock_irqsave(&priv->tx_lock, flags);
> +       mbxno = find_first_zero_bit(priv->tx_free_mbx, TI_HECC_MAX_TX_MBOX);
> +       if (mbxno == TI_HECC_MAX_TX_MBOX) {
> +               netif_stop_queue(ndev);
> +               if (printk_ratelimit())
> +                       dev_err(priv->ndev->dev.parent,
> +                               "Out of TX buffers ...\n");
> +               spin_unlock_irqrestore(&priv->tx_lock, flags);
> +               return NETDEV_TX_BUSY;

Could'nt the NETDEV_TX_BUSY be avoided by stopping the queue earlier?

> +
> +       }
> +       set_bit(mbxno, priv->tx_free_mbx);
> +       spin_unlock_irqrestore(&priv->tx_lock, flags);

Hm, I wonder how the driver ensures that packages go out in order. How
does the CAN hardware schedule pending TX message objects? Vladislav
posted a test programs recently to check message ordering. See:

https://lists.berlios.de/pipermail/socketcan-core/2009-August/002871.html

> +
> +       /* Prepare mailbox for transmission */
> +       hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
> +       data = cf->can_dlc & 0xF;
> +       if (cf->can_id & CAN_RTR_FLAG) /* Remote transmission request */
> +               data |= HECC_CANMCF_RTR;
> +       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);
> +       data = (cf->data[0] << 24) | (cf->data[1] << 16) |
> +                       (cf->data[2] << 8) | cf->data[3];
> +       hecc_write(priv, HECC_CANMDL(mbxno), data);
> +       if (cf->can_dlc > 4) {
> +               data = (cf->data[4] << 24) | (cf->data[5] << 16) |
> +                       (cf->data[6] << 8) | cf->data[7];
> +               hecc_write(priv, HECC_CANMDH(mbxno), data);
> +       }
> +
[slip]
> +
> +       /* Enable interrupt for mbox and start transmission */
> +       hecc_clear_bit(priv, HECC_CANMD, (1 << mbxno));
> +       hecc_set_bit(priv, HECC_CANME, (1 << mbxno));
> +       hecc_set_bit(priv, HECC_CANMIM, (1 << mbxno));
> +       hecc_set_bit(priv, HECC_CANTRS, (1 << mbxno));
> +
> +       stats->tx_bytes += cf->can_dlc;

Should be done when the TX done interrupr is handled.

> +       ndev->trans_start = jiffies;
> +       can_put_echo_skb(skb, ndev, mbxno);

Please call can_put_echo_skb() before starting transmission.

> +
> +       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;
> +
> +       skb = dev_alloc_skb(sizeof(struct can_frame));
> +       if (!skb) {
> +               if (printk_ratelimit())
> +                       dev_err(priv->ndev->dev.parent,
> +                               "dev_alloc_skb() failed\n");
> +               return -ENOMEM;
> +       }
> +       skb->dev = priv->ndev;
> +       skb->protocol = 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));
> +       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));
> +       /* The below statements are for readability sake */
> +       cf->data[0] = ((data & 0xFF000000) >> 24);
> +       cf->data[1] = ((data & 0xFF0000) >> 16);
> +       cf->data[2] = ((data & 0xFF00) >> 8);
> +       cf->data[3] = (data & 0xFF);
> +       if (cf->can_dlc > 4) {
> +               data = hecc_read(priv, HECC_CANMDH(mbxno));
> +               cf->data[4] = ((data & 0xFF000000) >> 24);
> +               cf->data[5] = ((data & 0xFF0000) >> 16);
> +               cf->data[6] = ((data & 0xFF00) >> 8);
> +               cf->data[7] = (data & 0xFF);
> +       }
> +
[snip]
> +
> +       /* prepare mailbox for next receive */
> +       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));
> +
> +       stats->rx_bytes += cf->can_dlc;
> +       netif_rx(skb);

Hm, IIRC, netif_receive_skb(skb) should be used with NAPI.

> +       stats->rx_packets++;
> +       priv->ndev->last_rx = jiffies;
> +
> +       return 0;
> +}
> +
> +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);
> +       int num_pkts = 0;
> +       unsigned long pending_pkts;
> +       int mbxno;
> +
> +       if (!netif_running(ndev))
> +               return 0;
> +
> +       pending_pkts = hecc_read(priv, HECC_CANRMP);
> +       while (pending_pkts && (num_pkts < quota)) {
> +               mbxno = find_first_bit(&pending_pkts, HECC_MAX_MAILBOXES);

Here I also wonder if the messages are handled in the correct order.

> +               if (mbxno == HECC_MAX_MAILBOXES) {
> +                       dev_info(priv->ndev->dev.parent,
> +                               "Reached max mailboxes. No rx pkts\n");
> +                       return num_pkts;
> +               }
> +
> +               if (ti_hecc_rx_pkt(priv, mbxno) < 0)
> +                       return num_pkts;
> +
> +               clear_bit(mbxno, &pending_pkts);
> +               hecc_set_bit(priv, HECC_CANRMP, (1 << mbxno));
> +               ++num_pkts;
> +       }
> +
> +       /* Enable packet interrupt if all pkts are handled */
> +       if (0 == hecc_read(priv, HECC_CANRMP)) {
> +               napi_complete(napi);
> +               /* Re-enable RX mailbox interrupts */
> +               mbxno = hecc_read(priv, HECC_CANMIM);
> +               mbxno |= (~((1 << TI_HECC_MAX_TX_MBOX) - 1));
> +               hecc_write(priv, HECC_CANMIM, mbxno);
> +       }
> +
> +       return num_pkts;
> +}
> +
> +/**
> + * ti_hecc_error: TI HECC error routine
> + *
> + * Handles HECC error - handles error condition and send a packet up the stack
> + */
> +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;
> +       int data;
> +
> +       /* propogate the error condition to the can stack */
> +       skb = dev_alloc_skb(sizeof(struct can_frame));
> +       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);
> +       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_RMLIF) { /* Message lost interrupt */
> +               data = hecc_read(priv, HECC_CANRML);
> +               hecc_write(priv, HECC_CANRML, data);
> +               ++priv->message_lost_cnt;

Debugging!?

> +               cf->can_id |= CAN_ERR_CRTL;
> +               cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +               stats->rx_over_errors++;
> +               stats->rx_errors++;
> +               dev_info(priv->ndev->dev.parent, "Message lost interrupt\n");

Debugging!?

> +       }
> +
> +       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_info(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_info(priv->ndev->dev.parent, "Error passive interrupt\n");

Please use dev_dbg.

> +               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) {
> +               priv->can.state = CAN_STATE_BUS_OFF;
> +               cf->can_id |= CAN_ERR_BUSOFF;
> +               hecc_set_bit(priv, HECC_CANES, HECC_CANES_BO);
> +               dev_info(priv->ndev->dev.parent, "Bus Off interrupt\n");

can_bus_off() already calls dev_dbg("bus-off").


> +               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);
> +       }
> +
> +       if (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);
> +               dev_info(priv->ndev->dev.parent, "Bus Off condition\n");
> +               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);
> +       }

I think the two if blocks above use idendical code. What is the
difference. Using if (a || b) would be more appropriate.

> +
> +       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);

OK, here you use netif_receive_skb().

> +       ndev->last_rx = jiffies;
> +       stats->rx_packets++;
> +       stats->rx_bytes += cf->can_dlc;
> +       return 0;
> +}

At a first glance, error message creation looks good. I will have a
closer look some time later.

> +
> +/**
> + * ti_hecc_interrupt: TI HECC interrupt routine
> + *
> + * Handles HECC interrupts - disables interrupt if receive pkts that will
> + * be enabled when rx pkts are compelte (napi_complete is done)
> + */
> +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;
> +       struct sk_buff *skb;
> +       struct can_frame *cf;
> +       unsigned int int_status;
> +       unsigned long ack;
> +       int mbxno;
> +       unsigned long flags;
> +
> +       if (priv->int_line)
> +               int_status = hecc_read(priv, HECC_CANGIF1);
> +       else
> +               int_status = hecc_read(priv, HECC_CANGIF0);
> +
> +       if (0 == int_status)
> +               return IRQ_NONE;
> +
> +       /* Handle message alarm interrupt */
> +       if (int_status & HECC_CANGIF_MAIF) {
> +               ++priv->message_alarm_cnt;
> +               dev_info(priv->ndev->dev.parent, "Message alarm interrupt\n");
> +       }
> +
> +       /* Handle local network timer counter overflow interrupt */
> +       if (int_status & HECC_CANGIF_TCOIF) {
> +               ++priv->timer_overflow_cnt;
> +               dev_info(priv->ndev->dev.parent,
> +                       "Local network timer counter overflow interrupt\n");
> +       }
> +
> +       /* Handle write denied interrupt */
> +       if (int_status & HECC_CANGIF_WDIF) {
> +               ++priv->write_denied_cnt;
> +               dev_info(priv->ndev->dev.parent, "Write denied interrupt\n");
> +       }
> +
> +       /* Handle wake up interrupt */
> +       if (int_status & HECC_CANGIF_WUIF) {
> +               ++priv->wake_up_cnt;
> +               dev_info(priv->ndev->dev.parent, "Wake up interrupt\n");
> +       }

Do the interrupt sources above occur. Does it harm or even signal a
malfunctioning? If yes, use dev_err and dev_dbg otherwise. And as
mentioned above, the statistic counters are most likely for debugging
purposes only.

> +
> +       ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));

Hm, you create an error frame for each interrupt!? What do you see with:

 # candump any,0:0,#FFFFFFFF

> +
> +       /* Handle Abort acknowledge interrupt */
> +       if (int_status & HECC_CANGIF_AAIF) {
> +               ack = hecc_read(priv, HECC_CANAA);
> +               while (ack) {
> +                       mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
> +                       if (mbxno == HECC_MAX_MAILBOXES) {
> +                               break;
> +                       } else {
> +                               clear_bit(mbxno, &ack);
> +                               /* release echo pkt & update counters */
> +                               hecc_set_bit(priv, HECC_CANAA, (1 << mbxno));
> +                               hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
> +                               /* FIXME: since net-next tree's dev.h does not
> +                                * include can_free_echo_skb() doing equivalent
> +                                * of can_free_echo_skb(ndev, mbxno);
> +                                */
> +                               if (priv->can.echo_skb[mbxno]) {
> +                                       kfree_skb(priv->can.echo_skb[mbxno]);
> +                                       priv->can.echo_skb[mbxno] = NULL;
> +                               }
> +                               if (netif_queue_stopped(ndev))
> +                                       netif_wake_queue(ndev);
> +                               spin_lock_irqsave(&priv->tx_lock, flags);
> +                               clear_bit(mbxno, priv->tx_free_mbx);
> +                               spin_unlock_irqrestore(&priv->tx_lock, flags);
> +                       }
> +               }
> +       }

Can that interrupt happen? I have not found any code aborting messages.

> +
> +       /* Handle mailbox interrupt */
> +       if (int_status & HECC_CANGIF_GMIF) {
> +               ack = hecc_read(priv, HECC_CANTA);
> +               while (ack) {
> +                       mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
> +                       if (mbxno == HECC_MAX_MAILBOXES) {
> +                               break;
> +                       } else {
> +                               clear_bit(mbxno, &ack);
> +                               hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
> +                               hecc_set_bit(priv, HECC_CANTA, (1 << mbxno));
> +                               skb = priv->can.echo_skb[mbxno];
> +                               cf = (struct can_frame *) (skb->data);

Please don't access the echo skb's directly. Try to get the dlc from the
hardware.

> +                               can_get_echo_skb(ndev, mbxno);
> +                               stats->tx_bytes += cf->can_dlc;
> +                               spin_lock_irqsave(&priv->tx_lock, flags);
> +                               clear_bit(mbxno, priv->tx_free_mbx);
> +                               spin_unlock_irqrestore(&priv->tx_lock, flags);
> +                               stats->tx_packets++;
> +                       }
> +               }
> +               if (netif_queue_stopped(ndev))
> +                       netif_wake_queue(ndev);
> +
> +               /* Disable RX mailbox interrupts and let NAPI reenable them */
> +               ack = hecc_read(priv, HECC_CANMIM);
> +               ack &= ((1 << TI_HECC_MAX_TX_MBOX) - 1);
> +               hecc_write(priv, HECC_CANMIM, ack);
> +               napi_schedule(&priv->napi);

You schedule an RX event even if no RX message is pending? This does not
look very efficient to me.

> +       }
> +
> +       /* 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;
> +}
> +
> +/* NOTE: yet to test suspend/resume */

Please remove suspend/resume code if it's not tested.

> +static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +       struct net_device *ndev = platform_get_drvdata(pdev);
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +
> +       if (netif_running(ndev)) {
> +               netif_stop_queue(ndev);
> +               netif_device_detach(ndev);
> +       }
> +
> +       hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> +       priv->can.state = CAN_STATE_SLEEPING;
> +       clk_disable(priv->clk);
> +
> +       return 0;
> +}
> +
> +/* NOTE: yet to test suspend/resume */
> +static int ti_hecc_resume(struct platform_device *pdev)
> +{
> +       struct net_device *ndev = platform_get_drvdata(pdev);
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +
> +       clk_enable(priv->clk);
> +       hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> +       priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +       if (netif_running(ndev)) {
> +               netif_device_attach(ndev);
> +               netif_start_queue(ndev);
> +       }
> +
> +       return 0;
> +}
> +
> +static int ti_hecc_open(struct net_device *ndev)
> +{
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +       int err;
> +
> +       dev_info(ndev->dev.parent, "opening device\n");
> +
> +       if (request_irq(ndev->irq, ti_hecc_interrupt, IRQF_DISABLED,
> +                               ndev->name, ndev)) {
> +               dev_err(ndev->dev.parent, "error requesting interrupt\n");
> +               return -EAGAIN;

Please return the error returned by request_irq.

> +       }
> +
> +       /* Open common can device */
> +       err = open_candev(ndev);
> +       if (err) {
> +               dev_err(ndev->dev.parent, "open_candev() failed %08X\n", err);

free_irq?

> +               return err;
> +       }
> +
> +       /* Initialize device and start net queue */
> +       spin_lock_init(&priv->tx_lock);
> +
> +       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);
> +
> +       dev_info(ndev->dev.parent, "closing device\n");
> +       napi_disable(&priv->napi);
> +       netif_stop_queue(ndev);
> +       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;
> +
> +       printk(KERN_INFO DRV_NAME " probing devices...\n");
> +       pdata = pdev->dev.platform_data;
> +       if (!pdata) {
> +               printk(KERN_ERR "No platform data available - exiting\n");

dev_err here and below.is more appropriate.

> +               return -ENODEV;
> +       }
> +
> +       mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!mem) {
> +               printk(KERN_ERR "no mem resource?\n");
> +               err = -ENODEV;
> +               goto probe_exit;
> +       }
> +       irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +       if (!irq) {
> +               printk(KERN_ERR "no irq resource?\n");
> +               err = -ENODEV;
> +               goto probe_exit;
> +       }
> +       if (!request_mem_region(mem->start, (mem->end - mem->start) + 1,
> +               pdev->name)) {
> +               printk(KERN_ERR "HECC region already claimed\n");
> +               err = -EBUSY;
> +               goto probe_exit;
> +       }
> +       addr = ioremap(mem->start, mem->end - mem->start + 1);
> +       if (!addr) {
> +               printk(KERN_ERR "ioremap failed\n");
> +               err = -ENOMEM;
> +               goto probe_exit_free_region;
> +       }
> +
> +       ndev = alloc_candev(sizeof(struct ti_hecc_priv));
> +       if (!ndev) {
> +               printk(KERN_ERR "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_bittiming      = ti_hecc_set_bittiming;
> +       priv->can.do_set_mode           = ti_hecc_do_set_mode;
> +       priv->can.do_get_state          = ti_hecc_get_state;

Coding style!?

> +       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(ndev->dev.parent, "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,
> +                       TI_HECC_DEF_NAPI_WEIGHT);
> +
> +       err = register_candev(ndev);
> +       if (err) {
> +               dev_err(ndev->dev.parent, "register_candev() failed\n");
> +               err = -ENODEV;
> +               goto probe_exit_clk;
> +       }
> +       dev_info(ndev->dev.parent, "regs=%p, irq=%d\n",
> +               priv->base, (unsigned int) ndev->irq);

Please use a more meaningful message, e.g.:

	dev_info(&pdev->dev,
	         "device registered (reg_base=%#p, irq=%d)\n",
		 priv->reg_base, dev->irq);


> +
> +#ifdef CONFIG_DEBUG_FS
> +       hecc_debug_init(priv);
> +#endif
> +       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, mem->end - mem->start + 1);
> +probe_exit:
> +       dev_err(ndev->dev.parent, "probe error = %08X\n", err);
> +       return err;
> +}
> +
> +static int __devexit ti_hecc_remove(struct platform_device *pdev)
> +{
> +       struct resource *res;
> +       struct net_device *ndev = platform_get_drvdata(pdev);
> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
> +
> +#ifdef CONFIG_DEBUG_FS
> +       hecc_debug_exit();
> +#endif /* CONFIG_DEBUG_FS */
> +
> +       clk_put(priv->clk);
> +       res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       iounmap(priv->base);
> +       release_mem_region(res->start, res->end - res->start + 1);
> +       unregister_candev(ndev);
> +       free_candev(ndev);
> +       platform_set_drvdata(pdev, NULL);
> +       dev_info(ndev->dev.parent, "driver removed\n");
> +
> +       return 0;
> +}
> +
> +/* TI HECC netdevice driver: platform driver structure */
> +static struct platform_driver ti_hecc_driver = {
> +       .driver = {
> +               .name    = "ti_hecc",

Maybe:
                  .name    = DRV_NAME,


> +               .owner   = THIS_MODULE,
> +       },
> +       .probe = ti_hecc_probe,
> +       .remove = __devexit_p(ti_hecc_remove),
> +       .suspend = ti_hecc_suspend,
> +       .resume = ti_hecc_resume,
> +};
> +
> +static int __init ti_hecc_init_driver(void)
> +{
> +       printk(KERN_INFO DRV_DESC "\n");
> +       return platform_driver_register(&ti_hecc_driver);
> +}
> +module_init(ti_hecc_init_driver);
> +
> +static void __exit ti_hecc_exit_driver(void)
> +{
> +       printk(KERN_INFO DRV_DESC " :Exit\n");
> +       platform_driver_unregister(&ti_hecc_driver);
> +}
> +module_exit(ti_hecc_exit_driver);
> +
> +MODULE_AUTHOR("Anant Gole <anantgole@ti.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION(DRV_DESC);
> diff --git a/include/linux/can/platform/ti_hecc_platform.h b/include/linux/can/platform/ti_hecc_platform.h
> new file mode 100644
> index 0000000..4a57daf
> --- /dev/null
> +++ b/include/linux/can/platform/ti_hecc_platform.h
> @@ -0,0 +1,40 @@
> +/*
> + * TI HECC (High End CAN Controller) driver platform header
> + *
> + * 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.
> + *
> + */
> +
> +/**
> + * struct hecc_platform_data - HECC Platform Data
> + *
> + * @scc_hecc_offset:   mostly 0 - should really never change
> + * @scc_ram_offset:    SCC RAM offset
> + * @hecc_ram_offset:   HECC RAM offset
> + * @mbox_offset:       Mailbox RAM offset
> + * @int_line:          Interrupt line to use - 0 or 1
> + * @version:           version for future use
> + *
> + * Platform data structure to get all platform specific settings.
> + * this structure also accounts the fact that the IP may have different
> + * RAM and mailbox offsets for different SOC's
> + */
> +struct ti_hecc_platform_data {
> +       unsigned int scc_hecc_offset;
> +       unsigned int scc_ram_offset;
> +       unsigned int hecc_ram_offset;
> +       unsigned int mbox_offset;
> +       unsigned int int_line;
> +       unsigned int version;
> +};

Thanks for your contribution.

Wolfgang.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-29  7:06 ` Wolfgang Grandegger
@ 2009-08-31  7:22   ` Gole, Anant
  2009-08-31  8:59     ` Wolfgang Grandegger
  0 siblings, 1 reply; 13+ messages in thread
From: Gole, Anant @ 2009-08-31  7:22 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: Linux Netdev List, Socketcan-core@lists.berlios.de


>-----Original Message-----
>From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>Sent: Saturday, August 29, 2009 12:36 PM
>To: Gole, Anant
>Cc: Linux Netdev List; Socketcan-core@lists.berlios.de
>Subject: Re: [PATCH] net-next:can: add TI CAN (HECC) driver
>
>> TI HECC (High End CAN Controller) module is found on many TI devices. It
>has
>> 32 harwdare mailboxes with full implementation of CAN protocol version
>2.0B
>> and bus speeds up to 1Mbps. The module specifications are available at TI
>web
>> <http://www.ti.com>.
>>
>> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
>
>Nice driver, even using the NAPI interface. First some general comments.
>Please remove debugging code mainly useful for development, e.g. the
>CONFIG_DEBUG_FS block, many dev_info calls and special statistics
>counters. Also use dev_dbg for the remaining debug messages useful for
>the normal user, like state changes. More comments inline.

Ack on most comments you gave - some good bugs uncovered with this review. Thanks. I am answering those which I have some comment on.

[snip]

>
>> +#define HECC_MODULE_VERSION     "0.2"
>
>A version number is will usually not maintained. May drivers have it but
>it's never changed.

I understand but unless this is a strong objection I would like to keep it as it would be nice to update it when any major change happens.

[snip]

>> +
>> +/* 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,
>
>Please check if tseg2_min is a valid value. Usually it's larger.
>
Yes I did. This controller allows tseg2_min = 1

[snip]
>
>> +       struct napi_struct napi;
>> +       struct net_device *ndev;
>> +       struct clk *clk;
>> +       void __iomem *base;
>> +       unsigned int scc_ram_offset;
>> +       unsigned int hecc_ram_offset;
>> +       unsigned int mbox_offset;
>> +       unsigned int int_line;
>> +       DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
>> +       spinlock_t tx_lock;
>
>Please document the spinlock tx_lock. What is it used for.

It is to protect the tx_free_mbx bitmap above - will document.

[snip]

>Right, automatic bus-off recovery should be disabled. To make that clear:
>
>           hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_ABO);

Since CANMC register is set to 0 and ABO bit is part of it I did not make this statement explicitly (it will be redundant). I will update the comment above to state that if it changes in future then enable auto bus off with that statement

[snip]

>> +
>> +static int ti_hecc_do_set_mode(struct net_device *ndev, enum can_mode
>mode)
>> +{
>> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
>> +       int ret = 0;
>> +
>> +       switch (mode) {
>> +       case CAN_MODE_SLEEP:
>> +               dev_info(priv->ndev->dev.parent, "device going to
>sleep\n");
>> +               if (netif_running(ndev)) {
>> +                       netif_stop_queue(ndev);
>> +                       netif_device_detach(ndev);
>> +                       /* Put HECC in low power mode */
>> +                       hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>> +               }
>> +               priv->can.state = CAN_STATE_SLEEPING;
>> +               break;
>
>Has sleeping been tested? Actually, we do not have an interface yet to
>enter sleep mode. Please remove. If there is a need for it, we should
>work togehter to refine the interface and to provide a solution.
>
No I have not yet tested sleep/suspend/resume. Will remove based upon response to CAN_MODE_STOP comment below

>> +       case CAN_MODE_STOP:
>> +               dev_info(priv->ndev->dev.parent, "device stopping\n");
>> +               ti_hecc_stop(ndev);
>> +               break;
>
>Only CAN_MODE_START is used by the CAN devicde interface for bus-off
>recovery.
>
I saw other drivers (like mscan) also have the code for sleep/stop. Agreed that I have not tested it as you mentioned that the CAN infrastructure does not call it right now. Do you want me to remove it?

[snip]

>> +
>> +       }
>> +       set_bit(mbxno, priv->tx_free_mbx);
>> +       spin_unlock_irqrestore(&priv->tx_lock, flags);
>
>Hm, I wonder how the driver ensures that packages go out in order. How
>does the CAN hardware schedule pending TX message objects? Vladislav
>posted a test programs recently to check message ordering. See:
>
>https://lists.berlios.de/pipermail/socketcan-core/2009-August/002871.html

TI HECC hardware has mailbox priority field and higher mailbox number will transmit if two mailboxes have same priority level. But the code does not use that feature as yet. Let me think of how I can ensure out of order issue.

[snip]
>> +
>> +       pending_pkts = hecc_read(priv, HECC_CANRMP);
>> +       while (pending_pkts && (num_pkts < quota)) {
>> +               mbxno = find_first_bit(&pending_pkts,
>HECC_MAX_MAILBOXES);
>
>Here I also wonder if the messages are handled in the correct order.

No I have not taken care of that. Let me look into the pkt handling to see how I can do it based upon hardware capability.

[snip]

>
>> +
>> +       ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));
>
>Hm, you create an error frame for each interrupt!? What do you see with:
>
> # candump any,0:0,#FFFFFFFF
>
Good catch. I did not observe unnecessary error frames via candump (or I really missed them). But I will correct this anyway.

>> +
>> +       /* Handle Abort acknowledge interrupt */
>> +       if (int_status & HECC_CANGIF_AAIF) {
>> +               ack = hecc_read(priv, HECC_CANAA);
>> +               while (ack) {
>> +                       mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
>> +                       if (mbxno == HECC_MAX_MAILBOXES) {
>> +                               break;
>> +                       } else {
>> +                               clear_bit(mbxno, &ack);
>> +                               /* release echo pkt & update counters */
>> +                               hecc_set_bit(priv, HECC_CANAA, (1 <<
>mbxno));
>> +                               hecc_clear_bit(priv, HECC_CANME, (1 <<
>mbxno));
>> +                               /* FIXME: since net-next tree's dev.h
>does not
>> +                                * include can_free_echo_skb() doing
>equivalent
>> +                                * of can_free_echo_skb(ndev, mbxno);
>> +                                */
>> +                               if (priv->can.echo_skb[mbxno]) {
>> +                                       kfree_skb(priv-
>>can.echo_skb[mbxno]);
>> +                                       priv->can.echo_skb[mbxno] = NULL;
>> +                               }
>> +                               if (netif_queue_stopped(ndev))
>> +                                       netif_wake_queue(ndev);
>> +                               spin_lock_irqsave(&priv->tx_lock, flags);
>> +                               clear_bit(mbxno, priv->tx_free_mbx);
>> +                               spin_unlock_irqrestore(&priv->tx_lock,
>flags);
>> +                       }
>> +               }
>> +       }
>
>Can that interrupt happen? I have not found any code aborting messages.

It can happen only if a tx mailbox is told to abort. This interrupt is an ack for the same. I am not using the tx abort feature so will remove this.

[snip]
>> +       }
>> +
>> +       /* Open common can device */
>> +       err = open_candev(ndev);
>> +       if (err) {
>> +               dev_err(ndev->dev.parent, "open_candev() failed %08X\n",
>err);
>
>free_irq?

Good catch. Will fix it.

>
>> +               return err;
>> +       }
>> +
>
>Thanks for your contribution.
>
>Wolfgang.
>

Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.

--
Anant

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-31  7:22   ` Gole, Anant
@ 2009-08-31  8:59     ` Wolfgang Grandegger
  2009-08-31 10:29       ` Oliver Hartkopp
  0 siblings, 1 reply; 13+ messages in thread
From: Wolfgang Grandegger @ 2009-08-31  8:59 UTC (permalink / raw)
  To: Gole, Anant; +Cc: Socketcan-core@lists.berlios.de, Linux Netdev List

Gole, Anant wrote:
>> -----Original Message-----
>> From: Wolfgang Grandegger [mailto:wg@grandegger.com]
>> Sent: Saturday, August 29, 2009 12:36 PM
>> To: Gole, Anant
>> Cc: Linux Netdev List; Socketcan-core@lists.berlios.de
>> Subject: Re: [PATCH] net-next:can: add TI CAN (HECC) driver
>>
>>> TI HECC (High End CAN Controller) module is found on many TI devices. It
>> has
>>> 32 harwdare mailboxes with full implementation of CAN protocol version
>> 2.0B
>>> and bus speeds up to 1Mbps. The module specifications are available at TI
>> web
>>> <http://www.ti.com>.
>>>
>>> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
>> Nice driver, even using the NAPI interface. First some general comments.
>> Please remove debugging code mainly useful for development, e.g. the
>> CONFIG_DEBUG_FS block, many dev_info calls and special statistics
>> counters. Also use dev_dbg for the remaining debug messages useful for
>> the normal user, like state changes. More comments inline.
> 
> Ack on most comments you gave - some good bugs uncovered with this review. Thanks. I am answering those which I have some comment on.
> 
> [snip]
> 
>>> +#define HECC_MODULE_VERSION     "0.2"
>> A version number is will usually not maintained. May drivers have it but
>> it's never changed.
> 
> I understand but unless this is a strong objection I would like to keep it as it would be nice to update it when any major change happens.

It's *not* a strong objection but I personally believe that git is doing
a much better job.

> [snip]
> 
>>> +
>>> +/* 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,
>> Please check if tseg2_min is a valid value. Usually it's larger.
>>
> Yes I did. This controller allows tseg2_min = 1

OK.
> 
> [snip]
>>> +       struct napi_struct napi;
>>> +       struct net_device *ndev;
>>> +       struct clk *clk;
>>> +       void __iomem *base;
>>> +       unsigned int scc_ram_offset;
>>> +       unsigned int hecc_ram_offset;
>>> +       unsigned int mbox_offset;
>>> +       unsigned int int_line;
>>> +       DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
>>> +       spinlock_t tx_lock;
>> Please document the spinlock tx_lock. What is it used for.
> 
> It is to protect the tx_free_mbx bitmap above - will document.

I know, but for kernel inclusion it should be documented.

> [snip]
> 
>> Right, automatic bus-off recovery should be disabled. To make that clear:
>>
>>           hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_ABO);
> 
> Since CANMC register is set to 0 and ABO bit is part of it I did not make this statement explicitly (it will be redundant). I will update the comment above to state that if it changes in future then enable auto bus off with that statement

OK.

> [snip]
> 
>>> +
>>> +static int ti_hecc_do_set_mode(struct net_device *ndev, enum can_mode
>> mode)
>>> +{
>>> +       struct ti_hecc_priv *priv = netdev_priv(ndev);
>>> +       int ret = 0;
>>> +
>>> +       switch (mode) {
>>> +       case CAN_MODE_SLEEP:
>>> +               dev_info(priv->ndev->dev.parent, "device going to
>> sleep\n");
>>> +               if (netif_running(ndev)) {
>>> +                       netif_stop_queue(ndev);
>>> +                       netif_device_detach(ndev);
>>> +                       /* Put HECC in low power mode */
>>> +                       hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
>>> +               }
>>> +               priv->can.state = CAN_STATE_SLEEPING;
>>> +               break;
>> Has sleeping been tested? Actually, we do not have an interface yet to
>> enter sleep mode. Please remove. If there is a need for it, we should
>> work togehter to refine the interface and to provide a solution.
>>
> No I have not yet tested sleep/suspend/resume. Will remove based upon response to CAN_MODE_STOP comment below
> 
>>> +       case CAN_MODE_STOP:
>>> +               dev_info(priv->ndev->dev.parent, "device stopping\n");
>>> +               ti_hecc_stop(ndev);
>>> +               break;
>> Only CAN_MODE_START is used by the CAN devicde interface for bus-off
>> recovery.
>>
> I saw other drivers (like mscan) also have the code for sleep/stop. Agreed that I have not tested it as you mentioned that the CAN infrastructure does not call it right now. Do you want me to remove it?

Yes, for kernel inclusion it should be removed. We are more relaxed to
keep that "preliminary" and "un-tested" code in the BerliOS SVN
repository. Note that the MSCAN driver is not yet mainline

> [snip]
> 
>>> +
>>> +       }
>>> +       set_bit(mbxno, priv->tx_free_mbx);
>>> +       spin_unlock_irqrestore(&priv->tx_lock, flags);
>> Hm, I wonder how the driver ensures that packages go out in order. How
>> does the CAN hardware schedule pending TX message objects? Vladislav
>> posted a test programs recently to check message ordering. See:
>>
>> https://lists.berlios.de/pipermail/socketcan-core/2009-August/002871.html
> 
> TI HECC hardware has mailbox priority field and higher mailbox number will transmit if two mailboxes have same priority level. But the code does not use that feature as yet. Let me think of how I can ensure out of order issue.

Have a look to the AT91 driver, it seems to be quite similar.

> [snip]
>>> +
>>> +       pending_pkts = hecc_read(priv, HECC_CANRMP);
>>> +       while (pending_pkts && (num_pkts < quota)) {
>>> +               mbxno = find_first_bit(&pending_pkts,
>> HECC_MAX_MAILBOXES);
>>
>> Here I also wonder if the messages are handled in the correct order.
> 
> No I have not taken care of that. Let me look into the pkt handling to see how I can do it based upon hardware capability.
> 
> [snip]
> 
>>> +
>>> +       ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));
>> Hm, you create an error frame for each interrupt!? What do you see with:
>>
>> # candump any,0:0,#FFFFFFFF
>>
> Good catch. I did not observe unnecessary error frames via candump (or I really missed them). But I will correct this anyway.

I see. That's because the error code of can_id is 0 and the mask does
not trigger.

>>> +
>>> +       /* Handle Abort acknowledge interrupt */
>>> +       if (int_status & HECC_CANGIF_AAIF) {
>>> +               ack = hecc_read(priv, HECC_CANAA);
>>> +               while (ack) {
>>> +                       mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
>>> +                       if (mbxno == HECC_MAX_MAILBOXES) {
>>> +                               break;
>>> +                       } else {
>>> +                               clear_bit(mbxno, &ack);
>>> +                               /* release echo pkt & update counters */
>>> +                               hecc_set_bit(priv, HECC_CANAA, (1 <<
>> mbxno));
>>> +                               hecc_clear_bit(priv, HECC_CANME, (1 <<
>> mbxno));
>>> +                               /* FIXME: since net-next tree's dev.h
>> does not
>>> +                                * include can_free_echo_skb() doing
>> equivalent
>>> +                                * of can_free_echo_skb(ndev, mbxno);
>>> +                                */
>>> +                               if (priv->can.echo_skb[mbxno]) {
>>> +                                       kfree_skb(priv-
>>> can.echo_skb[mbxno]);
>>> +                                       priv->can.echo_skb[mbxno] = NULL;
>>> +                               }
>>> +                               if (netif_queue_stopped(ndev))
>>> +                                       netif_wake_queue(ndev);
>>> +                               spin_lock_irqsave(&priv->tx_lock, flags);
>>> +                               clear_bit(mbxno, priv->tx_free_mbx);
>>> +                               spin_unlock_irqrestore(&priv->tx_lock,
>> flags);
>>> +                       }
>>> +               }
>>> +       }
>> Can that interrupt happen? I have not found any code aborting messages.
> 
> It can happen only if a tx mailbox is told to abort. This interrupt is an ack for the same. I am not using the tx abort feature so will remove this.
> 
> [snip]
>>> +       }
>>> +
>>> +       /* Open common can device */
>>> +       err = open_candev(ndev);
>>> +       if (err) {
>>> +               dev_err(ndev->dev.parent, "open_candev() failed %08X\n",
>> err);
>>
>> free_irq?
> 
> Good catch. Will fix it.
> 
>>> +               return err;
>>> +       }
>>> +
>> Thanks for your contribution.
>>
>> Wolfgang.
>>
> 
> Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.

v2 will fit but I'm wondering who is moderating the ML. Oliver?

Wolfgang.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-31  8:59     ` Wolfgang Grandegger
@ 2009-08-31 10:29       ` Oliver Hartkopp
  2009-08-31 10:43         ` Jan Kiszka
  0 siblings, 1 reply; 13+ messages in thread
From: Oliver Hartkopp @ 2009-08-31 10:29 UTC (permalink / raw)
  To: Wolfgang Grandegger, Jan Kiszka
  Cc: Gole, Anant, Socketcan-core@lists.berlios.de, Linux Netdev List

Wolfgang Grandegger wrote:
> Gole, Anant wrote:


>>>
>> Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.
> 
> v2 will fit but I'm wondering who is moderating the ML. Oliver?
> 

No. SocketCAN Core is Jan Kiszka.

Jan, can you increase the message size in the SocketCAN-core-ML settings to 150kB?

Regards,
Oliver


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-31 10:29       ` Oliver Hartkopp
@ 2009-08-31 10:43         ` Jan Kiszka
  0 siblings, 0 replies; 13+ messages in thread
From: Jan Kiszka @ 2009-08-31 10:43 UTC (permalink / raw)
  To: Oliver Hartkopp
  Cc: Wolfgang Grandegger, Gole, Anant, Socketcan-core@lists.berlios.de,
	Linux Netdev List

Oliver Hartkopp wrote:
> Wolfgang Grandegger wrote:
>> Gole, Anant wrote:
> 
> 
>>> Appreciate the review. I will send v2 of patch. Not sure still if the original patch reached the socketcan mailing list due to size.
>> v2 will fit but I'm wondering who is moderating the ML. Oliver?
>>
> 
> No. SocketCAN Core is Jan Kiszka.
> 
> Jan, can you increase the message size in the SocketCAN-core-ML settings to 150kB?

Done.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-08-28 11:18 Anant Gole
  2009-08-28 12:44 ` Oliver Hartkopp
@ 2009-09-01  9:04 ` Wolfram Sang
  2009-09-01  9:32   ` Gole, Anant
  1 sibling, 1 reply; 13+ messages in thread
From: Wolfram Sang @ 2009-09-01  9:04 UTC (permalink / raw)
  To: Anant Gole; +Cc: socketcan-core, netdev

[-- Attachment #1: Type: text/plain, Size: 52343 bytes --]

Hello Anant,

On Fri, Aug 28, 2009 at 04:48:02PM +0530, Anant Gole wrote:
> TI HECC (High End CAN Controller) module is found on many TI devices. It has
> 32 harwdare mailboxes with full implementation of CAN protocol version 2.0B
> and bus speeds up to 1Mbps. The module specifications are available at TI web
> <http://www.ti.com>.
> 
> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.

One minor thing I found while glimpsing at the code...

> 
> Signed-off-by: Anant Gole <anantgole@ti.com>
> ---
>  drivers/net/can/Kconfig                       |    9 +
>  drivers/net/can/Makefile                      |    2 +
>  drivers/net/can/ti_hecc.c                     | 1352 +++++++++++++++++++++++++
>  include/linux/can/platform/ti_hecc_platform.h |   40 +
>  4 files changed, 1403 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/can/ti_hecc.c
>  create mode 100644 include/linux/can/platform/ti_hecc_platform.h
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 30ae55d..fae62df 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -75,6 +75,15 @@ 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 (HECC)"
> +        default N
> +        ---help---
> +	  This driver adds support for TI High End CAN Controller module
> +	  found on many TI devices. The specifications of this module are
> +	  are available from TI web (http://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..d923512 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -9,4 +9,6 @@ can-dev-y			:= dev.o
>  
>  obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
>  
> +obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
> +
>  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..4741b4a
> --- /dev/null
> +++ b/drivers/net/can/ti_hecc.c
> @@ -0,0 +1,1352 @@
> +/*
> + * 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.
> + *
> + */
> +
> +#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>
> +#include <linux/can.h>
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/can/platform/ti_hecc_platform.h>
> +
> +#define DRV_NAME "TI HECC"
> +#define HECC_MODULE_VERSION     "0.2"
> +MODULE_VERSION(HECC_MODULE_VERSION);
> +#define DRV_DESC "TI High End CAN Controller Driver " HECC_MODULE_VERSION
> +
> +#define HECC_MAX_MAILBOXES	32	/* hardware mboxes - do not change */
> +
> +#if (CAN_ECHO_SKB_MAX > 16)
> +#define TI_HECC_MAX_TX_MBOX	16
> +#else
> +#define TI_HECC_MAX_TX_MBOX	CAN_ECHO_SKB_MAX
> +#endif
> +#define TI_HECC_MAX_RX_MBOX	(HECC_MAX_MAILBOXES - TI_HECC_MAX_TX_MBOX)
> +
> +#define TI_HECC_DEF_NAPI_WEIGHT	TI_HECC_MAX_RX_MBOX
> +
> +/* 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)
> +
> +#define HECC_SET_REG		0xFFFFFFFF
> +#define HECC_CANID_MASK		0x3FF	/* 18 bits mask for extended id's */
> +
> +#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	0xFF00	/* all except maif and tcoif */
> +#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;
> +	struct napi_struct napi;
> +	struct net_device *ndev;
> +	struct clk *clk;
> +	void __iomem *base;
> +	unsigned int scc_ram_offset;
> +	unsigned int hecc_ram_offset;
> +	unsigned int mbox_offset;
> +	unsigned int int_line;
> +	DECLARE_BITMAP(tx_free_mbx, TI_HECC_MAX_TX_MBOX);
> +	spinlock_t tx_lock;
> +
> +	/* Statistics */
> +	unsigned out_of_tx_mbox;
> +	unsigned write_denied_cnt;
> +	unsigned message_lost_cnt;
> +	unsigned wake_up_cnt;
> +	unsigned message_alarm_cnt;
> +	unsigned timer_overflow_cnt;
> +};
> +
> +static inline
> +void hecc_write(struct ti_hecc_priv *priv, int reg, unsigned int val)
> +{
> +	__raw_writel(val, priv->base + reg);
> +}
> +
> +static inline unsigned int 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));
> +}
> +
> +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
> +unsigned int hecc_get_bit(struct ti_hecc_priv *priv, int reg, int bit)
> +{
> +	return (hecc_read(priv, reg) & bit) ? 1 : 0;
> +}
> +
> +#ifdef CONFIG_DEBUG_FS
> +
> +static struct ti_hecc_priv *debug_priv;
> +
> +#define PRINTMBOXREG(r) seq_printf(s, "%d\t%08X %08X %08X %08X %08X\n", r,\
> +	hecc_read(debug_priv, HECC_CANMID(r)),\
> +	hecc_read(debug_priv, HECC_CANMCF(r)),\
> +	hecc_read(debug_priv, HECC_CANMDH(r)),\
> +	hecc_read(debug_priv, HECC_CANMDL(r)),\
> +	hecc_read(debug_priv, HECC_CANLAM(r)))
> +
> +/* Print mailbox data */
> +static void hecc_print_mbox_regs(struct seq_file *s)
> +{
> +	int cnt = 0;
> +	static struct ti_hecc_priv *priv;
> +	priv = debug_priv;
> +	seq_printf(s, "\n--- %s %s - mailbox regs ---\n\n",
> +		DRV_NAME, HECC_MODULE_VERSION);
> +	seq_printf(s, "MbxNo\tMID\t MCF\t  MDH\t   MDL\t   LAM\n");
> +	seq_printf(s, "-----------------------------------------------\n");
> +	for (cnt = 0; cnt < HECC_MAX_MAILBOXES; cnt++)
> +		PRINTMBOXREG(cnt);
> +}
> +
> +#define PRINTREG(d, r) seq_printf(s, "%s\t%08x\n", d, hecc_read(debug_priv, r))
> +/* Print HECC registers */
> +static void hecc_print_regs(struct seq_file *s)
> +{
> +	static struct ti_hecc_priv *priv;
> +	priv = debug_priv;
> +
> +	seq_printf(s, "\n--- %s %s - module regs ---\n\n",
> +		DRV_NAME, HECC_MODULE_VERSION);
> +	PRINTREG("HECC_CANME\tMailbox Enable\t", HECC_CANME);
> +	PRINTREG("HECC_CANMD\tMailbox Direction", HECC_CANMD);
> +	PRINTREG("HECC_CANTRS\tTransmit request set", HECC_CANTRS);
> +	PRINTREG("HECC_CANTRR\tTransmit request reset", HECC_CANTRR);
> +	PRINTREG("HECC_CANTA\tTransmission ack", HECC_CANTA);
> +	PRINTREG("HECC_CANAA\tAbort acknowledge", HECC_CANAA);
> +	PRINTREG("HECC_CANRMP\tReceive message pending", HECC_CANRMP);
> +	PRINTREG("HECC_CANRML\tRemote message lost", HECC_CANRML);
> +	PRINTREG("HECC_CANRFP\tRemote frame pending", HECC_CANRFP);
> +	PRINTREG("HECC_CANGAM (SCC) Global accept mask", HECC_CANGAM);
> +	PRINTREG("HECC_CANMC\tMaster control\t", HECC_CANMC);
> +	PRINTREG("HECC_CANBTC\tBit timing config", HECC_CANBTC);
> +	PRINTREG("HECC_CANES\tError and status", HECC_CANES);
> +	PRINTREG("HECC_CANTEC\tTransmit error counter", HECC_CANTEC);
> +	PRINTREG("HECC_CANREC\tReceive error counter", HECC_CANREC);
> +	PRINTREG("HECC_CANGIF0\tGlobal interrupt flag 0", HECC_CANGIF0);
> +	PRINTREG("HECC_CANGIM\tGlobal interrupt mask", HECC_CANGIM);
> +	PRINTREG("HECC_CANGIF1\tGlobal interrupt flag 1", HECC_CANGIF1);
> +	PRINTREG("HECC_CANMIM\tMailbox interrupt mask", HECC_CANMIM);
> +	PRINTREG("HECC_CANMIL\tMailbox interrupt level", HECC_CANMIL);
> +	PRINTREG("HECC_CANOPC\tOverwrite prot control", HECC_CANOPC);
> +	PRINTREG("HECC_CANTIOC\tTransmit I/O control", HECC_CANTIOC);
> +	PRINTREG("HECC_CANRIOC\tReceive I/O control", HECC_CANRIOC);
> +	PRINTREG("HECC_CANLNT (HECC) Local network time", HECC_CANLNT);
> +	PRINTREG("HECC_CANTOC (HECC) Time-out control", HECC_CANTOC);
> +	PRINTREG("HECC_CANTOS (HECC) Time-out status", HECC_CANTOS);
> +	PRINTREG("HECC_CANTIOCE (SCC) Enh TX I/O ctrl", HECC_CANTIOCE);
> +	PRINTREG("HECC_CANRIOCE (SCC) Enh RX I/O ctrl", HECC_CANRIOCE);
> +	seq_printf(s, "HECC_CANLAM0 (SCC) \t\t\t%08X\n",
> +		hecc_read(priv, HECC_CANLAM0));
> +	seq_printf(s, "HECC_CANLAM3 (SCC) \t\t\t%08X\n",
> +		hecc_read(priv, HECC_CANLAM3));
> +	seq_printf(s, "\n");
> +}
> +
> +static char *hecc_debug_can_state[] = {
> +	"CAN_STATE_ERROR_ACTIVE",
> +	"CAN_STATE_ERROR_WARNING",
> +	"CAN_STATE_ERROR_PASSIVE",
> +	"CAN_STATE_BUS_OFF",
> +	"CAN_STATE_STOPPED",
> +	"CAN_STATE_SLEEPING",
> +	"CAN_STATE_MAX"
> +};
> +
> +/* Print status and statistics */
> +static void hecc_print_status(struct seq_file *s)
> +{
> +	seq_printf(s, "\n--- %s %s - status ---\n\n",
> +		DRV_NAME, HECC_MODULE_VERSION);
> +	seq_printf(s, "\n--- ti_hecc status ---\n\n");
> +	seq_printf(s, "CAN state \t\t= %s\n",
> +		hecc_debug_can_state[debug_priv->can.state]);
> +	seq_printf(s, "CAN restart_ms \t\t= %u\n", debug_priv->can.restart_ms);
> +	seq_printf(s, "CAN input clock \t= %u\n", debug_priv->can.clock.freq);
> +	seq_printf(s, "CAN Bittiming\n");
> +	seq_printf(s, "\tbitrate \t= %u\n",
> +			debug_priv->can.bittiming.bitrate);
> +	seq_printf(s, "\tsample_point \t= %u\n",
> +			debug_priv->can.bittiming.sample_point);
> +	seq_printf(s, "\ttq \t\t= %u\n",
> +			debug_priv->can.bittiming.tq);
> +	seq_printf(s, "\tprop_seg \t= %u\n",
> +			debug_priv->can.bittiming.prop_seg);
> +	seq_printf(s, "\tphase_seg1 \t= %u\n",
> +			debug_priv->can.bittiming.phase_seg1);
> +	seq_printf(s, "\tphase_seg2 \t= %u\n",
> +			debug_priv->can.bittiming.phase_seg2);
> +	seq_printf(s, "\tsjw \t\t= %u\n",
> +			debug_priv->can.bittiming.sjw);
> +	seq_printf(s, "\tbrp \t\t= %u\n",
> +			debug_priv->can.bittiming.brp);
> +	seq_printf(s, "CAN Bittiming Constants\n");
> +	seq_printf(s, "\ttseg1 min-max \t= %u-%u\n",
> +			debug_priv->can.bittiming_const->tseg1_min,
> +			debug_priv->can.bittiming_const->tseg1_max);
> +	seq_printf(s, "\ttseg2 min-max \t= %u-%u\n",
> +			debug_priv->can.bittiming_const->tseg2_min,
> +			debug_priv->can.bittiming_const->tseg2_max);
> +	seq_printf(s, "\tsjw_max \t= %u\n",
> +			debug_priv->can.bittiming_const->sjw_max);
> +	seq_printf(s, "\tbrp min-max \t= %u-%u\n",
> +			debug_priv->can.bittiming_const->brp_min,
> +			debug_priv->can.bittiming_const->brp_max);
> +	seq_printf(s, "\tbrp_inc \t= %u\n",
> +			debug_priv->can.bittiming_const->brp_inc);
> +	seq_printf(s, "CAN out of TX mbox\t= %d\n",
> +		debug_priv->out_of_tx_mbox);
> +	seq_printf(s, "CAN write denied cnt \t= %d\n",
> +		debug_priv->write_denied_cnt);
> +	seq_printf(s, "CAN message lost cnt \t= %d\n",
> +		debug_priv->message_lost_cnt);
> +	seq_printf(s, "CAN wake up cnt \t= %d\n", debug_priv->wake_up_cnt);
> +	seq_printf(s, "CAN message alarm cnt \t= %d\n",
> +		debug_priv->message_alarm_cnt);
> +	seq_printf(s, "CAN timer overflow cnt\t= %d\n",
> +		debug_priv->timer_overflow_cnt);
> +	/* CAN statistics */
> +	seq_printf(s, "CAN stats bus error \t= %d\n",
> +		debug_priv->can.can_stats.bus_error);
> +	seq_printf(s, "CAN stats error warning\t= %d\n",
> +		debug_priv->can.can_stats.error_warning);
> +	seq_printf(s, "CAN stats error passive\t= %d\n",
> +		debug_priv->can.can_stats.error_passive);
> +	seq_printf(s, "CAN stats bus off\t= %d\n",
> +		debug_priv->can.can_stats.bus_off);
> +	seq_printf(s, "CAN stats arbitration lost= %d\n",
> +		debug_priv->can.can_stats.arbitration_lost);
> +	seq_printf(s, "CAN stats restarts\t= %d\n",
> +		debug_priv->can.can_stats.restarts);
> +}
> +
> +/** Toggle HECC Self-Test i.e loopback bit
> + * INFO: Reading this debug variable toggles the loopback status on the device.
> + * This is done to simplify the debug function to set looback
> + */
> +static int hecc_debug_loopback(struct seq_file *s)
> +{
> +	static int toggle;
> +
> +	/* Put module in self test mode i.e. loopback */
> +	if (toggle) {
> +		seq_printf(s, "In Self Test Mode (loopback)\n");
> +		hecc_set_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
> +		toggle = 0;
> +	} else {
> +		seq_printf(s, "Out of Self Test Mode (NO loopback)\n");
> +		hecc_clear_bit(debug_priv, HECC_CANMC, HECC_CANMC_STM);
> +		toggle = 1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hecc_debug_show(struct seq_file *s, void *unused)
> +{
> +	void (*func)(struct seq_file *) = s->private;
> +	func(s);
> +	return 0;
> +}
> +
> +static int hecc_debug_open(struct inode *inode, struct file *file)
> +{
> +	return single_open(file, hecc_debug_show, inode->i_private);
> +}
> +
> +/* HECC debug operations */
> +static const struct file_operations hecc_debug_fops = {
> +	.open		= hecc_debug_open,
> +	.read		= seq_read,
> +	.llseek		= seq_lseek,
> +	.release	= single_release,
> +};
> +
> +static struct dentry *hecc_dir;
> +static unsigned int hecc_debug_state;
> +
> +static int hecc_debug_init(struct ti_hecc_priv *priv)
> +{
> +	debug_priv = priv;
> +
> +	hecc_dir = debugfs_create_dir("ti_hecc", NULL);
> +	if (IS_ERR(hecc_dir)) {
> +		int err = PTR_ERR(hecc_dir);
> +		hecc_dir = NULL;
> +		return err;
> +	}
> +
> +	debugfs_create_file("ti_hecc_regs", S_IRUGO,
> +			hecc_dir, &hecc_print_regs, &hecc_debug_fops);
> +	debugfs_create_file("ti_hecc_mbox_regs", S_IRUGO,
> +			hecc_dir, &hecc_print_mbox_regs, &hecc_debug_fops);
> +	debugfs_create_file("ti_hecc_status", S_IRUGO,
> +			hecc_dir, &hecc_print_status, &hecc_debug_fops);
> +	debugfs_create_file("ti_hecc_loopback", S_IRUGO,
> +			hecc_dir, &hecc_debug_loopback, &hecc_debug_fops);
> +	debugfs_create_u32("ti_hecc_debug", S_IWUGO,
> +			hecc_dir, &hecc_debug_state);
> +	debugfs_create_u32("ti_hecc_bittiming", S_IWUGO,
> +			hecc_dir, &debug_priv->can.bittiming.bitrate);
> +	debugfs_create_u32("ti_hecc_restart_ms", S_IWUGO,
> +			hecc_dir, &debug_priv->can.restart_ms);
> +	debugfs_create_u32("prop_seg", S_IWUGO,
> +			hecc_dir, &debug_priv->can.bittiming.prop_seg);
> +	debugfs_create_u32("phase_seg2", S_IWUGO,
> +			hecc_dir, &debug_priv->can.bittiming.phase_seg2);
> +	debugfs_create_u32("phase_seg1", S_IWUGO,
> +			hecc_dir, &debug_priv->can.bittiming.phase_seg1);
> +	debugfs_create_u32("brp", S_IWUGO,
> +			hecc_dir, &debug_priv->can.bittiming.brp);
> +	debugfs_create_u32("sjw", S_IWUGO,
> +			hecc_dir, &debug_priv->can.bittiming.sjw);
> +
> +	return 0;
> +}
> +
> +static void hecc_debug_exit(void)
> +{
> +	if (hecc_dir)
> +		debugfs_remove_recursive(hecc_dir);
> +}
> +
> +#endif /* CONFIG_DEBUG_FS */
> +
> +
> +static int ti_hecc_get_state(const struct net_device *ndev,
> +	enum can_state *state)
> +{
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +	*state = priv->can.state;
> +	return 0;
> +}
> +
> +static int ti_hecc_set_bittiming(struct net_device *ndev)
> +{
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +	struct can_bittiming *bit_timing = &priv->can.bittiming;
> +	unsigned int can_btc = 0;
> +
> +	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)
> +		can_btc |= HECC_CANBTC_SAM;
> +	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);
> +
> +	return 0;
> +}
> +
> +/**
> + * ti_hecc_reset: Reset HECC module and set bit timings
> + *
> + * Resets HECC by writing to change config request bit and then sets
> + * bit-timing registers in the module to enable the module for operation
> + */
> +static void ti_hecc_reset(struct net_device *ndev)
> +{
> +#define HECC_CCE_WAIT_COUNT	1000
> +	unsigned int 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);
> +
> +	/* if change control request not enabled */
> +	if (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
> +		/* 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 large counter to respect the specs
> +		 */
> +		cnt = HECC_CCE_WAIT_COUNT;
> +		while (!hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
> +			--cnt;
> +			if (0 == cnt) {
> +				dev_info(ndev->dev.parent,
> +					"timing out on CCE after reset\n");
> +				break;
> +			}
> +			if (printk_ratelimit())
> +				dev_dbg(ndev->dev.parent,
> +					"waiting CCE after reset\n");
> +		}
> +	}
> +
> +	/* Set bit timing on the device */
> +	ti_hecc_set_bittiming(priv->ndev);
> +
> +	/* 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);
> +	*/
> +
> +	/* Wait till CCE bit clears */
> +	/* 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 large counter to respect the specs
> +	 */
> +	cnt = HECC_CCE_WAIT_COUNT;
> +	while (hecc_get_bit(priv, HECC_CANES, HECC_CANES_CCE)) {
> +		--cnt;
> +		if (0 == cnt) {
> +			dev_info(ndev->dev.parent,
> +				"timing out on CCE after bittiming\n");
> +			break;
> +		}
> +		if (printk_ratelimit())
> +			dev_dbg(ndev->dev.parent,
> +				"waiting CCE after bittiming\n");
> +	}
> +
> +	/* 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);
> +}
> +
> +/**
> + * ti_hecc_start: Starts HECC module
> + *
> + * If CAN state is not stopped, reset the module, init bit timings
> + * and start the device for operation
> + */
> +static void ti_hecc_start(struct net_device *ndev)
> +{
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +	int cnt, mbxno;
> +
> +	ti_hecc_reset(ndev);
> +
> +	bitmap_zero(priv->tx_free_mbx, TI_HECC_MAX_TX_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 < TI_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_CANTRS, 0);
> +	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)
> +{
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +	int ret = 0;
> +
> +	switch (mode) {
> +	case CAN_MODE_SLEEP:
> +		dev_info(priv->ndev->dev.parent, "device going to sleep\n");
> +		if (netif_running(ndev)) {
> +			netif_stop_queue(ndev);
> +			netif_device_detach(ndev);
> +			/* Put HECC in low power mode */
> +			hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> +		}
> +		priv->can.state = CAN_STATE_SLEEPING;
> +		break;
> +	case CAN_MODE_STOP:
> +		dev_info(priv->ndev->dev.parent, "device stopping\n");
> +		ti_hecc_stop(ndev);
> +		break;
> +	case CAN_MODE_START:
> +		dev_info(priv->ndev->dev.parent, "device (re)starting\n");
> +		++priv->can.can_stats.restarts;
> +		ti_hecc_start(ndev);
> +		if (netif_queue_stopped(ndev))
> +			netif_wake_queue(ndev);
> +		break;
> +	default:
> +		ret = -EOPNOTSUPP;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int ti_hecc_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct can_frame *cf = (struct can_frame *)skb->data;
> +	u32 mbxno = 0;
> +	u32 data;
> +	unsigned long flags;
> +
> +	/* Find the first mailbox that is free for xmit */
> +	spin_lock_irqsave(&priv->tx_lock, flags);
> +	mbxno = find_first_zero_bit(priv->tx_free_mbx, TI_HECC_MAX_TX_MBOX);
> +	if (mbxno == TI_HECC_MAX_TX_MBOX) {
> +		netif_stop_queue(ndev);
> +		if (printk_ratelimit())
> +			dev_err(priv->ndev->dev.parent,
> +				"Out of TX buffers ...\n");
> +		spin_unlock_irqrestore(&priv->tx_lock, flags);
> +		return NETDEV_TX_BUSY;
> +
> +	}
> +	set_bit(mbxno, priv->tx_free_mbx);
> +	spin_unlock_irqrestore(&priv->tx_lock, flags);
> +
> +	/* Prepare mailbox for transmission */
> +	hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
> +	data = cf->can_dlc & 0xF;
> +	if (cf->can_id & CAN_RTR_FLAG) /* Remote transmission request */
> +		data |= HECC_CANMCF_RTR;
> +	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);
> +	data = (cf->data[0] << 24) | (cf->data[1] << 16) |
> +			(cf->data[2] << 8) | cf->data[3];
> +	hecc_write(priv, HECC_CANMDL(mbxno), data);
> +	if (cf->can_dlc > 4) {
> +		data = (cf->data[4] << 24) | (cf->data[5] << 16) |
> +			(cf->data[6] << 8) | cf->data[7];
> +		hecc_write(priv, HECC_CANMDH(mbxno), data);
> +	}
> +
> +#ifdef CONFIG_DEBUG_FS
> +	if (hecc_debug_state) {
> +		printk(KERN_ERR "Mbxno=%d, CANMID=%08X, CANMCF=%08X," \
> +			" CANMDH=%08X, CANMDL=%08X\n", mbxno,
> +			hecc_read(priv, HECC_CANMID(mbxno)),
> +			hecc_read(priv, HECC_CANMCF(mbxno)),
> +			hecc_read(priv, HECC_CANMDH(mbxno)),
> +			hecc_read(priv, HECC_CANMDL(mbxno)));
> +		printk(KERN_INFO "HECC_TX: %02X, %02X, %02X, %02X, %02X," \
> +			" %02X, %02X, %02X\n",
> +			cf->data[0], cf->data[1], cf->data[2], cf->data[3],
> +			cf->data[4], cf->data[5], cf->data[6], cf->data[7]);
> +	}
> +#endif /* CONFIG_DEBUG_FS */
> +
> +	/* Enable interrupt for mbox and start transmission */
> +	hecc_clear_bit(priv, HECC_CANMD, (1 << mbxno));
> +	hecc_set_bit(priv, HECC_CANME, (1 << mbxno));
> +	hecc_set_bit(priv, HECC_CANMIM, (1 << mbxno));
> +	hecc_set_bit(priv, HECC_CANTRS, (1 << mbxno));
> +
> +	stats->tx_bytes += cf->can_dlc;
> +	ndev->trans_start = jiffies;
> +	can_put_echo_skb(skb, ndev, 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;
> +
> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> +	if (!skb) {
> +		if (printk_ratelimit())
> +			dev_err(priv->ndev->dev.parent,
> +				"dev_alloc_skb() failed\n");
> +		return -ENOMEM;
> +	}
> +	skb->dev = priv->ndev;
> +	skb->protocol = 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));
> +	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));
> +	/* The below statements are for readability sake */
> +	cf->data[0] = ((data & 0xFF000000) >> 24);
> +	cf->data[1] = ((data & 0xFF0000) >> 16);
> +	cf->data[2] = ((data & 0xFF00) >> 8);
> +	cf->data[3] = (data & 0xFF);
> +	if (cf->can_dlc > 4) {
> +		data = hecc_read(priv, HECC_CANMDH(mbxno));
> +		cf->data[4] = ((data & 0xFF000000) >> 24);
> +		cf->data[5] = ((data & 0xFF0000) >> 16);
> +		cf->data[6] = ((data & 0xFF00) >> 8);
> +		cf->data[7] = (data & 0xFF);
> +	}
> +
> +#ifdef CONFIG_DEBUG_FS
> +	if (hecc_debug_state) {
> +		printk(KERN_ERR "Mbxno=%d, CANMID=%08X, CANMCF=%08X," \
> +			" CANMDH=%08X, CANMDL=%08X\n", mbxno,
> +			hecc_read(priv, HECC_CANMID(mbxno)),
> +			hecc_read(priv, HECC_CANMCF(mbxno)),
> +			hecc_read(priv, HECC_CANMDH(mbxno)),
> +			hecc_read(priv, HECC_CANMDL(mbxno)));
> +		printk(KERN_INFO "HECC_RX: %02X, %02X, %02X, %02X, %02X,"\
> +			" %02X, %02X, %02X\n",
> +			cf->data[0], cf->data[1], cf->data[2], cf->data[3],
> +			cf->data[4], cf->data[5], cf->data[6], cf->data[7]);
> +	}
> +#endif /* CONFIG_DEBUG_FS */
> +
> +	/* prepare mailbox for next receive */
> +	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));
> +
> +	stats->rx_bytes += cf->can_dlc;
> +	netif_rx(skb);
> +	stats->rx_packets++;
> +	priv->ndev->last_rx = jiffies;
> +
> +	return 0;
> +}
> +
> +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);
> +	int num_pkts = 0;
> +	unsigned long pending_pkts;
> +	int mbxno;
> +
> +	if (!netif_running(ndev))
> +		return 0;
> +
> +	pending_pkts = hecc_read(priv, HECC_CANRMP);
> +	while (pending_pkts && (num_pkts < quota)) {
> +		mbxno = find_first_bit(&pending_pkts, HECC_MAX_MAILBOXES);
> +		if (mbxno == HECC_MAX_MAILBOXES) {
> +			dev_info(priv->ndev->dev.parent,
> +				"Reached max mailboxes. No rx pkts\n");
> +			return num_pkts;
> +		}
> +
> +		if (ti_hecc_rx_pkt(priv, mbxno) < 0)
> +			return num_pkts;
> +
> +		clear_bit(mbxno, &pending_pkts);
> +		hecc_set_bit(priv, HECC_CANRMP, (1 << mbxno));
> +		++num_pkts;
> +	}
> +
> +	/* Enable packet interrupt if all pkts are handled */
> +	if (0 == hecc_read(priv, HECC_CANRMP)) {
> +		napi_complete(napi);
> +		/* Re-enable RX mailbox interrupts */
> +		mbxno = hecc_read(priv, HECC_CANMIM);
> +		mbxno |= (~((1 << TI_HECC_MAX_TX_MBOX) - 1));
> +		hecc_write(priv, HECC_CANMIM, mbxno);
> +	}
> +
> +	return num_pkts;
> +}
> +
> +/**
> + * ti_hecc_error: TI HECC error routine
> + *
> + * Handles HECC error - handles error condition and send a packet up the stack
> + */
> +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;
> +	int data;
> +
> +	/* propogate the error condition to the can stack */
> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> +	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);
> +	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_RMLIF) { /* Message lost interrupt */
> +		data = hecc_read(priv, HECC_CANRML);
> +		hecc_write(priv, HECC_CANRML, data);
> +		++priv->message_lost_cnt;
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +		stats->rx_over_errors++;
> +		stats->rx_errors++;
> +		dev_info(priv->ndev->dev.parent, "Message lost interrupt\n");
> +	}
> +
> +	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_info(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_info(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) {
> +		priv->can.state = CAN_STATE_BUS_OFF;
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		hecc_set_bit(priv, HECC_CANES, HECC_CANES_BO);
> +		dev_info(priv->ndev->dev.parent, "Bus Off interrupt\n");
> +		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);
> +	}
> +
> +	if (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);
> +		dev_info(priv->ndev->dev.parent, "Bus Off condition\n");
> +		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);
> +	}
> +
> +	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;
> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +	return 0;
> +}
> +
> +
> +/**
> + * ti_hecc_interrupt: TI HECC interrupt routine
> + *
> + * Handles HECC interrupts - disables interrupt if receive pkts that will
> + * be enabled when rx pkts are compelte (napi_complete is done)
> + */
> +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;
> +	struct sk_buff *skb;
> +	struct can_frame *cf;
> +	unsigned int int_status;
> +	unsigned long ack;
> +	int mbxno;
> +	unsigned long flags;
> +
> +	if (priv->int_line)
> +		int_status = hecc_read(priv, HECC_CANGIF1);
> +	else
> +		int_status = hecc_read(priv, HECC_CANGIF0);
> +
> +	if (0 == int_status)
> +		return IRQ_NONE;
> +
> +	/* Handle message alarm interrupt */
> +	if (int_status & HECC_CANGIF_MAIF) {
> +		++priv->message_alarm_cnt;
> +		dev_info(priv->ndev->dev.parent, "Message alarm interrupt\n");
> +	}
> +
> +	/* Handle local network timer counter overflow interrupt */
> +	if (int_status & HECC_CANGIF_TCOIF) {
> +		++priv->timer_overflow_cnt;
> +		dev_info(priv->ndev->dev.parent,
> +			"Local network timer counter overflow interrupt\n");
> +	}
> +
> +	/* Handle write denied interrupt */
> +	if (int_status & HECC_CANGIF_WDIF) {
> +		++priv->write_denied_cnt;
> +		dev_info(priv->ndev->dev.parent, "Write denied interrupt\n");
> +	}
> +
> +	/* Handle wake up interrupt */
> +	if (int_status & HECC_CANGIF_WUIF) {
> +		++priv->wake_up_cnt;
> +		dev_info(priv->ndev->dev.parent, "Wake up interrupt\n");
> +	}
> +
> +	ti_hecc_error(ndev, int_status, hecc_read(priv, HECC_CANES));
> +
> +	/* Handle Abort acknowledge interrupt */
> +	if (int_status & HECC_CANGIF_AAIF) {
> +		ack = hecc_read(priv, HECC_CANAA);
> +		while (ack) {
> +			mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
> +			if (mbxno == HECC_MAX_MAILBOXES) {
> +				break;
> +			} else {
> +				clear_bit(mbxno, &ack);
> +				/* release echo pkt & update counters */
> +				hecc_set_bit(priv, HECC_CANAA, (1 << mbxno));
> +				hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
> +				/* FIXME: since net-next tree's dev.h does not
> +				 * include can_free_echo_skb() doing equivalent
> +				 * of can_free_echo_skb(ndev, mbxno);
> +				 */
> +				if (priv->can.echo_skb[mbxno]) {
> +					kfree_skb(priv->can.echo_skb[mbxno]);
> +					priv->can.echo_skb[mbxno] = NULL;
> +				}
> +				if (netif_queue_stopped(ndev))
> +					netif_wake_queue(ndev);
> +				spin_lock_irqsave(&priv->tx_lock, flags);
> +				clear_bit(mbxno, priv->tx_free_mbx);
> +				spin_unlock_irqrestore(&priv->tx_lock, flags);
> +			}
> +		}
> +	}
> +
> +	/* Handle mailbox interrupt */
> +	if (int_status & HECC_CANGIF_GMIF) {
> +		ack = hecc_read(priv, HECC_CANTA);
> +		while (ack) {
> +			mbxno = find_first_bit(&ack, HECC_MAX_MAILBOXES);
> +			if (mbxno == HECC_MAX_MAILBOXES) {
> +				break;
> +			} else {
> +				clear_bit(mbxno, &ack);
> +				hecc_clear_bit(priv, HECC_CANME, (1 << mbxno));
> +				hecc_set_bit(priv, HECC_CANTA, (1 << mbxno));
> +				skb = priv->can.echo_skb[mbxno];
> +				cf = (struct can_frame *) (skb->data);
> +				can_get_echo_skb(ndev, mbxno);
> +				stats->tx_bytes += cf->can_dlc;
> +				spin_lock_irqsave(&priv->tx_lock, flags);
> +				clear_bit(mbxno, priv->tx_free_mbx);
> +				spin_unlock_irqrestore(&priv->tx_lock, flags);
> +				stats->tx_packets++;
> +			}
> +		}
> +		if (netif_queue_stopped(ndev))
> +			netif_wake_queue(ndev);
> +
> +		/* Disable RX mailbox interrupts and let NAPI reenable them */
> +		ack = hecc_read(priv, HECC_CANMIM);
> +		ack &= ((1 << TI_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;
> +}
> +
> +/* NOTE: yet to test suspend/resume */
> +static int ti_hecc_suspend(struct platform_device *pdev, pm_message_t state)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +
> +	if (netif_running(ndev)) {
> +		netif_stop_queue(ndev);
> +		netif_device_detach(ndev);
> +	}
> +
> +	hecc_set_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> +	priv->can.state = CAN_STATE_SLEEPING;
> +	clk_disable(priv->clk);
> +
> +	return 0;
> +}
> +
> +/* NOTE: yet to test suspend/resume */
> +static int ti_hecc_resume(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +
> +	clk_enable(priv->clk);
> +	hecc_clear_bit(priv, HECC_CANMC, HECC_CANMC_PDR);
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +	if (netif_running(ndev)) {
> +		netif_device_attach(ndev);
> +		netif_start_queue(ndev);
> +	}
> +
> +	return 0;
> +}
> +
> +static int ti_hecc_open(struct net_device *ndev)
> +{
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +	int err;
> +
> +	dev_info(ndev->dev.parent, "opening device\n");
> +
> +	if (request_irq(ndev->irq, ti_hecc_interrupt, IRQF_DISABLED,
> +				ndev->name, ndev)) {
> +		dev_err(ndev->dev.parent, "error requesting interrupt\n");
> +		return -EAGAIN;
> +	}
> +
> +	/* Open common can device */
> +	err = open_candev(ndev);
> +	if (err) {
> +		dev_err(ndev->dev.parent, "open_candev() failed %08X\n", err);
> +		return err;
> +	}
> +
> +	/* Initialize device and start net queue */
> +	spin_lock_init(&priv->tx_lock);
> +
> +	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);
> +
> +	dev_info(ndev->dev.parent, "closing device\n");
> +	napi_disable(&priv->napi);
> +	netif_stop_queue(ndev);
> +	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;
> +
> +	printk(KERN_INFO DRV_NAME " probing devices...\n");
> +	pdata = pdev->dev.platform_data;
> +	if (!pdata) {
> +		printk(KERN_ERR "No platform data available - exiting\n");
> +		return -ENODEV;
> +	}
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	if (!mem) {
> +		printk(KERN_ERR "no mem resource?\n");
> +		err = -ENODEV;
> +		goto probe_exit;
> +	}
> +	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
> +	if (!irq) {
> +		printk(KERN_ERR "no irq resource?\n");
> +		err = -ENODEV;
> +		goto probe_exit;
> +	}
> +	if (!request_mem_region(mem->start, (mem->end - mem->start) + 1,

Use resource_size(mem).

> +		pdev->name)) {
> +		printk(KERN_ERR "HECC region already claimed\n");
> +		err = -EBUSY;
> +		goto probe_exit;
> +	}
> +	addr = ioremap(mem->start, mem->end - mem->start + 1);

ditto

> +	if (!addr) {
> +		printk(KERN_ERR "ioremap failed\n");
> +		err = -ENOMEM;
> +		goto probe_exit_free_region;
> +	}
> +
> +	ndev = alloc_candev(sizeof(struct ti_hecc_priv));
> +	if (!ndev) {
> +		printk(KERN_ERR "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_bittiming	= ti_hecc_set_bittiming;
> +	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(ndev->dev.parent, "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,
> +			TI_HECC_DEF_NAPI_WEIGHT);
> +
> +	err = register_candev(ndev);
> +	if (err) {
> +		dev_err(ndev->dev.parent, "register_candev() failed\n");
> +		err = -ENODEV;
> +		goto probe_exit_clk;
> +	}
> +	dev_info(ndev->dev.parent, "regs=%p, irq=%d\n",
> +		priv->base, (unsigned int) ndev->irq);
> +
> +#ifdef CONFIG_DEBUG_FS
> +	hecc_debug_init(priv);
> +#endif
> +	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, mem->end - mem->start + 1);

ditto

> +probe_exit:
> +	dev_err(ndev->dev.parent, "probe error = %08X\n", err);
> +	return err;
> +}
> +
> +static int __devexit ti_hecc_remove(struct platform_device *pdev)
> +{
> +	struct resource *res;
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct ti_hecc_priv *priv = netdev_priv(ndev);
> +
> +#ifdef CONFIG_DEBUG_FS
> +	hecc_debug_exit();
> +#endif /* CONFIG_DEBUG_FS */
> +
> +	clk_put(priv->clk);
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	iounmap(priv->base);
> +	release_mem_region(res->start, res->end - res->start + 1);

ditto

> +	unregister_candev(ndev);
> +	free_candev(ndev);
> +	platform_set_drvdata(pdev, NULL);
> +	dev_info(ndev->dev.parent, "driver removed\n");
> +
> +	return 0;
> +}
> +
> +/* TI HECC netdevice driver: platform driver structure */
> +static struct platform_driver ti_hecc_driver = {
> +	.driver = {
> +		.name    = "ti_hecc",
> +		.owner   = THIS_MODULE,
> +	},
> +	.probe = ti_hecc_probe,
> +	.remove = __devexit_p(ti_hecc_remove),
> +	.suspend = ti_hecc_suspend,
> +	.resume = ti_hecc_resume,
> +};
> +
> +static int __init ti_hecc_init_driver(void)
> +{
> +	printk(KERN_INFO DRV_DESC "\n");
> +	return platform_driver_register(&ti_hecc_driver);
> +}
> +module_init(ti_hecc_init_driver);
> +
> +static void __exit ti_hecc_exit_driver(void)
> +{
> +	printk(KERN_INFO DRV_DESC " :Exit\n");
> +	platform_driver_unregister(&ti_hecc_driver);
> +}
> +module_exit(ti_hecc_exit_driver);
> +
> +MODULE_AUTHOR("Anant Gole <anantgole@ti.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION(DRV_DESC);
> diff --git a/include/linux/can/platform/ti_hecc_platform.h b/include/linux/can/platform/ti_hecc_platform.h
> new file mode 100644
> index 0000000..4a57daf
> --- /dev/null
> +++ b/include/linux/can/platform/ti_hecc_platform.h
> @@ -0,0 +1,40 @@
> +/*
> + * TI HECC (High End CAN Controller) driver platform header
> + *
> + * 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.
> + *
> + */
> +
> +/**
> + * struct hecc_platform_data - HECC Platform Data
> + *
> + * @scc_hecc_offset:	mostly 0 - should really never change
> + * @scc_ram_offset:	SCC RAM offset
> + * @hecc_ram_offset:	HECC RAM offset
> + * @mbox_offset:	Mailbox RAM offset
> + * @int_line:		Interrupt line to use - 0 or 1
> + * @version:		version for future use
> + *
> + * Platform data structure to get all platform specific settings.
> + * this structure also accounts the fact that the IP may have different
> + * RAM and mailbox offsets for different SOC's
> + */
> +struct ti_hecc_platform_data {
> +	unsigned int scc_hecc_offset;
> +	unsigned int scc_ram_offset;
> +	unsigned int hecc_ram_offset;
> +	unsigned int mbox_offset;
> +	unsigned int int_line;
> +	unsigned int version;
> +};
> +
> +
> -- 
> 1.6.2.4
> 
> _______________________________________________
> Socketcan-core mailing list
> Socketcan-core@lists.berlios.de
> https://lists.berlios.de/mailman/listinfo/socketcan-core

Regards,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH] net-next:can: add TI CAN (HECC) driver
  2009-09-01  9:04 ` Wolfram Sang
@ 2009-09-01  9:32   ` Gole, Anant
  0 siblings, 0 replies; 13+ messages in thread
From: Gole, Anant @ 2009-09-01  9:32 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: socketcan-core@lists.berlios.de, netdev@vger.kernel.org


[snip]
>>
>> This driver is tested on OMAP3517 EVM. Suspend/Resume not tested as yet.
>
>One minor thing I found while glimpsing at the code...
>
[snip]
>> +	}
>> +	if (!request_mem_region(mem->start, (mem->end - mem->start) + 1,
>
>Use resource_size(mem).
>
>> +		pdev->name)) {
>> +		printk(KERN_ERR "HECC region already claimed\n");
>> +		err = -EBUSY;
>> +		goto probe_exit;
>> +	}
>> +	addr = ioremap(mem->start, mem->end - mem->start + 1);
>
>ditto

Ack. Thanks for the comment. I will include in V2 patch.

Regards,
Anant

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH] net-next:can: add TI CAN (HECC) driver
@ 2009-10-05 10:02 Anant Gole
       [not found] ` <1254736974-6685-1-git-send-email-anantgole-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 13+ messages in thread
From: Anant Gole @ 2009-10-05 10:02 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	linux-arm-kernel-xIg/pKzrS19vn6HldHNs0ANdhmdF6hFW

TI HECC (High End CAN Controller) module is found on many TI devices. It
has 32 hardware mailboxes with full implementation of CAN protocol 2.0B
with bus speeds up to 1Mbps. Specifications of the module are available
on TI web <http://www.ti.com>

Signed-off-by: Anant Gole <anantgole-l0cyMroinI0@public.gmane.org>
---
 drivers/net/can/Kconfig              |    7 +
 drivers/net/can/Makefile             |    1 +
 drivers/net/can/ti_hecc.c            | 1006 ++++++++++++++++++++++++++++++++++
 include/linux/can/platform/ti_hecc.h |   40 ++
 4 files changed, 1054 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 df32c10..57a8733 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -95,6 +95,13 @@ config CAN_AT91
 	---help---
 	  This is a driver for the SoC CAN controller in Atmel's AT91SAM9263.
 
+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 0dea627..31f4ab5 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -11,5 +11,6 @@ obj-y				+= usb/
 
 obj-$(CONFIG_CAN_SJA1000)	+= sja1000/
 obj-$(CONFIG_CAN_AT91)		+= at91_can.o
+obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
 
 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..9090103
--- /dev/null
+++ b/drivers/net/can/ti_hecc.c
@@ -0,0 +1,1006 @@
+/*
+ * 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 am3517_evm_hecc_pdata = {
+ *         .scc_hecc_offset        = 0,
+ *         .scc_ram_offset         = 0x3000,
+ *         .hecc_ram_offset        = 0x3000,
+ *         .mbx_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/types.h>
+#include <linux/interrupt.h>
+#include <linux/errno.h>
+#include <linux/netdevice.h>
+#include <linux/skbuff.h>
+#include <linux/platform_device.h>
+#include <linux/clk.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.6"
+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 mailboxes - do not change */
+#define MAX_TX_PRIO		0x3F	/* hardware value - do not change */
+
+/*
+ * 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_MAX_TX_MBOX	BIT(HECC_MB_TX_SHIFT)
+
+#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
+
+#define HECC_TX_PRIO_SHIFT	(HECC_MB_TX_SHIFT)
+#define HECC_TX_PRIO_MASK	(MAX_TX_PRIO << 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)
+#define HECC_TX_MBOX_MASK	(~(BIT(HECC_MAX_TX_MBOX) - 1))
+#define HECC_DEF_NAPI_WEIGHT	HECC_MAX_RX_MBOX
+
+/*
+ * Important Note: RX mailbox configuration
+ * 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_MAX_RX_MBOX	(HECC_MAX_MAILBOXES - HECC_MAX_TX_MBOX)
+#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	(~(BIT(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 */
+
+/* Mailbox registers */
+#define HECC_CANMID		0x0
+#define HECC_CANMCF		0x4
+#define HECC_CANMDL		0x8
+#define HECC_CANMDH		0xC
+
+#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 transmit 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 mbx_offset;
+	u32 int_line;
+	spinlock_t mbx_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) & MAX_TX_PRIO;
+}
+
+static inline void hecc_write_lam(struct ti_hecc_priv *priv, u32 mbxno, u32 val)
+{
+	__raw_writel(val, priv->base + priv->hecc_ram_offset + mbxno * 4);
+}
+
+static inline void hecc_write_mbx(struct ti_hecc_priv *priv, u32 mbxno,
+	u32 reg, u32 val)
+{
+	__raw_writel(val, priv->base + priv->mbx_offset + mbxno * 0x10 +
+			reg);
+}
+
+static inline u32 hecc_read_mbx(struct ti_hecc_priv *priv, u32 mbxno, u32 reg)
+{
+	return __raw_readl(priv->base + priv->mbx_offset + mbxno * 0x10 +
+			reg);
+}
+
+static inline void hecc_write(struct ti_hecc_priv *priv, u32 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,
+	u32 bit_mask)
+{
+	hecc_write(priv, reg, hecc_read(priv, reg) | bit_mask);
+}
+
+static inline void hecc_clear_bit(struct ti_hecc_priv *priv, int reg,
+	u32 bit_mask)
+{
+	hecc_write(priv, reg, hecc_read(priv, reg) & ~bit_mask);
+}
+
+static inline u32 hecc_get_bit(struct ti_hecc_priv *priv, int reg, u32 bit_mask)
+{
+	return (hecc_read(priv, reg) & bit_mask) ? 1 : 0;
+}
+
+static int ti_hecc_get_state(const struct net_device *ndev,
+	enum can_state *state)
+{
+	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;
+
+	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;
+	else
+		dev_info(priv->ndev->dev.parent,
+			"WARN: Triple sampling not set due to h/w limitations"
+			" at %d bitrate", bit_timing->bitrate);
+
+	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) && cnt != 0) {
+		--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) && cnt != 0) {
+		--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);
+	u32 cnt, mbxno, mbx_mask;
+
+	/* 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);
+
+	/* Prepare configured mailboxes to receive messages */
+	for (cnt = 0; cnt < HECC_MAX_RX_MBOX; cnt++) {
+		mbxno = HECC_MAX_MAILBOXES - 1 - cnt;
+		mbx_mask = BIT(mbxno);
+		hecc_clear_bit(priv, HECC_CANME, mbx_mask);
+		hecc_write_mbx(priv, mbxno, HECC_CANMID, HECC_CANMID_AME);
+		hecc_write_lam(priv, mbxno, HECC_SET_REG);
+		hecc_set_bit(priv, HECC_CANMD, mbx_mask);
+		hecc_set_bit(priv, HECC_CANME, mbx_mask);
+		hecc_set_bit(priv, HECC_CANMIM, mbx_mask);
+	}
+
+	/* 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, mbx_mask, data;
+	unsigned long flags;
+
+	mbxno = get_tx_head_mb(priv);
+	mbx_mask = BIT(mbxno);
+	spin_lock_irqsave(&priv->mbx_lock, flags);
+	if (unlikely(hecc_read(priv, HECC_CANME) & mbx_mask)) {
+		spin_unlock_irqrestore(&priv->mbx_lock, flags);
+		netif_stop_queue(ndev);
+		dev_err(priv->ndev->dev.parent,
+			"BUG: TX mbx not ready tx_head=%08X, tx_tail=%08X\n",
+			priv->tx_head, priv->tx_tail);
+		return NETDEV_TX_BUSY;
+	}
+	spin_unlock_irqrestore(&priv->mbx_lock, flags);
+
+	/* Prepare mailbox for transmission */
+	data = min_t(u8, cf->can_dlc, 8);
+	if (cf->can_id & CAN_RTR_FLAG) /* Remote transmission request */
+		data |= HECC_CANMCF_RTR;
+	data |= get_tx_head_prio(priv) << 8;
+	hecc_write_mbx(priv, mbxno, HECC_CANMCF, 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_mbx(priv, mbxno, HECC_CANMID, data);
+	hecc_write_mbx(priv, mbxno, HECC_CANMDL,
+		be32_to_cpu(*(u32 *)(cf->data)));
+	if (cf->can_dlc > 4) {
+		hecc_write_mbx(priv, mbxno, HECC_CANMDH,
+			be32_to_cpu(*(u32 *)(cf->data + 4)));
+	} else {
+		*(u32 *)(cf->data + 4) = 0;
+	}
+	can_put_echo_skb(skb, ndev, mbxno);
+
+	spin_lock_irqsave(&priv->mbx_lock, flags);
+	--priv->tx_head;
+	if ((hecc_read(priv, HECC_CANME) & BIT(get_tx_head_mb(priv))) ||
+		(priv->tx_head & HECC_TX_MASK) == HECC_TX_MASK) {
+		netif_stop_queue(ndev);
+	}
+	hecc_set_bit(priv, HECC_CANME, mbx_mask);
+	spin_unlock_irqrestore(&priv->mbx_lock, flags);
+
+	hecc_clear_bit(priv, HECC_CANMD, mbx_mask);
+	hecc_set_bit(priv, HECC_CANMIM, mbx_mask);
+	hecc_write(priv, HECC_CANTRS, mbx_mask);
+
+	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, mbx_mask;
+	unsigned long flags;
+
+	skb = netdev_alloc_skb(priv->ndev, sizeof(struct can_frame));
+	if (!skb) {
+		if (printk_ratelimit())
+			dev_err(priv->ndev->dev.parent,
+				"ti_hecc_rx_pkt: netdev_alloc_skb() failed\n");
+		return -ENOMEM;
+	}
+	skb->protocol = htons(ETH_P_CAN);
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	mbx_mask = BIT(mbxno);
+	cf = (struct can_frame *)skb_put(skb, sizeof(struct can_frame));
+	data = hecc_read_mbx(priv, mbxno, HECC_CANMID);
+	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_mbx(priv, mbxno, HECC_CANMCF);
+	if (data & HECC_CANMCF_RTR)
+		cf->can_id |= CAN_RTR_FLAG;
+	cf->can_dlc = data & 0xF;
+	data = hecc_read_mbx(priv, mbxno, HECC_CANMDL);
+	*(u32 *)(cf->data) = cpu_to_be32(data);
+	if (cf->can_dlc > 4) {
+		data = hecc_read_mbx(priv, mbxno, HECC_CANMDH);
+		*(u32 *)(cf->data + 4) = cpu_to_be32(data);
+	} else {
+		*(u32 *)(cf->data + 4) = 0;
+	}
+	spin_lock_irqsave(&priv->mbx_lock, flags);
+	hecc_clear_bit(priv, HECC_CANME, mbx_mask);
+	hecc_write(priv, HECC_CANRMP, mbx_mask);
+	/* 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, mbx_mask);
+	spin_unlock_irqrestore(&priv->mbx_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 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->mbx_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->mbx_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 (hecc_read(priv, HECC_CANRMP) == 0) {
+		napi_complete(napi);
+		/* Re-enable RX mailbox interrupts */
+		mbx_mask = hecc_read(priv, HECC_CANMIM);
+		mbx_mask |= HECC_TX_MBOX_MASK;
+		hecc_write(priv, HECC_CANMIM, mbx_mask);
+	}
+
+	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 = netdev_alloc_skb(ndev, sizeof(struct can_frame));
+	if (!skb) {
+		if (printk_ratelimit())
+			dev_err(priv->ndev->dev.parent,
+				"ti_hecc_error: netdev_alloc_skb() failed\n");
+		return -ENOMEM;
+	}
+	skb->protocol = 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;
+	cf->can_dlc = CAN_ERR_DLC;
+
+	if (int_status & HECC_CANGIF_WLIF) { /* warning level int */
+		if ((int_status & HECC_CANGIF_BOIF) == 0) {
+			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 ((int_status & HECC_CANGIF_BOIF) == 0) {
+			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);
+		/* Disable all interrupts in bus-off to avoid int hog */
+		hecc_write(priv, HECC_CANGIM, 0);
+		can_bus_off(ndev);
+	}
+
+	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);
+	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, mbx_mask, int_status, err_status;
+	unsigned long ack, flags;
+
+	int_status = hecc_read(priv,
+		(priv->int_line) ? HECC_CANGIF1 : HECC_CANGIF0);
+
+	if (!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);
+			mbx_mask = BIT(mbxno);
+			if (!(mbx_mask & hecc_read(priv, HECC_CANTA)))
+				break;
+			hecc_clear_bit(priv, HECC_CANMIM, mbx_mask);
+			hecc_write(priv, HECC_CANTA, mbx_mask);
+			spin_lock_irqsave(&priv->mbx_lock, flags);
+			hecc_clear_bit(priv, HECC_CANME, mbx_mask);
+			spin_unlock_irqrestore(&priv->mbx_lock, flags);
+			stats->tx_bytes += hecc_read_mbx(priv, mbxno,
+						HECC_CANMCF) & 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 &= BIT(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_SHARED,
+			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 %d\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 = -ENODEV;
+
+	pdata = pdev->dev.platform_data;
+	if (!pdata) {
+		dev_err(&pdev->dev, "No platform data\n");
+		goto probe_exit;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem) {
+		dev_err(&pdev->dev, "No mem resources\n");
+		goto probe_exit;
+	}
+	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!irq) {
+		dev_err(&pdev->dev, "No irq resource\n");
+		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->mbx_offset = pdata->mbx_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;
+
+	priv->clk = clk_get(&pdev->dev, "hecc_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");
+		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;
+}
+
+static int __devexit ti_hecc_remove(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct net_device *ndev = platform_get_drvdata(pdev);
+	struct ti_hecc_priv *priv = netdev_priv(ndev);
+
+	clk_put(priv->clk);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	iounmap(priv->base);
+	release_mem_region(res->start, resource_size(res));
+	unregister_candev(ndev);
+	free_candev(ndev);
+	platform_set_drvdata(pdev, NULL);
+
+	return 0;
+}
+
+/* TI HECC netdevice driver: platform driver structure */
+static struct platform_driver ti_hecc_driver = {
+	.driver = {
+		.name    = DRV_NAME,
+		.owner   = THIS_MODULE,
+	},
+	.probe = ti_hecc_probe,
+	.remove = __devexit_p(ti_hecc_remove),
+};
+
+static int __init ti_hecc_init_driver(void)
+{
+	printk(KERN_INFO DRV_DESC "\n");
+	return platform_driver_register(&ti_hecc_driver);
+}
+module_init(ti_hecc_init_driver);
+
+static void __exit ti_hecc_exit_driver(void)
+{
+	printk(KERN_INFO DRV_DESC " unloaded\n");
+	platform_driver_unregister(&ti_hecc_driver);
+}
+module_exit(ti_hecc_exit_driver);
+
+MODULE_AUTHOR("Anant Gole <anantgole-l0cyMroinI0@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION(DRV_DESC);
diff --git a/include/linux/can/platform/ti_hecc.h b/include/linux/can/platform/ti_hecc.h
new file mode 100644
index 0000000..4688c7b
--- /dev/null
+++ b/include/linux/can/platform/ti_hecc.h
@@ -0,0 +1,40 @@
+/*
+ * TI HECC (High End CAN Controller) driver platform header
+ *
+ * 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.
+ *
+ */
+
+/**
+ * struct hecc_platform_data - HECC Platform Data
+ *
+ * @scc_hecc_offset:	mostly 0 - should really never change
+ * @scc_ram_offset:	SCC RAM offset
+ * @hecc_ram_offset:	HECC RAM offset
+ * @mbx_offset:		Mailbox RAM offset
+ * @int_line:		Interrupt line to use - 0 or 1
+ * @version:		version for future use
+ *
+ * Platform data structure to get all platform specific settings.
+ * this structure also accounts the fact that the IP may have different
+ * RAM and mailbox offsets for different SOC's
+ */
+struct ti_hecc_platform_data {
+	u32 scc_hecc_offset;
+	u32 scc_ram_offset;
+	u32 hecc_ram_offset;
+	u32 mbx_offset;
+	u32 int_line;
+	u32 version;
+};
+
+
-- 
1.6.2.4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH] net-next:can: add TI CAN (HECC) driver
       [not found] ` <1254736974-6685-1-git-send-email-anantgole-l0cyMroinI0@public.gmane.org>
@ 2009-10-05 11:25   ` Wolfgang Grandegger
  0 siblings, 0 replies; 13+ messages in thread
From: Wolfgang Grandegger @ 2009-10-05 11:25 UTC (permalink / raw)
  To: Anant Gole
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-xIg/pKzrS19vn6HldHNs0ANdhmdF6hFW

Anant Gole wrote:
> TI HECC (High End CAN Controller) module is found on many TI devices. It
> has 32 hardware mailboxes with full implementation of CAN protocol 2.0B
> with bus speeds up to 1Mbps. Specifications of the module are available
> on TI web <http://www.ti.com>
> 
> Signed-off-by: Anant Gole <anantgole-l0cyMroinI0@public.gmane.org>

I already reviewed this driver on the Socketcan-core ML and it's almost
OK from the Socket-CAN point of view. Just one issue...

[snip]
> diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
> new file mode 100644
> index 0000000..9090103
> --- /dev/null
> +++ b/drivers/net/can/ti_hecc.c
[snip]
> +static int ti_hecc_set_btc(struct ti_hecc_priv *priv)
> +{
> +	struct can_bittiming *bit_timing = &priv->can.bittiming;
> +	u32 can_btc;
> +
> +	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;
> +	else
> +		dev_info(priv->ndev->dev.parent,
> +			"WARN: Triple sampling not set due to h/w limitations"
> +			" at %d bitrate", bit_timing->bitrate);

That's not correct from my point of view. The warning message should
only be printed when the user tries to set triple sampling. I think it
should be similar to:

	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) {
		if (bit_timing->brp > 4)
			can_btc |= HECC_CANBTC_SAM;
		else
			dev_warn(priv->ndev->dev.parent,
			"Triple sampling not set due to h/w "
			limitations");
	}

Apart from that, the patch looks OK.

Wolfgang.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2009-10-05 11:25 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-10-05 10:02 [PATCH] net-next:can: add TI CAN (HECC) driver Anant Gole
     [not found] ` <1254736974-6685-1-git-send-email-anantgole-l0cyMroinI0@public.gmane.org>
2009-10-05 11:25   ` Wolfgang Grandegger
     [not found] <2A3DCF3DA181AD40BDE86A3150B27B6B02F621106B@dbde02.ent.ti.com>
2009-08-29  7:06 ` Wolfgang Grandegger
2009-08-31  7:22   ` Gole, Anant
2009-08-31  8:59     ` Wolfgang Grandegger
2009-08-31 10:29       ` Oliver Hartkopp
2009-08-31 10:43         ` Jan Kiszka
  -- strict thread matches above, loose matches on Subject: below --
2009-08-28 11:18 Anant Gole
2009-08-28 12:44 ` Oliver Hartkopp
2009-08-28 13:13   ` Gole, Anant
2009-08-28 13:27     ` Oliver Hartkopp
2009-09-01  9:04 ` Wolfram Sang
2009-09-01  9:32   ` Gole, Anant

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).