Commit be19a922 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde
Browse files

Merge patch series "can: kvaser_pciefd: Fix ISR race conditions" 

Axel Forsman <axfo@kvaser.com> says:

This patch series fixes a couple of race conditions in the
kvaser_pciefd driver surfaced by enabling MSI interrupts and the new
Kvaser PCIe 8xCAN.

Changes since version 2:
* Rebase onto linux-can/main to resolve del_timer()/timer_delete()
  merge conflict.
* Reword 2nd commit message slightly.

Changes since version 1:
* Change type of srb_cmd_reg from "__le32 __iomem *" to
  "void __iomem *".
* Maintain TX FIFO count in driver instead of querying HW.
* Stop queue at end of .start_xmit() if full.

Link: https://patch.msgid.link/20250520114332.8961-1-axfo@kvaser.com


Signed-off-by: default avatarMarc Kleine-Budde <mkl@pengutronix.de>
parents 9e89db3d 6d820b81

drivers/net/can/kvaser_pciefd.c

+102 −80
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 
#include <linux/netdevice.h>

#include <linux/pci.h>

#include <linux/timer.h>

#include <net/netdev_queues.h>



MODULE_LICENSE("Dual BSD/GPL");

MODULE_AUTHOR("Kvaser AB <support@kvaser.com>");
@@ -410,10 +411,13 @@ struct kvaser_pciefd_can { 
	void __iomem *reg_base;

	struct can_berr_counter bec;

	u8 cmd_seq;

	u8 tx_max_count;

	u8 tx_idx;

	u8 ack_idx;

	int err_rep_cnt;

	int echo_idx;

	unsigned int completed_tx_pkts;

	unsigned int completed_tx_bytes;

	spinlock_t lock; /* Locks sensitive registers (e.g. MODE) */

	spinlock_t echo_lock; /* Locks the message echo buffer */

	struct timer_list bec_poll_timer;

	struct completion start_comp, flush_comp;

};
@@ -714,6 +718,9 @@ static int kvaser_pciefd_open(struct net_device *netdev) 
	int ret;

	struct kvaser_pciefd_can *can = netdev_priv(netdev);



	can->tx_idx = 0;

	can->ack_idx = 0;



	ret = open_candev(netdev);

	if (ret)

		return ret;
@@ -745,21 +752,26 @@ static int kvaser_pciefd_stop(struct net_device *netdev) 
		timer_delete(&can->bec_poll_timer);

	}

	can->can.state = CAN_STATE_STOPPED;

	netdev_reset_queue(netdev);

	close_candev(netdev);



	return ret;

}



static unsigned int kvaser_pciefd_tx_avail(const struct kvaser_pciefd_can *can)

{

	return can->tx_max_count - (READ_ONCE(can->tx_idx) - READ_ONCE(can->ack_idx));

}



static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p,

					   struct kvaser_pciefd_can *can,

					   struct can_priv *can, u8 seq,

					   struct sk_buff *skb)

{

	struct canfd_frame *cf = (struct canfd_frame *)skb->data;

	int packet_size;

	int seq = can->echo_idx;



	memset(p, 0, sizeof(*p));

	if (can->can.ctrlmode & CAN_CTRLMODE_ONE_SHOT)

	if (can->ctrlmode & CAN_CTRLMODE_ONE_SHOT)

		p->header[1] |= KVASER_PCIEFD_TPACKET_SMS;



	if (cf->can_id & CAN_RTR_FLAG)
@@ -782,7 +794,7 @@ static int kvaser_pciefd_prepare_tx_packet(struct kvaser_pciefd_tx_packet *p, 
	} else {

		p->header[1] |=

			FIELD_PREP(KVASER_PCIEFD_RPACKET_DLC_MASK,

				   can_get_cc_dlc((struct can_frame *)cf, can->can.ctrlmode));

				   can_get_cc_dlc((struct can_frame *)cf, can->ctrlmode));

	}



	p->header[1] |= FIELD_PREP(KVASER_PCIEFD_PACKET_SEQ_MASK, seq);
@@ -797,22 +809,24 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb, 
					    struct net_device *netdev)

{

	struct kvaser_pciefd_can *can = netdev_priv(netdev);

	unsigned long irq_flags;

	struct kvaser_pciefd_tx_packet packet;

	unsigned int seq = can->tx_idx & (can->can.echo_skb_max - 1);

	unsigned int frame_len;

	int nr_words;

	u8 count;



	if (can_dev_dropped_skb(netdev, skb))

		return NETDEV_TX_OK;

	if (!netif_subqueue_maybe_stop(netdev, 0, kvaser_pciefd_tx_avail(can), 1, 1))

		return NETDEV_TX_BUSY;



	nr_words = kvaser_pciefd_prepare_tx_packet(&packet, can, skb);

	nr_words = kvaser_pciefd_prepare_tx_packet(&packet, &can->can, seq, 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);



	/* Move echo index to the next slot */

	can->echo_idx = (can->echo_idx + 1) % can->can.echo_skb_max;

	WRITE_ONCE(can->can.echo_skb[seq], NULL);

	frame_len = can_skb_get_frame_len(skb);

	can_put_echo_skb(skb, netdev, seq, frame_len);

	netdev_sent_queue(netdev, frame_len);

	WRITE_ONCE(can->tx_idx, can->tx_idx + 1);



	/* Write header to fifo */

	iowrite32(packet.header[0],
@@ -836,14 +850,7 @@ static netdev_tx_t kvaser_pciefd_start_xmit(struct sk_buff *skb, 
			     KVASER_PCIEFD_KCAN_FIFO_LAST_REG);

	}



	count = FIELD_GET(KVASER_PCIEFD_KCAN_TX_NR_PACKETS_CURRENT_MASK,

			  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 >= can->can.echo_skb_max || can->can.echo_skb[can->echo_idx])

		netif_stop_queue(netdev);

	spin_unlock_irqrestore(&can->echo_lock, irq_flags);

	netif_subqueue_maybe_stop(netdev, 0, kvaser_pciefd_tx_avail(can), 1, 1);



	return NETDEV_TX_OK;

}
@@ -970,6 +977,8 @@ static int kvaser_pciefd_setup_can_ctrls(struct kvaser_pciefd *pcie) 
		can->kv_pcie = pcie;

		can->cmd_seq = 0;

		can->err_rep_cnt = 0;

		can->completed_tx_pkts = 0;

		can->completed_tx_bytes = 0;

		can->bec.txerr = 0;

		can->bec.rxerr = 0;
@@ -983,11 +992,10 @@ 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));

		can->tx_max_count = min(KVASER_PCIEFD_CAN_TX_MAX_COUNT, tx_nr_packets_max - 1);



		can->can.clock.freq = pcie->freq;

		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);

		can->can.echo_skb_max = roundup_pow_of_two(can->tx_max_count);

		spin_lock_init(&can->lock);



		can->can.bittiming_const = &kvaser_pciefd_bittiming_const;
@@ -1201,7 +1209,7 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie, 
		skb = alloc_canfd_skb(priv->dev, &cf);

		if (!skb) {

			priv->dev->stats.rx_dropped++;

			return -ENOMEM;

			return 0;

		}



		cf->len = can_fd_dlc2len(dlc);
@@ -1213,7 +1221,7 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie, 
		skb = alloc_can_skb(priv->dev, (struct can_frame **)&cf);

		if (!skb) {

			priv->dev->stats.rx_dropped++;

			return -ENOMEM;

			return 0;

		}

		can_frame_set_cc_len((struct can_frame *)cf, dlc, priv->ctrlmode);

	}
@@ -1231,7 +1239,9 @@ static int kvaser_pciefd_handle_data_packet(struct kvaser_pciefd *pcie, 
	priv->dev->stats.rx_packets++;

	kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);



	return netif_rx(skb);

	netif_rx(skb);



	return 0;

}



static void kvaser_pciefd_change_state(struct kvaser_pciefd_can *can,
@@ -1510,19 +1520,21 @@ 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 len;

		u8 count;

		unsigned int len, frame_len = 0;

		struct sk_buff *skb;



		if (echo_idx != (can->ack_idx & (can->can.echo_skb_max - 1)))

			return 0;

		skb = can->can.echo_skb[echo_idx];

		if (skb)

		if (!skb)

			return 0;

		kvaser_pciefd_set_skb_timestamp(pcie, skb, p->timestamp);

		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_NR_PACKETS_REG));

		len = can_get_echo_skb(can->can.dev, echo_idx, &frame_len);



		if (count < can->can.echo_skb_max && netif_queue_stopped(can->can.dev))

			netif_wake_queue(can->can.dev);

		/* Pairs with barrier in kvaser_pciefd_start_xmit() */

		smp_store_release(&can->ack_idx, can->ack_idx + 1);

		can->completed_tx_pkts++;

		can->completed_tx_bytes += frame_len;



		if (!one_shot_fail) {

			can->can.dev->stats.tx_bytes += len;
@@ -1638,32 +1650,51 @@ static int kvaser_pciefd_read_buffer(struct kvaser_pciefd *pcie, int dma_buf) 
{

	int pos = 0;

	int res = 0;

	unsigned int i;



	do {

		res = kvaser_pciefd_read_packet(pcie, &pos, dma_buf);

	} while (!res && pos > 0 && pos < KVASER_PCIEFD_DMA_SIZE);



	/* Report ACKs in this buffer to BQL en masse for correct periods */

	for (i = 0; i < pcie->nr_channels; ++i) {

		struct kvaser_pciefd_can *can = pcie->can[i];



		if (!can->completed_tx_pkts)

			continue;

		netif_subqueue_completed_wake(can->can.dev, 0,

					      can->completed_tx_pkts,

					      can->completed_tx_bytes,

					      kvaser_pciefd_tx_avail(can), 1);

		can->completed_tx_pkts = 0;

		can->completed_tx_bytes = 0;

	}



	return res;

}



static u32 kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)

static void kvaser_pciefd_receive_irq(struct kvaser_pciefd *pcie)

{

	void __iomem *srb_cmd_reg = KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_CMD_REG;

	u32 irq = ioread32(KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_IRQ_REG);



	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD0)

	iowrite32(irq, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_IRQ_REG);



	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD0) {

		kvaser_pciefd_read_buffer(pcie, 0);

		iowrite32(KVASER_PCIEFD_SRB_CMD_RDB0, srb_cmd_reg); /* Rearm buffer */

	}



	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD1)

	if (irq & KVASER_PCIEFD_SRB_IRQ_DPD1) {

		kvaser_pciefd_read_buffer(pcie, 1);

		iowrite32(KVASER_PCIEFD_SRB_CMD_RDB1, srb_cmd_reg); /* Rearm buffer */

	}



	if (unlikely(irq & KVASER_PCIEFD_SRB_IRQ_DOF0 ||

		     irq & KVASER_PCIEFD_SRB_IRQ_DOF1 ||

		     irq & KVASER_PCIEFD_SRB_IRQ_DUF0 ||

		     irq & KVASER_PCIEFD_SRB_IRQ_DUF1))

		dev_err(&pcie->pci->dev, "DMA IRQ error 0x%08X\n", irq);



	iowrite32(irq, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_IRQ_REG);

	return irq;

}



static void kvaser_pciefd_transmit_irq(struct kvaser_pciefd_can *can)
@@ -1691,29 +1722,22 @@ static irqreturn_t kvaser_pciefd_irq_handler(int irq, void *dev) 
	struct kvaser_pciefd *pcie = (struct kvaser_pciefd *)dev;

	const struct kvaser_pciefd_irq_mask *irq_mask = pcie->driver_data->irq_mask;

	u32 pci_irq = ioread32(KVASER_PCIEFD_PCI_IRQ_ADDR(pcie));

	u32 srb_irq = 0;

	u32 srb_release = 0;

	int i;



	if (!(pci_irq & irq_mask->all))

		return IRQ_NONE;



	iowrite32(0, KVASER_PCIEFD_PCI_IEN_ADDR(pcie));



	if (pci_irq & irq_mask->kcan_rx0)

		srb_irq = kvaser_pciefd_receive_irq(pcie);

		kvaser_pciefd_receive_irq(pcie);



	for (i = 0; i < pcie->nr_channels; i++) {

		if (pci_irq & irq_mask->kcan_tx[i])

			kvaser_pciefd_transmit_irq(pcie->can[i]);

	}



	if (srb_irq & KVASER_PCIEFD_SRB_IRQ_DPD0)

		srb_release |= KVASER_PCIEFD_SRB_CMD_RDB0;



	if (srb_irq & KVASER_PCIEFD_SRB_IRQ_DPD1)

		srb_release |= KVASER_PCIEFD_SRB_CMD_RDB1;



	if (srb_release)

		iowrite32(srb_release, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_CMD_REG);

	iowrite32(irq_mask->all, KVASER_PCIEFD_PCI_IEN_ADDR(pcie));



	return IRQ_HANDLED;

}
@@ -1733,13 +1757,22 @@ static void kvaser_pciefd_teardown_can_ctrls(struct kvaser_pciefd *pcie) 
	}

}



static void kvaser_pciefd_disable_irq_srcs(struct kvaser_pciefd *pcie)

{

	unsigned int i;



	/* Masking PCI_IRQ is insufficient as running ISR will unmask it */

	iowrite32(0, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_IEN_REG);

	for (i = 0; i < pcie->nr_channels; ++i)

		iowrite32(0, pcie->can[i]->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);

}



static int kvaser_pciefd_probe(struct pci_dev *pdev,

			       const struct pci_device_id *id)

{

	int ret;

	struct kvaser_pciefd *pcie;

	const struct kvaser_pciefd_irq_mask *irq_mask;

	void __iomem *irq_en_base;



	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);

	if (!pcie)
@@ -1805,8 +1838,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev, 
		  KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_IEN_REG);



	/* Enable PCI interrupts */

	irq_en_base = KVASER_PCIEFD_PCI_IEN_ADDR(pcie);

	iowrite32(irq_mask->all, irq_en_base);

	iowrite32(irq_mask->all, KVASER_PCIEFD_PCI_IEN_ADDR(pcie));

	/* Ready the DMA buffers */

	iowrite32(KVASER_PCIEFD_SRB_CMD_RDB0,

		  KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_CMD_REG);
@@ -1820,8 +1852,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev, 
	return 0;



err_free_irq:

	/* Disable PCI interrupts */

	iowrite32(0, irq_en_base);

	kvaser_pciefd_disable_irq_srcs(pcie);

	free_irq(pcie->pci->irq, pcie);



err_pci_free_irq_vectors:
@@ -1844,35 +1875,26 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev, 
	return ret;

}



static void kvaser_pciefd_remove_all_ctrls(struct kvaser_pciefd *pcie)

static void kvaser_pciefd_remove(struct pci_dev *pdev)

{

	int i;

	struct kvaser_pciefd *pcie = pci_get_drvdata(pdev);

	unsigned int i;



	for (i = 0; i < pcie->nr_channels; i++) {

	for (i = 0; i < pcie->nr_channels; ++i) {

		struct kvaser_pciefd_can *can = pcie->can[i];



		if (can) {

			iowrite32(0, can->reg_base + KVASER_PCIEFD_KCAN_IEN_REG);

		unregister_candev(can->can.dev);

		timer_delete(&can->bec_poll_timer);

		kvaser_pciefd_pwm_stop(can);

			free_candev(can->can.dev);

		}

	}

	}



static void kvaser_pciefd_remove(struct pci_dev *pdev)

{

	struct kvaser_pciefd *pcie = pci_get_drvdata(pdev);



	kvaser_pciefd_remove_all_ctrls(pcie);



	/* Disable interrupts */

	iowrite32(0, KVASER_PCIEFD_SRB_ADDR(pcie) + KVASER_PCIEFD_SRB_CTRL_REG);

	iowrite32(0, KVASER_PCIEFD_PCI_IEN_ADDR(pcie));



	kvaser_pciefd_disable_irq_srcs(pcie);

	free_irq(pcie->pci->irq, pcie);

	pci_free_irq_vectors(pcie->pci);



	for (i = 0; i < pcie->nr_channels; ++i)

		free_candev(pcie->can[i]->can.dev);



	pci_iounmap(pdev, pcie->reg_base);

	pci_release_regions(pdev);

	pci_disable_device(pdev);