Netdev List
 help / color / mirror / Atom feed
* [PATCH NEXT 0/5]qlcnic: fixes
From: Amit Kumar Salecha @ 2010-06-17 12:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman


Hi
   Sending series of 5 patches, mainly to improve rx performance and
   fix netif_stop_queue race.

   Please apply them on net-next.

-Amit

^ permalink raw reply

* [PATCH NEXT 4/5] qlcnic: fix race in tx stop queue
From: Amit Kumar Salecha @ 2010-06-17 12:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Rajesh K Borundia
In-Reply-To: <1276779402-496-1-git-send-email-amit.salecha@qlogic.com>

From: Rajesh K Borundia <rajesh.borundia@qlogic.com>

There is a race between netif_stop_queue and netif_stopped_queue
check. So check once again if buffers are available to avoid race.
With above logic we can also get rid of tx lock in process_cmd_ring.

Signed-off-by: Rajesh K Borundia <rajesh.borundia@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h      |    8 +++++---
 drivers/net/qlcnic/qlcnic_hw.c   |   12 +++++++++---
 drivers/net/qlcnic/qlcnic_init.c |    2 ++
 drivers/net/qlcnic/qlcnic_main.c |   23 ++++++++++-------------
 4 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 9970cff..99ccdd8 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -113,8 +113,10 @@
 #define TX_UDPV6_PKT	0x0c
 
 /* Tx defines */
-#define MAX_BUFFERS_PER_CMD	32
-#define TX_STOP_THRESH		((MAX_SKB_FRAGS >> 2) + 4)
+#define MAX_TSO_HEADER_DESC	2
+#define MGMT_CMD_DESC_RESV	4
+#define TX_STOP_THRESH		((MAX_SKB_FRAGS >> 2) + MAX_TSO_HEADER_DESC \
+							+ MGMT_CMD_DESC_RESV)
 #define QLCNIC_MAX_TX_TIMEOUTS	2
 
 /*
@@ -369,7 +371,7 @@ struct qlcnic_recv_crb {
  */
 struct qlcnic_cmd_buffer {
 	struct sk_buff *skb;
-	struct qlcnic_skb_frag frag_array[MAX_BUFFERS_PER_CMD + 1];
+	struct qlcnic_skb_frag frag_array[MAX_SKB_FRAGS + 1];
 	u32 frag_count;
 };
 
diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index f776956..d9becb9 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -338,9 +338,15 @@ qlcnic_send_cmd_descs(struct qlcnic_adapter *adapter,
 
 	if (nr_desc >= qlcnic_tx_avail(tx_ring)) {
 		netif_tx_stop_queue(tx_ring->txq);
-		__netif_tx_unlock_bh(tx_ring->txq);
-		adapter->stats.xmit_off++;
-		return -EBUSY;
+		smp_mb();
+		if (qlcnic_tx_avail(tx_ring) > nr_desc) {
+			if (qlcnic_tx_avail(tx_ring) > TX_STOP_THRESH)
+				netif_tx_wake_queue(tx_ring->txq);
+		} else {
+			adapter->stats.xmit_off++;
+			__netif_tx_unlock_bh(tx_ring->txq);
+			return -EBUSY;
+		}
 	}
 
 	do {
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index 2bd00d5..058ce61 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -181,7 +181,9 @@ skip_rds:
 
 	tx_ring = adapter->tx_ring;
 	vfree(tx_ring->cmd_buf_arr);
+	tx_ring->cmd_buf_arr = NULL;
 	kfree(adapter->tx_ring);
+	adapter->tx_ring = NULL;
 }
 
 int qlcnic_alloc_sw_resources(struct qlcnic_adapter *adapter)
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 06d2dfd..655bccd 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -132,12 +132,6 @@ qlcnic_update_cmd_producer(struct qlcnic_adapter *adapter,
 		struct qlcnic_host_tx_ring *tx_ring)
 {
 	writel(tx_ring->producer, tx_ring->crb_cmd_producer);
-
-	if (qlcnic_tx_avail(tx_ring) <= TX_STOP_THRESH) {
-		netif_stop_queue(adapter->netdev);
-		smp_mb();
-		adapter->stats.xmit_off++;
-	}
 }
 
 static const u32 msi_tgt_status[8] = {
@@ -1137,7 +1131,7 @@ qlcnic_setup_netdev(struct qlcnic_adapter *adapter,
 	adapter->max_mc_count = 38;
 
 	netdev->netdev_ops	   = &qlcnic_netdev_ops;
-	netdev->watchdog_timeo     = 2*HZ;
+	netdev->watchdog_timeo     = 5*HZ;
 
 	qlcnic_change_mtu(netdev, netdev->mtu);
 
@@ -1709,10 +1703,15 @@ qlcnic_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 	/* 4 fragments per cmd des */
 	no_of_desc = (frag_count + 3) >> 2;
 
-	if (unlikely(no_of_desc + 2 > qlcnic_tx_avail(tx_ring))) {
+	if (unlikely(qlcnic_tx_avail(tx_ring) <= TX_STOP_THRESH)) {
 		netif_stop_queue(netdev);
-		adapter->stats.xmit_off++;
-		return NETDEV_TX_BUSY;
+		smp_mb();
+		if (qlcnic_tx_avail(tx_ring) > TX_STOP_THRESH)
+			netif_start_queue(netdev);
+		else {
+			adapter->stats.xmit_off++;
+			return NETDEV_TX_BUSY;
+		}
 	}
 
 	producer = tx_ring->producer;
@@ -2018,14 +2017,12 @@ static int qlcnic_process_cmd_ring(struct qlcnic_adapter *adapter)
 		smp_mb();
 
 		if (netif_queue_stopped(netdev) && netif_carrier_ok(netdev)) {
-			__netif_tx_lock(tx_ring->txq, smp_processor_id());
 			if (qlcnic_tx_avail(tx_ring) > TX_STOP_THRESH) {
 				netif_wake_queue(netdev);
-				adapter->tx_timeo_cnt = 0;
 				adapter->stats.xmit_on++;
 			}
-			__netif_tx_unlock(tx_ring->txq);
 		}
+		adapter->tx_timeo_cnt = 0;
 	}
 	/*
 	 * If everything is freed up to consumer then check if the ring is full
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH NEXT 3/5] qlcnic: seperate interrupt for TX
From: Amit Kumar Salecha @ 2010-06-17 12:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, schacko
In-Reply-To: <1276779402-496-1-git-send-email-amit.salecha@qlogic.com>

From: schacko <schacko@qlogic.com>

Earlier all poll routine can process rx and tx, But now
one poll routine to process rx + tx and other for rx only.
Last msix vector will be used for separate tx interrupt.

o This is supported from fw version 4.4.2.
o Bump version 5.0.5

Signed-off-by: Sony Chacko <schacko@qlogic.com>
---
 drivers/net/qlcnic/qlcnic.h      |   14 +++++++++-----
 drivers/net/qlcnic/qlcnic_ctx.c  |    7 ++++++-
 drivers/net/qlcnic/qlcnic_init.c |   35 +++++++++++++++++++++++++----------
 drivers/net/qlcnic/qlcnic_main.c |   35 ++++++++++++++++++++++++++++++++---
 4 files changed, 72 insertions(+), 19 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic.h b/drivers/net/qlcnic/qlcnic.h
index 7d31caa..9970cff 100644
--- a/drivers/net/qlcnic/qlcnic.h
+++ b/drivers/net/qlcnic/qlcnic.h
@@ -51,8 +51,8 @@
 
 #define _QLCNIC_LINUX_MAJOR 5
 #define _QLCNIC_LINUX_MINOR 0
-#define _QLCNIC_LINUX_SUBVERSION 4
-#define QLCNIC_LINUX_VERSIONID  "5.0.4"
+#define _QLCNIC_LINUX_SUBVERSION 5
+#define QLCNIC_LINUX_VERSIONID  "5.0.5"
 #define QLCNIC_DRV_IDC_VER  0x01
 
 #define QLCNIC_VERSION_CODE(a, b, c)	(((a) << 24) + ((b) << 16) + (c))
@@ -68,6 +68,7 @@
 #define QLCNIC_DECODE_VERSION(v) \
 	QLCNIC_VERSION_CODE(((v) & 0xff), (((v) >> 8) & 0xff), ((v) >> 16))
 
+#define QLCNIC_MIN_FW_VERSION     QLCNIC_VERSION_CODE(4, 4, 2)
 #define QLCNIC_NUM_FLASH_SECTORS (64)
 #define QLCNIC_FLASH_SECTOR_SIZE (64 * 1024)
 #define QLCNIC_FLASH_TOTAL_SIZE  (QLCNIC_NUM_FLASH_SECTORS \
@@ -567,6 +568,7 @@ struct qlcnic_recv_context {
 #define QLCNIC_CAP0_LSO 		(1 << 6)
 #define QLCNIC_CAP0_JUMBO_CONTIGUOUS	(1 << 7)
 #define QLCNIC_CAP0_LRO_CONTIGUOUS	(1 << 8)
+#define QLCNIC_CAP0_VALIDOFF		(1 << 11)
 
 /*
  * Context state
@@ -602,9 +604,10 @@ struct qlcnic_hostrq_rx_ctx {
 	__le32 sds_ring_offset;	/* Offset to SDS config */
 	__le16 num_rds_rings;	/* Count of RDS rings */
 	__le16 num_sds_rings;	/* Count of SDS rings */
-	__le16 rsvd1;		/* Padding */
-	__le16 rsvd2;		/* Padding */
-	u8  reserved[128]; 	/* reserve space for future expansion*/
+	__le16 valid_field_offset;
+	u8  txrx_sds_binding;
+	u8  msix_handler;
+	u8  reserved[128];      /* reserve space for future expansion*/
 	/* MUST BE 64-bit aligned.
 	   The following is packed:
 	   - N hostrq_rds_rings
@@ -1109,6 +1112,7 @@ void qlcnic_request_firmware(struct qlcnic_adapter *adapter);
 void qlcnic_release_firmware(struct qlcnic_adapter *adapter);
 int qlcnic_pinit_from_rom(struct qlcnic_adapter *adapter);
 int qlcnic_setup_idc_param(struct qlcnic_adapter *adapter);
+int qlcnic_check_flash_fw_ver(struct qlcnic_adapter *adapter);
 
 int qlcnic_rom_fast_read(struct qlcnic_adapter *adapter, int addr, int *valp);
 int qlcnic_rom_fast_read_words(struct qlcnic_adapter *adapter, int addr,
diff --git a/drivers/net/qlcnic/qlcnic_ctx.c b/drivers/net/qlcnic/qlcnic_ctx.c
index 90ed6fb..7c96c8e 100644
--- a/drivers/net/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/qlcnic/qlcnic_ctx.c
@@ -152,9 +152,14 @@ qlcnic_fw_cmd_create_rx_ctx(struct qlcnic_adapter *adapter)
 
 	prq->host_rsp_dma_addr = cpu_to_le64(cardrsp_phys_addr);
 
-	cap = (QLCNIC_CAP0_LEGACY_CONTEXT | QLCNIC_CAP0_LEGACY_MN);
+	cap = (QLCNIC_CAP0_LEGACY_CONTEXT | QLCNIC_CAP0_LEGACY_MN
+						| QLCNIC_CAP0_VALIDOFF);
 	cap |= (QLCNIC_CAP0_JUMBO_CONTIGUOUS | QLCNIC_CAP0_LRO_CONTIGUOUS);
 
+	prq->valid_field_offset = offsetof(struct qlcnic_hostrq_rx_ctx,
+							 msix_handler);
+	prq->txrx_sds_binding = nsds_rings - 1;
+
 	prq->capabilities[0] = cpu_to_le32(cap);
 	prq->host_int_crb_mode =
 		cpu_to_le32(QLCNIC_HOST_INT_CRB_MODE_SHARED);
diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index 317750d..2bd00d5 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -543,16 +543,34 @@ qlcnic_setup_idc_param(struct qlcnic_adapter *adapter) {
 	return 0;
 }
 
+int
+qlcnic_check_flash_fw_ver(struct qlcnic_adapter *adapter)
+{
+	u32 ver = -1, min_ver;
+
+	qlcnic_rom_fast_read(adapter, QLCNIC_FW_VERSION_OFFSET, (int *)&ver);
+
+	ver = QLCNIC_DECODE_VERSION(ver);
+	min_ver = QLCNIC_MIN_FW_VERSION;
+
+	if (ver < min_ver) {
+		dev_err(&adapter->pdev->dev,
+			"firmware version %d.%d.%d unsupported."
+			"Min supported version %d.%d.%d\n",
+			_major(ver), _minor(ver), _build(ver),
+			_major(min_ver), _minor(min_ver), _build(min_ver));
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int
 qlcnic_has_mn(struct qlcnic_adapter *adapter)
 {
-	u32 capability, flashed_ver;
+	u32 capability;
 	capability = 0;
 
-	qlcnic_rom_fast_read(adapter,
-			QLCNIC_FW_VERSION_OFFSET, (int *)&flashed_ver);
-	flashed_ver = QLCNIC_DECODE_VERSION(flashed_ver);
-
 	capability = QLCRD32(adapter, QLCNIC_PEG_TUNE_CAPABILITY);
 	if (capability & QLCNIC_PEG_TUNE_MN_PRESENT)
 		return 1;
@@ -1006,7 +1024,7 @@ static int
 qlcnic_validate_firmware(struct qlcnic_adapter *adapter)
 {
 	__le32 val;
-	u32 ver, min_ver, bios, min_size;
+	u32 ver, bios, min_size;
 	struct pci_dev *pdev = adapter->pdev;
 	const struct firmware *fw = adapter->fw;
 	u8 fw_type = adapter->fw_type;
@@ -1028,12 +1046,9 @@ qlcnic_validate_firmware(struct qlcnic_adapter *adapter)
 		return -EINVAL;
 
 	val = qlcnic_get_fw_version(adapter);
-
-	min_ver = QLCNIC_VERSION_CODE(4, 0, 216);
-
 	ver = QLCNIC_DECODE_VERSION(val);
 
-	if ((_major(ver) > _QLCNIC_LINUX_MAJOR) || (ver < min_ver)) {
+	if (ver < QLCNIC_MIN_FW_VERSION) {
 		dev_err(&pdev->dev,
 				"%s: firmware version %d.%d.%d unsupported\n",
 		fw_name[fw_type], _major(ver), _minor(ver), _build(ver));
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 28ed28c..06d2dfd 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -83,6 +83,7 @@ static void qlcnic_schedule_work(struct qlcnic_adapter *adapter,
 		work_func_t func, int delay);
 static void qlcnic_cancel_fw_work(struct qlcnic_adapter *adapter);
 static int qlcnic_poll(struct napi_struct *napi, int budget);
+static int qlcnic_rx_poll(struct napi_struct *napi, int budget);
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void qlcnic_poll_controller(struct net_device *netdev);
 #endif
@@ -195,8 +196,13 @@ qlcnic_napi_add(struct qlcnic_adapter *adapter, struct net_device *netdev)
 
 	for (ring = 0; ring < adapter->max_sds_rings; ring++) {
 		sds_ring = &recv_ctx->sds_rings[ring];
-		netif_napi_add(netdev, &sds_ring->napi,
-				qlcnic_poll, QLCNIC_NETDEV_WEIGHT);
+
+		if (ring == adapter->max_sds_rings - 1)
+			netif_napi_add(netdev, &sds_ring->napi, qlcnic_poll,
+				QLCNIC_NETDEV_WEIGHT/adapter->max_sds_rings);
+		else
+			netif_napi_add(netdev, &sds_ring->napi,
+				qlcnic_rx_poll, QLCNIC_NETDEV_WEIGHT*2);
 	}
 
 	return 0;
@@ -743,8 +749,12 @@ qlcnic_start_firmware(struct qlcnic_adapter *adapter)
 
 	if (load_fw_file)
 		qlcnic_request_firmware(adapter);
-	else
+	else {
+		if (qlcnic_check_flash_fw_ver(adapter))
+			goto err_out;
+
 		adapter->fw_type = QLCNIC_FLASH_ROMIMAGE;
+	}
 
 	err = qlcnic_need_fw_reset(adapter);
 	if (err < 0)
@@ -2060,6 +2070,25 @@ static int qlcnic_poll(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
+static int qlcnic_rx_poll(struct napi_struct *napi, int budget)
+{
+	struct qlcnic_host_sds_ring *sds_ring =
+		container_of(napi, struct qlcnic_host_sds_ring, napi);
+
+	struct qlcnic_adapter *adapter = sds_ring->adapter;
+	int work_done;
+
+	work_done = qlcnic_process_rcv_ring(sds_ring, budget);
+
+	if (work_done < budget) {
+		napi_complete(&sds_ring->napi);
+		if (test_bit(__QLCNIC_DEV_UP, &adapter->state))
+			qlcnic_enable_int(sds_ring);
+	}
+
+	return work_done;
+}
+
 #ifdef CONFIG_NET_POLL_CONTROLLER
 static void qlcnic_poll_controller(struct net_device *netdev)
 {
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH NEXT 1/5] qlcnic: fix device soft reset
From: Amit Kumar Salecha @ 2010-06-17 12:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Sucheta Chakraborty
In-Reply-To: <1276779402-496-1-git-send-email-amit.salecha@qlogic.com>

From: Sucheta Chakraborty <sucheta@dut4145.unminc.com>

During device soft reset, don't halt every device block.
Access to some blocks is required during recovery.

Signed-off-by: Sucheta Chakraborty <sucheta@dut4145.unminc.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic_init.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic_init.c b/drivers/net/qlcnic/qlcnic_init.c
index 635c990..317750d 100644
--- a/drivers/net/qlcnic/qlcnic_init.c
+++ b/drivers/net/qlcnic/qlcnic_init.c
@@ -413,7 +413,7 @@ int qlcnic_pinit_from_rom(struct qlcnic_adapter *adapter)
 
 	/* resetall */
 	qlcnic_rom_lock(adapter);
-	QLCWR32(adapter, QLCNIC_ROMUSB_GLB_SW_RESET, 0xffffffff);
+	QLCWR32(adapter, QLCNIC_ROMUSB_GLB_SW_RESET, 0xfeffffff);
 	qlcnic_rom_unlock(adapter);
 
 	if (qlcnic_rom_fast_read(adapter, 0, &n) != 0 || (n != 0xcafecafe) ||
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH NEXT 2/5] qlcnic: change driver description
From: Amit Kumar Salecha @ 2010-06-17 12:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, Sritej Velaga
In-Reply-To: <1276779402-496-1-git-send-email-amit.salecha@qlogic.com>

From: Sritej Velaga <sritej.velaga@qlogic.com>

o Remove extra printing of mac address
o This driver also supports NIC only Qlogic adapters.

Signed-off-by: Sritej Velaga <sritej.velaga@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic_ctx.c  |    5 ++---
 drivers/net/qlcnic/qlcnic_main.c |    8 ++++----
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic_ctx.c b/drivers/net/qlcnic/qlcnic_ctx.c
index 42feb23..90ed6fb 100644
--- a/drivers/net/qlcnic/qlcnic_ctx.c
+++ b/drivers/net/qlcnic/qlcnic_ctx.c
@@ -589,11 +589,10 @@ int qlcnic_get_mac_address(struct qlcnic_adapter *adapter, u8 *mac)
 			0,
 			QLCNIC_CDRP_CMD_MAC_ADDRESS);
 
-	if (err == QLCNIC_RCODE_SUCCESS) {
+	if (err == QLCNIC_RCODE_SUCCESS)
 		qlcnic_fetch_mac(adapter, QLCNIC_ARG1_CRB_OFFSET,
 				QLCNIC_ARG2_CRB_OFFSET, 0, mac);
-		dev_info(&adapter->pdev->dev, "MAC address: %pM\n", mac);
-	} else {
+	else {
 		dev_err(&adapter->pdev->dev,
 			"Failed to get mac address%d\n", err);
 		err = -EIO;
diff --git a/drivers/net/qlcnic/qlcnic_main.c b/drivers/net/qlcnic/qlcnic_main.c
index 128a0a7..28ed28c 100644
--- a/drivers/net/qlcnic/qlcnic_main.c
+++ b/drivers/net/qlcnic/qlcnic_main.c
@@ -35,14 +35,14 @@
 #include <linux/inetdevice.h>
 #include <linux/sysfs.h>
 
-MODULE_DESCRIPTION("QLogic 10 GbE Converged Ethernet Driver");
+MODULE_DESCRIPTION("QLogic 1/10 GbE Converged/Intelligent Ethernet Driver");
 MODULE_LICENSE("GPL");
 MODULE_VERSION(QLCNIC_LINUX_VERSIONID);
 MODULE_FIRMWARE(QLCNIC_UNIFIED_ROMIMAGE_NAME);
 
 char qlcnic_driver_name[] = "qlcnic";
-static const char qlcnic_driver_string[] = "QLogic Converged Ethernet Driver v"
-    QLCNIC_LINUX_VERSIONID;
+static const char qlcnic_driver_string[] = "QLogic 1/10 GbE "
+	"Converged/Intelligent Ethernet Driver v" QLCNIC_LINUX_VERSIONID;
 
 static int port_mode = QLCNIC_PORT_MODE_AUTO_NEG;
 
@@ -661,7 +661,7 @@ static void get_brd_name(struct qlcnic_adapter *adapter, char *name)
 	}
 
 	if (!found)
-		name = "Unknown";
+		sprintf(name, "%pM Gigabit Ethernet", adapter->mac_addr);
 }
 
 static void
-- 
1.6.0.2


^ permalink raw reply related

* [PATCH NEXT 5/5] qlcnic: fix register access
From: Amit Kumar Salecha @ 2010-06-17 12:56 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman
In-Reply-To: <1276779402-496-1-git-send-email-amit.salecha@qlogic.com>

For certain set of register, base window addresses are not defined.
In such cases window should not set.
Return with error for such cases to avoid NMI.

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/qlcnic/qlcnic_hw.c |   21 ++++++++++++++-------
 1 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/net/qlcnic/qlcnic_hw.c b/drivers/net/qlcnic/qlcnic_hw.c
index d9becb9..10ba723 100644
--- a/drivers/net/qlcnic/qlcnic_hw.c
+++ b/drivers/net/qlcnic/qlcnic_hw.c
@@ -766,7 +766,7 @@ qlcnic_pci_get_crb_addr_2M(struct qlcnic_adapter *adapter,
  * Out: 'off' is 2M pci map addr
  * side effect: lock crb window
  */
-static void
+static int
 qlcnic_pci_set_crbwindow_2M(struct qlcnic_adapter *adapter, ulong off)
 {
 	u32 window;
@@ -775,6 +775,10 @@ qlcnic_pci_set_crbwindow_2M(struct qlcnic_adapter *adapter, ulong off)
 	off -= QLCNIC_PCI_CRBSPACE;
 
 	window = CRB_HI(off);
+	if (window == 0) {
+		dev_err(&adapter->pdev->dev, "Invalid offset 0x%lx\n", off);
+		return -EIO;
+	}
 
 	writel(window, addr);
 	if (readl(addr) != window) {
@@ -782,7 +786,9 @@ qlcnic_pci_set_crbwindow_2M(struct qlcnic_adapter *adapter, ulong off)
 			dev_warn(&adapter->pdev->dev,
 				"failed to set CRB window to %d off 0x%lx\n",
 				window, off);
+		return -EIO;
 	}
+	return 0;
 }
 
 int
@@ -803,11 +809,12 @@ qlcnic_hw_write_wx_2M(struct qlcnic_adapter *adapter, ulong off, u32 data)
 		/* indirect access */
 		write_lock_irqsave(&adapter->ahw.crb_lock, flags);
 		crb_win_lock(adapter);
-		qlcnic_pci_set_crbwindow_2M(adapter, off);
-		writel(data, addr);
+		rv = qlcnic_pci_set_crbwindow_2M(adapter, off);
+		if (!rv)
+			writel(data, addr);
 		crb_win_unlock(adapter);
 		write_unlock_irqrestore(&adapter->ahw.crb_lock, flags);
-		return 0;
+		return rv;
 	}
 
 	dev_err(&adapter->pdev->dev,
@@ -821,7 +828,7 @@ qlcnic_hw_read_wx_2M(struct qlcnic_adapter *adapter, ulong off)
 {
 	unsigned long flags;
 	int rv;
-	u32 data;
+	u32 data = -1;
 	void __iomem *addr = NULL;
 
 	rv = qlcnic_pci_get_crb_addr_2M(adapter, off, &addr);
@@ -833,8 +840,8 @@ qlcnic_hw_read_wx_2M(struct qlcnic_adapter *adapter, ulong off)
 		/* indirect access */
 		write_lock_irqsave(&adapter->ahw.crb_lock, flags);
 		crb_win_lock(adapter);
-		qlcnic_pci_set_crbwindow_2M(adapter, off);
-		data = readl(addr);
+		if (!qlcnic_pci_set_crbwindow_2M(adapter, off))
+			data = readl(addr);
 		crb_win_unlock(adapter);
 		write_unlock_irqrestore(&adapter->ahw.crb_lock, flags);
 		return data;
-- 
1.6.0.2


^ permalink raw reply related

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: James Bottomley @ 2010-06-17 13:12 UTC (permalink / raw)
  To: Michael Chan
  Cc: 'FUJITA Tomonori', vapier@gentoo.org,
	netdev@vger.kernel.org, linux-parisc@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <C27F8246C663564A84BB7AB3439772421B79CBCA1F@IRVEXCHCCR01.corp.ad.broadcom.com>

On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote:
> FUJITA Tomonori wrote:
> 
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Date: Thu, 17 Jun 2010 13:06:15 +0900
> > Subject: [PATCH] bnx2: fix dma_get_ops compilation breakage
> >
> > This removes dma_get_ops() prefetch optimization in bnx2.
> >
> > bnx2 uses dma_get_ops() to see if dma_sync_single_for_cpu() is
> > noop. bnx2 does prefetch if it's noop.
> >
> > But dma_get_ops() isn't available on all the architectures (only the
> > architectures that uses dma_map_ops struct have it). Using
> > dma_get_ops() in drivers leads to compilation breakage on many
> > archtectures.
> >
> > Currently, we don't have a way to see if dma_sync_single_for_cpu() is
> > noop. If it can improve the performance notably, we can add the new
> > DMA API for it.
> 
> This prefetch improves performance noticeably when the driver is
> handling incoming 64-byte packets at a sustained rate.

So why not do it unconditionally?  The worst that can happen is that you
pull in a stale cache line which will get cleaned in the dma_sync, thus
slightly degrading performance on incoherent architectures.

Alternatively, come up with a dma prefetch infrastructure ... all you're
really doing is hinting to the architecture that you'll sync this region
next.

James



^ permalink raw reply

* Re: pull request: wireless-2.6 2010-06-16 v2
From: John W. Linville @ 2010-06-17 11:48 UTC (permalink / raw)
  To: sedat.dilek; +Cc: David Miller, linux-wireless, netdev, linux-kernel
In-Reply-To: <AANLkTinbJD3bWYSJ3aZZIDi1cUE_BOvLI-iUQtPF50Hm@mail.gmail.com>

On Thu, Jun 17, 2010 at 12:57:22AM +0200, Sedat Dilek wrote:
> Quick feedback:
> 
> I have applied latest wireless-2.6 master GIT against 2.6.35-rc3:
> 
> wireless-2.6/0001-iwlwifi-serialize-station-management-actions.patch
> wireless-2.6/0002-iwlagn-verify-flow-id-in-compressed-BA-packet.patch
> wireless-2.6/0003-libertas_tf-Fix-warning-in-lbtf_rx-for-stats-struct.patch
> wireless-2.6/0004-p54pci-add-Symbol-AP-300-minipci-adapters-pciid.patch
> wireless-2.6/0005-wireless-orphan-ipw2x00-drivers.patch
> wireless-2.6/0006-iwlwifi-cancel-scan-watchdog-in-iwl_bg_abort_scan.patch
> wireless-2.6/0007-hostap-Protect-against-initialization-interrupt.patch
> wireless-2.6/0008-mac80211-fix-warn-enum-may-be-used-uninitialized.patch
> 
> Unfortunately, I get here an issue with iwl3945 (full dmesg attached):
> 
> [ dmesg ]
> [   26.423844]  [<f924b2f1>] ? iwl_tx_cmd_complete+0x51/0x1c5 [iwlcore]
> ...
> [   26.432470] iwl3945 0000:10:00.0: Can't stop Rx DMA.
> 
> Hope this helps.

I think a full bug report at bugzilla.kernel.org would help more. :-)

In that report, please include the full output of dmesg and a
description of what you were doing when message above appeared.

Thanks,

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: Michael Chan @ 2010-06-17 13:30 UTC (permalink / raw)
  To: 'James Bottomley'
  Cc: 'FUJITA Tomonori', vapier@gentoo.org,
	netdev@vger.kernel.org, linux-parisc@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <1276780336.2789.6.camel@mulgrave.site>

James Bottomley wrote:

> On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote:
> > This prefetch improves performance noticeably when the driver is
> > handling incoming 64-byte packets at a sustained rate.
>
> So why not do it unconditionally?  The worst that can happen
> is that you
> pull in a stale cache line which will get cleaned in the
> dma_sync, thus
> slightly degrading performance on incoherent architectures.

The original patch was an unconditional prefetch.  There was
some discussion that it might not be correct if the DMA wasn't
sync'ed yet on some archs.  If the concensus is that it is ok to
do so, that would be the simplest solution.

>
> Alternatively, come up with a dma prefetch infrastructure ...
> all you're
> really doing is hinting to the architecture that you'll sync
> this region
> next.
>
> James
>
>
>
>


^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: James Bottomley @ 2010-06-17 13:36 UTC (permalink / raw)
  To: Michael Chan
  Cc: 'FUJITA Tomonori', vapier@gentoo.org,
	netdev@vger.kernel.org, linux-parisc@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <C27F8246C663564A84BB7AB3439772421B79CBCA21@IRVEXCHCCR01.corp.ad.broadcom.com>

On Thu, 2010-06-17 at 06:30 -0700, Michael Chan wrote: 
> James Bottomley wrote:
> 
> > On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote:
> > > This prefetch improves performance noticeably when the driver is
> > > handling incoming 64-byte packets at a sustained rate.
> >
> > So why not do it unconditionally?  The worst that can happen
> > is that you
> > pull in a stale cache line which will get cleaned in the
> > dma_sync, thus
> > slightly degrading performance on incoherent architectures.
> 
> The original patch was an unconditional prefetch.  There was
> some discussion that it might not be correct if the DMA wasn't
> sync'ed yet on some archs.  If the concensus is that it is ok to
> do so, that would be the simplest solution.

It's definitely not "correct" in that it may pull in stale data.  But it
should be harmless in that if it does, the subsequent sync will destroy
the cache line (even if it actually pulled in correct data) and prevent
the actual use of the prefetched data being wrong (or indeed being
prefetched at all).

James

^ permalink raw reply

* tcp: sending reset to the already closed socket
From: Konstantin Khorenko @ 2010-06-17 13:37 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: David S. Miller

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

Hi All,

i've found that tcp_close() can be called for an already closed socket, but still sends reset in this case (tcp_send_active_reset()) which seems to be incorrect.
Moreover, a packet with reset is sent with different source port as original port number has been already cleared on socket.
Besides that incrementing stat counter for LINUX_MIB_TCPABORTONCLOSE also does not look correct in this case.

Initially this issue was found on 2.6.18-x RHEL5 kernel, but the same seems to be true for the current mainstream kernel (checked on 2.6.35-rc3).
Please, correct me if i missed something.

How that happens:
1) the server receives a packet for socket in TCP_CLOSE_WAIT state that triggers a tcp_reset():

Call Trace:
 <IRQ>  [<ffffffff8025b9b9>] tcp_reset+0x12f/0x1e8
 [<ffffffff80046125>] tcp_rcv_state_process+0x1c0/0xa08
 [<ffffffff8003eb22>] tcp_v4_do_rcv+0x310/0x37a
 [<ffffffff80028bea>] tcp_v4_rcv+0x74d/0xb43
 [<ffffffff8024ef4c>] ip_local_deliver_finish+0x0/0x259
 [<ffffffff80037131>] ip_local_deliver+0x200/0x2f4
 [<ffffffff8003843c>] ip_rcv+0x64c/0x69f
 [<ffffffff80021d89>] netif_receive_skb+0x4c4/0x4fa
 [<ffffffff80032eca>] process_backlog+0x90/0xec
 [<ffffffff8000cc50>] net_rx_action+0xbb/0x1f1
 [<ffffffff80012d3a>] __do_softirq+0xf5/0x1ce
 [<ffffffff8001147a>] handle_IRQ_event+0x56/0xb0
 [<ffffffff8006334c>] call_softirq+0x1c/0x28
 [<ffffffff80070476>] do_softirq+0x2c/0x85
 [<ffffffff80070441>] do_IRQ+0x149/0x152
 [<ffffffff80062665>] ret_from_intr+0x0/0xa
 <EOI>  [<ffffffff80008a2e>] __handle_mm_fault+0x6cd/0x1303
 [<ffffffff80008903>] __handle_mm_fault+0x5a2/0x1303
 [<ffffffff80033a9d>] cache_free_debugcheck+0x21f/0x22e
 [<ffffffff8006a263>] do_page_fault+0x49a/0x7dc
 [<ffffffff80066487>] thread_return+0x89/0x174
 [<ffffffff800c5aee>] audit_syscall_exit+0x341/0x35c
 [<ffffffff80062e39>] error_exit+0x0/0x84

tcp_rcv_state_process()
...  // (sk_state == TCP_CLOSE_WAIT here)
...
        /* step 2: check RST bit */
        if(th->rst) {
                tcp_reset(sk);
                goto discard;
        }
...
---------------------------------
tcp_rcv_state_process
 tcp_reset
  tcp_done
   tcp_set_state(sk, TCP_CLOSE);
     inet_put_port
      __inet_put_port
       inet_sk(sk)->num = 0;

   sk->sk_shutdown = SHUTDOWN_MASK;


2) After that the process (socket owner) tries to write something to that socket and "inet_autobind" sets a _new_ (which differs from the original!) port number for the socket:

 Call Trace:
  [<ffffffff80255a12>] inet_bind_hash+0x33/0x5f
  [<ffffffff80257180>] inet_csk_get_port+0x216/0x268
  [<ffffffff8026bcc9>] inet_autobind+0x22/0x8f
  [<ffffffff80049140>] inet_sendmsg+0x27/0x57
  [<ffffffff8003a9d9>] do_sock_write+0xae/0xea
  [<ffffffff80226ac7>] sock_writev+0xdc/0xf6
  [<ffffffff800680c7>] _spin_lock_irqsave+0x9/0xe
  [<ffffffff8001fb49>] __pollwait+0x0/0xdd
  [<ffffffff8008d533>] default_wake_function+0x0/0xe
  [<ffffffff800a4f10>] autoremove_wake_function+0x0/0x2e
  [<ffffffff800f0b49>] do_readv_writev+0x163/0x274
  [<ffffffff80066538>] thread_return+0x13a/0x174
  [<ffffffff800145d8>] tcp_poll+0x0/0x1c9
  [<ffffffff800c56d3>] audit_syscall_entry+0x180/0x1b3
  [<ffffffff800f0dd0>] sys_writev+0x49/0xe4
  [<ffffffff800622dd>] tracesys+0xd5/0xe0


3) sendmsg fails at last with -EPIPE (=> 'write' returns -EPIPE in userspace):

F: tcp_sendmsg1 -EPIPE: sk=ffff81000bda00d0, sport=49847, old_state=7, new_state=7, sk_err=0, sk_shutdown=3

Call Trace:
 [<ffffffff80027557>] tcp_sendmsg+0xcb/0xe87
 [<ffffffff80033300>] release_sock+0x10/0xae
 [<ffffffff8016f20f>] vgacon_cursor+0x0/0x1a7
 [<ffffffff8026bd32>] inet_autobind+0x8b/0x8f
 [<ffffffff8003a9d9>] do_sock_write+0xae/0xea
 [<ffffffff80226ac7>] sock_writev+0xdc/0xf6
 [<ffffffff800680c7>] _spin_lock_irqsave+0x9/0xe
 [<ffffffff8001fb49>] __pollwait+0x0/0xdd
 [<ffffffff8008d533>] default_wake_function+0x0/0xe
 [<ffffffff800a4f10>] autoremove_wake_function+0x0/0x2e
 [<ffffffff800f0b49>] do_readv_writev+0x163/0x274
 [<ffffffff80066538>] thread_return+0x13a/0x174
 [<ffffffff800145d8>] tcp_poll+0x0/0x1c9
 [<ffffffff800c56d3>] audit_syscall_entry+0x180/0x1b3
 [<ffffffff800f0dd0>] sys_writev+0x49/0xe4
 [<ffffffff800622dd>] tracesys+0xd5/0xe0

tcp_sendmsg()
...
        /* Wait for a connection to finish. */
        if ((1 << sk->sk_state) & ~(TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) {
                int old_state = sk->sk_state;
                if ((err = sk_stream_wait_connect(sk, &timeo)) != 0) {
if (f_d && (err == -EPIPE)) {
        printk("F: tcp_sendmsg1 -EPIPE: sk=%p, sport=%u, old_state=%d, new_state=%d, "
                "sk_err=%d, sk_shutdown=%d\n",
                sk, ntohs(inet_sk(sk)->sport), old_state, sk->sk_state,
                sk->sk_err, sk->sk_shutdown);
        dump_stack();
}
                        goto out_err;
                }
        }
...

4) Then the process (socket owner) understands that it's time to close that socket and does that (and thus triggers sending reset packet):

Call Trace:
...
 [<ffffffff80032077>] dev_queue_xmit+0x343/0x3d6
 [<ffffffff80034698>] ip_output+0x351/0x384
 [<ffffffff80251ae9>] dst_output+0x0/0xe
 [<ffffffff80036ec6>] ip_queue_xmit+0x567/0x5d2
 [<ffffffff80095700>] vprintk+0x21/0x33
 [<ffffffff800070f0>] check_poison_obj+0x2e/0x206
 [<ffffffff80013587>] poison_obj+0x36/0x45
 [<ffffffff8025dea6>] tcp_send_active_reset+0x15/0x14d
 [<ffffffff80023481>] dbg_redzone1+0x1c/0x25
 [<ffffffff8025dea6>] tcp_send_active_reset+0x15/0x14d
 [<ffffffff8000ca94>] cache_alloc_debugcheck_after+0x189/0x1c8
 [<ffffffff80023405>] tcp_transmit_skb+0x764/0x786
 [<ffffffff8025df8a>] tcp_send_active_reset+0xf9/0x14d
 [<ffffffff80258ff1>] tcp_close+0x39a/0x960
 [<ffffffff8026be12>] inet_release+0x69/0x80
 [<ffffffff80059b31>] sock_release+0x4f/0xcf
 [<ffffffff80059d4c>] sock_close+0x2c/0x30
 [<ffffffff800133c9>] __fput+0xac/0x197
 [<ffffffff800252bc>] filp_close+0x59/0x61
 [<ffffffff8001eff6>] sys_close+0x85/0xc7
 [<ffffffff800622dd>] tracesys+0xd5/0xe0


So, in brief:
* a received packet for socket in TCP_CLOSE_WAIT state triggers tcp_reset() which clears inet_sk(sk)->num and put socket into TCP_CLOSE state
* an attempt to write to that socket forces inet_autobind() to get a new port (but the write itself fails with -EPIPE)
* tcp_close() called for socket in TCP_CLOSE state sends an active reset via socket with newly allocated port


Please, take a look at this and if nothing is missed, here is a patch with the fix.

Thank you.

--
Best regards,

Konstantin Khorenko

[-- Attachment #2: diff-net-tcp-reset-on-close-20100617 --]
[-- Type: text/plain, Size: 803 bytes --]

net: do not send reset to already closed tcp sockets

This adds an additional check in tcp_close() for already closed
sockets. We do not want to send anything to closed sockets.

Signed-off-by: Konstantin Khorenko <khorenko@openvz.org>
---

--- ./net/ipv4/tcp.c.reset	2010-05-17 01:17:36.000000000 +0400
+++ ./net/ipv4/tcp.c	2010-06-17 16:42:11.000000000 +0400
@@ -1898,6 +1898,10 @@ void tcp_close(struct sock *sk, long tim
 
 	sk_mem_reclaim(sk);
 
+	/* If socket has been already reset (e.g. in tcp_reset()) - kill it. */
+	if (sk->sk_state == TCP_CLOSE)
+		goto adjudge_to_death;
+
 	/* As outlined in RFC 2525, section 2.17, we send a RST here because
 	 * data was lost. To witness the awful effects of the old behavior of
 	 * always doing a FIN, run an older 2.1.x kernel or 2.0.x, start a bulk

^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: FUJITA Tomonori @ 2010-06-17 14:05 UTC (permalink / raw)
  To: mchan
  Cc: JBottomley, fujita.tomonori, vapier, netdev, linux-parisc,
	linux-kernel
In-Reply-To: <C27F8246C663564A84BB7AB3439772421B79CBCA21@IRVEXCHCCR01.corp.ad.broadcom.com>

On Thu, 17 Jun 2010 06:30:39 -0700
"Michael Chan" <mchan@broadcom.com> wrote:

> James Bottomley wrote:
> 
> > On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote:
> > > This prefetch improves performance noticeably when the driver is
> > > handling incoming 64-byte packets at a sustained rate.
> >
> > So why not do it unconditionally?  The worst that can happen
> > is that you
> > pull in a stale cache line which will get cleaned in the
> > dma_sync, thus
> > slightly degrading performance on incoherent architectures.
> 
> The original patch was an unconditional prefetch.  There was
> some discussion that it might not be correct if the DMA wasn't
> sync'ed yet on some archs.  If the concensus is that it is ok to
> do so, that would be the simplest solution.

As James said, it just adds useless prefetch on incoherent
architectures. sync_single_for_cpu is called later so we can see the
correct data. One useless prefetch is unlikely to lead performance
drop.

You might prefer this v2.

=
From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Subject: [PATCH v2] bnx2: fix dma_get_ops compilation breakage

This removes dma_get_ops() prefetch optimization in bnx2.

bnx2 uses dma_get_ops() to see if dma_sync_single_for_cpu() is
noop. bnx2 does prefetch if it's noop.

But dma_get_ops() isn't available on all the architectures (only the
architectures that uses dma_map_ops struct have it). Using
dma_get_ops() in drivers leads to compilation breakage on many
archtectures.

This patch removes dma_get_ops() and changes bnx2 to do prefetch on
all the architectures. This adds useless prefetch on incoherent
architectures but this is harmless. It is also unlikely to cause the
performance drop.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
---
 drivers/net/bnx2.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 949d7a9..85f1692 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -3099,12 +3099,10 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi *bnapi, int budget)
 		skb = rx_buf->skb;
 		prefetchw(skb);
 
-		if (!get_dma_ops(&pdev->dev)->sync_single_for_cpu) {
-			next_rx_buf =
-				&rxr->rx_buf_ring[
-					RX_RING_IDX(NEXT_RX_BD(sw_cons))];
-			prefetch(next_rx_buf->desc);
-		}
+		next_rx_buf =
+			&rxr->rx_buf_ring[RX_RING_IDX(NEXT_RX_BD(sw_cons))];
+		prefetch(next_rx_buf->desc);
+
 		rx_buf->skb = NULL;
 
 		dma_addr = dma_unmap_addr(rx_buf, mapping);
-- 
1.6.5


^ permalink raw reply related

* RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag" (802.1p packet)
From: Vladislav Zolotarov @ 2010-06-17 14:08 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Arnd Bergmann, Patrick McHardy, Pedro Garcia,
	netdev@vger.kernel.org, Ben Hutchings
In-Reply-To: <1276770518.2519.1.camel@edumazet-laptop>



> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
> Behalf Of Eric Dumazet
> Sent: Thursday, June 17, 2010 1:29 PM
> To: Vladislav Zolotarov
> Cc: Arnd Bergmann; Patrick McHardy; Pedro Garcia; netdev@vger.kernel.org; Ben
> Hutchings
> Subject: RE: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> (802.1p packet)
> 
> Le jeudi 17 juin 2010 à 01:56 -0700, Vladislav Zolotarov a écrit :
> >
> > > -----Original Message-----
> > > From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On
> > > Behalf Of Eric Dumazet
> > > Sent: Wednesday, June 16, 2010 9:58 PM
> > > To: Arnd Bergmann
> > > Cc: Patrick McHardy; Pedro Garcia; netdev@vger.kernel.org; Ben Hutchings
> > > Subject: Re: [PATCH] vlan_dev: VLAN 0 should be treated as "no vlan tag"
> > > (802.1p packet)
> > >
> > > Le mercredi 16 juin 2010 à 20:26 +0200, Arnd Bergmann a écrit :
> > > > On Wednesday 16 June 2010 17:28:23 Patrick McHardy wrote:
> > > >
> > > > > Since we don't have any special VLAN handling in the bridging code, I
> > > > > guess it comes down to optionally using a different ethertype value
> > > > > (0x88a8) in the VLAN code. We probably also need some indication from
> > > > > device drivers whether they are able to add these headers to avoid
> > > > > trying to offload tagging in case they're not.
> > > >
> > > > It's probably a little more than just supporting the new ethertype, but
> not
> > > > much. The outer tag can be handled like our current VLAN module does,
> > > > but the standard does not allow a regular frame to be encapsulated
> > > directly,
> > > > but rather requires one of
> > > >
> > > > 1. In 802.1ad: an 802.1Q VLAN tag (ethertype 0x8100) followed by the
> frame
> > > > 2. In 802.1ah: A service tag (ethertype 0x88e7) followed by the 802.1Q
> VLAN
> > > tag
> > > >    and then the frame.
> > > >
> > > > Maybe what we can do is extend the vlan code to understand all three
> frame
> > > > formats (q, ad and ah) or at least the first two so we configure both
> the
> > > > provider VID and the Customer VID for the interface in case of 802.1ad
> but
> > > > only the regular VID in 802.1Q.
> > > >
> > > > Device drivers can then flag whether they support both formats or just
> > > > the regular Q tag.
> > > >
> > > > 	Arnd
> > >
> > > Speaking of device drivers, I see bnx2 (hardware accelerated) is able to
> > > insert a 8021q tag in case no vlgrp is defined (the 8201q tag that was
> > > removed by NIC)... interesting ping pong games, since our 8021q stack
> > > will remove it again, eventually.
> > >
> > > So VLAN 0 'problem' on bnx2 could be solved with following patch
> > > (avoiding this insert if vtag==0)
> > >
> > >
> > >
> > > diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> > > index 522de9f..b5d4d05 100644
> > > --- a/drivers/net/bnx2.c
> > > +++ b/drivers/net/bnx2.c
> > > @@ -3192,7 +3192,7 @@ bnx2_rx_int(struct bnx2 *bp, struct bnx2_napi
> *bnapi,
> > > int budget)
> > >  				hw_vlan = 1;
> > >  			else
> > >  #endif
> > > -			{
> > > +			if (vtag) {
> > >  				struct vlan_ethhdr *ve = (struct vlan_ethhdr *)
> > >  					__skb_push(skb, 4);
> > >
> > >
> > >
> > > --
> >
> > This way u will loose all the priority information that was on the VLAN
> header.
> 
> 16bits vtag = 0 : there is no priority information.

According to IEEE 802.1p PCP=0 is legal priority, CFI bit is usually zero. So, VTAG=0 would mark a priority tagged frame with a priority 0 and it should be handled differently than a frame with no priority at all and your patch will prevent it.

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


^ permalink raw reply

* Re: [PATCH] socketcan: add a driver for FlexCAN controllers.
From: Marc Kleine-Budde @ 2010-06-17 14:10 UTC (permalink / raw)
  To: Hans J. Koch
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100617105201.GA2015-hikPBsva6T+Nj9Bq2fkWzw@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 26250 bytes --]

Hey Hans,

Hans J. Koch wrote:
> This adds a driver for FlexCAN based CAN controllers,
> e.g. found in Freescale i.MX35 SoCs.
> 
> The original version of this driver was posted by Sascha Hauer in July 2009:
> http://kerneltrap.org/mailarchive/linux-netdev/2009/7/29/6251621
> 
> I took this version, added NAPI support, and fixed some problems found
> during testing. Well, here is the result. Please review.

I got busy, involved in a $CUSTOMER project, but now I have time to
review your patch...I'm going to send mine, too.

> Thanks,
> Hans
> 
> Signed-off-by: Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> ---
>  drivers/net/can/Kconfig   |    6 +
>  drivers/net/can/Makefile  |    1 +
>  drivers/net/can/flexcan.c |  828 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 835 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/net/can/flexcan.c
> 
> diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
> index 2c5227c..4250c99 100644
> --- a/drivers/net/can/Kconfig
> +++ b/drivers/net/can/Kconfig
> @@ -73,6 +73,12 @@ config CAN_JANZ_ICAN3
>  	  This driver can also be built as a module. If so, the module will be
>  	  called janz-ican3.ko.
>  
> +config CAN_FLEXCAN
> +	tristate "Support for Freescale FLEXCAN based chips"
> +	depends on CAN_DEV
> +	---help---
> +	  Driver for Freescale FlexCAN.
> +
>  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 9047cd0..0057537 100644
> --- a/drivers/net/can/Makefile
> +++ b/drivers/net/can/Makefile
> @@ -16,5 +16,6 @@ obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
>  obj-$(CONFIG_CAN_MCP251X)	+= mcp251x.o
>  obj-$(CONFIG_CAN_BFIN)		+= bfin_can.o
>  obj-$(CONFIG_CAN_JANZ_ICAN3)	+= janz-ican3.o
> +obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
>  
>  ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> new file mode 100644
> index 0000000..ab00873
> --- /dev/null
> +++ b/drivers/net/can/flexcan.c
> @@ -0,0 +1,828 @@
> +/*
> + * FLEXCAN CAN controller driver
> + *
> + * Copyright (C) 2005-2006 Varma Electronics Oy
> + * Copyright (C) 2009 Sascha Hauer, Pengutronix
> + * Copyright (C) 2010 Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
> + *
> + * Based on code originally by Andrey Volkov
> + *
> + * Licensed under the terms of the GPL v2.
> + *
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/if_arp.h>
> +#include <linux/if_ether.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/netdevice.h>
> +#include <linux/platform_device.h>
> +
> +#include <linux/can/dev.h>
> +#include <linux/can/error.h>
> +#include <linux/can/netlink.h>
> +
> +#define DRIVER_NAME "flexcan"
> +
> +#define TX_ECHO_SKB_MAX			1
> +#define FLEXCAN_DEF_NAPI_WEIGHT		6
> +
> +/* FLEXCAN module configuration register (CANMCR) bits */
> +#define CANMCR_MDIS				(1 << 31)
> +#define CANMCR_FRZ				(1 << 30)
> +#define CANMCR_FEN				(1 << 29)
> +#define CANMCR_HALT				(1 << 28)
> +#define CANMCR_NOT_RDY				(1 << 27)
> +#define CANMCR_SOFTRST				(1 << 25)
> +#define CANMCR_FRZACK				(1 << 24)
> +#define CANMCR_SUPV				(1 << 23)
> +#define CANMCR_SRX_DIS				(1 << 17)
> +#define CANMCR_MAXMB(x)				((x) & 0x0f)
> +#define CANMCR_IDAM_A				(0 << 8)
> +#define CANMCR_IDAM_B				(1 << 8)
> +#define CANMCR_IDAM_C				(2 << 8)
> +
> +/* FLEXCAN control register (CANCTRL) bits */
> +#define CANCTRL_PRESDIV(x)			(((x) & 0xff) << 24)
> +#define CANCTRL_RJW(x)				(((x) & 0x03) << 22)
> +#define CANCTRL_PSEG1(x)			(((x) & 0x07) << 19)
> +#define CANCTRL_PSEG2(x)			(((x) & 0x07) << 16)
> +#define CANCTRL_BOFFMSK				(1 << 15)
> +#define CANCTRL_ERRMSK				(1 << 14)
> +#define CANCTRL_CLKSRC				(1 << 13)
> +#define CANCTRL_LPB				(1 << 12)
> +#define CANCTRL_TWRN_MSK			(1 << 11)
> +#define CANCTRL_RWRN_MSK			(1 << 10)
> +#define CANCTRL_SAMP				(1 << 7)
> +#define CANCTRL_BOFFREC				(1 << 6)
> +#define CANCTRL_TSYNC				(1 << 5)
> +#define CANCTRL_LBUF				(1 << 4)
> +#define CANCTRL_LOM				(1 << 3)
> +#define CANCTRL_PROPSEG(x)			((x) & 0x07)
> +
> +/* FLEXCAN error counter register (ERRCNT) bits */
> +#define ERRCNT_REXECTR(x)			(((x) & 0xff) << 8)
> +#define ERRCNT_TXECTR(x)			((x) & 0xff)
> +
> +/* FLEXCAN error and status register (ERRSTAT) bits */
> +#define ERRSTAT_TWRNINT				(1 << 17)
> +#define ERRSTAT_RWRNINT				(1 << 16)
> +#define ERRSTAT_BIT1ERR				(1 << 15)
> +#define ERRSTAT_BIT0ERR				(1 << 14)
> +#define ERRSTAT_ACKERR				(1 << 13)
> +#define ERRSTAT_CRCERR				(1 << 12)
> +#define ERRSTAT_FRMERR				(1 << 11)
> +#define ERRSTAT_STFERR				(1 << 10)
> +#define ERRSTAT_TXWRN				(1 << 9)
> +#define ERRSTAT_RXWRN				(1 << 8)
> +#define ERRSTAT_IDLE				(1 << 7)
> +#define ERRSTAT_TXRX				(1 << 6)
> +#define ERRSTAT_FLTCONF_MASK			(3 << 4)
> +#define ERRSTAT_FLTCONF_ERROR_ACTIVE		(0 << 4)
> +#define ERRSTAT_FLTCONF_ERROR_PASSIVE		(1 << 4)
> +#define ERRSTAT_FLTCONF_ERROR_BUS_OFF		(2 << 4)
> +#define ERRSTAT_BOFFINT				(1 << 2)
> +#define ERRSTAT_ERRINT				(1 << 1)
> +#define ERRSTAT_WAKINT				(1 << 0)
> +#define ERRSTAT_INT	(ERRSTAT_BOFFINT | ERRSTAT_ERRINT | ERRSTAT_TWRNINT | \
> +				ERRSTAT_RWRNINT)
> +
> +/* FLEXCAN interrupt flag register (IFLAG) bits */
> +#define IFLAG_BUF(x)				(1 << (x))
> +#define IFLAG_RX_FIFO_OVERFLOW			(1 << 7)
> +#define IFLAG_RX_FIFO_WARN			(1 << 6)
> +#define IFLAG_RX_FIFO_AVAILABLE			(1 << 5)
> +
> +/* FLEXCAN message buffers */
> +#define MB_CNT_CODE(x)				(((x) & 0xf) << 24)
> +#define MB_CNT_SRR				(1 << 22)
> +#define MB_CNT_IDE				(1 << 21)
> +#define MB_CNT_RTR				(1 << 20)
> +#define MB_CNT_LENGTH(x)			(((x) & 0xf) << 16)
> +#define MB_CNT_TIMESTAMP(x)			((x) & 0xffff)
> +
> +#define MB_ID_STD				(0x7ff << 18)
> +#define MB_ID_EXT				0x1fffffff
> +#define MB_CODE_MASK				0xf0ffffff
> +
> +#define TX_ECHO_SKB_MAX				1
> +
> +/* Structure of the message buffer */
> +struct flexcan_mb {
> +	u32	can_ctrl;
> +	u32	can_id;
> +	u32	data[2];
> +};
> +
> +/* Structure of the hardware registers */
> +struct flexcan_regs {
> +	u32	canmcr;		/* 0x00 */
> +	u32	canctrl;	/* 0x04 */
> +	u32	timer;		/* 0x08 */
> +	u32	reserved1;	/* 0x0c */
> +	u32	rxgmask;	/* 0x10 */
> +	u32	rx14mask;	/* 0x14 */
> +	u32	rx15mask;	/* 0x18 */
> +	u32	errcnt;		/* 0x1c */
> +	u32	errstat;	/* 0x20 */
> +	u32	imask2;		/* 0x24 */
> +	u32	imask1;		/* 0x28 */
> +	u32	iflag2;		/* 0x2c */
> +	u32	iflag1;		/* 0x30 */
> +	u32	reserved4[19];
> +	struct	flexcan_mb cantxfg[64];
> +};
> +
> +struct flexcan_priv {
> +	struct can_priv can;
> +	void __iomem *base;
> +
> +	struct net_device *dev;
> +	struct napi_struct napi;
> +	struct clk *clk;
> +};
> +
> +static struct can_bittiming_const flexcan_bittiming_const = {
> +	.name = DRIVER_NAME,
> +	.tseg1_min = 1,
> +	.tseg1_max = 16,
> +	.tseg2_min = 2,
> +	.tseg2_max = 8,
> +	.sjw_max = 4,
> +	.brp_min = 1,
> +	.brp_max = 256,
> +	.brp_inc = 1,
> +};
> +
> +/* Mailboxes 0..7 are for RX FIFO, 8 for TX */
> +#define TX_BUF_ID	8
> +
> +static void disable_mode_on(struct flexcan_regs __iomem *regs)
> +{
> +	u32 reg = readl(&regs->canmcr);
> +
> +	writel(reg | CANMCR_MDIS, &regs->canmcr);
> +	udelay(100);
> +}
> +
> +static void disable_mode_off(struct flexcan_regs __iomem *regs)
> +{
> +	u32 reg = readl(&regs->canmcr);
> +
> +	writel(reg & ~CANMCR_MDIS, &regs->canmcr);
> +	udelay(100);
> +}
> +
> +static int freeze_mode_on(struct flexcan_regs __iomem *regs)
> +{
> +	u32 reg = readl(&regs->canmcr);
> +	int timeout = 10000;
> +
> +	if (reg & CANMCR_FRZACK)
> +		return 0;
> +
> +	writel(reg | CANMCR_FRZ, &regs->canmcr);
> +	writel(reg | CANMCR_HALT, &regs->canmcr);
> +	while (!(reg & CANMCR_FRZACK)) {
> +		if (--timeout < 0)
> +			return -EIO;
> +		udelay(10);
> +		reg = readl(&regs->canmcr);
> +	}
> +	return 0;
> +}
> +
> +static int freeze_mode_off(struct flexcan_regs __iomem *regs)
> +{
> +	u32 reg = readl(&regs->canmcr);
> +	int timeout = 10000;
> +
> +	if (!(reg & CANMCR_FRZACK))
> +		return 0;
> +
> +	writel(reg & ~(CANMCR_FRZ | CANMCR_HALT), &regs->canmcr);
> +	while (reg & CANMCR_FRZACK) {
> +		if (--timeout < 0)
> +			return -EIO;
> +		udelay(10);
> +		reg = readl(&regs->canmcr);
> +	}
> +	return 0;
> +}
> +
> +static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
> +{
> +	struct can_frame *frame = (struct can_frame *)skb->data;
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 can_id;
> +	u32 ctrl = MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
> +	u32 reg = readl(&regs->canctrl);
> +

check for invalid can frames with can_dropped_invalid_skb

> +	reg = readl(&regs->cantxfg[TX_BUF_ID].can_ctrl);
> +
> +	if (reg != MB_CNT_CODE(0x8))
> +		writel(MB_CNT_CODE(0x08), &regs->cantxfg[TX_BUF_ID].can_ctrl);
> +
> +	netif_stop_queue(dev);
> +
> +	if (frame->can_id & CAN_EFF_FLAG) {
> +		can_id = frame->can_id & CAN_EFF_MASK;
> +		ctrl |= MB_CNT_IDE | MB_CNT_SRR;
> +	} else {
> +		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
> +	}
> +
> +	if (frame->can_id & CAN_RTR_FLAG)
> +		ctrl |= MB_CNT_RTR;
> +
> +	if (frame->can_dlc > 0) {
> +		u32 data;
> +		data = frame->data[0] << 24;
> +		data |= frame->data[1] << 16;
> +		data |= frame->data[2] << 8;
> +		data |= frame->data[3];
> +		writel(data, &regs->cantxfg[TX_BUF_ID].data[0]);

we can use cpu_to_be32 here

> +	}
> +	if (frame->can_dlc > 3) {
> +		u32 data;
> +		data = frame->data[4] << 24;
> +		data |= frame->data[5] << 16;
> +		data |= frame->data[6] << 8;
> +		data |= frame->data[7];
> +		writel(data, &regs->cantxfg[TX_BUF_ID].data[1]);
> +	}
> +
> +	writel(can_id, &regs->cantxfg[TX_BUF_ID].can_id);
> +	writel(ctrl, &regs->cantxfg[TX_BUF_ID].can_ctrl);
> +
> +	kfree_skb(skb);
> +
> +	return NETDEV_TX_OK;
> +}
> +
> +static void flexcan_rx_frame(struct net_device *ndev,
> +		struct flexcan_mb __iomem *mb)
> +{
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct sk_buff *skb;
> +	struct can_frame *frame;
> +	int ctrl, length;
> +	u32 id;
> +
> +	ctrl = readl(&mb->can_ctrl);
> +	length = (ctrl >> 16) & 0x0f;
> +	if (length > 8) {
> +		stats->rx_dropped++;
> +		return;
> +	}

do not drop, use get_can_dlc()

> +
> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> +	if (!skb) {
> +		stats->rx_dropped++;
> +		return;
> +	}
> +
> +	frame = (struct can_frame *)skb_put(skb,
> +			sizeof(struct can_frame));
> +
> +	frame->can_dlc = length;
> +	id = readl(&mb->can_id);
> +
> +	if (ctrl & MB_CNT_IDE) {
> +		frame->can_id = id & CAN_EFF_MASK;
> +		frame->can_id |= CAN_EFF_FLAG;
> +	} else {
> +		frame->can_id  = (id >> 18) & CAN_SFF_MASK;
> +	}
> +
> +	if (ctrl & MB_CNT_RTR)
> +		frame->can_id |= CAN_RTR_FLAG;
> +
> +	if (length > 0) {
> +		u32 data = readl(&mb->data[0]);
> +		frame->data[0] = (data >> 24) & 0xff;
> +		frame->data[1] = (data >> 16) & 0xff;
> +		frame->data[2] = (data >> 8) & 0xff;
> +		frame->data[3] = data & 0xff;

we can use be32_to_cpu here

> +	}
> +	if (length > 3) {
> +		u32 data = readl(&mb->data[1]);
> +		frame->data[4] = (data >> 24) & 0xff;
> +		frame->data[5] = (data >> 16) & 0xff;
> +		frame->data[6] = (data >> 8) & 0xff;
> +		frame->data[7] = data & 0xff;
> +	}
> +
> +	stats->rx_packets++;
> +	stats->rx_bytes += frame->can_dlc;

> +	skb->dev = ndev;
> +	skb->protocol = __constant_htons(ETH_P_CAN);
> +	skb->ip_summed = CHECKSUM_UNNECESSARY;

these are not needed

> +
> +	netif_rx(skb);

use 	netif_receive_skb(skb) in napi

> +}
> +
> +static int flexcan_rx_poll(struct napi_struct *napi, int quota)
> +{
> +	struct net_device *ndev = napi->dev;
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 iflags, imask;
> +	int num_pkts = 0;
> +
> +	if (!netif_running(ndev))
> +		return 0;
> +
> +	iflags = readl(&regs->iflag1);
> +
> +	while ((iflags & IFLAG_RX_FIFO_AVAILABLE) && (num_pkts < quota)) {
> +		struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
> +
> +		flexcan_rx_frame(ndev, mb);
> +		writel(IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
> +		readl(&regs->timer);
> +		num_pkts++;
> +		iflags = readl(&regs->iflag1);
> +	}
> +
> +	if (num_pkts < quota) {
> +		napi_complete(napi);
> +		/* Re-enable RX mailbox interrupts */
> +		imask = readl(&regs->imask1);
> +		writel(imask | IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);

is this read modify write save without locking against the interrupt
handler?

> +	}
> +
> +	return num_pkts;
> +}
> +
> +static void flexcan_error(struct net_device *ndev, u32 stat)
> +{
> +	struct can_frame *cf;
> +	struct sk_buff *skb;
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	enum can_state state = priv->can.state;
> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
> +
> +	skb = dev_alloc_skb(sizeof(struct can_frame));
> +	if (!skb)
> +		return;
> +
> +	skb->dev = ndev;
> +	skb->protocol = __constant_htons(ETH_P_CAN);
> +	skb->ip_summed = CHECKSUM_UNNECESSARY;
> +
> +	cf = (struct can_frame *)skb_put(skb, sizeof(*cf));
> +	memset(cf, 0, sizeof(*cf));
> +
> +	cf->can_id = CAN_ERR_FLAG;
> +	cf->can_dlc = CAN_ERR_DLC;

use alloc_can_err_skb instead

> +
> +	if (stat & ERRSTAT_RWRNINT) {
> +		error_warning = 1;
> +		cf->data[1] |= CAN_ERR_CRTL_RX_WARNING;
> +	}
> +
> +	if (stat & ERRSTAT_TWRNINT) {
> +		error_warning = 1;
> +		cf->data[1] |= CAN_ERR_CRTL_TX_WARNING;
> +	}
> +
> +	switch ((stat >> 4) & 0x3) {
> +	case 0:
> +		state = CAN_STATE_ERROR_ACTIVE;
> +		break;
> +	case 1:
> +		state = CAN_STATE_ERROR_PASSIVE;
> +		break;
> +	default:
> +		state = CAN_STATE_BUS_OFF;
> +		break;
> +	}
> +
> +	if (stat & ERRSTAT_BOFFINT) {
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		state = CAN_STATE_BUS_OFF;
> +	}
> +
> +	if (stat & ERRSTAT_BIT1ERR) {
> +		rx_errors = 1;
> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +		cf->data[2] |= CAN_ERR_PROT_BIT1;
> +	}
> +
> +	if (stat & ERRSTAT_BIT0ERR) {
> +		rx_errors = 1;
> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +		cf->data[2] |= CAN_ERR_PROT_BIT0;
> +	}
> +
> +	if (stat & ERRSTAT_FRMERR) {
> +		rx_errors = 1;
> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +		cf->data[2] |= CAN_ERR_PROT_FORM;
> +	}
> +
> +	if (stat & ERRSTAT_STFERR) {
> +		rx_errors = 1;
> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
> +		cf->data[2] |= CAN_ERR_PROT_STUFF;
> +	}
> +
> +
> +	if (stat & ERRSTAT_ACKERR) {
> +		tx_errors = 1;
> +		cf->can_id |= CAN_ERR_ACK;
> +	}
> +
> +	if (state == CAN_STATE_BUS_OFF)
> +		can_bus_off(ndev);
> +	if (error_warning)
> +		priv->can.can_stats.error_warning++;
> +	if (rx_errors)
> +		stats->rx_errors++;
> +	if (tx_errors)
> +		stats->tx_errors++;
> +
> +	priv->can.state = state;
> +
> +	netif_rx(skb);
> +
> +	ndev->last_rx = jiffies;

IIRC not needed anymore

> +	stats->rx_packets++;
> +	stats->rx_bytes += cf->can_dlc;
> +}
> +
> +static irqreturn_t flexcan_isr(int irq, void *dev_id)
> +{
> +	struct net_device *ndev = dev_id;
> +	struct net_device_stats *stats = &ndev->stats;
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 iflags, imask, errstat;
> +
> +	errstat = readl(&regs->errstat);
> +	if (errstat & ERRSTAT_INT) {
> +		flexcan_error(ndev, errstat);
> +		writel(errstat & ERRSTAT_INT, &regs->errstat);

IMHO the bit errors should be handled in NAPI to avoid IRQ storms.

> +	}
> +
> +	iflags = readl(&regs->iflag1);
> +
> +	if (iflags & IFLAG_RX_FIFO_OVERFLOW) {
> +		writel(IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
> +		stats->rx_over_errors++;
> +		stats->rx_errors++;
> +	}
> +
> +	if (iflags & (1 << TX_BUF_ID)) {
> +		stats->tx_packets++;
> +		writel((1 << TX_BUF_ID), &regs->iflag1);
> +		netif_wake_queue(ndev);
> +	}
> +
> +	if (iflags & IFLAG_RX_FIFO_AVAILABLE) {
> +		/* disable RX interrupts */
> +		imask = readl(&regs->imask1);
> +		writel(imask & ~IFLAG_RX_FIFO_AVAILABLE, &regs->imask1);
> +		/* Let NAPI poll received packets */
> +		napi_schedule(&priv->napi);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void init_regs(struct flexcan_regs __iomem *regs)
> +{
> +	u32 reg;
> +	int i;
> +
> +	freeze_mode_on(regs);
> +
> +	/* Enable error and bus off interrupt */
> +	reg = readl(&regs->canctrl);
> +	reg |= CANCTRL_CLKSRC | CANCTRL_ERRMSK | CANCTRL_BOFFMSK |
> +		CANCTRL_BOFFREC | CANCTRL_TWRN_MSK | CANCTRL_TWRN_MSK;
> +	writel(reg, &regs->canctrl);
> +
> +	/* Set lowest buffer transmitted first */
> +	reg |= CANCTRL_LBUF;
> +	writel(reg, &regs->canctrl);
> +
> +	for (i = 0; i < 64; i++) {
> +		writel(0, &regs->cantxfg[i].can_ctrl);
> +		writel(0, &regs->cantxfg[i].can_id);
> +		writel(0, &regs->cantxfg[i].data[0]);
> +		writel(0, &regs->cantxfg[i].data[1]);
> +
> +		/* Put MB into rx queue */
> +		writel(MB_CNT_CODE(0x04), &regs->cantxfg[i].can_ctrl);
> +	}
> +	writel(MB_CNT_CODE(0x08), &regs->cantxfg[TX_BUF_ID].can_ctrl);
> +
> +	/* acceptance mask/acceptance code (accept everything) */
> +	writel(0x0, &regs->rxgmask);
> +	writel(0x0, &regs->rx14mask);
> +	writel(0x0, &regs->rx15mask);
> +
> +	reg = readl(&regs->canmcr) & ~0x0f;
> +	reg |= CANMCR_IDAM_C | CANMCR_FEN | CANMCR_MAXMB(TX_BUF_ID);
> +	writel(reg, &regs->canmcr);
> +}
> +
> +static int flexcan_set_bittiming(struct net_device *ndev)
> +{
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct can_bittiming *bt = &priv->can.bittiming;
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg;
> +
> +	clk_enable(priv->clk);
> +
> +	disable_mode_on(regs);
> +
> +	reg = readl(&regs->canctrl);
> +	reg &= ~(CANCTRL_SAMP | CANCTRL_PRESDIV(0xff) |
> +			CANCTRL_PSEG1(7) | CANCTRL_PSEG2(7) |
> +			CANCTRL_PROPSEG(7));
> +	reg |= CANCTRL_PRESDIV(bt->brp - 1) |
> +		CANCTRL_PSEG1(bt->phase_seg1 - 1) |
> +		CANCTRL_PSEG2(bt->phase_seg2 - 1) |
> +		CANCTRL_RJW(3) |
                            ^

is this intended?

> +		CANCTRL_PROPSEG(bt->prop_seg - 1);
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
> +		reg |= CANCTRL_SAMP;
> +	writel(reg, &regs->canctrl);
> +
> +	dev_dbg(&ndev->dev, "flexcan_set_bittiming: canctrl=0x%08x\n", reg);
> +
> +	clk_disable(priv->clk);
> +
> +	return 0;
> +}
> +
> +static int flexcan_open(struct net_device *ndev)
> +{
> +	int ret;
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +
> +	clk_enable(priv->clk);
> +
> +	ret = open_candev(ndev);
> +	if (ret)
> +		return ret;
> +
> +	disable_mode_off(regs);
> +	init_regs(regs);
> +
> +	/* Enable flexcan module */
> +	freeze_mode_off(regs);
> +
> +	/* Enable interrupts */
> +	writel(IFLAG_RX_FIFO_OVERFLOW | IFLAG_RX_FIFO_AVAILABLE |
> +			IFLAG_BUF(TX_BUF_ID),
> +			&regs->imask1);
> +
> +	napi_enable(&priv->napi);
> +	netif_start_queue(ndev);
> +
> +	ret = request_irq(ndev->irq, flexcan_isr, 0, DRIVER_NAME, ndev);
> +	if (!ret)
> +		return 0;
> +
> +	close_candev(ndev);
> +	return ret;
> +}
> +
> +static int flexcan_close(struct net_device *ndev)
> +{
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +
> +	netif_stop_queue(ndev);
> +	napi_disable(&priv->napi);
> +
> +	/* Disable all interrupts */
> +	writel(0, &regs->imask1);
> +	free_irq(ndev->irq, ndev);
> +
> +	close_candev(ndev);
> +
> +	/* Disable module */
> +	disable_mode_on(regs);
> +	clk_disable(priv->clk);
> +	return 0;
> +}
> +
> +static int flexcan_set_mode(struct net_device *ndev, enum can_mode mode)
> +{
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg;
> +
> +	switch (mode) {
> +	case CAN_MODE_START:
> +		reg = readl(&regs->canctrl);
> +		reg &= ~CANCTRL_BOFFREC;
> +		writel(reg, &regs->canctrl);
> +		reg |= CANCTRL_BOFFREC;
> +		writel(reg, &regs->canctrl);
> +		priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +		if (netif_queue_stopped(ndev))
> +			netif_wake_queue(ndev);
> +		break;
> +
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct net_device_ops flexcan_netdev_ops = {
> +       .ndo_open		= flexcan_open,
> +       .ndo_stop		= flexcan_close,
> +       .ndo_start_xmit		= flexcan_start_xmit,
> +};
> +
> +static int register_flexcandev(struct net_device *ndev)
> +{
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg;
> +
> +	/* Ensure clock is enabled so we can access registers */
> +	clk_enable(priv->clk);
> +	reg = readl(&regs->canmcr);
> +	reg &= ~CANMCR_MDIS;
> +	reg |= CANMCR_FEN;
> +	writel(reg, &regs->canmcr);
> +	init_regs(regs);
> +	udelay(100);
> +
> +	reg = readl(&regs->canmcr);
> +	clk_disable(priv->clk);
> +
> +	/* Currently we only support newer versions of this core featuring
> +	 * a RX FIFO. Older cores found on some Coldfire derivates are not
> +	 * yet supported.
> +	 */
> +	if (!(reg & CANMCR_FEN)) {
> +		dev_err(&ndev->dev, "Could not enable RX FIFO, unsupported "
> +				"core");
> +		return -ENODEV;
> +	}
> +
> +	ndev->flags |= IFF_ECHO; /* we support local echo in hardware */
> +	ndev->netdev_ops = &flexcan_netdev_ops;
> +
> +	return register_candev(ndev);
> +}
> +
> +static void unregister_flexcandev(struct net_device *ndev)
> +{
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct flexcan_regs __iomem *regs = priv->base;
> +	u32 reg;
> +
> +	clk_enable(priv->clk);
> +	reg = readl(&regs->canmcr);
> +	reg |= CANMCR_FRZ | CANMCR_HALT | CANMCR_MDIS;
> +	writel(reg, &regs->canmcr);
> +	clk_disable(priv->clk);
> +
> +	unregister_candev(ndev);
> +}
> +
> +static int __devinit flexcan_probe(struct platform_device *pdev)
> +{
> +	struct resource *mem;
> +	struct net_device *ndev;
> +	struct flexcan_priv *priv;
> +	u32 mem_size;
> +	int ret;
> +
> +	ndev = alloc_candev(sizeof(struct flexcan_priv), TX_ECHO_SKB_MAX);
> +	if (!ndev) {
> +		dev_err(&pdev->dev, "flexcan: alloc_candev failed.\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv = netdev_priv(ndev);
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	ndev->irq = platform_get_irq(pdev, 0);
> +	if (!mem || !ndev->irq) {
> +		dev_err(&pdev->dev, "flexcan: mem || irq failed.\n");
> +		ret = -ENODEV;
> +		goto failed_req;
> +	}
> +
> +	mem_size = resource_size(mem);
> +
> +	if (!request_mem_region(mem->start, mem_size, DRIVER_NAME)) {
> +		dev_err(&pdev->dev, "flexcan: request_mem_region failed.\n");
> +		ret = -EBUSY;
> +		goto failed_req;
> +	}
> +
> +	SET_NETDEV_DEV(ndev, &pdev->dev);
> +
> +	priv->base = ioremap(mem->start, mem_size);
> +	if (!priv->base) {
> +		dev_err(&pdev->dev, "flexcan: ioremap failed.\n");
> +		ret = -ENOMEM;
> +		goto failed_map;
> +	}
> +
> +	priv->clk = clk_get(&pdev->dev, NULL);
> +	if (IS_ERR(priv->clk)) {
> +		dev_err(&pdev->dev, "flexcan: clk_get failed.\n");
> +		ret = PTR_ERR(priv->clk);
> +		goto failed_clock;
> +	}
> +	priv->can.clock.freq = clk_get_rate(priv->clk);
> +
> +	platform_set_drvdata(pdev, ndev);
> +
> +	priv->can.do_set_bittiming = flexcan_set_bittiming;

using this callback is error prone, with respect to 3-sample and
friends, see discussion on socketcan-core.

> +	priv->can.bittiming_const = &flexcan_bittiming_const;
> +	priv->can.do_set_mode = flexcan_set_mode;
> +	priv->can.restart_ms = 500;

should this be set in the driver?

> +
> +	netif_napi_add(ndev, &priv->napi, flexcan_rx_poll,
> +				FLEXCAN_DEF_NAPI_WEIGHT);
> +
> +	ret = register_flexcandev(ndev);
> +	if (ret) {
> +		dev_err(&pdev->dev, "flexcan: register_flexcandev failed.\n");
> +		goto failed_register;
> +	}
> +
> +	dev_info(&pdev->dev, "flexcan: probe() succeeded...\n");
> +	return 0;
> +
> +failed_register:
> +	clk_put(priv->clk);
> +failed_clock:
> +	iounmap(priv->base);
> +failed_map:
> +	release_mem_region(mem->start, mem_size);
> +failed_req:
> +	free_candev(ndev);
> +
> +	return ret;
> +}
> +
> +static int __devexit flexcan_remove(struct platform_device *pdev)
> +{
> +	struct net_device *ndev = platform_get_drvdata(pdev);
> +	struct flexcan_priv *priv = netdev_priv(ndev);
> +	struct resource *mem;
> +
> +	unregister_flexcandev(ndev);
> +	netif_napi_del(&priv->napi);
> +	platform_set_drvdata(pdev, NULL);
> +	iounmap(priv->base);
> +	clk_put(priv->clk);
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	release_mem_region(mem->start, resource_size(mem));
> +	free_candev(ndev);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver flexcan_driver = {
> +	.driver = {
> +		   .name = DRIVER_NAME,
> +		   },
> +	.probe = flexcan_probe,
> +	.remove = __devexit_p(flexcan_remove),
> +};
> +
> +static int __init flexcan_init(void)
> +{
> +	return platform_driver_register(&flexcan_driver);
> +}
> +
> +static void __exit flexcan_exit(void)
> +{
> +	platform_driver_unregister(&flexcan_driver);
> +}
> +
> +module_init(flexcan_init);
> +module_exit(flexcan_exit);
> +
> +MODULE_AUTHOR("Hans J. Koch <hjk-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("SocketCAN driver for FlexCAN based chips");

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 #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* [PATCH] CAN: Add Flexcan CAN controller driver
From: Marc Kleine-Budde @ 2010-06-17 14:21 UTC (permalink / raw)
  To: socketcan-core-0fE9KPoRgkgATYTw5x5z8w
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde,
	wg-5Yr1BZd7O62+XT7JhA+gdA

From: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

This core is found on some Freescale SoCs and also some Coldfire
SoCs. Support for Coldfire is missing though at the moment as
they have an older revision of the core which does not have RX FIFO
support.

Signed-off-by: Sascha Hauer <s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Signed-off-by: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/net/can/Kconfig              |    6 +
 drivers/net/can/Makefile             |    1 +
 drivers/net/can/flexcan.c            | 1010 ++++++++++++++++++++++++++++++++++
 include/linux/can/platform/flexcan.h |   20 +
 4 files changed, 1037 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/can/flexcan.c
 create mode 100644 include/linux/can/platform/flexcan.h

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 05b7517..3d932a4 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -63,6 +63,12 @@ config CAN_BFIN
 	  To compile this driver as a module, choose M here: the
 	  module will be called bfin_can.
 
+config CAN_FLEXCAN
+	tristate "Support for Freescale FLEXCAN based chips"
+	depends on CAN_DEV
+	---help---
+	  Say Y here if you want to support for Freescale FlexCAN.
+
 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 7a702f2..5bf3621 100644
--- a/drivers/net/can/Makefile
+++ b/drivers/net/can/Makefile
@@ -15,5 +15,6 @@ obj-$(CONFIG_CAN_AT91)		+= at91_can.o
 obj-$(CONFIG_CAN_TI_HECC)	+= ti_hecc.o
 obj-$(CONFIG_CAN_MCP251X)	+= mcp251x.o
 obj-$(CONFIG_CAN_BFIN)		+= bfin_can.o
+obj-$(CONFIG_CAN_FLEXCAN)	+= flexcan.o
 
 ccflags-$(CONFIG_CAN_DEBUG_DEVICES) := -DDEBUG
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
new file mode 100644
index 0000000..f341989
--- /dev/null
+++ b/drivers/net/can/flexcan.c
@@ -0,0 +1,1010 @@
+/*
+ * flexcan.c - FLEXCAN CAN controller driver
+ *
+ * Copyright (c) 2005-2006 Varma Electronics Oy
+ * Copyright (c) 2009 Sascha Hauer, Pengutronix
+ * Copyright (c) 2010 Marc Kleine-Budde, Pengutronix
+ *
+ * Based on code originally by Andrey Volkov <avolkov-ppI4tVfbJvJWk0Htik3J/w@public.gmane.org>
+ *
+ * LICENCE:
+ *  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, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/netdevice.h>
+#include <linux/can.h>
+#include <linux/can/dev.h>
+#include <linux/can/error.h>
+#include <linux/can/platform/flexcan.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/if_arp.h>
+#include <linux/if_ether.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+
+#include <mach/clock.h>
+
+#define DRV_NAME		"flexcan"
+#define FLEXCAN_NAPI_WEIGHT	8
+
+
+/* FLEXCAN module configuration register (CANMCR) bits */
+#define FLEXCAN_MCR_MDIS		BIT(31)
+#define FLEXCAN_MCR_FRZ			BIT(30)
+#define FLEXCAN_MCR_FEN			BIT(29)
+#define FLEXCAN_MCR_HALT		BIT(28)
+#define FLEXCAN_MCR_NOT_RDY		BIT(27)
+#define FLEXCAN_MCR_WAK_MSK		BIT(26)
+#define FLEXCAN_MCR_SOFTRST		BIT(25)
+#define FLEXCAN_MCR_FRZ_ACK		BIT(24)
+#define FLEXCAN_MCR_SUPV		BIT(23)
+#define FLEXCAN_MCR_SLF_WAK		BIT(22)
+#define FLEXCAN_MCR_WRN_EN		BIT(21)
+#define FLEXCAN_MCR_LPM_ACK		BIT(20)
+#define FLEXCAN_MCR_WAK_SRC		BIT(19)
+#define FLEXCAN_MCR_DOZE		BIT(18)
+#define FLEXCAN_MCR_SRX_DIS		BIT(17)
+#define FLEXCAN_MCR_BCC			BIT(16)
+#define FLEXCAN_MCR_LPRIO_EN		BIT(13)
+#define FLEXCAN_MCR_AEN			BIT(12)
+#define FLEXCAN_MCR_MAXMB(x)		((x) & 0xf)
+#define FLEXCAN_MCR_IDAM_A		(0 << 8)
+#define FLEXCAN_MCR_IDAM_B		(1 << 8)
+#define FLEXCAN_MCR_IDAM_C		(2 << 8)
+#define FLEXCAN_MCR_IDAM_D		(3 << 8)
+
+/* FLEXCAN control register (CANCTRL) bits */
+#define FLEXCAN_CTRL_PRESDIV(x)		(((x) & 0xff) << 24)
+#define FLEXCAN_CTRL_RJW(x)		(((x) & 0x03) << 22)
+#define FLEXCAN_CTRL_PSEG1(x)		(((x) & 0x07) << 19)
+#define FLEXCAN_CTRL_PSEG2(x)		(((x) & 0x07) << 16)
+#define FLEXCAN_CTRL_BOFF_MSK		BIT(15)
+#define FLEXCAN_CTRL_ERR_MSK		BIT(14)
+#define FLEXCAN_CTRL_CLK_SRC		BIT(13)
+#define FLEXCAN_CTRL_LPB		BIT(12)
+#define FLEXCAN_CTRL_TWRN_MSK		BIT(11)
+#define FLEXCAN_CTRL_RWRN_MSK		BIT(10)
+#define FLEXCAN_CTRL_SMP		BIT(7)
+#define FLEXCAN_CTRL_BOFF_REC		BIT(6)
+#define FLEXCAN_CTRL_TSYNC		BIT(5)
+#define FLEXCAN_CTRL_LBUF		BIT(4)
+#define FLEXCAN_CTRL_LOM		BIT(3)
+#define FLEXCAN_CTRL_PROPSEG(x)		((x) & 0x07)
+
+/* FLEXCAN error and status register (ESR) bits */
+#define FLEXCAN_ESR_TWRN_INT		BIT(17)
+#define FLEXCAN_ESR_RWRN_INT		BIT(16)
+#define FLEXCAN_ESR_BIT1_ERR		BIT(15)
+#define FLEXCAN_ESR_BIT0_ERR		BIT(14)
+#define FLEXCAN_ESR_ACK_ERR		BIT(13)
+#define FLEXCAN_ESR_CRC_ERR		BIT(12)
+#define FLEXCAN_ESR_FRM_ERR		BIT(11)
+#define FLEXCAN_ESR_STF_ERR		BIT(10)
+#define FLEXCAN_ESR_TX_WRN		BIT(9)
+#define FLEXCAN_ESR_RX_WRN		BIT(8)
+#define FLEXCAN_ESR_IDLE		BIT(7)
+#define FLEXCAN_ESR_TXRX		BIT(6)
+#define FLEXCAN_EST_FLT_CONF_SHIFT	(4)
+#define FLEXCAN_ESR_FLT_CONF_MASK	(0x2 << FLEXCAN_EST_FLT_CONF_SHIFT)
+#define FLEXCAN_ESR_FLT_CONF_ACTIVE	(0x0 << FLEXCAN_EST_FLT_CONF_SHIFT)
+#define FLEXCAN_ESR_FLT_CONF_PASSIVE	(0x1 << FLEXCAN_EST_FLT_CONF_SHIFT)
+#define FLEXCAN_ESR_BOFF_INT		BIT(2)
+#define FLEXCAN_ESR_ERR_INT		BIT(1)
+#define FLEXCAN_ESR_WAK_INT		BIT(0)
+#define FLEXCAN_ESR_ERR_FRAME \
+	(FLEXCAN_ESR_BIT1_ERR | FLEXCAN_ESR_BIT0_ERR | \
+	 FLEXCAN_ESR_ACK_ERR | FLEXCAN_ESR_CRC_ERR | \
+	 FLEXCAN_ESR_FRM_ERR | FLEXCAN_ESR_STF_ERR)
+#define FLEXCAN_ESR_ERR_LINE \
+	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | FLEXCAN_ESR_BOFF_INT)
+
+/* FLEXCAN interrupt flag register (IFLAG) bits */
+#define FLEXCAN_TX_BUF_ID		8
+#define FLEXCAN_IFLAG_BUF(x)		BIT(x)
+#define FLEXCAN_IFLAG_RX_FIFO_OVERFLOW	BIT(7)
+#define FLEXCAN_IFLAG_RX_FIFO_WARN	BIT(6)
+#define FLEXCAN_IFLAG_RX_FIFO_AVAILABLE	BIT(5)
+#define FLEXCAN_IFLAG_DEFAULT \
+	(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW | FLEXCAN_IFLAG_RX_FIFO_AVAILABLE | \
+	 FLEXCAN_IFLAG_BUF(FLEXCAN_TX_BUF_ID))
+
+/* FLEXCAN message buffers */
+#define FLEXCAN_MB_CNT_CODE(x)		(((x) & 0xf) << 24)
+#define FLEXCAN_MB_CNT_SRR		BIT(22)
+#define FLEXCAN_MB_CNT_IDE		BIT(21)
+#define FLEXCAN_MB_CNT_RTR		BIT(20)
+#define FLEXCAN_MB_CNT_LENGTH(x)	(((x) & 0xf) << 16)
+#define FLEXCAN_MB_CNT_TIMESTAMP(x)	((x) & 0xffff)
+
+#define FLEXCAN_MB_CODE_MASK		(0xf0ffffff)
+
+/* Structure of the message buffer */
+struct flexcan_mb {
+	u32 can_ctrl;
+	u32 can_id;
+	u32 data[2];
+};
+
+/* Structure of the hardware registers */
+struct flexcan_regs {
+	u32 mcr;		/* 0x00 */
+	u32 ctrl;		/* 0x04 */
+	u32 timer;		/* 0x08 */
+	u32 _reserved1;		/* 0x0c */
+	u32 rxgmask;		/* 0x10 */
+	u32 rx14mask;		/* 0x14 */
+	u32 rx15mask;		/* 0x18 */
+	u32 ecr;		/* 0x1c */
+	u32 esr;		/* 0x20 */
+	u32 imask2;		/* 0x24 */
+	u32 imask1;		/* 0x28 */
+	u32 iflag2;		/* 0x2c */
+	u32 iflag1;		/* 0x30 */
+	u32 _reserved2[19];
+	struct flexcan_mb cantxfg[64];
+};
+
+struct flexcan_priv {
+	struct can_priv can;
+	struct net_device *dev;
+	struct napi_struct napi;
+
+	void __iomem *base;
+	u32 reg_esr;
+	u32 reg_ctrl_default;
+
+	struct clk *clk;
+	struct flexcan_platform_data *pdata;
+};
+
+static struct can_bittiming_const flexcan_bittiming_const = {
+	.name = DRV_NAME,
+	.tseg1_min = 4,
+	.tseg1_max = 16,
+	.tseg2_min = 2,
+	.tseg2_max = 8,
+	.sjw_max = 4,
+	.brp_min = 1,
+	.brp_max = 256,
+	.brp_inc = 1,
+};
+
+/*
+ * Swtich transceiver on or off
+ */
+static void flexcan_transceiver_switch(const struct flexcan_priv *priv, int on)
+{
+	if (priv->pdata && priv->pdata->transceiver_switch)
+		priv->pdata->transceiver_switch(on);
+}
+
+static inline void flexcan_chip_enable(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->mcr);
+	reg &= ~FLEXCAN_MCR_MDIS;
+	writel(reg, &regs->mcr);
+
+	udelay(10);
+}
+
+static inline void flexcan_chip_disable(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->mcr);
+	reg |= FLEXCAN_MCR_MDIS;
+	writel(reg, &regs->mcr);
+}
+
+static int flexcan_get_berr_counter(const struct net_device *dev,
+				    struct can_berr_counter *bec)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg = readl(&regs->ecr);
+
+	bec->txerr = (reg >> 0) & 0xff;
+	bec->rxerr = (reg >> 8) & 0xff;
+
+	return 0;
+}
+
+
+static int flexcan_start_xmit(struct sk_buff *skb, struct net_device *dev)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct net_device_stats *stats = &dev->stats;
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct can_frame *frame = (struct can_frame *)skb->data;
+	u32 can_id;
+	u32 ctrl = FLEXCAN_MB_CNT_CODE(0xc) | (frame->can_dlc << 16);
+
+	if (can_dropped_invalid_skb(dev, skb))
+		return NETDEV_TX_OK;
+
+	netif_stop_queue(dev);
+
+	if (frame->can_id & CAN_EFF_FLAG) {
+		can_id = frame->can_id & CAN_EFF_MASK;
+		ctrl |= FLEXCAN_MB_CNT_IDE | FLEXCAN_MB_CNT_SRR;
+	} else {
+		can_id = (frame->can_id & CAN_SFF_MASK) << 18;
+	}
+
+	if (frame->can_id & CAN_RTR_FLAG)
+		ctrl |= FLEXCAN_MB_CNT_RTR;
+
+	if (frame->can_dlc > 0) {
+		u32 data;
+		data = frame->data[0] << 24;
+		data |= frame->data[1] << 16;
+		data |= frame->data[2] << 8;
+		data |= frame->data[3];
+		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[0]);
+	}
+	if (frame->can_dlc > 3) {
+		u32 data;
+		data = frame->data[4] << 24;
+		data |= frame->data[5] << 16;
+		data |= frame->data[6] << 8;
+		data |= frame->data[7];
+		writel(data, &regs->cantxfg[FLEXCAN_TX_BUF_ID].data[1]);
+	}
+
+	writel(can_id, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_id);
+	writel(ctrl, &regs->cantxfg[FLEXCAN_TX_BUF_ID].can_ctrl);
+
+	kfree_skb(skb);
+
+	stats->tx_bytes += frame->can_dlc;
+
+	return NETDEV_TX_OK;
+}
+
+
+static void flexcan_poll_err_frame(struct net_device *dev,
+				   struct can_frame *cf, u32 reg_esr)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	int error_warning = 0, rx_errors = 0, tx_errors = 0;
+
+	if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_BIT1;
+	}
+
+	if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_BIT0;
+	}
+
+	if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_FORM;
+	}
+
+	if (reg_esr & FLEXCAN_ESR_STF_ERR) {
+		rx_errors = 1;
+		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+		cf->data[2] |= CAN_ERR_PROT_STUFF;
+	}
+
+
+	if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
+		tx_errors = 1;
+		cf->can_id |= CAN_ERR_ACK;
+	}
+
+	if (error_warning)
+		priv->can.can_stats.error_warning++;
+	if (rx_errors)
+		dev->stats.rx_errors++;
+	if (tx_errors)
+		dev->stats.tx_errors++;
+
+}
+
+static void flexcan_poll_err(struct net_device *dev, u32 reg_esr)
+{
+	struct sk_buff *skb;
+	struct can_frame *cf;
+
+	skb = alloc_can_err_skb(dev, &cf);
+	if (unlikely(!skb))
+		return;
+
+	flexcan_poll_err_frame(dev, cf, reg_esr);
+	netif_receive_skb(skb);
+
+	dev->stats.rx_packets++;
+	dev->stats.rx_bytes += cf->can_dlc;
+}
+
+static void flexcan_read_fifo(const struct net_device *dev, struct can_frame *cf)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct flexcan_mb __iomem *mb = &regs->cantxfg[0];
+	u32 reg_ctrl, reg_id;
+
+	reg_ctrl = readl(&mb->can_ctrl);
+	reg_id = readl(&mb->can_id);
+	if (reg_ctrl & FLEXCAN_MB_CNT_IDE)
+		cf->can_id = ((reg_id >> 0) & CAN_EFF_MASK) | CAN_EFF_FLAG;
+	else
+		cf->can_id = (reg_id >> 18) & CAN_SFF_MASK;
+
+	if (reg_ctrl & FLEXCAN_MB_CNT_RTR)
+		cf->can_id |= CAN_RTR_FLAG;
+	cf->can_dlc = get_can_dlc((reg_ctrl >> 16) & 0xf);
+
+	*(__be32 *)(cf->data + 0) = cpu_to_be32(readl(&mb->data[0]));
+	*(__be32 *)(cf->data + 4) = cpu_to_be32(readl(&mb->data[1]));
+
+	/* mark as read */
+	writel(FLEXCAN_IFLAG_RX_FIFO_AVAILABLE, &regs->iflag1);
+	readl(&regs->timer);
+}
+
+static void flexcan_read_frame(struct net_device *dev)
+{
+	struct net_device_stats *stats = &dev->stats;
+	struct can_frame *cf;
+	struct sk_buff *skb;
+
+	skb = alloc_can_skb(dev, &cf);
+	if (unlikely(!skb)) {
+		stats->rx_dropped++;
+		return;
+	}
+
+	flexcan_read_fifo(dev, cf);
+	netif_receive_skb(skb);
+
+	stats->rx_packets++;
+	stats->rx_bytes += cf->can_dlc;
+}
+
+static int flexcan_poll(struct napi_struct *napi, int quota)
+{
+	struct net_device *dev = napi->dev;
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_iflag1, reg_esr;
+	int work_done = 0;
+
+	reg_iflag1 = readl(&regs->iflag1);
+
+	/* first handle RX-FIFO */
+	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
+	       work_done < quota) {
+		flexcan_read_frame(dev);
+
+		work_done++;
+		reg_iflag1 = readl(&regs->iflag1);
+	}
+
+	/*
+	 * The error bits are clear on read,
+	 * so use saved value from irq handler.
+	 */
+	reg_esr = readl(&regs->esr) | priv->reg_esr;
+	if (work_done < quota) {
+		flexcan_poll_err(dev, reg_esr);
+		work_done++;
+	}
+
+	if (work_done < quota) {
+		napi_complete(napi);
+		/* enable IRQs */
+		writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+		writel(priv->reg_ctrl_default, &regs->ctrl);
+	}
+
+	return work_done;
+}
+
+static void flexcan_irq_err_state(struct net_device *dev,
+				  struct can_frame *cf, enum can_state new_state)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct can_berr_counter bec;
+
+	flexcan_get_berr_counter(dev, &bec);
+
+	switch (priv->can.state) {
+	case CAN_STATE_ERROR_ACTIVE:
+		/*
+		 * from: ERROR_ACTIVE
+		 * to  : ERROR_WARNING, ERROR_PASSIVE, BUS_OFF
+		 * =>  : there was a warning int
+		 */
+		if (new_state >= CAN_STATE_ERROR_WARNING &&
+		    new_state <= CAN_STATE_BUS_OFF) {
+			dev_dbg(dev->dev.parent, "Error Warning IRQ\n");
+			priv->can.can_stats.error_warning++;
+
+			cf->can_id |= CAN_ERR_CRTL;
+			cf->data[1] = (bec.txerr > bec.rxerr) ?
+				CAN_ERR_CRTL_TX_WARNING :
+				CAN_ERR_CRTL_RX_WARNING;
+		}
+	case CAN_STATE_ERROR_WARNING:	/* fallthrough */
+		/*
+		 * from: ERROR_ACTIVE, ERROR_WARNING
+		 * to  : ERROR_PASSIVE, BUS_OFF
+		 * =>  : error passive int
+		 */
+		if (new_state >= CAN_STATE_ERROR_PASSIVE &&
+		    new_state <= CAN_STATE_BUS_OFF) {
+			dev_dbg(dev->dev.parent, "Error Passive IRQ\n");
+			priv->can.can_stats.error_passive++;
+
+			cf->can_id |= CAN_ERR_CRTL;
+			cf->data[1] = (bec.txerr > bec.rxerr) ?
+				CAN_ERR_CRTL_TX_PASSIVE :
+				CAN_ERR_CRTL_RX_PASSIVE;
+		}
+		break;
+	case CAN_STATE_BUS_OFF:
+		dev_err(dev->dev.parent,
+			"BUG! hardware recovered automatically from BUS_OFF\n");
+		break;
+	default:
+		break;
+	}
+
+	/* process state changes depending on the new state */
+	switch (new_state) {
+	case CAN_STATE_BUS_OFF:
+		cf->can_id |= CAN_ERR_BUSOFF;
+		can_bus_off(dev);
+		break;
+	default:
+		break;
+	}
+}
+
+static void flexcan_irq_err(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	struct sk_buff *skb;
+	struct can_frame *cf;
+	enum can_state new_state;
+	u32 reg_esr;
+	int flt;
+
+	reg_esr = readl(&regs->esr);
+	writel(reg_esr, &regs->esr);
+
+	flt = reg_esr & FLEXCAN_ESR_FLT_CONF_MASK;
+	if (likely(flt == FLEXCAN_ESR_FLT_CONF_ACTIVE)) {
+		if (likely(!(reg_esr & (FLEXCAN_ESR_TX_WRN |
+					FLEXCAN_ESR_RX_WRN))))
+			new_state = CAN_STATE_ERROR_ACTIVE;
+		else
+			new_state = CAN_STATE_ERROR_WARNING;
+	} else if (unlikely(flt == FLEXCAN_ESR_FLT_CONF_PASSIVE))
+		new_state = CAN_STATE_ERROR_PASSIVE;
+	else
+		new_state = CAN_STATE_BUS_OFF;
+
+	/* state hasn't changed */
+	if (likely(new_state == priv->can.state))
+		return;
+
+	skb = alloc_can_err_skb(dev, &cf);
+	if (unlikely(!skb))
+		return;
+
+	flexcan_irq_err_state(dev, cf, new_state);
+	netif_rx(skb);
+
+	dev->stats.rx_packets++;
+	dev->stats.rx_bytes += cf->can_dlc;
+
+	priv->can.state = new_state;
+}
+
+static irqreturn_t flexcan_irq(int irq, void *dev_id)
+{
+	struct net_device *dev = dev_id;
+	struct net_device_stats *stats = &dev->stats;
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg_iflag1, reg_esr;
+
+	reg_iflag1 = readl(&regs->iflag1);
+	reg_esr = readl(&regs->esr);
+
+	/* receive or error interrupt -> napi */
+	if ((reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE) ||
+	    (reg_esr & FLEXCAN_ESR_ERR_FRAME)) {
+		/*
+		 * The error bits are cleared on read,
+		 * save for later use.
+		 */
+		priv->reg_esr = reg_esr;
+		writel(FLEXCAN_IFLAG_DEFAULT & ~FLEXCAN_IFLAG_RX_FIFO_AVAILABLE,
+		       &regs->imask1);
+		writel(priv->reg_ctrl_default & ~FLEXCAN_CTRL_ERR_MSK,
+		       &regs->ctrl);
+		napi_schedule(&priv->napi);
+	}
+
+	/* FIFO overflow */
+	if (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_OVERFLOW) {
+		writel(FLEXCAN_IFLAG_RX_FIFO_OVERFLOW, &regs->iflag1);
+		dev->stats.rx_over_errors++;
+		dev->stats.rx_errors++;
+	}
+
+	/* transmission complete interrupt */
+	if (reg_iflag1 & (1 << FLEXCAN_TX_BUF_ID)) {
+		stats->tx_packets++;
+		writel((1 << FLEXCAN_TX_BUF_ID), &regs->iflag1);
+		netif_wake_queue(dev);
+	}
+
+	/* handle state changes */
+	flexcan_irq_err(dev);
+
+	return IRQ_HANDLED;
+}
+
+static void flexcan_set_bittiming(struct net_device *dev)
+{
+	const struct flexcan_priv *priv = netdev_priv(dev);
+	const struct can_bittiming *bt = &priv->can.bittiming;
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	reg = readl(&regs->ctrl);
+	reg &= ~(FLEXCAN_CTRL_PRESDIV(0xff) |
+		 FLEXCAN_CTRL_RJW(0x3) |
+		 FLEXCAN_CTRL_PSEG1(0x7) |
+		 FLEXCAN_CTRL_PSEG2(0x7) |
+		 FLEXCAN_CTRL_PROPSEG(0x7) |
+		 FLEXCAN_CTRL_LPB |
+		 FLEXCAN_CTRL_SMP |
+		 FLEXCAN_CTRL_LOM);
+
+	reg |= FLEXCAN_CTRL_PRESDIV(bt->brp - 1) |
+		FLEXCAN_CTRL_PSEG1(bt->phase_seg1 - 1) |
+		FLEXCAN_CTRL_PSEG2(bt->phase_seg2 - 1) |
+		FLEXCAN_CTRL_RJW(bt->sjw - 1) |
+		FLEXCAN_CTRL_PROPSEG(bt->prop_seg - 1);
+
+	if (priv->can.ctrlmode & CAN_CTRLMODE_LOOPBACK)
+		reg |= FLEXCAN_CTRL_LPB;
+	if (priv->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
+		reg |= FLEXCAN_CTRL_LOM;
+	if (priv->can.ctrlmode & CAN_CTRLMODE_3_SAMPLES)
+		reg |= FLEXCAN_CTRL_SMP;
+
+	dev_info(dev->dev.parent, "writing ctrl=0x%08x\n", reg);
+	writel(reg, &regs->ctrl);
+
+	/* print chip status */
+	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
+		readl(&regs->mcr), readl(&regs->ctrl));
+}
+
+/*
+ * flexcan_chip_start
+ *
+ * this functions is entered with clocks enabled
+ *
+ */
+static int flexcan_chip_start(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	unsigned int i;
+	int err;
+	u32 reg_mcr, reg_ctrl;
+
+	/* enable module */
+	flexcan_chip_enable(priv);
+
+	/* soft reset */
+	writel(FLEXCAN_MCR_SOFTRST, &regs->mcr);
+	udelay(10);
+
+	reg_mcr = readl(&regs->mcr);
+	if (reg_mcr & FLEXCAN_MCR_SOFTRST) {
+		dev_err(dev->dev.parent,
+			"Failed to softreset can module (mcr=0x%08x)\n", reg_mcr);
+		err = -ENODEV;
+		goto out;
+	}
+
+	flexcan_set_bittiming(dev);
+
+	/*
+	 * MCR
+	 *
+	 * enable freeze
+	 * enable fifo
+	 * halt now
+	 * only supervisor access
+	 * enable warning int
+	 * choose format C
+	 *
+	 */
+	reg_mcr = readl(&regs->mcr);
+	reg_mcr |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_FEN | FLEXCAN_MCR_HALT |
+		FLEXCAN_MCR_SUPV | FLEXCAN_MCR_WRN_EN |
+		FLEXCAN_MCR_IDAM_C;
+	dev_dbg(dev->dev.parent, "%s: writing mcr=0x%08x", __func__, reg_mcr);
+	writel(reg_mcr, &regs->mcr);
+
+	/*
+	 * CTRL
+	 *
+	 * enable bus off interrupt
+	 * disable auto busoff recovery
+	 * enable tx and rx warning interrupt
+	 * transmit lowest buffer first
+	 */
+	reg_ctrl = readl(&regs->ctrl);
+	reg_ctrl |= FLEXCAN_CTRL_BOFF_MSK |FLEXCAN_CTRL_BOFF_REC |
+		FLEXCAN_CTRL_TWRN_MSK | FLEXCAN_CTRL_RWRN_MSK |
+		FLEXCAN_CTRL_LBUF;
+	/*
+	 * TODO: for now turn on the error interrupt, otherwise we
+	 * don't get any warning or bus passive interrupts.
+	 */
+	reg_ctrl |= FLEXCAN_CTRL_ERR_MSK;
+
+	/* save for later use */
+	priv->reg_ctrl_default = reg_ctrl;
+	dev_dbg(dev->dev.parent, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
+	writel(reg_ctrl, &regs->ctrl);
+
+	for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) {
+		writel(0, &regs->cantxfg[i].can_ctrl);
+		writel(0, &regs->cantxfg[i].can_id);
+		writel(0, &regs->cantxfg[i].data[0]);
+		writel(0, &regs->cantxfg[i].data[1]);
+
+		/* put MB into rx queue */
+		writel(FLEXCAN_MB_CNT_CODE(0x4), &regs->cantxfg[i].can_ctrl);
+	}
+
+	/* acceptance mask/acceptance code (accept everything) */
+	writel(0x0, &regs->rxgmask);
+	writel(0x0, &regs->rx14mask);
+	writel(0x0, &regs->rx15mask);
+
+	flexcan_transceiver_switch(priv, 1);
+
+	/* synchronize with the can bus */
+	reg_mcr = readl(&regs->mcr);
+	reg_mcr &= ~FLEXCAN_MCR_HALT;
+	writel(reg_mcr, &regs->mcr);
+
+	priv->can.state = CAN_STATE_ERROR_ACTIVE;
+
+	/* enable FIFO interrupts */
+	writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
+
+	/* print chip status */
+	dev_dbg(dev->dev.parent, "%s: mcr=0x%08x ctrl=0x%08x\n", __func__,
+		readl(&regs->mcr), readl(&regs->ctrl));
+
+	return 0;
+
+ out:
+	flexcan_chip_disable(priv);
+	return err;
+}
+
+/*
+ * flexcan_chip_stop
+ *
+ * this functions is entered with clocks enabled
+ *
+ */
+static void flexcan_chip_stop(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg;
+
+	/* Disable all interrupts */
+	writel(0, &regs->imask1);
+
+	/* Disable + halt module */
+	reg = readl(&regs->mcr);
+	reg |= FLEXCAN_MCR_MDIS | FLEXCAN_MCR_HALT;
+	writel(reg, &regs->mcr);
+
+	flexcan_transceiver_switch(priv, 0);
+	priv->can.state = CAN_STATE_STOPPED;
+
+	return;
+}
+
+static int flexcan_open(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	int err;
+
+	clk_enable(priv->clk);
+
+	err = open_candev(dev);
+	if (err)
+		goto out;
+
+	err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
+	if (err)
+		goto out_close;
+
+	/* start chip and queuing */
+	err = flexcan_chip_start(dev);
+	if (err)
+		goto out_close;
+	napi_enable(&priv->napi);
+	netif_start_queue(dev);
+
+	return 0;
+
+ out_close:
+	close_candev(dev);
+ out:
+	clk_disable(priv->clk);
+
+	return err;
+}
+
+static int flexcan_close(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+
+	netif_stop_queue(dev);
+	napi_disable(&priv->napi);
+	flexcan_chip_stop(dev);
+
+	free_irq(dev->irq, dev);
+	clk_disable(priv->clk);
+
+	close_candev(dev);
+
+	return 0;
+}
+
+static int flexcan_set_mode(struct net_device *dev, enum can_mode mode)
+{
+	int err;
+
+	switch (mode) {
+	case CAN_MODE_START:
+		err = flexcan_chip_start(dev);
+		if (err)
+			return err;
+
+		netif_wake_queue(dev);
+		break;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	return 0;
+}
+
+static const struct net_device_ops flexcan_netdev_ops = {
+	.ndo_open	= flexcan_open,
+	.ndo_stop	= flexcan_close,
+	.ndo_start_xmit	= flexcan_start_xmit,
+};
+
+static int __devinit register_flexcandev(struct net_device *dev)
+{
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct flexcan_regs __iomem *regs = priv->base;
+	u32 reg, err;
+
+	clk_enable(priv->clk);
+
+	/* select "bus clock", chip must be disabled */
+	flexcan_chip_disable(priv);
+	reg = readl(&regs->ctrl);
+	reg |= FLEXCAN_CTRL_CLK_SRC;
+	writel(reg, &regs->ctrl);
+
+	flexcan_chip_enable(priv);
+
+	/* set freeze, halt and activate FIFO, restrict register access */
+	reg = readl(&regs->mcr);
+	reg |= FLEXCAN_MCR_FRZ | FLEXCAN_MCR_HALT |
+		FLEXCAN_MCR_FEN | FLEXCAN_MCR_SUPV;
+	writel(reg, &regs->mcr);
+
+	/*
+	 * Currently we only support newer versions of this core
+	 * featuring a RX FIFO. Older cores found on some Coldfire
+	 * derivates are not yet supported.
+	 */
+	reg = readl(&regs->mcr);
+	if (!(reg & FLEXCAN_MCR_FEN)) {
+		dev_err(dev->dev.parent,
+			"Could not enable RX FIFO, unsupported core\n");
+		err = -ENODEV;
+		goto out;
+	}
+
+	err = register_candev(dev);
+
+ out:
+	/* disable core and turn off clocks */
+	flexcan_chip_disable(priv);
+	clk_disable(priv->clk);
+
+	return err;
+}
+
+static void __devexit unregister_flexcandev(struct net_device *dev)
+{
+	unregister_candev(dev);
+}
+
+static int __devinit flexcan_probe(struct platform_device *pdev)
+{
+	struct net_device *dev;
+	struct flexcan_priv *priv;
+	struct resource *mem;
+	struct clk *clk;
+	void __iomem *base;
+	resource_size_t mem_size;
+	int err, irq;
+
+	clk = clk_get(&pdev->dev, NULL);
+	if (IS_ERR(clk)) {
+		dev_err(&pdev->dev, "no clock defined\n");
+		err = PTR_ERR(clk);
+		goto failed_clock;
+	}
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	irq = platform_get_irq(pdev, 0);
+	if (!mem || irq <= 0) {
+		err = -ENODEV;
+		goto failed_get;
+	}
+
+	mem_size = resource_size(mem);
+	if (!request_mem_region(mem->start, mem_size, pdev->name)) {
+		err = -EBUSY;
+		goto failed_req;
+	}
+
+	base = ioremap(mem->start, mem_size);
+	if (!base) {
+		err = -ENOMEM;
+		goto failed_map;
+	}
+
+	dev = alloc_candev(sizeof(struct flexcan_priv), 0);
+	if (!dev) {
+		err = -ENOMEM;
+		goto failed_alloc;
+	}
+
+	dev->netdev_ops = &flexcan_netdev_ops;
+	dev->irq = irq;
+	dev->flags |= IFF_ECHO; /* we support local echo in hardware */
+
+	priv = netdev_priv(dev);
+	priv->can.clock.freq = clk_get_rate(clk);
+	priv->can.bittiming_const = &flexcan_bittiming_const;
+	priv->can.do_set_mode = flexcan_set_mode;
+	priv->can.do_get_berr_counter = flexcan_get_berr_counter;
+	priv->can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
+		CAN_CTRLMODE_LISTENONLY	| CAN_CTRLMODE_3_SAMPLES;
+	priv->base = base;
+	priv->dev = dev;
+	priv->clk = clk;
+	priv->pdata = pdev->dev.platform_data;
+
+	netif_napi_add(dev, &priv->napi, flexcan_poll, FLEXCAN_NAPI_WEIGHT);
+
+	dev_set_drvdata(&pdev->dev, dev);
+	SET_NETDEV_DEV(dev, &pdev->dev);
+
+	err = register_flexcandev(dev);
+	if (err) {
+		dev_err(&pdev->dev, "registering netdev failed\n");
+		goto failed_register;
+	}
+
+	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
+		 priv->base, dev->irq);
+
+	return 0;
+
+ failed_register:
+	free_candev(dev);
+ failed_alloc:
+	iounmap(base);
+ failed_map:
+	release_mem_region(mem->start, mem_size);
+ failed_req:
+	clk_put(clk);
+ failed_get:
+ failed_clock:
+	return err;
+}
+
+static int __devexit flexcan_remove(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+	struct flexcan_priv *priv = netdev_priv(dev);
+	struct resource *mem;
+
+	unregister_flexcandev(dev);
+	platform_set_drvdata(pdev, NULL);
+	free_candev(dev);
+	iounmap(priv->base);
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	release_mem_region(mem->start, resource_size(mem));
+
+	clk_put(priv->clk);
+
+	return 0;
+}
+
+static struct platform_driver flexcan_driver = {
+	.driver.name = DRV_NAME,
+	.probe = flexcan_probe,
+	.remove = __devexit_p(flexcan_remove),
+};
+
+static int __init flexcan_init(void)
+{
+	pr_info("%s netdevice driver\n", DRV_NAME);
+	return platform_driver_register(&flexcan_driver);
+}
+
+static void __exit flexcan_exit(void)
+{
+	platform_driver_unregister(&flexcan_driver);
+	pr_info("%s: driver removed\n", DRV_NAME);
+}
+
+module_init(flexcan_init);
+module_exit(flexcan_exit);
+
+MODULE_AUTHOR("Sascha Hauer <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>, "
+	      "Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("CAN port driver for flexcan based chip");
diff --git a/include/linux/can/platform/flexcan.h b/include/linux/can/platform/flexcan.h
new file mode 100644
index 0000000..72b713a
--- /dev/null
+++ b/include/linux/can/platform/flexcan.h
@@ -0,0 +1,20 @@
+/*
+ * Copyright (C) 2010 Marc Kleine-Budde <kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
+ *
+ * This file is released under the GPLv2
+ *
+ */
+
+#ifndef __CAN_PLATFORM_FLEXCAN_H
+#define __CAN_PLATFORM_FLEXCAN_H
+
+/**
+ * struct flexcan_platform_data - flex CAN controller platform data
+ * @transceiver_enable:         - called to power on/off the transceiver
+ *
+ */
+struct flexcan_platform_data {
+	void (*transceiver_switch)(int enable);
+};
+
+#endif /* __CAN_PLATFORM_FLEXCAN_H */
-- 
1.7.1

^ permalink raw reply related

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: David Miller @ 2010-06-17 14:36 UTC (permalink / raw)
  To: fujita.tomonori
  Cc: mchan, lethal, vapier, JBottomley, netdev, linux-parisc,
	linux-kernel
In-Reply-To: <20100617211946A.fujita.tomonori@lab.ntt.co.jp>

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Thu, 17 Jun 2010 21:21:13 +0900

> On Wed, 16 Jun 2010 23:24:44 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
> 
>> David, why is dma_is_consistent() always returning 1 on sparc?  The
>> streaming DMA is not consistent.
> 
> I think that there are some confusion about dma_is_consistent(). Some
> architectures think that dma_is_consistent() is supposed to return 1
> if they can allocate coherent memory (note that some architectures
> can't allocate coherent memory).

Right, and that's why it's defined this way.

If the desired meaning is different, just me know and I'll fix the
sparc definition.


^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: Michael Chan @ 2010-06-17 14:42 UTC (permalink / raw)
  To: 'FUJITA Tomonori', 'davem@davemloft.net'
  Cc: JBottomley@Novell.com, vapier@gentoo.org, netdev@vger.kernel.org,
	linux-parisc@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20100617230435J.fujita.tomonori@lab.ntt.co.jp>

FUJITA Tomonori wrote:

> On Thu, 17 Jun 2010 06:30:39 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
>
> > James Bottomley wrote:
> >
> > > On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote:
> > > > This prefetch improves performance noticeably when the driver is
> > > > handling incoming 64-byte packets at a sustained rate.
> > >
> > > So why not do it unconditionally?  The worst that can happen
> > > is that you
> > > pull in a stale cache line which will get cleaned in the
> > > dma_sync, thus
> > > slightly degrading performance on incoherent architectures.
> >
> > The original patch was an unconditional prefetch.  There was
> > some discussion that it might not be correct if the DMA wasn't
> > sync'ed yet on some archs.  If the concensus is that it is ok to
> > do so, that would be the simplest solution.
>
> As James said, it just adds useless prefetch on incoherent
> architectures. sync_single_for_cpu is called later so we can see the
> correct data. One useless prefetch is unlikely to lead performance
> drop.
>
> You might prefer this v2.

Yes, thanks.
Acked-by: Michael Chan <mchan@broadcom.com>

>
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH v2] bnx2: fix dma_get_ops compilation breakage
>
> This removes dma_get_ops() prefetch optimization in bnx2.
>
> bnx2 uses dma_get_ops() to see if dma_sync_single_for_cpu() is
> noop. bnx2 does prefetch if it's noop.
>
> But dma_get_ops() isn't available on all the architectures (only the
> architectures that uses dma_map_ops struct have it). Using
> dma_get_ops() in drivers leads to compilation breakage on many
> archtectures.
>
> This patch removes dma_get_ops() and changes bnx2 to do prefetch on
> all the architectures. This adds useless prefetch on incoherent
> architectures but this is harmless. It is also unlikely to cause the
> performance drop.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> ---
>  drivers/net/bnx2.c |   10 ++++------
>  1 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
> index 949d7a9..85f1692 100644
> --- a/drivers/net/bnx2.c
> +++ b/drivers/net/bnx2.c
> @@ -3099,12 +3099,10 @@ bnx2_rx_int(struct bnx2 *bp, struct
> bnx2_napi *bnapi, int budget)
>               skb = rx_buf->skb;
>               prefetchw(skb);
>
> -             if (!get_dma_ops(&pdev->dev)->sync_single_for_cpu) {
> -                     next_rx_buf =
> -                             &rxr->rx_buf_ring[
> -
> RX_RING_IDX(NEXT_RX_BD(sw_cons))];
> -                     prefetch(next_rx_buf->desc);
> -             }
> +             next_rx_buf =
> +
> &rxr->rx_buf_ring[RX_RING_IDX(NEXT_RX_BD(sw_cons))];
> +             prefetch(next_rx_buf->desc);
> +
>               rx_buf->skb = NULL;
>
>               dma_addr = dma_unmap_addr(rx_buf, mapping);
> --
> 1.6.5
>
>
>


^ permalink raw reply

* RE: ipv6: netif_carrier_(on|off) with traces afterwards
From: Einar EL Lueck @ 2010-06-17 14:50 UTC (permalink / raw)
  To: Tantilov, Emil S; +Cc: NetDev
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A1365FF43AB7B9@rrsmsx501.amr.corp.intel.com>

"Tantilov, Emil S" <emil.s.tantilov@intel.com> wrote on 06/16/2010 05:55:45
PM:

> From:
>
> "Tantilov, Emil S" <emil.s.tantilov@intel.com>
>
> To:
>
> Einar EL Lueck/Germany/IBM@IBMDE
>
> Cc:
>
> NetDev <netdev@vger.kernel.org>
>
> Date:
>
> 06/16/2010 05:55 PM
>
> Subject:
>
> RE: ipv6: netif_carrier_(on|off) with traces afterwards
>
> >-----Original Message-----
> >From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On
> >Behalf Of Einar EL Lueck
> >Sent: Wednesday, June 16, 2010 6:30 AM
> >To: netdev@vger.kernel.org
> >Subject: ipv6: netif_carrier_(on|off) with traces afterwards
> >
> >
> >Hi,
> >With IPv6 addresses configured and and a network card doing
> >netif_carrier_off|on we see afterwards in 2.6.34 on S/390 some traces in
> >fib.
> >
> >Example sequence of operations:
> >ip -6 addr add fd00:10:30:49:4008:ffff:35:2/80 dev eth1
> >ip link set eth1 up
> >ip link set eth1 down
> ># the following lines have as effect netif_carrier_off and then on
(among
> >other stuff)
> >echo 0 > /sys/bus/ccwgroup/drivers/qeth/devices/0.0.e300/online
> >echo 1 > /sys/bus/ccwgroup/drivers/qeth/devices/0.0.e300/online
> ># end of plugging
> >ip link set eth1  up
> >ip link set eth1 down
> >=> at this point we get the following trace:
> >
> >Badness
> >at /home/autobuild/BUILD/linux-2.6.34-20100531/net/ipv6/ip6_fib.c:1160
>
> <snip>
>
> >
> >Has anybody seen effects like this before on other platforms, or has
> >anybody suggestions for the root cause?
>
> I had similar issues. The following patches fixed it for me:
>
> http://marc.info/?l=linux-netdev&m=127472600330413
>
> http://marc.info/?l=linux-netdev&m=127472599530407
>
> Thanks,
> Emil
>

Fixed it for me, too.
Thanks,
Einar.


^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: FUJITA Tomonori @ 2010-06-17 14:50 UTC (permalink / raw)
  To: davem
  Cc: fujita.tomonori, mchan, lethal, vapier, JBottomley, netdev,
	linux-parisc, linux-kernel
In-Reply-To: <20100617.073653.193708702.davem@davemloft.net>

On Thu, 17 Jun 2010 07:36:53 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:

> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Date: Thu, 17 Jun 2010 21:21:13 +0900
> 
> > On Wed, 16 Jun 2010 23:24:44 -0700
> > "Michael Chan" <mchan@broadcom.com> wrote:
> > 
> >> David, why is dma_is_consistent() always returning 1 on sparc?  The
> >> streaming DMA is not consistent.
> > 
> > I think that there are some confusion about dma_is_consistent(). Some
> > architectures think that dma_is_consistent() is supposed to return 1
> > if they can allocate coherent memory (note that some architectures
> > can't allocate coherent memory).
> 
> Right, and that's why it's defined this way.
> 
> If the desired meaning is different, just me know and I'll fix the
> sparc definition.

I think that there are some other architectures do the same. We need
to make sure that all the architectures define dma_is_consistent() in
the same meaning if drivers need it. However, I'm not sure we really
need dma_is_consistent(). There is only one user of it (and I think we
could remove it).

In the bnx2 case, we can simply prefetch on all the archs (or just
remove the optimization).

^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: FUJITA Tomonori @ 2010-06-17 14:50 UTC (permalink / raw)
  To: mchan; +Cc: JBottomley, vapier, netdev, linux-parisc, linux-kernel
In-Reply-To: <20100617230435J.fujita.tomonori@lab.ntt.co.jp>

Oops, some typos.

On Thu, 17 Jun 2010 23:05:59 +0900
FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:

> On Thu, 17 Jun 2010 06:30:39 -0700
> "Michael Chan" <mchan@broadcom.com> wrote:
> 
> > James Bottomley wrote:
> > 
> > > On Thu, 2010-06-17 at 05:54 -0700, Michael Chan wrote:
> > > > This prefetch improves performance noticeably when the driver is
> > > > handling incoming 64-byte packets at a sustained rate.
> > >
> > > So why not do it unconditionally?  The worst that can happen
> > > is that you
> > > pull in a stale cache line which will get cleaned in the
> > > dma_sync, thus
> > > slightly degrading performance on incoherent architectures.
> > 
> > The original patch was an unconditional prefetch.  There was
> > some discussion that it might not be correct if the DMA wasn't
> > sync'ed yet on some archs.  If the concensus is that it is ok to
> > do so, that would be the simplest solution.
> 
> As James said, it just adds useless prefetch on incoherent
> architectures. sync_single_for_cpu is called later so we can see the
> correct data. One useless prefetch is unlikely to lead performance
> drop.
> 
> You might prefer this v2.
> 
> =
> From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Subject: [PATCH v2] bnx2: fix dma_get_ops compilation breakage
> 
> This removes dma_get_ops() prefetch optimization in bnx2.
> 
> bnx2 uses dma_get_ops() to see if dma_sync_single_for_cpu() is
> noop. bnx2 does prefetch if it's noop.
> 
> But dma_get_ops() isn't available on all the architectures (only the
> architectures that uses dma_map_ops struct have it). Using
> dma_get_ops() in drivers leads to compilation breakage on many
> archtectures.

s/archtectures/architectures/

> This patch removes dma_get_ops() and changes bnx2 to do prefetch on
> all the architectures. This adds useless prefetch on incoherent
                                                       ~~~~~~~~~~
                                             s/incoherent/non-coherent/
(thanks to Alan)

> architectures but this is harmless. It is also unlikely to cause the
> performance drop.

^ permalink raw reply

* Re: mpd client timeouts (bisected) 2.6.35-rc3
From: Christoph Fritz @ 2010-06-17 15:05 UTC (permalink / raw)
  To: markus@trippelsdorf.de
  Cc: John Fastabend, David Miller, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, yanmin_zhang, alex.shi, tim.c.chen
In-Reply-To: <20100613205922.GA1806@arch.tripp.de>

On Sun, 2010-06-13 at 22:59 +0200, markus@trippelsdorf.de wrote:
> On Sun, Jun 13, 2010 at 01:36:30PM -0700, John Fastabend wrote:

> > [PATCH] net: fix deliver_no_wcard regression on loopback device
> 
> This solves the problem here. Thanks.

here too

Tested-by: Christoph Fritz <chf.fritz@googlemail.com>

^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: Paul Mundt @ 2010-06-17 15:30 UTC (permalink / raw)
  To: FUJITA Tomonori
  Cc: davem, mchan, vapier, JBottomley, netdev, linux-parisc,
	linux-kernel
In-Reply-To: <20100617234520S.fujita.tomonori@lab.ntt.co.jp>

On Thu, Jun 17, 2010 at 11:50:35PM +0900, FUJITA Tomonori wrote:
> On Thu, 17 Jun 2010 07:36:53 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
> > From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Date: Thu, 17 Jun 2010 21:21:13 +0900
> > 
> > > On Wed, 16 Jun 2010 23:24:44 -0700
> > > "Michael Chan" <mchan@broadcom.com> wrote:
> > > 
> > >> David, why is dma_is_consistent() always returning 1 on sparc?  The
> > >> streaming DMA is not consistent.
> > > 
> > > I think that there are some confusion about dma_is_consistent(). Some
> > > architectures think that dma_is_consistent() is supposed to return 1
> > > if they can allocate coherent memory (note that some architectures
> > > can't allocate coherent memory).
> > 
> > Right, and that's why it's defined this way.
> > 
> > If the desired meaning is different, just me know and I'll fix the
> > sparc definition.
> 
> I think that there are some other architectures do the same. We need
> to make sure that all the architectures define dma_is_consistent() in
> the same meaning if drivers need it. However, I'm not sure we really
> need dma_is_consistent(). There is only one user of it (and I think we
> could remove it).
> 
> In the bnx2 case, we can simply prefetch on all the archs (or just
> remove the optimization).

I think its worthwhile keeping, especially since the consistency can vary
on a per struct device level. If there's a benefit with these sorts of
prefetch micro-optimizations in drivers when it doesn't cost us that much
to provide the hint, I don't really see the harm. If dma_is_consistent()
is suddenly supposed to take on other meanings, or it's supposed to mean
something entirely different, then this is something we should deal with
separately.

I don't see any harm in letting drivers know whether we can support
consistent DMA allocs for a given struct device or not though, even if
the micro-optimization is marginal at best.

At least I've conditionalized the definition on SH, and it seems other
archictures have done so too. It's not clear what we'd gain from throwing
that away as long as they're generally in agreement on what it means.

^ permalink raw reply

* Re: bnx2 fails to compile on parisc because of missing get_dma_ops()
From: David Miller @ 2010-06-17 15:52 UTC (permalink / raw)
  To: fujita.tomonori
  Cc: mchan, JBottomley, vapier, netdev, linux-parisc, linux-kernel
In-Reply-To: <20100617234855R.fujita.tomonori@lab.ntt.co.jp>

From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Date: Thu, 17 Jun 2010 23:50:36 +0900

> Oops, some typos.

I've applied the patch with the typos fixed, thanks!

^ permalink raw reply

* Re: [PATCH NEXT 0/5]qlcnic: fixes
From: David Miller @ 2010-06-17 15:58 UTC (permalink / raw)
  To: amit.salecha; +Cc: netdev, ameen.rahman
In-Reply-To: <1276779402-496-1-git-send-email-amit.salecha@qlogic.com>

From: Amit Kumar Salecha <amit.salecha@qlogic.com>
Date: Thu, 17 Jun 2010 05:56:37 -0700

> 
> Hi
>    Sending series of 5 patches, mainly to improve rx performance and
>    fix netif_stop_queue race.
> 
>    Please apply them on net-next.

All applied, thank you.

^ permalink raw reply

* [PATCH] gainfar.c : skb_over_panic
From: Eran Liberty @ 2010-06-17 16:32 UTC (permalink / raw)
  To: galak, netdev

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

Hi Kumar,

I have demonstrated skb_over_panic with linux 2.6.32.15 on a mpc8548 
based product.

--------------- skb_over_panic snip ---------------
[  949.759344] skb_over_panic: text:c020fcec len:1474 put:1474 head:d8dc4000
data:d8dc4140 tail:0xd8dc4822 end:0xd8dc4760 dev:eth0
[  949.770913] ------------[ cut here ]------------
[  949.775522] kernel BUG at net/core/skbuff.c:127!
[  949.780132] Oops: Exception in kernel mode, sig: 5 [#1]
[  949.785347] EXTRICOM85xx
[  949.787870] Modules linked in: ...
[  949.806518] NIP: c02325bc LR: c02325bc CTR: c01f01f4
[  949.811475] REGS: d9a59bb0 TRAP: 0700   Tainted: P            (2.6.32.15)
[  949.818253] MSR: 00029000 <EE,ME,CE>  CR: 24004422  XER: 20000000
[  949.824364] TASK = d9a2a580[1877] 'insmod' THREAD: d9a58000
[  949.829753] GPR00: c02325bc d9a59c60 d9a2a580 00000089 0000f1d1 ffffffff
c01ed50c 0000f1d1
[  949.838132] GPR08: 00000030 c04accb4 0000f1d1 00004000 84004422 10019100
100936f8 dc3152f0
[  949.846510] GPR16: 00000040 00029000 c041fa14 dc315340 00000001 c041fa00
0000000a d9a65800
[  949.854887] GPR24: d9a58000 0000003f d9e42480 dc315000 d9a65950 000005c2
d9d05900 d8dc4260
[  949.863461] NIP [c02325bc] skb_over_panic+0x48/0x5c
[  949.868331] LR [c02325bc] skb_over_panic+0x48/0x5c
[  949.873111] Call Trace:
[  949.875551] [d9a59c60] [c02325bc] skb_over_panic+0x48/0x5c (unreliable)
[  949.882166] [d9a59c70] [c0233cf8] skb_put+0x5c/0x60
[  949.887051] [d9a59c80] [c020fcec] gfar_clean_rx_ring+0x210/0x444
[  949.893054] [d9a59cd0] [c0211620] gfar_poll+0x238/0x364
[  949.898284] [d9a59d20] [c02403e4] net_rx_action+0x8c/0x178
[  949.903773] [d9a59d50] [c004278c] __do_softirq+0xa0/0x110
[  949.909171] [d9a59d90] [c0004c24] do_softirq+0x54/0x58
[  949.914306] [d9a59da0] [c0042604] irq_exit+0x98/0x9c
[  949.919267] [d9a59db0] [c0004edc] do_IRQ+0x9c/0xb4
[  949.924060] [d9a59dd0] [c000fe8c] ret_from_except+0x0/0x18
[  949.929544] [d9a59e90] [c00685d0] load_module+0x64/0x1638
[  949.934938] [d9a59f20] [c0069c28] sys_init_module+0x84/0x218
[  949.940593] [d9a59f40] [c000f838] ret_from_syscall+0x0/0x3c
[  949.946159] Instruction dump:
[  949.949121] 80a30054 2f800000 80e300a0 810300a4 81630098 8143009c 
419e0020
3c60c042
[  949.956889] 90010008 7d695b78 38633b2c 4be0b2fd <0fe00000> 48000000 
3d20c03f
38090970
[  949.964835] Kernel panic - not syncing: Fatal exception in interrupt
[  949.971179] Call Trace:
[  949.973619] [d9a59a00] [c0006ff0] show_stack+0x44/0x16c (unreliable)
[  949.979972] [d9a59a40] [c003c8b4] panic+0x90/0x168
[  949.984759] [d9a59a90] [c000ceb8] die+0x164/0x19c
[  949.989459] [d9a59ab0] [c000d174] _exception+0x120/0x144
[  949.994768] [d9a59ba0] [c000fe40] ret_from_except_full+0x0/0x4c
[  950.000685] [d9a59c60] [c02325bc] skb_over_panic+0x48/0x5c
[  950.006166] [d9a59c70] [c0233cf8] skb_put+0x5c/0x60
[  950.011042] [d9a59c80] [c020fcec] gfar_clean_rx_ring+0x210/0x444
[  950.017045] [d9a59cd0] [c0211620] gfar_poll+0x238/0x364
[  950.022267] [d9a59d20] [c02403e4] net_rx_action+0x8c/0x178
[  950.027750] [d9a59d50] [c004278c] __do_softirq+0xa0/0x110
[  950.033145] [d9a59d90] [c0004c24] do_softirq+0x54/0x58
[  950.038279] [d9a59da0] [c0042604] irq_exit+0x98/0x9c
[  950.043239] [d9a59db0] [c0004edc] do_IRQ+0x9c/0xb4
[  950.048027] [d9a59dd0] [c000fe8c] ret_from_except+0x0/0x18
[  950.053508] [d9a59e90] [c00685d0] load_module+0x64/0x1638
[  950.058902] [d9a59f20] [c0069c28] sys_init_module+0x84/0x218
[  950.064558] [d9a59f40] [c000f838] ret_from_syscall+0x0/0x3c
[  950.070126] Rebooting in 1 seconds..
[  951.067504] ------------[ cut here ]------------


The skb_over_panic occurs due to calling skb_put() within 
gfar_clean_rx_ring(). This happens if (and only if) shortly prior to the 
event and a few lined above the skb_put(), an skb was queued back to the 
priv->rx_recycle queue due to RXBD_LAST or RXBD_ERR status.

--------------- driver/net/gianfar.c: gfar_clean_rx_ring() ---------------
1851                 if (unlikely(!newskb || !(bdp->status & RXBD_LAST) ||
1852                                  bdp->status & RXBD_ERR)) {
1853                         count_errors(bdp->status, dev);
1854
1855                         if (unlikely(!newskb))
1856                                 newskb = skb;
1857                         else if (skb) {
1858                                 /*
1859                                  * We need to reset ->data to what it
1860                                  * was before gfar_new_skb() re-aligned
1861                                  * it to an RXBUF_ALIGNMENT boundary
1862                                  * before we put the skb back on the
1863                                  * recycle list.
1864                                  */
1865                                 skb->data = skb->head + NET_SKB_PAD;

This happens first...

1866                                 __skb_queue_head(&priv->rx_recycle, 
skb);
1867                         }
1868                 } else {
1869                         /* Increment the number of packets */
1870                         dev->stats.rx_packets++;
1871                         howmany++;
1872
1873                         if (likely(skb)) {
1874                                 pkt_len = bdp->length - ETH_FCS_LEN;
1875                                 /* Remove the FCS from the packet 
length */

After relatively short time this will create skb_over_panic

1876                                 skb_put(skb, pkt_len);
1877                                 dev->stats.rx_bytes += pkt_len;
1878
1879                                 if (in_irq() || irqs_disabled())

--------------------------------------------------------------------------

As seen in line 1865 there is an attempt to fix the skb prior to its 
re-queuing but we can look at gfar_clean_tx_ring() where it calls 
skb_recycle_check() prior to re-queuing, which looks more professional.

--------------- driver/net/gianfar.c: gfar_clean_tx_ring() ---------------
1621                 if (skb_queue_len(&priv->rx_recycle) < 
priv->rx_ring_size &&
1622                                 skb_recycle_check(skb, 
priv->rx_buffer_size +
1623                                         RXBUF_ALIGNMENT))
1624                         __skb_queue_head(&priv->rx_recycle, skb);
1625                 else
1626                         dev_kfree_skb_any(skb);
--------------------------------------------------------------------------

Duplicating the above code for the gfar_clean_rx_ring() function 
effectively eliminated the  skb_over_run and thus I propose the attached 
patch

-- Liberty
<https://svn.extricom.com/lxr/ident?v=linux-2.6.32.15;i=gfar_clean_rx_ring>

[-- Attachment #2: gianfar_skb_over_panic.patch --]
[-- Type: text/x-diff, Size: 853 bytes --]

---
 drivers/net/gianfar.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1854,15 +1854,13 @@
 			if (unlikely(!newskb))
 				newskb = skb;
 			else if (skb) {
-				/*
-				 * We need to reset ->data to what it
-				 * was before gfar_new_skb() re-aligned
-				 * it to an RXBUF_ALIGNMENT boundary
-				 * before we put the skb back on the
-				 * recycle list.
-				 */
-				skb->data = skb->head + NET_SKB_PAD;
-				__skb_queue_head(&priv->rx_recycle, skb);
+				if (skb_queue_len(&priv->rx_recycle) < priv->rx_ring_size &&
+						skb_recycle_check(skb, priv->rx_buffer_size +
+							RXBUF_ALIGNMENT)) {
+					__skb_queue_head(&priv->rx_recycle, skb);
+				} else {
+					dev_kfree_skb_any(skb);
+				}
 			}
 		} else {
 			/* Increment the number of packets */

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox