[PATCH v2 0/4] can: c_can: Add support for Bosch D_CAN controller

[PATCH v2 0/4] can: c_can: Add support for Bosch D_CAN controller

[AnilKumar Ch]

This patch series adds support for Bosch D_CAN controller to the
existing C_CAN controller driver.

Bosch D_CAN controller is a full-CAN implementation compliant to CAN
protocol version 2.0 part A and B. Bosch D_CAN user manual can be
obtained from: http://www.semiconductors.bosch.de/media/
en/pdf/ipmodules_1/can/d_can_users_manual_111.pdf

D_CAN device is used on many SoCs like AM335x, DM8148 and DM813x EVMs
from TI, D_CAN details on AM335x can be accessed from:
http://www.ti.com/lit/ug/spruh73c/spruh73c.pdf

This patch series also fixes some issues in the current c_can driver.
These issues were found while integrating d_can driver to c_can driver.

These patches have been tested on AM335x EVM using some additional
patches adding runtime PM support and some code to initialize the
AM335x D_CAN RAM. I will submit these patches once these patches are
accepted.

Due to lack of hardware I am not able to test c_can functionality.
I appreciate if anyone can test C_CAN functionality with this patch
series.

Changes form v1:
        - Changed the Macro implementation to access registers via an
          array with virtual register index.
        - Dropped "can: c_can: fix "BUG! echo_skb is occupied!" during
          transmit" patch because it's accepted.
        - Reworked on second patch (acc. to v1) based on Marc's comments.
        - Found one more issue and added as separate patch.
        - Third patch (acc. to v1) is split into two patches, first
          patch only do c_can modifications to incorporate the current
          implementation and second one adds d_can support.

AnilKumar Ch (4):
  can: c_can: fix an interrupt thrash issue with c_can driver
  can: c_can: fix: enable CAN HW interrupts after napi_enable()
  can: c_can: Move overlay structure to array with offset as index
  can: c_can: Add support for Bosch D_CAN controller

 drivers/net/can/c_can/c_can.c          |  122 +++++++++++++++---------------
 drivers/net/can/c_can/c_can.h          |  127 ++++++++++++++++++++++----------
 drivers/net/can/c_can/c_can_platform.c |   54 ++++++++------
 include/linux/can/platform/c_can.h     |   42 +++++++++++
 4 files changed, 223 insertions(+), 122 deletions(-)
 create mode 100644 include/linux/can/platform/c_can.h

[PATCH v2 1/4] can: c_can: fix an interrupt thrash issue with c_can driver

[AnilKumar Ch]

This patch fixes an interrupt thrash issue with c_can driver.

In c_can_isr() function interrupts are disabled and enabled only in
c_can_poll() function. c_can_isr() & c_can_poll() both read the
irqstatus flag. However, irqstatus is always read as 0 in c_can_poll()
because all C_CAN interrupts are disabled in c_can_isr(). This causes
all interrupts to be re-enabled in c_can_poll() which in turn causes
another interrupt since the event is not really handled. This keeps
happening causing a flood of interrupts.

To fix this, read the irqstatus register in isr and use the same cached
value in the poll function.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
Changes from v1:
	- Incorporated Marc's comments on irqstatus usage.

 drivers/net/can/c_can/c_can.c |    7 +++----
 drivers/net/can/c_can/c_can.h |    1 +
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 9ac28df..fa01621 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -952,7 +952,7 @@ static int c_can_poll(struct napi_struct *napi, int quota)
 	struct net_device *dev = napi->dev;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+	irqstatus = priv->irqstatus;
 	if (!irqstatus)
 		goto end;
 
@@ -1030,12 +1030,11 @@ end:
 
 static irqreturn_t c_can_isr(int irq, void *dev_id)
 {
-	u16 irqstatus;
 	struct net_device *dev = (struct net_device *)dev_id;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
-	if (!irqstatus)
+	priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+	if (!priv->irqstatus)
 		return IRQ_NONE;
 
 	/* disable all interrupts and schedule the NAPI */
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 9b7fbef..5f32d34 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -76,6 +76,7 @@ struct c_can_priv {
 	unsigned int tx_next;
 	unsigned int tx_echo;
 	void *priv;		/* for board-specific data */
+	u16 irqstatus;
 };
 
 struct net_device *alloc_c_can_dev(void);
-- 
1.7.0.4

[PATCH v2 2/4] can: c_can: fix: enable CAN HW interrupts after napi_enable()

[AnilKumar Ch]

Fix the issue of C_CAN interrupts getting disabled forever even though
we do can configuration multiple times using can utils. According to
NAPI usage we disable all the hardware interrupts in ISR and re-enable
them in poll(). Current implementation calls napi_enable() after hardware
interrupts are enabled. If we get any interrupts between these two steps
then we do not process those interrupts because napi is not enabled. Mostly
these interrupts come because of STATUS is not 0x7 or ERROR interrupts. If
napi_enable() happens before hardware interrupts enabled then c_can_poll()
function will be called eventual re-enabling of interrupts will happen.

This patch moves the napi_enable() call before interrupts enabled.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index fa01621..8dc84d6 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -1064,10 +1064,11 @@ static int c_can_open(struct net_device *dev)
 		goto exit_irq_fail;
 	}
 
+	napi_enable(&priv->napi);
+
 	/* start the c_can controller */
 	c_can_start(dev);
 
-	napi_enable(&priv->napi);
 	netif_start_queue(dev);
 
 	return 0;
-- 
1.7.0.4

[PATCH v2 3/4] can: c_can: Move overlay structure to array with offset as index

[AnilKumar Ch]

c_can uses overlay structure for accessing c_can module registers.
With this kind of implementation it is difficult to add one more ip
which is similar to c_can in functionality but different register
offsets.

This patch changes the overlay structure implementation to an array
with register offset as index. This way we can overcome the above
limitation.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can.c          |  114 ++++++++++++++++----------------
 drivers/net/can/c_can/c_can.h          |   96 ++++++++++++++++-----------
 drivers/net/can/c_can/c_can_platform.c |   25 ++++---
 3 files changed, 129 insertions(+), 106 deletions(-)

diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index 8dc84d6..4d40dcf 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -209,10 +209,10 @@ static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
 			C_CAN_MSG_OBJ_TX_FIRST;
 }
 
-static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg)
+static u32 c_can_read_reg32(struct c_can_priv *priv, int reg)
 {
 	u32 val = priv->read_reg(priv, reg);
-	val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
+	val |= ((u32) priv->read_reg(priv, reg + 1)) << 16;
 	return val;
 }
 
@@ -220,14 +220,14 @@ static void c_can_enable_all_interrupts(struct c_can_priv *priv,
 						int enable)
 {
 	unsigned int cntrl_save = priv->read_reg(priv,
-						&priv->regs->control);
+						C_CAN_CTRL_REG);
 
 	if (enable)
 		cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
 	else
 		cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
 
-	priv->write_reg(priv, &priv->regs->control, cntrl_save);
+	priv->write_reg(priv, C_CAN_CTRL_REG, cntrl_save);
 }
 
 static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
@@ -235,7 +235,7 @@ static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
 	int count = MIN_TIMEOUT_VALUE;
 
 	while (count && priv->read_reg(priv,
-				&priv->regs->ifregs[iface].com_req) &
+				C_CAN_IF1_COMREQ_REG) &
 				IF_COMR_BUSY) {
 		count--;
 		udelay(1);
@@ -258,9 +258,9 @@ static inline void c_can_object_get(struct net_device *dev,
 	 * register and message RAM must be complete in 6 CAN-CLK
 	 * period.
 	 */
-	priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
+	priv->write_reg(priv, C_CAN_IF1_COMMSK_REG,
 			IFX_WRITE_LOW_16BIT(mask));
-	priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
+	priv->write_reg(priv, C_CAN_IF1_COMREQ_REG,
 			IFX_WRITE_LOW_16BIT(objno));
 
 	if (c_can_msg_obj_is_busy(priv, iface))
@@ -278,9 +278,9 @@ static inline void c_can_object_put(struct net_device *dev,
 	 * register and message RAM must be complete in 6 CAN-CLK
 	 * period.
 	 */
-	priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
+	priv->write_reg(priv, C_CAN_IF1_COMMSK_REG,
 			(IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
-	priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
+	priv->write_reg(priv, C_CAN_IF1_COMREQ_REG,
 			IFX_WRITE_LOW_16BIT(objno));
 
 	if (c_can_msg_obj_is_busy(priv, iface))
@@ -306,18 +306,18 @@ static void c_can_write_msg_object(struct net_device *dev,
 
 	flags |= IF_ARB_MSGVAL;
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
+	priv->write_reg(priv, C_CAN_IF1_ARB1_REG,
 				IFX_WRITE_LOW_16BIT(id));
-	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, flags |
+	priv->write_reg(priv, C_CAN_IF1_ARB2_REG, flags |
 				IFX_WRITE_HIGH_16BIT(id));
 
 	for (i = 0; i < frame->can_dlc; i += 2) {
-		priv->write_reg(priv, &priv->regs->ifregs[iface].data[i / 2],
+		priv->write_reg(priv, C_CAN_IF1_DATA1_REG + i / 2,
 				frame->data[i] | (frame->data[i + 1] << 8));
 	}
 
 	/* enable interrupt for this message object */
-	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
+	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
 			IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
 			frame->can_dlc);
 	c_can_object_put(dev, iface, objno, IF_COMM_ALL);
@@ -329,7 +329,7 @@ static inline void c_can_mark_rx_msg_obj(struct net_device *dev,
 {
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
+	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
 			ctrl_mask & ~(IF_MCONT_MSGLST | IF_MCONT_INTPND));
 	c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
 
@@ -343,7 +343,7 @@ static inline void c_can_activate_all_lower_rx_msg_obj(struct net_device *dev,
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_MSG_RX_LOW_LAST; i++) {
-		priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
+		priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
 				ctrl_mask & ~(IF_MCONT_MSGLST |
 					IF_MCONT_INTPND | IF_MCONT_NEWDAT));
 		c_can_object_put(dev, iface, i, IF_COMM_CONTROL);
@@ -356,7 +356,7 @@ static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
 {
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
+	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
 			ctrl_mask & ~(IF_MCONT_MSGLST |
 				IF_MCONT_INTPND | IF_MCONT_NEWDAT));
 	c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
@@ -374,7 +374,7 @@ static void c_can_handle_lost_msg_obj(struct net_device *dev,
 
 	c_can_object_get(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
+	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
 			IF_MCONT_CLR_MSGLST);
 
 	c_can_object_put(dev, 0, objno, IF_COMM_CONTROL);
@@ -410,8 +410,8 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
 
 	frame->can_dlc = get_can_dlc(ctrl & 0x0F);
 
-	flags =	priv->read_reg(priv, &priv->regs->ifregs[iface].arb2);
-	val = priv->read_reg(priv, &priv->regs->ifregs[iface].arb1) |
+	flags =	priv->read_reg(priv, C_CAN_IF1_ARB2_REG);
+	val = priv->read_reg(priv, C_CAN_IF1_ARB1_REG) |
 		(flags << 16);
 
 	if (flags & IF_ARB_MSGXTD)
@@ -424,7 +424,7 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
 	else {
 		for (i = 0; i < frame->can_dlc; i += 2) {
 			data = priv->read_reg(priv,
-				&priv->regs->ifregs[iface].data[i / 2]);
+				C_CAN_IF1_DATA1_REG + i / 2);
 			frame->data[i] = data;
 			frame->data[i + 1] = data >> 8;
 		}
@@ -444,40 +444,40 @@ static void c_can_setup_receive_object(struct net_device *dev, int iface,
 {
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].mask1,
+	priv->write_reg(priv, C_CAN_IF1_MASK1_REG,
 			IFX_WRITE_LOW_16BIT(mask));
-	priv->write_reg(priv, &priv->regs->ifregs[iface].mask2,
+	priv->write_reg(priv, C_CAN_IF1_MASK2_REG,
 			IFX_WRITE_HIGH_16BIT(mask));
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
+	priv->write_reg(priv, C_CAN_IF1_ARB1_REG,
 			IFX_WRITE_LOW_16BIT(id));
-	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2,
+	priv->write_reg(priv, C_CAN_IF1_ARB2_REG,
 			(IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, mcont);
+	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG, mcont);
 	c_can_object_put(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
 
 	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
-			c_can_read_reg32(priv, &priv->regs->msgval1));
+			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
 }
 
 static void c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
 {
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1, 0);
-	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, 0);
-	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, 0);
+	priv->write_reg(priv, C_CAN_IF1_ARB1_REG, 0);
+	priv->write_reg(priv, C_CAN_IF1_ARB2_REG, 0);
+	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG, 0);
 
 	c_can_object_put(dev, iface, objno, IF_COMM_ARB | IF_COMM_CONTROL);
 
 	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
-			c_can_read_reg32(priv, &priv->regs->msgval1));
+			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
 }
 
 static inline int c_can_is_next_tx_obj_busy(struct c_can_priv *priv, int objno)
 {
-	int val = c_can_read_reg32(priv, &priv->regs->txrqst1);
+	int val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
 
 	/*
 	 * as transmission request register's bit n-1 corresponds to
@@ -540,12 +540,12 @@ static int c_can_set_bittiming(struct net_device *dev)
 	netdev_info(dev,
 		"setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
 
-	ctrl_save = priv->read_reg(priv, &priv->regs->control);
-	priv->write_reg(priv, &priv->regs->control,
+	ctrl_save = priv->read_reg(priv, C_CAN_CTRL_REG);
+	priv->write_reg(priv, C_CAN_CTRL_REG,
 			ctrl_save | CONTROL_CCE | CONTROL_INIT);
-	priv->write_reg(priv, &priv->regs->btr, reg_btr);
-	priv->write_reg(priv, &priv->regs->brp_ext, reg_brpe);
-	priv->write_reg(priv, &priv->regs->control, ctrl_save);
+	priv->write_reg(priv, C_CAN_BTR_REG, reg_btr);
+	priv->write_reg(priv, C_CAN_BRPEXT_REG, reg_brpe);
+	priv->write_reg(priv, C_CAN_CTRL_REG, ctrl_save);
 
 	return 0;
 }
@@ -587,36 +587,36 @@ static void c_can_chip_config(struct net_device *dev)
 	struct c_can_priv *priv = netdev_priv(dev);
 
 	/* enable automatic retransmission */
-	priv->write_reg(priv, &priv->regs->control,
+	priv->write_reg(priv, C_CAN_CTRL_REG,
 			CONTROL_ENABLE_AR);
 
 	if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
 					CAN_CTRLMODE_LOOPBACK)) {
 		/* loopback + silent mode : useful for hot self-test */
-		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
 				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
-		priv->write_reg(priv, &priv->regs->test,
+		priv->write_reg(priv, C_CAN_TEST_REG,
 				TEST_LBACK | TEST_SILENT);
 	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
 		/* loopback mode : useful for self-test function */
-		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
 				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
-		priv->write_reg(priv, &priv->regs->test, TEST_LBACK);
+		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
 	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
 		/* silent mode : bus-monitoring mode */
-		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
+		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
 				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
-		priv->write_reg(priv, &priv->regs->test, TEST_SILENT);
+		priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
 	} else
 		/* normal mode*/
-		priv->write_reg(priv, &priv->regs->control,
+		priv->write_reg(priv, C_CAN_CTRL_REG,
 				CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
 
 	/* configure message objects */
 	c_can_configure_msg_objects(dev);
 
 	/* set a `lec` value so that we can check for updates later */
-	priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
+	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
 
 	/* set bittiming params */
 	c_can_set_bittiming(dev);
@@ -669,7 +669,7 @@ static int c_can_get_berr_counter(const struct net_device *dev,
 	unsigned int reg_err_counter;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
+	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
 	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
 				ERR_CNT_REC_SHIFT;
 	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
@@ -697,12 +697,12 @@ static void c_can_do_tx(struct net_device *dev)
 
 	for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
 		msg_obj_no = get_tx_echo_msg_obj(priv);
-		val = c_can_read_reg32(priv, &priv->regs->txrqst1);
+		val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
 		if (!(val & (1 << (msg_obj_no - 1)))) {
 			can_get_echo_skb(dev,
 					msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
 			stats->tx_bytes += priv->read_reg(priv,
-					&priv->regs->ifregs[0].msg_cntrl)
+					C_CAN_IF1_MSGCTRL_REG)
 					& IF_MCONT_DLC_MASK;
 			stats->tx_packets++;
 			c_can_inval_msg_object(dev, 0, msg_obj_no);
@@ -744,11 +744,11 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
 	u32 num_rx_pkts = 0;
 	unsigned int msg_obj, msg_ctrl_save;
 	struct c_can_priv *priv = netdev_priv(dev);
-	u32 val = c_can_read_reg32(priv, &priv->regs->intpnd1);
+	u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
 
 	for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
 			msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
-			val = c_can_read_reg32(priv, &priv->regs->intpnd1),
+			val = c_can_read_reg32(priv, C_CAN_INTPND1_REG),
 			msg_obj++) {
 		/*
 		 * as interrupt pending register's bit n-1 corresponds to
@@ -758,7 +758,7 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
 			c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
 					~IF_COMM_TXRQST);
 			msg_ctrl_save = priv->read_reg(priv,
-					&priv->regs->ifregs[0].msg_cntrl);
+					C_CAN_IF1_MSGCTRL_REG);
 
 			if (msg_ctrl_save & IF_MCONT_EOB)
 				return num_rx_pkts;
@@ -819,7 +819,7 @@ static int c_can_handle_state_change(struct net_device *dev,
 		return 0;
 
 	c_can_get_berr_counter(dev, &bec);
-	reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
+	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
 	rx_err_passive = (reg_err_counter & ERR_CNT_RP_MASK) >>
 				ERR_CNT_RP_SHIFT;
 
@@ -935,7 +935,7 @@ static int c_can_handle_bus_err(struct net_device *dev,
 	}
 
 	/* set a `lec` value so that we can check for updates later */
-	priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
+	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
 
 	netif_receive_skb(skb);
 	stats->rx_packets++;
@@ -959,15 +959,15 @@ static int c_can_poll(struct napi_struct *napi, int quota)
 	/* status events have the highest priority */
 	if (irqstatus == STATUS_INTERRUPT) {
 		priv->current_status = priv->read_reg(priv,
-					&priv->regs->status);
+					C_CAN_STS_REG);
 
 		/* handle Tx/Rx events */
 		if (priv->current_status & STATUS_TXOK)
-			priv->write_reg(priv, &priv->regs->status,
+			priv->write_reg(priv, C_CAN_STS_REG,
 					priv->current_status & ~STATUS_TXOK);
 
 		if (priv->current_status & STATUS_RXOK)
-			priv->write_reg(priv, &priv->regs->status,
+			priv->write_reg(priv, C_CAN_STS_REG,
 					priv->current_status & ~STATUS_RXOK);
 
 		/* handle state changes */
@@ -1033,7 +1033,7 @@ static irqreturn_t c_can_isr(int irq, void *dev_id)
 	struct net_device *dev = (struct net_device *)dev_id;
 	struct c_can_priv *priv = netdev_priv(dev);
 
-	priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
+	priv->irqstatus = priv->read_reg(priv, C_CAN_INT_REG);
 	if (!priv->irqstatus)
 		return IRQ_NONE;
 
diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 5f32d34..747d478 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -22,43 +22,62 @@
 #ifndef C_CAN_H
 #define C_CAN_H
 
-/* c_can IF registers */
-struct c_can_if_regs {
-	u16 com_req;
-	u16 com_mask;
-	u16 mask1;
-	u16 mask2;
-	u16 arb1;
-	u16 arb2;
-	u16 msg_cntrl;
-	u16 data[4];
-	u16 _reserved[13];
+enum {
+	C_CAN_CTRL_REG = 0,
+	C_CAN_STS_REG,
+	C_CAN_ERR_CNT_REG,
+	C_CAN_BTR_REG,
+	C_CAN_INT_REG,
+	C_CAN_TEST_REG,
+	C_CAN_BRPEXT_REG,
+	C_CAN_IF1_COMREQ_REG,
+	C_CAN_IF1_COMMSK_REG,
+	C_CAN_IF1_MASK1_REG,
+	C_CAN_IF1_MASK2_REG,
+	C_CAN_IF1_ARB1_REG,
+	C_CAN_IF1_ARB2_REG,
+	C_CAN_IF1_MSGCTRL_REG,
+	C_CAN_IF1_DATA1_REG,
+	C_CAN_IF1_DATA2_REG,
+	C_CAN_IF1_DATA3_REG,
+	C_CAN_IF1_DATA4_REG,
+	C_CAN_TXRQST1_REG,
+	C_CAN_TXRQST2_REG,
+	C_CAN_NEWDAT1_REG,
+	C_CAN_NEWDAT2_REG,
+	C_CAN_INTPND1_REG,
+	C_CAN_INTPND2_REG,
+	C_CAN_MSGVAL1_REG,
+	C_CAN_MSGVAL2_REG,
 };
 
-/* c_can hardware registers */
-struct c_can_regs {
-	u16 control;
-	u16 status;
-	u16 err_cnt;
-	u16 btr;
-	u16 interrupt;
-	u16 test;
-	u16 brp_ext;
-	u16 _reserved1;
-	struct c_can_if_regs ifregs[2]; /* [0] = IF1 and [1] = IF2 */
-	u16 _reserved2[8];
-	u16 txrqst1;
-	u16 txrqst2;
-	u16 _reserved3[6];
-	u16 newdat1;
-	u16 newdat2;
-	u16 _reserved4[6];
-	u16 intpnd1;
-	u16 intpnd2;
-	u16 _reserved5[6];
-	u16 msgval1;
-	u16 msgval2;
-	u16 _reserved6[6];
+static u16 reg_map_c_can[] = {
+	[C_CAN_CTRL_REG]	= 0x00,
+	[C_CAN_STS_REG]		= 0x02,
+	[C_CAN_ERR_CNT_REG]	= 0x04,
+	[C_CAN_BTR_REG]		= 0x06,
+	[C_CAN_INT_REG]		= 0x08,
+	[C_CAN_TEST_REG]	= 0x0A,
+	[C_CAN_BRPEXT_REG]	= 0x0C,
+	[C_CAN_IF1_COMREQ_REG]	= 0x10,
+	[C_CAN_IF1_COMMSK_REG]	= 0x12,
+	[C_CAN_IF1_MASK1_REG]	= 0x14,
+	[C_CAN_IF1_MASK2_REG]	= 0x16,
+	[C_CAN_IF1_ARB1_REG]	= 0x18,
+	[C_CAN_IF1_ARB2_REG]	= 0x1A,
+	[C_CAN_IF1_MSGCTRL_REG]	= 0x1C,
+	[C_CAN_IF1_DATA1_REG]	= 0x1E,
+	[C_CAN_IF1_DATA2_REG]	= 0x20,
+	[C_CAN_IF1_DATA3_REG]	= 0x22,
+	[C_CAN_IF1_DATA4_REG]	= 0x24,
+	[C_CAN_TXRQST1_REG]	= 0x80,
+	[C_CAN_TXRQST2_REG]	= 0x82,
+	[C_CAN_NEWDAT1_REG]	= 0x90,
+	[C_CAN_NEWDAT2_REG]	= 0x92,
+	[C_CAN_INTPND1_REG]	= 0xA0,
+	[C_CAN_INTPND2_REG]	= 0xA2,
+	[C_CAN_MSGVAL1_REG]	= 0xB0,
+	[C_CAN_MSGVAL2_REG]	= 0xB2,
 };
 
 /* c_can private data structure */
@@ -69,9 +88,10 @@ struct c_can_priv {
 	int tx_object;
 	int current_status;
 	int last_status;
-	u16 (*read_reg) (struct c_can_priv *priv, void *reg);
-	void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
-	struct c_can_regs __iomem *regs;
+	u16 (*read_reg) (struct c_can_priv *priv, int reg);
+	void (*write_reg) (struct c_can_priv *priv, int reg, u16 val);
+	void __iomem *base;
+	u16 *regs;
 	unsigned long irq_flags; /* for request_irq() */
 	unsigned int tx_next;
 	unsigned int tx_echo;
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 5e1a5ff..0cca9db 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -32,6 +32,7 @@
 #include <linux/clk.h>
 
 #include <linux/can/dev.h>
+#include <linux/can/platform/c_can.h>
 
 #include "c_can.h"
 
@@ -42,27 +43,27 @@
  * Handle the same by providing a common read/write interface.
  */
 static u16 c_can_plat_read_reg_aligned_to_16bit(struct c_can_priv *priv,
-						void *reg)
+						int reg)
 {
-	return readw(reg);
+	return readw(priv->base + priv->regs[reg]);
 }
 
 static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv *priv,
-						void *reg, u16 val)
+						int reg, u16 val)
 {
-	writew(val, reg);
+	writew(val, priv->base + priv->regs[reg]);
 }
 
 static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv *priv,
-						void *reg)
+						int reg)
 {
-	return readw(reg + (long)reg - (long)priv->regs);
+	return readw(priv->base + 2 * priv->regs[reg]);
 }
 
 static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv *priv,
-						void *reg, u16 val)
+						int reg, u16 val)
 {
-	writew(val, reg + (long)reg - (long)priv->regs);
+	writew(val, priv->base + 2 * priv->regs[reg]);
 }
 
 static int __devinit c_can_plat_probe(struct platform_device *pdev)
@@ -71,6 +72,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	void __iomem *addr;
 	struct net_device *dev;
 	struct c_can_priv *priv;
+	struct c_can_platform_data *pdata = pdev->dev.platform_data;
 	struct resource *mem;
 	int irq;
 #ifdef CONFIG_HAVE_CLK
@@ -115,9 +117,10 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	}
 
 	priv = netdev_priv(dev);
+	priv->regs = reg_map_c_can;
 
 	dev->irq = irq;
-	priv->regs = addr;
+	priv->base = addr;
 #ifdef CONFIG_HAVE_CLK
 	priv->can.clock.freq = clk_get_rate(clk);
 	priv->priv = clk;
@@ -146,7 +149,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	}
 
 	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
-		 KBUILD_MODNAME, priv->regs, dev->irq);
+		 KBUILD_MODNAME, priv->base, dev->irq);
 	return 0;
 
 exit_free_device:
@@ -176,7 +179,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
 	platform_set_drvdata(pdev, NULL);
 
 	free_c_can_dev(dev);
-	iounmap(priv->regs);
+	iounmap(priv->base);
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	release_mem_region(mem->start, resource_size(mem));
-- 
1.7.0.4

[PATCH v2 4/4] can: c_can: Add support for Bosch D_CAN controller

[AnilKumar Ch]

This patch adds the support for D_CAN controller driver to the existing
C_CAN driver.

Bosch D_CAN controller is a full-CAN implementation which is compliant
to CAN protocol version 2.0 part A and B. Bosch D_CAN user manual can be
obtained from: http://www.semiconductors.bosch.de/media/en/pdf/
ipmodules_1/can/d_can_users_manual_111.pdf

A new array is added for accessing the d_can registers, according to d_can
controller register space.

Current D_CAN implementation has following limitations, this is done
to avoid large changes to the C_CAN driver.
1. Message objects are limited to 32, 16 for RX and 16 for TX. C_CAN IP
   supports upto 32 message objects but in case of D_CAN we can configure
   upto 128 message objects.
2. Using two 16bit reads/writes for accessing the 32bit D_CAN registers.
3. These patches have been tested on little endian machine, there might
   be some hidden endian-related issues due to the nature of the accesses
   (32-bit registers accessed as 2 16-bit registers). However, I do not
   have a big-endian D_CAN implementation to confirm.

Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
---
 drivers/net/can/c_can/c_can.h          |   30 ++++++++++++++++++++++
 drivers/net/can/c_can/c_can_platform.c |   31 +++++++++++++----------
 include/linux/can/platform/c_can.h     |   42 ++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+), 13 deletions(-)
 create mode 100644 include/linux/can/platform/c_can.h

diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
index 747d478..63a8727 100644
--- a/drivers/net/can/c_can/c_can.h
+++ b/drivers/net/can/c_can/c_can.h
@@ -80,6 +80,35 @@ static u16 reg_map_c_can[] = {
 	[C_CAN_MSGVAL2_REG]	= 0xB2,
 };
 
+static u16 reg_map_d_can[] = {
+	[C_CAN_CTRL_REG]	= 0x00,
+	[C_CAN_STS_REG]		= 0x04,
+	[C_CAN_ERR_CNT_REG]	= 0x08,
+	[C_CAN_BTR_REG]		= 0x0C,
+	[C_CAN_BRPEXT_REG]	= 0x0E,
+	[C_CAN_INT_REG]		= 0x10,
+	[C_CAN_TEST_REG]	= 0x14,
+	[C_CAN_TXRQST1_REG]	= 0x88,
+	[C_CAN_TXRQST2_REG]	= 0x8A,
+	[C_CAN_NEWDAT1_REG]	= 0x9C,
+	[C_CAN_NEWDAT2_REG]	= 0x9E,
+	[C_CAN_INTPND1_REG]	= 0xB0,
+	[C_CAN_INTPND2_REG]	= 0xB2,
+	[C_CAN_MSGVAL1_REG]	= 0xC4,
+	[C_CAN_MSGVAL2_REG]	= 0xC6,
+	[C_CAN_IF1_COMREQ_REG]	= 0x100,
+	[C_CAN_IF1_COMMSK_REG]	= 0x102,
+	[C_CAN_IF1_MASK1_REG]	= 0x104,
+	[C_CAN_IF1_MASK2_REG]	= 0x106,
+	[C_CAN_IF1_ARB1_REG]	= 0x108,
+	[C_CAN_IF1_ARB2_REG]	= 0x10A,
+	[C_CAN_IF1_MSGCTRL_REG]	= 0x10C,
+	[C_CAN_IF1_DATA1_REG]	= 0x110,
+	[C_CAN_IF1_DATA2_REG]	= 0x112,
+	[C_CAN_IF1_DATA3_REG]	= 0x114,
+	[C_CAN_IF1_DATA4_REG]	= 0x116,
+};
+
 /* c_can private data structure */
 struct c_can_priv {
 	struct can_priv can;	/* must be the first member */
@@ -97,6 +126,7 @@ struct c_can_priv {
 	unsigned int tx_echo;
 	void *priv;		/* for board-specific data */
 	u16 irqstatus;
+	unsigned int dev_type;
 };
 
 struct net_device *alloc_c_can_dev(void);
diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
index 0cca9db..394b05d 100644
--- a/drivers/net/can/c_can/c_can_platform.c
+++ b/drivers/net/can/c_can/c_can_platform.c
@@ -117,7 +117,24 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	}
 
 	priv = netdev_priv(dev);
-	priv->regs = reg_map_c_can;
+	if (pdata && (pdata->dev_type == DEV_TYPE_D_CAN)) {
+		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
+		priv->dev_type = DEV_TYPE_D_CAN;
+		priv->regs = reg_map_d_can;
+		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
+		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
+	} else {
+		priv->dev_type = DEV_TYPE_C_CAN;
+		priv->regs = reg_map_c_can;
+		if ((mem->flags & IORESOURCE_MEM_TYPE_MASK) ==
+					IORESOURCE_MEM_32BIT) {
+			priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
+			priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
+		} else {
+			priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
+			priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
+		}
+	}
 
 	dev->irq = irq;
 	priv->base = addr;
@@ -126,18 +143,6 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
 	priv->priv = clk;
 #endif
 
-	switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
-	case IORESOURCE_MEM_32BIT:
-		priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
-		priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
-		break;
-	case IORESOURCE_MEM_16BIT:
-	default:
-		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
-		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
-		break;
-	}
-
 	platform_set_drvdata(pdev, dev);
 	SET_NETDEV_DEV(dev, &pdev->dev);
 
diff --git a/include/linux/can/platform/c_can.h b/include/linux/can/platform/c_can.h
new file mode 100644
index 0000000..e95201a
--- /dev/null
+++ b/include/linux/can/platform/c_can.h
@@ -0,0 +1,42 @@
+/*
+ * C_CAN controller driver platform header
+ *
+ * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * Bosch C_CAN/D_CAN controller is compliant to CAN protocol version 2.0
+ * part A and B.
+ * Bosch C_CAN user manual can be obtained from:
+ * http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
+ * users_manual_c_can.pdf
+ * Bosch D_CAN user manual can be obtained from:
+ * http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/can/
+ * d_can_users_manual_111.pdf
+ *
+ * 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.
+ */
+
+#ifndef __CAN_PLATFORM_TI_C_CAN_H__
+#define __CAN_PLATFORM_TI_C_CAN_H__
+
+#define DEV_TYPE_C_CAN		0
+#define DEV_TYPE_D_CAN		1
+
+/**
+ * struct c_can_platform_data - CCAN Platform Data
+ *
+ * @dev_type:		Device type C_CAN/D_CAN
+ *
+ * Platform data structure to get all platform specific settings.
+ */
+
+struct c_can_platform_data {
+	unsigned int dev_type;
+};
+#endif
-- 
1.7.0.4

Re: [PATCH v2 1/4] can: c_can: fix an interrupt thrash issue with c_can driver

[Marc Kleine-Budde]

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

05/10/2012 01:34 PM, AnilKumar Ch wrote:
> This patch fixes an interrupt thrash issue with c_can driver.
> 
> In c_can_isr() function interrupts are disabled and enabled only in
> c_can_poll() function. c_can_isr() & c_can_poll() both read the
> irqstatus flag. However, irqstatus is always read as 0 in c_can_poll()
> because all C_CAN interrupts are disabled in c_can_isr(). This causes
> all interrupts to be re-enabled in c_can_poll() which in turn causes
> another interrupt since the event is not really handled. This keeps
> happening causing a flood of interrupts.
> 
> To fix this, read the irqstatus register in isr and use the same cached
> value in the poll function.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>

Can you or someone else please test this patch on c_can hardware, and
add her/his Tested-by? This patch is a stable candidate, isn't it?

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

cheers, Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH v2 2/4] can: c_can: fix: enable CAN HW interrupts after napi_enable()

[Marc Kleine-Budde]

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

On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> Fix the issue of C_CAN interrupts getting disabled forever even though
> we do can configuration multiple times using can utils. According to
> NAPI usage we disable all the hardware interrupts in ISR and re-enable
> them in poll(). Current implementation calls napi_enable() after hardware
> interrupts are enabled. If we get any interrupts between these two steps
> then we do not process those interrupts because napi is not enabled. Mostly
> these interrupts come because of STATUS is not 0x7 or ERROR interrupts. If
> napi_enable() happens before hardware interrupts enabled then c_can_poll()
> function will be called eventual re-enabling of interrupts will happen.
> 
> This patch moves the napi_enable() call before interrupts enabled.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>

Please test on c_can, also this is a stable candidate. What about add
the term "race condition" to the subject?

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH v2 4/4] can: c_can: Add support for Bosch D_CAN controller

[Marc Kleine-Budde]

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

On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> This patch adds the support for D_CAN controller driver to the existing
> C_CAN driver.
> 
> Bosch D_CAN controller is a full-CAN implementation which is compliant
> to CAN protocol version 2.0 part A and B. Bosch D_CAN user manual can be
> obtained from: http://www.semiconductors.bosch.de/media/en/pdf/
> ipmodules_1/can/d_can_users_manual_111.pdf
> 
> A new array is added for accessing the d_can registers, according to d_can
> controller register space.
> 
> Current D_CAN implementation has following limitations, this is done
> to avoid large changes to the C_CAN driver.
> 1. Message objects are limited to 32, 16 for RX and 16 for TX. C_CAN IP
>    supports upto 32 message objects but in case of D_CAN we can configure
>    upto 128 message objects.
> 2. Using two 16bit reads/writes for accessing the 32bit D_CAN registers.
> 3. These patches have been tested on little endian machine, there might
>    be some hidden endian-related issues due to the nature of the accesses
>    (32-bit registers accessed as 2 16-bit registers). However, I do not
>    have a big-endian D_CAN implementation to confirm.
> 
> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>

Have a look at the at91_can.c and make use of a platform_device id_table:

http://lxr.linux.no/linux+v3.3.5/drivers/net/can/at91_can.c#L1364

This way you can register a "c_can" device or a "d_can" device in your
arm board file.

More comments inline.

Cheers, Marc

> ---
>  drivers/net/can/c_can/c_can.h          |   30 ++++++++++++++++++++++
>  drivers/net/can/c_can/c_can_platform.c |   31 +++++++++++++----------
>  include/linux/can/platform/c_can.h     |   42 ++++++++++++++++++++++++++++++++
>  3 files changed, 90 insertions(+), 13 deletions(-)
>  create mode 100644 include/linux/can/platform/c_can.h
> 
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 747d478..63a8727 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -80,6 +80,35 @@ static u16 reg_map_c_can[] = {
>  	[C_CAN_MSGVAL2_REG]	= 0xB2,
>  };
>  
> +static u16 reg_map_d_can[] = {
> +	[C_CAN_CTRL_REG]	= 0x00,
> +	[C_CAN_STS_REG]		= 0x04,
> +	[C_CAN_ERR_CNT_REG]	= 0x08,
> +	[C_CAN_BTR_REG]		= 0x0C,
> +	[C_CAN_BRPEXT_REG]	= 0x0E,
> +	[C_CAN_INT_REG]		= 0x10,
> +	[C_CAN_TEST_REG]	= 0x14,
> +	[C_CAN_TXRQST1_REG]	= 0x88,
> +	[C_CAN_TXRQST2_REG]	= 0x8A,
> +	[C_CAN_NEWDAT1_REG]	= 0x9C,
> +	[C_CAN_NEWDAT2_REG]	= 0x9E,
> +	[C_CAN_INTPND1_REG]	= 0xB0,
> +	[C_CAN_INTPND2_REG]	= 0xB2,
> +	[C_CAN_MSGVAL1_REG]	= 0xC4,
> +	[C_CAN_MSGVAL2_REG]	= 0xC6,
> +	[C_CAN_IF1_COMREQ_REG]	= 0x100,
> +	[C_CAN_IF1_COMMSK_REG]	= 0x102,
> +	[C_CAN_IF1_MASK1_REG]	= 0x104,
> +	[C_CAN_IF1_MASK2_REG]	= 0x106,
> +	[C_CAN_IF1_ARB1_REG]	= 0x108,
> +	[C_CAN_IF1_ARB2_REG]	= 0x10A,
> +	[C_CAN_IF1_MSGCTRL_REG]	= 0x10C,
> +	[C_CAN_IF1_DATA1_REG]	= 0x110,
> +	[C_CAN_IF1_DATA2_REG]	= 0x112,
> +	[C_CAN_IF1_DATA3_REG]	= 0x114,
> +	[C_CAN_IF1_DATA4_REG]	= 0x116,
> +};
> +
>  /* c_can private data structure */
>  struct c_can_priv {
>  	struct can_priv can;	/* must be the first member */
> @@ -97,6 +126,7 @@ struct c_can_priv {
>  	unsigned int tx_echo;
>  	void *priv;		/* for board-specific data */
>  	u16 irqstatus;
> +	unsigned int dev_type;

not needed, please use id_table.

>  };
>  
>  struct net_device *alloc_c_can_dev(void);
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 0cca9db..394b05d 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -117,7 +117,24 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	}
>  
>  	priv = netdev_priv(dev);
> -	priv->regs = reg_map_c_can;
> +	if (pdata && (pdata->dev_type == DEV_TYPE_D_CAN)) {
> +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
> +		priv->dev_type = DEV_TYPE_D_CAN;
> +		priv->regs = reg_map_d_can;
> +		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> +		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> +	} else {
> +		priv->dev_type = DEV_TYPE_C_CAN;
> +		priv->regs = reg_map_c_can;
> +		if ((mem->flags & IORESOURCE_MEM_TYPE_MASK) ==
> +					IORESOURCE_MEM_32BIT) {
> +			priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
> +			priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> +		} else {
> +			priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> +			priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> +		}

I personally like the switch..case more.

> +	}
>  
>  	dev->irq = irq;
>  	priv->base = addr;
> @@ -126,18 +143,6 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	priv->priv = clk;
>  #endif
>  
> -	switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> -	case IORESOURCE_MEM_32BIT:
> -		priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
> -		priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> -		break;
> -	case IORESOURCE_MEM_16BIT:
> -	default:
> -		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> -		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> -		break;
> -	}
> -
>  	platform_set_drvdata(pdev, dev);
>  	SET_NETDEV_DEV(dev, &pdev->dev);
>  
> diff --git a/include/linux/can/platform/c_can.h b/include/linux/can/platform/c_can.h
> new file mode 100644
> index 0000000..e95201a
> --- /dev/null
> +++ b/include/linux/can/platform/c_can.h
> @@ -0,0 +1,42 @@
> +/*
> + * C_CAN controller driver platform header
> + *
> + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * Bosch C_CAN/D_CAN controller is compliant to CAN protocol version 2.0
> + * part A and B.
> + * Bosch C_CAN user manual can be obtained from:
> + * http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> + * users_manual_c_can.pdf
> + * Bosch D_CAN user manual can be obtained from:
> + * http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/can/
> + * d_can_users_manual_111.pdf
> + *
> + * 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.
> + */
> +
> +#ifndef __CAN_PLATFORM_TI_C_CAN_H__
> +#define __CAN_PLATFORM_TI_C_CAN_H__
> +
> +#define DEV_TYPE_C_CAN		0
> +#define DEV_TYPE_D_CAN		1
> +
> +/**
> + * struct c_can_platform_data - CCAN Platform Data
> + *
> + * @dev_type:		Device type C_CAN/D_CAN
> + *
> + * Platform data structure to get all platform specific settings.
> + */
> +
> +struct c_can_platform_data {
> +	unsigned int dev_type;
> +};

not needed, use id_table.

> +#endif


-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

Re: [PATCH v2 3/4] can: c_can: Move overlay structure to array with offset as index

[Marc Kleine-Budde]

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

On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> c_can uses overlay structure for accessing c_can module registers.
> With this kind of implementation it is difficult to add one more ip
> which is similar to c_can in functionality but different register
> offsets.
> 
> This patch changes the overlay structure implementation to an array
> with register offset as index. This way we can overcome the above
> limitation.

The array index implementation looks very nice.

I suggest to use the enum instead of a plain "int reg" in the
c_can_read_* function arguments.

General question: What happend to "iface", like in this hunk below?

>  	while (count && priv->read_reg(priv,
> -				&priv->regs->ifregs[iface].com_req) &
> +				C_CAN_IF1_COMREQ_REG) &
>  				IF_COMR_BUSY) {

More comments inline:

> Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> ---
>  drivers/net/can/c_can/c_can.c          |  114 ++++++++++++++++----------------
>  drivers/net/can/c_can/c_can.h          |   96 ++++++++++++++++-----------
>  drivers/net/can/c_can/c_can_platform.c |   25 ++++---
>  3 files changed, 129 insertions(+), 106 deletions(-)
> 
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> index 8dc84d6..4d40dcf 100644
> --- a/drivers/net/can/c_can/c_can.c
> +++ b/drivers/net/can/c_can/c_can.c
> @@ -209,10 +209,10 @@ static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
>  			C_CAN_MSG_OBJ_TX_FIRST;
>  }
>  
> -static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg)
> +static u32 c_can_read_reg32(struct c_can_priv *priv, int reg)
>  {
>  	u32 val = priv->read_reg(priv, reg);
> -	val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
> +	val |= ((u32) priv->read_reg(priv, reg + 1)) << 16;
>  	return val;
>  }
>  
> @@ -220,14 +220,14 @@ static void c_can_enable_all_interrupts(struct c_can_priv *priv,
>  						int enable)
>  {
>  	unsigned int cntrl_save = priv->read_reg(priv,
> -						&priv->regs->control);
> +						C_CAN_CTRL_REG);
>  
>  	if (enable)
>  		cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
>  	else
>  		cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
>  
> -	priv->write_reg(priv, &priv->regs->control, cntrl_save);
> +	priv->write_reg(priv, C_CAN_CTRL_REG, cntrl_save);
>  }
>  
>  static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
> @@ -235,7 +235,7 @@ static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
>  	int count = MIN_TIMEOUT_VALUE;
>  
>  	while (count && priv->read_reg(priv,
> -				&priv->regs->ifregs[iface].com_req) &
> +				C_CAN_IF1_COMREQ_REG) &
>  				IF_COMR_BUSY) {
>  		count--;
>  		udelay(1);
> @@ -258,9 +258,9 @@ static inline void c_can_object_get(struct net_device *dev,
>  	 * register and message RAM must be complete in 6 CAN-CLK
>  	 * period.
>  	 */
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> +	priv->write_reg(priv, C_CAN_IF1_COMMSK_REG,
>  			IFX_WRITE_LOW_16BIT(mask));
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> +	priv->write_reg(priv, C_CAN_IF1_COMREQ_REG,
>  			IFX_WRITE_LOW_16BIT(objno));
>  
>  	if (c_can_msg_obj_is_busy(priv, iface))
> @@ -278,9 +278,9 @@ static inline void c_can_object_put(struct net_device *dev,
>  	 * register and message RAM must be complete in 6 CAN-CLK
>  	 * period.
>  	 */
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> +	priv->write_reg(priv, C_CAN_IF1_COMMSK_REG,
>  			(IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> +	priv->write_reg(priv, C_CAN_IF1_COMREQ_REG,
>  			IFX_WRITE_LOW_16BIT(objno));
>  
>  	if (c_can_msg_obj_is_busy(priv, iface))
> @@ -306,18 +306,18 @@ static void c_can_write_msg_object(struct net_device *dev,
>  
>  	flags |= IF_ARB_MSGVAL;
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> +	priv->write_reg(priv, C_CAN_IF1_ARB1_REG,
>  				IFX_WRITE_LOW_16BIT(id));
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, flags |
> +	priv->write_reg(priv, C_CAN_IF1_ARB2_REG, flags |
>  				IFX_WRITE_HIGH_16BIT(id));
>  
>  	for (i = 0; i < frame->can_dlc; i += 2) {
> -		priv->write_reg(priv, &priv->regs->ifregs[iface].data[i / 2],
> +		priv->write_reg(priv, C_CAN_IF1_DATA1_REG + i / 2,
>  				frame->data[i] | (frame->data[i + 1] << 8));
>  	}
>  
>  	/* enable interrupt for this message object */
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
>  			IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
>  			frame->can_dlc);
>  	c_can_object_put(dev, iface, objno, IF_COMM_ALL);
> @@ -329,7 +329,7 @@ static inline void c_can_mark_rx_msg_obj(struct net_device *dev,
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
>  			ctrl_mask & ~(IF_MCONT_MSGLST | IF_MCONT_INTPND));
>  	c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
>  
> @@ -343,7 +343,7 @@ static inline void c_can_activate_all_lower_rx_msg_obj(struct net_device *dev,
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
>  	for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_MSG_RX_LOW_LAST; i++) {
> -		priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> +		priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
>  				ctrl_mask & ~(IF_MCONT_MSGLST |
>  					IF_MCONT_INTPND | IF_MCONT_NEWDAT));
>  		c_can_object_put(dev, iface, i, IF_COMM_CONTROL);
> @@ -356,7 +356,7 @@ static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
>  			ctrl_mask & ~(IF_MCONT_MSGLST |
>  				IF_MCONT_INTPND | IF_MCONT_NEWDAT));
>  	c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> @@ -374,7 +374,7 @@ static void c_can_handle_lost_msg_obj(struct net_device *dev,
>  
>  	c_can_object_get(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
>  			IF_MCONT_CLR_MSGLST);
>  
>  	c_can_object_put(dev, 0, objno, IF_COMM_CONTROL);
> @@ -410,8 +410,8 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
>  
>  	frame->can_dlc = get_can_dlc(ctrl & 0x0F);
>  
> -	flags =	priv->read_reg(priv, &priv->regs->ifregs[iface].arb2);
> -	val = priv->read_reg(priv, &priv->regs->ifregs[iface].arb1) |
> +	flags =	priv->read_reg(priv, C_CAN_IF1_ARB2_REG);
> +	val = priv->read_reg(priv, C_CAN_IF1_ARB1_REG) |
>  		(flags << 16);
>  
>  	if (flags & IF_ARB_MSGXTD)
> @@ -424,7 +424,7 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
>  	else {
>  		for (i = 0; i < frame->can_dlc; i += 2) {
>  			data = priv->read_reg(priv,
> -				&priv->regs->ifregs[iface].data[i / 2]);
> +				C_CAN_IF1_DATA1_REG + i / 2);
>  			frame->data[i] = data;
>  			frame->data[i + 1] = data >> 8;
>  		}
> @@ -444,40 +444,40 @@ static void c_can_setup_receive_object(struct net_device *dev, int iface,
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].mask1,
> +	priv->write_reg(priv, C_CAN_IF1_MASK1_REG,
>  			IFX_WRITE_LOW_16BIT(mask));
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].mask2,
> +	priv->write_reg(priv, C_CAN_IF1_MASK2_REG,
>  			IFX_WRITE_HIGH_16BIT(mask));
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> +	priv->write_reg(priv, C_CAN_IF1_ARB1_REG,
>  			IFX_WRITE_LOW_16BIT(id));
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2,
> +	priv->write_reg(priv, C_CAN_IF1_ARB2_REG,
>  			(IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, mcont);
> +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG, mcont);
>  	c_can_object_put(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
>  
>  	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> -			c_can_read_reg32(priv, &priv->regs->msgval1));
> +			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
>  }
>  
>  static void c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
>  {
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1, 0);
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, 0);
> -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, 0);
> +	priv->write_reg(priv, C_CAN_IF1_ARB1_REG, 0);
> +	priv->write_reg(priv, C_CAN_IF1_ARB2_REG, 0);
> +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG, 0);
>  
>  	c_can_object_put(dev, iface, objno, IF_COMM_ARB | IF_COMM_CONTROL);
>  
>  	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> -			c_can_read_reg32(priv, &priv->regs->msgval1));
> +			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
>  }
>  
>  static inline int c_can_is_next_tx_obj_busy(struct c_can_priv *priv, int objno)
>  {
> -	int val = c_can_read_reg32(priv, &priv->regs->txrqst1);
> +	int val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
>  
>  	/*
>  	 * as transmission request register's bit n-1 corresponds to
> @@ -540,12 +540,12 @@ static int c_can_set_bittiming(struct net_device *dev)
>  	netdev_info(dev,
>  		"setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
>  
> -	ctrl_save = priv->read_reg(priv, &priv->regs->control);
> -	priv->write_reg(priv, &priv->regs->control,
> +	ctrl_save = priv->read_reg(priv, C_CAN_CTRL_REG);
> +	priv->write_reg(priv, C_CAN_CTRL_REG,
>  			ctrl_save | CONTROL_CCE | CONTROL_INIT);
> -	priv->write_reg(priv, &priv->regs->btr, reg_btr);
> -	priv->write_reg(priv, &priv->regs->brp_ext, reg_brpe);
> -	priv->write_reg(priv, &priv->regs->control, ctrl_save);
> +	priv->write_reg(priv, C_CAN_BTR_REG, reg_btr);
> +	priv->write_reg(priv, C_CAN_BRPEXT_REG, reg_brpe);
> +	priv->write_reg(priv, C_CAN_CTRL_REG, ctrl_save);
>  
>  	return 0;
>  }
> @@ -587,36 +587,36 @@ static void c_can_chip_config(struct net_device *dev)
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
>  	/* enable automatic retransmission */
> -	priv->write_reg(priv, &priv->regs->control,
> +	priv->write_reg(priv, C_CAN_CTRL_REG,
>  			CONTROL_ENABLE_AR);
>  
>  	if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
>  					CAN_CTRLMODE_LOOPBACK)) {
>  		/* loopback + silent mode : useful for hot self-test */
> -		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
>  				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> -		priv->write_reg(priv, &priv->regs->test,
> +		priv->write_reg(priv, C_CAN_TEST_REG,
>  				TEST_LBACK | TEST_SILENT);
>  	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
>  		/* loopback mode : useful for self-test function */
> -		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
>  				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> -		priv->write_reg(priv, &priv->regs->test, TEST_LBACK);
> +		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
>  	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
>  		/* silent mode : bus-monitoring mode */
> -		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
>  				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> -		priv->write_reg(priv, &priv->regs->test, TEST_SILENT);
> +		priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
>  	} else
>  		/* normal mode*/
> -		priv->write_reg(priv, &priv->regs->control,
> +		priv->write_reg(priv, C_CAN_CTRL_REG,
>  				CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
>  
>  	/* configure message objects */
>  	c_can_configure_msg_objects(dev);
>  
>  	/* set a `lec` value so that we can check for updates later */
> -	priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
> +	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
>  
>  	/* set bittiming params */
>  	c_can_set_bittiming(dev);
> @@ -669,7 +669,7 @@ static int c_can_get_berr_counter(const struct net_device *dev,
>  	unsigned int reg_err_counter;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> +	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
>  	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
>  				ERR_CNT_REC_SHIFT;
>  	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
> @@ -697,12 +697,12 @@ static void c_can_do_tx(struct net_device *dev)
>  
>  	for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
>  		msg_obj_no = get_tx_echo_msg_obj(priv);
> -		val = c_can_read_reg32(priv, &priv->regs->txrqst1);
> +		val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
>  		if (!(val & (1 << (msg_obj_no - 1)))) {
>  			can_get_echo_skb(dev,
>  					msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
>  			stats->tx_bytes += priv->read_reg(priv,
> -					&priv->regs->ifregs[0].msg_cntrl)
> +					C_CAN_IF1_MSGCTRL_REG)
>  					& IF_MCONT_DLC_MASK;
>  			stats->tx_packets++;
>  			c_can_inval_msg_object(dev, 0, msg_obj_no);
> @@ -744,11 +744,11 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
>  	u32 num_rx_pkts = 0;
>  	unsigned int msg_obj, msg_ctrl_save;
>  	struct c_can_priv *priv = netdev_priv(dev);
> -	u32 val = c_can_read_reg32(priv, &priv->regs->intpnd1);
> +	u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
>  
>  	for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
>  			msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
> -			val = c_can_read_reg32(priv, &priv->regs->intpnd1),
> +			val = c_can_read_reg32(priv, C_CAN_INTPND1_REG),
>  			msg_obj++) {
>  		/*
>  		 * as interrupt pending register's bit n-1 corresponds to
> @@ -758,7 +758,7 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
>  			c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
>  					~IF_COMM_TXRQST);
>  			msg_ctrl_save = priv->read_reg(priv,
> -					&priv->regs->ifregs[0].msg_cntrl);
> +					C_CAN_IF1_MSGCTRL_REG);
>  
>  			if (msg_ctrl_save & IF_MCONT_EOB)
>  				return num_rx_pkts;
> @@ -819,7 +819,7 @@ static int c_can_handle_state_change(struct net_device *dev,
>  		return 0;
>  
>  	c_can_get_berr_counter(dev, &bec);
> -	reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> +	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
>  	rx_err_passive = (reg_err_counter & ERR_CNT_RP_MASK) >>
>  				ERR_CNT_RP_SHIFT;
>  
> @@ -935,7 +935,7 @@ static int c_can_handle_bus_err(struct net_device *dev,
>  	}
>  
>  	/* set a `lec` value so that we can check for updates later */
> -	priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
> +	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
>  
>  	netif_receive_skb(skb);
>  	stats->rx_packets++;
> @@ -959,15 +959,15 @@ static int c_can_poll(struct napi_struct *napi, int quota)
>  	/* status events have the highest priority */
>  	if (irqstatus == STATUS_INTERRUPT) {
>  		priv->current_status = priv->read_reg(priv,
> -					&priv->regs->status);
> +					C_CAN_STS_REG);
>  
>  		/* handle Tx/Rx events */
>  		if (priv->current_status & STATUS_TXOK)
> -			priv->write_reg(priv, &priv->regs->status,
> +			priv->write_reg(priv, C_CAN_STS_REG,
>  					priv->current_status & ~STATUS_TXOK);
>  
>  		if (priv->current_status & STATUS_RXOK)
> -			priv->write_reg(priv, &priv->regs->status,
> +			priv->write_reg(priv, C_CAN_STS_REG,
>  					priv->current_status & ~STATUS_RXOK);
>  
>  		/* handle state changes */
> @@ -1033,7 +1033,7 @@ static irqreturn_t c_can_isr(int irq, void *dev_id)
>  	struct net_device *dev = (struct net_device *)dev_id;
>  	struct c_can_priv *priv = netdev_priv(dev);
>  
> -	priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> +	priv->irqstatus = priv->read_reg(priv, C_CAN_INT_REG);
>  	if (!priv->irqstatus)
>  		return IRQ_NONE;
>  
> diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> index 5f32d34..747d478 100644
> --- a/drivers/net/can/c_can/c_can.h
> +++ b/drivers/net/can/c_can/c_can.h
> @@ -22,43 +22,62 @@
>  #ifndef C_CAN_H
>  #define C_CAN_H
>  
> -/* c_can IF registers */
> -struct c_can_if_regs {
> -	u16 com_req;
> -	u16 com_mask;
> -	u16 mask1;
> -	u16 mask2;
> -	u16 arb1;
> -	u16 arb2;
> -	u16 msg_cntrl;
> -	u16 data[4];
> -	u16 _reserved[13];
> +enum {
> +	C_CAN_CTRL_REG = 0,
> +	C_CAN_STS_REG,
> +	C_CAN_ERR_CNT_REG,
> +	C_CAN_BTR_REG,
> +	C_CAN_INT_REG,
> +	C_CAN_TEST_REG,
> +	C_CAN_BRPEXT_REG,
> +	C_CAN_IF1_COMREQ_REG,
> +	C_CAN_IF1_COMMSK_REG,
> +	C_CAN_IF1_MASK1_REG,
> +	C_CAN_IF1_MASK2_REG,
> +	C_CAN_IF1_ARB1_REG,
> +	C_CAN_IF1_ARB2_REG,
> +	C_CAN_IF1_MSGCTRL_REG,
> +	C_CAN_IF1_DATA1_REG,
> +	C_CAN_IF1_DATA2_REG,
> +	C_CAN_IF1_DATA3_REG,
> +	C_CAN_IF1_DATA4_REG,
> +	C_CAN_TXRQST1_REG,
> +	C_CAN_TXRQST2_REG,
> +	C_CAN_NEWDAT1_REG,
> +	C_CAN_NEWDAT2_REG,
> +	C_CAN_INTPND1_REG,
> +	C_CAN_INTPND2_REG,
> +	C_CAN_MSGVAL1_REG,
> +	C_CAN_MSGVAL2_REG,
>  };
>  
> -/* c_can hardware registers */
> -struct c_can_regs {
> -	u16 control;
> -	u16 status;
> -	u16 err_cnt;
> -	u16 btr;
> -	u16 interrupt;
> -	u16 test;
> -	u16 brp_ext;
> -	u16 _reserved1;
> -	struct c_can_if_regs ifregs[2]; /* [0] = IF1 and [1] = IF2 */
> -	u16 _reserved2[8];
> -	u16 txrqst1;
> -	u16 txrqst2;
> -	u16 _reserved3[6];
> -	u16 newdat1;
> -	u16 newdat2;
> -	u16 _reserved4[6];
> -	u16 intpnd1;
> -	u16 intpnd2;
> -	u16 _reserved5[6];
> -	u16 msgval1;
> -	u16 msgval2;
> -	u16 _reserved6[6];
> +static u16 reg_map_c_can[] = {

const?

> +	[C_CAN_CTRL_REG]	= 0x00,
> +	[C_CAN_STS_REG]		= 0x02,
> +	[C_CAN_ERR_CNT_REG]	= 0x04,
> +	[C_CAN_BTR_REG]		= 0x06,
> +	[C_CAN_INT_REG]		= 0x08,
> +	[C_CAN_TEST_REG]	= 0x0A,
> +	[C_CAN_BRPEXT_REG]	= 0x0C,
> +	[C_CAN_IF1_COMREQ_REG]	= 0x10,
> +	[C_CAN_IF1_COMMSK_REG]	= 0x12,
> +	[C_CAN_IF1_MASK1_REG]	= 0x14,
> +	[C_CAN_IF1_MASK2_REG]	= 0x16,
> +	[C_CAN_IF1_ARB1_REG]	= 0x18,
> +	[C_CAN_IF1_ARB2_REG]	= 0x1A,
> +	[C_CAN_IF1_MSGCTRL_REG]	= 0x1C,
> +	[C_CAN_IF1_DATA1_REG]	= 0x1E,
> +	[C_CAN_IF1_DATA2_REG]	= 0x20,
> +	[C_CAN_IF1_DATA3_REG]	= 0x22,
> +	[C_CAN_IF1_DATA4_REG]	= 0x24,
> +	[C_CAN_TXRQST1_REG]	= 0x80,
> +	[C_CAN_TXRQST2_REG]	= 0x82,
> +	[C_CAN_NEWDAT1_REG]	= 0x90,
> +	[C_CAN_NEWDAT2_REG]	= 0x92,
> +	[C_CAN_INTPND1_REG]	= 0xA0,
> +	[C_CAN_INTPND2_REG]	= 0xA2,
> +	[C_CAN_MSGVAL1_REG]	= 0xB0,
> +	[C_CAN_MSGVAL2_REG]	= 0xB2,
>  };
>  
>  /* c_can private data structure */
> @@ -69,9 +88,10 @@ struct c_can_priv {
>  	int tx_object;
>  	int current_status;
>  	int last_status;
> -	u16 (*read_reg) (struct c_can_priv *priv, void *reg);
> -	void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
> -	struct c_can_regs __iomem *regs;
> +	u16 (*read_reg) (struct c_can_priv *priv, int reg);
> +	void (*write_reg) (struct c_can_priv *priv, int reg, u16 val);
> +	void __iomem *base;
> +	u16 *regs;
>  	unsigned long irq_flags; /* for request_irq() */
>  	unsigned int tx_next;
>  	unsigned int tx_echo;
> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> index 5e1a5ff..0cca9db 100644
> --- a/drivers/net/can/c_can/c_can_platform.c
> +++ b/drivers/net/can/c_can/c_can_platform.c
> @@ -32,6 +32,7 @@
>  #include <linux/clk.h>
>  
>  #include <linux/can/dev.h>
> +#include <linux/can/platform/c_can.h>
>  
>  #include "c_can.h"
>  
> @@ -42,27 +43,27 @@
>   * Handle the same by providing a common read/write interface.
>   */
>  static u16 c_can_plat_read_reg_aligned_to_16bit(struct c_can_priv *priv,
> -						void *reg)
> +						int reg)
>  {
> -	return readw(reg);
> +	return readw(priv->base + priv->regs[reg]);
>  }
>  
>  static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv *priv,
> -						void *reg, u16 val)
> +						int reg, u16 val)
>  {
> -	writew(val, reg);
> +	writew(val, priv->base + priv->regs[reg]);
>  }
>  
>  static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv *priv,
> -						void *reg)
> +						int reg)
>  {
> -	return readw(reg + (long)reg - (long)priv->regs);
> +	return readw(priv->base + 2 * priv->regs[reg]);
>  }
>  
>  static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv *priv,
> -						void *reg, u16 val)
> +						int reg, u16 val)
>  {
> -	writew(val, reg + (long)reg - (long)priv->regs);
> +	writew(val, priv->base + 2 * priv->regs[reg]);
>  }
>  
>  static int __devinit c_can_plat_probe(struct platform_device *pdev)
> @@ -71,6 +72,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	void __iomem *addr;
>  	struct net_device *dev;
>  	struct c_can_priv *priv;
> +	struct c_can_platform_data *pdata = pdev->dev.platform_data;
>  	struct resource *mem;
>  	int irq;
>  #ifdef CONFIG_HAVE_CLK
> @@ -115,9 +117,10 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	}
>  
>  	priv = netdev_priv(dev);
> +	priv->regs = reg_map_c_can;
>  
>  	dev->irq = irq;
> -	priv->regs = addr;
> +	priv->base = addr;
>  #ifdef CONFIG_HAVE_CLK
>  	priv->can.clock.freq = clk_get_rate(clk);
>  	priv->priv = clk;
> @@ -146,7 +149,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
>  	}
>  
>  	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> -		 KBUILD_MODNAME, priv->regs, dev->irq);
> +		 KBUILD_MODNAME, priv->base, dev->irq);
>  	return 0;
>  
>  exit_free_device:
> @@ -176,7 +179,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, NULL);
>  
>  	free_c_can_dev(dev);
> -	iounmap(priv->regs);
> +	iounmap(priv->base);
>  
>  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	release_mem_region(mem->start, resource_size(mem));

Marc
-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

RE: [PATCH v2 2/4] can: c_can: fix: enable CAN HW interrupts after napi_enable()

[AnilKumar, Chimata]

Hi Marc,

Thanks for the comments,

On Fri, May 11, 2012 at 00:46:55, Marc Kleine-Budde wrote:
> On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> > Fix the issue of C_CAN interrupts getting disabled forever even though
> > we do can configuration multiple times using can utils. According to
> > NAPI usage we disable all the hardware interrupts in ISR and re-enable
> > them in poll(). Current implementation calls napi_enable() after hardware
> > interrupts are enabled. If we get any interrupts between these two steps
> > then we do not process those interrupts because napi is not enabled. Mostly
> > these interrupts come because of STATUS is not 0x7 or ERROR interrupts. If
> > napi_enable() happens before hardware interrupts enabled then c_can_poll()
> > function will be called eventual re-enabling of interrupts will happen.
> > 
> > This patch moves the napi_enable() call before interrupts enabled.
> > 
> > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> 
> Please test on c_can, also this is a stable candidate. What about add
> the term "race condition" to the subject?

I am changing it to
"can: c_can: fix race condition in c_can_open()"

> 
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> 
> Marc
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 
> 

RE: [PATCH v2 3/4] can: c_can: Move overlay structure to array with offset as index

[AnilKumar, Chimata]

On Fri, May 11, 2012 at 01:42:43, Marc Kleine-Budde wrote:
> On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> > c_can uses overlay structure for accessing c_can module registers.
> > With this kind of implementation it is difficult to add one more ip
> > which is similar to c_can in functionality but different register
> > offsets.
> > 
> > This patch changes the overlay structure implementation to an array
> > with register offset as index. This way we can overcome the above
> > limitation.
> 
> The array index implementation looks very nice.
> 
> I suggest to use the enum instead of a plain "int reg" in the
> c_can_read_* function arguments.

That is better compared to int reg, I will change.

> 
> General question: What happend to "iface", like in this hunk below?

In entire driver iface is used as "0" means IF register bank one. So I defined
enum like "C_CAN_IF1_*". If IF register bank 2 is used in the driver at that
time we can define "C_CAN_IF2_*" and used appropriately.

> 
> >  	while (count && priv->read_reg(priv,
> > -				&priv->regs->ifregs[iface].com_req) &
> > +				C_CAN_IF1_COMREQ_REG) &
> >  				IF_COMR_BUSY) {
> 
> More comments inline:
> 
> > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> > ---
> >  drivers/net/can/c_can/c_can.c          |  114 ++++++++++++++++----------------
> >  drivers/net/can/c_can/c_can.h          |   96 ++++++++++++++++-----------
> >  drivers/net/can/c_can/c_can_platform.c |   25 ++++---
> >  3 files changed, 129 insertions(+), 106 deletions(-)
> > 
> > diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
> > index 8dc84d6..4d40dcf 100644
> > --- a/drivers/net/can/c_can/c_can.c
> > +++ b/drivers/net/can/c_can/c_can.c
> > @@ -209,10 +209,10 @@ static inline int get_tx_echo_msg_obj(const struct c_can_priv *priv)
> >  			C_CAN_MSG_OBJ_TX_FIRST;
> >  }
> >  
> > -static u32 c_can_read_reg32(struct c_can_priv *priv, void *reg)
> > +static u32 c_can_read_reg32(struct c_can_priv *priv, int reg)
> >  {
> >  	u32 val = priv->read_reg(priv, reg);
> > -	val |= ((u32) priv->read_reg(priv, reg + 2)) << 16;
> > +	val |= ((u32) priv->read_reg(priv, reg + 1)) << 16;
> >  	return val;
> >  }
> >  
> > @@ -220,14 +220,14 @@ static void c_can_enable_all_interrupts(struct c_can_priv *priv,
> >  						int enable)
> >  {
> >  	unsigned int cntrl_save = priv->read_reg(priv,
> > -						&priv->regs->control);
> > +						C_CAN_CTRL_REG);
> >  
> >  	if (enable)
> >  		cntrl_save |= (CONTROL_SIE | CONTROL_EIE | CONTROL_IE);
> >  	else
> >  		cntrl_save &= ~(CONTROL_EIE | CONTROL_IE | CONTROL_SIE);
> >  
> > -	priv->write_reg(priv, &priv->regs->control, cntrl_save);
> > +	priv->write_reg(priv, C_CAN_CTRL_REG, cntrl_save);
> >  }
> >  
> >  static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
> > @@ -235,7 +235,7 @@ static inline int c_can_msg_obj_is_busy(struct c_can_priv *priv, int iface)
> >  	int count = MIN_TIMEOUT_VALUE;
> >  
> >  	while (count && priv->read_reg(priv,
> > -				&priv->regs->ifregs[iface].com_req) &
> > +				C_CAN_IF1_COMREQ_REG) &
> >  				IF_COMR_BUSY) {
> >  		count--;
> >  		udelay(1);
> > @@ -258,9 +258,9 @@ static inline void c_can_object_get(struct net_device *dev,
> >  	 * register and message RAM must be complete in 6 CAN-CLK
> >  	 * period.
> >  	 */
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> > +	priv->write_reg(priv, C_CAN_IF1_COMMSK_REG,
> >  			IFX_WRITE_LOW_16BIT(mask));
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> > +	priv->write_reg(priv, C_CAN_IF1_COMREQ_REG,
> >  			IFX_WRITE_LOW_16BIT(objno));
> >  
> >  	if (c_can_msg_obj_is_busy(priv, iface))
> > @@ -278,9 +278,9 @@ static inline void c_can_object_put(struct net_device *dev,
> >  	 * register and message RAM must be complete in 6 CAN-CLK
> >  	 * period.
> >  	 */
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_mask,
> > +	priv->write_reg(priv, C_CAN_IF1_COMMSK_REG,
> >  			(IF_COMM_WR | IFX_WRITE_LOW_16BIT(mask)));
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].com_req,
> > +	priv->write_reg(priv, C_CAN_IF1_COMREQ_REG,
> >  			IFX_WRITE_LOW_16BIT(objno));
> >  
> >  	if (c_can_msg_obj_is_busy(priv, iface))
> > @@ -306,18 +306,18 @@ static void c_can_write_msg_object(struct net_device *dev,
> >  
> >  	flags |= IF_ARB_MSGVAL;
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> > +	priv->write_reg(priv, C_CAN_IF1_ARB1_REG,
> >  				IFX_WRITE_LOW_16BIT(id));
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, flags |
> > +	priv->write_reg(priv, C_CAN_IF1_ARB2_REG, flags |
> >  				IFX_WRITE_HIGH_16BIT(id));
> >  
> >  	for (i = 0; i < frame->can_dlc; i += 2) {
> > -		priv->write_reg(priv, &priv->regs->ifregs[iface].data[i / 2],
> > +		priv->write_reg(priv, C_CAN_IF1_DATA1_REG + i / 2,
> >  				frame->data[i] | (frame->data[i + 1] << 8));
> >  	}
> >  
> >  	/* enable interrupt for this message object */
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
> >  			IF_MCONT_TXIE | IF_MCONT_TXRQST | IF_MCONT_EOB |
> >  			frame->can_dlc);
> >  	c_can_object_put(dev, iface, objno, IF_COMM_ALL);
> > @@ -329,7 +329,7 @@ static inline void c_can_mark_rx_msg_obj(struct net_device *dev,
> >  {
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
> >  			ctrl_mask & ~(IF_MCONT_MSGLST | IF_MCONT_INTPND));
> >  	c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> >  
> > @@ -343,7 +343,7 @@ static inline void c_can_activate_all_lower_rx_msg_obj(struct net_device *dev,
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> >  	for (i = C_CAN_MSG_OBJ_RX_FIRST; i <= C_CAN_MSG_RX_LOW_LAST; i++) {
> > -		priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +		priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
> >  				ctrl_mask & ~(IF_MCONT_MSGLST |
> >  					IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> >  		c_can_object_put(dev, iface, i, IF_COMM_CONTROL);
> > @@ -356,7 +356,7 @@ static inline void c_can_activate_rx_msg_obj(struct net_device *dev,
> >  {
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
> >  			ctrl_mask & ~(IF_MCONT_MSGLST |
> >  				IF_MCONT_INTPND | IF_MCONT_NEWDAT));
> >  	c_can_object_put(dev, iface, obj, IF_COMM_CONTROL);
> > @@ -374,7 +374,7 @@ static void c_can_handle_lost_msg_obj(struct net_device *dev,
> >  
> >  	c_can_object_get(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl,
> > +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG,
> >  			IF_MCONT_CLR_MSGLST);
> >  
> >  	c_can_object_put(dev, 0, objno, IF_COMM_CONTROL);
> > @@ -410,8 +410,8 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
> >  
> >  	frame->can_dlc = get_can_dlc(ctrl & 0x0F);
> >  
> > -	flags =	priv->read_reg(priv, &priv->regs->ifregs[iface].arb2);
> > -	val = priv->read_reg(priv, &priv->regs->ifregs[iface].arb1) |
> > +	flags =	priv->read_reg(priv, C_CAN_IF1_ARB2_REG);
> > +	val = priv->read_reg(priv, C_CAN_IF1_ARB1_REG) |
> >  		(flags << 16);
> >  
> >  	if (flags & IF_ARB_MSGXTD)
> > @@ -424,7 +424,7 @@ static int c_can_read_msg_object(struct net_device *dev, int iface, int ctrl)
> >  	else {
> >  		for (i = 0; i < frame->can_dlc; i += 2) {
> >  			data = priv->read_reg(priv,
> > -				&priv->regs->ifregs[iface].data[i / 2]);
> > +				C_CAN_IF1_DATA1_REG + i / 2);
> >  			frame->data[i] = data;
> >  			frame->data[i + 1] = data >> 8;
> >  		}
> > @@ -444,40 +444,40 @@ static void c_can_setup_receive_object(struct net_device *dev, int iface,
> >  {
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].mask1,
> > +	priv->write_reg(priv, C_CAN_IF1_MASK1_REG,
> >  			IFX_WRITE_LOW_16BIT(mask));
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].mask2,
> > +	priv->write_reg(priv, C_CAN_IF1_MASK2_REG,
> >  			IFX_WRITE_HIGH_16BIT(mask));
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1,
> > +	priv->write_reg(priv, C_CAN_IF1_ARB1_REG,
> >  			IFX_WRITE_LOW_16BIT(id));
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2,
> > +	priv->write_reg(priv, C_CAN_IF1_ARB2_REG,
> >  			(IF_ARB_MSGVAL | IFX_WRITE_HIGH_16BIT(id)));
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, mcont);
> > +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG, mcont);
> >  	c_can_object_put(dev, iface, objno, IF_COMM_ALL & ~IF_COMM_TXRQST);
> >  
> >  	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> > -			c_can_read_reg32(priv, &priv->regs->msgval1));
> > +			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
> >  }
> >  
> >  static void c_can_inval_msg_object(struct net_device *dev, int iface, int objno)
> >  {
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb1, 0);
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].arb2, 0);
> > -	priv->write_reg(priv, &priv->regs->ifregs[iface].msg_cntrl, 0);
> > +	priv->write_reg(priv, C_CAN_IF1_ARB1_REG, 0);
> > +	priv->write_reg(priv, C_CAN_IF1_ARB2_REG, 0);
> > +	priv->write_reg(priv, C_CAN_IF1_MSGCTRL_REG, 0);
> >  
> >  	c_can_object_put(dev, iface, objno, IF_COMM_ARB | IF_COMM_CONTROL);
> >  
> >  	netdev_dbg(dev, "obj no:%d, msgval:0x%08x\n", objno,
> > -			c_can_read_reg32(priv, &priv->regs->msgval1));
> > +			c_can_read_reg32(priv, C_CAN_MSGVAL1_REG));
> >  }
> >  
> >  static inline int c_can_is_next_tx_obj_busy(struct c_can_priv *priv, int objno)
> >  {
> > -	int val = c_can_read_reg32(priv, &priv->regs->txrqst1);
> > +	int val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
> >  
> >  	/*
> >  	 * as transmission request register's bit n-1 corresponds to
> > @@ -540,12 +540,12 @@ static int c_can_set_bittiming(struct net_device *dev)
> >  	netdev_info(dev,
> >  		"setting BTR=%04x BRPE=%04x\n", reg_btr, reg_brpe);
> >  
> > -	ctrl_save = priv->read_reg(priv, &priv->regs->control);
> > -	priv->write_reg(priv, &priv->regs->control,
> > +	ctrl_save = priv->read_reg(priv, C_CAN_CTRL_REG);
> > +	priv->write_reg(priv, C_CAN_CTRL_REG,
> >  			ctrl_save | CONTROL_CCE | CONTROL_INIT);
> > -	priv->write_reg(priv, &priv->regs->btr, reg_btr);
> > -	priv->write_reg(priv, &priv->regs->brp_ext, reg_brpe);
> > -	priv->write_reg(priv, &priv->regs->control, ctrl_save);
> > +	priv->write_reg(priv, C_CAN_BTR_REG, reg_btr);
> > +	priv->write_reg(priv, C_CAN_BRPEXT_REG, reg_brpe);
> > +	priv->write_reg(priv, C_CAN_CTRL_REG, ctrl_save);
> >  
> >  	return 0;
> >  }
> > @@ -587,36 +587,36 @@ static void c_can_chip_config(struct net_device *dev)
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> >  	/* enable automatic retransmission */
> > -	priv->write_reg(priv, &priv->regs->control,
> > +	priv->write_reg(priv, C_CAN_CTRL_REG,
> >  			CONTROL_ENABLE_AR);
> >  
> >  	if (priv->can.ctrlmode & (CAN_CTRLMODE_LISTENONLY &
> >  					CAN_CTRLMODE_LOOPBACK)) {
> >  		/* loopback + silent mode : useful for hot self-test */
> > -		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> >  				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > -		priv->write_reg(priv, &priv->regs->test,
> > +		priv->write_reg(priv, C_CAN_TEST_REG,
> >  				TEST_LBACK | TEST_SILENT);
> >  	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> >  		/* loopback mode : useful for self-test function */
> > -		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> >  				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > -		priv->write_reg(priv, &priv->regs->test, TEST_LBACK);
> > +		priv->write_reg(priv, C_CAN_TEST_REG, TEST_LBACK);
> >  	} else if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY) {
> >  		/* silent mode : bus-monitoring mode */
> > -		priv->write_reg(priv, &priv->regs->control, CONTROL_EIE |
> > +		priv->write_reg(priv, C_CAN_CTRL_REG, CONTROL_EIE |
> >  				CONTROL_SIE | CONTROL_IE | CONTROL_TEST);
> > -		priv->write_reg(priv, &priv->regs->test, TEST_SILENT);
> > +		priv->write_reg(priv, C_CAN_TEST_REG, TEST_SILENT);
> >  	} else
> >  		/* normal mode*/
> > -		priv->write_reg(priv, &priv->regs->control,
> > +		priv->write_reg(priv, C_CAN_CTRL_REG,
> >  				CONTROL_EIE | CONTROL_SIE | CONTROL_IE);
> >  
> >  	/* configure message objects */
> >  	c_can_configure_msg_objects(dev);
> >  
> >  	/* set a `lec` value so that we can check for updates later */
> > -	priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
> > +	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
> >  
> >  	/* set bittiming params */
> >  	c_can_set_bittiming(dev);
> > @@ -669,7 +669,7 @@ static int c_can_get_berr_counter(const struct net_device *dev,
> >  	unsigned int reg_err_counter;
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > -	reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
> >  	bec->rxerr = (reg_err_counter & ERR_CNT_REC_MASK) >>
> >  				ERR_CNT_REC_SHIFT;
> >  	bec->txerr = reg_err_counter & ERR_CNT_TEC_MASK;
> > @@ -697,12 +697,12 @@ static void c_can_do_tx(struct net_device *dev)
> >  
> >  	for (/* nix */; (priv->tx_next - priv->tx_echo) > 0; priv->tx_echo++) {
> >  		msg_obj_no = get_tx_echo_msg_obj(priv);
> > -		val = c_can_read_reg32(priv, &priv->regs->txrqst1);
> > +		val = c_can_read_reg32(priv, C_CAN_TXRQST1_REG);
> >  		if (!(val & (1 << (msg_obj_no - 1)))) {
> >  			can_get_echo_skb(dev,
> >  					msg_obj_no - C_CAN_MSG_OBJ_TX_FIRST);
> >  			stats->tx_bytes += priv->read_reg(priv,
> > -					&priv->regs->ifregs[0].msg_cntrl)
> > +					C_CAN_IF1_MSGCTRL_REG)
> >  					& IF_MCONT_DLC_MASK;
> >  			stats->tx_packets++;
> >  			c_can_inval_msg_object(dev, 0, msg_obj_no);
> > @@ -744,11 +744,11 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
> >  	u32 num_rx_pkts = 0;
> >  	unsigned int msg_obj, msg_ctrl_save;
> >  	struct c_can_priv *priv = netdev_priv(dev);
> > -	u32 val = c_can_read_reg32(priv, &priv->regs->intpnd1);
> > +	u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
> >  
> >  	for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
> >  			msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
> > -			val = c_can_read_reg32(priv, &priv->regs->intpnd1),
> > +			val = c_can_read_reg32(priv, C_CAN_INTPND1_REG),
> >  			msg_obj++) {
> >  		/*
> >  		 * as interrupt pending register's bit n-1 corresponds to
> > @@ -758,7 +758,7 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
> >  			c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
> >  					~IF_COMM_TXRQST);
> >  			msg_ctrl_save = priv->read_reg(priv,
> > -					&priv->regs->ifregs[0].msg_cntrl);
> > +					C_CAN_IF1_MSGCTRL_REG);
> >  
> >  			if (msg_ctrl_save & IF_MCONT_EOB)
> >  				return num_rx_pkts;
> > @@ -819,7 +819,7 @@ static int c_can_handle_state_change(struct net_device *dev,
> >  		return 0;
> >  
> >  	c_can_get_berr_counter(dev, &bec);
> > -	reg_err_counter = priv->read_reg(priv, &priv->regs->err_cnt);
> > +	reg_err_counter = priv->read_reg(priv, C_CAN_ERR_CNT_REG);
> >  	rx_err_passive = (reg_err_counter & ERR_CNT_RP_MASK) >>
> >  				ERR_CNT_RP_SHIFT;
> >  
> > @@ -935,7 +935,7 @@ static int c_can_handle_bus_err(struct net_device *dev,
> >  	}
> >  
> >  	/* set a `lec` value so that we can check for updates later */
> > -	priv->write_reg(priv, &priv->regs->status, LEC_UNUSED);
> > +	priv->write_reg(priv, C_CAN_STS_REG, LEC_UNUSED);
> >  
> >  	netif_receive_skb(skb);
> >  	stats->rx_packets++;
> > @@ -959,15 +959,15 @@ static int c_can_poll(struct napi_struct *napi, int quota)
> >  	/* status events have the highest priority */
> >  	if (irqstatus == STATUS_INTERRUPT) {
> >  		priv->current_status = priv->read_reg(priv,
> > -					&priv->regs->status);
> > +					C_CAN_STS_REG);
> >  
> >  		/* handle Tx/Rx events */
> >  		if (priv->current_status & STATUS_TXOK)
> > -			priv->write_reg(priv, &priv->regs->status,
> > +			priv->write_reg(priv, C_CAN_STS_REG,
> >  					priv->current_status & ~STATUS_TXOK);
> >  
> >  		if (priv->current_status & STATUS_RXOK)
> > -			priv->write_reg(priv, &priv->regs->status,
> > +			priv->write_reg(priv, C_CAN_STS_REG,
> >  					priv->current_status & ~STATUS_RXOK);
> >  
> >  		/* handle state changes */
> > @@ -1033,7 +1033,7 @@ static irqreturn_t c_can_isr(int irq, void *dev_id)
> >  	struct net_device *dev = (struct net_device *)dev_id;
> >  	struct c_can_priv *priv = netdev_priv(dev);
> >  
> > -	priv->irqstatus = priv->read_reg(priv, &priv->regs->interrupt);
> > +	priv->irqstatus = priv->read_reg(priv, C_CAN_INT_REG);
> >  	if (!priv->irqstatus)
> >  		return IRQ_NONE;
> >  
> > diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> > index 5f32d34..747d478 100644
> > --- a/drivers/net/can/c_can/c_can.h
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -22,43 +22,62 @@
> >  #ifndef C_CAN_H
> >  #define C_CAN_H
> >  
> > -/* c_can IF registers */
> > -struct c_can_if_regs {
> > -	u16 com_req;
> > -	u16 com_mask;
> > -	u16 mask1;
> > -	u16 mask2;
> > -	u16 arb1;
> > -	u16 arb2;
> > -	u16 msg_cntrl;
> > -	u16 data[4];
> > -	u16 _reserved[13];
> > +enum {
> > +	C_CAN_CTRL_REG = 0,
> > +	C_CAN_STS_REG,
> > +	C_CAN_ERR_CNT_REG,
> > +	C_CAN_BTR_REG,
> > +	C_CAN_INT_REG,
> > +	C_CAN_TEST_REG,
> > +	C_CAN_BRPEXT_REG,
> > +	C_CAN_IF1_COMREQ_REG,
> > +	C_CAN_IF1_COMMSK_REG,
> > +	C_CAN_IF1_MASK1_REG,
> > +	C_CAN_IF1_MASK2_REG,
> > +	C_CAN_IF1_ARB1_REG,
> > +	C_CAN_IF1_ARB2_REG,
> > +	C_CAN_IF1_MSGCTRL_REG,
> > +	C_CAN_IF1_DATA1_REG,
> > +	C_CAN_IF1_DATA2_REG,
> > +	C_CAN_IF1_DATA3_REG,
> > +	C_CAN_IF1_DATA4_REG,
> > +	C_CAN_TXRQST1_REG,
> > +	C_CAN_TXRQST2_REG,
> > +	C_CAN_NEWDAT1_REG,
> > +	C_CAN_NEWDAT2_REG,
> > +	C_CAN_INTPND1_REG,
> > +	C_CAN_INTPND2_REG,
> > +	C_CAN_MSGVAL1_REG,
> > +	C_CAN_MSGVAL2_REG,
> >  };
> >  
> > -/* c_can hardware registers */
> > -struct c_can_regs {
> > -	u16 control;
> > -	u16 status;
> > -	u16 err_cnt;
> > -	u16 btr;
> > -	u16 interrupt;
> > -	u16 test;
> > -	u16 brp_ext;
> > -	u16 _reserved1;
> > -	struct c_can_if_regs ifregs[2]; /* [0] = IF1 and [1] = IF2 */
> > -	u16 _reserved2[8];
> > -	u16 txrqst1;
> > -	u16 txrqst2;
> > -	u16 _reserved3[6];
> > -	u16 newdat1;
> > -	u16 newdat2;
> > -	u16 _reserved4[6];
> > -	u16 intpnd1;
> > -	u16 intpnd2;
> > -	u16 _reserved5[6];
> > -	u16 msgval1;
> > -	u16 msgval2;
> > -	u16 _reserved6[6];
> > +static u16 reg_map_c_can[] = {
> 
> const?

Initially added but removed because we need to change the register offset for
32bit align. I was thinking wrongly, in that context we are not modifying the
array value but offset is multiplying by 2 after we get.

I will change to const.

> 
> > +	[C_CAN_CTRL_REG]	= 0x00,
> > +	[C_CAN_STS_REG]		= 0x02,
> > +	[C_CAN_ERR_CNT_REG]	= 0x04,
> > +	[C_CAN_BTR_REG]		= 0x06,
> > +	[C_CAN_INT_REG]		= 0x08,
> > +	[C_CAN_TEST_REG]	= 0x0A,
> > +	[C_CAN_BRPEXT_REG]	= 0x0C,
> > +	[C_CAN_IF1_COMREQ_REG]	= 0x10,
> > +	[C_CAN_IF1_COMMSK_REG]	= 0x12,
> > +	[C_CAN_IF1_MASK1_REG]	= 0x14,
> > +	[C_CAN_IF1_MASK2_REG]	= 0x16,
> > +	[C_CAN_IF1_ARB1_REG]	= 0x18,
> > +	[C_CAN_IF1_ARB2_REG]	= 0x1A,
> > +	[C_CAN_IF1_MSGCTRL_REG]	= 0x1C,
> > +	[C_CAN_IF1_DATA1_REG]	= 0x1E,
> > +	[C_CAN_IF1_DATA2_REG]	= 0x20,
> > +	[C_CAN_IF1_DATA3_REG]	= 0x22,
> > +	[C_CAN_IF1_DATA4_REG]	= 0x24,
> > +	[C_CAN_TXRQST1_REG]	= 0x80,
> > +	[C_CAN_TXRQST2_REG]	= 0x82,
> > +	[C_CAN_NEWDAT1_REG]	= 0x90,
> > +	[C_CAN_NEWDAT2_REG]	= 0x92,
> > +	[C_CAN_INTPND1_REG]	= 0xA0,
> > +	[C_CAN_INTPND2_REG]	= 0xA2,
> > +	[C_CAN_MSGVAL1_REG]	= 0xB0,
> > +	[C_CAN_MSGVAL2_REG]	= 0xB2,
> >  };
> >  
> >  /* c_can private data structure */
> > @@ -69,9 +88,10 @@ struct c_can_priv {
> >  	int tx_object;
> >  	int current_status;
> >  	int last_status;
> > -	u16 (*read_reg) (struct c_can_priv *priv, void *reg);
> > -	void (*write_reg) (struct c_can_priv *priv, void *reg, u16 val);
> > -	struct c_can_regs __iomem *regs;
> > +	u16 (*read_reg) (struct c_can_priv *priv, int reg);
> > +	void (*write_reg) (struct c_can_priv *priv, int reg, u16 val);
> > +	void __iomem *base;
> > +	u16 *regs;
> >  	unsigned long irq_flags; /* for request_irq() */
> >  	unsigned int tx_next;
> >  	unsigned int tx_echo;
> > diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> > index 5e1a5ff..0cca9db 100644
> > --- a/drivers/net/can/c_can/c_can_platform.c
> > +++ b/drivers/net/can/c_can/c_can_platform.c
> > @@ -32,6 +32,7 @@
> >  #include <linux/clk.h>
> >  
> >  #include <linux/can/dev.h>
> > +#include <linux/can/platform/c_can.h>
> >  
> >  #include "c_can.h"
> >  
> > @@ -42,27 +43,27 @@
> >   * Handle the same by providing a common read/write interface.
> >   */
> >  static u16 c_can_plat_read_reg_aligned_to_16bit(struct c_can_priv *priv,
> > -						void *reg)
> > +						int reg)
> >  {
> > -	return readw(reg);
> > +	return readw(priv->base + priv->regs[reg]);
> >  }
> >  
> >  static void c_can_plat_write_reg_aligned_to_16bit(struct c_can_priv *priv,
> > -						void *reg, u16 val)
> > +						int reg, u16 val)
> >  {
> > -	writew(val, reg);
> > +	writew(val, priv->base + priv->regs[reg]);
> >  }
> >  
> >  static u16 c_can_plat_read_reg_aligned_to_32bit(struct c_can_priv *priv,
> > -						void *reg)
> > +						int reg)
> >  {
> > -	return readw(reg + (long)reg - (long)priv->regs);
> > +	return readw(priv->base + 2 * priv->regs[reg]);
> >  }
> >  
> >  static void c_can_plat_write_reg_aligned_to_32bit(struct c_can_priv *priv,
> > -						void *reg, u16 val)
> > +						int reg, u16 val)
> >  {
> > -	writew(val, reg + (long)reg - (long)priv->regs);
> > +	writew(val, priv->base + 2 * priv->regs[reg]);
> >  }
> >  
> >  static int __devinit c_can_plat_probe(struct platform_device *pdev)
> > @@ -71,6 +72,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >  	void __iomem *addr;
> >  	struct net_device *dev;
> >  	struct c_can_priv *priv;
> > +	struct c_can_platform_data *pdata = pdev->dev.platform_data;
> >  	struct resource *mem;
> >  	int irq;
> >  #ifdef CONFIG_HAVE_CLK
> > @@ -115,9 +117,10 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	priv = netdev_priv(dev);
> > +	priv->regs = reg_map_c_can;
> >  
> >  	dev->irq = irq;
> > -	priv->regs = addr;
> > +	priv->base = addr;
> >  #ifdef CONFIG_HAVE_CLK
> >  	priv->can.clock.freq = clk_get_rate(clk);
> >  	priv->priv = clk;
> > @@ -146,7 +149,7 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> > -		 KBUILD_MODNAME, priv->regs, dev->irq);
> > +		 KBUILD_MODNAME, priv->base, dev->irq);
> >  	return 0;
> >  
> >  exit_free_device:
> > @@ -176,7 +179,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev)
> >  	platform_set_drvdata(pdev, NULL);
> >  
> >  	free_c_can_dev(dev);
> > -	iounmap(priv->regs);
> > +	iounmap(priv->base);
> >  
> >  	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> >  	release_mem_region(mem->start, resource_size(mem));
> 
> Marc
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 
> 

RE: [PATCH v2 4/4] can: c_can: Add support for Bosch D_CAN controller

[AnilKumar, Chimata]

On Fri, May 11, 2012 at 01:04:13, Marc Kleine-Budde wrote:
> On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> > This patch adds the support for D_CAN controller driver to the existing
> > C_CAN driver.
> > 
> > Bosch D_CAN controller is a full-CAN implementation which is compliant
> > to CAN protocol version 2.0 part A and B. Bosch D_CAN user manual can be
> > obtained from: http://www.semiconductors.bosch.de/media/en/pdf/
> > ipmodules_1/can/d_can_users_manual_111.pdf
> > 
> > A new array is added for accessing the d_can registers, according to d_can
> > controller register space.
> > 
> > Current D_CAN implementation has following limitations, this is done
> > to avoid large changes to the C_CAN driver.
> > 1. Message objects are limited to 32, 16 for RX and 16 for TX. C_CAN IP
> >    supports upto 32 message objects but in case of D_CAN we can configure
> >    upto 128 message objects.
> > 2. Using two 16bit reads/writes for accessing the 32bit D_CAN registers.
> > 3. These patches have been tested on little endian machine, there might
> >    be some hidden endian-related issues due to the nature of the accesses
> >    (32-bit registers accessed as 2 16-bit registers). However, I do not
> >    have a big-endian D_CAN implementation to confirm.
> > 
> > Signed-off-by: AnilKumar Ch <anilkumar@ti.com>
> 
> Have a look at the at91_can.c and make use of a platform_device id_table:
> 
> http://lxr.linux.no/linux+v3.3.5/drivers/net/can/at91_can.c#L1364
> 
> This way you can register a "c_can" device or a "d_can" device in your
> arm board file.
> 

Changed

> 
> > ---
> >  drivers/net/can/c_can/c_can.h          |   30 ++++++++++++++++++++++
> >  drivers/net/can/c_can/c_can_platform.c |   31 +++++++++++++----------
> >  include/linux/can/platform/c_can.h     |   42 ++++++++++++++++++++++++++++++++
> >  3 files changed, 90 insertions(+), 13 deletions(-)
> >  create mode 100644 include/linux/can/platform/c_can.h
> > 
> > diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h
> > index 747d478..63a8727 100644
> > --- a/drivers/net/can/c_can/c_can.h
> > +++ b/drivers/net/can/c_can/c_can.h
> > @@ -80,6 +80,35 @@ static u16 reg_map_c_can[] = {
> >  	[C_CAN_MSGVAL2_REG]	= 0xB2,
> >  };
> >  
> > +static u16 reg_map_d_can[] = {
> > +	[C_CAN_CTRL_REG]	= 0x00,
> > +	[C_CAN_STS_REG]		= 0x04,
> > +	[C_CAN_ERR_CNT_REG]	= 0x08,
> > +	[C_CAN_BTR_REG]		= 0x0C,
> > +	[C_CAN_BRPEXT_REG]	= 0x0E,
> > +	[C_CAN_INT_REG]		= 0x10,
> > +	[C_CAN_TEST_REG]	= 0x14,
> > +	[C_CAN_TXRQST1_REG]	= 0x88,
> > +	[C_CAN_TXRQST2_REG]	= 0x8A,
> > +	[C_CAN_NEWDAT1_REG]	= 0x9C,
> > +	[C_CAN_NEWDAT2_REG]	= 0x9E,
> > +	[C_CAN_INTPND1_REG]	= 0xB0,
> > +	[C_CAN_INTPND2_REG]	= 0xB2,
> > +	[C_CAN_MSGVAL1_REG]	= 0xC4,
> > +	[C_CAN_MSGVAL2_REG]	= 0xC6,
> > +	[C_CAN_IF1_COMREQ_REG]	= 0x100,
> > +	[C_CAN_IF1_COMMSK_REG]	= 0x102,
> > +	[C_CAN_IF1_MASK1_REG]	= 0x104,
> > +	[C_CAN_IF1_MASK2_REG]	= 0x106,
> > +	[C_CAN_IF1_ARB1_REG]	= 0x108,
> > +	[C_CAN_IF1_ARB2_REG]	= 0x10A,
> > +	[C_CAN_IF1_MSGCTRL_REG]	= 0x10C,
> > +	[C_CAN_IF1_DATA1_REG]	= 0x110,
> > +	[C_CAN_IF1_DATA2_REG]	= 0x112,
> > +	[C_CAN_IF1_DATA3_REG]	= 0x114,
> > +	[C_CAN_IF1_DATA4_REG]	= 0x116,
> > +};
> > +
> >  /* c_can private data structure */
> >  struct c_can_priv {
> >  	struct can_priv can;	/* must be the first member */
> > @@ -97,6 +126,7 @@ struct c_can_priv {
> >  	unsigned int tx_echo;
> >  	void *priv;		/* for board-specific data */
> >  	u16 irqstatus;
> > +	unsigned int dev_type;
> 
> not needed, please use id_table.

removed

> 
> >  };
> >  
> >  struct net_device *alloc_c_can_dev(void);
> > diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c
> > index 0cca9db..394b05d 100644
> > --- a/drivers/net/can/c_can/c_can_platform.c
> > +++ b/drivers/net/can/c_can/c_can_platform.c
> > @@ -117,7 +117,24 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	priv = netdev_priv(dev);
> > -	priv->regs = reg_map_c_can;
> > +	if (pdata && (pdata->dev_type == DEV_TYPE_D_CAN)) {
> > +		priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES;
> > +		priv->dev_type = DEV_TYPE_D_CAN;
> > +		priv->regs = reg_map_d_can;
> > +		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> > +		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> > +	} else {
> > +		priv->dev_type = DEV_TYPE_C_CAN;
> > +		priv->regs = reg_map_c_can;
> > +		if ((mem->flags & IORESOURCE_MEM_TYPE_MASK) ==
> > +					IORESOURCE_MEM_32BIT) {
> > +			priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
> > +			priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> > +		} else {
> > +			priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> > +			priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> > +		}
> 
> I personally like the switch..case more.

I will change.

> 
> > +	}
> >  
> >  	dev->irq = irq;
> >  	priv->base = addr;
> > @@ -126,18 +143,6 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)
> >  	priv->priv = clk;
> >  #endif
> >  
> > -	switch (mem->flags & IORESOURCE_MEM_TYPE_MASK) {
> > -	case IORESOURCE_MEM_32BIT:
> > -		priv->read_reg = c_can_plat_read_reg_aligned_to_32bit;
> > -		priv->write_reg = c_can_plat_write_reg_aligned_to_32bit;
> > -		break;
> > -	case IORESOURCE_MEM_16BIT:
> > -	default:
> > -		priv->read_reg = c_can_plat_read_reg_aligned_to_16bit;
> > -		priv->write_reg = c_can_plat_write_reg_aligned_to_16bit;
> > -		break;
> > -	}
> > -
> >  	platform_set_drvdata(pdev, dev);
> >  	SET_NETDEV_DEV(dev, &pdev->dev);
> >  
> > diff --git a/include/linux/can/platform/c_can.h b/include/linux/can/platform/c_can.h
> > new file mode 100644
> > index 0000000..e95201a
> > --- /dev/null
> > +++ b/include/linux/can/platform/c_can.h
> > @@ -0,0 +1,42 @@
> > +/*
> > + * C_CAN controller driver platform header
> > + *
> > + * Copyright (C) 2012 Texas Instruments Incorporated - http://www.ti.com/
> > + *
> > + * Bosch C_CAN/D_CAN controller is compliant to CAN protocol version 2.0
> > + * part A and B.
> > + * Bosch C_CAN user manual can be obtained from:
> > + * http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/c_can/
> > + * users_manual_c_can.pdf
> > + * Bosch D_CAN user manual can be obtained from:
> > + * http://www.semiconductors.bosch.de/media/en/pdf/ipmodules_1/can/
> > + * d_can_users_manual_111.pdf
> > + *
> > + * 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.
> > + */
> > +
> > +#ifndef __CAN_PLATFORM_TI_C_CAN_H__
> > +#define __CAN_PLATFORM_TI_C_CAN_H__
> > +
> > +#define DEV_TYPE_C_CAN		0
> > +#define DEV_TYPE_D_CAN		1
> > +
> > +/**
> > + * struct c_can_platform_data - CCAN Platform Data
> > + *
> > + * @dev_type:		Device type C_CAN/D_CAN
> > + *
> > + * Platform data structure to get all platform specific settings.
> > + */
> > +
> > +struct c_can_platform_data {
> > +	unsigned int dev_type;
> > +};
> 
> not needed, use id_table.

Removed.

> 
> > +#endif
> 
> 
> -- 
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |
> 
> 

Re: [PATCH v2 3/4] can: c_can: Move overlay structure to array with offset as index

[Wolfgang Grandegger]

On 05/11/2012 01:09 PM, AnilKumar, Chimata wrote:
> On Fri, May 11, 2012 at 01:42:43, Marc Kleine-Budde wrote:
>> On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
>>> c_can uses overlay structure for accessing c_can module registers.
>>> With this kind of implementation it is difficult to add one more ip
>>> which is similar to c_can in functionality but different register
>>> offsets.
>>>
>>> This patch changes the overlay structure implementation to an array
>>> with register offset as index. This way we can overcome the above
>>> limitation.
>>
>> The array index implementation looks very nice.
>>
>> I suggest to use the enum instead of a plain "int reg" in the
>> c_can_read_* function arguments.
> 
> That is better compared to int reg, I will change.
> 
>>
>> General question: What happend to "iface", like in this hunk below?
> 
> In entire driver iface is used as "0" means IF register bank one. So I defined
> enum like "C_CAN_IF1_*". If IF register bank 2 is used in the driver at that
> time we can define "C_CAN_IF2_*" and used appropriately.

But switching IF1->IF2 would then be awkward. Anyway, at the moment
switching seems not required and therefore there is no need to introduce
another array but...

> 
>>
>>>  	while (count && priv->read_reg(priv,
>>> -				&priv->regs->ifregs[iface].com_req) &
>>> +				C_CAN_IF1_COMREQ_REG) &
>>>  				IF_COMR_BUSY) {

... what is then "iface" still good for? I see that the functions still
have that argument.

Apart from that the patch series look quite good now.

Wolfgang.

RE: [PATCH v2 3/4] can: c_can: Move overlay structure to array with offset as index

[AnilKumar, Chimata]

Hi Wolfgang,

Thanks for the comments,

On Fri, May 11, 2012 at 20:10:48, Wolfgang Grandegger wrote:
> On 05/11/2012 01:09 PM, AnilKumar, Chimata wrote:
> > On Fri, May 11, 2012 at 01:42:43, Marc Kleine-Budde wrote:
> >> On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
> >>> c_can uses overlay structure for accessing c_can module registers.
> >>> With this kind of implementation it is difficult to add one more ip
> >>> which is similar to c_can in functionality but different register
> >>> offsets.
> >>>
> >>> This patch changes the overlay structure implementation to an array
> >>> with register offset as index. This way we can overcome the above
> >>> limitation.
> >>
> >> The array index implementation looks very nice.
> >>
> >> I suggest to use the enum instead of a plain "int reg" in the
> >> c_can_read_* function arguments.
> > 
> > That is better compared to int reg, I will change.
> > 
> >>
> >> General question: What happend to "iface", like in this hunk below?
> > 
> > In entire driver iface is used as "0" means IF register bank one. So I defined
> > enum like "C_CAN_IF1_*". If IF register bank 2 is used in the driver at that
> > time we can define "C_CAN_IF2_*" and used appropriately.
> 
> But switching IF1->IF2 would then be awkward. Anyway, at the moment
> switching seems not required and therefore there is no need to introduce
> another array but...
> 
> > 
> >>
> >>>  	while (count && priv->read_reg(priv,
> >>> -				&priv->regs->ifregs[iface].com_req) &
> >>> +				C_CAN_IF1_COMREQ_REG) &
> >>>  				IF_COMR_BUSY) {
> 
> ... what is then "iface" still good for? I see that the functions still
> have that argument.
> 
> Apart from that the patch series look quite good now.

How about this implementation?

1. C_CAN_IF2_* will be added to enum next to C_CAN_IF1_* register flags

2. Add array fields for C_CAN_IF2_* with coresponding offsets.

3. #define IF_ENUM_REG_LEN	11 (eleven interface registers are present)

4. priv->read_reg(priv, C_CAN_IF1_COMREQ_REG + iface * IF_ENUM_REG_LEN);

Ultimately 11 * 3 + 1 + few more lines will be added to the patch.

Regards
AnilKumar

Re: [PATCH v2 3/4] can: c_can: Move overlay structure to array with offset as index

[Marc Kleine-Budde]

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

On 05/11/2012 05:23 PM, AnilKumar, Chimata wrote:
> On Fri, May 11, 2012 at 20:10:48, Wolfgang Grandegger wrote:
>> On 05/11/2012 01:09 PM, AnilKumar, Chimata wrote:
>>> On Fri, May 11, 2012 at 01:42:43, Marc Kleine-Budde wrote:
>>>> On 05/10/2012 01:34 PM, AnilKumar Ch wrote:
>>>>> c_can uses overlay structure for accessing c_can module registers.
>>>>> With this kind of implementation it is difficult to add one more ip
>>>>> which is similar to c_can in functionality but different register
>>>>> offsets.
>>>>>
>>>>> This patch changes the overlay structure implementation to an array
>>>>> with register offset as index. This way we can overcome the above
>>>>> limitation.
>>>>
>>>> The array index implementation looks very nice.
>>>>
>>>> I suggest to use the enum instead of a plain "int reg" in the
>>>> c_can_read_* function arguments.
>>>
>>> That is better compared to int reg, I will change.
>>>
>>>>
>>>> General question: What happend to "iface", like in this hunk below?
>>>
>>> In entire driver iface is used as "0" means IF register bank one. So I defined
>>> enum like "C_CAN_IF1_*". If IF register bank 2 is used in the driver at that
>>> time we can define "C_CAN_IF2_*" and used appropriately.
>>
>> But switching IF1->IF2 would then be awkward. Anyway, at the moment
>> switching seems not required and therefore there is no need to introduce
>> another array but...
>>
>>>
>>>>
>>>>>  	while (count && priv->read_reg(priv,
>>>>> -				&priv->regs->ifregs[iface].com_req) &
>>>>> +				C_CAN_IF1_COMREQ_REG) &
>>>>>  				IF_COMR_BUSY) {
>>
>> ... what is then "iface" still good for? I see that the functions still
>> have that argument.
>>
>> Apart from that the patch series look quite good now.
> 
> How about this implementation?
> 
> 1. C_CAN_IF2_* will be added to enum next to C_CAN_IF1_* register flags
> 
> 2. Add array fields for C_CAN_IF2_* with coresponding offsets.
> 
> 3. #define IF_ENUM_REG_LEN	11 (eleven interface registers are present)
> 
> 4. priv->read_reg(priv, C_CAN_IF1_COMREQ_REG + iface * IF_ENUM_REG_LEN);
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

make this a MACRO, taking iface as an argument

> Ultimately 11 * 3 + 1 + few more lines will be added to the patch.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]