[PATCH v2 00/14] can: kvaser_pciefd: Fixes and improvments

[PATCH v2 00/14] can: kvaser_pciefd: Fixes and improvments

[Jimmy Assarsson]

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

Jimmy Assarsson (14):
  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: Sort register definitions
  can: kvaser_pciefd: Use FIELD_{GET,PREP} and GENMASK where appropriate
  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 | 955 ++++++++++++--------------------
 2 files changed, 342 insertions(+), 616 deletions(-)

-- 
Changes in v2:
  - Add new patches, to replace opencoded masking+shifting with
    FIELD_{GET,PREP} and GENMASK, suggested by Vincent MAILHOL [1]

[1] https://lore.kernel.org/linux-can/5b10f6cd-c96c-3198-3df4-557e5e91b2e7@kvaser.com/T/#m14c3851e70cfb8c903388e068968a4a379d183e6

2.40.0

[PATCH v2 01/14] can: kvaser_pciefd: Remove useless write to interrupt register

[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

[PATCH v2 02/14] can: kvaser_pciefd: Remove handler for unused KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK

[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

[PATCH v2 03/14] can: kvaser_pciefd: Add function to set skb hwtstamps

[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

[PATCH v2 04/14] can: kvaser_pciefd: Set hardware timestamp on transmitted packets

[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

[PATCH v2 05/14] can: kvaser_pciefd: Define unsigned constants with type suffix 'U'

[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

[PATCH v2 06/14] can: kvaser_pciefd: Remove SPI flash parameter read functionality

[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

[PATCH v2 07/14] can: kvaser_pciefd: Sort includes in alphabetic order

[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

[PATCH v2 08/14] can: kvaser_pciefd: Rename device ID defines

[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

[PATCH v2 09/14] can: kvaser_pciefd: Change return type for kvaser_pciefd_{receive,transmit,set_tx}_irq()

[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

[PATCH v2 10/14] can: kvaser_pciefd: Sort register definitions

[Jimmy Assarsson]

Sort the registers defines, in the same order as the register bits/fields
are defined.
Sort register bits/fields in MSB-to-LSB order.
Update and add comments.

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
  - New in v2

 drivers/net/can/kvaser_pciefd.c | 210 +++++++++++++++++---------------
 1 file changed, 110 insertions(+), 100 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index feef044c6b0a..d2e520f9eaa7 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -30,7 +30,6 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_DMA_COUNT 2U
 
 #define KVASER_PCIEFD_DMA_SIZE (4U * 1024U)
-#define KVASER_PCIEFD_64BIT_DMA_BIT BIT(0)
 
 #define KVASER_PCIEFD_VENDOR 0x1a07
 #define KVASER_PCIEFD_4HS_DEVICE_ID 0x000d
@@ -42,24 +41,8 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* PCIe IRQ registers */
 #define KVASER_PCIEFD_IRQ_REG 0x40
 #define KVASER_PCIEFD_IEN_REG 0x50
-/* DMA map */
+/* DMA address translation map register base */
 #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
-#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_STAT_REG 0x418
-#define KVASER_PCIEFD_KCAN_MODE_REG 0x41c
-#define KVASER_PCIEFD_KCAN_BTRN_REG 0x420
-#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 */
@@ -77,33 +60,54 @@ 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)
+/* 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
+#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_STAT_REG 0x418
+#define KVASER_PCIEFD_KCAN_MODE_REG 0x41c
+#define KVASER_PCIEFD_KCAN_BTRN_REG 0x420
+#define KVASER_PCIEFD_KCAN_BUS_LOAD_REG 0x424
+#define KVASER_PCIEFD_KCAN_BTRD_REG 0x428
+#define KVASER_PCIEFD_KCAN_PWM_REG 0x430
 
-#define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
+/* PCI interrupt fields */
 #define KVASER_PCIEFD_IRQ_SRB BIT(4)
+#define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
+
+/* Enable 64-bit DMA address translation */
+#define KVASER_PCIEFD_64BIT_DMA_BIT BIT(0)
 
+/* System build information fields */
 #define KVASER_PCIEFD_SYSID_NRCHAN_SHIFT 24
 #define KVASER_PCIEFD_SYSID_MAJOR_VER_SHIFT 16
 #define KVASER_PCIEFD_SYSID_BUILD_VER_SHIFT 1
 
 /* Reset DMA buffer 0, 1 and FIFO offset */
-#define KVASER_PCIEFD_SRB_CMD_RDB0 BIT(4)
 #define KVASER_PCIEFD_SRB_CMD_RDB1 BIT(5)
+#define KVASER_PCIEFD_SRB_CMD_RDB0 BIT(4)
 #define KVASER_PCIEFD_SRB_CMD_FOR BIT(0)
 
-/* DMA packet done, buffer 0 and 1 */
-#define KVASER_PCIEFD_SRB_IRQ_DPD0 BIT(8)
-#define KVASER_PCIEFD_SRB_IRQ_DPD1 BIT(9)
-/* DMA overflow, buffer 0 and 1 */
-#define KVASER_PCIEFD_SRB_IRQ_DOF0 BIT(10)
-#define KVASER_PCIEFD_SRB_IRQ_DOF1 BIT(11)
 /* DMA underflow, buffer 0 and 1 */
-#define KVASER_PCIEFD_SRB_IRQ_DUF0 BIT(12)
 #define KVASER_PCIEFD_SRB_IRQ_DUF1 BIT(13)
+#define KVASER_PCIEFD_SRB_IRQ_DUF0 BIT(12)
+/* DMA overflow, buffer 0 and 1 */
+#define KVASER_PCIEFD_SRB_IRQ_DOF1 BIT(11)
+#define KVASER_PCIEFD_SRB_IRQ_DOF0 BIT(10)
+/* DMA packet done, buffer 0 and 1 */
+#define KVASER_PCIEFD_SRB_IRQ_DPD1 BIT(9)
+#define KVASER_PCIEFD_SRB_IRQ_DPD0 BIT(8)
 
+/* Got DMA support */
+#define KVASER_PCIEFD_SRB_STAT_DMA BIT(24)
 /* DMA idle */
 #define KVASER_PCIEFD_SRB_STAT_DI BIT(15)
-/* DMA support */
-#define KVASER_PCIEFD_SRB_STAT_DMA BIT(24)
 
 /* SRB current packet level */
 #define KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK 0xff
@@ -111,80 +115,86 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* DMA Enable */
 #define KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE BIT(0)
 
-/* Kvaser KCAN definitions */
+/* KCAN CTRL packet types */
 #define KVASER_PCIEFD_KCAN_CTRL_EFLUSH (4 << 29)
 #define KVASER_PCIEFD_KCAN_CTRL_EFRAME (5 << 29)
 
+/* Command sequence number */
 #define KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT 16
-/* Request status packet */
-#define KVASER_PCIEFD_KCAN_CMD_SRQ BIT(0)
 /* Abort, flush and reset */
 #define KVASER_PCIEFD_KCAN_CMD_AT BIT(1)
+/* Request status packet */
+#define KVASER_PCIEFD_KCAN_CMD_SRQ BIT(0)
 
-/* Tx FIFO unaligned read */
-#define KVASER_PCIEFD_KCAN_IRQ_TAR BIT(0)
-/* Tx FIFO unaligned end */
-#define KVASER_PCIEFD_KCAN_IRQ_TAE BIT(1)
-/* Bus parameter protection error */
-#define KVASER_PCIEFD_KCAN_IRQ_BPP BIT(2)
-/* FDF bit when controller is in classic mode */
-#define KVASER_PCIEFD_KCAN_IRQ_FDIC BIT(3)
-/* Rx FIFO overflow */
-#define KVASER_PCIEFD_KCAN_IRQ_ROF BIT(5)
-/* Abort done */
-#define KVASER_PCIEFD_KCAN_IRQ_ABD BIT(13)
-/* Tx buffer flush done */
-#define KVASER_PCIEFD_KCAN_IRQ_TFD BIT(14)
-/* Tx FIFO overflow */
-#define KVASER_PCIEFD_KCAN_IRQ_TOF BIT(15)
-/* Tx FIFO empty */
-#define KVASER_PCIEFD_KCAN_IRQ_TE BIT(16)
 /* Transmitter unaligned */
 #define KVASER_PCIEFD_KCAN_IRQ_TAL BIT(17)
+/* Tx FIFO empty */
+#define KVASER_PCIEFD_KCAN_IRQ_TE BIT(16)
+/* Tx FIFO overflow */
+#define KVASER_PCIEFD_KCAN_IRQ_TOF BIT(15)
+/* Tx buffer flush done */
+#define KVASER_PCIEFD_KCAN_IRQ_TFD BIT(14)
+/* Abort done */
+#define KVASER_PCIEFD_KCAN_IRQ_ABD BIT(13)
+/* Rx FIFO overflow */
+#define KVASER_PCIEFD_KCAN_IRQ_ROF BIT(5)
+/* FDF bit when controller is in classic CAN mode */
+#define KVASER_PCIEFD_KCAN_IRQ_FDIC BIT(3)
+/* Bus parameter protection error */
+#define KVASER_PCIEFD_KCAN_IRQ_BPP BIT(2)
+/* Tx FIFO unaligned end */
+#define KVASER_PCIEFD_KCAN_IRQ_TAE BIT(1)
+/* Tx FIFO unaligned read */
+#define KVASER_PCIEFD_KCAN_IRQ_TAR BIT(0)
 
+/* Tx FIFO size */
 #define KVASER_PCIEFD_KCAN_TX_NPACKETS_MAX_SHIFT 16
 
+/* Current status packet sequence number */
 #define KVASER_PCIEFD_KCAN_STAT_SEQNO_SHIFT 24
-/* Abort request */
-#define KVASER_PCIEFD_KCAN_STAT_AR BIT(7)
-/* Idle state. Controller in reset mode and no abort or flush pending */
-#define KVASER_PCIEFD_KCAN_STAT_IDLE BIT(10)
-/* Bus off */
-#define KVASER_PCIEFD_KCAN_STAT_BOFF BIT(11)
-/* Reset mode request */
-#define KVASER_PCIEFD_KCAN_STAT_RMR BIT(14)
-/* Controller in reset mode */
-#define KVASER_PCIEFD_KCAN_STAT_IRM BIT(15)
-/* Controller got one-shot capability */
-#define KVASER_PCIEFD_KCAN_STAT_CAP BIT(16)
 /* Controller got CAN FD capability */
 #define KVASER_PCIEFD_KCAN_STAT_FD BIT(19)
+/* Controller got one-shot capability */
+#define KVASER_PCIEFD_KCAN_STAT_CAP BIT(16)
+/* Controller in reset mode */
+#define KVASER_PCIEFD_KCAN_STAT_IRM BIT(15)
+/* Reset mode request */
+#define KVASER_PCIEFD_KCAN_STAT_RMR BIT(14)
+/* Bus off */
+#define KVASER_PCIEFD_KCAN_STAT_BOFF BIT(11)
+/* Idle state. Controller in reset mode and no abort or flush pending */
+#define KVASER_PCIEFD_KCAN_STAT_IDLE BIT(10)
+/* Abort request */
+#define KVASER_PCIEFD_KCAN_STAT_AR BIT(7)
+/* Controller is bus off */
 #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)
 
-/* Reset mode */
-#define KVASER_PCIEFD_KCAN_MODE_RM BIT(8)
-/* Listen only mode */
-#define KVASER_PCIEFD_KCAN_MODE_LOM BIT(9)
-/* Error packet enable */
-#define KVASER_PCIEFD_KCAN_MODE_EPEN BIT(12)
-/* CAN FD non-ISO */
-#define KVASER_PCIEFD_KCAN_MODE_NIFDEN BIT(15)
-/* Acknowledgment packet type */
-#define KVASER_PCIEFD_KCAN_MODE_APT BIT(20)
-/* Active error flag enable. Clear to force error passive */
-#define KVASER_PCIEFD_KCAN_MODE_EEN BIT(23)
 /* Classic CAN mode */
 #define KVASER_PCIEFD_KCAN_MODE_CCM BIT(31)
+/* Active error flag enable. Clear to force error passive */
+#define KVASER_PCIEFD_KCAN_MODE_EEN BIT(23)
+/* Acknowledgment packet type */
+#define KVASER_PCIEFD_KCAN_MODE_APT BIT(20)
+/* CAN FD non-ISO */
+#define KVASER_PCIEFD_KCAN_MODE_NIFDEN BIT(15)
+/* Error packet enable */
+#define KVASER_PCIEFD_KCAN_MODE_EPEN BIT(12)
+/* Listen only mode */
+#define KVASER_PCIEFD_KCAN_MODE_LOM BIT(9)
+/* Reset mode */
+#define KVASER_PCIEFD_KCAN_MODE_RM BIT(8)
 
-#define KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT 13
-#define KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT 17
+/* BTRN and BTRD fields */
 #define KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT 26
+#define KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT 17
+#define KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT 13
 
+/* PWM Control fields */
 #define KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT 16
 
-/* Kvaser KCAN packet types */
+/* KCAN packet type IDs */
 #define KVASER_PCIEFD_PACK_TYPE_DATA 0
 #define KVASER_PCIEFD_PACK_TYPE_ACK 1
 #define KVASER_PCIEFD_PACK_TYPE_TXRQ 2
@@ -195,41 +205,41 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_PACK_TYPE_STATUS 8
 #define KVASER_PCIEFD_PACK_TYPE_BUS_LOAD 9
 
-/* Kvaser KCAN packet common definitions */
-#define KVASER_PCIEFD_PACKET_SEQ_MSK 0xff
-#define KVASER_PCIEFD_PACKET_CHID_SHIFT 25
+/* Common KCAN packet definitions, second word */
 #define KVASER_PCIEFD_PACKET_TYPE_SHIFT 28
+#define KVASER_PCIEFD_PACKET_CHID_SHIFT 25
+#define KVASER_PCIEFD_PACKET_SEQ_MSK 0xff
 
-/* Kvaser KCAN TDATA and RDATA first word */
+/* KCAN Transmit/Receive data packet, first word */
 #define KVASER_PCIEFD_RPACKET_IDE BIT(30)
 #define KVASER_PCIEFD_RPACKET_RTR BIT(29)
-/* Kvaser KCAN TDATA and RDATA second word */
-#define KVASER_PCIEFD_RPACKET_ESI BIT(13)
-#define KVASER_PCIEFD_RPACKET_BRS BIT(14)
+/* KCAN Transmit data packet, second word */
+#define KVASER_PCIEFD_TPACKET_AREQ BIT(31)
+#define KVASER_PCIEFD_TPACKET_SMS BIT(16)
+/* KCAN Transmit/Receive data packet, second word */
 #define KVASER_PCIEFD_RPACKET_FDF BIT(15)
+#define KVASER_PCIEFD_RPACKET_BRS BIT(14)
+#define KVASER_PCIEFD_RPACKET_ESI BIT(13)
 #define KVASER_PCIEFD_RPACKET_DLC_SHIFT 8
-/* Kvaser KCAN TDATA second word */
-#define KVASER_PCIEFD_TPACKET_SMS BIT(16)
-#define KVASER_PCIEFD_TPACKET_AREQ BIT(31)
 
-/* Kvaser KCAN APACKET */
-#define KVASER_PCIEFD_APACKET_FLU BIT(8)
-#define KVASER_PCIEFD_APACKET_CT BIT(9)
-#define KVASER_PCIEFD_APACKET_ABL BIT(10)
+/* KCAN Transmit acknowledge packet, first word */
 #define KVASER_PCIEFD_APACKET_NACK BIT(11)
+#define KVASER_PCIEFD_APACKET_ABL BIT(10)
+#define KVASER_PCIEFD_APACKET_CT BIT(9)
+#define KVASER_PCIEFD_APACKET_FLU BIT(8)
 
-/* Kvaser KCAN SPACK first word */
-#define KVASER_PCIEFD_SPACK_RXERR_SHIFT 8
-#define KVASER_PCIEFD_SPACK_BOFF BIT(16)
-#define KVASER_PCIEFD_SPACK_IDET BIT(20)
-#define KVASER_PCIEFD_SPACK_IRM BIT(21)
+/* KCAN Status packet, first word */
 #define KVASER_PCIEFD_SPACK_RMCD BIT(22)
-/* Kvaser KCAN SPACK second word */
-#define KVASER_PCIEFD_SPACK_AUTO BIT(21)
-#define KVASER_PCIEFD_SPACK_EWLR BIT(23)
+#define KVASER_PCIEFD_SPACK_IRM BIT(21)
+#define KVASER_PCIEFD_SPACK_IDET BIT(20)
+#define KVASER_PCIEFD_SPACK_BOFF BIT(16)
+#define KVASER_PCIEFD_SPACK_RXERR_SHIFT 8
+/* KCAN Status packet, second word */
 #define KVASER_PCIEFD_SPACK_EPLR BIT(24)
+#define KVASER_PCIEFD_SPACK_EWLR BIT(23)
+#define KVASER_PCIEFD_SPACK_AUTO BIT(21)
 
-/* Kvaser KCAN_EPACK second word */
+/* KCAN Error detected packet, second word */
 #define KVASER_PCIEFD_EPACK_DIR_TX BIT(0)
 
 struct kvaser_pciefd;
-- 
2.40.0

[PATCH v2 11/14] can: kvaser_pciefd: Use FIELD_{GET,PREP} and GENMASK where appropriate

[Jimmy Assarsson]

Replace opencoded masking and shifting, with GENMASK, FIELD_GET and
FIELD_PREP macros.

Suggested-by: Vincent MAILHOL <mailhol.vincent@wanadoo.fr>
Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
  - New in v2, suggested by Vincent MAILHOL [1]

[1] https://lore.kernel.org/linux-can/5b10f6cd-c96c-3198-3df4-557e5e91b2e7@kvaser.com/T/#m14c3851e70cfb8c903388e068968a4a379d183e6

 drivers/net/can/kvaser_pciefd.c | 168 +++++++++++++++++---------------
 1 file changed, 89 insertions(+), 79 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index d2e520f9eaa7..8779091c448c 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -5,6 +5,7 @@
  *  - PEAK linux canfd driver
  */
 
+#include <linux/bitfield.h>
 #include <linux/can/dev.h>
 #include <linux/device.h>
 #include <linux/ethtool.h>
@@ -26,7 +27,7 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_BEC_POLL_FREQ (jiffies + msecs_to_jiffies(200))
 #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_MAX_CAN_CHANNELS 4UL
 #define KVASER_PCIEFD_DMA_COUNT 2U
 
 #define KVASER_PCIEFD_DMA_SIZE (4U * 1024U)
@@ -79,15 +80,16 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 
 /* PCI interrupt fields */
 #define KVASER_PCIEFD_IRQ_SRB BIT(4)
-#define KVASER_PCIEFD_IRQ_ALL_MSK 0x1f
+#define KVASER_PCIEFD_IRQ_ALL_MASK GENMASK(4, 0)
 
 /* Enable 64-bit DMA address translation */
 #define KVASER_PCIEFD_64BIT_DMA_BIT BIT(0)
 
 /* System build information fields */
-#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_MASK GENMASK(31, 24)
+#define KVASER_PCIEFD_SYSID_VERSION_MAJOR_MASK GENMASK(23, 16)
+#define KVASER_PCIEFD_SYSID_VERSION_MINOR_MASK GENMASK(7, 0)
+#define KVASER_PCIEFD_SYSID_BUILD_SEQ_MASK GENMASK(15, 1)
 
 /* Reset DMA buffer 0, 1 and FIFO offset */
 #define KVASER_PCIEFD_SRB_CMD_RDB1 BIT(5)
@@ -110,17 +112,18 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_SRB_STAT_DI BIT(15)
 
 /* SRB current packet level */
-#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK 0xff
+#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK GENMASK(7, 0)
 
 /* DMA Enable */
 #define KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE BIT(0)
 
 /* KCAN CTRL packet types */
-#define KVASER_PCIEFD_KCAN_CTRL_EFLUSH (4 << 29)
-#define KVASER_PCIEFD_KCAN_CTRL_EFRAME (5 << 29)
+#define KVASER_PCIEFD_KCAN_CTRL_TYPE_MASK GENMASK(31, 29)
+#define KVASER_PCIEFD_KCAN_CTRL_TYPE_EFLUSH 0x4
+#define KVASER_PCIEFD_KCAN_CTRL_TYPE_EFRAME 0x5
 
 /* Command sequence number */
-#define KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT 16
+#define KVASER_PCIEFD_KCAN_CMD_SEQ_MASK GENMASK(23, 16)
 /* Abort, flush and reset */
 #define KVASER_PCIEFD_KCAN_CMD_AT BIT(1)
 /* Request status packet */
@@ -148,10 +151,12 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_KCAN_IRQ_TAR BIT(0)
 
 /* Tx FIFO size */
-#define KVASER_PCIEFD_KCAN_TX_NPACKETS_MAX_SHIFT 16
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_MASK GENMASK(23, 16)
+/* Tx FIFO current packet level */
+#define KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK GENMASK(7, 0)
 
 /* Current status packet sequence number */
-#define KVASER_PCIEFD_KCAN_STAT_SEQNO_SHIFT 24
+#define KVASER_PCIEFD_KCAN_STAT_SEQNO_MASK GENMASK(31, 24)
 /* Controller got CAN FD capability */
 #define KVASER_PCIEFD_KCAN_STAT_FD BIT(19)
 /* Controller got one-shot capability */
@@ -187,12 +192,14 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_KCAN_MODE_RM BIT(8)
 
 /* BTRN and BTRD fields */
-#define KVASER_PCIEFD_KCAN_BTRN_TSEG2_SHIFT 26
-#define KVASER_PCIEFD_KCAN_BTRN_TSEG1_SHIFT 17
-#define KVASER_PCIEFD_KCAN_BTRN_SJW_SHIFT 13
+#define KVASER_PCIEFD_KCAN_BTRN_TSEG2_MASK GENMASK(30, 26)
+#define KVASER_PCIEFD_KCAN_BTRN_TSEG1_MASK GENMASK(25, 17)
+#define KVASER_PCIEFD_KCAN_BTRN_SJW_MASK GENMASK(16, 13)
+#define KVASER_PCIEFD_KCAN_BTRN_BRP_MASK GENMASK(12, 0)
 
 /* PWM Control fields */
-#define KVASER_PCIEFD_KCAN_PWM_TOP_SHIFT 16
+#define KVASER_PCIEFD_KCAN_PWM_TOP_MASK GENMASK(23, 16)
+#define KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK GENMASK(7, 0)
 
 /* KCAN packet type IDs */
 #define KVASER_PCIEFD_PACK_TYPE_DATA 0
@@ -206,13 +213,14 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_PACK_TYPE_BUS_LOAD 9
 
 /* Common KCAN packet definitions, second word */
-#define KVASER_PCIEFD_PACKET_TYPE_SHIFT 28
-#define KVASER_PCIEFD_PACKET_CHID_SHIFT 25
-#define KVASER_PCIEFD_PACKET_SEQ_MSK 0xff
+#define KVASER_PCIEFD_PACKET_TYPE_MASK GENMASK(31, 28)
+#define KVASER_PCIEFD_PACKET_CHID_MASK GENMASK(27, 25)
+#define KVASER_PCIEFD_PACKET_SEQ_MASK GENMASK(7, 0)
 
 /* KCAN Transmit/Receive data packet, first word */
 #define KVASER_PCIEFD_RPACKET_IDE BIT(30)
 #define KVASER_PCIEFD_RPACKET_RTR BIT(29)
+#define KVASER_PCIEFD_RPACKET_ID_MASK GENMASK(28, 0)
 /* KCAN Transmit data packet, second word */
 #define KVASER_PCIEFD_TPACKET_AREQ BIT(31)
 #define KVASER_PCIEFD_TPACKET_SMS BIT(16)
@@ -220,7 +228,7 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_RPACKET_FDF BIT(15)
 #define KVASER_PCIEFD_RPACKET_BRS BIT(14)
 #define KVASER_PCIEFD_RPACKET_ESI BIT(13)
-#define KVASER_PCIEFD_RPACKET_DLC_SHIFT 8
+#define KVASER_PCIEFD_RPACKET_DLC_MASK GENMASK(11, 8)
 
 /* KCAN Transmit acknowledge packet, first word */
 #define KVASER_PCIEFD_APACKET_NACK BIT(11)
@@ -233,7 +241,8 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_SPACK_IRM BIT(21)
 #define KVASER_PCIEFD_SPACK_IDET BIT(20)
 #define KVASER_PCIEFD_SPACK_BOFF BIT(16)
-#define KVASER_PCIEFD_SPACK_RXERR_SHIFT 8
+#define KVASER_PCIEFD_SPACK_RXERR_MASK GENMASK(15, 8)
+#define KVASER_PCIEFD_SPACK_TXERR_MASK GENMASK(7, 0)
 /* KCAN Status packet, second word */
 #define KVASER_PCIEFD_SPACK_EPLR BIT(24)
 #define KVASER_PCIEFD_SPACK_EWLR BIT(23)
@@ -318,7 +327,7 @@ static void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
 	u32 cmd;
 
 	cmd = KVASER_PCIEFD_KCAN_CMD_SRQ;
-	cmd |= ++can->cmd_seq << KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT;
+	cmd |= FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq);
 	iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
 }
 
@@ -418,7 +427,7 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
 
 		/* 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;
+		cmd |= FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq);
 		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
 	} else if (!(status & KVASER_PCIEFD_KCAN_STAT_RMR)) {
 		u32 mode;
@@ -489,10 +498,10 @@ 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;
+	top = FIELD_GET(KVASER_PCIEFD_KCAN_PWM_TOP_MASK, pwm_ctrl);
 
 	/* Set duty cycle to zero */
-	pwm_ctrl |= top;
+	pwm_ctrl |= FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK, top);
 	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
 	spin_unlock_irqrestore(&can->lock, irq);
 }
@@ -509,14 +518,14 @@ static void kvaser_pciefd_pwm_start(struct kvaser_pciefd_can *can)
 	/* 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;
+	pwm_ctrl = FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK, top);
+	pwm_ctrl |= FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TOP_MASK, top);
 	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
 
 	/* 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;
+	pwm_ctrl = FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK, trigger);
+	pwm_ctrl |= FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TOP_MASK, top);
 	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
 	spin_unlock_irqrestore(&can->lock, irq);
 }
@@ -581,8 +590,8 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
 	if (cf->can_id & CAN_EFF_FLAG)
 		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[0] |= FIELD_PREP(KVASER_PCIEFD_RPACKET_ID_MASK, cf->can_id);
+	p->header[1] |= FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK, can_fd_len2dlc(cf->len));
 	p->header[1] |= KVASER_PCIEFD_TPACKET_AREQ;
 
 	if (can_is_canfd_skb(skb)) {
@@ -593,7 +602,7 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
 			p->header[1] |= KVASER_PCIEFD_RPACKET_ESI;
 	}
 
-	p->header[1] |= seq & KVASER_PCIEFD_PACKET_SEQ_MSK;
+	p->header[1] |= FIELD_PREP(KVASER_PCIEFD_PACKET_SEQ_MASK, seq);
 
 	packet_size = cf->len;
 	memcpy(p->data, cf->data, packet_size);
@@ -645,7 +654,8 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 			     KVASER_PCIEFD_KCAN_FIFO_LAST_REG);
 	}
 
-	count = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG);
+	count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
+			  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
 	/* No room for a new message, stop the queue until at least one
 	 * successful transmit
 	 */
@@ -670,12 +680,10 @@ 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 = FIELD_PREP(KVASER_PCIEFD_KCAN_BTRN_TSEG2_MASK, bt->phase_seg2 - 1) |
+	       FIELD_PREP(KVASER_PCIEFD_KCAN_BTRN_TSEG1_MASK, bt->prop_seg + bt->phase_seg1 - 1) |
+	       FIELD_PREP(KVASER_PCIEFD_KCAN_BTRN_SJW_MASK, bt->sjw - 1) |
+	       FIELD_PREP(KVASER_PCIEFD_KCAN_BTRN_BRP_MASK, bt->brp - 1);
 
 	spin_lock_irqsave(&can->lock, irq_flags);
 	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);
@@ -771,7 +779,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_max;
 
 		netdev = alloc_candev(sizeof(struct kvaser_pciefd_can),
 				      KVASER_PCIEFD_CAN_TX_MAX_COUNT);
@@ -798,10 +806,10 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		/* 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) &
-		      0xff) < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
+		tx_nr_packets_max =
+			FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_MASK,
+				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
+		if (tx_nr_packets_max < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
 			dev_err(&pcie->pci->dev,
 				"Max Tx count is smaller than expected\n");
 
@@ -924,8 +932,9 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 		  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;
+	srb_packet_count =
+		FIELD_GET(KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK,
+			  ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG));
 	while (srb_packet_count) {
 		/* Drop current packet in FIFO */
 		ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_FIFO_LAST_REG);
@@ -947,17 +956,17 @@ 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));
+				FIELD_GET(KVASER_PCIEFD_SYSID_VERSION_NR_CHAN_MASK, version));
 
 	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);
+	dev_dbg(&pcie->pci->dev, "Version %lu.%lu.%lu\n",
+		FIELD_GET(KVASER_PCIEFD_SYSID_VERSION_MAJOR_MASK, version),
+		FIELD_GET(KVASER_PCIEFD_SYSID_VERSION_MINOR_MASK, version),
+		FIELD_GET(KVASER_PCIEFD_SYSID_BUILD_SEQ_MASK, build));
 
 	srb_status = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_STAT_REG);
 	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DMA)) {
@@ -986,7 +995,7 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 	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 = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1014,11 +1023,11 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 		}
 	}
 
-	cf->can_id = p->header[0] & CAN_EFF_MASK;
+	cf->can_id = FIELD_GET(KVASER_PCIEFD_RPACKET_ID_MASK, p->header[0]);
 	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);
+	cf->len = can_fd_dlc2len(FIELD_GET(KVASER_PCIEFD_RPACKET_DLC_MASK, p->header[1]));
 
 	if (p->header[0] & KVASER_PCIEFD_RPACKET_RTR) {
 		cf->can_id |= CAN_RTR_FLAG;
@@ -1095,8 +1104,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 = FIELD_GET(KVASER_PCIEFD_SPACK_TXERR_MASK, p->header[0]);
+	bec.rxerr = FIELD_GET(KVASER_PCIEFD_SPACK_RXERR_MASK, p->header[0]);
 
 	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state,
 				      &rx_state);
@@ -1145,7 +1154,7 @@ 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 = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1169,8 +1178,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 = FIELD_GET(KVASER_PCIEFD_SPACK_TXERR_MASK, p->header[0]);
+	bec.rxerr = FIELD_GET(KVASER_PCIEFD_SPACK_RXERR_MASK, p->header[0]);
 
 	kvaser_pciefd_packet_to_state(p, &bec, &new_state, &tx_state,
 				      &rx_state);
@@ -1220,7 +1229,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 = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1228,34 +1237,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;
+	cmdseq = FIELD_GET(KVASER_PCIEFD_KCAN_STAT_SEQNO_MASK, status);
 
 	/* 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_MSK) &&
+	    cmdseq == FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[1]) &&
 	    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;
+		cmd |= FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq);
 		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 == FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[1]) &&
 		   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 = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
+				     ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
 
 		if (!count)
-			iowrite32(KVASER_PCIEFD_KCAN_CTRL_EFLUSH,
+			iowrite32(FIELD_PREP(KVASER_PCIEFD_KCAN_CTRL_TYPE_MASK,
+					     KVASER_PCIEFD_KCAN_CTRL_TYPE_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 == FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[1])) {
 		/* Response to status request received */
 		kvaser_pciefd_handle_status_resp(can, p);
 		if (can->can.state != CAN_STATE_BUS_OFF &&
@@ -1306,7 +1316,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 = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1324,7 +1334,7 @@ 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 echo_idx = FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[0]);
 		int dlc;
 		u8 count;
 		struct sk_buff *skb;
@@ -1333,8 +1343,8 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 		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;
+		count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
+				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
 
 		if (count < KVASER_PCIEFD_CAN_TX_MAX_COUNT &&
 		    netif_queue_stopped(can->can.dev))
@@ -1355,7 +1365,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 = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
@@ -1394,15 +1404,15 @@ 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 = FIELD_GET(KVASER_PCIEFD_PACKET_TYPE_MASK, p->header[1]);
 	switch (type) {
 	case KVASER_PCIEFD_PACK_TYPE_DATA:
 		ret = kvaser_pciefd_handle_data_packet(pcie, p, &buffer[pos]);
 		if (!(p->header[0] & KVASER_PCIEFD_RPACKET_RTR)) {
 			u8 data_len;
 
-			data_len = can_fd_dlc2len(p->header[1] >>
-					       KVASER_PCIEFD_RPACKET_DLC_SHIFT);
+			data_len = can_fd_dlc2len(FIELD_GET(KVASER_PCIEFD_RPACKET_DLC_MASK,
+							    p->header[1]));
 			pos += DIV_ROUND_UP(data_len, 4);
 		}
 		break;
@@ -1520,7 +1530,7 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev)
 
 	board_irq = ioread32(pcie->reg_base + KVASER_PCIEFD_IRQ_REG);
 
-	if (!(board_irq & KVASER_PCIEFD_IRQ_ALL_MSK))
+	if (!(board_irq & KVASER_PCIEFD_IRQ_ALL_MASK))
 		return IRQ_NONE;
 
 	if (board_irq & KVASER_PCIEFD_IRQ_SRB)
@@ -1612,7 +1622,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 		  pcie->reg_base + KVASER_PCIEFD_SRB_IEN_REG);
 
 	/* Enable PCI interrupts */
-	iowrite32(KVASER_PCIEFD_IRQ_ALL_MSK,
+	iowrite32(KVASER_PCIEFD_IRQ_ALL_MASK,
 		  pcie->reg_base + KVASER_PCIEFD_IEN_REG);
 
 	/* Ready the DMA buffers */
-- 
2.40.0

[PATCH v2 12/14] can: kvaser_pciefd: Add len8_dlc support

[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>
---
Changes in v2:
  - Use FIELD_{GET,PREP} instead of shifting

 drivers/net/can/kvaser_pciefd.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 8779091c448c..e3d730264462 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -591,15 +591,20 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,
 		p->header[0] |= KVASER_PCIEFD_RPACKET_IDE;
 
 	p->header[0] |= FIELD_PREP(KVASER_PCIEFD_RPACKET_ID_MASK, cf->can_id);
-	p->header[1] |= FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK, can_fd_len2dlc(cf->len));
 	p->header[1] |= KVASER_PCIEFD_TPACKET_AREQ;
 
 	if (can_is_canfd_skb(skb)) {
+		p->header[1] |= FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK,
+					   can_fd_len2dlc(cf->len));
 		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] |=
+			FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK,
+				   can_get_cc_dlc((struct can_frame *)cf, can->can.ctrlmode));
 	}
 
 	p->header[1] |= FIELD_PREP(KVASER_PCIEFD_PACKET_SEQ_MASK, seq);
@@ -834,7 +839,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)) {
@@ -996,12 +1002,14 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 	struct can_priv *priv;
 	struct net_device_stats *stats;
 	u8 ch_id = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
+	u8 dlc;
 
 	if (ch_id >= pcie->nr_channels)
 		return -EIO;
 
 	priv = &pcie->can[ch_id]->can;
 	stats = &priv->dev->stats;
+	dlc = FIELD_GET(KVASER_PCIEFD_RPACKET_DLC_MASK, p->header[1]);
 
 	if (p->header[1] & KVASER_PCIEFD_RPACKET_FDF) {
 		skb = alloc_canfd_skb(priv->dev, &cf);
@@ -1010,6 +1018,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;
 
@@ -1021,14 +1030,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 = FIELD_GET(KVASER_PCIEFD_RPACKET_ID_MASK, p->header[0]);
 	if (p->header[0] & KVASER_PCIEFD_RPACKET_IDE)
 		cf->can_id |= CAN_EFF_FLAG;
 
-	cf->len = can_fd_dlc2len(FIELD_GET(KVASER_PCIEFD_RPACKET_DLC_MASK, p->header[1]));
-
 	if (p->header[0] & KVASER_PCIEFD_RPACKET_RTR) {
 		cf->can_id |= CAN_RTR_FLAG;
 	} else {
-- 
2.40.0

[PATCH v2 13/14] can: kvaser_pciefd: Refactor code

[Jimmy Assarsson]

Refactor code;
 - Format code
 - Rename variables and macros
 - Remove intermediate variables
 - Add/remove blank lines
 - Reduce scope of variables
 - Add helper functions

Signed-off-by: Jimmy Assarsson <extja@kvaser.com>
---
Changes in v2:
  - Moved parts of this patch into
    can: kvaser_pciefd: Use FIELD_{GET,PREP} and GENMASK where appropriate
  - Add new helper functions, kvaser_pciefd_send_kcan_cmd() and kvaser_pciefd_abort_flush_reset()

 drivers/net/can/kvaser_pciefd.c | 221 +++++++++++++-------------------
 1 file changed, 87 insertions(+), 134 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index e3d730264462..0321b70a3b71 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -70,7 +70,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
@@ -124,6 +124,8 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 
 /* Command sequence number */
 #define KVASER_PCIEFD_KCAN_CMD_SEQ_MASK GENMASK(23, 16)
+/* Command bits */
+#define KVASER_PCIEFD_KCAN_CMD_MASK GENMASK(5, 0)
 /* Abort, flush and reset */
 #define KVASER_PCIEFD_KCAN_CMD_AT BIT(1)
 /* Request status packet */
@@ -172,9 +174,9 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* Abort request */
 #define KVASER_PCIEFD_KCAN_STAT_AR BIT(7)
 /* Controller is bus off */
-#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)
 
 /* Classic CAN mode */
 #define KVASER_PCIEFD_KCAN_MODE_CCM BIT(31)
@@ -202,15 +204,15 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK GENMASK(7, 0)
 
 /* KCAN packet type IDs */
-#define KVASER_PCIEFD_PACK_TYPE_DATA 0
-#define KVASER_PCIEFD_PACK_TYPE_ACK 1
-#define KVASER_PCIEFD_PACK_TYPE_TXRQ 2
-#define KVASER_PCIEFD_PACK_TYPE_ERROR 3
-#define KVASER_PCIEFD_PACK_TYPE_EFLUSH_ACK 4
-#define KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK 5
-#define KVASER_PCIEFD_PACK_TYPE_ACK_DATA 6
-#define KVASER_PCIEFD_PACK_TYPE_STATUS 8
-#define KVASER_PCIEFD_PACK_TYPE_BUS_LOAD 9
+#define KVASER_PCIEFD_PACK_TYPE_DATA 0x0
+#define KVASER_PCIEFD_PACK_TYPE_ACK 0x1
+#define KVASER_PCIEFD_PACK_TYPE_TXRQ 0x2
+#define KVASER_PCIEFD_PACK_TYPE_ERROR 0x3
+#define KVASER_PCIEFD_PACK_TYPE_EFLUSH_ACK 0x4
+#define KVASER_PCIEFD_PACK_TYPE_EFRAME_ACK 0x5
+#define KVASER_PCIEFD_PACK_TYPE_ACK_DATA 0x6
+#define KVASER_PCIEFD_PACK_TYPE_STATUS 0x8
+#define KVASER_PCIEFD_PACK_TYPE_BUS_LOAD 0x9
 
 /* Common KCAN packet definitions, second word */
 #define KVASER_PCIEFD_PACKET_TYPE_MASK GENMASK(31, 28)
@@ -322,13 +324,21 @@ static struct pci_device_id kvaser_pciefd_id_table[] = {
 };
 MODULE_DEVICE_TABLE(pci, kvaser_pciefd_id_table);
 
-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;
+	iowrite32(FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_MASK, cmd) |
+		  FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq),
+		  can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
+}
+
+static inline void kvaser_pciefd_request_status(struct kvaser_pciefd_can *can)
+{
+	kvaser_pciefd_send_kcan_cmd(can, KVASER_PCIEFD_KCAN_CMD_SRQ);
+}
 
-	cmd = KVASER_PCIEFD_KCAN_CMD_SRQ;
-	cmd |= FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq);
-	iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
+static inline void kvaser_pciefd_abort_flush_reset(struct kvaser_pciefd_can *can)
+{
+	kvaser_pciefd_send_kcan_cmd(can, KVASER_PCIEFD_KCAN_CMD_AT);
 }
 
 static void kvaser_pciefd_enable_err_gen(struct kvaser_pciefd_can *can)
@@ -383,7 +393,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;
@@ -400,7 +409,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 */
@@ -417,18 +425,13 @@ 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(GENMASK(31, 0), 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;
-
 		/* If controller is already idle, run abort, flush and reset */
-		cmd = KVASER_PCIEFD_KCAN_CMD_AT;
-		cmd |= FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq);
-		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
+		kvaser_pciefd_abort_flush_reset(can);
 	} else if (!(status & KVASER_PCIEFD_KCAN_STAT_RMR)) {
 		u32 mode;
 
@@ -437,7 +440,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);
 }
 
@@ -447,7 +449,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);
 
@@ -459,11 +460,9 @@ 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(GENMASK(31, 0), 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);
@@ -476,11 +475,10 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 	}
 	/* Reset interrupt handling */
 	iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
-	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
+	iowrite32(GENMASK(31, 0), can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
 
 	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;
@@ -499,7 +497,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 = FIELD_GET(KVASER_PCIEFD_KCAN_PWM_TOP_MASK, pwm_ctrl);
-
 	/* Set duty cycle to zero */
 	pwm_ctrl |= FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK, top);
 	iowrite32(pwm_ctrl, can->reg_base + KVASER_PCIEFD_KCAN_PWM_REG);
@@ -514,8 +511,7 @@ 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*/
+	/* Set frequency to 500 KHz */
 	top = can->kv_pcie->bus_freq / (2 * 500000) - 1;
 
 	pwm_ctrl = FIELD_PREP(KVASER_PCIEFD_KCAN_PWM_TRIGGER_MASK, top);
@@ -580,7 +576,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;
 
@@ -621,16 +616,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);
 
@@ -643,13 +637,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);
@@ -660,14 +654,13 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb,
 	}
 
 	count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
-			  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
+			  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;
@@ -692,16 +685,13 @@ static int kvaser_pciefd_set_bittiming(struct kvaser_pciefd_can *can, bool data)
 
 	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;
@@ -711,11 +701,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;
 }
 
@@ -753,6 +742,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;
 }
 
@@ -796,7 +786,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;
@@ -805,15 +794,14 @@ 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_nr_packets_max =
 			FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_MASK,
-				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
+				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
 		if (tx_nr_packets_max < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
 			dev_err(&pcie->pci->dev,
 				"Max Tx count is smaller than expected\n");
@@ -827,16 +815,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 |
@@ -855,10 +840,9 @@ 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);
+		iowrite32(GENMASK(31, 0), can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
 		iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
 			  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 
@@ -917,18 +901,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);
 	}
 
@@ -936,7 +918,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 =
 		FIELD_GET(KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK,
@@ -976,8 +957,7 @@ static int kvaser_pciefd_setup_board(struct kvaser_pciefd *pcie)
 
 	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;
 	}
 
@@ -987,9 +967,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;
 }
 
@@ -1000,7 +980,6 @@ 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 = FIELD_GET(KVASER_PCIEFD_PACKET_CHID_MASK, p->header[1]);
 	u8 dlc;
 
@@ -1008,26 +987,24 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie,
 		return -EIO;
 
 	priv = &pcie->can[ch_id]->can;
-	stats = &priv->dev->stats;
 	dlc = FIELD_GET(KVASER_PCIEFD_RPACKET_DLC_MASK, p->header[1]);
 
 	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);
@@ -1041,10 +1018,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);
@@ -1065,7 +1041,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);
@@ -1083,7 +1058,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;
@@ -1108,22 +1083,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 = FIELD_GET(KVASER_PCIEFD_SPACK_TXERR_MASK, p->header[0]);
 	bec.rxerr = FIELD_GET(KVASER_PCIEFD_SPACK_RXERR_MASK, p->header[0]);
 
-	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) {
@@ -1136,25 +1105,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;
 }
 
@@ -1168,13 +1137,13 @@ static int kvaser_pciefd_handle_error_packet(struct kvaser_pciefd *pcie,
 		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;
 }
 
@@ -1189,9 +1158,7 @@ static int kvaser_pciefd_handle_status_resp(struct kvaser_pciefd_can *can,
 	bec.txerr = FIELD_GET(KVASER_PCIEFD_SPACK_TXERR_MASK, p->header[0]);
 	bec.rxerr = FIELD_GET(KVASER_PCIEFD_SPACK_RXERR_MASK, p->header[0]);
 
-	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;
@@ -1199,15 +1166,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) {
@@ -1253,21 +1216,18 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 	    p->header[1] & KVASER_PCIEFD_SPACK_AUTO &&
 	    cmdseq == FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[1]) &&
 	    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 |= FIELD_PREP(KVASER_PCIEFD_KCAN_CMD_SEQ_MASK, ++can->cmd_seq);
-		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
+		kvaser_pciefd_abort_flush_reset(can);
 	} else if (p->header[0] & KVASER_PCIEFD_SPACK_IDET &&
 		   p->header[0] & KVASER_PCIEFD_SPACK_IRM &&
 		   cmdseq == FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[1]) &&
 		   status & KVASER_PCIEFD_KCAN_STAT_IDLE) {
 		/* Reset detected, send end of flush if no packet are in FIFO */
-		u8 count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
-				     ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
+		u8 count;
 
+		count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
+				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
 		if (!count)
 			iowrite32(FIELD_PREP(KVASER_PCIEFD_KCAN_CTRL_TYPE_MASK,
 					     KVASER_PCIEFD_KCAN_CTRL_TYPE_EFLUSH),
@@ -1278,11 +1238,10 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 		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);
@@ -1295,12 +1254,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;
@@ -1314,7 +1271,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");
 	}
 }
@@ -1343,26 +1300,24 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 		netdev_dbg(can->can.dev, "Packet was flushed\n");
 	} else {
 		int echo_idx = FIELD_GET(KVASER_PCIEFD_PACKET_SEQ_MASK, p->header[0]);
-		int dlc;
+		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);
+		len = can_get_echo_skb(can->can.dev, echo_idx, NULL);
 		count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
-				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NPACKETS_REG));
+				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
 
 		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++;
 		}
 	}
 
@@ -1562,13 +1517,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);
 		}
@@ -1669,14 +1623,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

[PATCH v2 14/14] can: kvaser_pciefd: Use TX FIFO size read from CAN controller

[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>
---
Changes in v2:
 - Rebased on
  can: kvaser_pciefd: Use FIELD_{GET,PREP} and GENMASK where appropriate

 drivers/net/can/kvaser_pciefd.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/kvaser_pciefd.c b/drivers/net/can/kvaser_pciefd.c
index 0321b70a3b71..db6256f2b1b3 100644
--- a/drivers/net/can/kvaser_pciefd.c
+++ b/drivers/net/can/kvaser_pciefd.c
@@ -658,8 +658,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);
 
@@ -802,16 +801,9 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		tx_nr_packets_max =
 			FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_MAX_MASK,
 				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
-		if (tx_nr_packets_max < KVASER_PCIEFD_CAN_TX_MAX_COUNT) {
-			dev_err(&pcie->pci->dev,
-				"Max Tx count is smaller than expected\n");
-
-			free_candev(netdev);
-			return -ENODEV;
-		}
 
 		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);
@@ -1311,8 +1303,7 @@ static int kvaser_pciefd_handle_ack_packet(struct kvaser_pciefd *pcie,
 		count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,
 				  ioread32(can->reg_base + KVASER_PCIEFD_KCAN_TX_NR_PACKETS_REG));
 
-		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

Re: [PATCH v2 00/14] can: kvaser_pciefd: Fixes and improvments

[Vincent MAILHOL]

On Mon. 29 May 2023 at 22:42, Jimmy Assarsson <extja@kvaser.com> wrote:
> This patch series contains various non critical fixes and improvments for
> the kvaser_pciefd driver.
>
> Jimmy Assarsson (14):
>   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: Sort register definitions
>   can: kvaser_pciefd: Use FIELD_{GET,PREP} and GENMASK where appropriate
>   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 | 955 ++++++++++++--------------------
>  2 files changed, 342 insertions(+), 616 deletions(-)
>
> --
> Changes in v2:
>   - Add new patches, to replace opencoded masking+shifting with
>     FIELD_{GET,PREP} and GENMASK, suggested by Vincent MAILHOL [1]

Thanks for addressing my comments.

Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr>

> [1] https://lore.kernel.org/linux-can/5b10f6cd-c96c-3198-3df4-557e5e91b2e7@kvaser.com/T/#m14c3851e70cfb8c903388e068968a4a379d183e6
>
> 2.40.0
>

Re: [PATCH v2 00/14] can: kvaser_pciefd: Fixes and improvments

[Marc Kleine-Budde]

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

On 29.05.2023 15:42:34, Jimmy Assarsson wrote:
> This patch series contains various non critical fixes and improvments for
> the kvaser_pciefd driver.

Applied to linux-can-next, fixing the typo while applying:
s/improvments/improvements/.

Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]