linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments
@ 2023-05-23  9:43 Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 01/12] can: kvaser_pciefd: Remove useless write to interrupt register Jimmy Assarsson
                   ` (16 more replies)
  0 siblings, 17 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

This patch series contains various non critical fixes and improvments for
the kvaser_pciefd driver.

Jimmy Assarsson (12):
  can: kvaser_pciefd: Remove useless write to interrupt register
  can: kvaser_pciefd: Remove handler for unused
    KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK
  can: kvaser_pciefd: Add function to set skb hwtstamps
  can: kvaser_pciefd: Set hardware timestamp on transmitted packets
  can: kvaser_pciefd: Define unsigned constants with type suffix 'U'
  can: kvaser_pciefd: Remove SPI flash parameter read functionality
  can: kvaser_pciefd: Sort includes in alphabetic order
  can: kvaser_pciefd: Rename device ID defines
  can: kvaser_pciefd: Change return type for
    kvaser_pciefd_{receive,transmit,set_tx}_irq()
  can: kvaser_pciefd: Add len8_dlc support
  can: kvaser_pciefd: Refactor code
  can: kvaser_pciefd: Use TX FIFO size read from CAN controller

 drivers/net/can/Kconfig         |   3 +-
 drivers/net/can/kvaser_pciefd.c | 622 +++++++++-----------------------
 2 files changed, 168 insertions(+), 457 deletions(-)

-- 
2.40.0


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

* [PATCH 01/12] can: kvaser_pciefd: Remove useless write to interrupt register
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 02/12] can: kvaser_pciefd: Remove handler for unused KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK Jimmy Assarsson
                   ` (15 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

The PCI interrupt register, KVASER_PCIEFD_IRQ_REG, is level triggered.
Writing to the register doesn't affect it.

Fixes: 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index be189edb256c..d60d17199a1b 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -1768,7 +1768,6 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
 			kvaser_pciefd_transmit_irq(pcie->can[i]);
 	}
 
-	iowrite32(board_irq, pcie->reg_base + KVASER_PCIEFD_IRQ_REG);
 	return IRQ_HANDLED;
 }
 
@@ -1842,9 +1841,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 		  KVASER_PCIEFD_SRB_IRQ_DUF0 | KVASER_PCIEFD_SRB_IRQ_DUF1,
 		  pcie->reg_base + KVASER_PCIEFD_SRB_IEN_REG);
 
-	/* Reset IRQ handling, expected to be off before */
-	iowrite32(KVASER_PCIEFD_IRQ_ALL_MSK,
-		  pcie->reg_base + KVASER_PCIEFD_IRQ_REG);
+	/* Enable PCI interrupts */
 	iowrite32(KVASER_PCIEFD_IRQ_ALL_MSK,
 		  pcie->reg_base + KVASER_PCIEFD_IEN_REG);
 
@@ -1906,10 +1903,8 @@ static void kvaser_pciefd_remove(struct pci_dev *pdev)
 
 	kvaser_pciefd_remove_all_ctrls(pcie);
 
-	/* Turn off IRQ generation */
+	/* Disable interrupts */
 	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_SRB_CTRL_REG);
-	iowrite32(KVASER_PCIEFD_IRQ_ALL_MSK,
-		  pcie->reg_base + KVASER_PCIEFD_IRQ_REG);
 	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_IEN_REG);
 
 	free_irq(pcie->pci->irq, pcie);
-- 
2.40.0


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

* [PATCH 02/12] can: kvaser_pciefd: Remove handler for unused KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 01/12] can: kvaser_pciefd: Remove useless write to interrupt register Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 03/12] can: kvaser_pciefd: Add function to set skb hwtstamps Jimmy Assarsson
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

The Kvaser KCAN controller got a feature to send error frames on request.
The packet KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK signals that the requested
error frame was transmitted.
Since this feature is not supported by the driver, drop the handler and add
KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK to the list of unexpected packet types.

Fixes: 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 39 +--------------------------------
 1 file changed, 1 insertion(+), 38 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index d60d17199a1b..1821ffa4ca79 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -1472,40 +1472,6 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 	return 0;
 }
 
-static int kvaser_pciefd_handle_eack_packet(struct kvaser_pciefd *pcie,
-					    struct kvaser_pciefd_rx_packet *p)
-{
-	struct kvaser_pciefd_can *can;
-	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
-
-	if (ch_id >= pcie->nr_channels)
-		return -EIO;
-
-	can = pcie->can[ch_id];
-
-	/* If this is the last flushed packet, send end of flush */
-	if (p->header[0] & KVASER_PCIEFD_APACKET_FLU) {
-		u8 count = ioread32(can->reg_base +
-				    KVASER_PCIEFD_KCAN_TX_NPACKETS_REG) & 0xff;
-
-		if (count == 0)
-			iowrite32(KVASER_PCIEFD_KCAN_CTRL_EFLUSH,
-				  can->reg_base + KVASER_PCIEFD_KCAN_CTRL_REG);
-	} else {
-		int echo_idx = p->header[0] & KVASER_PCIEFD_PACKET_SEQ_MSK;
-		int dlc = can_get_echo_skb(can->can.dev, echo_idx, NULL);
-		struct net_device_stats *stats = &can->can.dev->stats;
-
-		stats->tx_bytes += dlc;
-		stats->tx_packets++;
-
-		if (netif_queue_stopped(can->can.dev))
-			netif_wake_queue(can->can.dev);
-	}
-
-	return 0;
-}
-
 static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
 					     struct kvaser_pciefd_rx_packet *p)
 {
@@ -1644,16 +1610,13 @@ static int kvaser_pciefd_read_packet(struct kvaser_pciefd *pcie, int *start_pos,
 		ret = kvaser_pciefd_handle_error_packet(pcie, p);
 		break;
 
-	case KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK:
-		ret = kvaser_pciefd_handle_eack_packet(pcie, p);
-		break;
-
 	case KVASER_PCIEFD_PACK_TYPE_EFLUSH_ACK:
 		ret = kvaser_pciefd_handle_eflush_packet(pcie, p);
 		break;
 
 	case KVASER_PCIEFD_PACK_TYPE_ACK_DATA:
 	case KVASER_PCIEFD_PACK_TYPE_BUS_LOAD:
+	case KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK:
 	case KVASER_PCIEFD_PACK_TYPE_TXRQ:
 		dev_info(&pcie->pci->dev,
 			 "Received unexpected packet type 0x%08X\n", type);
-- 
2.40.0


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

* [PATCH 03/12] can: kvaser_pciefd: Add function to set skb hwtstamps
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 01/12] can: kvaser_pciefd: Remove useless write to interrupt register Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 02/12] can: kvaser_pciefd: Remove handler for unused KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 04/12] can: kvaser_pciefd: Set hardware timestamp on transmitted packets Jimmy Assarsson
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Add new function, kvaser_pciefd_set_skb_timestamp(), to set skb hwtstamps.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 1821ffa4ca79..7646338d4d44 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -538,6 +538,13 @@ static int kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can)
 	return 0;
 }
 
+static inline void kvaser_pciefd_set_skb_timestamp(const struct kvaser_pciefd *pcie,
+						   struct sk_buff *skb, u64 timestamp)
+{
+	skb_hwtstamps(skb)->hwtstamp =
+		ns_to_ktime(div_u64(timestamp * 1000, pcie->freq_to_ticks_div));
+}
+
 static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
 {
 	u32 mode;
@@ -1171,7 +1178,6 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 	struct canfd_frame *cf;
 	struct can_priv *priv;
 	struct net_device_stats *stats;
-	struct skb_shared_hwtstamps *shhwtstamps;
 	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
 
 	if (ch_id >= pcie->nr_channels)
@@ -1214,12 +1220,7 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 		stats->rx_bytes += cf->len;
 	}
 	stats->rx_packets++;
-
-	shhwtstamps = skb_hwtstamps(skb);
-
-	shhwtstamps->hwtstamp =
-		ns_to_ktime(div_u64(p->timestamp * 1000,
-				    pcie->freq_to_ticks_div));
+	kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
 
 	return netif_rx(skb);
 }
@@ -1282,7 +1283,6 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
 	struct net_device *ndev = can->can.dev;
 	struct sk_buff *skb;
 	struct can_frame *cf = NULL;
-	struct skb_shared_hwtstamps *shhwtstamps;
 	struct net_device_stats *stats = &ndev->stats;
 
 	old_state = can->can.state;
@@ -1323,10 +1323,7 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
 		return -ENOMEM;
 	}
 
-	shhwtstamps = skb_hwtstamps(skb);
-	shhwtstamps->hwtstamp =
-		ns_to_ktime(div_u64(p->timestamp * 1000,
-				    can->kv_pcie->freq_to_ticks_div));
+	kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
 	cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_CNT;
 
 	cf->data[6] = bec.txerr;
@@ -1374,7 +1371,6 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
 		struct net_device *ndev = can->can.dev;
 		struct sk_buff *skb;
 		struct can_frame *cf;
-		struct skb_shared_hwtstamps *shhwtstamps;
 
 		skb = alloc_can_err_skb(ndev, &cf);
 		if (!skb) {
@@ -1394,10 +1390,7 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
 			cf->can_id |= CAN_ERR_RESTARTED;
 		}
 
-		shhwtstamps = skb_hwtstamps(skb);
-		shhwtstamps->hwtstamp =
-			ns_to_ktime(div_u64(p->timestamp * 1000,
-					    can->kv_pcie->freq_to_ticks_div));
+		kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
 
 		cf->data[6] = bec.txerr;
 		cf->data[7] = bec.rxerr;
-- 
2.40.0


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

* [PATCH 04/12] can: kvaser_pciefd: Set hardware timestamp on transmitted packets
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (2 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 03/12] can: kvaser_pciefd: Add function to set skb hwtstamps Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 05/12] can: kvaser_pciefd: Define unsigned constants with type suffix 'U' Jimmy Assarsson
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Set hardware timestamp on transmitted packets.

Fixes: 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 7646338d4d44..88bad2c2b641 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -1485,6 +1485,7 @@ static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
 
 	if (skb) {
 		cf->can_id |= CAN_ERR_BUSERROR;
+		kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
 		netif_rx(skb);
 	} else {
 		stats->rx_dropped++;
@@ -1516,8 +1517,15 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 		netdev_dbg(can->can.dev, "Packet was flushed\n");
 	} else {
 		int echo_idx = p->header[0] & KVASER_PCIEFD_PACKET_SEQ_MSK;
-		int dlc = can_get_echo_skb(can->can.dev, echo_idx, NULL);
-		u8 count = ioread32(can->reg_base +
+		int dlc;
+		u8 count;
+		struct sk_buff *skb;
+
+		skb = can->can.echo_skb[echo_idx];
+		if (skb)
+			kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
+		dlc = can_get_echo_skb(can->can.dev, echo_idx, NULL);
+		count = ioread32(can->reg_base +
 				    KVASER_PCIEFD_KCAN_TX_NPACKETS_REG) & 0xff;
 
 		if (count < KVASER_PCIEFD_CAN_TX_MAX_COUNT &&
-- 
2.40.0


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

* [PATCH 05/12] can: kvaser_pciefd: Define unsigned constants with type suffix 'U'
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (3 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 04/12] can: kvaser_pciefd: Set hardware timestamp on transmitted packets Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 06/12] can: kvaser_pciefd: Remove SPI flash parameter read functionality Jimmy Assarsson
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Define unsigned constants with type suffix 'U'

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 88bad2c2b641..abb556fb5cb6 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -25,12 +25,12 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 
 #define KVASER_PCIEFD_WAIT_TIMEOUT msecs_to_jiffies(1000)
 #define KVASER_PCIEFD_BEC_POLL_FREQ (jiffies + msecs_to_jiffies(200))
-#define KVASER_PCIEFD_MAX_ERR_REP 256
-#define KVASER_PCIEFD_CAN_TX_MAX_COUNT 17
-#define KVASER_PCIEFD_MAX_CAN_CHANNELS 4
-#define KVASER_PCIEFD_DMA_COUNT 2
+#define KVASER_PCIEFD_MAX_ERR_REP 256U
+#define KVASER_PCIEFD_CAN_TX_MAX_COUNT 17U
+#define KVASER_PCIEFD_MAX_CAN_CHANNELS 4U
+#define KVASER_PCIEFD_DMA_COUNT 2U
 
-#define KVASER_PCIEFD_DMA_SIZE (4 * 1024)
+#define KVASER_PCIEFD_DMA_SIZE (4U * 1024U)
 #define KVASER_PCIEFD_64BIT_DMA_BIT BIT(0)
 
 #define KVASER_PCIEFD_VENDOR 0x1a07
-- 
2.40.0


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

* [PATCH 06/12] can: kvaser_pciefd: Remove SPI flash parameter read functionality
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (4 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 05/12] can: kvaser_pciefd: Define unsigned constants with type suffix 'U' Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 07/12] can: kvaser_pciefd: Sort includes in alphabetic order Jimmy Assarsson
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Remove SPI flash parameter read functionality, since it's only used for
reading the interface CAN controller count.
This information is already read from a register, making the information
redundant.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---

This additional sanity check doesn't justify the extra 150 lines of code.

 drivers/net/can/Kconfig         |   3 +-
 drivers/net/can/kvaser_pciefd.c | 220 +-------------------------------
 2 files changed, 5 insertions(+), 218 deletions(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index 3ceccafd701b..b929a9da7920 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -153,8 +153,7 @@ config CAN_JANZ_ICAN3
 config CAN_KVASER_PCIEFD
 	depends on PCI
 	tristate "Kvaser PCIe FD cards"
-	select CRC32
-	  help
+	help
 	  This is a driver for the Kvaser PCI Express CAN FD family.
 
 	  Supported devices:
diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index abb556fb5cb6..e24a8e77aef1 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -3,10 +3,10 @@
  * Parts of this driver are based on the following:
  *  - Kvaser linux pciefd driver (version 5.25)
  *  - PEAK linux canfd driver
- *  - Altera Avalon EPCS flash controller driver
  */
 
 #include <linux/kernel.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/ethtool.h>
@@ -14,7 +14,6 @@
 #include <linux/can/dev.h>
 #include <linux/timer.h>
 #include <linux/netdevice.h>
-#include <linux/crc32.h>
 #include <linux/iopoll.h>
 
 MODULE_LICENSE("Dual BSD/GPL");
@@ -78,13 +77,6 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_SRB_STAT_REG (KVASER_PCIEFD_SRB_BASE + 0x210)
 #define KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG (KVASER_PCIEFD_SRB_BASE + 0x214)
 #define KVASER_PCIEFD_SRB_CTRL_REG (KVASER_PCIEFD_SRB_BASE + 0x218)
-/* EPCS flash controller registers */
-#define KVASER_PCIEFD_SPI_BASE 0x1fc00
-#define KVASER_PCIEFD_SPI_RX_REG KVASER_PCIEFD_SPI_BASE
-#define KVASER_PCIEFD_SPI_TX_REG (KVASER_PCIEFD_SPI_BASE + 0x4)
-#define KVASER_PCIEFD_SPI_STATUS_REG (KVASER_PCIEFD_SPI_BASE + 0x8)
-#define KVASER_PCIEFD_SPI_CTRL_REG (KVASER_PCIEFD_SPI_BASE + 0xc)
-#define KVASER_PCIEFD_SPI_SSEL_REG (KVASER_PCIEFD_SPI_BASE + 0x14)
 
 #define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
 #define KVASER_PCIEFD_IRQ_SRB BIT(4)
@@ -119,23 +111,6 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* DMA Enable */
 #define KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE BIT(0)
 
-/* EPCS flash controller definitions */
-#define KVASER_PCIEFD_CFG_IMG_SZ (64 * 1024)
-#define KVASER_PCIEFD_CFG_IMG_OFFSET (31 * 65536L)
-#define KVASER_PCIEFD_CFG_MAX_PARAMS 256
-#define KVASER_PCIEFD_CFG_MAGIC 0xcafef00d
-#define KVASER_PCIEFD_CFG_PARAM_MAX_SZ 24
-#define KVASER_PCIEFD_CFG_SYS_VER 1
-#define KVASER_PCIEFD_CFG_PARAM_NR_CHAN 130
-#define KVASER_PCIEFD_SPI_TMT BIT(5)
-#define KVASER_PCIEFD_SPI_TRDY BIT(6)
-#define KVASER_PCIEFD_SPI_RRDY BIT(7)
-#define KVASER_PCIEFD_FLASH_ID_EPCS16 0x14
-/* Commands for controlling the onboard flash */
-#define KVASER_PCIEFD_FLASH_RES_CMD 0xab
-#define KVASER_PCIEFD_FLASH_READ_CMD 0x3
-#define KVASER_PCIEFD_FLASH_STATUS_CMD 0x5
-
 /* Kvaser KCAN definitions */
 #define KVASER_PCIEFD_KCAN_CTRL_EFLUSH (4 << 29)
 #define KVASER_PCIEFD_KCAN_CTRL_EFRAME (5 << 29)
@@ -306,20 +281,6 @@ static const struct can_bittiming_const kvaser_pciefd_bittiming_const = {
 	.brp_inc = 1,
 };
 
-struct kvaser_pciefd_cfg_param {
-	__le32 magic;
-	__le32 nr;
-	__le32 len;
-	u8 data[KVASER_PCIEFD_CFG_PARAM_MAX_SZ];
-};
-
-struct kvaser_pciefd_cfg_img {
-	__le32 version;
-	__le32 magic;
-	__le32 crc;
-	struct kvaser_pciefd_cfg_param params[KVASER_PCIEFD_CFG_MAX_PARAMS];
-};
-
 static struct pci_device_id kvaser_pciefd_id_table[] = {
 	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_4HS_ID), },
 	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_2HS_ID), },
@@ -330,164 +291,6 @@ static struct pci_device_id kvaser_pciefd_id_table[] = {
 };
 MODULE_DEVICE_TABLE(pci, kvaser_pciefd_id_table);
 
-/* Onboard flash memory functions */
-static int kvaser_pciefd_spi_wait_loop(struct kvaser_pciefd *pcie, int msk)
-{
-	u32 res;
-
-	return readl_poll_timeout(pcie->reg_base + KVASER_PCIEFD_SPI_STATUS_REG,
-			res, res & msk, 0, 10);
-}
-
-static int kvaser_pciefd_spi_cmd(struct kvaser_pciefd *pcie, const u8 *tx,
-				 u32 tx_len, u8 *rx, u32 rx_len)
-{
-	int c;
-
-	iowrite32(BIT(0), pcie->reg_base + KVASER_PCIEFD_SPI_SSEL_REG);
-	iowrite32(BIT(10), pcie->reg_base + KVASER_PCIEFD_SPI_CTRL_REG);
-	ioread32(pcie->reg_base + KVASER_PCIEFD_SPI_RX_REG);
-
-	c = tx_len;
-	while (c--) {
-		if (kvaser_pciefd_spi_wait_loop(pcie, KVASER_PCIEFD_SPI_TRDY))
-			return -EIO;
-
-		iowrite32(*tx++, pcie->reg_base + KVASER_PCIEFD_SPI_TX_REG);
-
-		if (kvaser_pciefd_spi_wait_loop(pcie, KVASER_PCIEFD_SPI_RRDY))
-			return -EIO;
-
-		ioread32(pcie->reg_base + KVASER_PCIEFD_SPI_RX_REG);
-	}
-
-	c = rx_len;
-	while (c-- > 0) {
-		if (kvaser_pciefd_spi_wait_loop(pcie, KVASER_PCIEFD_SPI_TRDY))
-			return -EIO;
-
-		iowrite32(0, pcie->reg_base + KVASER_PCIEFD_SPI_TX_REG);
-
-		if (kvaser_pciefd_spi_wait_loop(pcie, KVASER_PCIEFD_SPI_RRDY))
-			return -EIO;
-
-		*rx++ = ioread32(pcie->reg_base + KVASER_PCIEFD_SPI_RX_REG);
-	}
-
-	if (kvaser_pciefd_spi_wait_loop(pcie, KVASER_PCIEFD_SPI_TMT))
-		return -EIO;
-
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_SPI_CTRL_REG);
-
-	if (c != -1) {
-		dev_err(&pcie->pci->dev, "Flash SPI transfer failed\n");
-		return -EIO;
-	}
-
-	return 0;
-}
-
-static int kvaser_pciefd_cfg_read_and_verify(struct kvaser_pciefd *pcie,
-					     struct kvaser_pciefd_cfg_img *img)
-{
-	int offset = KVASER_PCIEFD_CFG_IMG_OFFSET;
-	int res, crc;
-	u8 *crc_buff;
-
-	u8 cmd[] = {
-		KVASER_PCIEFD_FLASH_READ_CMD,
-		(u8)((offset >> 16) & 0xff),
-		(u8)((offset >> 8) & 0xff),
-		(u8)(offset & 0xff)
-	};
-
-	res = kvaser_pciefd_spi_cmd(pcie, cmd, ARRAY_SIZE(cmd), (u8 *)img,
-				    KVASER_PCIEFD_CFG_IMG_SZ);
-	if (res)
-		return res;
-
-	crc_buff = (u8 *)img->params;
-
-	if (le32_to_cpu(img->version) != KVASER_PCIEFD_CFG_SYS_VER) {
-		dev_err(&pcie->pci->dev,
-			"Config flash corrupted, version number is wrong\n");
-		return -ENODEV;
-	}
-
-	if (le32_to_cpu(img->magic) != KVASER_PCIEFD_CFG_MAGIC) {
-		dev_err(&pcie->pci->dev,
-			"Config flash corrupted, magic number is wrong\n");
-		return -ENODEV;
-	}
-
-	crc = ~crc32_be(0xffffffff, crc_buff, sizeof(img->params));
-	if (le32_to_cpu(img->crc) != crc) {
-		dev_err(&pcie->pci->dev,
-			"Stored CRC does not match flash image contents\n");
-		return -EIO;
-	}
-
-	return 0;
-}
-
-static void kvaser_pciefd_cfg_read_params(struct kvaser_pciefd *pcie,
-					  struct kvaser_pciefd_cfg_img *img)
-{
-	struct kvaser_pciefd_cfg_param *param;
-
-	param = &img->params[KVASER_PCIEFD_CFG_PARAM_NR_CHAN];
-	memcpy(&pcie->nr_channels, param->data, le32_to_cpu(param->len));
-}
-
-static int kvaser_pciefd_read_cfg(struct kvaser_pciefd *pcie)
-{
-	int res;
-	struct kvaser_pciefd_cfg_img *img;
-
-	/* Read electronic signature */
-	u8 cmd[] = {KVASER_PCIEFD_FLASH_RES_CMD, 0, 0, 0};
-
-	res = kvaser_pciefd_spi_cmd(pcie, cmd, ARRAY_SIZE(cmd), cmd, 1);
-	if (res)
-		return -EIO;
-
-	img = kmalloc(KVASER_PCIEFD_CFG_IMG_SZ, GFP_KERNEL);
-	if (!img)
-		return -ENOMEM;
-
-	if (cmd[0] != KVASER_PCIEFD_FLASH_ID_EPCS16) {
-		dev_err(&pcie->pci->dev,
-			"Flash id is 0x%x instead of expected EPCS16 (0x%x)\n",
-			cmd[0], KVASER_PCIEFD_FLASH_ID_EPCS16);
-
-		res = -ENODEV;
-		goto image_free;
-	}
-
-	cmd[0] = KVASER_PCIEFD_FLASH_STATUS_CMD;
-	res = kvaser_pciefd_spi_cmd(pcie, cmd, 1, cmd, 1);
-	if (res) {
-		goto image_free;
-	} else if (cmd[0] & 1) {
-		res = -EIO;
-		/* No write is ever done, the WIP should never be set */
-		dev_err(&pcie->pci->dev, "Unexpected WIP bit set in flash\n");
-		goto image_free;
-	}
-
-	res = kvaser_pciefd_cfg_read_and_verify(pcie, img);
-	if (res) {
-		res = -EIO;
-		goto image_free;
-	}
-
-	kvaser_pciefd_cfg_read_params(pcie, img);
-
-image_free:
-	kfree(img);
-	return res;
-}
-
 static void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
 {
 	u32 cmd;
@@ -1125,25 +928,10 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 {
 	u32 sysid, srb_status, build;
-	u8 sysid_nr_chan;
-	int ret;
-
-	ret = kvaser_pciefd_read_cfg(pcie);
-	if (ret)
-		return ret;
 
 	sysid = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_VERSION_REG);
-	sysid_nr_chan = (sysid >> KVASER_PCIEFD_SYSID_NRCHAN_SHIFT) & 0xff;
-	if (pcie->nr_channels != sysid_nr_chan) {
-		dev_err(&pcie->pci->dev,
-			"Number of channels does not match: %u vs %u\n",
-			pcie->nr_channels,
-			sysid_nr_chan);
-		return -ENODEV;
-	}
-
-	if (pcie->nr_channels > KVASER_PCIEFD_MAX_CAN_CHANNELS)
-		pcie->nr_channels = KVASER_PCIEFD_MAX_CAN_CHANNELS;
+	pcie->nr_channels = min(KVASER_PCIEFD_MAX_CAN_CHANNELS,
+				((sysid >> KVASER_PCIEFD_SYSID_NRCHAN_SHIFT) & 0xff));
 
 	build = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_BUILD_REG);
 	dev_dbg(&pcie->pci->dev, "Version %u.%u.%u\n",
@@ -1167,7 +955,7 @@ static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 
 	/* Turn off all loopback functionality */
 	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_LOOP_REG);
-	return ret;
+	return 0;
 }
 
 static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
-- 
2.40.0


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

* [PATCH 07/12] can: kvaser_pciefd: Sort includes in alphabetic order
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (5 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 06/12] can: kvaser_pciefd: Remove SPI flash parameter read functionality Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 08/12] can: kvaser_pciefd: Rename device ID defines Jimmy Assarsson
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Sort the includes in alphabetic order.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index e24a8e77aef1..8be27a7dc7e9 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -5,16 +5,16 @@
  *  - PEAK linux canfd driver
  */
 
+#include <linux/can/dev.h>
+#include <linux/device.h>
+#include <linux/ethtool.h>
+#include <linux/iopoll.h>
 #include <linux/kernel.h>
 #include <linux/minmax.h>
 #include <linux/module.h>
-#include <linux/device.h>
-#include <linux/ethtool.h>
+#include <linux/netdevice.h>
 #include <linux/pci.h>
-#include <linux/can/dev.h>
 #include <linux/timer.h>
-#include <linux/netdevice.h>
-#include <linux/iopoll.h>
 
 MODULE_LICENSE("Dual BSD/GPL");
 MODULE_AUTHOR("Kvaser AB <support@kvaser.com>");
-- 
2.40.0


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

* [PATCH 08/12] can: kvaser_pciefd: Rename device ID defines
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (6 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 07/12] can: kvaser_pciefd: Sort includes in alphabetic order Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 09/12] can: kvaser_pciefd: Change return type for kvaser_pciefd_{receive,transmit,set_tx}_irq() Jimmy Assarsson
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Rename device ID defines to better match the product name of the supported
device.
Use 16 bit hexadecimal values for device IDs.
And format kvaser_pciefd_id_table using clang-format.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 34 ++++++++++++++++++++++-----------
 1 file changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 8be27a7dc7e9..bf3fa51069a9 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -33,11 +33,11 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_64BIT_DMA_BIT BIT(0)
 
 #define KVASER_PCIEFD_VENDOR 0x1a07
-#define KVASER_PCIEFD_4HS_ID 0x0d
-#define KVASER_PCIEFD_2HS_ID 0x0e
-#define KVASER_PCIEFD_HS_ID 0x0f
-#define KVASER_PCIEFD_MINIPCIE_HS_ID 0x10
-#define KVASER_PCIEFD_MINIPCIE_2HS_ID 0x11
+#define KVASER_PCIEFD_4HS_DEVICE_ID 0x000d
+#define KVASER_PCIEFD_2HS_V2_DEVICE_ID 0x000e
+#define KVASER_PCIEFD_HS_V2_DEVICE_ID 0x000f
+#define KVASER_PCIEFD_MINIPCIE_HS_V2_DEVICE_ID 0x0010
+#define KVASER_PCIEFD_MINIPCIE_2HS_V2_DEVICE_ID 0x0011
 
 /* PCIe IRQ registers */
 #define KVASER_PCIEFD_IRQ_REG 0x40
@@ -282,12 +282,24 @@ static const struct can_bittiming_const kvaser_pciefd_bittiming_const = {
 };
 
 static struct pci_device_id kvaser_pciefd_id_table[] = {
-	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_4HS_ID), },
-	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_2HS_ID), },
-	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_HS_ID), },
-	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_HS_ID), },
-	{ PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_2HS_ID), },
-	{ 0,},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_4HS_DEVICE_ID),
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_2HS_V2_DEVICE_ID),
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_HS_V2_DEVICE_ID),
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_HS_V2_DEVICE_ID),
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_2HS_V2_DEVICE_ID),
+	},
+	{
+		0,
+	},
 };
 MODULE_DEVICE_TABLE(pci, kvaser_pciefd_id_table);
 
-- 
2.40.0


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

* [PATCH 09/12] can: kvaser_pciefd: Change return type for kvaser_pciefd_{receive,transmit,set_tx}_irq()
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (7 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 08/12] can: kvaser_pciefd: Rename device ID defines Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 10/12] can: kvaser_pciefd: Add len8_dlc support Jimmy Assarsson
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Change return type to void for kvaser_pciefd_transmit_irq(),
kvaser_pciefd_receive_irq() and kvaser_pciefd_set_tx_irq().
These functions always return zero.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index bf3fa51069a9..feef044c6b0a 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -338,7 +338,7 @@ static void kvaser_pciefd_disable_err_gen(struct kvaser_pciefd_can *can)
 	spin_unlock_irqrestore(&can->lock, irq);
 }
 
-static int kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can)
+static void kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can)
 {
 	u32 msk;
 
@@ -349,8 +349,6 @@ static int kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can)
 	      KVASER_PCIEFD_KCAN_IRQ_TAR;
 
 	iowrite32(msk, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-
-	return 0;
 }
 
 static inline void kvaser_pciefd_set_skb_timestamp(const struct kvaser_pciefd *pcie,
@@ -1456,7 +1454,7 @@ static int kvaser_pciefd_read_buffer(struct kvaser_pciefd *pcie, int dma_buf)
 	return res;
 }
 
-static int kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
+static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
 {
 	u32 irq;
 
@@ -1482,10 +1480,9 @@ static int kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
 		dev_err(&pcie->pci->dev, "DMA IRQ error 0x%08X\n", irq);
 
 	iowrite32(irq, pcie->reg_base + KVASER_PCIEFD_SRB_IRQ_REG);
-	return 0;
 }
 
-static int kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
+static void kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
 {
 	u32 irq = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
 
@@ -1503,7 +1500,6 @@ static int kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
 		netdev_err(can->can.dev, "Rx FIFO overflow\n");
 
 	iowrite32(irq, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-	return 0;
 }
 
 static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
-- 
2.40.0


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

* [PATCH 10/12] can: kvaser_pciefd: Add len8_dlc support
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (8 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 09/12] can: kvaser_pciefd: Change return type for kvaser_pciefd_{receive,transmit,set_tx}_irq() Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 11/12] can: kvaser_pciefd: Refactor code Jimmy Assarsson
                   ` (6 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Add support for the Classical CAN raw DLC functionality to send and receive
DLC values from 9 .. 15.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index feef044c6b0a..3237c71afd2b 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -572,15 +572,19 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
 		p->header[0] |= KVASER_PCIEFD_RPACKET_IDE;
 
 	p->header[0] |= cf->can_id & CAN_EFF_MASK;
-	p->header[1] |= can_fd_len2dlc(cf->len) << KVASER_PCIEFD_RPACKET_DLC_SHIFT;
 	p->header[1] |= KVASER_PCIEFD_TPACKET_AREQ;
 
 	if (can_is_canfd_skb(skb)) {
+		p->header[1] |= can_fd_len2dlc(cf->len) << KVASER_PCIEFD_RPACKET_DLC_SHIFT;
 		p->header[1] |= KVASER_PCIEFD_RPACKET_FDF;
 		if (cf->flags & CANFD_BRS)
 			p->header[1] |= KVASER_PCIEFD_RPACKET_BRS;
 		if (cf->flags & CANFD_ESI)
 			p->header[1] |= KVASER_PCIEFD_RPACKET_ESI;
+	} else {
+		p->header[1] |= can_get_cc_dlc((struct can_frame *)cf,
+					       can->can.ctrlmode)
+				<< KVASER_PCIEFD_RPACKET_DLC_SHIFT;
 	}
 
 	p->header[1] |= seq & KVASER_PCIEFD_PACKET_SEQ_MSK;
@@ -816,7 +820,8 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 
 		can->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
 					      CAN_CTRLMODE_FD |
-					      CAN_CTRLMODE_FD_NON_ISO;
+					      CAN_CTRLMODE_FD_NON_ISO |
+					      CAN_CTRLMODE_CC_LEN8_DLC;
 
 		status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);
 		if (!(status & KVASER_PCIEFD_KCAN_STAT_FD)) {
@@ -977,12 +982,14 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 	struct can_priv *priv;
 	struct net_device_stats *stats;
 	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
+	u8 dlc;
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
 
 	priv = &pcie->can[ch_id]->can;
 	stats = &priv->dev->stats;
+	dlc = (p->header[1] >> KVASER_PCIEFD_RPACKET_DLC_SHIFT) & 0xf;
 
 	if (p->header[1] & KVASER_PCIEFD_RPACKET_FDF) {
 		skb = alloc_canfd_skb(priv->dev, &cf);
@@ -991,6 +998,7 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 			return -ENOMEM;
 		}
 
+		cf->len = can_fd_dlc2len(dlc);
 		if (p->header[1] & KVASER_PCIEFD_RPACKET_BRS)
 			cf->flags |= CANFD_BRS;
 
@@ -1002,14 +1010,13 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 			stats->rx_dropped++;
 			return -ENOMEM;
 		}
+		can_frame_set_cc_len((struct can_frame *)cf, dlc, priv->ctrlmode);
 	}
 
 	cf->can_id = p->header[0] & CAN_EFF_MASK;
 	if (p->header[0] & KVASER_PCIEFD_RPACKET_IDE)
 		cf->can_id |= CAN_EFF_FLAG;
 
-	cf->len = can_fd_dlc2len(p->header[1] >> KVASER_PCIEFD_RPACKET_DLC_SHIFT);
-
 	if (p->header[0] & KVASER_PCIEFD_RPACKET_RTR) {
 		cf->can_id |= CAN_RTR_FLAG;
 	} else {
-- 
2.40.0


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

* [PATCH 11/12] can: kvaser_pciefd: Refactor code
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (9 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 10/12] can: kvaser_pciefd: Add len8_dlc support Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23 11:27   ` Vincent MAILHOL
  2023-05-23  9:43 ` [PATCH 12/12] can: kvaser_pciefd: Use TX FIFO size read from CAN controller Jimmy Assarsson
                   ` (5 subsequent siblings)
  16 siblings, 1 reply; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Refactor code;
 - Format code
 - Replace constants with macros
 - Rename variables and macros
 - Remove intermediate variable
 - Add/remove blank lines
 - Add function to fetch channel id from Rx packets
 - Reduce scope of variables

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 236 +++++++++++++-------------------
 1 file changed, 97 insertions(+), 139 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 3237c71afd2b..0575bb84b280 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -53,7 +53,7 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_KCAN_CMD_REG 0x400
 #define KVASER_PCIEFD_KCAN_IEN_REG 0x408
 #define KVASER_PCIEFD_KCAN_IRQ_REG 0x410
-#define KVASER_PCIEFD_KCAN_TX_NPACKETS_REG 0x414
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG 0x414
 #define KVASER_PCIEFD_KCAN_STAT_REG 0x418
 #define KVASER_PCIEFD_KCAN_MODE_REG 0x41c
 #define KVASER_PCIEFD_KCAN_BTRN_REG 0x420
@@ -81,9 +81,9 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
 #define KVASER_PCIEFD_IRQ_SRB BIT(4)
 
-#define KVASER_PCIEFD_SYSID_NRCHAN_SHIFT 24
-#define KVASER_PCIEFD_SYSID_MAJOR_VER_SHIFT 16
-#define KVASER_PCIEFD_SYSID_BUILD_VER_SHIFT 1
+#define KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT 24
+#define KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT 16
+#define KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT 1
 
 /* Reset DMA buffer 0, 1 and FIFO offset */
 #define KVASER_PCIEFD_SRB_CMD_RDB0 BIT(4)
@@ -142,7 +142,7 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* Transmitter unaligned */
 #define KVASER_PCIEFD_KCAN_IRQ_TAL BIT(17)
 
-#define KVASER_PCIEFD_KCAN_TX_NPACKETS_MAX_SHIFT 16
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT 16
 
 #define KVASER_PCIEFD_KCAN_STAT_SEQNO_SHIFT 24
 /* Abort request */
@@ -159,9 +159,9 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_KCAN_STAT_CAP BIT(16)
 /* Controller got CAN FD capability */
 #define KVASER_PCIEFD_KCAN_STAT_FD BIT(19)
-#define KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MSK (KVASER_PCIEFD_KCAN_STAT_AR | \
-	KVASER_PCIEFD_KCAN_STAT_BOFF | KVASER_PCIEFD_KCAN_STAT_RMR | \
-	KVASER_PCIEFD_KCAN_STAT_IRM)
+#define KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MASK                         \
+	(KVASER_PCIEFD_KCAN_STAT_AR | KVASER_PCIEFD_KCAN_STAT_BOFF | \
+	 KVASER_PCIEFD_KCAN_STAT_RMR | KVASER_PCIEFD_KCAN_STAT_IRM)
 
 /* Reset mode */
 #define KVASER_PCIEFD_KCAN_MODE_RM BIT(8)
@@ -178,9 +178,13 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* Classic CAN mode */
 #define KVASER_PCIEFD_KCAN_MODE_CCM BIT(31)
 
+#define KVASER_PCIEFD_KCAN_BTRN_BRP_MASK 0x1fff
 #define KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT 13
+#define KVASER_PCIEFD_KCAN_BTRN_SJW_MASK 0xf
 #define KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT 17
+#define KVASER_PCIEFD_KCAN_BTRN_TSEG1_MASK 0x1ff
 #define KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT 26
+#define KVASER_PCIEFD_KCAN_BTRN_TSEG2_MASK 0x1f
 
 #define KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT 16
 
@@ -196,9 +200,11 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_PACK_TYPE_BUS_LOAD 9
 
 /* Kvaser KCAN packet common definitions */
-#define KVASER_PCIEFD_PACKET_SEQ_MSK 0xff
+#define KVASER_PCIEFD_PACKET_SEQ_MASK 0xff
 #define KVASER_PCIEFD_PACKET_CHID_SHIFT 25
+#define KVASER_PCIEFD_PACKET_CHID_MASK 0x7
 #define KVASER_PCIEFD_PACKET_TYPE_SHIFT 28
+#define KVASER_PCIEFD_PACKET_TYPE_MASK 0xf
 
 /* Kvaser KCAN TDATA and RDATA first word */
 #define KVASER_PCIEFD_RPACKET_IDE BIT(30)
@@ -303,6 +309,11 @@ static struct pci_device_id kvaser_pciefd_id_table[] = {
 };
 MODULE_DEVICE_TABLE(pci, kvaser_pciefd_id_table);
 
+static inline u8 kvaser_pciefd_rx_packet_get_ch_id(struct kvaser_pciefd_rx_packet *p)
+{
+	return (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & KVASER_PCIEFD_PACKET_CHID_MASK;
+}
+
 static void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
 {
 	u32 cmd;
@@ -364,7 +375,6 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	spin_lock_irqsave(&can->lock, irq);
-
 	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
 	if (can->can.ctrlmode & CAN_CTRLMODE_FD) {
 		mode &= ~KVASER_PCIEFD_KCAN_MODE_CCM;
@@ -381,7 +391,6 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
 		mode |= KVASER_PCIEFD_KCAN_MODE_LOM;
 	else
 		mode &= ~KVASER_PCIEFD_KCAN_MODE_LOM;
-
 	mode |= KVASER_PCIEFD_KCAN_MODE_EEN;
 	mode |= KVASER_PCIEFD_KCAN_MODE_EPEN;
 	/* Use ACK packet type */
@@ -401,7 +410,6 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
 	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
 	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
 		  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-
 	status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);
 	if (status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
 		u32 cmd;
@@ -418,7 +426,6 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
 		mode |= KVASER_PCIEFD_KCAN_MODE_RM;
 		iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
 	}
-
 	spin_unlock_irqrestore(&can->lock, irq);
 }
 
@@ -428,7 +435,6 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	del_timer(&can->bec_poll_timer);
-
 	if (!completion_done(&can->flush_comp))
 		kvaser_pciefd_start_controller_flush(can);
 
@@ -441,10 +447,8 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 	spin_lock_irqsave(&can->lock, irq);
 	iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-
 	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
 		  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-
 	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
 	mode &= ~KVASER_PCIEFD_KCAN_MODE_RM;
 	iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
@@ -461,7 +465,6 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 
 	kvaser_pciefd_set_tx_irq(can);
 	kvaser_pciefd_setup_controller(can);
-
 	can->can.state = CAN_STATE_ERROR_ACTIVE;
 	netif_wake_queue(can->can.dev);
 	can->bec.txerr = 0;
@@ -480,7 +483,6 @@ static void kvaser_pciefd_pwm_stop(struct kvaser_pciefd_can *can)
 	spin_lock_irqsave(&can->lock, irq);
 	pwm_ctrl = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
 	top = (pwm_ctrl >> KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT) & 0xff;
-
 	/* Set duty cycle to zero */
 	pwm_ctrl |= top;
 	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
@@ -495,10 +497,8 @@ static void kvaser_pciefd_pwm_start(struct kvaser_pciefd_can *can)
 
 	kvaser_pciefd_pwm_stop(can);
 	spin_lock_irqsave(&can->lock, irq);
-
 	/* Set frequency to 500 KHz*/
 	top = can->kv_pcie->bus_freq / (2 * 500000) - 1;
-
 	pwm_ctrl = top & 0xff;
 	pwm_ctrl |= (top & 0xff) << KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT;
 	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
@@ -561,7 +561,6 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
 	int seq = can->echo_idx;
 
 	memset(p, 0, sizeof(*p));
-
 	if (can->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
 		p->header[1] |= KVASER_PCIEFD_TPACKET_SMS;
 
@@ -587,7 +586,7 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
 				<< KVASER_PCIEFD_RPACKET_DLC_SHIFT;
 	}
 
-	p->header[1] |= seq & KVASER_PCIEFD_PACKET_SEQ_MSK;
+	p->header[1] |= seq & KVASER_PCIEFD_PACKET_SEQ_MASK;
 
 	packet_size = cf->len;
 	memcpy(p->data, cf->data, packet_size);
@@ -601,16 +600,15 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 	struct kvaser_pciefd_can *can = netdev_priv(netdev);
 	unsigned long irq_flags;
 	struct kvaser_pciefd_tx_packet packet;
-	int nwords;
+	int nr_words;
 	u8 count;
 
 	if (can_dev_dropped_skb(netdev, skb))
 		return NETDEV_TX_OK;
 
-	nwords = kvaser_pciefd_prepare_tx_packet(&packet, can, skb);
+	nr_words = kvaser_pciefd_prepare_tx_packet(&packet, can, skb);
 
 	spin_lock_irqsave(&can->echo_lock, irq_flags);
-
 	/* Prepare and save echo skb in internal slot */
 	can_put_echo_skb(skb, netdev, can->echo_idx, 0);
 
@@ -623,13 +621,13 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 	iowrite32(packet.header[1],
 		  can->reg_base + KVASER_PCIEFD_KCAN_FIFO_REG);
 
-	if (nwords) {
-		u32 data_last = ((u32 *)packet.data)[nwords - 1];
+	if (nr_words) {
+		u32 data_last = ((u32 *)packet.data)[nr_words - 1];
 
 		/* Write data to fifo, except last word */
 		iowrite32_rep(can->reg_base +
 			      KVASER_PCIEFD_KCAN_FIFO_REG, packet.data,
-			      nwords - 1);
+			      nr_words - 1);
 		/* Write last word to end of fifo */
 		__raw_writel(data_last, can->reg_base +
 			     KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
@@ -638,15 +636,13 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 		__raw_writel(0, can->reg_base +
 			     KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
 	}
-
-	count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG);
+	count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
 	/* No room for a new message, stop the queue until at least one
 	 * successful transmit
 	 */
 	if (count >= KVASER_PCIEFD_CAN_TX_MAX_COUNT ||
 	    can->can.echo_skb[can->echo_idx])
 		netif_stop_queue(netdev);
-
 	spin_unlock_irqrestore(&can->echo_lock, irq_flags);
 
 	return NETDEV_TX_OK;
@@ -664,25 +660,24 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
 	else
 		bt = &can->can.bittiming;
 
-	btrn = ((bt->phase_seg2 - 1) & 0x1f) <<
-	       KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT |
-	       (((bt->prop_seg + bt->phase_seg1) - 1) & 0x1ff) <<
-	       KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT |
-	       ((bt->sjw - 1) & 0xf) << KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT |
-	       ((bt->brp - 1) & 0x1fff);
+	btrn = ((bt->phase_seg2 - 1) & KVASER_PCIEFD_KCAN_BTRN_TSEG2_MASK)
+		       << KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT |
+	       (((bt->prop_seg + bt->phase_seg1) - 1) &
+		KVASER_PCIEFD_KCAN_BTRN_TSEG1_MASK)
+		       << KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT |
+	       ((bt->sjw - 1) & KVASER_PCIEFD_KCAN_BTRN_SJW_MASK)
+		       << KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT |
+	       ((bt->brp - 1) & KVASER_PCIEFD_KCAN_BTRN_BRP_MASK);
 
 	spin_lock_irqsave(&can->lock, irq_flags);
 	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
-
 	/* Put the circuit in reset mode */
 	iowrite32(mode | KVASER_PCIEFD_KCAN_MODE_RM,
 		  can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
 
 	/* Can only set bittiming if in reset mode */
 	ret = readl_poll_timeout(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG,
-				 test, test & KVASER_PCIEFD_KCAN_MODE_RM,
-				 0, 10);
-
+				 test, test & KVASER_PCIEFD_KCAN_MODE_RM, 0, 10);
 	if (ret) {
 		spin_unlock_irqrestore(&can->lock, irq_flags);
 		return -EBUSY;
@@ -692,11 +687,10 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
 		iowrite32(btrn, can->reg_base + KVASER_PCIEFD_KCAN_BTRD_REG);
 	else
 		iowrite32(btrn, can->reg_base + KVASER_PCIEFD_KCAN_BTRN_REG);
-
 	/* Restore previous reset mode status */
 	iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
-
 	spin_unlock_irqrestore(&can->lock, irq_flags);
+
 	return 0;
 }
 
@@ -734,6 +728,7 @@ static int kvaser_pciefd_get_berr_counter(const struct net_device *ndev,
 
 	bec->rxerr = can->bec.rxerr;
 	bec->txerr = can->bec.txerr;
+
 	return 0;
 }
 
@@ -765,7 +760,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 	for (i = 0; i < pcie->nr_channels; i++) {
 		struct net_device *netdev;
 		struct kvaser_pciefd_can *can;
-		u32 status, tx_npackets;
+		u32 status, tx_nr_packets;
 
 		netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
 				      KVASER_PCIEFD_CAN_TX_MAX_COUNT);
@@ -777,7 +772,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		netdev->ethtool_ops = &kvaser_pciefd_ethtool_ops;
 		can->reg_base = pcie->reg_base + KVASER_PCIEFD_KCAN0_BASE +
 				i * KVASER_PCIEFD_KCAN_BASE_OFFSET;
-
 		can->kv_pcie = pcie;
 		can->cmd_seq = 0;
 		can->err_rep_cnt = 0;
@@ -786,15 +780,13 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 
 		init_completion(&can->start_comp);
 		init_completion(&can->flush_comp);
-		timer_setup(&can->bec_poll_timer, kvaser_pciefd_bec_poll_timer,
-			    0);
+		timer_setup(&can->bec_poll_timer, kvaser_pciefd_bec_poll_timer, 0);
 
 		/* Disable Bus load reporting */
 		iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_BUS_LOAD_REG);
 
-		tx_npackets = ioread32(can->reg_base +
-				       KVASER_PCIEFD_KCAN_TX_NPACKETS_REG);
-		if (((tx_npackets >> KVASER_PCIEFD_KCAN_TX_NPACKETS_MAX_SHIFT) &
+		tx_nr_packets = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
+		if (((tx_nr_packets >> KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT) &
 		      0xff) < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
 			dev_err(&pcie->pci->dev,
 				"Max Tx count is smaller than expected\n");
@@ -808,16 +800,13 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		can->echo_idx = 0;
 		spin_lock_init(&can->echo_lock);
 		spin_lock_init(&can->lock);
+
 		can->can.bittiming_const = &kvaser_pciefd_bittiming_const;
 		can->can.data_bittiming_const = &kvaser_pciefd_bittiming_const;
-
 		can->can.do_set_bittiming = kvaser_pciefd_set_nominal_bittiming;
-		can->can.do_set_data_bittiming =
-			kvaser_pciefd_set_data_bittiming;
-
+		can->can.do_set_data_bittiming = kvaser_pciefd_set_data_bittiming;
 		can->can.do_set_mode = kvaser_pciefd_set_mode;
 		can->can.do_get_berr_counter = kvaser_pciefd_get_berr_counter;
-
 		can->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
 					      CAN_CTRLMODE_FD |
 					      CAN_CTRLMODE_FD_NON_ISO |
@@ -836,7 +825,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 			can->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
 
 		netdev->flags |= IFF_ECHO;
-
 		SET_NETDEV_DEV(netdev, &pcie->pci->dev);
 
 		iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
@@ -898,18 +886,16 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 	for (i = 0; i < KVASER_PCIEFD_DMA_COUNT; i++) {
 		unsigned int offset = KVASER_PCIEFD_DMA_MAP_BASE + 8 * i;
 
-		pcie->dma_data[i] =
-			dmam_alloc_coherent(&pcie->pci->dev,
-					    KVASER_PCIEFD_DMA_SIZE,
-					    &dma_addr[i],
-					    GFP_KERNEL);
+		pcie->dma_data[i] = dmam_alloc_coherent(&pcie->pci->dev,
+							KVASER_PCIEFD_DMA_SIZE,
+							&dma_addr[i],
+							GFP_KERNEL);
 
 		if (!pcie->dma_data[i] || !dma_addr[i]) {
 			dev_err(&pcie->pci->dev, "Rx dma_alloc(%u) failure\n",
 				KVASER_PCIEFD_DMA_SIZE);
 			return -ENOMEM;
 		}
-
 		kvaser_pciefd_write_dma_map(pcie, dma_addr[i], offset);
 	}
 
@@ -917,7 +903,6 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 	iowrite32(KVASER_PCIEFD_SRB_CMD_FOR | KVASER_PCIEFD_SRB_CMD_RDB0 |
 		  KVASER_PCIEFD_SRB_CMD_RDB1,
 		  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
-
 	/* Empty Rx FIFO */
 	srb_packet_count = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG) &
 			   KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK;
@@ -942,22 +927,21 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 
 static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 {
-	u32 sysid, srb_status, build;
+	u32 version, srb_status, build;
 
-	sysid = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_VERSION_REG);
+	version = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_VERSION_REG);
 	pcie->nr_channels = min(KVASER_PCIEFD_MAX_CAN_CHANNELS,
-				((sysid >> KVASER_PCIEFD_SYSID_NRCHAN_SHIFT) & 0xff));
+				((version >> KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT) & 0xff));
 
 	build = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_BUILD_REG);
 	dev_dbg(&pcie->pci->dev, "Version %u.%u.%u\n",
-		(sysid >> KVASER_PCIEFD_SYSID_MAJOR_VER_SHIFT) & 0xff,
-		sysid & 0xff,
-		(build >> KVASER_PCIEFD_SYSID_BUILD_VER_SHIFT) & 0x7fff);
+		(version >> KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT) & 0xff,
+		version & 0xff,
+		(build >> KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT) & 0x7fff);
 
 	srb_status = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_STAT_REG);
 	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DMA)) {
-		dev_err(&pcie->pci->dev,
-			"Hardware without DMA is not supported\n");
+		dev_err(&pcie->pci->dev, "Hardware without DMA is not supported\n");
 		return -ENODEV;
 	}
 
@@ -967,9 +951,9 @@ static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 	pcie->freq_to_ticks_div = pcie->freq / 1000000;
 	if (pcie->freq_to_ticks_div == 0)
 		pcie->freq_to_ticks_div = 1;
-
 	/* Turn off all loopback functionality */
 	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_LOOP_REG);
+
 	return 0;
 }
 
@@ -980,34 +964,31 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 	struct sk_buff *skb;
 	struct canfd_frame *cf;
 	struct can_priv *priv;
-	struct net_device_stats *stats;
-	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
+	u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
 	u8 dlc;
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
 
 	priv = &pcie->can[ch_id]->can;
-	stats = &priv->dev->stats;
 	dlc = (p->header[1] >> KVASER_PCIEFD_RPACKET_DLC_SHIFT) & 0xf;
 
 	if (p->header[1] & KVASER_PCIEFD_RPACKET_FDF) {
 		skb = alloc_canfd_skb(priv->dev, &cf);
 		if (!skb) {
-			stats->rx_dropped++;
+			priv->dev->stats.rx_dropped++;
 			return -ENOMEM;
 		}
 
 		cf->len = can_fd_dlc2len(dlc);
 		if (p->header[1] & KVASER_PCIEFD_RPACKET_BRS)
 			cf->flags |= CANFD_BRS;
-
 		if (p->header[1] & KVASER_PCIEFD_RPACKET_ESI)
 			cf->flags |= CANFD_ESI;
 	} else {
 		skb = alloc_can_skb(priv->dev, (struct can_frame **)&cf);
 		if (!skb) {
-			stats->rx_dropped++;
+			priv->dev->stats.rx_dropped++;
 			return -ENOMEM;
 		}
 		can_frame_set_cc_len((struct can_frame *)cf, dlc, priv->ctrlmode);
@@ -1021,10 +1002,9 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 		cf->can_id |= CAN_RTR_FLAG;
 	} else {
 		memcpy(cf->data, data, cf->len);
-
-		stats->rx_bytes += cf->len;
+		priv->dev->stats.rx_bytes += cf->len;
 	}
-	stats->rx_packets++;
+	priv->dev->stats.rx_packets++;
 	kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
 
 	return netif_rx(skb);
@@ -1045,7 +1025,6 @@ static void kvaser_pciefd_change_state(struct kvaser_pciefd_can *can,
 		spin_lock_irqsave(&can->lock, irq_flags);
 		netif_stop_queue(can->can.dev);
 		spin_unlock_irqrestore(&can->lock, irq_flags);
-
 		/* Prevent CAN controller from auto recover from bus off */
 		if (!can->can.restart_ms) {
 			kvaser_pciefd_start_controller_flush(can);
@@ -1063,7 +1042,7 @@ static void kvaser_pciefd_packet_to_state(struct kvaser_pciefd_rx_packet *p,
 	if (p->header[0] & KVASER_PCIEFD_SPACK_BOFF ||
 	    p->header[0] & KVASER_PCIEFD_SPACK_IRM)
 		*new_state = CAN_STATE_BUS_OFF;
-	else if (bec->txerr >= 255 ||  bec->rxerr >= 255)
+	else if (bec->txerr >= 255 || bec->rxerr >= 255)
 		*new_state = CAN_STATE_BUS_OFF;
 	else if (p->header[1] & KVASER_PCIEFD_SPACK_EPLR)
 		*new_state = CAN_STATE_ERROR_PASSIVE;
@@ -1088,22 +1067,16 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
 	struct net_device *ndev = can->can.dev;
 	struct sk_buff *skb;
 	struct can_frame *cf = NULL;
-	struct net_device_stats *stats = &ndev->stats;
 
 	old_state = can->can.state;
 
 	bec.txerr = p->header[0] & 0xff;
 	bec.rxerr = (p->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff;
 
-	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state,
-				      &rx_state);
-
+	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state, &rx_state);
 	skb = alloc_can_err_skb(ndev, &cf);
-
 	if (new_state != old_state) {
-		kvaser_pciefd_change_state(can, cf, new_state, tx_state,
-					   rx_state);
-
+		kvaser_pciefd_change_state(can, cf, new_state, tx_state, rx_state);
 		if (old_state == CAN_STATE_BUS_OFF &&
 		    new_state == CAN_STATE_ERROR_ACTIVE &&
 		    can->can.restart_ms) {
@@ -1116,25 +1089,25 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
 	can->err_rep_cnt++;
 	can->can.can_stats.bus_error++;
 	if (p->header[1] & KVASER_PCIEFD_EPACK_DIR_TX)
-		stats->tx_errors++;
+		ndev->stats.tx_errors++;
 	else
-		stats->rx_errors++;
+		ndev->stats.rx_errors++;
 
 	can->bec.txerr = bec.txerr;
 	can->bec.rxerr = bec.rxerr;
 
 	if (!skb) {
-		stats->rx_dropped++;
+		ndev->stats.rx_dropped++;
 		return -ENOMEM;
 	}
 
 	kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
 	cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_CNT;
-
 	cf->data[6] = bec.txerr;
 	cf->data[7] = bec.rxerr;
 
 	netif_rx(skb);
+
 	return 0;
 }
 
@@ -1142,19 +1115,19 @@ static int kvaser_pciefd_handle_error_packet(struct kvaser_pciefd *pcie,
 					     struct kvaser_pciefd_rx_packet *p)
 {
 	struct kvaser_pciefd_can *can;
-	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
+	u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
 
 	can = pcie->can[ch_id];
-
 	kvaser_pciefd_rx_error_frame(can, p);
 	if (can->err_rep_cnt >= KVASER_PCIEFD_MAX_ERR_REP)
 		/* Do not report more errors, until bec_poll_timer expires */
 		kvaser_pciefd_disable_err_gen(can);
 	/* Start polling the error counters */
 	mod_timer(&can->bec_poll_timer, KVASER_PCIEFD_BEC_POLL_FREQ);
+
 	return 0;
 }
 
@@ -1169,9 +1142,7 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
 	bec.txerr = p->header[0] & 0xff;
 	bec.rxerr = (p->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff;
 
-	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state,
-				      &rx_state);
-
+	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state, &rx_state);
 	if (new_state != old_state) {
 		struct net_device *ndev = can->can.dev;
 		struct sk_buff *skb;
@@ -1179,15 +1150,11 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
 
 		skb = alloc_can_err_skb(ndev, &cf);
 		if (!skb) {
-			struct net_device_stats *stats = &ndev->stats;
-
-			stats->rx_dropped++;
+			ndev->stats.rx_dropped++;
 			return -ENOMEM;
 		}
 
-		kvaser_pciefd_change_state(can, cf, new_state, tx_state,
-					   rx_state);
-
+		kvaser_pciefd_change_state(can, cf, new_state, tx_state, rx_state);
 		if (old_state == CAN_STATE_BUS_OFF &&
 		    new_state == CAN_STATE_ERROR_ACTIVE &&
 		    can->can.restart_ms) {
@@ -1217,7 +1184,7 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 	struct kvaser_pciefd_can *can;
 	u8 cmdseq;
 	u32 status;
-	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
+	u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1231,7 +1198,7 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 	if (p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
 	    p->header[0] & KVASER_PCIEFD_SPACK_RMCD &&
 	    p->header[1] & KVASER_PCIEFD_SPACK_AUTO &&
-	    cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MSK) &&
+	    cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK) &&
 	    status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
 		u32 cmd;
 
@@ -1242,26 +1209,24 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
 	} else if (p->header[0] & KVASER_PCIEFD_SPACK_IDET &&
 		   p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
-		   cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MSK) &&
+		   cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK) &&
 		   status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
 		/* Reset detected, send end of flush if no packet are in FIFO */
-		u8 count = ioread32(can->reg_base +
-				    KVASER_PCIEFD_KCAN_TX_NPACKETS_REG) & 0xff;
+		u8 count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
 
 		if (!count)
 			iowrite32(KVASER_PCIEFD_KCAN_CTRL_EFLUSH,
 				  can->reg_base + KVASER_PCIEFD_KCAN_CTRL_REG);
 	} else if (!(p->header[1] & KVASER_PCIEFD_SPACK_AUTO) &&
-		   cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MSK)) {
+		   cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK)) {
 		/* Response to status request received */
 		kvaser_pciefd_handle_status_resp(can, p);
 		if (can->can.state != CAN_STATE_BUS_OFF &&
 		    can->can.state != CAN_STATE_ERROR_ACTIVE) {
-			mod_timer(&can->bec_poll_timer,
-				  KVASER_PCIEFD_BEC_POLL_FREQ);
+			mod_timer(&can->bec_poll_timer, KVASER_PCIEFD_BEC_POLL_FREQ);
 		}
 	} else if (p->header[0] & KVASER_PCIEFD_SPACK_RMCD &&
-		   !(status & KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MSK)) {
+		   !(status & KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MASK)) {
 		/* Reset to bus on detected */
 		if (!completion_done(&can->start_comp))
 			complete(&can->start_comp);
@@ -1274,12 +1239,10 @@ static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
 					     struct kvaser_pciefd_rx_packet *p)
 {
 	struct sk_buff *skb;
-	struct net_device_stats *stats = &can->can.dev->stats;
 	struct can_frame *cf;
 
 	skb = alloc_can_err_skb(can->can.dev, &cf);
-
-	stats->tx_errors++;
+	can->can.dev->stats.tx_errors++;
 	if (p->header[0] & KVASER_PCIEFD_APACKET_ABL) {
 		if (skb)
 			cf->can_id |= CAN_ERR_LOSTARB;
@@ -1293,7 +1256,7 @@ static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
 		kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
 		netif_rx(skb);
 	} else {
-		stats->rx_dropped++;
+		can->can.dev->stats.rx_dropped++;
 		netdev_warn(can->can.dev, "No memory left for err_skb\n");
 	}
 }
@@ -1303,7 +1266,7 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 {
 	struct kvaser_pciefd_can *can;
 	bool one_shot_fail = false;
-	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
+	u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1321,27 +1284,24 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 	if (p->header[0] & KVASER_PCIEFD_APACKET_FLU) {
 		netdev_dbg(can->can.dev, "Packet was flushed\n");
 	} else {
-		int echo_idx = p->header[0] & KVASER_PCIEFD_PACKET_SEQ_MSK;
-		int dlc;
+		int echo_idx = p->header[0] & KVASER_PCIEFD_PACKET_SEQ_MASK;
+		int len;
 		u8 count;
 		struct sk_buff *skb;
 
 		skb = can->can.echo_skb[echo_idx];
 		if (skb)
 			kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
-		dlc = can_get_echo_skb(can->can.dev, echo_idx, NULL);
-		count = ioread32(can->reg_base +
-				    KVASER_PCIEFD_KCAN_TX_NPACKETS_REG) & 0xff;
+		len = can_get_echo_skb(can->can.dev, echo_idx, NULL);
+		count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
 
 		if (count < KVASER_PCIEFD_CAN_TX_MAX_COUNT &&
 		    netif_queue_stopped(can->can.dev))
 			netif_wake_queue(can->can.dev);
 
 		if (!one_shot_fail) {
-			struct net_device_stats *stats = &can->can.dev->stats;
-
-			stats->tx_bytes += dlc;
-			stats->tx_packets++;
+			can->can.dev->stats.tx_bytes += len;
+			can->can.dev->stats.tx_packets++;
 		}
 	}
 
@@ -1352,7 +1312,7 @@ static int kvaser_pciefd_handle_eflush_packet(struct kvaser_pciefd *pcie,
 					      struct kvaser_pciefd_rx_packet *p)
 {
 	struct kvaser_pciefd_can *can;
-	u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
+	u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1391,7 +1351,7 @@ static int kvaser_pciefd_read_packet(struct kvaser_pciefd *pcie, int *start_pos,
 	pos += 2;
 	p->timestamp = le64_to_cpu(timestamp);
 
-	type = (p->header[1] >> KVASER_PCIEFD_PACKET_TYPE_SHIFT) & 0xf;
+	type = (p->header[1] >> KVASER_PCIEFD_PACKET_TYPE_SHIFT) & KVASER_PCIEFD_PACKET_TYPE_MASK;
 	switch (type) {
 	case KVASER_PCIEFD_PACK_TYPE_DATA:
 		ret = kvaser_pciefd_handle_data_packet(pcie, p, &buffer[pos]);
@@ -1541,13 +1501,12 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
 static void kvaser_pciefd_teardown_can_ctrls(struct kvaser_pciefd *pcie)
 {
 	int i;
-	struct kvaser_pciefd_can *can;
 
 	for (i = 0; i < pcie->nr_channels; i++) {
-		can = pcie->can[i];
+		struct kvaser_pciefd_can *can = pcie->can[i];
+
 		if (can) {
-			iowrite32(0,
-				  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
+			iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 			kvaser_pciefd_pwm_stop(can);
 			free_candev(can->can.dev);
 		}
@@ -1648,14 +1607,13 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 
 static void kvaser_pciefd_remove_all_ctrls(struct kvaser_pciefd *pcie)
 {
-	struct kvaser_pciefd_can *can;
 	int i;
 
 	for (i = 0; i < pcie->nr_channels; i++) {
-		can = pcie->can[i];
+		struct kvaser_pciefd_can *can = pcie->can[i];
+
 		if (can) {
-			iowrite32(0,
-				  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
+			iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 			unregister_candev(can->can.dev);
 			del_timer(&can->bec_poll_timer);
 			kvaser_pciefd_pwm_stop(can);
-- 
2.40.0


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

* [PATCH 12/12] can: kvaser_pciefd: Use TX FIFO size read from CAN controller
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (10 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 11/12] can: kvaser_pciefd: Refactor code Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices Jimmy Assarsson
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Use the TX FIFO size read from CAN controller register, instead of using
hard coded value.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 20 ++++++--------------
 1 file changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 0575bb84b280..5f67414e2875 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -640,8 +640,7 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 	/* No room for a new message, stop the queue until at least one
 	 * successful transmit
 	 */
-	if (count >= KVASER_PCIEFD_CAN_TX_MAX_COUNT ||
-	    can->can.echo_skb[can->echo_idx])
+	if (count >= can->can.echo_skb_max || can->can.echo_skb[can->echo_idx])
 		netif_stop_queue(netdev);
 	spin_unlock_irqrestore(&can->echo_lock, irq_flags);
 
@@ -760,7 +759,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 	for (i = 0; i < pcie->nr_channels; i++) {
 		struct net_device *netdev;
 		struct kvaser_pciefd_can *can;
-		u32 status, tx_nr_packets;
+		u32 status, tx_nr_packets, tx_nr_packets_max;
 
 		netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
 				      KVASER_PCIEFD_CAN_TX_MAX_COUNT);
@@ -786,17 +785,11 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_BUS_LOAD_REG);
 
 		tx_nr_packets = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
-		if (((tx_nr_packets >> KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT) &
-		      0xff) < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
-			dev_err(&pcie->pci->dev,
-				"Max Tx count is smaller than expected\n");
-
-			free_candev(netdev);
-			return -ENODEV;
-		}
+		tx_nr_packets_max =
+			(tx_nr_packets >> KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT) & 0xff;
 
 		can->can.clock.freq = pcie->freq;
-		can->can.echo_skb_max = KVASER_PCIEFD_CAN_TX_MAX_COUNT;
+		can->can.echo_skb_max = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);
 		can->echo_idx = 0;
 		spin_lock_init(&can->echo_lock);
 		spin_lock_init(&can->lock);
@@ -1295,8 +1288,7 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 		len = can_get_echo_skb(can->can.dev, echo_idx, NULL);
 		count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
 
-		if (count < KVASER_PCIEFD_CAN_TX_MAX_COUNT &&
-		    netif_queue_stopped(can->can.dev))
+		if (count < can->can.echo_skb_max && netif_queue_stopped(can->can.dev))
 			netif_wake_queue(can->can.dev);
 
 		if (!one_shot_fail) {
-- 
2.40.0


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

* [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (11 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 12/12] can: kvaser_pciefd: Use TX FIFO size read from CAN controller Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:51   ` Jimmy Assarsson
  2023-06-22 15:16   ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 1/3] can: kvaser_pciefd: Move hardware specific constants and functions into a driver_data struct Jimmy Assarsson
                   ` (3 subsequent siblings)
  16 siblings, 2 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

This patch series adds support for a range of new Kvaser PCI Express
devices based on the SmartFusion2 SoC, to the kvaser_pciefd driver.

In the first patch, the hardware specific constants and functions are
moved into a driver_data struct.

In the second patch, we add the new devices and their hardware specific
constants and functions.

In the last patch, most of the register reading and writing + shifting
and masking, are wrapped in macros, to simplify the functions.


Note: This series depends on the changes in xxx

Jimmy Assarsson (3):
  can: kvaser_pciefd: Move hardware specific constants and functions
    into a driver_data struct
  can: kvaser_pciefd: Add support for new Kvaser pciefd devices
  can: kvaser_pciefd: Wrap register read and writes with macros

 drivers/net/can/Kconfig         |   5 +
 drivers/net/can/kvaser_pciefd.c | 678 ++++++++++++++++++++++----------
 2 files changed, 479 insertions(+), 204 deletions(-)

-- 
2.40.0


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

* [PATCH 1/3] can: kvaser_pciefd: Move hardware specific constants and functions into a driver_data struct
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (12 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 2/3] can: kvaser_pciefd: Add support for new Kvaser pciefd devices Jimmy Assarsson
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Move hardware specific address offsets, interrupt masks and DMA mapping
function, into struct kvaser_pciefd_driver_data, as a step towards adding
new devices based on different hardware.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 205 ++++++++++++++++++++++----------
 1 file changed, 140 insertions(+), 65 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 5f67414e2875..992af04c627c 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -33,20 +33,14 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_64BIT_DMA_BIT BIT(0)
 
 #define KVASER_PCIEFD_VENDOR 0x1a07
+/* Altera based devices */
 #define KVASER_PCIEFD_4HS_DEVICE_ID 0x000d
 #define KVASER_PCIEFD_2HS_V2_DEVICE_ID 0x000e
 #define KVASER_PCIEFD_HS_V2_DEVICE_ID 0x000f
 #define KVASER_PCIEFD_MINIPCIE_HS_V2_DEVICE_ID 0x0010
 #define KVASER_PCIEFD_MINIPCIE_2HS_V2_DEVICE_ID 0x0011
 
-/* PCIe IRQ registers */
-#define KVASER_PCIEFD_IRQ_REG 0x40
-#define KVASER_PCIEFD_IEN_REG 0x50
-/* DMA map */
-#define KVASER_PCIEFD_DMA_MAP_BASE 0x1000
 /* Kvaser KCAN CAN controller registers */
-#define KVASER_PCIEFD_KCAN0_BASE 0x10000
-#define KVASER_PCIEFD_KCAN_BASE_OFFSET 0x1000
 #define KVASER_PCIEFD_KCAN_FIFO_REG 0x100
 #define KVASER_PCIEFD_KCAN_FIFO_LAST_REG 0x180
 #define KVASER_PCIEFD_KCAN_CTRL_REG 0x2c0
@@ -60,26 +54,20 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_KCAN_BUS_LOAD_REG 0x424
 #define KVASER_PCIEFD_KCAN_BTRD_REG 0x428
 #define KVASER_PCIEFD_KCAN_PWM_REG 0x430
-/* Loopback control register */
-#define KVASER_PCIEFD_LOOP_REG 0x1f000
 /* System identification and information registers */
-#define KVASER_PCIEFD_SYSID_BASE 0x1f020
-#define KVASER_PCIEFD_SYSID_VERSION_REG (KVASER_PCIEFD_SYSID_BASE + 0x8)
-#define KVASER_PCIEFD_SYSID_CANFREQ_REG (KVASER_PCIEFD_SYSID_BASE + 0xc)
-#define KVASER_PCIEFD_SYSID_BUSFREQ_REG (KVASER_PCIEFD_SYSID_BASE + 0x10)
-#define KVASER_PCIEFD_SYSID_BUILD_REG (KVASER_PCIEFD_SYSID_BASE + 0x14)
+#define KVASER_PCIEFD_SYSID_VERSION_REG 0x8
+#define KVASER_PCIEFD_SYSID_CANFREQ_REG 0xc
+#define KVASER_PCIEFD_SYSID_BUSFREQ_REG 0x10
+#define KVASER_PCIEFD_SYSID_BUILD_REG 0x14
+/* Shared receive buffer FIFO registers */
+#define KVASER_PCIEFD_SRB_FIFO_LAST_REG 0x1f4
 /* Shared receive buffer registers */
-#define KVASER_PCIEFD_SRB_BASE 0x1f200
-#define KVASER_PCIEFD_SRB_FIFO_LAST_REG (KVASER_PCIEFD_SRB_BASE + 0x1f4)
-#define KVASER_PCIEFD_SRB_CMD_REG (KVASER_PCIEFD_SRB_BASE + 0x200)
-#define KVASER_PCIEFD_SRB_IEN_REG (KVASER_PCIEFD_SRB_BASE + 0x204)
-#define KVASER_PCIEFD_SRB_IRQ_REG (KVASER_PCIEFD_SRB_BASE + 0x20c)
-#define KVASER_PCIEFD_SRB_STAT_REG (KVASER_PCIEFD_SRB_BASE + 0x210)
-#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG (KVASER_PCIEFD_SRB_BASE + 0x214)
-#define KVASER_PCIEFD_SRB_CTRL_REG (KVASER_PCIEFD_SRB_BASE + 0x218)
-
-#define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
-#define KVASER_PCIEFD_IRQ_SRB BIT(4)
+#define KVASER_PCIEFD_SRB_CMD_REG 0x0
+#define KVASER_PCIEFD_SRB_IEN_REG 0x04
+#define KVASER_PCIEFD_SRB_IRQ_REG 0x0c
+#define KVASER_PCIEFD_SRB_STAT_REG 0x10
+#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG 0x14
+#define KVASER_PCIEFD_SRB_CTRL_REG 0x18
 
 #define KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT 24
 #define KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT 16
@@ -238,7 +226,69 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* Kvaser KCAN_EPACK second word */
 #define KVASER_PCIEFD_EPACK_DIR_TX BIT(0)
 
+#define KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, block) \
+	((pcie)->reg_base + (pcie)->driver_data->address_offset->block)
+
 struct kvaser_pciefd;
+static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie,
+					       dma_addr_t addr, int index);
+
+struct kvaser_pciefd_address_offset {
+	u32 serdes;
+	u32 pci_ien;
+	u32 pci_irq;
+	u32 sysid;
+	u32 loopback;
+	u32 kcan_srb_fifo;
+	u32 kcan_srb;
+	u32 kcan_ch0;
+	u32 kcan_ch1;
+};
+
+struct kvaser_pciefd_dev_ops {
+	void (*kvaser_pciefd_write_dma_map)(struct kvaser_pciefd *pcie,
+					    dma_addr_t addr, int index);
+};
+
+struct kvaser_pciefd_irq_mask {
+	u32 kcan_rx0;
+	u32 kcan_tx[KVASER_PCIEFD_MAX_CAN_CHANNELS];
+	u32 all;
+};
+
+struct kvaser_pciefd_driver_data {
+	const struct kvaser_pciefd_address_offset *address_offset;
+	const struct kvaser_pciefd_irq_mask *irq_mask;
+	const struct kvaser_pciefd_dev_ops *ops;
+};
+
+const struct kvaser_pciefd_address_offset kvaser_pciefd_altera_address_offset = {
+	.serdes = 0x1000,
+	.pci_ien = 0x50,
+	.pci_irq = 0x40,
+	.sysid = 0x1f020,
+	.loopback = 0x1f000,
+	.kcan_srb_fifo = 0x1f200,
+	.kcan_srb = 0x1f400,
+	.kcan_ch0 = 0x10000,
+	.kcan_ch1 = 0x11000,
+};
+
+const struct kvaser_pciefd_irq_mask kvaser_pciefd_altera_irq_mask = {
+	.kcan_rx0 = BIT(4),
+	.kcan_tx = { BIT(0), BIT(1), BIT(2), BIT(3) },
+	.all = 0x0000001f,
+};
+
+const struct kvaser_pciefd_dev_ops kvaser_pciefd_altera_dev_ops = {
+	.kvaser_pciefd_write_dma_map = kvaser_pciefd_write_dma_map_altera,
+};
+
+const struct kvaser_pciefd_driver_data kvaser_pciefd_altera_driver_data = {
+	.address_offset = &kvaser_pciefd_altera_address_offset,
+	.irq_mask = &kvaser_pciefd_altera_irq_mask,
+	.ops = &kvaser_pciefd_altera_dev_ops,
+};
 
 struct kvaser_pciefd_can {
 	struct can_priv can;
@@ -258,6 +308,7 @@ struct kvaser_pciefd {
 	struct pci_dev *pci;
 	void __iomem *reg_base;
 	struct kvaser_pciefd_can *can[KVASER_PCIEFD_MAX_CAN_CHANNELS];
+	const struct kvaser_pciefd_driver_data *driver_data;
 	void *dma_data[KVASER_PCIEFD_DMA_COUNT];
 	u8 nr_channels;
 	u32 bus_freq;
@@ -290,18 +341,23 @@ static const struct can_bittiming_const kvaser_pciefd_bittiming_const = {
 static struct pci_device_id kvaser_pciefd_id_table[] = {
 	{
 		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_4HS_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_altera_driver_data,
 	},
 	{
 		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_2HS_V2_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_altera_driver_data,
 	},
 	{
 		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_HS_V2_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_altera_driver_data,
 	},
 	{
 		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_HS_V2_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_altera_driver_data,
 	},
 	{
 		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_2HS_V2_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_altera_driver_data,
 	},
 	{
 		0,
@@ -755,7 +811,11 @@ static const struct ethtool_ops kvaser_pciefd_ethtool_ops = {
 static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 {
 	int i;
+	void __iomem *kcan_ch0_base;
+	u64 kcan_controller_span;
 
+	kcan_ch0_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_ch0);
+	kcan_controller_span = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_ch1) - kcan_ch0_base;
 	for (i = 0; i < pcie->nr_channels; i++) {
 		struct net_device *netdev;
 		struct kvaser_pciefd_can *can;
@@ -769,8 +829,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		can = netdev_priv(netdev);
 		netdev->netdev_ops = &kvaser_pciefd_netdev_ops;
 		netdev->ethtool_ops = &kvaser_pciefd_ethtool_ops;
-		can->reg_base = pcie->reg_base + KVASER_PCIEFD_KCAN0_BASE +
-				i * KVASER_PCIEFD_KCAN_BASE_OFFSET;
+		can->reg_base = kcan_ch0_base + i * kcan_controller_span;
 		can->kv_pcie = pcie;
 		can->cmd_seq = 0;
 		can->err_rep_cnt = 0;
@@ -851,9 +910,10 @@ static int kvaser_pciefd_reg_candev(struct kvaser_pciefd *pcie)
 	return 0;
 }
 
-static void kvaser_pciefd_write_dma_map(struct kvaser_pciefd *pcie,
-					dma_addr_t addr, int offset)
+static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie,
+					       dma_addr_t addr, int index)
 {
+	void __iomem *serdes_base;
 	u32 word1, word2;
 
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
@@ -863,22 +923,24 @@ static void kvaser_pciefd_write_dma_map(struct kvaser_pciefd *pcie,
 	word1 = addr;
 	word2 = 0;
 #endif
-	iowrite32(word1, pcie->reg_base + offset);
-	iowrite32(word2, pcie->reg_base + offset + 4);
+	serdes_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, serdes) + 0x8 * index;
+	iowrite32(word1, serdes_base);
+	iowrite32(word2, serdes_base + 0x4);
 }
 
 static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 {
+	void __iomem *kcan_srb_base;
+	const struct kvaser_pciefd_dev_ops *dev_ops = pcie->driver_data->ops;
 	int i;
 	u32 srb_status;
 	u32 srb_packet_count;
 	dma_addr_t dma_addr[KVASER_PCIEFD_DMA_COUNT];
 
+	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
 	/* Disable the DMA */
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_SRB_CTRL_REG);
+	iowrite32(0, kcan_srb_base + KVASER_PCIEFD_SRB_CTRL_REG);
 	for (i = 0; i < KVASER_PCIEFD_DMA_COUNT; i++) {
-		unsigned int offset = KVASER_PCIEFD_DMA_MAP_BASE + 8 * i;
-
 		pcie->dma_data[i] = dmam_alloc_coherent(&pcie->pci->dev,
 							KVASER_PCIEFD_DMA_SIZE,
 							&dma_addr[i],
@@ -889,23 +951,24 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 				KVASER_PCIEFD_DMA_SIZE);
 			return -ENOMEM;
 		}
-		kvaser_pciefd_write_dma_map(pcie, dma_addr[i], offset);
+		dev_ops->kvaser_pciefd_write_dma_map(pcie, dma_addr[i], i);
 	}
 
 	/* Reset Rx FIFO, and both DMA buffers */
 	iowrite32(KVASER_PCIEFD_SRB_CMD_FOR | KVASER_PCIEFD_SRB_CMD_RDB0 |
 		  KVASER_PCIEFD_SRB_CMD_RDB1,
-		  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
+		  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
 	/* Empty Rx FIFO */
-	srb_packet_count = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG) &
+	srb_packet_count = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG) &
 			   KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK;
 	while (srb_packet_count) {
 		/* Drop current packet in FIFO */
-		ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_FIFO_LAST_REG);
+		ioread32(KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb_fifo) +
+			 KVASER_PCIEFD_SRB_FIFO_LAST_REG);
 		srb_packet_count--;
 	}
 
-	srb_status = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_STAT_REG);
+	srb_status = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_STAT_REG);
 	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DI)) {
 		dev_err(&pcie->pci->dev, "DMA not idle before enabling\n");
 		return -EIO;
@@ -913,39 +976,43 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 
 	/* Enable the DMA */
 	iowrite32(KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE,
-		  pcie->reg_base + KVASER_PCIEFD_SRB_CTRL_REG);
+		  kcan_srb_base + KVASER_PCIEFD_SRB_CTRL_REG);
 
 	return 0;
 }
 
 static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 {
+	void __iomem *kcan_srb_base;
+	void __iomem *sysid_base;
 	u32 version, srb_status, build;
 
-	version = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_VERSION_REG);
+	sysid_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, sysid);
+	version = ioread32(sysid_base + KVASER_PCIEFD_SYSID_VERSION_REG);
 	pcie->nr_channels = min(KVASER_PCIEFD_MAX_CAN_CHANNELS,
 				((version >> KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT) & 0xff));
 
-	build = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_BUILD_REG);
+	build = ioread32(sysid_base + KVASER_PCIEFD_SYSID_BUILD_REG);
 	dev_dbg(&pcie->pci->dev, "Version %u.%u.%u\n",
 		(version >> KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT) & 0xff,
 		version & 0xff,
 		(build >> KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT) & 0x7fff);
 
-	srb_status = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_STAT_REG);
+	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
+	srb_status = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_STAT_REG);
 	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DMA)) {
 		dev_err(&pcie->pci->dev, "Hardware without DMA is not supported\n");
 		return -ENODEV;
 	}
 
-	pcie->bus_freq = ioread32(pcie->reg_base +
+	pcie->bus_freq = ioread32(sysid_base +
 				  KVASER_PCIEFD_SYSID_BUSFREQ_REG);
-	pcie->freq = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_CANFREQ_REG);
+	pcie->freq = ioread32(sysid_base + KVASER_PCIEFD_SYSID_CANFREQ_REG);
 	pcie->freq_to_ticks_div = pcie->freq / 1000000;
 	if (pcie->freq_to_ticks_div == 0)
 		pcie->freq_to_ticks_div = 1;
 	/* Turn off all loopback functionality */
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_LOOP_REG);
+	iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, loopback));
 
 	return 0;
 }
@@ -1415,21 +1482,23 @@ static int kvaser_pciefd_read_buffer(struct kvaser_pciefd *pcie, int dma_buf)
 
 static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
 {
+	void __iomem *kcan_srb_base;
 	u32 irq;
 
-	irq = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_IRQ_REG);
+	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
+	irq = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_IRQ_REG);
 	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD0) {
 		kvaser_pciefd_read_buffer(pcie, 0);
 		/* Reset DMA buffer 0 */
 		iowrite32(KVASER_PCIEFD_SRB_CMD_RDB0,
-			  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
+			  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
 	}
 
 	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD1) {
 		kvaser_pciefd_read_buffer(pcie, 1);
 		/* Reset DMA buffer 1 */
 		iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1,
-			  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
+			  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
 	}
 
 	if (irq & KVASER_PCIEFD_SRB_IRQ_DOF0 ||
@@ -1438,7 +1507,7 @@ static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
 	    irq & KVASER_PCIEFD_SRB_IRQ_DUF1)
 		dev_err(&pcie->pci->dev, "DMA IRQ error 0x%08X\n", irq);
 
-	iowrite32(irq, pcie->reg_base + KVASER_PCIEFD_SRB_IRQ_REG);
+	iowrite32(irq, kcan_srb_base + KVASER_PCIEFD_SRB_IRQ_REG);
 }
 
 static void kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
@@ -1464,15 +1533,16 @@ static void kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
 static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
 {
 	struct kvaser_pciefd *pcie = (struct kvaser_pciefd *)dev;
+	const struct kvaser_pciefd_irq_mask *irq_mask = pcie->driver_data->irq_mask;
 	u32 board_irq;
 	int i;
 
-	board_irq = ioread32(pcie->reg_base + KVASER_PCIEFD_IRQ_REG);
+	board_irq = ioread32(KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, pci_irq));
 
-	if (!(board_irq & KVASER_PCIEFD_IRQ_ALL_MSK))
+	if (!(board_irq & irq_mask->all))
 		return IRQ_NONE;
 
-	if (board_irq & KVASER_PCIEFD_IRQ_SRB)
+	if (board_irq & irq_mask->kcan_rx0)
 		kvaser_pciefd_receive_irq(pcie);
 
 	for (i = 0; i < pcie->nr_channels; i++) {
@@ -1483,7 +1553,7 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
 		}
 
 		/* Check that mask matches channel (i) IRQ mask */
-		if (board_irq & (1 << i))
+		if (board_irq & irq_mask->kcan_tx[i])
 			kvaser_pciefd_transmit_irq(pcie->can[i]);
 	}
 
@@ -1510,6 +1580,9 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 {
 	int err;
 	struct kvaser_pciefd *pcie;
+	const struct kvaser_pciefd_irq_mask *irq_mask;
+	void __iomem *kcan_srb_base;
+	void __iomem *irq_en_base;
 
 	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
@@ -1517,6 +1590,8 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 
 	pci_set_drvdata(pdev, pcie);
 	pcie->pci = pdev;
+	pcie->driver_data = (const struct kvaser_pciefd_driver_data *)id->driver_data;
+	irq_mask = pcie->driver_data->irq_mask;
 
 	err = pci_enable_device(pdev);
 	if (err)
@@ -1542,6 +1617,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
+	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
 	err = kvaser_pciefd_setup_can_ctrls(pcie);
 	if (err)
 		goto err_teardown_can_ctrls;
@@ -1552,22 +1628,21 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 		goto err_teardown_can_ctrls;
 
 	iowrite32(KVASER_PCIEFD_SRB_IRQ_DPD0 | KVASER_PCIEFD_SRB_IRQ_DPD1,
-		  pcie->reg_base + KVASER_PCIEFD_SRB_IRQ_REG);
+		  kcan_srb_base + KVASER_PCIEFD_SRB_IRQ_REG);
 
 	iowrite32(KVASER_PCIEFD_SRB_IRQ_DPD0 | KVASER_PCIEFD_SRB_IRQ_DPD1 |
 		  KVASER_PCIEFD_SRB_IRQ_DOF0 | KVASER_PCIEFD_SRB_IRQ_DOF1 |
 		  KVASER_PCIEFD_SRB_IRQ_DUF0 | KVASER_PCIEFD_SRB_IRQ_DUF1,
-		  pcie->reg_base + KVASER_PCIEFD_SRB_IEN_REG);
+		  kcan_srb_base + KVASER_PCIEFD_SRB_IEN_REG);
 
 	/* Enable PCI interrupts */
-	iowrite32(KVASER_PCIEFD_IRQ_ALL_MSK,
-		  pcie->reg_base + KVASER_PCIEFD_IEN_REG);
-
+	irq_en_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, pci_ien);
+	iowrite32(irq_mask->all, irq_en_base);
 	/* Ready the DMA buffers */
 	iowrite32(KVASER_PCIEFD_SRB_CMD_RDB0,
-		  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
+		  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
 	iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1,
-		  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
+		  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
 
 	err = kvaser_pciefd_reg_candev(pcie);
 	if (err)
@@ -1577,12 +1652,12 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 
 err_free_irq:
 	/* Disable PCI interrupts */
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_IEN_REG);
+	iowrite32(0, irq_en_base);
 	free_irq(pcie->pci->irq, pcie);
 
 err_teardown_can_ctrls:
 	kvaser_pciefd_teardown_can_ctrls(pcie);
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_SRB_CTRL_REG);
+	iowrite32(0, kcan_srb_base + KVASER_PCIEFD_SRB_CTRL_REG);
 	pci_clear_master(pdev);
 
 err_pci_iounmap:
@@ -1621,8 +1696,8 @@ static void kvaser_pciefd_remove(struct pci_dev *pdev)
 	kvaser_pciefd_remove_all_ctrls(pcie);
 
 	/* Disable interrupts */
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_SRB_CTRL_REG);
-	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_IEN_REG);
+	iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb) + KVASER_PCIEFD_SRB_CTRL_REG);
+	iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, pci_ien));
 
 	free_irq(pcie->pci->irq, pcie);
 
-- 
2.40.0


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

* [PATCH 2/3] can: kvaser_pciefd: Add support for new Kvaser pciefd devices
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (13 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 1/3] can: kvaser_pciefd: Move hardware specific constants and functions into a driver_data struct Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:43 ` [PATCH 3/3] can: kvaser_pciefd: Wrap register read and writes with macros Jimmy Assarsson
  2023-05-23  9:50 ` [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson

Add support for new Kvaser pciefd devices, based on SmartFusion2 SoC.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/Kconfig         |  5 +++
 drivers/net/can/kvaser_pciefd.c | 74 ++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/Kconfig b/drivers/net/can/Kconfig
index b929a9da7920..502dcfe4d16a 100644
--- a/drivers/net/can/Kconfig
+++ b/drivers/net/can/Kconfig
@@ -160,8 +160,13 @@ config CAN_KVASER_PCIEFD
 	    Kvaser PCIEcan 4xHS
 	    Kvaser PCIEcan 2xHS v2
 	    Kvaser PCIEcan HS v2
+	    Kvaser PCIEcan 1xCAN v3
+	    Kvaser PCIEcan 2xCAN v3
+	    Kvaser PCIEcan 4xCAN v2
 	    Kvaser Mini PCI Express HS v2
 	    Kvaser Mini PCI Express 2xHS v2
+	    Kvaser Mini PCI Express 1xCAN v3
+	    Kvaser Mini PCI Express 2xCAN v3
 
 config CAN_SLCAN
 	tristate "Serial / USB serial CAN Adaptors (slcan)"
diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 992af04c627c..ffe13a1a882a 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
 /* Copyright (C) 2018 KVASER AB, Sweden. All rights reserved.
  * Parts of this driver are based on the following:
- *  - Kvaser linux pciefd driver (version 5.25)
+ *  - Kvaser linux pciefd driver (version 5.41)
  *  - PEAK linux canfd driver
  */
 
@@ -40,6 +40,13 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_MINIPCIE_HS_V2_DEVICE_ID 0x0010
 #define KVASER_PCIEFD_MINIPCIE_2HS_V2_DEVICE_ID 0x0011
 
+/* SmartFusion2 based devices */
+#define KVASER_PCIEFD_2CAN_V3_DEVICE_ID 0x0012
+#define KVASER_PCIEFD_1CAN_V3_DEVICE_ID 0x0013
+#define KVASER_PCIEFD_4CAN_V2_DEVICE_ID 0x0014
+#define KVASER_PCIEFD_MINIPCIE_2CAN_V3_DEVICE_ID 0x0015
+#define KVASER_PCIEFD_MINIPCIE_1CAN_V3_DEVICE_ID 0x0016
+
 /* Kvaser KCAN CAN controller registers */
 #define KVASER_PCIEFD_KCAN_FIFO_REG 0x100
 #define KVASER_PCIEFD_KCAN_FIFO_LAST_REG 0x180
@@ -232,6 +239,8 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 struct kvaser_pciefd;
 static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie,
 					       dma_addr_t addr, int index);
+static void kvaser_pciefd_write_dma_map_sf2(struct kvaser_pciefd *pcie,
+					    dma_addr_t addr, int index);
 
 struct kvaser_pciefd_address_offset {
 	u32 serdes;
@@ -274,22 +283,50 @@ const struct kvaser_pciefd_address_offset kvaser_pciefd_altera_address_offset =
 	.kcan_ch1 = 0x11000,
 };
 
+const struct kvaser_pciefd_address_offset kvaser_pciefd_sf2_address_offset = {
+	.serdes = 0x280c8,
+	.pci_ien = 0x102004,
+	.pci_irq = 0x102008,
+	.sysid = 0x100000,
+	.loopback = 0x103000,
+	.kcan_srb_fifo = 0x120000,
+	.kcan_srb = 0x121000,
+	.kcan_ch0 = 0x140000,
+	.kcan_ch1 = 0x142000,
+};
+
 const struct kvaser_pciefd_irq_mask kvaser_pciefd_altera_irq_mask = {
 	.kcan_rx0 = BIT(4),
 	.kcan_tx = { BIT(0), BIT(1), BIT(2), BIT(3) },
 	.all = 0x0000001f,
 };
 
+const struct kvaser_pciefd_irq_mask kvaser_pciefd_sf2_irq_mask = {
+	.kcan_rx0 = BIT(4),
+	.kcan_tx = { BIT(16), BIT(17), BIT(18), BIT(19) },
+	.all = 0x000f0010,
+};
+
 const struct kvaser_pciefd_dev_ops kvaser_pciefd_altera_dev_ops = {
 	.kvaser_pciefd_write_dma_map = kvaser_pciefd_write_dma_map_altera,
 };
 
+const struct kvaser_pciefd_dev_ops kvaser_pciefd_sf2_dev_ops = {
+	.kvaser_pciefd_write_dma_map = kvaser_pciefd_write_dma_map_sf2,
+};
+
 const struct kvaser_pciefd_driver_data kvaser_pciefd_altera_driver_data = {
 	.address_offset = &kvaser_pciefd_altera_address_offset,
 	.irq_mask = &kvaser_pciefd_altera_irq_mask,
 	.ops = &kvaser_pciefd_altera_dev_ops,
 };
 
+const struct kvaser_pciefd_driver_data kvaser_pciefd_sf2_driver_data = {
+	.address_offset = &kvaser_pciefd_sf2_address_offset,
+	.irq_mask = &kvaser_pciefd_sf2_irq_mask,
+	.ops = &kvaser_pciefd_sf2_dev_ops,
+};
+
 struct kvaser_pciefd_can {
 	struct can_priv can;
 	struct kvaser_pciefd *kv_pcie;
@@ -359,6 +396,26 @@ static struct pci_device_id kvaser_pciefd_id_table[] = {
 		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_2HS_V2_DEVICE_ID),
 		.driver_data = (kernel_ulong_t)&kvaser_pciefd_altera_driver_data,
 	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_2CAN_V3_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_sf2_driver_data,
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_1CAN_V3_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_sf2_driver_data,
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_4CAN_V2_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_sf2_driver_data,
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_2CAN_V3_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_sf2_driver_data,
+	},
+	{
+		PCI_DEVICE(KVASER_PCIEFD_VENDOR, KVASER_PCIEFD_MINIPCIE_1CAN_V3_DEVICE_ID),
+		.driver_data = (kernel_ulong_t)&kvaser_pciefd_sf2_driver_data,
+	},
 	{
 		0,
 	},
@@ -928,6 +985,21 @@ static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie,
 	iowrite32(word2, serdes_base + 0x4);
 }
 
+static void kvaser_pciefd_write_dma_map_sf2(struct kvaser_pciefd *pcie,
+					    dma_addr_t addr, int index)
+{
+	void __iomem *serdes_base;
+	u32 lsb = addr & 0xfffff000;
+	u32 msb = 0x0;
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	msb = addr >> 32;
+#endif
+	serdes_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, serdes) + 0x10 * index;
+	iowrite32(lsb, serdes_base);
+	iowrite32(msb, serdes_base + 0x4);
+}
+
 static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 {
 	void __iomem *kcan_srb_base;
-- 
2.40.0


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

* [PATCH 3/3] can: kvaser_pciefd: Wrap register read and writes with macros
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (14 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 2/3] can: kvaser_pciefd: Add support for new Kvaser pciefd devices Jimmy Assarsson
@ 2023-05-23  9:43 ` Jimmy Assarsson
  2023-05-23  9:50 ` [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:43 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson, Jimmy Assarsson, Martin Jocic

Wrap register read and writes with macros, to simplify the code.

Co-developed-by: Martin Jocic <majoc@kvaser.com>
Signed-off-by: Martin Jocic <majoc@kvaser.com>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
 drivers/net/can/kvaser_pciefd.c | 495 ++++++++++++++++++++------------
 1 file changed, 309 insertions(+), 186 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index ffe13a1a882a..c5a410141dfb 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -233,8 +233,196 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* Kvaser KCAN_EPACK second word */
 #define KVASER_PCIEFD_EPACK_DIR_TX BIT(0)
 
+/* Macros for calculating addresses of registers */
 #define KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, block) \
 	((pcie)->reg_base + (pcie)->driver_data->address_offset->block)
+#define KVASER_PCIEFD_PCI_IEN_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), pci_ien))
+#define KVASER_PCIEFD_PCI_IRQ_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), pci_irq))
+#define KVASER_PCIEFD_SERDES_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), serdes))
+#define KVASER_PCIEFD_SYSID_VERSION_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), sysid) + KVASER_PCIEFD_SYSID_VERSION_REG)
+#define KVASER_PCIEFD_SYSID_CANFREQ_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), sysid) + KVASER_PCIEFD_SYSID_CANFREQ_REG)
+#define KVASER_PCIEFD_SYSID_BUSFREQ_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), sysid) + KVASER_PCIEFD_SYSID_BUSFREQ_REG)
+#define KVASER_PCIEFD_SYSID_BUILD_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), sysid) + KVASER_PCIEFD_SYSID_BUILD_REG)
+#define KVASER_PCIEFD_SRB_FIFO_LAST_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb_fifo) + KVASER_PCIEFD_SRB_FIFO_LAST_REG)
+#define KVASER_PCIEFD_SRB_CMD_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb) + KVASER_PCIEFD_SRB_CMD_REG)
+#define KVASER_PCIEFD_SRB_IEN_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb) + KVASER_PCIEFD_SRB_IEN_REG)
+#define KVASER_PCIEFD_SRB_IRQ_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb) + KVASER_PCIEFD_SRB_IRQ_REG)
+#define KVASER_PCIEFD_SRB_STAT_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb) + KVASER_PCIEFD_SRB_STAT_REG)
+#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb) + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG)
+#define KVASER_PCIEFD_SRB_CTRL_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_srb) + KVASER_PCIEFD_SRB_CTRL_REG)
+#define KVASER_PCIEFD_KCAN_CH0_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_ch0))
+#define KVASER_PCIEFD_KCAN_CH1_ADDR(pcie) \
+	(KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), kcan_ch1))
+
+/* Macros for calculating addresses of Kvaser KCAN registers */
+#define KVASER_PCIEFD_KCAN_FIFO_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_FIFO_REG)
+#define KVASER_PCIEFD_KCAN_FIFO_LAST_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_FIFO_LAST_REG)
+#define KVASER_PCIEFD_KCAN_CTRL_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_CTRL_REG)
+#define KVASER_PCIEFD_KCAN_CMD_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_CMD_REG)
+#define KVASER_PCIEFD_KCAN_IEN_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_IEN_REG)
+#define KVASER_PCIEFD_KCAN_IRQ_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG)
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG)
+#define KVASER_PCIEFD_KCAN_STAT_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_STAT_REG)
+#define KVASER_PCIEFD_KCAN_MODE_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_MODE_REG)
+#define KVASER_PCIEFD_KCAN_BTRN_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_BTRN_REG)
+#define KVASER_PCIEFD_KCAN_BUS_LOAD_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_BUS_LOAD_REG)
+#define KVASER_PCIEFD_KCAN_BTRD_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_BTRD_REG)
+#define KVASER_PCIEFD_KCAN_PWM_ADDR(can) \
+	((can)->reg_base + KVASER_PCIEFD_KCAN_PWM_REG)
+
+/* Macros for reading and writing registers */
+#define KVASER_PCIEFD_PCI_IEN_SET(pcie, value) \
+	(iowrite32((value), KVASER_PCIEFD_PCI_IEN_ADDR((pcie))))
+#define KVASER_PCIEFD_PCI_IRQ_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_PCI_IRQ_ADDR((pcie))))
+#define KVASER_PCIEFD_SYSID_VERSION_NR_CHANNELS_GET(pcie)      \
+	((ioread32(KVASER_PCIEFD_SYSID_VERSION_ADDR((pcie))) >> \
+	  KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT) &          \
+	 0xff)
+#define KVASER_PCIEFD_SYSID_VERSION_MINOR_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_SYSID_VERSION_ADDR((pcie))) & 0xff)
+#define KVASER_PCIEFD_SYSID_VERSION_MAJOR_GET(pcie)             \
+	((ioread32(KVASER_PCIEFD_SYSID_VERSION_ADDR((pcie))) >> \
+	  KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT) &            \
+	 0xff)
+#define KVASER_PCIEFD_SYSID_CANFREQ_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_SYSID_CANFREQ_ADDR((pcie))))
+#define KVASER_PCIEFD_SYSID_BUSFREQ_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_SYSID_BUSFREQ_ADDR((pcie))))
+#define KVASER_PCIEFD_SYSID_BUILD_GET(pcie)                   \
+	((ioread32(KVASER_PCIEFD_SYSID_BUILD_ADDR((pcie))) >> \
+	  KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT) &          \
+	 0x7fff)
+#define KVASER_PCIEFD_SRB_FIFO_LAST_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_SRB_FIFO_LAST_ADDR((pcie))))
+#define KVASER_PCIEFD_SRB_CMD_SET(pcie, value) \
+	(iowrite32((value), KVASER_PCIEFD_SRB_CMD_ADDR((pcie))))
+#define KVASER_PCIEFD_SRB_IEN_SET(pcie, value) \
+	(iowrite32((value), KVASER_PCIEFD_SRB_IEN_ADDR((pcie))))
+#define KVASER_PCIEFD_SRB_IRQ_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_SRB_IRQ_ADDR((pcie))))
+#define KVASER_PCIEFD_SRB_IRQ_SET(pcie, value) \
+	(iowrite32((value), KVASER_PCIEFD_SRB_IRQ_ADDR((pcie))))
+#define KVASER_PCIEFD_SRB_STAT_GET(pcie) \
+	(ioread32(KVASER_PCIEFD_SRB_STAT_ADDR((pcie))))
+#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_CURRENT_GET(pcie)         \
+	(ioread32(KVASER_PCIEFD_SRB_RX_NR_PACKETS_ADDR((pcie))) & \
+	 KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK)
+#define KVASER_PCIEFD_SRB_CTRL_SET(pcie, value) \
+	(iowrite32((value), KVASER_PCIEFD_SRB_CTRL_ADDR((pcie))))
+#define KVASER_PCIEFD_KCAN_FIFO_WRITE(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_FIFO_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_CTRL_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_CTRL_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_CMD_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_CMD_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_IEN_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_IEN_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_IRQ_GET(can) \
+	(ioread32(KVASER_PCIEFD_KCAN_IRQ_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_IRQ_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_IRQ_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_GET(can) \
+	(ioread32(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_ADDR((can))) & 0xff)
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_GET(can)               \
+	((ioread32(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_ADDR((can))) >> \
+	  KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT) &             \
+	 0xff)
+#define KVASER_PCIEFD_KCAN_STAT_GET(can) \
+	(ioread32(KVASER_PCIEFD_KCAN_STAT_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_MODE_GET(can) \
+	(ioread32(KVASER_PCIEFD_KCAN_MODE_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_MODE_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_MODE_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_BTRN_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_BTRN_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_BTRD_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_BTRD_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_PWM_GET(can) \
+	(ioread32(KVASER_PCIEFD_KCAN_PWM_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_PWM_SET(can, value) \
+	(iowrite32((value), KVASER_PCIEFD_KCAN_PWM_ADDR((can))))
+
+#define KVASER_PCIEFD_PCI_IEN_DISABLE_ALL(pcie) \
+	(KVASER_PCIEFD_PCI_IEN_SET((pcie), 0))
+#define KVASER_PCIEFD_PCI_IEN_ENABLE_ALL(pcie) \
+	(KVASER_PCIEFD_PCI_IEN_SET((pcie), (pcie)->driver_data->irq_mask->all))
+
+#define KVASER_PCIEFD_SRB_DMA_DISABLE(pcie) (KVASER_PCIEFD_SRB_CTRL_SET((pcie), 0))
+#define KVASER_PCIEFD_SRB_DMA_ENABLE(pcie) \
+	(KVASER_PCIEFD_SRB_CTRL_SET((pcie), KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE))
+
+#define KVASER_PCIEFD_SRB_IEN_ENABLE_ALL(pcie)                          \
+	(KVASER_PCIEFD_SRB_IEN_SET((pcie),                              \
+				   KVASER_PCIEFD_SRB_IRQ_DPD0 |         \
+					   KVASER_PCIEFD_SRB_IRQ_DPD1 | \
+					   KVASER_PCIEFD_SRB_IRQ_DOF0 | \
+					   KVASER_PCIEFD_SRB_IRQ_DOF1 | \
+					   KVASER_PCIEFD_SRB_IRQ_DUF0 | \
+					   KVASER_PCIEFD_SRB_IRQ_DUF1))
+
+#define KVASER_PCIEFD_KCAN_IEN_DISABLE_ALL(can) \
+	(KVASER_PCIEFD_KCAN_IEN_SET((can), 0))
+#define KVASER_PCIEFD_KCAN_IEN_ENABLE_ALL(can)                            \
+	(KVASER_PCIEFD_KCAN_IEN_SET((can),                                \
+				    KVASER_PCIEFD_KCAN_IRQ_TOF |          \
+					    KVASER_PCIEFD_KCAN_IRQ_ABD |  \
+					    KVASER_PCIEFD_KCAN_IRQ_TAE |  \
+					    KVASER_PCIEFD_KCAN_IRQ_TAL |  \
+					    KVASER_PCIEFD_KCAN_IRQ_FDIC | \
+					    KVASER_PCIEFD_KCAN_IRQ_BPP |  \
+					    KVASER_PCIEFD_KCAN_IRQ_TAR))
+#define KVASER_PCIEFD_KCAN_IEN_ENABLE_ABD(can) \
+	(KVASER_PCIEFD_KCAN_IEN_SET((can), KVASER_PCIEFD_KCAN_IRQ_ABD))
+#define KVASER_PCIEFD_KCAN_IRQ_CLEAR_ALL(can) \
+	(KVASER_PCIEFD_KCAN_IRQ_SET((can), GENMASK(31, 0)))
+#define KVASER_PCIEFD_KCAN_BUS_LOAD_DISABLE(can) \
+	(iowrite32(0, KVASER_PCIEFD_KCAN_BUS_LOAD_ADDR((can))))
+#define KVASER_PCIEFD_KCAN_CHANNEL_SPAN(pcie) \
+	(KVASER_PCIEFD_KCAN_CH1_ADDR((pcie)) - KVASER_PCIEFD_KCAN_CH0_ADDR((pcie)))
+#define KVASER_PCIEFD_KCAN_CHX_ADDR(pcie, i) \
+	(KVASER_PCIEFD_KCAN_CH0_ADDR((pcie)) + (i) * KVASER_PCIEFD_KCAN_CHANNEL_SPAN((pcie)))
+
+#define KVASER_PCIEFD_LOOPBACK_DISABLE(pcie) \
+	(iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR((pcie), loopback)))
+
+#define KVASER_PCIEFD_WRITE_DMA_MAP(pcie, addr, index) \
+	((pcie)->driver_data->ops->kvaser_pciefd_write_dma_map((pcie), (addr), (index)))
+
+#define KVASER_PCIEFD_PACKET_TYPE(packet) \
+	(((packet)->header[1] >> KVASER_PCIEFD_PACKET_TYPE_SHIFT) & KVASER_PCIEFD_PACKET_TYPE_MASK)
+
+#define KVASER_PCIEFD_SPACKET_TXERR_COUNT(packet) ((packet)->header[0] & 0xff)
+
+#define KVASER_PCIEFD_SPACKET_RXERR_COUNT(packet) \
+	(((packet)->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff)
 
 struct kvaser_pciefd;
 static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie,
@@ -427,13 +615,20 @@ static inline u8 kvaser_pciefd_rx_packet_get_ch_id(struct kvaser_pciefd_rx_packe
 	return (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & KVASER_PCIEFD_PACKET_CHID_MASK;
 }
 
-static void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
+static inline void kvaser_pciefd_send_kcan_cmd(struct kvaser_pciefd_can *can, u32 cmd)
 {
-	u32 cmd;
+	KVASER_PCIEFD_KCAN_CMD_SET(can,
+				   cmd | (++can->cmd_seq << KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT));
+}
 
-	cmd = KVASER_PCIEFD_KCAN_CMD_SRQ;
-	cmd |= ++can->cmd_seq << KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT;
-	iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
+static inline void kvaser_pciefd_kcan_abort_flush_reset(struct kvaser_pciefd_can *can)
+{
+	kvaser_pciefd_send_kcan_cmd(can, KVASER_PCIEFD_KCAN_CMD_AT);
+}
+
+static inline void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
+{
+	kvaser_pciefd_send_kcan_cmd(can, KVASER_PCIEFD_KCAN_CMD_SRQ);
 }
 
 static void kvaser_pciefd_enable_err_gen(struct kvaser_pciefd_can *can)
@@ -442,10 +637,10 @@ static void kvaser_pciefd_enable_err_gen(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	spin_lock_irqsave(&can->lock, irq);
-	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	mode = KVASER_PCIEFD_KCAN_MODE_GET(can);
 	if (!(mode & KVASER_PCIEFD_KCAN_MODE_EPEN)) {
 		mode |= KVASER_PCIEFD_KCAN_MODE_EPEN;
-		iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+		KVASER_PCIEFD_KCAN_MODE_SET(can, mode);
 	}
 	spin_unlock_irqrestore(&can->lock, irq);
 }
@@ -456,25 +651,12 @@ static void kvaser_pciefd_disable_err_gen(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	spin_lock_irqsave(&can->lock, irq);
-	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	mode = KVASER_PCIEFD_KCAN_MODE_GET(can);
 	mode &= ~KVASER_PCIEFD_KCAN_MODE_EPEN;
-	iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	KVASER_PCIEFD_KCAN_MODE_SET(can, mode);
 	spin_unlock_irqrestore(&can->lock, irq);
 }
 
-static void kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can)
-{
-	u32 msk;
-
-	msk = KVASER_PCIEFD_KCAN_IRQ_TE | KVASER_PCIEFD_KCAN_IRQ_ROF |
-	      KVASER_PCIEFD_KCAN_IRQ_TOF | KVASER_PCIEFD_KCAN_IRQ_ABD |
-	      KVASER_PCIEFD_KCAN_IRQ_TAE | KVASER_PCIEFD_KCAN_IRQ_TAL |
-	      KVASER_PCIEFD_KCAN_IRQ_FDIC | KVASER_PCIEFD_KCAN_IRQ_BPP |
-	      KVASER_PCIEFD_KCAN_IRQ_TAR;
-
-	iowrite32(msk, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-}
-
 static inline void kvaser_pciefd_set_skb_timestamp(const struct kvaser_pciefd *pcie,
 						   struct sk_buff *skb, u64 timestamp)
 {
@@ -488,7 +670,7 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	spin_lock_irqsave(&can->lock, irq);
-	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	mode = KVASER_PCIEFD_KCAN_MODE_GET(can);
 	if (can->can.ctrlmode & CAN_CTRLMODE_FD) {
 		mode &= ~KVASER_PCIEFD_KCAN_MODE_CCM;
 		if (can->can.ctrlmode & CAN_CTRLMODE_FD_NON_ISO)
@@ -509,7 +691,7 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
 	/* Use ACK packet type */
 	mode &= ~KVASER_PCIEFD_KCAN_MODE_APT;
 	mode &= ~KVASER_PCIEFD_KCAN_MODE_RM;
-	iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	KVASER_PCIEFD_KCAN_MODE_SET(can, mode);
 
 	spin_unlock_irqrestore(&can->lock, irq);
 }
@@ -520,24 +702,19 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	spin_lock_irqsave(&can->lock, irq);
-	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
-		  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-	status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);
+	KVASER_PCIEFD_KCAN_IRQ_CLEAR_ALL(can);
+	KVASER_PCIEFD_KCAN_IEN_ENABLE_ABD(can);
+	status = KVASER_PCIEFD_KCAN_STAT_GET(can);
 	if (status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
-		u32 cmd;
-
 		/* If controller is already idle, run abort, flush and reset */
-		cmd = KVASER_PCIEFD_KCAN_CMD_AT;
-		cmd |= ++can->cmd_seq << KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT;
-		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
+		kvaser_pciefd_kcan_abort_flush_reset(can);
 	} else if (!(status & KVASER_PCIEFD_KCAN_STAT_RMR)) {
 		u32 mode;
 
 		/* Put controller in reset mode */
-		mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+		mode = KVASER_PCIEFD_KCAN_MODE_GET(can);
 		mode |= KVASER_PCIEFD_KCAN_MODE_RM;
-		iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+		KVASER_PCIEFD_KCAN_MODE_SET(can, mode);
 	}
 	spin_unlock_irqrestore(&can->lock, irq);
 }
@@ -558,13 +735,12 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 	}
 
 	spin_lock_irqsave(&can->lock, irq);
-	iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
-		  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	KVASER_PCIEFD_KCAN_IEN_DISABLE_ALL(can);
+	KVASER_PCIEFD_KCAN_IRQ_CLEAR_ALL(can);
+	KVASER_PCIEFD_KCAN_IEN_ENABLE_ABD(can);
+	mode = KVASER_PCIEFD_KCAN_MODE_GET(can);
 	mode &= ~KVASER_PCIEFD_KCAN_MODE_RM;
-	iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	KVASER_PCIEFD_KCAN_MODE_SET(can, mode);
 	spin_unlock_irqrestore(&can->lock, irq);
 
 	if (!wait_for_completion_timeout(&can->start_comp,
@@ -573,10 +749,10 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 		return -ETIMEDOUT;
 	}
 	/* Reset interrupt handling */
-	iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
+	KVASER_PCIEFD_KCAN_IEN_DISABLE_ALL(can);
+	KVASER_PCIEFD_KCAN_IRQ_CLEAR_ALL(can);
 
-	kvaser_pciefd_set_tx_irq(can);
+	KVASER_PCIEFD_KCAN_IEN_ENABLE_ALL(can);
 	kvaser_pciefd_setup_controller(can);
 	can->can.state = CAN_STATE_ERROR_ACTIVE;
 	netif_wake_queue(can->can.dev);
@@ -594,11 +770,11 @@ static void kvaser_pciefd_pwm_stop(struct kvaser_pciefd_can *can)
 	unsigned long irq;
 
 	spin_lock_irqsave(&can->lock, irq);
-	pwm_ctrl = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
+	pwm_ctrl = KVASER_PCIEFD_KCAN_PWM_GET(can);
 	top = (pwm_ctrl >> KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT) & 0xff;
 	/* Set duty cycle to zero */
 	pwm_ctrl |= top;
-	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
+	KVASER_PCIEFD_KCAN_PWM_SET(can, pwm_ctrl);
 	spin_unlock_irqrestore(&can->lock, irq);
 }
 
@@ -614,13 +790,13 @@ static void kvaser_pciefd_pwm_start(struct kvaser_pciefd_can *can)
 	top = can->kv_pcie->bus_freq / (2 * 500000) - 1;
 	pwm_ctrl = top & 0xff;
 	pwm_ctrl |= (top & 0xff) << KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT;
-	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
+	KVASER_PCIEFD_KCAN_PWM_SET(can, pwm_ctrl);
 
 	/* Set duty cycle to 95 */
 	trigger = (100 * top - 95 * (top + 1) + 50) / 100;
 	pwm_ctrl = trigger & 0xff;
 	pwm_ctrl |= (top & 0xff) << KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT;
-	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
+	KVASER_PCIEFD_KCAN_PWM_SET(can, pwm_ctrl);
 	spin_unlock_irqrestore(&can->lock, irq);
 }
 
@@ -656,7 +832,7 @@ static int kvaser_pciefd_stop(struct net_device *netdev)
 		netdev_err(can->can.dev, "Timeout during stop\n");
 		ret = -ETIMEDOUT;
 	} else {
-		iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
+		KVASER_PCIEFD_KCAN_IEN_DISABLE_ALL(can);
 		del_timer(&can->bec_poll_timer);
 	}
 	can->can.state = CAN_STATE_STOPPED;
@@ -729,27 +905,22 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 	can->echo_idx = (can->echo_idx + 1) % can->can.echo_skb_max;
 
 	/* Write header to fifo */
-	iowrite32(packet.header[0],
-		  can->reg_base + KVASER_PCIEFD_KCAN_FIFO_REG);
-	iowrite32(packet.header[1],
-		  can->reg_base + KVASER_PCIEFD_KCAN_FIFO_REG);
+	KVASER_PCIEFD_KCAN_FIFO_WRITE(can, packet.header[0]);
+	KVASER_PCIEFD_KCAN_FIFO_WRITE(can, packet.header[1]);
 
 	if (nr_words) {
 		u32 data_last = ((u32 *)packet.data)[nr_words - 1];
 
 		/* Write data to fifo, except last word */
-		iowrite32_rep(can->reg_base +
-			      KVASER_PCIEFD_KCAN_FIFO_REG, packet.data,
+		iowrite32_rep(KVASER_PCIEFD_KCAN_FIFO_ADDR(can), packet.data,
 			      nr_words - 1);
 		/* Write last word to end of fifo */
-		__raw_writel(data_last, can->reg_base +
-			     KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
+		__raw_writel(data_last, KVASER_PCIEFD_KCAN_FIFO_LAST_ADDR(can));
 	} else {
 		/* Complete write to fifo */
-		__raw_writel(0, can->reg_base +
-			     KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
+		__raw_writel(0, KVASER_PCIEFD_KCAN_FIFO_LAST_ADDR(can));
 	}
-	count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
+	count = KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_GET(can);
 	/* No room for a new message, stop the queue until at least one
 	 * successful transmit
 	 */
@@ -782,13 +953,12 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
 	       ((bt->brp - 1) & KVASER_PCIEFD_KCAN_BTRN_BRP_MASK);
 
 	spin_lock_irqsave(&can->lock, irq_flags);
-	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	mode = KVASER_PCIEFD_KCAN_MODE_GET(can);
 	/* Put the circuit in reset mode */
-	iowrite32(mode | KVASER_PCIEFD_KCAN_MODE_RM,
-		  can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	KVASER_PCIEFD_KCAN_MODE_SET(can, mode | KVASER_PCIEFD_KCAN_MODE_RM);
 
 	/* Can only set bittiming if in reset mode */
-	ret = readl_poll_timeout(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG,
+	ret = readl_poll_timeout(KVASER_PCIEFD_KCAN_MODE_ADDR(can),
 				 test, test & KVASER_PCIEFD_KCAN_MODE_RM, 0, 10);
 	if (ret) {
 		spin_unlock_irqrestore(&can->lock, irq_flags);
@@ -796,11 +966,11 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
 	}
 
 	if (data)
-		iowrite32(btrn, can->reg_base + KVASER_PCIEFD_KCAN_BTRD_REG);
+		KVASER_PCIEFD_KCAN_BTRD_SET(can, btrn);
 	else
-		iowrite32(btrn, can->reg_base + KVASER_PCIEFD_KCAN_BTRN_REG);
+		KVASER_PCIEFD_KCAN_BTRN_SET(can, btrn);
 	/* Restore previous reset mode status */
-	iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
+	KVASER_PCIEFD_KCAN_MODE_SET(can, mode);
 	spin_unlock_irqrestore(&can->lock, irq_flags);
 
 	return 0;
@@ -868,15 +1038,11 @@ static const struct ethtool_ops kvaser_pciefd_ethtool_ops = {
 static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 {
 	int i;
-	void __iomem *kcan_ch0_base;
-	u64 kcan_controller_span;
 
-	kcan_ch0_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_ch0);
-	kcan_controller_span = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_ch1) - kcan_ch0_base;
 	for (i = 0; i < pcie->nr_channels; i++) {
 		struct net_device *netdev;
 		struct kvaser_pciefd_can *can;
-		u32 status, tx_nr_packets, tx_nr_packets_max;
+		u32 status;
 
 		netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
 				      KVASER_PCIEFD_CAN_TX_MAX_COUNT);
@@ -886,7 +1052,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		can = netdev_priv(netdev);
 		netdev->netdev_ops = &kvaser_pciefd_netdev_ops;
 		netdev->ethtool_ops = &kvaser_pciefd_ethtool_ops;
-		can->reg_base = kcan_ch0_base + i * kcan_controller_span;
+		can->reg_base = KVASER_PCIEFD_KCAN_CHX_ADDR(pcie, i);
 		can->kv_pcie = pcie;
 		can->cmd_seq = 0;
 		can->err_rep_cnt = 0;
@@ -898,14 +1064,12 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		timer_setup(&can->bec_poll_timer, kvaser_pciefd_bec_poll_timer, 0);
 
 		/* Disable Bus load reporting */
-		iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_BUS_LOAD_REG);
-
-		tx_nr_packets = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
-		tx_nr_packets_max =
-			(tx_nr_packets >> KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT) & 0xff;
+		KVASER_PCIEFD_KCAN_BUS_LOAD_DISABLE(can);
 
 		can->can.clock.freq = pcie->freq;
-		can->can.echo_skb_max = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);
+		can->can.echo_skb_max =
+			min(KVASER_PCIEFD_CAN_TX_MAX_COUNT,
+			    KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_GET(can) - 1);
 		can->echo_idx = 0;
 		spin_lock_init(&can->echo_lock);
 		spin_lock_init(&can->lock);
@@ -921,7 +1085,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 					      CAN_CTRLMODE_FD_NON_ISO |
 					      CAN_CTRLMODE_CC_LEN8_DLC;
 
-		status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);
+		status = KVASER_PCIEFD_KCAN_STAT_GET(can);
 		if (!(status & KVASER_PCIEFD_KCAN_STAT_FD)) {
 			dev_err(&pcie->pci->dev,
 				"CAN FD not supported as expected %d\n", i);
@@ -936,9 +1100,8 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		netdev->flags |= IFF_ECHO;
 		SET_NETDEV_DEV(netdev, &pcie->pci->dev);
 
-		iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-		iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
-			  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
+		KVASER_PCIEFD_KCAN_IRQ_CLEAR_ALL(can);
+		KVASER_PCIEFD_KCAN_IEN_ENABLE_ABD(can);
 
 		pcie->can[i] = can;
 		kvaser_pciefd_pwm_start(can);
@@ -980,7 +1143,7 @@ static void kvaser_pciefd_write_dma_map_altera(struct kvaser_pciefd *pcie,
 	word1 = addr;
 	word2 = 0;
 #endif
-	serdes_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, serdes) + 0x8 * index;
+	serdes_base = KVASER_PCIEFD_SERDES_ADDR(pcie) + 0x8 * index;
 	iowrite32(word1, serdes_base);
 	iowrite32(word2, serdes_base + 0x4);
 }
@@ -995,23 +1158,19 @@ static void kvaser_pciefd_write_dma_map_sf2(struct kvaser_pciefd *pcie,
 #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
 	msb = addr >> 32;
 #endif
-	serdes_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, serdes) + 0x10 * index;
+	serdes_base = KVASER_PCIEFD_SERDES_ADDR(pcie) + 0x10 * index;
 	iowrite32(lsb, serdes_base);
 	iowrite32(msb, serdes_base + 0x4);
 }
 
 static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 {
-	void __iomem *kcan_srb_base;
-	const struct kvaser_pciefd_dev_ops *dev_ops = pcie->driver_data->ops;
 	int i;
-	u32 srb_status;
 	u32 srb_packet_count;
 	dma_addr_t dma_addr[KVASER_PCIEFD_DMA_COUNT];
 
-	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
 	/* Disable the DMA */
-	iowrite32(0, kcan_srb_base + KVASER_PCIEFD_SRB_CTRL_REG);
+	KVASER_PCIEFD_SRB_DMA_DISABLE(pcie);
 	for (i = 0; i < KVASER_PCIEFD_DMA_COUNT; i++) {
 		pcie->dma_data[i] = dmam_alloc_coherent(&pcie->pci->dev,
 							KVASER_PCIEFD_DMA_SIZE,
@@ -1023,69 +1182,55 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 				KVASER_PCIEFD_DMA_SIZE);
 			return -ENOMEM;
 		}
-		dev_ops->kvaser_pciefd_write_dma_map(pcie, dma_addr[i], i);
+		KVASER_PCIEFD_WRITE_DMA_MAP(pcie, dma_addr[i], i);
 	}
 
 	/* Reset Rx FIFO, and both DMA buffers */
-	iowrite32(KVASER_PCIEFD_SRB_CMD_FOR | KVASER_PCIEFD_SRB_CMD_RDB0 |
-		  KVASER_PCIEFD_SRB_CMD_RDB1,
-		  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
+	KVASER_PCIEFD_SRB_CMD_SET(pcie, KVASER_PCIEFD_SRB_CMD_FOR |
+						KVASER_PCIEFD_SRB_CMD_RDB0 |
+						KVASER_PCIEFD_SRB_CMD_RDB1);
+
 	/* Empty Rx FIFO */
-	srb_packet_count = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG) &
-			   KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK;
+	srb_packet_count = KVASER_PCIEFD_SRB_RX_NR_PACKETS_CURRENT_GET(pcie);
 	while (srb_packet_count) {
 		/* Drop current packet in FIFO */
-		ioread32(KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb_fifo) +
-			 KVASER_PCIEFD_SRB_FIFO_LAST_REG);
+		KVASER_PCIEFD_SRB_FIFO_LAST_GET(pcie);
 		srb_packet_count--;
 	}
 
-	srb_status = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_STAT_REG);
-	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DI)) {
+	if (!(KVASER_PCIEFD_SRB_STAT_GET(pcie) & KVASER_PCIEFD_SRB_STAT_DI)) {
 		dev_err(&pcie->pci->dev, "DMA not idle before enabling\n");
 		return -EIO;
 	}
 
 	/* Enable the DMA */
-	iowrite32(KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE,
-		  kcan_srb_base + KVASER_PCIEFD_SRB_CTRL_REG);
+	KVASER_PCIEFD_SRB_DMA_ENABLE(pcie);
 
 	return 0;
 }
 
 static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 {
-	void __iomem *kcan_srb_base;
-	void __iomem *sysid_base;
-	u32 version, srb_status, build;
-
-	sysid_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, sysid);
-	version = ioread32(sysid_base + KVASER_PCIEFD_SYSID_VERSION_REG);
 	pcie->nr_channels = min(KVASER_PCIEFD_MAX_CAN_CHANNELS,
-				((version >> KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT) & 0xff));
+				KVASER_PCIEFD_SYSID_VERSION_NR_CHANNELS_GET(pcie));
 
-	build = ioread32(sysid_base + KVASER_PCIEFD_SYSID_BUILD_REG);
 	dev_dbg(&pcie->pci->dev, "Version %u.%u.%u\n",
-		(version >> KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT) & 0xff,
-		version & 0xff,
-		(build >> KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT) & 0x7fff);
+		KVASER_PCIEFD_SYSID_VERSION_MAJOR_GET(pcie),
+		KVASER_PCIEFD_SYSID_VERSION_MINOR_GET(pcie),
+		KVASER_PCIEFD_SYSID_BUILD_GET(pcie));
 
-	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
-	srb_status = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_STAT_REG);
-	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DMA)) {
+	if (!(KVASER_PCIEFD_SRB_STAT_GET(pcie) & KVASER_PCIEFD_SRB_STAT_DMA)) {
 		dev_err(&pcie->pci->dev, "Hardware without DMA is not supported\n");
 		return -ENODEV;
 	}
 
-	pcie->bus_freq = ioread32(sysid_base +
-				  KVASER_PCIEFD_SYSID_BUSFREQ_REG);
-	pcie->freq = ioread32(sysid_base + KVASER_PCIEFD_SYSID_CANFREQ_REG);
+	pcie->bus_freq = KVASER_PCIEFD_SYSID_BUSFREQ_GET(pcie);
+	pcie->freq = KVASER_PCIEFD_SYSID_CANFREQ_GET(pcie);
 	pcie->freq_to_ticks_div = pcie->freq / 1000000;
 	if (pcie->freq_to_ticks_div == 0)
 		pcie->freq_to_ticks_div = 1;
 	/* Turn off all loopback functionality */
-	iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, loopback));
-
+	KVASER_PCIEFD_LOOPBACK_DISABLE(pcie);
 	return 0;
 }
 
@@ -1202,8 +1347,8 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
 
 	old_state = can->can.state;
 
-	bec.txerr = p->header[0] & 0xff;
-	bec.rxerr = (p->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff;
+	bec.txerr = KVASER_PCIEFD_SPACKET_TXERR_COUNT(p);
+	bec.rxerr = KVASER_PCIEFD_SPACKET_RXERR_COUNT(p);
 
 	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state, &rx_state);
 	skb = alloc_can_err_skb(ndev, &cf);
@@ -1271,8 +1416,8 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
 
 	old_state = can->can.state;
 
-	bec.txerr = p->header[0] & 0xff;
-	bec.rxerr = (p->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff;
+	bec.txerr = KVASER_PCIEFD_SPACKET_TXERR_COUNT(p);
+	bec.rxerr = KVASER_PCIEFD_SPACKET_RXERR_COUNT(p);
 
 	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state, &rx_state);
 	if (new_state != old_state) {
@@ -1323,41 +1468,35 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 
 	can = pcie->can[ch_id];
 
-	status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);
-	cmdseq = (status >> KVASER_PCIEFD_KCAN_STAT_SEQNO_SHIFT) & 0xff;
+	status = KVASER_PCIEFD_KCAN_STAT_GET(can);
+	cmdseq = (status >> KVASER_PCIEFD_KCAN_STAT_SEQNO_SHIFT) & KVASER_PCIEFD_PACKET_SEQ_MASK;
 
 	/* Reset done, start abort and flush */
-	if (p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
-	    p->header[0] & KVASER_PCIEFD_SPACK_RMCD &&
-	    p->header[1] & KVASER_PCIEFD_SPACK_AUTO &&
-	    cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK) &&
-	    status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
-		u32 cmd;
-
-		iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
-			  can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-		cmd = KVASER_PCIEFD_KCAN_CMD_AT;
-		cmd |= ++can->cmd_seq << KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT;
-		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
-	} else if (p->header[0] & KVASER_PCIEFD_SPACK_IDET &&
-		   p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
-		   cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK) &&
-		   status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
+	if ((p->header[0] & KVASER_PCIEFD_SPACK_IRM) &&
+	    (p->header[0] & KVASER_PCIEFD_SPACK_RMCD) &&
+	    (p->header[1] & KVASER_PCIEFD_SPACK_AUTO) &&
+	    (cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK)) &&
+	    (status & KVASER_PCIEFD_KCAN_STAT_IDLE)) {
+		KVASER_PCIEFD_KCAN_IRQ_SET(can, KVASER_PCIEFD_KCAN_IRQ_ABD);
+		kvaser_pciefd_kcan_abort_flush_reset(can);
+	} else if ((p->header[0] & KVASER_PCIEFD_SPACK_IDET) &&
+		   (p->header[0] & KVASER_PCIEFD_SPACK_IRM) &&
+		   (cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK)) &&
+		   (status & KVASER_PCIEFD_KCAN_STAT_IDLE)) {
 		/* Reset detected, send end of flush if no packet are in FIFO */
-		u8 count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
+		u8 count = KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_GET(can);
 
 		if (!count)
-			iowrite32(KVASER_PCIEFD_KCAN_CTRL_EFLUSH,
-				  can->reg_base + KVASER_PCIEFD_KCAN_CTRL_REG);
+			KVASER_PCIEFD_KCAN_CTRL_SET(can, KVASER_PCIEFD_KCAN_CTRL_EFLUSH);
 	} else if (!(p->header[1] & KVASER_PCIEFD_SPACK_AUTO) &&
-		   cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK)) {
+		   (cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK))) {
 		/* Response to status request received */
 		kvaser_pciefd_handle_status_resp(can, p);
 		if (can->can.state != CAN_STATE_BUS_OFF &&
 		    can->can.state != CAN_STATE_ERROR_ACTIVE) {
 			mod_timer(&can->bec_poll_timer, KVASER_PCIEFD_BEC_POLL_FREQ);
 		}
-	} else if (p->header[0] & KVASER_PCIEFD_SPACK_RMCD &&
+	} else if ((p->header[0] & KVASER_PCIEFD_SPACK_RMCD) &&
 		   !(status & KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MASK)) {
 		/* Reset to bus on detected */
 		if (!completion_done(&can->start_comp))
@@ -1425,7 +1564,7 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 		if (skb)
 			kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
 		len = can_get_echo_skb(can->can.dev, echo_idx, NULL);
-		count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
+		count = KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_GET(can);
 
 		if (count < can->can.echo_skb_max && netif_queue_stopped(can->can.dev))
 			netif_wake_queue(can->can.dev);
@@ -1482,7 +1621,7 @@ static int kvaser_pciefd_read_packet(struct kvaser_pciefd *pcie, int *start_pos,
 	pos += 2;
 	p->timestamp = le64_to_cpu(timestamp);
 
-	type = (p->header[1] >> KVASER_PCIEFD_PACKET_TYPE_SHIFT) & KVASER_PCIEFD_PACKET_TYPE_MASK;
+	type = KVASER_PCIEFD_PACKET_TYPE(p);
 	switch (type) {
 	case KVASER_PCIEFD_PACK_TYPE_DATA:
 		ret = kvaser_pciefd_handle_data_packet(pcie, p, &buffer[pos]);
@@ -1554,23 +1693,18 @@ static int kvaser_pciefd_read_buffer(struct kvaser_pciefd *pcie, int dma_buf)
 
 static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
 {
-	void __iomem *kcan_srb_base;
-	u32 irq;
+	u32 irq = KVASER_PCIEFD_SRB_IRQ_GET(pcie);
 
-	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
-	irq = ioread32(kcan_srb_base + KVASER_PCIEFD_SRB_IRQ_REG);
 	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD0) {
 		kvaser_pciefd_read_buffer(pcie, 0);
 		/* Reset DMA buffer 0 */
-		iowrite32(KVASER_PCIEFD_SRB_CMD_RDB0,
-			  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
+		KVASER_PCIEFD_SRB_CMD_SET(pcie, KVASER_PCIEFD_SRB_CMD_RDB0);
 	}
 
 	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD1) {
 		kvaser_pciefd_read_buffer(pcie, 1);
 		/* Reset DMA buffer 1 */
-		iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1,
-			  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
+		KVASER_PCIEFD_SRB_CMD_SET(pcie, KVASER_PCIEFD_SRB_CMD_RDB1);
 	}
 
 	if (irq & KVASER_PCIEFD_SRB_IRQ_DOF0 ||
@@ -1579,12 +1713,12 @@ static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)
 	    irq & KVASER_PCIEFD_SRB_IRQ_DUF1)
 		dev_err(&pcie->pci->dev, "DMA IRQ error 0x%08X\n", irq);
 
-	iowrite32(irq, kcan_srb_base + KVASER_PCIEFD_SRB_IRQ_REG);
+	KVASER_PCIEFD_SRB_IRQ_SET(pcie, irq);
 }
 
 static void kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
 {
-	u32 irq = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
+	u32 irq = KVASER_PCIEFD_KCAN_IRQ_GET(can);
 
 	if (irq & KVASER_PCIEFD_KCAN_IRQ_TOF)
 		netdev_err(can->can.dev, "Tx FIFO overflow\n");
@@ -1599,18 +1733,16 @@ static void kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
 	if (irq & KVASER_PCIEFD_KCAN_IRQ_ROF)
 		netdev_err(can->can.dev, "Rx FIFO overflow\n");
 
-	iowrite32(irq, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
+	KVASER_PCIEFD_KCAN_IRQ_SET(can, irq);
 }
 
 static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
 {
 	struct kvaser_pciefd *pcie = (struct kvaser_pciefd *)dev;
 	const struct kvaser_pciefd_irq_mask *irq_mask = pcie->driver_data->irq_mask;
-	u32 board_irq;
+	u32 board_irq = KVASER_PCIEFD_PCI_IRQ_GET(pcie);
 	int i;
 
-	board_irq = ioread32(KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, pci_irq));
-
 	if (!(board_irq & irq_mask->all))
 		return IRQ_NONE;
 
@@ -1640,7 +1772,7 @@ static void kvaser_pciefd_teardown_can_ctrls(struct kvaser_pciefd *pcie)
 		struct kvaser_pciefd_can *can = pcie->can[i];
 
 		if (can) {
-			iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
+			KVASER_PCIEFD_KCAN_IEN_DISABLE_ALL(can);
 			kvaser_pciefd_pwm_stop(can);
 			free_candev(can->can.dev);
 		}
@@ -1653,8 +1785,6 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 	int err;
 	struct kvaser_pciefd *pcie;
 	const struct kvaser_pciefd_irq_mask *irq_mask;
-	void __iomem *kcan_srb_base;
-	void __iomem *irq_en_base;
 
 	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
 	if (!pcie)
@@ -1689,7 +1819,6 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	kcan_srb_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb);
 	err = kvaser_pciefd_setup_can_ctrls(pcie);
 	if (err)
 		goto err_teardown_can_ctrls;
@@ -1699,22 +1828,16 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_teardown_can_ctrls;
 
-	iowrite32(KVASER_PCIEFD_SRB_IRQ_DPD0 | KVASER_PCIEFD_SRB_IRQ_DPD1,
-		  kcan_srb_base + KVASER_PCIEFD_SRB_IRQ_REG);
-
-	iowrite32(KVASER_PCIEFD_SRB_IRQ_DPD0 | KVASER_PCIEFD_SRB_IRQ_DPD1 |
-		  KVASER_PCIEFD_SRB_IRQ_DOF0 | KVASER_PCIEFD_SRB_IRQ_DOF1 |
-		  KVASER_PCIEFD_SRB_IRQ_DUF0 | KVASER_PCIEFD_SRB_IRQ_DUF1,
-		  kcan_srb_base + KVASER_PCIEFD_SRB_IEN_REG);
+	/* Enable shared receive buffer interrupts */
+	KVASER_PCIEFD_SRB_IRQ_SET(pcie, KVASER_PCIEFD_SRB_IRQ_DPD0 |
+						KVASER_PCIEFD_SRB_IRQ_DPD1);
+	KVASER_PCIEFD_SRB_IEN_ENABLE_ALL(pcie);
 
 	/* Enable PCI interrupts */
-	irq_en_base = KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, pci_ien);
-	iowrite32(irq_mask->all, irq_en_base);
+	KVASER_PCIEFD_PCI_IEN_ENABLE_ALL(pcie);
 	/* Ready the DMA buffers */
-	iowrite32(KVASER_PCIEFD_SRB_CMD_RDB0,
-		  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
-	iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1,
-		  kcan_srb_base + KVASER_PCIEFD_SRB_CMD_REG);
+	KVASER_PCIEFD_SRB_CMD_SET(pcie, KVASER_PCIEFD_SRB_CMD_RDB0);
+	KVASER_PCIEFD_SRB_CMD_SET(pcie, KVASER_PCIEFD_SRB_CMD_RDB1);
 
 	err = kvaser_pciefd_reg_candev(pcie);
 	if (err)
@@ -1724,12 +1847,12 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 
 err_free_irq:
 	/* Disable PCI interrupts */
-	iowrite32(0, irq_en_base);
+	KVASER_PCIEFD_PCI_IEN_DISABLE_ALL(pcie);
 	free_irq(pcie->pci->irq, pcie);
 
 err_teardown_can_ctrls:
 	kvaser_pciefd_teardown_can_ctrls(pcie);
-	iowrite32(0, kcan_srb_base + KVASER_PCIEFD_SRB_CTRL_REG);
+	KVASER_PCIEFD_SRB_DMA_DISABLE(pcie);
 	pci_clear_master(pdev);
 
 err_pci_iounmap:
@@ -1752,7 +1875,7 @@ static void kvaser_pciefd_remove_all_ctrls(struct kvaser_pciefd *pcie)
 		struct kvaser_pciefd_can *can = pcie->can[i];
 
 		if (can) {
-			iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
+			KVASER_PCIEFD_KCAN_IEN_DISABLE_ALL(can);
 			unregister_candev(can->can.dev);
 			del_timer(&can->bec_poll_timer);
 			kvaser_pciefd_pwm_stop(can);
@@ -1768,8 +1891,8 @@ static void kvaser_pciefd_remove(struct pci_dev *pdev)
 	kvaser_pciefd_remove_all_ctrls(pcie);
 
 	/* Disable interrupts */
-	iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, kcan_srb) + KVASER_PCIEFD_SRB_CTRL_REG);
-	iowrite32(0, KVASER_PCIEFD_GET_BLOCK_ADDR(pcie, pci_ien));
+	KVASER_PCIEFD_SRB_DMA_DISABLE(pcie);
+	KVASER_PCIEFD_PCI_IEN_DISABLE_ALL(pcie);
 
 	free_irq(pcie->pci->irq, pcie);
 
-- 
2.40.0


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

* Re: [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments
  2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
                   ` (15 preceding siblings ...)
  2023-05-23  9:43 ` [PATCH 3/3] can: kvaser_pciefd: Wrap register read and writes with macros Jimmy Assarsson
@ 2023-05-23  9:50 ` Jimmy Assarsson
  16 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:50 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson

On 2023-05-23 11:43, Jimmy Assarsson wrote:
> This patch series contains various non critical fixes and improvements for
> the kvaser_pciefd driver.

I messed up and sent both patch series at the same time:
[PATCH 00/12] can: kvaser_pciefd: Fixes and improvments
[PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices

Let me know if I should resend them again in separate mails.

/jimmy

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

* Re: [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices
  2023-05-23  9:43 ` [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices Jimmy Assarsson
@ 2023-05-23  9:51   ` Jimmy Assarsson
  2023-06-22 15:16   ` Jimmy Assarsson
  1 sibling, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-23  9:51 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson

On 2023-05-23 11:43, Jimmy Assarsson wrote:
> This patch series adds support for a range of new Kvaser PCI Express
> devices based on the SmartFusion2 SoC, to the kvaser_pciefd driver.
> 
> In the first patch, the hardware specific constants and functions are
> moved into a driver_data struct.
> 
> In the second patch, we add the new devices and their hardware specific
> constants and functions.
> 
> In the last patch, most of the register reading and writing + shifting
> and masking, are wrapped in macros, to simplify the functions.
> 
> 
> Note: This series depends on the changes in xxx

xxx should be [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments [1]

[1] https://lore.kernel.org/linux-can/20230523094354.83792-1-extja@kvaser.com/

/jimmy

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

* Re: [PATCH 11/12] can: kvaser_pciefd: Refactor code
  2023-05-23  9:43 ` [PATCH 11/12] can: kvaser_pciefd: Refactor code Jimmy Assarsson
@ 2023-05-23 11:27   ` Vincent MAILHOL
  2023-05-24  7:40     ` Jimmy Assarsson
  0 siblings, 1 reply; 22+ messages in thread
From: Vincent MAILHOL @ 2023-05-23 11:27 UTC (permalink / raw)
  To: Jimmy Assarsson; +Cc: linux-can, Jimmy Assarsson

Hi Jimmy,

I have one single comment for this series.

On Tue. 23 May 2023 at 18:55, Jimmy Assarsson <extja@kvaser.com> wrote:
> Refactor code;
>  - Format code
>  - Replace constants with macros
>  - Rename variables and macros
>  - Remove intermediate variable
>  - Add/remove blank lines
>  - Add function to fetch channel id from Rx packets
>  - Reduce scope of variables
>
> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
> ---
>  drivers/net/can/kvaser_pciefd.c | 236 +++++++++++++-------------------
>  1 file changed, 97 insertions(+), 139 deletions(-)
>
> diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
> index 3237c71afd2b..0575bb84b280 100644
> --- a/drivers/net/can/kvaser_pciefd.c
> +++ b/drivers/net/can/kvaser_pciefd.c
> @@ -53,7 +53,7 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
>  #define KVASER_PCIEFD_KCAN_CMD_REG 0x400
>  #define KVASER_PCIEFD_KCAN_IEN_REG 0x408
>  #define KVASER_PCIEFD_KCAN_IRQ_REG 0x410
> -#define KVASER_PCIEFD_KCAN_TX_NPACKETS_REG 0x414
> +#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG 0x414
>  #define KVASER_PCIEFD_KCAN_STAT_REG 0x418
>  #define KVASER_PCIEFD_KCAN_MODE_REG 0x41c
>  #define KVASER_PCIEFD_KCAN_BTRN_REG 0x420
> @@ -81,9 +81,9 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
>  #define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
>  #define KVASER_PCIEFD_IRQ_SRB BIT(4)
>
> -#define KVASER_PCIEFD_SYSID_NRCHAN_SHIFT 24
> -#define KVASER_PCIEFD_SYSID_MAJOR_VER_SHIFT 16
> -#define KVASER_PCIEFD_SYSID_BUILD_VER_SHIFT 1
> +#define KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT 24
> +#define KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT 16
> +#define KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT 1
>
>  /* Reset DMA buffer 0, 1 and FIFO offset */
>  #define KVASER_PCIEFD_SRB_CMD_RDB0 BIT(4)
> @@ -142,7 +142,7 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
>  /* Transmitter unaligned */
>  #define KVASER_PCIEFD_KCAN_IRQ_TAL BIT(17)
>
> -#define KVASER_PCIEFD_KCAN_TX_NPACKETS_MAX_SHIFT 16
> +#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT 16
>
>  #define KVASER_PCIEFD_KCAN_STAT_SEQNO_SHIFT 24
>  /* Abort request */
> @@ -159,9 +159,9 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
>  #define KVASER_PCIEFD_KCAN_STAT_CAP BIT(16)
>  /* Controller got CAN FD capability */
>  #define KVASER_PCIEFD_KCAN_STAT_FD BIT(19)
> -#define KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MSK (KVASER_PCIEFD_KCAN_STAT_AR | \
> -       KVASER_PCIEFD_KCAN_STAT_BOFF | KVASER_PCIEFD_KCAN_STAT_RMR | \
> -       KVASER_PCIEFD_KCAN_STAT_IRM)
> +#define KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MASK                         \
> +       (KVASER_PCIEFD_KCAN_STAT_AR | KVASER_PCIEFD_KCAN_STAT_BOFF | \
> +        KVASER_PCIEFD_KCAN_STAT_RMR | KVASER_PCIEFD_KCAN_STAT_IRM)
>
>  /* Reset mode */
>  #define KVASER_PCIEFD_KCAN_MODE_RM BIT(8)
> @@ -178,9 +178,13 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
>  /* Classic CAN mode */
>  #define KVASER_PCIEFD_KCAN_MODE_CCM BIT(31)
>
> +#define KVASER_PCIEFD_KCAN_BTRN_BRP_MASK 0x1fff
>  #define KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT 13
> +#define KVASER_PCIEFD_KCAN_BTRN_SJW_MASK 0xf
>  #define KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT 17
> +#define KVASER_PCIEFD_KCAN_BTRN_TSEG1_MASK 0x1ff
>  #define KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT 26
> +#define KVASER_PCIEFD_KCAN_BTRN_TSEG2_MASK 0x1f
>
>  #define KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT 16
>
> @@ -196,9 +200,11 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
>  #define KVASER_PCIEFD_PACK_TYPE_BUS_LOAD 9
>
>  /* Kvaser KCAN packet common definitions */
> -#define KVASER_PCIEFD_PACKET_SEQ_MSK 0xff
> +#define KVASER_PCIEFD_PACKET_SEQ_MASK 0xff
>  #define KVASER_PCIEFD_PACKET_CHID_SHIFT 25
> +#define KVASER_PCIEFD_PACKET_CHID_MASK 0x7
>  #define KVASER_PCIEFD_PACKET_TYPE_SHIFT 28
> +#define KVASER_PCIEFD_PACKET_TYPE_MASK 0xf
>
>  /* Kvaser KCAN TDATA and RDATA first word */
>  #define KVASER_PCIEFD_RPACKET_IDE BIT(30)
> @@ -303,6 +309,11 @@ static struct pci_device_id kvaser_pciefd_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, kvaser_pciefd_id_table);
>
> +static inline u8 kvaser_pciefd_rx_packet_get_ch_id(struct kvaser_pciefd_rx_packet *p)
> +{
> +       return (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & KVASER_PCIEFD_PACKET_CHID_MASK;

Instead of shifting and appliying the mask, define a mask which is
already shifted with GEN_MASK.

  Then use the FIELD_GET and FIELD_PREP from linux/bitfield.h.

The same comment applies to the other shift and mask operations.

This GEN_MASK, FIELD_GET and FIELD_PREP can be a separate patch.

> +}
> +
>  static void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
>  {
>         u32 cmd;
> @@ -364,7 +375,6 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
>         unsigned long irq;
>
>         spin_lock_irqsave(&can->lock, irq);
> -
>         mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
>         if (can->can.ctrlmode & CAN_CTRLMODE_FD) {
>                 mode &= ~KVASER_PCIEFD_KCAN_MODE_CCM;
> @@ -381,7 +391,6 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
>                 mode |= KVASER_PCIEFD_KCAN_MODE_LOM;
>         else
>                 mode &= ~KVASER_PCIEFD_KCAN_MODE_LOM;
> -
>         mode |= KVASER_PCIEFD_KCAN_MODE_EEN;
>         mode |= KVASER_PCIEFD_KCAN_MODE_EPEN;
>         /* Use ACK packet type */
> @@ -401,7 +410,6 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
>         iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
>         iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
>                   can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
> -
>         status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);
>         if (status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
>                 u32 cmd;
> @@ -418,7 +426,6 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
>                 mode |= KVASER_PCIEFD_KCAN_MODE_RM;
>                 iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
>         }
> -
>         spin_unlock_irqrestore(&can->lock, irq);
>  }
>
> @@ -428,7 +435,6 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
>         unsigned long irq;
>
>         del_timer(&can->bec_poll_timer);
> -
>         if (!completion_done(&can->flush_comp))
>                 kvaser_pciefd_start_controller_flush(can);
>
> @@ -441,10 +447,8 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
>         spin_lock_irqsave(&can->lock, irq);
>         iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
>         iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
> -
>         iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
>                   can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
> -
>         mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
>         mode &= ~KVASER_PCIEFD_KCAN_MODE_RM;
>         iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
> @@ -461,7 +465,6 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
>
>         kvaser_pciefd_set_tx_irq(can);
>         kvaser_pciefd_setup_controller(can);
> -
>         can->can.state = CAN_STATE_ERROR_ACTIVE;
>         netif_wake_queue(can->can.dev);
>         can->bec.txerr = 0;
> @@ -480,7 +483,6 @@ static void kvaser_pciefd_pwm_stop(struct kvaser_pciefd_can *can)
>         spin_lock_irqsave(&can->lock, irq);
>         pwm_ctrl = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
>         top = (pwm_ctrl >> KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT) & 0xff;
> -
>         /* Set duty cycle to zero */
>         pwm_ctrl |= top;
>         iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
> @@ -495,10 +497,8 @@ static void kvaser_pciefd_pwm_start(struct kvaser_pciefd_can *can)
>
>         kvaser_pciefd_pwm_stop(can);
>         spin_lock_irqsave(&can->lock, irq);
> -
>         /* Set frequency to 500 KHz*/
>         top = can->kv_pcie->bus_freq / (2 * 500000) - 1;
> -
>         pwm_ctrl = top & 0xff;
>         pwm_ctrl |= (top & 0xff) << KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT;
>         iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
> @@ -561,7 +561,6 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
>         int seq = can->echo_idx;
>
>         memset(p, 0, sizeof(*p));
> -
>         if (can->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)
>                 p->header[1] |= KVASER_PCIEFD_TPACKET_SMS;
>
> @@ -587,7 +586,7 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
>                                 << KVASER_PCIEFD_RPACKET_DLC_SHIFT;
>         }
>
> -       p->header[1] |= seq & KVASER_PCIEFD_PACKET_SEQ_MSK;
> +       p->header[1] |= seq & KVASER_PCIEFD_PACKET_SEQ_MASK;
>
>         packet_size = cf->len;
>         memcpy(p->data, cf->data, packet_size);
> @@ -601,16 +600,15 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
>         struct kvaser_pciefd_can *can = netdev_priv(netdev);
>         unsigned long irq_flags;
>         struct kvaser_pciefd_tx_packet packet;
> -       int nwords;
> +       int nr_words;
>         u8 count;
>
>         if (can_dev_dropped_skb(netdev, skb))
>                 return NETDEV_TX_OK;
>
> -       nwords = kvaser_pciefd_prepare_tx_packet(&packet, can, skb);
> +       nr_words = kvaser_pciefd_prepare_tx_packet(&packet, can, skb);
>
>         spin_lock_irqsave(&can->echo_lock, irq_flags);
> -
>         /* Prepare and save echo skb in internal slot */
>         can_put_echo_skb(skb, netdev, can->echo_idx, 0);
>
> @@ -623,13 +621,13 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
>         iowrite32(packet.header[1],
>                   can->reg_base + KVASER_PCIEFD_KCAN_FIFO_REG);
>
> -       if (nwords) {
> -               u32 data_last = ((u32 *)packet.data)[nwords - 1];
> +       if (nr_words) {
> +               u32 data_last = ((u32 *)packet.data)[nr_words - 1];
>
>                 /* Write data to fifo, except last word */
>                 iowrite32_rep(can->reg_base +
>                               KVASER_PCIEFD_KCAN_FIFO_REG, packet.data,
> -                             nwords - 1);
> +                             nr_words - 1);
>                 /* Write last word to end of fifo */
>                 __raw_writel(data_last, can->reg_base +
>                              KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
> @@ -638,15 +636,13 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
>                 __raw_writel(0, can->reg_base +
>                              KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
>         }
> -
> -       count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG);
> +       count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
>         /* No room for a new message, stop the queue until at least one
>          * successful transmit
>          */
>         if (count >= KVASER_PCIEFD_CAN_TX_MAX_COUNT ||
>             can->can.echo_skb[can->echo_idx])
>                 netif_stop_queue(netdev);
> -
>         spin_unlock_irqrestore(&can->echo_lock, irq_flags);
>
>         return NETDEV_TX_OK;
> @@ -664,25 +660,24 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
>         else
>                 bt = &can->can.bittiming;
>
> -       btrn = ((bt->phase_seg2 - 1) & 0x1f) <<
> -              KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT |
> -              (((bt->prop_seg + bt->phase_seg1) - 1) & 0x1ff) <<
> -              KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT |
> -              ((bt->sjw - 1) & 0xf) << KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT |
> -              ((bt->brp - 1) & 0x1fff);
> +       btrn = ((bt->phase_seg2 - 1) & KVASER_PCIEFD_KCAN_BTRN_TSEG2_MASK)
> +                      << KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT |
> +              (((bt->prop_seg + bt->phase_seg1) - 1) &
> +               KVASER_PCIEFD_KCAN_BTRN_TSEG1_MASK)
> +                      << KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT |
> +              ((bt->sjw - 1) & KVASER_PCIEFD_KCAN_BTRN_SJW_MASK)
> +                      << KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT |
> +              ((bt->brp - 1) & KVASER_PCIEFD_KCAN_BTRN_BRP_MASK);
>
>         spin_lock_irqsave(&can->lock, irq_flags);
>         mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
> -
>         /* Put the circuit in reset mode */
>         iowrite32(mode | KVASER_PCIEFD_KCAN_MODE_RM,
>                   can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
>
>         /* Can only set bittiming if in reset mode */
>         ret = readl_poll_timeout(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG,
> -                                test, test & KVASER_PCIEFD_KCAN_MODE_RM,
> -                                0, 10);
> -
> +                                test, test & KVASER_PCIEFD_KCAN_MODE_RM, 0, 10);
>         if (ret) {
>                 spin_unlock_irqrestore(&can->lock, irq_flags);
>                 return -EBUSY;
> @@ -692,11 +687,10 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
>                 iowrite32(btrn, can->reg_base + KVASER_PCIEFD_KCAN_BTRD_REG);
>         else
>                 iowrite32(btrn, can->reg_base + KVASER_PCIEFD_KCAN_BTRN_REG);
> -
>         /* Restore previous reset mode status */
>         iowrite32(mode, can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
> -
>         spin_unlock_irqrestore(&can->lock, irq_flags);
> +
>         return 0;
>  }
>
> @@ -734,6 +728,7 @@ static int kvaser_pciefd_get_berr_counter(const struct net_device *ndev,
>
>         bec->rxerr = can->bec.rxerr;
>         bec->txerr = can->bec.txerr;
> +
>         return 0;
>  }
>
> @@ -765,7 +760,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
>         for (i = 0; i < pcie->nr_channels; i++) {
>                 struct net_device *netdev;
>                 struct kvaser_pciefd_can *can;
> -               u32 status, tx_npackets;
> +               u32 status, tx_nr_packets;
>
>                 netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
>                                       KVASER_PCIEFD_CAN_TX_MAX_COUNT);
> @@ -777,7 +772,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
>                 netdev->ethtool_ops = &kvaser_pciefd_ethtool_ops;
>                 can->reg_base = pcie->reg_base + KVASER_PCIEFD_KCAN0_BASE +
>                                 i * KVASER_PCIEFD_KCAN_BASE_OFFSET;
> -
>                 can->kv_pcie = pcie;
>                 can->cmd_seq = 0;
>                 can->err_rep_cnt = 0;
> @@ -786,15 +780,13 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
>
>                 init_completion(&can->start_comp);
>                 init_completion(&can->flush_comp);
> -               timer_setup(&can->bec_poll_timer, kvaser_pciefd_bec_poll_timer,
> -                           0);
> +               timer_setup(&can->bec_poll_timer, kvaser_pciefd_bec_poll_timer, 0);
>
>                 /* Disable Bus load reporting */
>                 iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_BUS_LOAD_REG);
>
> -               tx_npackets = ioread32(can->reg_base +
> -                                      KVASER_PCIEFD_KCAN_TX_NPACKETS_REG);
> -               if (((tx_npackets >> KVASER_PCIEFD_KCAN_TX_NPACKETS_MAX_SHIFT) &
> +               tx_nr_packets = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG);
> +               if (((tx_nr_packets >> KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_SHIFT) &
>                       0xff) < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
>                         dev_err(&pcie->pci->dev,
>                                 "Max Tx count is smaller than expected\n");
> @@ -808,16 +800,13 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
>                 can->echo_idx = 0;
>                 spin_lock_init(&can->echo_lock);
>                 spin_lock_init(&can->lock);
> +
>                 can->can.bittiming_const = &kvaser_pciefd_bittiming_const;
>                 can->can.data_bittiming_const = &kvaser_pciefd_bittiming_const;
> -
>                 can->can.do_set_bittiming = kvaser_pciefd_set_nominal_bittiming;
> -               can->can.do_set_data_bittiming =
> -                       kvaser_pciefd_set_data_bittiming;
> -
> +               can->can.do_set_data_bittiming = kvaser_pciefd_set_data_bittiming;
>                 can->can.do_set_mode = kvaser_pciefd_set_mode;
>                 can->can.do_get_berr_counter = kvaser_pciefd_get_berr_counter;
> -
>                 can->can.ctrlmode_supported = CAN_CTRLMODE_LISTENONLY |
>                                               CAN_CTRLMODE_FD |
>                                               CAN_CTRLMODE_FD_NON_ISO |
> @@ -836,7 +825,6 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
>                         can->can.ctrlmode_supported |= CAN_CTRLMODE_ONE_SHOT;
>
>                 netdev->flags |= IFF_ECHO;
> -
>                 SET_NETDEV_DEV(netdev, &pcie->pci->dev);
>
>                 iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
> @@ -898,18 +886,16 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
>         for (i = 0; i < KVASER_PCIEFD_DMA_COUNT; i++) {
>                 unsigned int offset = KVASER_PCIEFD_DMA_MAP_BASE + 8 * i;
>
> -               pcie->dma_data[i] =
> -                       dmam_alloc_coherent(&pcie->pci->dev,
> -                                           KVASER_PCIEFD_DMA_SIZE,
> -                                           &dma_addr[i],
> -                                           GFP_KERNEL);
> +               pcie->dma_data[i] = dmam_alloc_coherent(&pcie->pci->dev,
> +                                                       KVASER_PCIEFD_DMA_SIZE,
> +                                                       &dma_addr[i],
> +                                                       GFP_KERNEL);
>
>                 if (!pcie->dma_data[i] || !dma_addr[i]) {
>                         dev_err(&pcie->pci->dev, "Rx dma_alloc(%u) failure\n",
>                                 KVASER_PCIEFD_DMA_SIZE);
>                         return -ENOMEM;
>                 }
> -
>                 kvaser_pciefd_write_dma_map(pcie, dma_addr[i], offset);
>         }
>
> @@ -917,7 +903,6 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
>         iowrite32(KVASER_PCIEFD_SRB_CMD_FOR | KVASER_PCIEFD_SRB_CMD_RDB0 |
>                   KVASER_PCIEFD_SRB_CMD_RDB1,
>                   pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
> -
>         /* Empty Rx FIFO */
>         srb_packet_count = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG) &
>                            KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK;
> @@ -942,22 +927,21 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
>
>  static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
>  {
> -       u32 sysid, srb_status, build;
> +       u32 version, srb_status, build;
>
> -       sysid = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_VERSION_REG);
> +       version = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_VERSION_REG);
>         pcie->nr_channels = min(KVASER_PCIEFD_MAX_CAN_CHANNELS,
> -                               ((sysid >> KVASER_PCIEFD_SYSID_NRCHAN_SHIFT) & 0xff));
> +                               ((version >> KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_SHIFT) & 0xff));
>
>         build = ioread32(pcie->reg_base + KVASER_PCIEFD_SYSID_BUILD_REG);
>         dev_dbg(&pcie->pci->dev, "Version %u.%u.%u\n",
> -               (sysid >> KVASER_PCIEFD_SYSID_MAJOR_VER_SHIFT) & 0xff,
> -               sysid & 0xff,
> -               (build >> KVASER_PCIEFD_SYSID_BUILD_VER_SHIFT) & 0x7fff);
> +               (version >> KVASER_PCIEFD_SYSID_VERSION_MAJOR_SHIFT) & 0xff,
> +               version & 0xff,
> +               (build >> KVASER_PCIEFD_SYSID_BUILD_VERSION_SHIFT) & 0x7fff);
>
>         srb_status = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_STAT_REG);
>         if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DMA)) {
> -               dev_err(&pcie->pci->dev,
> -                       "Hardware without DMA is not supported\n");
> +               dev_err(&pcie->pci->dev, "Hardware without DMA is not supported\n");
>                 return -ENODEV;
>         }
>
> @@ -967,9 +951,9 @@ static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
>         pcie->freq_to_ticks_div = pcie->freq / 1000000;
>         if (pcie->freq_to_ticks_div == 0)
>                 pcie->freq_to_ticks_div = 1;
> -
>         /* Turn off all loopback functionality */
>         iowrite32(0, pcie->reg_base + KVASER_PCIEFD_LOOP_REG);
> +
>         return 0;
>  }
>
> @@ -980,34 +964,31 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
>         struct sk_buff *skb;
>         struct canfd_frame *cf;
>         struct can_priv *priv;
> -       struct net_device_stats *stats;
> -       u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
> +       u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
>         u8 dlc;
>
>         if (ch_id >= pcie->nr_channels)
>                 return -EIO;
>
>         priv = &pcie->can[ch_id]->can;
> -       stats = &priv->dev->stats;
>         dlc = (p->header[1] >> KVASER_PCIEFD_RPACKET_DLC_SHIFT) & 0xf;
>
>         if (p->header[1] & KVASER_PCIEFD_RPACKET_FDF) {
>                 skb = alloc_canfd_skb(priv->dev, &cf);
>                 if (!skb) {
> -                       stats->rx_dropped++;
> +                       priv->dev->stats.rx_dropped++;
>                         return -ENOMEM;
>                 }
>
>                 cf->len = can_fd_dlc2len(dlc);
>                 if (p->header[1] & KVASER_PCIEFD_RPACKET_BRS)
>                         cf->flags |= CANFD_BRS;
> -
>                 if (p->header[1] & KVASER_PCIEFD_RPACKET_ESI)
>                         cf->flags |= CANFD_ESI;
>         } else {
>                 skb = alloc_can_skb(priv->dev, (struct can_frame **)&cf);
>                 if (!skb) {
> -                       stats->rx_dropped++;
> +                       priv->dev->stats.rx_dropped++;
>                         return -ENOMEM;
>                 }
>                 can_frame_set_cc_len((struct can_frame *)cf, dlc, priv->ctrlmode);
> @@ -1021,10 +1002,9 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
>                 cf->can_id |= CAN_RTR_FLAG;
>         } else {
>                 memcpy(cf->data, data, cf->len);
> -
> -               stats->rx_bytes += cf->len;
> +               priv->dev->stats.rx_bytes += cf->len;
>         }
> -       stats->rx_packets++;
> +       priv->dev->stats.rx_packets++;
>         kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
>
>         return netif_rx(skb);
> @@ -1045,7 +1025,6 @@ static void kvaser_pciefd_change_state(struct kvaser_pciefd_can *can,
>                 spin_lock_irqsave(&can->lock, irq_flags);
>                 netif_stop_queue(can->can.dev);
>                 spin_unlock_irqrestore(&can->lock, irq_flags);
> -
>                 /* Prevent CAN controller from auto recover from bus off */
>                 if (!can->can.restart_ms) {
>                         kvaser_pciefd_start_controller_flush(can);
> @@ -1063,7 +1042,7 @@ static void kvaser_pciefd_packet_to_state(struct kvaser_pciefd_rx_packet *p,
>         if (p->header[0] & KVASER_PCIEFD_SPACK_BOFF ||
>             p->header[0] & KVASER_PCIEFD_SPACK_IRM)
>                 *new_state = CAN_STATE_BUS_OFF;
> -       else if (bec->txerr >= 255 ||  bec->rxerr >= 255)
> +       else if (bec->txerr >= 255 || bec->rxerr >= 255)
>                 *new_state = CAN_STATE_BUS_OFF;
>         else if (p->header[1] & KVASER_PCIEFD_SPACK_EPLR)
>                 *new_state = CAN_STATE_ERROR_PASSIVE;
> @@ -1088,22 +1067,16 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
>         struct net_device *ndev = can->can.dev;
>         struct sk_buff *skb;
>         struct can_frame *cf = NULL;
> -       struct net_device_stats *stats = &ndev->stats;
>
>         old_state = can->can.state;
>
>         bec.txerr = p->header[0] & 0xff;
>         bec.rxerr = (p->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff;
>
> -       kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state,
> -                                     &rx_state);
> -
> +       kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state, &rx_state);
>         skb = alloc_can_err_skb(ndev, &cf);
> -
>         if (new_state != old_state) {
> -               kvaser_pciefd_change_state(can, cf, new_state, tx_state,
> -                                          rx_state);
> -
> +               kvaser_pciefd_change_state(can, cf, new_state, tx_state, rx_state);
>                 if (old_state == CAN_STATE_BUS_OFF &&
>                     new_state == CAN_STATE_ERROR_ACTIVE &&
>                     can->can.restart_ms) {
> @@ -1116,25 +1089,25 @@ static int kvaser_pciefd_rx_error_frame(struct kvaser_pciefd_can *can,
>         can->err_rep_cnt++;
>         can->can.can_stats.bus_error++;
>         if (p->header[1] & KVASER_PCIEFD_EPACK_DIR_TX)
> -               stats->tx_errors++;
> +               ndev->stats.tx_errors++;
>         else
> -               stats->rx_errors++;
> +               ndev->stats.rx_errors++;
>
>         can->bec.txerr = bec.txerr;
>         can->bec.rxerr = bec.rxerr;
>
>         if (!skb) {
> -               stats->rx_dropped++;
> +               ndev->stats.rx_dropped++;
>                 return -ENOMEM;
>         }
>
>         kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
>         cf->can_id |= CAN_ERR_BUSERROR | CAN_ERR_CNT;
> -
>         cf->data[6] = bec.txerr;
>         cf->data[7] = bec.rxerr;
>
>         netif_rx(skb);
> +
>         return 0;
>  }
>
> @@ -1142,19 +1115,19 @@ static int kvaser_pciefd_handle_error_packet(struct kvaser_pciefd *pcie,
>                                              struct kvaser_pciefd_rx_packet *p)
>  {
>         struct kvaser_pciefd_can *can;
> -       u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
> +       u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
>
>         if (ch_id >= pcie->nr_channels)
>                 return -EIO;
>
>         can = pcie->can[ch_id];
> -
>         kvaser_pciefd_rx_error_frame(can, p);
>         if (can->err_rep_cnt >= KVASER_PCIEFD_MAX_ERR_REP)
>                 /* Do not report more errors, until bec_poll_timer expires */
>                 kvaser_pciefd_disable_err_gen(can);
>         /* Start polling the error counters */
>         mod_timer(&can->bec_poll_timer, KVASER_PCIEFD_BEC_POLL_FREQ);
> +
>         return 0;
>  }
>
> @@ -1169,9 +1142,7 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
>         bec.txerr = p->header[0] & 0xff;
>         bec.rxerr = (p->header[0] >> KVASER_PCIEFD_SPACK_RXERR_SHIFT) & 0xff;
>
> -       kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state,
> -                                     &rx_state);
> -
> +       kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state, &rx_state);
>         if (new_state != old_state) {
>                 struct net_device *ndev = can->can.dev;
>                 struct sk_buff *skb;
> @@ -1179,15 +1150,11 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
>
>                 skb = alloc_can_err_skb(ndev, &cf);
>                 if (!skb) {
> -                       struct net_device_stats *stats = &ndev->stats;
> -
> -                       stats->rx_dropped++;
> +                       ndev->stats.rx_dropped++;
>                         return -ENOMEM;
>                 }
>
> -               kvaser_pciefd_change_state(can, cf, new_state, tx_state,
> -                                          rx_state);
> -
> +               kvaser_pciefd_change_state(can, cf, new_state, tx_state, rx_state);
>                 if (old_state == CAN_STATE_BUS_OFF &&
>                     new_state == CAN_STATE_ERROR_ACTIVE &&
>                     can->can.restart_ms) {
> @@ -1217,7 +1184,7 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
>         struct kvaser_pciefd_can *can;
>         u8 cmdseq;
>         u32 status;
> -       u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
> +       u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
>
>         if (ch_id >= pcie->nr_channels)
>                 return -EIO;
> @@ -1231,7 +1198,7 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
>         if (p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
>             p->header[0] & KVASER_PCIEFD_SPACK_RMCD &&
>             p->header[1] & KVASER_PCIEFD_SPACK_AUTO &&
> -           cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MSK) &&
> +           cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK) &&
>             status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
>                 u32 cmd;
>
> @@ -1242,26 +1209,24 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
>                 iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
>         } else if (p->header[0] & KVASER_PCIEFD_SPACK_IDET &&
>                    p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
> -                  cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MSK) &&
> +                  cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK) &&
>                    status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
>                 /* Reset detected, send end of flush if no packet are in FIFO */
> -               u8 count = ioread32(can->reg_base +
> -                                   KVASER_PCIEFD_KCAN_TX_NPACKETS_REG) & 0xff;
> +               u8 count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
>
>                 if (!count)
>                         iowrite32(KVASER_PCIEFD_KCAN_CTRL_EFLUSH,
>                                   can->reg_base + KVASER_PCIEFD_KCAN_CTRL_REG);
>         } else if (!(p->header[1] & KVASER_PCIEFD_SPACK_AUTO) &&
> -                  cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MSK)) {
> +                  cmdseq == (p->header[1] & KVASER_PCIEFD_PACKET_SEQ_MASK)) {
>                 /* Response to status request received */
>                 kvaser_pciefd_handle_status_resp(can, p);
>                 if (can->can.state != CAN_STATE_BUS_OFF &&
>                     can->can.state != CAN_STATE_ERROR_ACTIVE) {
> -                       mod_timer(&can->bec_poll_timer,
> -                                 KVASER_PCIEFD_BEC_POLL_FREQ);
> +                       mod_timer(&can->bec_poll_timer, KVASER_PCIEFD_BEC_POLL_FREQ);
>                 }
>         } else if (p->header[0] & KVASER_PCIEFD_SPACK_RMCD &&
> -                  !(status & KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MSK)) {
> +                  !(status & KVASER_PCIEFD_KCAN_STAT_BUS_OFF_MASK)) {
>                 /* Reset to bus on detected */
>                 if (!completion_done(&can->start_comp))
>                         complete(&can->start_comp);
> @@ -1274,12 +1239,10 @@ static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
>                                              struct kvaser_pciefd_rx_packet *p)
>  {
>         struct sk_buff *skb;
> -       struct net_device_stats *stats = &can->can.dev->stats;
>         struct can_frame *cf;
>
>         skb = alloc_can_err_skb(can->can.dev, &cf);
> -
> -       stats->tx_errors++;
> +       can->can.dev->stats.tx_errors++;
>         if (p->header[0] & KVASER_PCIEFD_APACKET_ABL) {
>                 if (skb)
>                         cf->can_id |= CAN_ERR_LOSTARB;
> @@ -1293,7 +1256,7 @@ static void kvaser_pciefd_handle_nack_packet(struct kvaser_pciefd_can *can,
>                 kvaser_pciefd_set_skb_timestamp(can->kv_pcie, skb, p->timestamp);
>                 netif_rx(skb);
>         } else {
> -               stats->rx_dropped++;
> +               can->can.dev->stats.rx_dropped++;
>                 netdev_warn(can->can.dev, "No memory left for err_skb\n");
>         }
>  }
> @@ -1303,7 +1266,7 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
>  {
>         struct kvaser_pciefd_can *can;
>         bool one_shot_fail = false;
> -       u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
> +       u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
>
>         if (ch_id >= pcie->nr_channels)
>                 return -EIO;
> @@ -1321,27 +1284,24 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
>         if (p->header[0] & KVASER_PCIEFD_APACKET_FLU) {
>                 netdev_dbg(can->can.dev, "Packet was flushed\n");
>         } else {
> -               int echo_idx = p->header[0] & KVASER_PCIEFD_PACKET_SEQ_MSK;
> -               int dlc;
> +               int echo_idx = p->header[0] & KVASER_PCIEFD_PACKET_SEQ_MASK;
> +               int len;
>                 u8 count;
>                 struct sk_buff *skb;
>
>                 skb = can->can.echo_skb[echo_idx];
>                 if (skb)
>                         kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);
> -               dlc = can_get_echo_skb(can->can.dev, echo_idx, NULL);
> -               count = ioread32(can->reg_base +
> -                                   KVASER_PCIEFD_KCAN_TX_NPACKETS_REG) & 0xff;
> +               len = can_get_echo_skb(can->can.dev, echo_idx, NULL);
> +               count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG) & 0xff;
>
>                 if (count < KVASER_PCIEFD_CAN_TX_MAX_COUNT &&
>                     netif_queue_stopped(can->can.dev))
>                         netif_wake_queue(can->can.dev);
>
>                 if (!one_shot_fail) {
> -                       struct net_device_stats *stats = &can->can.dev->stats;
> -
> -                       stats->tx_bytes += dlc;
> -                       stats->tx_packets++;
> +                       can->can.dev->stats.tx_bytes += len;
> +                       can->can.dev->stats.tx_packets++;
>                 }
>         }
>
> @@ -1352,7 +1312,7 @@ static int kvaser_pciefd_handle_eflush_packet(struct kvaser_pciefd *pcie,
>                                               struct kvaser_pciefd_rx_packet *p)
>  {
>         struct kvaser_pciefd_can *can;
> -       u8 ch_id = (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & 0x7;
> +       u8 ch_id = kvaser_pciefd_rx_packet_get_ch_id(p);
>
>         if (ch_id >= pcie->nr_channels)
>                 return -EIO;
> @@ -1391,7 +1351,7 @@ static int kvaser_pciefd_read_packet(struct kvaser_pciefd *pcie, int *start_pos,
>         pos += 2;
>         p->timestamp = le64_to_cpu(timestamp);
>
> -       type = (p->header[1] >> KVASER_PCIEFD_PACKET_TYPE_SHIFT) & 0xf;
> +       type = (p->header[1] >> KVASER_PCIEFD_PACKET_TYPE_SHIFT) & KVASER_PCIEFD_PACKET_TYPE_MASK;
>         switch (type) {
>         case KVASER_PCIEFD_PACK_TYPE_DATA:
>                 ret = kvaser_pciefd_handle_data_packet(pcie, p, &buffer[pos]);
> @@ -1541,13 +1501,12 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
>  static void kvaser_pciefd_teardown_can_ctrls(struct kvaser_pciefd *pcie)
>  {
>         int i;
> -       struct kvaser_pciefd_can *can;
>
>         for (i = 0; i < pcie->nr_channels; i++) {
> -               can = pcie->can[i];
> +               struct kvaser_pciefd_can *can = pcie->can[i];
> +
>                 if (can) {
> -                       iowrite32(0,
> -                                 can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
> +                       iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
>                         kvaser_pciefd_pwm_stop(can);
>                         free_candev(can->can.dev);
>                 }
> @@ -1648,14 +1607,13 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
>
>  static void kvaser_pciefd_remove_all_ctrls(struct kvaser_pciefd *pcie)
>  {
> -       struct kvaser_pciefd_can *can;
>         int i;
>
>         for (i = 0; i < pcie->nr_channels; i++) {
> -               can = pcie->can[i];
> +               struct kvaser_pciefd_can *can = pcie->can[i];
> +
>                 if (can) {
> -                       iowrite32(0,
> -                                 can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
> +                       iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
>                         unregister_candev(can->can.dev);
>                         del_timer(&can->bec_poll_timer);
>                         kvaser_pciefd_pwm_stop(can);
> --
> 2.40.0
>

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

* Re: [PATCH 11/12] can: kvaser_pciefd: Refactor code
  2023-05-23 11:27   ` Vincent MAILHOL
@ 2023-05-24  7:40     ` Jimmy Assarsson
  0 siblings, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-05-24  7:40 UTC (permalink / raw)
  To: Vincent MAILHOL; +Cc: linux-can, Jimmy Assarsson

On 2023-05-23 13:27, Vincent MAILHOL wrote:
> Hi Jimmy,
> 
> I have one single comment for this series.

Hi Vincent,

Thanks for the feedback!

> On Tue. 23 May 2023 at 18:55, Jimmy Assarsson <extja@kvaser.com> wrote:
>> Refactor code;
>>   - Format code
>>   - Replace constants with macros
>>   - Rename variables and macros
>>   - Remove intermediate variable
>>   - Add/remove blank lines
>>   - Add function to fetch channel id from Rx packets
>>   - Reduce scope of variables
>>
>> Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
>> ---

...

>> +static inline u8 kvaser_pciefd_rx_packet_get_ch_id(struct kvaser_pciefd_rx_packet *p)
>> +{
>> +       return (p->header[1] >> KVASER_PCIEFD_PACKET_CHID_SHIFT) & KVASER_PCIEFD_PACKET_CHID_MASK;
> 
> Instead of shifting and appliying the mask, define a mask which is
> already shifted with GEN_MASK.
> 
>    Then use the FIELD_GET and FIELD_PREP from linux/bitfield.h.
> 
> The same comment applies to the other shift and mask operations.
> 
> This GEN_MASK, FIELD_GET and FIELD_PREP can be a separate patch.

Good point, will fix this in v2.

Best regards,
jimmy

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

* Re: [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices
  2023-05-23  9:43 ` [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices Jimmy Assarsson
  2023-05-23  9:51   ` Jimmy Assarsson
@ 2023-06-22 15:16   ` Jimmy Assarsson
  1 sibling, 0 replies; 22+ messages in thread
From: Jimmy Assarsson @ 2023-06-22 15:16 UTC (permalink / raw)
  To: linux-can; +Cc: Jimmy Assarsson

On 5/23/23 11:43, Jimmy Assarsson wrote:
> This patch series adds support for a range of new Kvaser PCI Express
> devices based on the SmartFusion2 SoC, to the kvaser_pciefd driver.
> 
> In the first patch, the hardware specific constants and functions are
> moved into a driver_data struct.
> 
> In the second patch, we add the new devices and their hardware specific
> constants and functions.
> 
> In the last patch, most of the register reading and writing + shifting
> and masking, are wrapped in macros, to simplify the functions.
> 
> 
> Note: This series depends on the changes in xxx
> 
> Jimmy Assarsson (3):
>    can: kvaser_pciefd: Move hardware specific constants and functions
>      into a driver_data struct
>    can: kvaser_pciefd: Add support for new Kvaser pciefd devices
>    can: kvaser_pciefd: Wrap register read and writes with macros
> 
>   drivers/net/can/Kconfig         |   5 +
>   drivers/net/can/kvaser_pciefd.c | 678 ++++++++++++++++++++++----------
>   2 files changed, 479 insertions(+), 204 deletions(-)

Sent v2 of this series
https://lore.kernel.org/linux-can/20230622151153.294844-1-extja@kvaser.com/

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

end of thread, other threads:[~2023-06-22 15:16 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-23  9:43 [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 01/12] can: kvaser_pciefd: Remove useless write to interrupt register Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 02/12] can: kvaser_pciefd: Remove handler for unused KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 03/12] can: kvaser_pciefd: Add function to set skb hwtstamps Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 04/12] can: kvaser_pciefd: Set hardware timestamp on transmitted packets Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 05/12] can: kvaser_pciefd: Define unsigned constants with type suffix 'U' Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 06/12] can: kvaser_pciefd: Remove SPI flash parameter read functionality Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 07/12] can: kvaser_pciefd: Sort includes in alphabetic order Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 08/12] can: kvaser_pciefd: Rename device ID defines Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 09/12] can: kvaser_pciefd: Change return type for kvaser_pciefd_{receive,transmit,set_tx}_irq() Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 10/12] can: kvaser_pciefd: Add len8_dlc support Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 11/12] can: kvaser_pciefd: Refactor code Jimmy Assarsson
2023-05-23 11:27   ` Vincent MAILHOL
2023-05-24  7:40     ` Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 12/12] can: kvaser_pciefd: Use TX FIFO size read from CAN controller Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 0/3] can: kvaser_pciefd: Add support for new Kvaser PCI Express devices Jimmy Assarsson
2023-05-23  9:51   ` Jimmy Assarsson
2023-06-22 15:16   ` Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 1/3] can: kvaser_pciefd: Move hardware specific constants and functions into a driver_data struct Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 2/3] can: kvaser_pciefd: Add support for new Kvaser pciefd devices Jimmy Assarsson
2023-05-23  9:43 ` [PATCH 3/3] can: kvaser_pciefd: Wrap register read and writes with macros Jimmy Assarsson
2023-05-23  9:50 ` [PATCH 00/12] can: kvaser_pciefd: Fixes and improvments Jimmy Assarsson

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