Re: [PATCH v4 2/2] can: m_can: add Bosch M_CAN controller support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Mon, Jul 14, 2014 at 06:17:17PM +0530, Varka Bhadram wrote:
> On 07/14/2014 05:10 PM, Dong Aisheng wrote:
> 
> (...)
> 
> >diff --git a/drivers/net/can/m_can/Makefile b/drivers/net/can/m_can/Makefile
> >new file mode 100644
> >index 0000000..a6aae67
> >--- /dev/null
> >+++ b/drivers/net/can/m_can/Makefile
> >@@ -0,0 +1,7 @@
> >+#
> >+#  Makefile for the Bosch M_CAN controller drivers.
> >+#
> >+
> >+obj-$(CONFIG_CAN_M_CAN) += m_can.o
> >+
> >+ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> 
> I think this will enable the debugging for all the time ?????
> 
> >diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> >new file mode 100644
> >index 0000000..8e48213
> >--- /dev/null
> >+++ b/drivers/net/can/m_can/m_can.c
> >@@ -0,0 +1,1189 @@
> >+/*
> >+ * CAN bus driver for Bosch M_CAN controller
> >+ *
> >+ * Copyright (C) 2014 Freescale Semiconductor, Inc.
> >+ *	Dong Aisheng <b29396@xxxxxxxxxxxxx>
> >+ *
> >+ * Bosch M_CAN user manual can be obtained from:
> >+ * http://www.bosch-semiconductors.de/media/pdf_1/ipmodules_1/m_can/
> >+ * mcan_users_manual_v302.pdf
> >+ *
> >+ * This file is licensed under the terms of the GNU General Public
> >+ * License version 2. This program is licensed "as is" without any
> >+ * warranty of any kind, whether express or implied.
> >+ */
> >+
> >+#include <linux/clk.h>
> >+#include <linux/delay.h>
> >+#include <linux/interrupt.h>
> >+#include <linux/io.h>
> >+#include <linux/kernel.h>
> >+#include <linux/module.h>
> >+#include <linux/netdevice.h>
> >+#include <linux/of.h>
> >+#include <linux/of_device.h>
> >+#include <linux/platform_device.h>
> >+
> >+#include <linux/can/dev.h>
> >+
> >+/* napi related */
> >+#define M_CAN_NAPI_WEIGHT	64
> >+
> >+/* message ram configuration data length */
> >+#define MRAM_CFG_LEN	8
> >+
> >+/* registers definition */
> >+enum m_can_reg {
> >+	M_CAN_CREL	= 0x0,
> >+	M_CAN_ENDN	= 0x4,
> >+	M_CAN_CUST	= 0x8,
> >+	M_CAN_FBTP	= 0xc,
> >+	M_CAN_TEST	= 0x10,
> >+	M_CAN_RWD	= 0x14,
> >+	M_CAN_CCCR	= 0x18,
> >+	M_CAN_BTP	= 0x1c,
> >+	M_CAN_TSCC	= 0x20,
> >+	M_CAN_TSCV	= 0x24,
> >+	M_CAN_TOCC	= 0x28,
> >+	M_CAN_TOCV	= 0x2c,
> >+	M_CAN_ECR	= 0x40,
> >+	M_CAN_PSR	= 0x44,
> >+	M_CAN_IR	= 0x50,
> >+	M_CAN_IE	= 0x54,
> >+	M_CAN_ILS	= 0x58,
> >+	M_CAN_ILE	= 0x5c,
> >+	M_CAN_GFC	= 0x80,
> >+	M_CAN_SIDFC	= 0x84,
> >+	M_CAN_XIDFC	= 0x88,
> >+	M_CAN_XIDAM	= 0x90,
> >+	M_CAN_HPMS	= 0x94,
> >+	M_CAN_NDAT1	= 0x98,
> >+	M_CAN_NDAT2	= 0x9c,
> >+	M_CAN_RXF0C	= 0xa0,
> >+	M_CAN_RXF0S	= 0xa4,
> >+	M_CAN_RXF0A	= 0xa8,
> >+	M_CAN_RXBC	= 0xac,
> >+	M_CAN_RXF1C	= 0xb0,
> >+	M_CAN_RXF1S	= 0xb4,
> >+	M_CAN_RXF1A	= 0xb8,
> >+	M_CAN_RXESC	= 0xbc,
> >+	M_CAN_TXBC	= 0xc0,
> >+	M_CAN_TXFQS	= 0xc4,
> >+	M_CAN_TXESC	= 0xc8,
> >+	M_CAN_TXBRP	= 0xcc,
> >+	M_CAN_TXBAR	= 0xd0,
> >+	M_CAN_TXBCR	= 0xd4,
> >+	M_CAN_TXBTO	= 0xd8,
> >+	M_CAN_TXBCF	= 0xdc,
> >+	M_CAN_TXBTIE	= 0xe0,
> >+	M_CAN_TXBCIE	= 0xe4,
> >+	M_CAN_TXEFC	= 0xf0,
> >+	M_CAN_TXEFS	= 0xf4,
> >+	M_CAN_TXEFA	= 0xf8,
> >+};
> >+
> >+/* m_can lec values */
> >+enum m_can_lec_type {
> >+	LEC_NO_ERROR = 0,
> >+	LEC_STUFF_ERROR,
> >+	LEC_FORM_ERROR,
> >+	LEC_ACK_ERROR,
> >+	LEC_BIT1_ERROR,
> >+	LEC_BIT0_ERROR,
> >+	LEC_CRC_ERROR,
> >+	LEC_UNUSED,
> >+};
> >+
> >+enum m_can_mram_cfg {
> >+	MRAM_SIDF = 0,
> >+	MRAM_XIDF,
> >+	MRAM_RXF0,
> >+	MRAM_RXF1,
> >+	MRAM_RXB,
> >+	MRAM_TXE,
> >+	MRAM_TXB,
> >+	MRAM_CFG_NUM,
> >+};
> >+
> >+/* Test Register (TEST) */
> >+#define TEST_LBCK	BIT(4)
> >+
> >+/* CC Control Register(CCCR) */
> >+#define CCCR_TEST	BIT(7)
> >+#define CCCR_MON	BIT(5)
> >+#define CCCR_CCE	BIT(1)
> >+#define CCCR_INIT	BIT(0)
> >+
> >+/* Bit Timing & Prescaler Register (BTP) */
> >+#define BTR_BRP_MASK		0x3ff
> >+#define BTR_BRP_SHIFT		16
> >+#define BTR_TSEG1_SHIFT		8
> >+#define BTR_TSEG1_MASK		(0x3f << BTR_TSEG1_SHIFT)
> >+#define BTR_TSEG2_SHIFT		4
> >+#define BTR_TSEG2_MASK		(0xf << BTR_TSEG2_SHIFT)
> >+#define BTR_SJW_SHIFT		0
> >+#define BTR_SJW_MASK		0xf
> >+
> >+/* Error Counter Register(ECR) */
> >+#define ECR_RP			BIT(15)
> >+#define ECR_REC_SHIFT		8
> >+#define ECR_REC_MASK		(0x7f << ECR_REC_SHIFT)
> >+#define ECR_TEC_SHIFT		0
> >+#define ECR_TEC_MASK		0xff
> >+
> >+/* Protocol Status Register(PSR) */
> >+#define PSR_BO		BIT(7)
> >+#define PSR_EW		BIT(6)
> >+#define PSR_EP		BIT(5)
> >+#define PSR_LEC_MASK	0x7
> >+
> >+/* Interrupt Register(IR) */
> >+#define IR_ALL_INT	0xffffffff
> >+#define IR_STE		BIT(31)
> >+#define IR_FOE		BIT(30)
> >+#define IR_ACKE		BIT(29)
> >+#define IR_BE		BIT(28)
> >+#define IR_CRCE		BIT(27)
> >+#define IR_WDI		BIT(26)
> >+#define IR_BO		BIT(25)
> >+#define IR_EW		BIT(24)
> >+#define IR_EP		BIT(23)
> >+#define IR_ELO		BIT(22)
> >+#define IR_BEU		BIT(21)
> >+#define IR_BEC		BIT(20)
> >+#define IR_DRX		BIT(19)
> >+#define IR_TOO		BIT(18)
> >+#define IR_MRAF		BIT(17)
> >+#define IR_TSW		BIT(16)
> >+#define IR_TEFL		BIT(15)
> >+#define IR_TEFF		BIT(14)
> >+#define IR_TEFW		BIT(13)
> >+#define IR_TEFN		BIT(12)
> >+#define IR_TFE		BIT(11)
> >+#define IR_TCF		BIT(10)
> >+#define IR_TC		BIT(9)
> >+#define IR_HPM		BIT(8)
> >+#define IR_RF1L		BIT(7)
> >+#define IR_RF1F		BIT(6)
> >+#define IR_RF1W		BIT(5)
> >+#define IR_RF1N		BIT(4)
> >+#define IR_RF0L		BIT(3)
> >+#define IR_RF0F		BIT(2)
> >+#define IR_RF0W		BIT(1)
> >+#define IR_RF0N		BIT(0)
> >+#define IR_ERR_STATE	(IR_BO | IR_EW | IR_EP)
> >+#define IR_ERR_LEC	(IR_STE	| IR_FOE | IR_ACKE | IR_BE | IR_CRCE)
> >+#define IR_ERR_BUS	(IR_ERR_LEC | IR_WDI | IR_ELO | IR_BEU | IR_BEC \
> >+			| IR_TOO | IR_MRAF | IR_TSW | IR_TEFL | IR_RF1L \
> >+			| IR_RF0L)
> >+#define IR_ERR_ALL	(IR_ERR_STATE | IR_ERR_BUS)
> >+
> >+/* Interrupt Line Select (ILS) */
> >+#define ILS_ALL_INT0	0x0
> >+#define ILS_ALL_INT1	0xFFFFFFFF
> >+
> >+/* Interrupt Line Enable (ILE) */
> >+#define ILE_EINT0	BIT(0)
> >+#define ILE_EINT1	BIT(1)
> >+
> >+/* Rx FIFO 0/1 Configuration (RXF0C/RXF1C) */
> >+#define RXFC_FWM_OFF	24
> >+#define RXFC_FWM_MASK	0x7f
> >+#define RXFC_FWM_1	(1 << RXFC_FWM_OFF)
> >+#define RXFC_FS_OFF	16
> >+#define RXFC_FS_MASK	0x7f
> >+
> >+/* Rx FIFO 0/1 Status (RXF0S/RXF1S) */
> >+#define RXFS_RFL	BIT(25)
> >+#define RXFS_FF		BIT(24)
> >+#define RXFS_FPI_OFF	16
> >+#define RXFS_FPI_MASK	0x3f0000
> >+#define RXFS_FGI_OFF	8
> >+#define RXFS_FGI_MASK	0x3f00
> >+#define RXFS_FFL_MASK	0x7f
> >+
> >+/* Tx Buffer Configuration(TXBC) */
> >+#define TXBC_NDTB_OFF	16
> >+#define TXBC_NDTB_MASK	0x3f
> >+
> >+/* Tx Buffer Element Size Configuration(TXESC) */
> >+#define TXESC_TBDS_8BYTES	0x0
> >+/* Tx Buffer Element */
> >+#define TX_BUF_XTD	BIT(30)
> >+#define TX_BUF_RTR	BIT(29)
> >+
> >+/* Rx Buffer Element Size Configuration(TXESC) */
> >+#define M_CAN_RXESC_8BYTES	0x0
> >+/* Tx Buffer Element */
> >+#define RX_BUF_ESI	BIT(31)
> >+#define RX_BUF_XTD	BIT(30)
> >+#define RX_BUF_RTR	BIT(29)
> >+
> >+/* Tx Event FIFO Con.guration (TXEFC) */
> >+#define TXEFC_EFS_OFF	16
> >+#define TXEFC_EFS_MASK	0x3f
> >+
> >+/* Message RAM Configuration (in bytes) */
> >+#define SIDF_ELEMENT_SIZE	4
> >+#define XIDF_ELEMENT_SIZE	8
> >+#define RXF0_ELEMENT_SIZE	16
> >+#define RXF1_ELEMENT_SIZE	16
> >+#define RXB_ELEMENT_SIZE	16
> >+#define TXE_ELEMENT_SIZE	8
> >+#define TXB_ELEMENT_SIZE	16
> 
> Alignment for all the includes
> 

What do you mean?

> >+
> >+/* address offset and element number for each FIFO/Buffer in the Message RAM */
> >+struct mram_cfg {
> >+	u16 off;
> >+	u8  num;
> >+};
> >+
> >+/* m_can private data structure */
> >+struct m_can_priv {
> >+	struct can_priv can;	/* must be the first member */
> >+	struct napi_struct napi;
> >+	struct net_device *dev;
> >+	struct device *device;
> >+	struct clk *hclk;
> >+	struct clk *cclk;
> >+	void __iomem *base;
> >+	u32 irqstatus;
> >+
> >+	/* message ram configuration */
> >+	void __iomem *mram_base;
> >+	struct mram_cfg mcfg[MRAM_CFG_NUM];
> >+};
> >+
> >+static inline u32 m_can_read(const struct m_can_priv *priv, enum m_can_reg reg)
> >+{
> >+	return readl(priv->base + reg);
> >+}
> >+
> >+static inline void m_can_write(const struct m_can_priv *priv,
> >+			       enum m_can_reg reg, u32 val)
> >+{
> >+	writel(val, priv->base + reg);
> >+}
> >+
> >+static inline u32 m_can_fifo_read(const struct m_can_priv *priv,
> >+				  u32 fgi, unsigned int offset)
> >+{
> >+	return readl(priv->mram_base + priv->mcfg[MRAM_RXF0].off +
> >+				fgi * RXF0_ELEMENT_SIZE + offset);
> 
> Here alignment should match the open parenthesis.
> ...
> return readl(priv->mram_base + priv->mcfg[MRAM_RXF0].off +
> 	     fgi * RXF0_ELEMENT_SIZE + offset);
> 

I've fixed all checkpatch warnings.
I don't know why this line is not reported, as well as some other lines
you pointed out.

Do you know the reason?

> >+}
> >+
> >+static inline void m_can_config_endisable(const struct m_can_priv *priv,
> >+					  bool enable)
> >+{
> >+	u32 cccr = m_can_read(priv, M_CAN_CCCR);
> >+	u32 timeout = 10;
> >+	u32 val = 0;
> >+
> >+	if (enable) {
> >+		/* enable m_can configuration */
> >+		m_can_write(priv, M_CAN_CCCR, cccr | CCCR_INIT);
> >+		/* CCCR.CCE can only be set/reset while CCCR.INIT = '1' */
> >+		m_can_write(priv, M_CAN_CCCR, cccr | CCCR_INIT | CCCR_CCE);
> >+	} else {
> >+		m_can_write(priv, M_CAN_CCCR, cccr & ~(CCCR_INIT | CCCR_CCE));
> >+	}
> >+
> >+	/* there's a delay for module initialization */
> >+	if (enable)
> >+		val = CCCR_INIT | CCCR_CCE;
> >+
> >+	while ((m_can_read(priv, M_CAN_CCCR) & (CCCR_INIT | CCCR_CCE))
> >+				!= val)
> 
> Ditto..
> 
> >  {
> >+		if (timeout == 0) {
> >+			netdev_warn(priv->dev, "Failed to init module\n");
> >+			return;
> >+		}
> >+		timeout--;
> >+		udelay(1);
> >+	}
> >+}
> >+
> >+static inline void m_can_enable_all_interrupts(const struct m_can_priv *priv)
> >+{
> >+	m_can_write(priv, M_CAN_ILE, ILE_EINT0 | ILE_EINT1);
> >+}
> >+
> >+static inline void m_can_disable_all_interrupts(const struct m_can_priv *priv)
> >+{
> >+	m_can_write(priv, M_CAN_ILE, 0x0);
> >+}
> >+
> >+static void m_can_read_fifo(const struct net_device *dev, struct can_frame *cf,
> >+			    u32 rxfs)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	u32 flags, fgi;
> >+
> >+	/* calculate the fifo get index for where to read data */
> >+	fgi = (rxfs & RXFS_FGI_MASK) >> RXFS_FGI_OFF;
> >+	flags = m_can_fifo_read(priv, fgi, 0x0);
> >+	if (flags & RX_BUF_XTD)
> >+		cf->can_id = (flags & CAN_EFF_MASK) | CAN_EFF_FLAG;
> >+	else
> >+		cf->can_id = (flags >> 18) & CAN_SFF_MASK;
> >+
> >+	if (flags & RX_BUF_RTR) {
> >+		cf->can_id |= CAN_RTR_FLAG;
> >+	} else {
> >+		flags = m_can_fifo_read(priv, fgi, 0x4);
> >+		cf->can_dlc = get_can_dlc((flags >> 16) & 0x0F);
> >+		*(u32 *)(cf->data + 0) = m_can_fifo_read(priv, fgi, 0x8);
> >+		*(u32 *)(cf->data + 4) = m_can_fifo_read(priv, fgi, 0xC);
> >+	}
> >+
> >+	/* acknowledge rx fifo 0 */
> >+	m_can_write(priv, M_CAN_RXF0A, fgi);
> >+}
> >+
> >+static int m_can_do_rx_poll(struct net_device *dev, int quota)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	struct net_device_stats *stats = &dev->stats;
> >+	struct sk_buff *skb;
> >+	struct can_frame *frame;
> >+	u32 pkts = 0;
> >+	u32 rxfs;
> >+
> >+	rxfs = m_can_read(priv, M_CAN_RXF0S);
> >+	if (!(rxfs & RXFS_FFL_MASK)) {
> >+		netdev_dbg(dev, "no messages in fifo0\n");
> >+		return 0;
> >+	}
> >+
> >+	while ((rxfs & RXFS_FFL_MASK) && (quota > 0)) {
> >+		if (rxfs & RXFS_RFL)
> >+			netdev_warn(dev, "Rx FIFO 0 Message Lost\n");
> >+
> >+		skb = alloc_can_skb(dev, &frame);
> >+		if (!skb) {
> >+			stats->rx_dropped++;
> >+			return 0;
> >+		}
> >+
> >+		m_can_read_fifo(dev, frame, rxfs);
> >+
> >+		stats->rx_packets++;
> >+		stats->rx_bytes += frame->can_dlc;
> >+
> >+		netif_receive_skb(skb);
> >+
> >+		quota--;
> >+		pkts++;
> >+		rxfs = m_can_read(priv, M_CAN_RXF0S);
> >+	};
> >+
> >+	if (pkts)
> >+		can_led_event(dev, CAN_LED_EVENT_RX);
> >+
> >+	return pkts;
> >+}
> >+
> >+static int m_can_handle_lost_msg(struct net_device *dev)
> >+{
> >+	struct net_device_stats *stats = &dev->stats;
> >+	struct sk_buff *skb;
> >+	struct can_frame *frame;
> >+
> >+	netdev_err(dev, "msg lost in rxf0\n");
> >+
> >+	stats->rx_errors++;
> >+	stats->rx_over_errors++;
> >+
> >+	skb = alloc_can_err_skb(dev, &frame);
> >+	if (unlikely(!skb))
> >+		return 0;
> >+
> >+	frame->can_id |= CAN_ERR_CRTL;
> >+	frame->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> >+
> >+	netif_receive_skb(skb);
> >+
> >+	return 1;
> >+}
> >+
> >+static int m_can_handle_lec_err(struct net_device *dev,
> >+				enum m_can_lec_type lec_type)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	struct net_device_stats *stats = &dev->stats;
> >+	struct can_frame *cf;
> >+	struct sk_buff *skb;
> >+
> >+	/* early exit if no lec update */
> >+	if (lec_type == LEC_UNUSED)
> >+		return 0;
> >+
> >+	if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> >+		return 0;
> >+
> >+	priv->can.can_stats.bus_error++;
> >+	stats->rx_errors++;
> >+
> >+	/* propagate the error condition to the CAN stack */
> >+	skb = alloc_can_err_skb(dev, &cf);
> >+	if (unlikely(!skb))
> >+		return 0;
> >+
> >+	/* check for 'last error code' which tells us the
> >+	 * type of the last error to occur on the CAN bus
> >+	 */
> >+	cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> >+	cf->data[2] |= CAN_ERR_PROT_UNSPEC;
> >+
> >+	switch (lec_type) {
> >+	case LEC_STUFF_ERROR:
> >+		netdev_dbg(dev, "stuff error\n");
> >+		cf->data[2] |= CAN_ERR_PROT_STUFF;
> >+		break;
> >+	case LEC_FORM_ERROR:
> >+		netdev_dbg(dev, "form error\n");
> >+		cf->data[2] |= CAN_ERR_PROT_FORM;
> >+		break;
> >+	case LEC_ACK_ERROR:
> >+		netdev_dbg(dev, "ack error\n");
> >+		cf->data[3] |= (CAN_ERR_PROT_LOC_ACK |
> >+				CAN_ERR_PROT_LOC_ACK_DEL);
> >+		break;
> >+	case LEC_BIT1_ERROR:
> >+		netdev_dbg(dev, "bit1 error\n");
> >+		cf->data[2] |= CAN_ERR_PROT_BIT1;
> >+		break;
> >+	case LEC_BIT0_ERROR:
> >+		netdev_dbg(dev, "bit0 error\n");
> >+		cf->data[2] |= CAN_ERR_PROT_BIT0;
> >+		break;
> >+	case LEC_CRC_ERROR:
> >+		netdev_dbg(dev, "CRC error\n");
> >+		cf->data[3] |= (CAN_ERR_PROT_LOC_CRC_SEQ |
> >+				CAN_ERR_PROT_LOC_CRC_DEL);
> >+		break;
> >+	default:
> >+		break;
> >+	}
> >+
> >+	stats->rx_packets++;
> >+	stats->rx_bytes += cf->can_dlc;
> >+	netif_receive_skb(skb);
> >+
> >+	return 1;
> >+}
> >+
> >+static int m_can_get_berr_counter(const struct net_device *dev,
> >+				  struct can_berr_counter *bec)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	unsigned int ecr;
> >+	int err;
> >+
> >+	err = clk_prepare_enable(priv->hclk);
> >+	if (err)
> >+		return err;
> >+
> >+	err = clk_prepare_enable(priv->cclk);
> >+	if (err) {
> >+		clk_disable_unprepare(priv->hclk);
> >+		return err;
> >+	}
> >+
> >+	ecr = m_can_read(priv, M_CAN_ECR);
> >+	bec->rxerr = (ecr & ECR_REC_MASK) >> ECR_REC_SHIFT;
> >+	bec->txerr = ecr & ECR_TEC_MASK;
> >+
> >+	clk_disable_unprepare(priv->cclk);
> >+	clk_disable_unprepare(priv->hclk);
> >+
> >+	return 0;
> >+}
> >+
> >+static int m_can_handle_state_change(struct net_device *dev,
> >+				     enum can_state new_state)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	struct net_device_stats *stats = &dev->stats;
> >+	struct can_frame *cf;
> >+	struct sk_buff *skb;
> >+	struct can_berr_counter bec;
> >+	unsigned int ecr;
> >+
> >+	switch (new_state) {
> >+	case CAN_STATE_ERROR_ACTIVE:
> >+		/* error warning state */
> >+		priv->can.can_stats.error_warning++;
> >+		priv->can.state = CAN_STATE_ERROR_WARNING;
> >+		break;
> >+	case CAN_STATE_ERROR_PASSIVE:
> >+		/* error passive state */
> >+		priv->can.can_stats.error_passive++;
> >+		priv->can.state = CAN_STATE_ERROR_PASSIVE;
> >+		break;
> >+	case CAN_STATE_BUS_OFF:
> >+		/* bus-off state */
> >+		priv->can.state = CAN_STATE_BUS_OFF;
> >+		m_can_disable_all_interrupts(priv);
> >+		can_bus_off(dev);
> >+		break;
> >+	default:
> >+		break;
> >+	}
> >+
> >+	/* propagate the error condition to the CAN stack */
> >+	skb = alloc_can_err_skb(dev, &cf);
> >+	if (unlikely(!skb))
> >+		return 0;
> >+
> >+	m_can_get_berr_counter(dev, &bec);
> >+
> >+	switch (new_state) {
> >+	case CAN_STATE_ERROR_ACTIVE:
> >+		/* error warning state */
> >+		cf->can_id |= CAN_ERR_CRTL;
> >+		cf->data[1] = (bec.txerr > bec.rxerr) ?
> >+			CAN_ERR_CRTL_TX_WARNING :
> >+			CAN_ERR_CRTL_RX_WARNING;
> >+		cf->data[6] = bec.txerr;
> >+		cf->data[7] = bec.rxerr;
> >+		break;
> >+	case CAN_STATE_ERROR_PASSIVE:
> >+		/* error passive state */
> >+		cf->can_id |= CAN_ERR_CRTL;
> >+		ecr = m_can_read(priv, M_CAN_ECR);
> >+		if (ecr & ECR_RP)
> >+			cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE;
> >+		if (bec.txerr > 127)
> >+			cf->data[1] |= CAN_ERR_CRTL_TX_PASSIVE;
> >+		cf->data[6] = bec.txerr;
> >+		cf->data[7] = bec.rxerr;
> >+		break;
> >+	case CAN_STATE_BUS_OFF:
> >+		/* bus-off state */
> >+		cf->can_id |= CAN_ERR_BUSOFF;
> >+		break;
> >+	default:
> >+		break;
> >+	}
> >+
> >+	stats->rx_packets++;
> >+	stats->rx_bytes += cf->can_dlc;
> >+	netif_receive_skb(skb);
> >+
> >+	return 1;
> >+}
> >+
> >+static int m_can_handle_state_errors(struct net_device *dev, u32 psr)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	int work_done = 0;
> >+
> >+	if ((psr & PSR_EW) &&
> >+	    (priv->can.state != CAN_STATE_ERROR_WARNING)) {
> >+		netdev_dbg(dev, "entered error warning state\n");
> >+		work_done += m_can_handle_state_change(dev,
> >+				CAN_STATE_ERROR_WARNING);
> 
> Ditto
> 
> >+	}
> >+
> >+	if ((psr & PSR_EP) &&
> >+	    (priv->can.state != CAN_STATE_ERROR_PASSIVE)) {
> >+		netdev_dbg(dev, "entered error warning state\n");
> >+		work_done += m_can_handle_state_change(dev,
> >+				CAN_STATE_ERROR_PASSIVE);
> >+	}
> >+
> 
> Ditto
> 
> >+	if ((psr & PSR_BO) &&
> >+	    (priv->can.state != CAN_STATE_BUS_OFF)) {
> >+		netdev_dbg(dev, "entered error warning state\n");
> >+		work_done += m_can_handle_state_change(dev,
> >+				CAN_STATE_BUS_OFF);
> 
> Ditto
> 
> >+	}
> >+
> >+	return work_done;
> >+}
> >+
> >+static void m_can_handle_other_err(struct net_device *dev, u32 irqstatus)
> >+{
> >+	if (irqstatus & IR_WDI)
> >+		netdev_err(dev, "Message RAM Watchdog event due to missing READY\n");
> >+	if (irqstatus & IR_BEU)
> >+		netdev_err(dev, "Error Logging Overflow\n");
> >+	if (irqstatus & IR_BEU)
> >+		netdev_err(dev, "Bit Error Uncorrected\n");
> >+	if (irqstatus & IR_BEC)
> >+		netdev_err(dev, "Bit Error Corrected\n");
> >+	if (irqstatus & IR_TOO)
> >+		netdev_err(dev, "Timeout reached\n");
> >+	if (irqstatus & IR_MRAF)
> >+		netdev_err(dev, "Message RAM access failure occurred\n");
> >+}
> >+
> >+static int m_can_handle_bus_errors(struct net_device *dev, u32 irqstatus,
> >+				   u32 psr)
> >+{
> >+	int work_done = 0;
> >+
> >+	if (irqstatus & IR_RF0L)
> >+		work_done += m_can_handle_lost_msg(dev);
> >+
> >+	/* handle lec errors on the bus */
> >+	if (psr & LEC_UNUSED)
> >+		work_done += m_can_handle_lec_err(dev,
> >+				psr & LEC_UNUSED);
> 
> Ditto
> 

Checkpatch not report them....

> >+
> >+	/* other unproccessed error interrupts */
> >+	m_can_handle_other_err(dev, irqstatus);
> >+
> >+	return work_done;
> >+}
> >+
> >+static int m_can_poll(struct napi_struct *napi, int quota)
> >+{
> >+	struct net_device *dev = napi->dev;
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	int work_done = 0;
> >+	u32 irqstatus, psr;
> >+
> >+	irqstatus = priv->irqstatus | m_can_read(priv, M_CAN_IR);
> >+	if (!irqstatus)
> >+		goto end;
> >+
> >+	psr = m_can_read(priv, M_CAN_PSR);
> >+	if (irqstatus & IR_ERR_STATE)
> >+		work_done += m_can_handle_state_errors(dev, psr);
> >+
> >+	if (irqstatus & IR_ERR_BUS)
> >+		work_done += m_can_handle_bus_errors(dev, irqstatus, psr);
> >+
> >+	if (irqstatus & IR_RF0N)
> >+		work_done += m_can_do_rx_poll(dev, (quota - work_done));
> >+
> >+	if (work_done < quota) {
> >+		napi_complete(napi);
> >+		m_can_enable_all_interrupts(priv);
> >+	}
> >+
> >+end:
> >+	return work_done;
> >+}
> >+
> >+static irqreturn_t m_can_isr(int irq, void *dev_id)
> >+{
> >+	struct net_device *dev = (struct net_device *)dev_id;
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	struct net_device_stats *stats = &dev->stats;
> >+	u32 ir;
> >+
> >+	ir = m_can_read(priv, M_CAN_IR);
> >+	if (!ir)
> >+		return IRQ_NONE;
> >+
> >+	/* ACK all irqs */
> >+	if (ir & IR_ALL_INT)
> >+		m_can_write(priv, M_CAN_IR, ir);
> >+
> >+	/* schedule NAPI in case of
> >+	 * - rx IRQ
> >+	 * - state change IRQ
> >+	 * - bus error IRQ and bus error reporting
> >+	 */
> >+	if ((ir & IR_RF0N) || (ir & IR_ERR_ALL)) {
> >+		priv->irqstatus = ir;
> >+		m_can_disable_all_interrupts(priv);
> >+		napi_schedule(&priv->napi);
> >+	}
> >+
> >+	/* transmission complete interrupt */
> >+	if (ir & IR_TC) {
> >+		stats->tx_bytes += can_get_echo_skb(dev, 0);
> >+		stats->tx_packets++;
> >+		can_led_event(dev, CAN_LED_EVENT_TX);
> >+		netif_wake_queue(dev);
> >+	}
> >+
> >+	return IRQ_HANDLED;
> >+}
> >+
> >+static const struct can_bittiming_const m_can_bittiming_const = {
> >+	.name = KBUILD_MODNAME,
> >+	.tseg1_min = 2,		/* Time segment 1 = prop_seg + phase_seg1 */
> >+	.tseg1_max = 64,
> >+	.tseg2_min = 1,		/* Time segment 2 = phase_seg2 */
> >+	.tseg2_max = 16,
> >+	.sjw_max = 16,
> >+	.brp_min = 1,
> >+	.brp_max = 1024,
> >+	.brp_inc = 1,
> >+};
> >+
> >+static int m_can_set_bittiming(struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	const struct can_bittiming *bt = &priv->can.bittiming;
> >+	u16 brp, sjw, tseg1, tseg2;
> >+	u32 reg_btp;
> >+
> >+	brp = bt->brp - 1;
> >+	sjw = bt->sjw - 1;
> >+	tseg1 = bt->prop_seg + bt->phase_seg1 - 1;
> >+	tseg2 = bt->phase_seg2 - 1;
> >+	reg_btp = (brp << BTR_BRP_SHIFT) | (sjw << BTR_SJW_SHIFT) |
> >+			(tseg1 << BTR_TSEG1_SHIFT) | (tseg2 << BTR_TSEG2_SHIFT);
> >+	m_can_write(priv, M_CAN_BTP, reg_btp);
> >+	netdev_dbg(dev, "setting BTP 0x%x\n", reg_btp);
> >+
> >+	return 0;
> >+}
> >+
> >+/* Configure M_CAN chip:
> >+ * - set rx buffer/fifo element size
> >+ * - configure rx fifo
> >+ * - accept non-matching frame into fifo 0
> >+ * - configure tx buffer
> >+ * - configure mode
> >+ * - setup bittiming
> >+ */
> >+static void m_can_chip_config(struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	u32 cccr, test;
> >+
> >+	m_can_config_endisable(priv, true);
> >+
> >+	/* RX Buffer/FIFO Element Size 8 bytes data field */
> >+	m_can_write(priv, M_CAN_RXESC, M_CAN_RXESC_8BYTES);
> >+
> >+	/* Accept Non-matching Frames Into FIFO 0 */
> >+	m_can_write(priv, M_CAN_GFC, 0x0);
> >+
> >+	/* only support one Tx Buffer currently */
> >+	m_can_write(priv, M_CAN_TXBC, (1 << TXBC_NDTB_OFF) |
> >+			priv->mcfg[MRAM_TXB].off);
> >+
> >+	/* only support 8 bytes firstly */
> >+	m_can_write(priv, M_CAN_TXESC, TXESC_TBDS_8BYTES);
> >+
> >+	m_can_write(priv, M_CAN_TXEFC, (1 << TXEFC_EFS_OFF) |
> >+			priv->mcfg[MRAM_TXE].off);
> >+
> >+	/* rx fifo configuration, blocking mode, fifo size 1 */
> >+	m_can_write(priv, M_CAN_RXF0C,
> >+		    (priv->mcfg[MRAM_RXF0].num << RXFC_FS_OFF) |
> >+		    RXFC_FWM_1 | priv->mcfg[MRAM_RXF0].off);
> >+
> >+	m_can_write(priv, M_CAN_RXF1C,
> >+		    (priv->mcfg[MRAM_RXF1].num << RXFC_FS_OFF) |
> >+		    RXFC_FWM_1 | priv->mcfg[MRAM_RXF1].off);
> >+
> >+	cccr = m_can_read(priv, M_CAN_CCCR);
> >+	cccr &= ~(CCCR_TEST | CCCR_MON);
> >+	test = m_can_read(priv, M_CAN_TEST);
> >+	test &= ~TEST_LBCK;
> >+
> >+	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
> >+		cccr |= CCCR_MON;
> >+
> >+	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) {
> >+		cccr |= CCCR_TEST;
> >+		test |= TEST_LBCK;
> >+	}
> >+
> >+	m_can_write(priv, M_CAN_CCCR, cccr);
> >+	m_can_write(priv, M_CAN_TEST, test);
> >+
> >+	/* enable interrupts */
> >+	m_can_write(priv, M_CAN_IR, IR_ALL_INT);
> >+	if (!(priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING))
> >+		m_can_write(priv, M_CAN_IE, IR_ALL_INT & ~IR_ERR_LEC);
> >+	else
> >+		m_can_write(priv, M_CAN_IE, IR_ALL_INT);
> >+
> >+	/* route all interrupts to INT0 */
> >+	m_can_write(priv, M_CAN_ILS, ILS_ALL_INT0);
> >+
> >+	/* set bittiming params */
> >+	m_can_set_bittiming(dev);
> >+
> >+	m_can_config_endisable(priv, false);
> >+}
> >+
> >+static void m_can_start(struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+
> >+	/* basic m_can configuration */
> >+	m_can_chip_config(dev);
> >+
> >+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >+
> >+	m_can_enable_all_interrupts(priv);
> >+}
> >+
> >+static int m_can_set_mode(struct net_device *dev, enum can_mode mode)
> >+{
> >+	switch (mode) {
> >+	case CAN_MODE_START:
> >+		m_can_start(dev);
> >+		netif_wake_queue(dev);
> >+		break;
> >+	default:
> >+		return -EOPNOTSUPP;
> >+	}
> >+
> 
> I think for single we don't need switch case...
> 

It's easy for extend to support other modes.
This is also the same as other exist drivers.

> >+	return 0;
> >+}
> >+
> >+static void free_m_can_dev(struct net_device *dev)
> >+{
> >+	free_candev(dev);
> >+}
> >+
> >+static struct net_device *alloc_m_can_dev(void)
> >+{
> >+	struct net_device *dev;
> >+	struct m_can_priv *priv;
> >+
> >+	dev = alloc_candev(sizeof(*priv), 1);
> >+	if (!dev)
> >+		return NULL;
> >+
> >+	priv = netdev_priv(dev);
> >+	netif_napi_add(dev, &priv->napi, m_can_poll, M_CAN_NAPI_WEIGHT);
> >+
> >+	priv->dev = dev;
> >+	priv->can.bittiming_const = &m_can_bittiming_const;
> >+	priv->can.do_set_mode = m_can_set_mode;
> >+	priv->can.do_get_berr_counter = m_can_get_berr_counter;
> >+	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
> >+					CAN_CTRLMODE_LISTENONLY |
> >+					CAN_CTRLMODE_BERR_REPORTING;
> >+
> >+	return dev;
> >+}
> >+
> >+static int m_can_open(struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	int err;
> >+
> >+	err = clk_prepare_enable(priv->hclk);
> >+	if (err)
> >+		return err;
> >+
> >+	err = clk_prepare_enable(priv->cclk);
> >+	if (err)
> >+		goto exit_disable_hclk;
> >+
> >+	/* open the can device */
> >+	err = open_candev(dev);
> >+	if (err) {
> >+		netdev_err(dev, "failed to open can device\n");
> >+		goto exit_disable_cclk;
> >+	}
> >+
> >+	/* register interrupt handler */
> >+	err = request_irq(dev->irq, m_can_isr, IRQF_SHARED, dev->name,
> >+			  dev);
> >+	if (err < 0) {
> >+		netdev_err(dev, "failed to request interrupt\n");
> >+		goto exit_irq_fail;
> >+	}
> >+
> >+	/* start the m_can controller */
> >+	m_can_start(dev);
> >+
> >+	can_led_event(dev, CAN_LED_EVENT_OPEN);
> >+	napi_enable(&priv->napi);
> >+	netif_start_queue(dev);
> >+
> >+	return 0;
> >+
> >+exit_irq_fail:
> >+	close_candev(dev);
> >+exit_disable_cclk:
> >+	clk_disable_unprepare(priv->cclk);
> >+exit_disable_hclk:
> >+	clk_disable_unprepare(priv->hclk);
> >+	return err;
> >+}
> >+
> >+static void m_can_stop(struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+
> >+	/* disable all interrupts */
> >+	m_can_disable_all_interrupts(priv);
> >+
> >+	clk_disable_unprepare(priv->hclk);
> >+	clk_disable_unprepare(priv->cclk);
> >+
> >+	/* set the state as STOPPED */
> >+	priv->can.state = CAN_STATE_STOPPED;
> >+}
> >+
> >+static int m_can_close(struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+
> >+	netif_stop_queue(dev);
> >+	napi_disable(&priv->napi);
> >+	m_can_stop(dev);
> >+	free_irq(dev->irq, dev);
> >+	close_candev(dev);
> >+	can_led_event(dev, CAN_LED_EVENT_STOP);
> >+
> >+	return 0;
> >+}
> >+
> >+static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
> >+				    struct net_device *dev)
> >+{
> >+	struct m_can_priv *priv = netdev_priv(dev);
> >+	struct can_frame *cf = (struct can_frame *)skb->data;
> >+	u32 flags = 0, id;
> >+	void __iomem *fifo_addr;
> >+
> >+	if (can_dropped_invalid_skb(dev, skb))
> >+		return NETDEV_TX_OK;
> >+
> >+	netif_stop_queue(dev);
> >+
> >+	if (cf->can_id & CAN_RTR_FLAG)
> >+		flags |= TX_BUF_RTR;
> >+
> >+	if (cf->can_id & CAN_EFF_FLAG) {
> >+		id = cf->can_id & CAN_EFF_MASK;
> >+		flags |= TX_BUF_XTD;
> >+	} else {
> >+		id = ((cf->can_id & CAN_SFF_MASK) << 18);
> >+	}
> >+
> >+	/* message ram configuration */
> >+	fifo_addr = priv->mram_base + priv->mcfg[MRAM_TXB].off;
> >+	writel(id | flags, fifo_addr);
> >+	writel(cf->can_dlc << 16, fifo_addr + 0x4);
> >+	writel(*(u32 *)(cf->data + 0), fifo_addr + 0x8);
> >+	writel(*(u32 *)(cf->data + 4), fifo_addr + 0xc);
> >+
> >+	can_put_echo_skb(skb, dev, 0);
> >+
> >+	/* enable first TX buffer to start transfer  */
> >+	m_can_write(priv, M_CAN_TXBTIE, 0x1);
> >+	m_can_write(priv, M_CAN_TXBAR, 0x1);
> >+
> >+	return NETDEV_TX_OK;
> >+}
> >+
> >+static const struct net_device_ops m_can_netdev_ops = {
> >+	.ndo_open = m_can_open,
> >+	.ndo_stop = m_can_close,
> >+	.ndo_start_xmit = m_can_start_xmit,
> >+};
> >+
> >+static int register_m_can_dev(struct net_device *dev)
> >+{
> >+	dev->flags |= IFF_ECHO;	/* we support local echo */
> >+	dev->netdev_ops = &m_can_netdev_ops;
> >+
> >+	return register_candev(dev);
> >+}
> >+
> >+static const struct of_device_id m_can_of_table[] = {
> >+	{ .compatible = "bosch,m_can", .data = NULL },
> >+	{ /* sentinel */ },
> >+};
> >+MODULE_DEVICE_TABLE(of, m_can_of_table);
> >+
> 
> Move this device ids before the platform_driver struct ...
> 
> No need to include .data=NULL
> 
> >+static int m_can_of_parse_mram(struct platform_device *pdev,
> >+			       struct m_can_priv *priv)
> >+{
> >+	struct device_node *np = pdev->dev.of_node;
> >+	struct resource *res;
> >+	void __iomem *addr;
> >+	u32 out_val[MRAM_CFG_LEN];
> >+	int ret;
> >+
> >+	/* message ram could be shared */
> >+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
> >+	if (!res)
> >+		return -ENODEV;
> >+
> >+	addr = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> >+	if (!addr)
> >+		return -ENOMEM;
> >+
> >+	/* get message ram configuration */
> >+	ret = of_property_read_u32_array(np, "bosch,mram-cfg",
> >+					 out_val, sizeof(out_val) / 4);
> >+	if (ret) {
> >+		dev_err(&pdev->dev, "can not get message ram configuration\n");
> >+		return -ENODEV;
> >+	}
> >+
> >+	priv->mram_base = addr;
> >+	priv->mcfg[MRAM_SIDF].off = out_val[0];
> >+	priv->mcfg[MRAM_SIDF].num = out_val[1];
> >+	priv->mcfg[MRAM_XIDF].off = priv->mcfg[MRAM_SIDF].off +
> >+			priv->mcfg[MRAM_SIDF].num * SIDF_ELEMENT_SIZE;
> >+	priv->mcfg[MRAM_XIDF].num = out_val[2];
> >+	priv->mcfg[MRAM_RXF0].off = priv->mcfg[MRAM_XIDF].off +
> >+			priv->mcfg[MRAM_XIDF].num * XIDF_ELEMENT_SIZE;
> >+	priv->mcfg[MRAM_RXF0].num = out_val[3] & RXFC_FS_MASK;
> >+	priv->mcfg[MRAM_RXF1].off = priv->mcfg[MRAM_RXF0].off +
> >+			priv->mcfg[MRAM_RXF0].num * RXF0_ELEMENT_SIZE;
> >+	priv->mcfg[MRAM_RXF1].num = out_val[4] & RXFC_FS_MASK;
> >+	priv->mcfg[MRAM_RXB].off = priv->mcfg[MRAM_RXF1].off +
> >+			priv->mcfg[MRAM_RXF1].num * RXF1_ELEMENT_SIZE;
> >+	priv->mcfg[MRAM_RXB].num = out_val[5];
> >+	priv->mcfg[MRAM_TXE].off = priv->mcfg[MRAM_RXB].off +
> >+			priv->mcfg[MRAM_RXB].num * RXB_ELEMENT_SIZE;
> >+	priv->mcfg[MRAM_TXE].num = out_val[6];
> >+	priv->mcfg[MRAM_TXB].off = priv->mcfg[MRAM_TXE].off +
> >+			priv->mcfg[MRAM_TXE].num * TXE_ELEMENT_SIZE;
> >+	priv->mcfg[MRAM_TXB].num = out_val[7] & TXBC_NDTB_MASK;
> >+
> >+	dev_dbg(&pdev->dev, "mram_base %p sidf 0x%x %d xidf 0x%x %d rxf0 0x%x %d rxf1 0x%x %d rxb 0x%x %d txe 0x%x %d txb 0x%x %d\n",
> >+		priv->mram_base,
> >+		priv->mcfg[MRAM_SIDF].off, priv->mcfg[MRAM_SIDF].num,
> >+		priv->mcfg[MRAM_XIDF].off, priv->mcfg[MRAM_XIDF].num,
> >+		priv->mcfg[MRAM_RXF0].off, priv->mcfg[MRAM_RXF0].num,
> >+		priv->mcfg[MRAM_RXF1].off, priv->mcfg[MRAM_RXF1].num,
> >+		priv->mcfg[MRAM_RXB].off, priv->mcfg[MRAM_RXB].num,
> >+		priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
> >+		priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
> >+
> >+	return 0;
> >+}
> >+
> >+static int m_can_plat_probe(struct platform_device *pdev)
> >+{
> >+	struct net_device *dev;
> >+	struct m_can_priv *priv;
> >+	struct resource *res;
> >+	void __iomem *addr;
> >+	struct clk *hclk, *cclk;
> >+	int irq, ret;
> >+
> >+	hclk = devm_clk_get(&pdev->dev, "hclk");
> >+	cclk = devm_clk_get(&pdev->dev, "cclk");
> >+	if (IS_ERR(hclk) || IS_ERR(cclk)) {
> >+		dev_err(&pdev->dev, "no clock find\n");
> >+		return -ENODEV;
> >+	}
> >+
> >+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "m_can");
> >+	addr = devm_ioremap_resource(&pdev->dev, res);
> >+	irq = platform_get_irq_byname(pdev, "int0");
> >+	if (IS_ERR(addr) || irq < 0)
> >+		return -EINVAL;
> >+
> >+	/* allocate the m_can device */
> >+	dev = alloc_m_can_dev();
> >+	if (!dev)
> >+		return -ENOMEM;
> >+
> >+	priv = netdev_priv(dev);
> >+	dev->irq = irq;
> >+	priv->base = addr;
> >+	priv->device = &pdev->dev;
> >+	priv->hclk = hclk;
> >+	priv->cclk = cclk;
> >+	priv->can.clock.freq = clk_get_rate(cclk);
> >+
> >+	ret = m_can_of_parse_mram(pdev, priv);
> >+	if (ret)
> >+		goto failed_free_dev;
> >+
> >+	platform_set_drvdata(pdev, dev);
> >+	SET_NETDEV_DEV(dev, &pdev->dev);
> >+
> >+	ret = register_m_can_dev(dev);
> >+	if (ret) {
> >+		dev_err(&pdev->dev, "registering %s failed (err=%d)\n",
> >+			KBUILD_MODNAME, ret);
> >+		goto failed_free_dev;
> >+	}
> >+
> >+	devm_can_led_init(dev);
> >+
> >+	dev_info(&pdev->dev, "%s device registered (regs=%p, irq=%d)\n",
> >+		 KBUILD_MODNAME, priv->base, dev->irq);
> >+
> >+	return 0;
> >+
> >+failed_free_dev:
> >+	free_m_can_dev(dev);
> >+	return ret;
> >+}
> >+
> >+static __maybe_unused int m_can_suspend(struct device *dev)
> >+{
> >+	struct net_device *ndev = dev_get_drvdata(dev);
> >+	struct m_can_priv *priv = netdev_priv(ndev);
> >+
> >+	if (netif_running(ndev)) {
> >+		netif_stop_queue(ndev);
> >+		netif_device_detach(ndev);
> >+	}
> >+
> >+	/* TODO: enter low power */
> >+
> >+	priv->can.state = CAN_STATE_SLEEPING;
> >+
> >+	return 0;
> >+}
> >+
> >+static __maybe_unused int m_can_resume(struct device *dev)
> >+{
> >+	struct net_device *ndev = dev_get_drvdata(dev);
> >+	struct m_can_priv *priv = netdev_priv(ndev);
> >+
> >+	/* TODO: exit low power */
> >+
> >+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >+
> >+	if (netif_running(ndev)) {
> >+		netif_device_attach(ndev);
> >+		netif_start_queue(ndev);
> >+	}
> >+
> >+	return 0;
> >+}
> >+
> >+static void unregister_m_can_dev(struct net_device *dev)
> >+{
> >+	unregister_candev(dev);
> >+}
> >+
> >+static int m_can_plat_remove(struct platform_device *pdev)
> >+{
> >+	struct net_device *dev = platform_get_drvdata(pdev);
> >+
> >+	unregister_m_can_dev(dev);
> >+	platform_set_drvdata(pdev, NULL);
> >+
> >+	free_m_can_dev(dev);
> >+
> >+	return 0;
> >+}
> >+
> >+static const struct dev_pm_ops m_can_pmops = {
> >+	SET_SYSTEM_SLEEP_PM_OPS(m_can_suspend, m_can_resume)
> >+};
> >+
> 
> Give the device ids here. So that we can see the of_match_table directly...
> 
> Every driver follows the same concept...
> 

Will do it, thanks for reminder.

> >+static struct platform_driver m_can_plat_driver = {
> >+	.driver = {
> >+		.name = KBUILD_MODNAME,
> >+		.of_match_table = of_match_ptr(m_can_of_table),
> 
> This driver will always populate through the dts files. So no need to
> use 'of_match_ptr'.
> 

Could remove it.

> >+		.pm     = &m_can_pmops,
> >+	},
> >+	.probe = m_can_plat_probe,
> >+	.remove = m_can_plat_remove,
> >+};
> >+
> >+module_platform_driver(m_can_plat_driver);
> >+
> >+MODULE_AUTHOR("Dong Aisheng <b29396@xxxxxxxxxxxxx>");
> >+MODULE_LICENSE("GPL v2");
> >+MODULE_DESCRIPTION("CAN bus driver for Bosch M_CAN controller");
> 
> 
> -- 
> Regards,
> Varka Bhadram.
> 

Regards
Dong Aisheng
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux