On 02/06/2014 11:19 AM, Kedareswara rao Appana wrote: > This patch adds xilinx CAN controller support. > This driver supports both ZYNQ CANPS IP and > Soft IP AXI CAN controller. > > Signed-off-by: Kedareswara rao Appana <appa...@xilinx.com>
First review, see comments inline. Marc > --- > This patch is rebased on the 3.14 rc1 kernel. > --- > .../devicetree/bindings/net/can/xilinx_can.txt | 43 + > drivers/net/can/Kconfig | 8 + > drivers/net/can/Makefile | 1 + > drivers/net/can/xilinx_can.c | 1150 > ++++++++++++++++++++ > 4 files changed, 1202 insertions(+), 0 deletions(-) > create mode 100644 Documentation/devicetree/bindings/net/can/xilinx_can.txt > create mode 100644 drivers/net/can/xilinx_can.c > > diff --git a/Documentation/devicetree/bindings/net/can/xilinx_can.txt > b/Documentation/devicetree/bindings/net/can/xilinx_can.txt > new file mode 100644 > index 0000000..34f9643 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/can/xilinx_can.txt > @@ -0,0 +1,43 @@ > +Xilinx Axi CAN/Zynq CANPS controller Device Tree Bindings > +--------------------------------------------------------- > + > +Required properties: > +- compatible : Should be "xlnx,zynq-can-1.00.a" for Zynq CAN > + controllers and "xlnx,axi-can-1.00.a" for Axi CAN > + controllers. > +- reg : Physical base address and size of the Axi > CAN/Zynq > + CANPS registers map. > +- interrupts : Property with a value describing the interrupt > + number. > +- interrupt-parent : Must be core interrupt controller > +- clock-names : List of input clock names - "ref_clk", > "aper_clk" > + (See clock bindings for details. Two clocks are > + required for Zynq CAN. For Axi CAN > + case it is one(ref_clk)). > +- clocks : Clock phandles (see clock bindings for details). > +- xlnx,can-tx-dpth : Can Tx fifo depth (Required for Axi CAN). > +- xlnx,can-rx-dpth : Can Rx fifo depth (Required for Axi CAN). > + > + > +Example: > + > +For Zynq CANPS Dts file: > + zynq_can_0: zynq-can@e0008000 { > + compatible = "xlnx,zynq-can-1.00.a"; > + clocks = <&clkc 19>, <&clkc 36>; > + clock-names = "ref_clk", "aper_clk"; > + reg = <0xe0008000 0x1000>; > + interrupts = <0 28 4>; > + interrupt-parent = <&intc>; Above xlnx,can-{rx,tx}-dpth is mentioned as required, but it's not in the Zynq example. > + }; > +For Axi CAN Dts file: > + axi_can_0: axi-can@40000000 { > + compatible = "xlnx,axi-can-1.00.a"; > + clocks = <&clkc 0>; > + clock-names = "ref_clk" ; > + reg = <0x40000000 0x10000>; > + interrupt-parent = <&intc>; > + interrupts = <0 59 1>; > + xlnx,can-tx-dpth = <0x40>; > + xlnx,can-rx-dpth = <0x40>; > + }; > diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig > index d447b88..2344fb5 100644 > --- a/drivers/net/can/Kconfig > +++ b/drivers/net/can/Kconfig > @@ -125,6 +125,14 @@ config CAN_GRCAN > endian syntheses of the cores would need some modifications on > the hardware level to work. > > +config CAN_XILINXCAN > + tristate "Xilinx CAN" > + depends on ARCH_ZYNQ || MICROBLAZE > + default n "default n" is default, please remove. > + ---help--- > + Xilinx CAN driver. This driver supports both soft AXI CAN IP and > + Zynq CANPS IP. > + > source "drivers/net/can/mscan/Kconfig" > > source "drivers/net/can/sja1000/Kconfig" > diff --git a/drivers/net/can/Makefile b/drivers/net/can/Makefile > index c744039..0b8e11e 100644 > --- a/drivers/net/can/Makefile > +++ b/drivers/net/can/Makefile > @@ -25,5 +25,6 @@ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ican3.o > obj-$(CONFIG_CAN_FLEXCAN) += flexcan.o > obj-$(CONFIG_PCH_CAN) += pch_can.o > obj-$(CONFIG_CAN_GRCAN) += grcan.o > +obj-$(CONFIG_CAN_XILINXCAN) += xilinx_can.o > > ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG > diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c > new file mode 100644 > index 0000000..c1b2b9d > --- /dev/null > +++ b/drivers/net/can/xilinx_can.c > @@ -0,0 +1,1150 @@ > +/* Xilinx CAN device driver > + * > + * Copyright (C) 2012 - 2014 Xilinx, Inc. > + * Copyright (C) 2009 PetaLogix. All rights reserved. > + * > + * Description: > + * This driver is developed for Axi CAN IP and for Zynq CANPS Controller. > + * 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, either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <linux/clk.h> > +#include <linux/errno.h> > +#include <linux/init.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/platform_device.h> > +#include <linux/skbuff.h> > +#include <linux/string.h> > +#include <linux/types.h> > +#include <linux/can/dev.h> > +#include <linux/can/error.h> > +#include <linux/can/led.h> > + > +#define DRIVER_NAME "XILINX_CAN" > + > +/* CAN registers set */ > +#define XCAN_SRR_OFFSET 0x00 /* Software reset */ > +#define XCAN_MSR_OFFSET 0x04 /* Mode select */ > +#define XCAN_BRPR_OFFSET 0x08 /* Baud rate prescaler */ > +#define XCAN_BTR_OFFSET 0x0C /* Bit timing */ > +#define XCAN_ECR_OFFSET 0x10 /* Error counter */ > +#define XCAN_ESR_OFFSET 0x14 /* Error status */ > +#define XCAN_SR_OFFSET 0x18 /* Status */ > +#define XCAN_ISR_OFFSET 0x1C /* Interrupt status */ > +#define XCAN_IER_OFFSET 0x20 /* Interrupt enable */ > +#define XCAN_ICR_OFFSET 0x24 /* Interrupt clear */ > +#define XCAN_TXFIFO_ID_OFFSET 0x30 /* TX FIFO ID */ > +#define XCAN_TXFIFO_DLC_OFFSET 0x34 /* TX FIFO DLC */ > +#define XCAN_TXFIFO_DW1_OFFSET 0x38 /* TX FIFO Data Word 1 */ > +#define XCAN_TXFIFO_DW2_OFFSET 0x3C /* TX FIFO Data Word 2 */ > +#define XCAN_RXFIFO_ID_OFFSET 0x50 /* RX FIFO ID */ > +#define XCAN_RXFIFO_DLC_OFFSET 0x54 /* RX FIFO DLC */ > +#define XCAN_RXFIFO_DW1_OFFSET 0x58 /* RX FIFO Data Word 1 */ > +#define XCAN_RXFIFO_DW2_OFFSET 0x5C /* RX FIFO Data Word 2 */ > + > +/* CAN register bit masks - XCAN_<REG>_<BIT>_MASK */ > +#define XCAN_SRR_CEN_MASK 0x00000002 /* CAN enable */ > +#define XCAN_SRR_RESET_MASK 0x00000001 /* Soft Reset the CAN core */ > +#define XCAN_MSR_LBACK_MASK 0x00000002 /* Loop back mode select */ > +#define XCAN_MSR_SLEEP_MASK 0x00000001 /* Sleep mode select */ > +#define XCAN_BRPR_BRP_MASK 0x000000FF /* Baud rate prescaler */ > +#define XCAN_BTR_SJW_MASK 0x00000180 /* Synchronous jump width */ > +#define XCAN_BTR_TS2_MASK 0x00000070 /* Time segment 2 */ > +#define XCAN_BTR_TS1_MASK 0x0000000F /* Time segment 1 */ > +#define XCAN_ECR_REC_MASK 0x0000FF00 /* Receive error counter */ > +#define XCAN_ECR_TEC_MASK 0x000000FF /* Transmit error counter */ > +#define XCAN_ESR_ACKER_MASK 0x00000010 /* ACK error */ > +#define XCAN_ESR_BERR_MASK 0x00000008 /* Bit error */ > +#define XCAN_ESR_STER_MASK 0x00000004 /* Stuff error */ > +#define XCAN_ESR_FMER_MASK 0x00000002 /* Form error */ > +#define XCAN_ESR_CRCER_MASK 0x00000001 /* CRC error */ > +#define XCAN_SR_TXFLL_MASK 0x00000400 /* TX FIFO is full */ > +#define XCAN_SR_ESTAT_MASK 0x00000180 /* Error status */ > +#define XCAN_SR_ERRWRN_MASK 0x00000040 /* Error warning */ > +#define XCAN_SR_NORMAL_MASK 0x00000008 /* Normal mode */ > +#define XCAN_SR_LBACK_MASK 0x00000002 /* Loop back mode */ > +#define XCAN_SR_CONFIG_MASK 0x00000001 /* Configuration mode */ > +#define XCAN_IXR_TXFEMP_MASK 0x00004000 /* TX FIFO Empty */ > +#define XCAN_IXR_WKUP_MASK 0x00000800 /* Wake up interrupt */ > +#define XCAN_IXR_SLP_MASK 0x00000400 /* Sleep interrupt */ > +#define XCAN_IXR_BSOFF_MASK 0x00000200 /* Bus off interrupt */ > +#define XCAN_IXR_ERROR_MASK 0x00000100 /* Error interrupt */ > +#define XCAN_IXR_RXNEMP_MASK 0x00000080 /* RX FIFO NotEmpty intr */ > +#define XCAN_IXR_RXOFLW_MASK 0x00000040 /* RX FIFO Overflow intr */ > +#define XCAN_IXR_RXOK_MASK 0x00000010 /* Message received intr */ > +#define XCAN_IXR_TXOK_MASK 0x00000002 /* TX successful intr */ > +#define XCAN_IXR_ARBLST_MASK 0x00000001 /* Arbitration lost intr */ > +#define XCAN_IDR_ID1_MASK 0xFFE00000 /* Standard msg identifier */ > +#define XCAN_IDR_SRR_MASK 0x00100000 /* Substitute remote TXreq */ > +#define XCAN_IDR_IDE_MASK 0x00080000 /* Identifier extension */ > +#define XCAN_IDR_ID2_MASK 0x0007FFFE /* Extended message ident */ > +#define XCAN_IDR_RTR_MASK 0x00000001 /* Remote TX request */ > +#define XCAN_DLCR_DLC_MASK 0xF0000000 /* Data length code */ > + > +#define XCAN_INTR_ALL (XCAN_IXR_TXOK_MASK | > XCAN_IXR_BSOFF_MASK |\ > + XCAN_IXR_WKUP_MASK | XCAN_IXR_SLP_MASK | \ > + XCAN_IXR_RXNEMP_MASK | XCAN_IXR_ERROR_MASK | \ > + XCAN_IXR_ARBLST_MASK | XCAN_IXR_RXOK_MASK) > + > +/* CAN register bit shift - XCAN_<REG>_<BIT>_SHIFT */ > +#define XCAN_BTR_SJW_SHIFT 7 /* Synchronous jump width */ > +#define XCAN_BTR_TS2_SHIFT 4 /* Time segment 2 */ > +#define XCAN_IDR_ID1_SHIFT 21 /* Standard Messg Identifier */ > +#define XCAN_IDR_ID2_SHIFT 1 /* Extended Message Identifier */ > +#define XCAN_DLCR_DLC_SHIFT 28 /* Data length code */ > +#define XCAN_ESR_REC_SHIFT 8 /* Rx Error Count */ > + > +/* CAN frame length constants */ > +#define XCAN_ECHO_SKB_MAX 64 > +#define XCAN_NAPI_WEIGHT 64 > +#define XCAN_FRAME_MAX_DATA_LEN 8 > +#define XCAN_TIMEOUT (50 * HZ) > + > +/** > + * struct xcan_priv - This definition define CAN driver instance > + * @can: CAN private data structure. > + * @open_time: For holding timeout values > + * @waiting_ech_skb_index: Pointer for skb > + * @ech_skb_next: This tell the next packet in the queue > + * @waiting_ech_skb_num: Gives the number of packets waiting > + * @xcan_echo_skb_max_tx: Maximum number packets the driver can send > + * @xcan_echo_skb_max_rx: Maximum number packets the driver can receive > + * @napi: NAPI structure > + * @ech_skb_lock: For spinlock purpose > + * @read_reg: For reading data from CAN registers > + * @write_reg: For writing data to CAN registers > + * @dev: Network device data structure > + * @reg_base: Ioremapped address to registers > + * @irq_flags: For request_irq() > + * @aperclk: Pointer to struct clk > + * @devclk: Pointer to struct clk > + */ > +struct xcan_priv { > + struct can_priv can; > + int open_time; > + int waiting_ech_skb_index; > + int ech_skb_next; > + int waiting_ech_skb_num; > + int xcan_echo_skb_max_tx; > + int xcan_echo_skb_max_rx; > + struct napi_struct napi; > + spinlock_t ech_skb_lock; > + u32 (*read_reg)(const struct xcan_priv *priv, int reg); > + void (*write_reg)(const struct xcan_priv *priv, int reg, u32 val); Why do you have {read,write}_reg function here? > + struct net_device *dev; > + void __iomem *reg_base; > + unsigned long irq_flags; > + struct clk *aperclk; > + struct clk *devclk; > +}; > + > +/* CAN Bittiming constants as per Xilinx CAN specs */ > +static struct can_bittiming_const xcan_bittiming_const = { > + .name = DRIVER_NAME, > + .tseg1_min = 1, > + .tseg1_max = 16, > + .tseg2_min = 1, > + .tseg2_max = 8, > + .sjw_max = 4, > + .brp_min = 1, > + .brp_max = 256, > + .brp_inc = 1, > +}; > + > +/** > + * xcan_write_reg - Write a value to the device register > + * @priv: Driver private data structure > + * @reg: Register offset > + * @val: Value to write at the Register offset > + * > + * Write data to the paricular CAN register > + */ > +static void xcan_write_reg(const struct xcan_priv *priv, int reg, u32 val) > +{ > + writel(val, priv->reg_base + reg); > +} > + > +/** > + * xcan_read_reg - Read a value from the device register > + * @priv: Driver private data structure > + * @reg: Register offset > + * > + * Read data from the particular CAN register > + * Return: value read from the CAN register > + */ > +static u32 xcan_read_reg(const struct xcan_priv *priv, int reg) > +{ > + return readl(priv->reg_base + reg); > +} > + > +/** > + * set_reset_mode - Resets the CAN device mode > + * @ndev: Pointer to net_device structure > + * > + * This is the driver reset mode routine.The driver > + * enters into configuration mode. > + * > + * Return: 0 on success and failure value on error > + */ > +static int set_reset_mode(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + unsigned long timeout; > + > + priv->can.state = CAN_STATE_STOPPED; > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_OFFSET); > + > + timeout = jiffies + XCAN_TIMEOUT; > + while (!(priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_CONFIG_MASK)) { > + if (time_after(jiffies, timeout)) { > + netdev_warn(ndev, "timedout waiting for config mode\n"); > + return -ETIMEDOUT; > + } > + schedule_timeout(1); better use usleep_range() > + } > + > + return 0; > +} > + > +/** > + * xcan_set_bittiming - CAN set bit timing routine > + * @ndev: Pointer to net_device structure > + * > + * This is the driver set bittiming routine. > + * Return: 0 on success and failure value on error > + */ > +static int xcan_set_bittiming(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + struct can_bittiming *bt = &priv->can.bittiming; > + u32 btr0, btr1; > + u32 is_config_mode; > + > + /* Check whether Xilinx CAN is in configuration mode. > + * It cannot set bit timing if Xilinx CAN is not in configuration mode. > + */ > + is_config_mode = priv->read_reg(priv, XCAN_SR_OFFSET) & > + XCAN_SR_CONFIG_MASK; > + if (!is_config_mode) { > + netdev_alert(ndev, > + "Cannot set bittiming can is not in config mode\n"); > + return -EPERM; > + } > + > + netdev_dbg(ndev, "brp=%d,prop=%d,phase_seg1:%d,phase_reg2=%d,sjw=%d\n", > + bt->brp, bt->prop_seg, bt->phase_seg1, bt->phase_seg2, > + bt->sjw); I think this dbg can be removed, as it just prints the userspace values. > + > + /* Setting Baud Rate prescalar value in BRPR Register */ > + btr0 = (bt->brp - 1) & XCAN_BRPR_BRP_MASK; > + > + /* Setting Time Segment 1 in BTR Register */ > + btr1 = (bt->prop_seg + bt->phase_seg1 - 1) & XCAN_BTR_TS1_MASK; > + > + /* Setting Time Segment 2 in BTR Register */ > + btr1 |= ((bt->phase_seg2 - 1) << XCAN_BTR_TS2_SHIFT) & > + XCAN_BTR_TS2_MASK; > + > + /* Setting Synchronous jump width in BTR Register */ > + btr1 |= ((bt->sjw - 1) << XCAN_BTR_SJW_SHIFT) & XCAN_BTR_SJW_MASK; > + > + if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES) > + netdev_info(ndev, "Doesn't support Triple Sampling\n"); no need to check, it's not passed to the driver until you advertise you support it (see priv->can.ctrlmode_supported). > + netdev_dbg(ndev, "Setting BTR0=0x%02x BTR1=0x%02x\n", btr0, btr1); > + > + priv->write_reg(priv, XCAN_BRPR_OFFSET, btr0); > + priv->write_reg(priv, XCAN_BTR_OFFSET, btr1); > + > + netdev_dbg(ndev, "BRPR=0x%08x, BTR=0x%08x\n", > + priv->read_reg(priv, XCAN_BRPR_OFFSET), > + priv->read_reg(priv, XCAN_BTR_OFFSET)); One of the dbg should be enough. > + > + return 0; > +} > + > +/** > + * xcan_start - This the drivers start routine > + * @ndev: Pointer to net_device structure > + * > + * This is the drivers start routine. > + * Based on the State of the CAN device it puts > + * the CAN device into a proper mode. > + * > + * Return: 0 always > + */ > +static int xcan_start(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + > + /* Check if it is in reset mode */ > + if (priv->can.state != CAN_STATE_STOPPED) > + set_reset_mode(ndev); Please check return value of set_reset_mode > + > + /* Enable interrupts */ > + priv->write_reg(priv, XCAN_IER_OFFSET, XCAN_INTR_ALL); > + > + /* Check whether it is loopback mode or normal mode */ > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) > + /* Put device into loopback mode */ > + priv->write_reg(priv, XCAN_MSR_OFFSET, XCAN_MSR_LBACK_MASK); > + else > + /* The device is in normal mode */ > + priv->write_reg(priv, XCAN_MSR_OFFSET, 0); > + > + if (priv->can.state == CAN_STATE_STOPPED) { I think your driver is always in CAN_STATE_STOPPED, right? > + /* Enable Xilinx CAN */ > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK); > + priv->can.state = CAN_STATE_ERROR_ACTIVE; > + if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK) { > + while ((priv->read_reg(priv, XCAN_SR_OFFSET) & > + XCAN_SR_LBACK_MASK) == 0) > + ; > + } else { > + while ((priv->read_reg(priv, XCAN_SR_OFFSET) > + & XCAN_SR_NORMAL_MASK) == 0) > + ; Please add a timeout to the loops. > + } > + netdev_dbg(ndev, "status:#x%08x\n", > + priv->read_reg(priv, XCAN_SR_OFFSET)); > + } > + priv->can.state = CAN_STATE_ERROR_ACTIVE; > + return 0; > +} > + > +/** > + * xcan_do_set_mode - This sets the mode of the driver > + * @ndev: Pointer to net_device structure > + * @mode: Tells the mode of the driver > + * > + * This check the drivers state and calls the > + * the corresponding modes to set. > + * > + * Return: 0 on success and failure value on error > + */ > +static int xcan_do_set_mode(struct net_device *ndev, enum can_mode mode) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + int ret; > + > + netdev_dbg(ndev, "Setting the mode of the driver%s\n", __func__); please remove dbg > + > + if (!priv->open_time) > + return -EINVAL; please remove open_time completely. > + > + switch (mode) { > + case CAN_MODE_START: > + ret = xcan_start(ndev); > + if (ret < 0) > + netdev_err(ndev, "xcan_start failed!\n"); > + > + if (netif_queue_stopped(ndev)) > + netif_wake_queue(ndev); just call wake_queue > + break; > + default: > + ret = -EOPNOTSUPP; > + break; > + } > + > + return ret; > +} > + > +/** > + * xcan_start_xmit - Starts the transmission > + * @skb: sk_buff pointer that contains data to be Txed > + * @ndev: Pointer to net_device structure > + * > + * This function is invoked from upper layers to initiate transmission. This > + * function uses the next available free txbuff and populates their fields to > + * start the transmission. > + * > + * Return: 0 on success and failure value on error > + */ > +static int xcan_start_xmit(struct sk_buff *skb, struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &ndev->stats; > + struct can_frame *cf = (struct can_frame *)skb->data; > + u32 id, dlc, tmp_dw1, tmp_dw2 = 0, data1, data2 = 0; > + unsigned long flags; > + Please add here: if (can_dropped_invalid_skb(dev, skb)) return NETDEV_TX_OK; > + /* Check if the TX buffer is full */ > + if (priv->read_reg(priv, XCAN_SR_OFFSET) & XCAN_SR_TXFLL_MASK) { > + netif_stop_queue(ndev); > + netdev_err(ndev, "TX register is still full!\n"); > + return NETDEV_TX_BUSY; > + } else if (priv->waiting_ech_skb_num == priv->xcan_echo_skb_max_tx) { > + netif_stop_queue(ndev); > + netdev_err(ndev, "waiting:0x%08x, max:0x%08x\n", > + priv->waiting_ech_skb_num, priv->xcan_echo_skb_max_tx); > + return NETDEV_TX_BUSY; > + } You should handle flow control after you put the CAN frame into the hardware, but before activating the TX complete interrutp. > + /* Watch carefully on the bit sequence */ > + if ((cf->can_id & CAN_EFF_FLAG) == 0) { Nitpick easier to read is: if (cf->can_id & CAN_EFF_FLAG) { /* EFF handling */ } else { /* STD handling */ } > + /* Standard CAN ID format */ > + id = ((cf->can_id & CAN_SFF_MASK) << XCAN_IDR_ID1_SHIFT) & > + XCAN_IDR_ID1_MASK; > + > + if (cf->can_id & CAN_RTR_FLAG) > + /* Extended frames remote TX request */ > + id |= XCAN_IDR_SRR_MASK; > + } else { > + /* Extended CAN ID format */ > + id = ((cf->can_id & CAN_EFF_MASK) << XCAN_IDR_ID2_SHIFT) & > + XCAN_IDR_ID2_MASK; > + id |= (((cf->can_id & CAN_EFF_MASK) >> > + (CAN_EFF_ID_BITS-CAN_SFF_ID_BITS)) << > + XCAN_IDR_ID1_SHIFT) & XCAN_IDR_ID1_MASK; > + > + /* The substibute remote TX request bit should be "1" > + * for extended frames as in the Xilinx CAN datasheet > + */ > + id |= XCAN_IDR_IDE_MASK | XCAN_IDR_SRR_MASK; > + > + if (cf->can_id & CAN_RTR_FLAG) > + /* Extended frames remote TX request */ > + id |= XCAN_IDR_RTR_MASK; > + } > + > + dlc = (cf->can_dlc & 0xf) << XCAN_DLCR_DLC_SHIFT; With the above check can_dlc is valid. > + > + tmp_dw1 = le32_to_cpup((u32 *)(cf->data)); > + data1 = htonl(tmp_dw1); This looks broken. cf->data is in big endian, what is the endianess of your registers? > + if (dlc > 4) { > + tmp_dw2 = le32_to_cpup((u32 *)(cf->data + 4)); > + data2 = htonl(tmp_dw2); > + } > + > + netdev_dbg(ndev, "tx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n", > + id, dlc, data1, data2); please remove the dbg > + /* Write the Frame to Xilinx CAN TX FIFO */ > + priv->write_reg(priv, XCAN_TXFIFO_ID_OFFSET, id); > + priv->write_reg(priv, XCAN_TXFIFO_DLC_OFFSET, dlc); > + priv->write_reg(priv, XCAN_TXFIFO_DW1_OFFSET, data1); > + priv->write_reg(priv, XCAN_TXFIFO_DW2_OFFSET, data2); Which write triggers the transmission? > + stats->tx_bytes += cf->can_dlc; Can you move the tx_bytes += to your tx-complete routine? > + ndev->trans_start = jiffies; Please remove > + > + can_put_echo_skb(skb, ndev, priv->ech_skb_next); This looks racy, first fill the echo_skb, then start the transmission. > + > + priv->ech_skb_next = (priv->ech_skb_next + 1) % > + priv->xcan_echo_skb_max_tx; > + > + spin_lock_irqsave(&priv->ech_skb_lock, flags); > + priv->waiting_ech_skb_num++; > + spin_unlock_irqrestore(&priv->ech_skb_lock, flags); > + Please move the flow controll handling here. > + return NETDEV_TX_OK; > +} > + > +/** > + * xcan_rx - Is called from CAN isr to complete the received > + * frame processing > + * @ndev: Pointer to net_device structure > + * > + * This function is invoked from the CAN isr(poll) to process the Rx frames. > It > + * does minimal processing and invokes "netif_receive_skb" to complete > further > + * processing. > + * Return: 0 on success and negative error value on error > + */ > +static int xcan_rx(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &ndev->stats; > + struct can_frame *cf; > + struct sk_buff *skb; > + u32 id_xcan, dlc, data1, data2; > + > + skb = alloc_can_skb(ndev, &cf); > + if (!skb) > + return -ENOMEM; > + > + /* Read a frame from Xilinx zynq CANPS */ > + id_xcan = priv->read_reg(priv, XCAN_RXFIFO_ID_OFFSET); > + dlc = priv->read_reg(priv, XCAN_RXFIFO_DLC_OFFSET) & XCAN_DLCR_DLC_MASK; > + data1 = priv->read_reg(priv, XCAN_RXFIFO_DW1_OFFSET); > + data2 = priv->read_reg(priv, XCAN_RXFIFO_DW2_OFFSET); If you don't use data? below, don't read them in the first place. Better move the read below, where you fill the data of the can_frame. > + netdev_dbg(ndev, "rx:id=0x%08x,dlc=0x%08x,d1=0x%08x,d2=0x%08x\n", > + id_xcan, dlc, data1, data2); > please remove dbg + > + /* Change Xilinx CAN data length format to socketCAN data format */ > + cf->can_dlc = get_can_dlc((dlc & XCAN_DLCR_DLC_MASK) >> > + XCAN_DLCR_DLC_SHIFT); > + > + /* Change Xilinx CAN ID format to socketCAN ID format */ > + if (id_xcan & XCAN_IDR_IDE_MASK) { > + /* The received frame is an Extended format frame */ > + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> 3; > + cf->can_id |= (id_xcan & XCAN_IDR_ID2_MASK) >> > + XCAN_IDR_ID2_SHIFT; > + cf->can_id |= CAN_EFF_FLAG; > + if (id_xcan & XCAN_IDR_RTR_MASK) > + cf->can_id |= CAN_RTR_FLAG; > + } else { > + /* The received frame is a standard format frame */ > + cf->can_id = (id_xcan & XCAN_IDR_ID1_MASK) >> > + XCAN_IDR_ID1_SHIFT; > + if (id_xcan & XCAN_IDR_RTR_MASK) > + cf->can_id |= CAN_RTR_FLAG; > + } > + > + /* Change Xilinx CAN data format to socketCAN data format */ Don't fill cf->data if RTR is set. The endianess handling looks weird here, too. > + *(u32 *)(cf->data) = ntohl(data1); > + if (cf->can_dlc > 4) > + *(u32 *)(cf->data + 4) = ntohl(data2); > + else > + *(u32 *)(cf->data + 4) = 0; no need to set to zero > + stats->rx_bytes += cf->can_dlc; please group rx_bytes and rx_packets handling > + > + can_led_event(ndev, CAN_LED_EVENT_RX); > + > + netif_receive_skb(skb); > + > + stats->rx_packets++; > + return 0; > +} > + > +/** > + * xcan_err_interrupt - error frame Isr > + * @ndev: net_device pointer > + * @isr: interrupt status register value > + * > + * This is the CAN error interrupt and it will > + * check the the type of error and forward the error > + * frame to upper layers. > + */ > +static void xcan_err_interrupt(struct net_device *ndev, u32 isr) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &ndev->stats; > + struct can_frame *cf; > + struct sk_buff *skb; > + u32 err_status, status; > + > + skb = alloc_can_err_skb(ndev, &cf); > + if (!skb) { > + netdev_err(ndev, "alloc_can_err_skb() failed!\n"); > + return; > + } > + > + err_status = priv->read_reg(priv, XCAN_ESR_OFFSET); > + priv->write_reg(priv, XCAN_ESR_OFFSET, err_status); > + status = priv->read_reg(priv, XCAN_SR_OFFSET); > + > + if (isr & XCAN_IXR_BSOFF_MASK) { > + priv->can.state = CAN_STATE_BUS_OFF; > + cf->can_id |= CAN_ERR_BUSOFF; > + priv->can.can_stats.bus_off++; > + /* Leave device in Config Mode in bus-off state */ > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK); > + can_bus_off(ndev); > + } else if ((status & XCAN_SR_ESTAT_MASK) == XCAN_SR_ESTAT_MASK) { > + cf->can_id |= CAN_ERR_CRTL; > + priv->can.state = CAN_STATE_ERROR_PASSIVE; > + priv->can.can_stats.error_passive++; > + cf->data[1] |= CAN_ERR_CRTL_RX_PASSIVE | > + CAN_ERR_CRTL_TX_PASSIVE; > + } else if (status & XCAN_SR_ERRWRN_MASK) { > + cf->can_id |= CAN_ERR_CRTL; > + priv->can.state = CAN_STATE_ERROR_WARNING; > + priv->can.can_stats.error_warning++; > + cf->data[1] |= CAN_ERR_CRTL_RX_WARNING | > + CAN_ERR_CRTL_TX_WARNING; > + } > + > + /* Check for Arbitration lost interrupt */ > + if (isr & XCAN_IXR_ARBLST_MASK) { > + cf->can_id |= CAN_ERR_LOSTARB; > + cf->data[0] = CAN_ERR_LOSTARB_UNSPEC; > + priv->can.can_stats.arbitration_lost++; > + } > + > + /* Check for RX FIFO Overflow interrupt */ > + if (isr & XCAN_IXR_RXOFLW_MASK) { > + cf->can_id |= CAN_ERR_CRTL; > + cf->data[1] |= CAN_ERR_CRTL_RX_OVERFLOW; > + stats->rx_over_errors++; > + stats->rx_errors++; > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK); > + } > + > + /* Check for error interrupt */ > + if (isr & XCAN_IXR_ERROR_MASK) { > + cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR; > + cf->data[2] |= CAN_ERR_PROT_UNSPEC; > + > + /* Check for Ack error interrupt */ > + if (err_status & XCAN_ESR_ACKER_MASK) { > + cf->can_id |= CAN_ERR_ACK; > + cf->data[3] |= CAN_ERR_PROT_LOC_ACK; > + stats->tx_errors++; > + } > + > + /* Check for Bit error interrupt */ > + if (err_status & XCAN_ESR_BERR_MASK) { > + cf->can_id |= CAN_ERR_PROT; > + cf->data[2] = CAN_ERR_PROT_BIT; > + stats->tx_errors++; > + } > + > + /* Check for Stuff error interrupt */ > + if (err_status & XCAN_ESR_STER_MASK) { > + cf->can_id |= CAN_ERR_PROT; > + cf->data[2] = CAN_ERR_PROT_STUFF; > + stats->rx_errors++; > + } > + > + /* Check for Form error interrupt */ > + if (err_status & XCAN_ESR_FMER_MASK) { > + cf->can_id |= CAN_ERR_PROT; > + cf->data[2] = CAN_ERR_PROT_FORM; > + stats->rx_errors++; > + } > + > + /* Check for CRC error interrupt */ > + if (err_status & XCAN_ESR_CRCER_MASK) { > + cf->can_id |= CAN_ERR_PROT; > + cf->data[3] = CAN_ERR_PROT_LOC_CRC_SEQ | > + CAN_ERR_PROT_LOC_CRC_DEL; > + stats->rx_errors++; > + } > + priv->can.can_stats.bus_error++; > + } > + > + netif_rx(skb); > + stats->rx_packets++; > + stats->rx_bytes += cf->can_dlc; > + > + netdev_dbg(ndev, "%s: error status register:0x%x\n", > + __func__, priv->read_reg(priv, XCAN_ESR_OFFSET)); > +} > + > +/** > + * xcan_state_interrupt - It will check the state of the CAN device > + * @ndev: net_device pointer > + * @isr: interrupt status register value > + * > + * This will checks the state of the CAN device > + * and puts the device into appropriate state. > + */ > +static void xcan_state_interrupt(struct net_device *ndev, u32 isr) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + > + /* Check for Sleep interrupt if set put CAN device in sleep state */ > + if (isr & XCAN_IXR_SLP_MASK) > + priv->can.state = CAN_STATE_SLEEPING; > + > + /* Check for Wake up interrupt if set put CAN device in Active state */ > + if (isr & XCAN_IXR_WKUP_MASK) > + priv->can.state = CAN_STATE_ERROR_ACTIVE; > +} > + > +/** > + * xcan_rx_poll - Poll routine for rx packets (NAPI) > + * @napi: napi structure pointer > + * @quota: Max number of rx packets to be processed. > + * > + * This is the poll routine for rx part. > + * It will process the packets maximux quota value. > + * > + * Return: number of packets received > + */ > +static int xcan_rx_poll(struct napi_struct *napi, int quota) > +{ > + struct net_device *ndev = napi->dev; > + struct xcan_priv *priv = netdev_priv(ndev); > + u32 isr, ier; > + int work_done = 0; > + > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET); > + while ((isr & XCAN_IXR_RXNEMP_MASK) && (work_done < quota)) { > + if (isr & XCAN_IXR_RXOK_MASK) { > + priv->write_reg(priv, XCAN_ICR_OFFSET, > + XCAN_IXR_RXOK_MASK); > + if (xcan_rx(ndev) < 0) > + return work_done; > + work_done++; > + } else { > + priv->write_reg(priv, XCAN_ICR_OFFSET, > + XCAN_IXR_RXNEMP_MASK); > + break; > + } > + priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_RXNEMP_MASK); > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET); > + } > + > + if (work_done < quota) { > + napi_complete(napi); > + ier = priv->read_reg(priv, XCAN_IER_OFFSET); > + ier |= (XCAN_IXR_RXOK_MASK | XCAN_IXR_RXNEMP_MASK); > + priv->write_reg(priv, XCAN_IER_OFFSET, ier); > + } > + return work_done; > +} > + > +/** > + * xcan_tx_interrupt - Tx Done Isr > + * @ndev: net_device pointer > + */ > +static void xcan_tx_interrupt(struct net_device *ndev) > +{ > + unsigned long flags; > + struct xcan_priv *priv = netdev_priv(ndev); > + struct net_device_stats *stats = &ndev->stats; > + u32 processed = 0, txpackets; > + > + stats->tx_packets++; > + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__, > + priv->waiting_ech_skb_num, priv->waiting_ech_skb_index); > + > + txpackets = priv->waiting_ech_skb_num; > + > + if (txpackets) { > + can_get_echo_skb(ndev, priv->waiting_ech_skb_index); > + priv->waiting_ech_skb_index = > + (priv->waiting_ech_skb_index + 1) % > + priv->xcan_echo_skb_max_tx; > + processed++; > + txpackets--; > + } > + > + spin_lock_irqsave(&priv->ech_skb_lock, flags); > + priv->waiting_ech_skb_num -= processed; > + spin_unlock_irqrestore(&priv->ech_skb_lock, flags); > + > + netdev_dbg(ndev, "%s: waiting total:%d,current:%d\n", __func__, > + priv->waiting_ech_skb_num, priv->waiting_ech_skb_index); > + > + netif_wake_queue(ndev); > + > + can_led_event(ndev, CAN_LED_EVENT_TX); > +} > + > +/** > + * xcan_interrupt - CAN Isr > + * @irq: irq number > + * @dev_id: device id poniter > + * > + * This is the xilinx CAN Isr. It checks for the type of interrupt > + * and invokes the corresponding ISR. > + * > + * Return: > + * IRQ_NONE - If CAN device is in sleep mode, IRQ_HANDLED otherwise > + */ > +static irqreturn_t xcan_interrupt(int irq, void *dev_id) > +{ > + struct net_device *ndev = (struct net_device *)dev_id; > + struct xcan_priv *priv = netdev_priv(ndev); > + u32 isr, ier; > + > + if (priv->can.state == CAN_STATE_STOPPED) This should not happen, please remove. > + return IRQ_NONE; > + > + /* Get the interrupt status from Xilinx CAN */ > + isr = priv->read_reg(priv, XCAN_ISR_OFFSET); > + if (!isr) > + return IRQ_NONE; > + > + netdev_dbg(ndev, "%s: isr:#x%08x, err:#x%08x\n", __func__, > + isr, priv->read_reg(priv, XCAN_ESR_OFFSET)); > + > + /* Check for the type of interrupt and Processing it */ > + if (isr & (XCAN_IXR_SLP_MASK | XCAN_IXR_WKUP_MASK)) { > + priv->write_reg(priv, XCAN_ICR_OFFSET, (XCAN_IXR_SLP_MASK | > + XCAN_IXR_WKUP_MASK)); > + xcan_state_interrupt(ndev, isr); > + } > + > + /* Check for Tx interrupt and Processing it */ > + if (isr & XCAN_IXR_TXOK_MASK) { > + priv->write_reg(priv, XCAN_ICR_OFFSET, XCAN_IXR_TXOK_MASK); > + xcan_tx_interrupt(ndev); > + } > + > + /* Check for the type of error interrupt and Processing it */ > + if (isr & (XCAN_IXR_ERROR_MASK | XCAN_IXR_RXOFLW_MASK | > + XCAN_IXR_BSOFF_MASK | XCAN_IXR_ARBLST_MASK)) { > + priv->write_reg(priv, XCAN_ICR_OFFSET, (XCAN_IXR_ERROR_MASK | > + XCAN_IXR_RXOFLW_MASK | XCAN_IXR_BSOFF_MASK | > + XCAN_IXR_ARBLST_MASK)); > + xcan_err_interrupt(ndev, isr); > + } > + > + /* Check for the type of receive interrupt and Processing it */ > + if (isr & (XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK)) { > + ier = priv->read_reg(priv, XCAN_IER_OFFSET); > + ier &= ~(XCAN_IXR_RXNEMP_MASK | XCAN_IXR_RXOK_MASK); > + priv->write_reg(priv, XCAN_IER_OFFSET, ier); > + napi_schedule(&priv->napi); > + } > + return IRQ_HANDLED; > +} > + > +/** > + * xcan_stop - Driver stop routine > + * @ndev: Pointer to net_device structure > + * > + * This is the drivers stop routine. It will disable the > + * interrupts and put the device into configuration mode. > + */ > +static void xcan_stop(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + u32 ier; > + > + /* Disable interrupts and leave the can in configuration mode */ > + ier = priv->read_reg(priv, XCAN_IER_OFFSET); > + ier &= ~XCAN_INTR_ALL; > + priv->write_reg(priv, XCAN_IER_OFFSET, ier); > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_RESET_MASK); > + priv->can.state = CAN_STATE_STOPPED; > +} > + > +/** > + * xcan_open - Driver open routine > + * @ndev: Pointer to net_device structure > + * > + * This is the driver open routine. > + * Return: 0 on success and failure value on error > + */ > +static int xcan_open(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + int err; > + > + /* Set chip into reset mode */ > + err = set_reset_mode(ndev); > + if (err < 0) > + netdev_err(ndev, "mode resetting failed failed!\n"); > + > + /* Common open */ > + err = open_candev(ndev); > + if (err) > + return err; > + > + err = xcan_start(ndev); > + if (err < 0) > + netdev_err(ndev, "xcan_start failed!\n"); > + > + > + can_led_event(ndev, CAN_LED_EVENT_OPEN); > + napi_enable(&priv->napi); > + netif_start_queue(ndev); > + > + return 0; > +} > + > +/** > + * xcan_close - Driver close routine > + * @ndev: Pointer to net_device structure > + * > + * Return: 0 always > + */ > +static int xcan_close(struct net_device *ndev) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + > + netif_stop_queue(ndev); > + napi_disable(&priv->napi); > + xcan_stop(ndev); > + close_candev(ndev); > + > + can_led_event(ndev, CAN_LED_EVENT_STOP); > + > + return 0; > +} > + > +/** > + * xcan_get_berr_counter - error counter routine > + * @ndev: Pointer to net_device structure > + * @bec: Pointer to can_berr_counter structure > + * > + * This is the driver error counter routine. > + * Return: 0 always > + */ > +static int xcan_get_berr_counter(const struct net_device *ndev, > + struct can_berr_counter *bec) > +{ > + struct xcan_priv *priv = netdev_priv(ndev); > + > + bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) & XCAN_ECR_TEC_MASK; > + bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) & > + XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT); > + return 0; > +} > + > +static const struct net_device_ops xcan_netdev_ops = { > + .ndo_open = xcan_open, > + .ndo_stop = xcan_close, > + .ndo_start_xmit = xcan_start_xmit, > +}; > + > +#ifdef CONFIG_PM_SLEEP > +/** > + * xcan_suspend - Suspend method for the driver > + * @_dev: Address of the platform_device structure > + * > + * Put the driver into low power mode. > + * Return: 0 always > + */ > +static int xcan_suspend(struct device *_dev) > +{ > + struct platform_device *pdev = container_of(_dev, > + struct platform_device, dev); > + struct net_device *ndev = platform_get_drvdata(pdev); > + struct xcan_priv *priv = netdev_priv(ndev); > + > + if (netif_running(ndev)) { > + netif_stop_queue(ndev); > + netif_device_detach(ndev); > + } > + > + priv->write_reg(priv, XCAN_MSR_OFFSET, XCAN_MSR_SLEEP_MASK); > + priv->can.state = CAN_STATE_SLEEPING; > + > + clk_disable(priv->aperclk); > + clk_disable(priv->devclk); > + > + return 0; > +} > + > +/** > + * xcan_resume - Resume from suspend > + * @dev: Address of the platformdevice structure > + * > + * Resume operation after suspend. > + * Return: 0 on success and failure value on error > + */ > +static int xcan_resume(struct device *dev) > +{ > + struct platform_device *pdev = container_of(dev, > + struct platform_device, dev); > + struct net_device *ndev = platform_get_drvdata(pdev); > + struct xcan_priv *priv = netdev_priv(ndev); > + int ret; > + > + ret = clk_enable(priv->aperclk); > + if (ret) { > + dev_err(dev, "Cannot enable clock.\n"); > + return ret; > + } > + ret = clk_enable(priv->devclk); > + if (ret) { > + dev_err(dev, "Cannot enable clock.\n"); > + return ret; > + } > + > + priv->write_reg(priv, XCAN_MSR_OFFSET, 0); > + priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK); > + priv->can.state = CAN_STATE_ERROR_ACTIVE; > + > + if (netif_running(ndev)) { > + netif_device_attach(ndev); > + netif_start_queue(ndev); > + } > + > + return 0; > +} > +#endif > + > +static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend, xcan_resume); > + > +/** > + * xcan_probe - Platform registration call > + * @pdev: Handle to the platform device structure > + * > + * This function does all the memory allocation and registration for the CAN > + * device. > + * > + * Return: 0 on success and failure value on error > + */ > +static int xcan_probe(struct platform_device *pdev) > +{ > + struct resource *res; /* IO mem resources */ > + struct net_device *ndev; > + struct xcan_priv *priv; > + int ret, fifodep; > + > + /* Create a CAN device instance */ > + ndev = alloc_candev(sizeof(struct xcan_priv), XCAN_ECHO_SKB_MAX); > + if (!ndev) > + return -ENOMEM; > + > + priv = netdev_priv(ndev); > + priv->dev = ndev; > + priv->can.bittiming_const = &xcan_bittiming_const; > + priv->can.do_set_bittiming = xcan_set_bittiming; > + priv->can.do_set_mode = xcan_do_set_mode; > + priv->can.do_get_berr_counter = xcan_get_berr_counter; > + priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK | > + CAN_CTRLMODE_BERR_REPORTING; > + priv->xcan_echo_skb_max_tx = XCAN_ECHO_SKB_MAX; > + priv->xcan_echo_skb_max_rx = XCAN_NAPI_WEIGHT; > + > + /* Get IRQ for the device */ > + ndev->irq = platform_get_irq(pdev, 0); > + ret = devm_request_irq(&pdev->dev, ndev->irq, &xcan_interrupt, > + priv->irq_flags, dev_name(&pdev->dev), > + (void *)ndev); We usually request the interrupt on in the open() function > + if (ret < 0) { > + dev_err(&pdev->dev, "Irq allocation for CAN failed\n"); > + goto err_free; > + } > + > + spin_lock_init(&priv->ech_skb_lock); > + ndev->flags |= IFF_ECHO; /* We support local echo */ > + > + platform_set_drvdata(pdev, ndev); > + SET_NETDEV_DEV(ndev, &pdev->dev); > + ndev->netdev_ops = &xcan_netdev_ops; > + > + /* Get the virtual base address for the device */ > + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + priv->reg_base = devm_ioremap_resource(&pdev->dev, res); > + if (IS_ERR(priv->reg_base)) { > + ret = PTR_ERR(priv->reg_base); > + goto err_free; > + } > + ndev->mem_start = res->start; > + ndev->mem_end = res->end; > + > + priv->write_reg = xcan_write_reg; > + priv->read_reg = xcan_read_reg; > + > + /* Getting the CAN devclk info */ > + priv->devclk = devm_clk_get(&pdev->dev, "ref_clk"); > + if (IS_ERR(priv->devclk)) { > + dev_err(&pdev->dev, "Device clock not found.\n"); > + ret = PTR_ERR(priv->devclk); > + goto err_free; > + } > + > + /* Check for type of CAN device */ > + if (of_device_is_compatible(pdev->dev.of_node, > + "xlnx,zynq-can-1.00.a")) { > + priv->aperclk = devm_clk_get(&pdev->dev, "aper_clk"); > + if (IS_ERR(priv->aperclk)) { > + dev_err(&pdev->dev, "aper clock not found\n"); > + ret = PTR_ERR(priv->aperclk); > + goto err_free; > + } > + } else { > + priv->aperclk = priv->devclk; > + ret = of_property_read_u32(pdev->dev.of_node, > + "xlnx,can-tx-dpth", &fifodep); > + if (ret < 0) > + goto err_free; > + priv->xcan_echo_skb_max_tx = fifodep; > + ret = of_property_read_u32(pdev->dev.of_node, > + "xlnx,can-rx-dpth", &fifodep); > + if (ret < 0) > + goto err_free; > + priv->xcan_echo_skb_max_rx = fifodep; > + } > + > + ret = clk_prepare_enable(priv->devclk); > + if (ret) { > + dev_err(&pdev->dev, "unable to enable device clock\n"); > + goto err_free; > + } > + > + ret = clk_prepare_enable(priv->aperclk); > + if (ret) { > + dev_err(&pdev->dev, "unable to enable aper clock\n"); > + goto err_unprepar_disabledev; > + } > + > + priv->can.clock.freq = clk_get_rate(priv->devclk); > + > + netif_napi_add(ndev, &priv->napi, xcan_rx_poll, > + priv->xcan_echo_skb_max_rx); > + ret = register_candev(ndev); > + if (ret) { > + dev_err(&pdev->dev, "fail to register failed (err=%d)\n", ret); > + goto err_unprepar_disableaper; > + } > + > + devm_can_led_init(ndev); > + dev_info(&pdev->dev, > + "reg_base=0x%p irq=%d clock=%d, tx fifo depth:%d\n", > + priv->reg_base, ndev->irq, priv->can.clock.freq, > + priv->xcan_echo_skb_max_tx); > + > + return 0; > + > +err_unprepar_disableaper: > + clk_disable_unprepare(priv->aperclk); > +err_unprepar_disabledev: > + clk_disable_unprepare(priv->devclk); > +err_free: > + free_candev(ndev); > + > + return ret; > +} > + > +/** > + * xcan_remove - Unregister the device after releasing the resources > + * @pdev: Handle to the platform device structure > + * > + * This function frees all the resources allocated to the device. > + * Return: 0 always > + */ > +static int xcan_remove(struct platform_device *pdev) > +{ > + struct net_device *ndev = platform_get_drvdata(pdev); > + struct xcan_priv *priv = netdev_priv(ndev); > + > + if (set_reset_mode(ndev) < 0) > + netdev_err(ndev, "mode resetting failed!\n"); > + > + unregister_candev(ndev); > + netif_napi_del(&priv->napi); > + clk_disable_unprepare(priv->aperclk); > + clk_disable_unprepare(priv->devclk); > + > + free_candev(ndev); > + > + return 0; > +} > + > +/* Match table for OF platform binding */ > +static struct of_device_id xcan_of_match[] = { > + { .compatible = "xlnx,zynq-can-1.00.a", }, > + { .compatible = "xlnx,axi-can-1.00.a", }, > + { /* end of list */ }, > +}; > +MODULE_DEVICE_TABLE(of, xcan_of_match); > + > +static struct platform_driver xcan_driver = { > + .probe = xcan_probe, > + .remove = xcan_remove, > + .driver = { > + .owner = THIS_MODULE, > + .name = DRIVER_NAME, > + .pm = &xcan_dev_pm_ops, > + .of_match_table = xcan_of_match, > + }, > +}; > + > +module_platform_driver(xcan_driver); > + > +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Xilinx Inc"); > +MODULE_DESCRIPTION("Xilinx CAN interface"); > -- 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 |
signature.asc
Description: OpenPGP digital signature