can: kvaser_pciefd: Bug fixes

PATCH Applied Jimmy Assarsson 2 participants 8 messages May 16, 2023 – May 17, 2023

Subsystems (2)

Stable branch CAN network drivers

lore

Share & tools

lore

Jimmy Assarsson

author

2023-05-16 13:43 UTC #0 lore raw

cover can: kvaser_pciefd: Bug fixes

This patch series contains various bug fixes for the kvaser_pciefd driver.

Jimmy Assarsson (6):
can: kvaser_pciefd: Set CAN_STATE_STOPPED in kvaser_pciefd_stop()
can: kvaser_pciefd: Clear listen-only bit if not explicitly requested
can: kvaser_pciefd: Call request_irq() before enabling interrupts
can: kvaser_pciefd: Empty SRB buffer in probe
can: kvaser_pciefd: Do not send EFLUSH command on TFD interrupt
can: kvaser_pciefd: Disable interrupts in probe error path

 drivers/net/can/kvaser_pciefd.c | 51 +++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 22 deletions(-)

Signature · 1 line

2.40.0

Jimmy Assarsson

author

2023-05-16 13:43 UTC #1 lore raw

PATCH 1/6 can: kvaser_pciefd: Set CAN_STATE_STOPPED in kvaser_pciefd_stop()

Set can.state to CAN_STATE_STOPPED in kvaser_pciefd_stop(). Without this fix, wrong CAN state was repported after the interface was brought down.

Fixes 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")

Cc [email protected]

Signed-off-by Jimmy Assarsson <[email protected]>

---
 drivers/net/can/kvaser_pciefd.c | 1 +
 1 file changed, 1 insertion(+)

drivers/net/can/kvaser_pciefd.c +1 -0

index 53e8a914c88b..867b421b9506 100644
@@ -719,6 +719,7 @@ static int kvaser_pciefd_stop(struct net_device *netdev)
 		iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 		del_timer(&can->bec_poll_timer);
 	}
+	can->can.state = CAN_STATE_STOPPED;
 	close_candev(netdev);
 
 	return ret;

Signature · 1 line

2.40.0

Jimmy Assarsson

author

2023-05-16 13:43 UTC #2 lore raw

PATCH 2/6 can: kvaser_pciefd: Clear listen-only bit if not explicitly requested

In reply to Jimmy Assarsson

The listen-only bit was never cleared, causing the controller to always use listen-only mode, if previously set.

Fixes 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")

Cc [email protected]

Signed-off-by Jimmy Assarsson <[email protected]>

---
 drivers/net/can/kvaser_pciefd.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/net/can/kvaser_pciefd.c +2 -0

index 867b421b9506..cdc894d12885 100644
@@ -554,6 +554,8 @@ static void kvaser_pciefd_setup_controller(struct kvaser_pciefd_can *can)
 
 	if (can->can.ctrlmode & CAN_CTRLMODE_LISTENONLY)
 		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;

Signature · 1 line

2.40.0

Jimmy Assarsson

author

2023-05-16 13:43 UTC #3 lore raw

PATCH 3/6 can: kvaser_pciefd: Call request_irq() before enabling interrupts

In reply to Jimmy Assarsson

Make sure the interrupt handler is registered before enabling interrupts.

Fixes 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")

Cc [email protected]

Signed-off-by Jimmy Assarsson <[email protected]>

---
 drivers/net/can/kvaser_pciefd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/net/can/kvaser_pciefd.c +5 -5

index cdc894d12885..4b8591d48735 100644
@@ -1827,6 +1827,11 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 	if (err)
 		goto err_teardown_can_ctrls;
 
+	err = request_irq(pcie->pci->irq, kvaser_pciefd_irq_handler,
+			  IRQF_SHARED, KVASER_PCIEFD_DRV_NAME, pcie);
+	if (err)
+		goto err_teardown_can_ctrls;
+
 	iowrite32(KVASER_PCIEFD_SRB_IRQ_DPD0 | KVASER_PCIEFD_SRB_IRQ_DPD1,
 		  pcie->reg_base + KVASER_PCIEFD_SRB_IRQ_REG);

@@ -1847,11 +1852,6 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 	iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1,
 		  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
 
-	err = request_irq(pcie->pci->irq, kvaser_pciefd_irq_handler,
-			  IRQF_SHARED, KVASER_PCIEFD_DRV_NAME, pcie);
-	if (err)
-		goto err_teardown_can_ctrls;
-
 	err = kvaser_pciefd_reg_candev(pcie);
 	if (err)
 		goto err_free_irq;

Signature · 1 line

2.40.0

Jimmy Assarsson

author

2023-05-16 13:43 UTC #4 lore raw

PATCH 4/6 can: kvaser_pciefd: Empty SRB buffer in probe

In reply to Jimmy Assarsson

Empty the "Shared receive buffer" (SRB) in probe, to assure we start in a known state, and don't process any irrelevant packets.

Fixes 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")

Cc [email protected]

Signed-off-by Jimmy Assarsson <[email protected]>

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

drivers/net/can/kvaser_pciefd.c +15 -0

index 4b8591d48735..0e03c1cd47b3 100644
@@ -71,10 +71,12 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 #define KVASER_PCIEFD_SYSID_BUILD_REG (KVASER_PCIEFD_SYSID_BASE + 0x14)
 /* Shared receive buffer registers */
 #define KVASER_PCIEFD_SRB_BASE 0x1f200
+#define KVASER_PCIEFD_SRB_FIFO_LAST_REG (KVASER_PCIEFD_SRB_BASE + 0x1f4)
 #define KVASER_PCIEFD_SRB_CMD_REG (KVASER_PCIEFD_SRB_BASE + 0x200)
 #define KVASER_PCIEFD_SRB_IEN_REG (KVASER_PCIEFD_SRB_BASE + 0x204)
 #define KVASER_PCIEFD_SRB_IRQ_REG (KVASER_PCIEFD_SRB_BASE + 0x20c)
 #define KVASER_PCIEFD_SRB_STAT_REG (KVASER_PCIEFD_SRB_BASE + 0x210)
+#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG (KVASER_PCIEFD_SRB_BASE + 0x214)
 #define KVASER_PCIEFD_SRB_CTRL_REG (KVASER_PCIEFD_SRB_BASE + 0x218)
 /* EPCS flash controller registers */
 #define KVASER_PCIEFD_SPI_BASE 0x1fc00

@@ -111,6 +113,9 @@ MODULE_DESCRIPTION("CAN driver for Kvaser CAN/PCIe devices");
 /* DMA support */
 #define KVASER_PCIEFD_SRB_STAT_DMA BIT(24)
 
+/* SRB current packet level */
+#define KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK 0xff
+
 /* DMA Enable */
 #define KVASER_PCIEFD_SRB_CTRL_DMA_ENABLE BIT(0)

@@ -1061,6 +1066,7 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 {
 	int i;
 	u32 srb_status;
+	u32 srb_packet_count;
 	dma_addr_t dma_addr[KVASER_PCIEFD_DMA_COUNT];
 
 	/* Disable the DMA */

@@ -1088,6 +1094,15 @@ static int kvaser_pciefd_setup_dma(struct kvaser_pciefd *pcie)
 		  KVASER_PCIEFD_SRB_CMD_RDB1,
 		  pcie->reg_base + KVASER_PCIEFD_SRB_CMD_REG);
 
+	/* Empty Rx FIFO */
+	srb_packet_count = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_RX_NR_PACKETS_REG) &
+			   KVASER_PCIEFD_SRB_RX_NR_PACKETS_MASK;
+	while (srb_packet_count) {
+		/* Drop current packet in FIFO */
+		ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_FIFO_LAST_REG);
+		srb_packet_count--;
+	}
+
 	srb_status = ioread32(pcie->reg_base + KVASER_PCIEFD_SRB_STAT_REG);
 	if (!(srb_status & KVASER_PCIEFD_SRB_STAT_DI)) {
 		dev_err(&pcie->pci->dev, "DMA not idle before enabling\n");

Signature · 1 line

2.40.0

Jimmy Assarsson

author

2023-05-16 13:43 UTC #5 lore raw

PATCH 5/6 can: kvaser_pciefd: Do not send EFLUSH command on TFD interrupt

In reply to Jimmy Assarsson

Under certain circumstances we send two EFLUSH commands, resulting in two EFLUSH ack packets, while only expecting a single EFLUSH ack. This can cause the driver Tx flush completion to get out of sync.

To avoid this problem, don't enable the "Transmit buffer flush done" (TFD) interrupt and remove the code handling it.
Now we only send EFLUSH command after receiving status packet with "Init detected" (IDET) bit set.

Fixes 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")

Cc [email protected]

Signed-off-by Jimmy Assarsson <[email protected]>

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

drivers/net/can/kvaser_pciefd.c +4 ~~-17~~

index 0e03c1cd47b3..4fafb7658349 100644
@@ -531,7 +531,7 @@ static int kvaser_pciefd_set_tx_irq(struct kvaser_pciefd_can *can)
 	      KVASER_PCIEFD_KCAN_IRQ_TOF | KVASER_PCIEFD_KCAN_IRQ_ABD |
 	      KVASER_PCIEFD_KCAN_IRQ_TAE | KVASER_PCIEFD_KCAN_IRQ_TAL |
 	      KVASER_PCIEFD_KCAN_IRQ_FDIC | KVASER_PCIEFD_KCAN_IRQ_BPP |
-	      KVASER_PCIEFD_KCAN_IRQ_TAR | KVASER_PCIEFD_KCAN_IRQ_TFD;
+	      KVASER_PCIEFD_KCAN_IRQ_TAR;
 
 	iowrite32(msk, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);

@@ -579,7 +579,7 @@ static void kvaser_pciefd_start_controller_flush(struct kvaser_pciefd_can *can)
 
 	spin_lock_irqsave(&can->lock, irq);
 	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD | KVASER_PCIEFD_KCAN_IRQ_TFD,
+	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
 		  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 
 	status = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_STAT_REG);

@@ -622,7 +622,7 @@ static int kvaser_pciefd_bus_on(struct kvaser_pciefd_can *can)
 	iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 	iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
 
-	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD | KVASER_PCIEFD_KCAN_IRQ_TFD,
+	iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
 		  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 
 	mode = ioread32(can->reg_base + KVASER_PCIEFD_KCAN_MODE_REG);

@@ -1015,8 +1015,7 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie)
 		SET_NETDEV_DEV(netdev, &pcie->pci->dev);
 
 		iowrite32(-1, can->reg_base + KVASER_PCIEFD_KCAN_IRQ_REG);
-		iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD |
-			  KVASER_PCIEFD_KCAN_IRQ_TFD,
+		iowrite32(KVASER_PCIEFD_KCAN_IRQ_ABD,
 			  can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);
 
 		pcie->can[i] = can;

@@ -1443,9 +1442,6 @@ static int kvaser_pciefd_handle_status_packet(struct kvaser_pciefd *pcie,
 		cmd = KVASER_PCIEFD_KCAN_CMD_AT;
 		cmd |= ++can->cmd_seq << KVASER_PCIEFD_KCAN_CMD_SEQ_SHIFT;
 		iowrite32(cmd, can->reg_base + KVASER_PCIEFD_KCAN_CMD_REG);
-
-		iowrite32(KVASER_PCIEFD_KCAN_IRQ_TFD,
-			  can->reg_base + KVASER_PCIEFD_KCAN_IEN_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) &&

@@ -1732,15 +1728,6 @@ static int kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
 	if (irq & KVASER_PCIEFD_KCAN_IRQ_TOF)
 		netdev_err(can->can.dev, "Tx FIFO overflow\n");
 
-	if (irq & KVASER_PCIEFD_KCAN_IRQ_TFD) {
-		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);
-	}
-
 	if (irq & KVASER_PCIEFD_KCAN_IRQ_BPP)
 		netdev_err(can->can.dev,
 			   "Fail to change bittiming, when not in reset mode\n");

Signature · 1 line

2.40.0

Jimmy Assarsson

author

2023-05-16 13:43 UTC #6 lore raw

PATCH 6/6 can: kvaser_pciefd: Disable interrupts in probe error path

In reply to Jimmy Assarsson

Disable interrupts in error path of probe function.

Fixes 26ad340e582d ("can: kvaser_pciefd: Add driver for Kvaser PCIEcan devices")

Cc [email protected]

Signed-off-by Jimmy Assarsson <[email protected]>

---
 drivers/net/can/kvaser_pciefd.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/net/can/kvaser_pciefd.c +2 -0

index 4fafb7658349..be189edb256c 100644
@@ -1861,6 +1861,8 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
 	return 0;
 
 err_free_irq:
+	/* Disable PCI interrupts */
+	iowrite32(0, pcie->reg_base + KVASER_PCIEFD_IEN_REG);
 	free_irq(pcie->pci->irq, pcie);
 
 err_teardown_can_ctrls:

Signature · 1 line

2.40.0

Marc Kleine-Budde

maintainer Applied

2023-05-17 07:03 UTC #7 lore raw

In reply to Jimmy Assarsson

Quoted 1 line · Jimmy Assarsson

This patch series contains various bug fixes for the kvaser_pciefd driver.

Applied to linux-can.

Thanks,
Marc

Signature · 4 lines

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   |