Hi Marc, Simon and everyone,
thanks again for taking the time for review.
This version has the fixes for the issues you pointed out in v3.
It is tested on tcan455x but I don't have hardware with mcan on the SoC
myself so any testing is appreciated.
The series implements many small and bigger throughput improvements and
adds rx/tx coalescing at the end.
Based on v6.4-rc1. Also available at
https://gitlab.baylibre.com/msp8/linux/-/tree/topic/mcan-optimization/v6.4?ref_type=heads
Best,
Markus
Changes in v4:
- Create and use struct m_can_fifo_element in m_can_tx_handler
- Fix memcpy_and_pad to copy the full buffer
- Fixed a few checkpatch warnings
- Change putidx to be unsigned
- Print hard_xmit error only once when TX FIFO is full
Changes in v3:
- Remove parenthesis in error messages
- Use memcpy_and_pad for buffer copy in 'can: m_can: Write transmit
header and data in one transaction'.
- Replace spin_lock with spin_lock_irqsave. I got a report of a
interrupt that was calling start_xmit just after the netqueue was
woken up before the locked region was exited. spin_lock_irqsave should
fix this. I attached the full stack at the end of the mail if someone
wants to know.
- Rebased to v6.3-rc1.
- Removed tcan4x5x patches from this series.
Changes in v2:
- Rebased on v6.2-rc5
- Fixed missing/broken accounting for non peripheral m_can devices.
previous versions:
v1 - https://lore.kernel.org/lkml/[email protected]
v2 - https://lore.kernel.org/lkml/[email protected]
v3 - https://lore.kernel.org/lkml/[email protected]/
Markus Schneider-Pargmann (12):
can: m_can: Write transmit header and data in one transaction
can: m_can: Implement receive coalescing
can: m_can: Implement transmit coalescing
can: m_can: Add rx coalescing ethtool support
can: m_can: Add tx coalescing ethtool support
can: m_can: Use u32 for putidx
can: m_can: Cache tx putidx
can: m_can: Use the workqueue as queue
can: m_can: Introduce a tx_fifo_in_flight counter
can: m_can: Use tx_fifo_in_flight for netif_queue control
can: m_can: Implement BQL
can: m_can: Implement transmit submission coalescing
drivers/net/can/m_can/m_can.c | 516 +++++++++++++++++++++++++---------
drivers/net/can/m_can/m_can.h | 35 ++-
2 files changed, 418 insertions(+), 133 deletions(-)
base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.40.1
m_can offers the possibility to set an interrupt on reaching a watermark
level in the receive FIFO. This can be used to implement coalescing.
Unfortunately there is no hardware timeout available to trigger an
interrupt if only a few messages were received within a given time. To
solve this I am using a hrtimer to wake up the irq thread after x
microseconds.
The timer is always started if receive coalescing is enabled and new
received frames were available during an interrupt. The timer is stopped
if during a interrupt handling no new data was available.
If the timer is started the new item interrupt is disabled and the
watermark interrupt takes over. If the timer is not started again, the
new item interrupt is enabled again, notifying the handler about every
new item received.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
drivers/net/can/m_can/m_can.c | 70 ++++++++++++++++++++++++++++++++---
drivers/net/can/m_can/m_can.h | 7 ++++
2 files changed, 71 insertions(+), 6 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 5251073987ee..02dfb416fbd2 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -419,6 +419,22 @@ static void m_can_config_endisable(struct m_can_classdev *cdev, bool enable)
}
}
+static void m_can_interrupt_enable(struct m_can_classdev *cdev, u32 interrupts)
+{
+ if (cdev->active_interrupts == interrupts)
+ return;
+ cdev->ops->write_reg(cdev, M_CAN_IE, interrupts);
+ cdev->active_interrupts = interrupts;
+}
+
+static void m_can_coalescing_disable(struct m_can_classdev *cdev)
+{
+ u32 new_interrupts = cdev->active_interrupts | IR_RF0N;
+
+ hrtimer_cancel(&cdev->irq_timer);
+ m_can_interrupt_enable(cdev, new_interrupts);
+}
+
static inline void m_can_enable_all_interrupts(struct m_can_classdev *cdev)
{
/* Only interrupt line 0 is used in this driver */
@@ -427,6 +443,7 @@ static inline void m_can_enable_all_interrupts(struct m_can_classdev *cdev)
static inline void m_can_disable_all_interrupts(struct m_can_classdev *cdev)
{
+ m_can_coalescing_disable(cdev);
m_can_write(cdev, M_CAN_ILE, 0x0);
}
@@ -1076,15 +1093,39 @@ static int m_can_echo_tx_event(struct net_device *dev)
return err;
}
+static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
+{
+ u32 new_interrupts = cdev->active_interrupts;
+ bool enable_timer = false;
+
+ if (cdev->rx_coalesce_usecs_irq > 0 && (ir & (IR_RF0N | IR_RF0W))) {
+ enable_timer = true;
+ new_interrupts &= ~IR_RF0N;
+ } else if (!hrtimer_active(&cdev->irq_timer)) {
+ new_interrupts |= IR_RF0N;
+ }
+
+ m_can_interrupt_enable(cdev, new_interrupts);
+ if (enable_timer) {
+ hrtimer_start(&cdev->irq_timer,
+ ns_to_ktime(cdev->rx_coalesce_usecs_irq * NSEC_PER_USEC),
+ HRTIMER_MODE_REL);
+ }
+}
+
static irqreturn_t m_can_isr(int irq, void *dev_id)
{
struct net_device *dev = (struct net_device *)dev_id;
struct m_can_classdev *cdev = netdev_priv(dev);
u32 ir;
- if (pm_runtime_suspended(cdev->dev))
+ if (pm_runtime_suspended(cdev->dev)) {
+ m_can_coalescing_disable(cdev);
return IRQ_NONE;
+ }
+
ir = m_can_read(cdev, M_CAN_IR);
+ m_can_coalescing_update(cdev, ir);
if (!ir)
return IRQ_NONE;
@@ -1099,13 +1140,17 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
* - state change IRQ
* - bus error IRQ and bus error reporting
*/
- if ((ir & IR_RF0N) || (ir & IR_ERR_ALL_30X)) {
+ if (ir & (IR_RF0N | IR_RF0W | IR_ERR_ALL_30X)) {
cdev->irqstatus = ir;
if (!cdev->is_peripheral) {
m_can_disable_all_interrupts(cdev);
napi_schedule(&cdev->napi);
- } else if (m_can_rx_peripheral(dev, ir) < 0) {
- goto out_fail;
+ } else {
+ int pkts;
+
+ pkts = m_can_rx_peripheral(dev, ir);
+ if (pkts < 0)
+ goto out_fail;
}
}
@@ -1141,6 +1186,15 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static enum hrtimer_restart m_can_irq_timer(struct hrtimer *timer)
+{
+ struct m_can_classdev *cdev = container_of(timer, struct m_can_classdev, irq_timer);
+
+ irq_wake_thread(cdev->net->irq, cdev->net);
+
+ return HRTIMER_NORESTART;
+}
+
static const struct can_bittiming_const m_can_bittiming_const_30X = {
.name = KBUILD_MODNAME,
.tseg1_min = 2, /* Time segment 1 = prop_seg + phase_seg1 */
@@ -1281,7 +1335,7 @@ static int m_can_chip_config(struct net_device *dev)
/* Disable unused interrupts */
interrupts &= ~(IR_ARA | IR_ELO | IR_DRX | IR_TEFF | IR_TEFW | IR_TFE |
IR_TCF | IR_HPM | IR_RF1F | IR_RF1W | IR_RF1N |
- IR_RF0F | IR_RF0W);
+ IR_RF0F);
m_can_config_endisable(cdev, true);
@@ -1325,6 +1379,7 @@ static int m_can_chip_config(struct net_device *dev)
/* rx fifo configuration, blocking mode, fifo size 1 */
m_can_write(cdev, M_CAN_RXF0C,
+ FIELD_PREP(RXFC_FWM_MASK, cdev->rx_max_coalesced_frames_irq) |
FIELD_PREP(RXFC_FS_MASK, cdev->mcfg[MRAM_RXF0].num) |
cdev->mcfg[MRAM_RXF0].off);
@@ -1383,7 +1438,7 @@ static int m_can_chip_config(struct net_device *dev)
else
interrupts &= ~(IR_ERR_LEC_31X);
}
- m_can_write(cdev, M_CAN_IE, interrupts);
+ m_can_interrupt_enable(cdev, interrupts);
/* route all interrupts to INT0 */
m_can_write(cdev, M_CAN_ILS, ILS_ALL_INT0);
@@ -2048,6 +2103,9 @@ int m_can_class_register(struct m_can_classdev *cdev)
of_can_transceiver(cdev->net);
+ hrtimer_init(&cdev->irq_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ cdev->irq_timer.function = m_can_irq_timer;
+
dev_info(cdev->dev, "%s device registered (irq=%d, version=%d)\n",
KBUILD_MODNAME, cdev->net->irq, cdev->version);
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index a839dc71dc9b..c59099d3f5b9 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -84,6 +84,8 @@ struct m_can_classdev {
struct sk_buff *tx_skb;
struct phy *transceiver;
+ struct hrtimer irq_timer;
+
struct m_can_ops *ops;
int version;
@@ -92,6 +94,11 @@ struct m_can_classdev {
int pm_clock_support;
int is_peripheral;
+ // Cached M_CAN_IE register content
+ u32 active_interrupts;
+ u32 rx_max_coalesced_frames_irq;
+ u32 rx_coalesce_usecs_irq;
+
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.40.1
putidx is not an integer normally, it is an unsigned field used in
hardware registers. Use a u32 for it.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index d1435d1466b2..6f8043636c54 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -467,7 +467,7 @@ static void m_can_clean(struct net_device *net)
struct m_can_classdev *cdev = netdev_priv(net);
if (cdev->tx_skb) {
- int putidx = 0;
+ u32 putidx = 0;
net->stats.tx_errors++;
if (cdev->version > 30)
@@ -1673,12 +1673,12 @@ static int m_can_close(struct net_device *dev)
return 0;
}
-static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
+static int m_can_next_echo_skb_occupied(struct net_device *dev, u32 putidx)
{
struct m_can_classdev *cdev = netdev_priv(dev);
/*get wrap around for loopback skb index */
unsigned int wrap = cdev->can.echo_skb_max;
- int next_idx;
+ u32 next_idx;
/* calculate next index */
next_idx = (++putidx >= wrap ? 0 : putidx);
@@ -1698,7 +1698,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
u32 cccr, fdflags;
u32 txfqs;
int err;
- int putidx;
+ u32 putidx;
cdev->tx_skb = NULL;
--
2.40.1
m_can_tx_handler is the only place where data is written to the tx fifo.
We can calculate the putidx in the driver code here to avoid the
dependency on the txfqs register.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
drivers/net/can/m_can/m_can.c | 8 +++++++-
drivers/net/can/m_can/m_can.h | 3 +++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 6f8043636c54..40acd78cc0ed 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1482,6 +1482,10 @@ static int m_can_start(struct net_device *dev)
m_can_enable_all_interrupts(cdev);
+ if (cdev->version > 30)
+ cdev->tx_fifo_putidx = FIELD_GET(TXFQS_TFQPI_MASK,
+ m_can_read(cdev, M_CAN_TXFQS));
+
return 0;
}
@@ -1772,7 +1776,7 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
}
/* get put index for frame */
- putidx = FIELD_GET(TXFQS_TFQPI_MASK, txfqs);
+ putidx = cdev->tx_fifo_putidx;
/* Construct DLC Field, with CAN-FD configuration.
* Use the put index of the fifo as the message marker,
@@ -1806,6 +1810,8 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
/* Enable TX FIFO element to start transfer */
m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
+ cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
+ 0 : cdev->tx_fifo_putidx);
/* stop network queue if fifo full */
if (m_can_tx_fifo_full(cdev) ||
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index d0c21eddb6ec..548ae908ac4e 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -102,6 +102,9 @@ struct m_can_classdev {
u32 tx_max_coalesced_frames_irq;
u32 tx_coalesce_usecs_irq;
+ // Store this internally to avoid fetch delays on peripheral chips
+ int tx_fifo_putidx;
+
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.40.1
m_can supports submitting multiple transmits with one register write.
This is an interesting option to reduce the number of SPI transfers for
peripheral chips.
The m_can_tx_op is extended with a bool that signals if it is the last
transmission and the submit should be executed immediately.
The worker then writes the skb to the FIFO and submits it only if the
submit bool is set. If it isn't set, the worker will write the next skb
which is waiting in the workqueue to the FIFO, etc.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
Notes:
Notes:
- I ran into lost messages in the receive FIFO when using this
implementation. I guess this only shows up with my test setup in
loopback mode and maybe not enough CPU power.
- I put this behind the tx-frames ethtool coalescing option as we do
wait before submitting packages but it is something different than the
tx-frames-irq option. I am not sure if this is the correct option,
please let me know.
drivers/net/can/m_can/m_can.c | 55 ++++++++++++++++++++++++++++++++---
drivers/net/can/m_can/m_can.h | 6 ++++
2 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 62e275c87c29..50909b9c0e7c 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1515,6 +1515,9 @@ static int m_can_start(struct net_device *dev)
if (ret)
return ret;
+ netdev_queue_set_dql_min_limit(netdev_get_tx_queue(cdev->net, 0),
+ cdev->tx_max_coalesced_frames);
+
cdev->can.state = CAN_STATE_ERROR_ACTIVE;
m_can_enable_all_interrupts(cdev);
@@ -1812,8 +1815,13 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
*/
can_put_echo_skb(skb, dev, putidx, frame_len);
- /* Enable TX FIFO element to start transfer */
- m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
+ if (cdev->is_peripheral) {
+ /* Delay enabling TX FIFO element */
+ cdev->tx_peripheral_submit |= BIT(putidx);
+ } else {
+ /* Enable TX FIFO element to start transfer */
+ m_can_write(cdev, M_CAN_TXBAR, BIT(putidx));
+ }
cdev->tx_fifo_putidx = (++cdev->tx_fifo_putidx >= cdev->can.echo_skb_max ?
0 : cdev->tx_fifo_putidx);
}
@@ -1826,6 +1834,17 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
return NETDEV_TX_BUSY;
}
+static void m_can_tx_submit(struct m_can_classdev *cdev)
+{
+ if (cdev->version == 30)
+ return;
+ if (!cdev->is_peripheral)
+ return;
+
+ m_can_write(cdev, M_CAN_TXBAR, cdev->tx_peripheral_submit);
+ cdev->tx_peripheral_submit = 0;
+}
+
static void m_can_tx_work_queue(struct work_struct *ws)
{
struct m_can_tx_op *op = container_of(ws, struct m_can_tx_op, work);
@@ -1834,11 +1853,15 @@ static void m_can_tx_work_queue(struct work_struct *ws)
op->skb = NULL;
m_can_tx_handler(cdev, skb);
+ if (op->submit)
+ m_can_tx_submit(cdev);
}
-static void m_can_tx_queue_skb(struct m_can_classdev *cdev, struct sk_buff *skb)
+static void m_can_tx_queue_skb(struct m_can_classdev *cdev, struct sk_buff *skb,
+ bool submit)
{
cdev->tx_ops[cdev->next_tx_op].skb = skb;
+ cdev->tx_ops[cdev->next_tx_op].submit = submit;
queue_work(cdev->tx_wq, &cdev->tx_ops[cdev->next_tx_op].work);
++cdev->next_tx_op;
@@ -1850,6 +1873,7 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
struct sk_buff *skb)
{
netdev_tx_t err;
+ bool submit;
if (cdev->can.state == CAN_STATE_BUS_OFF) {
m_can_clean(cdev->net);
@@ -1860,7 +1884,15 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
if (err != NETDEV_TX_OK)
return err;
- m_can_tx_queue_skb(cdev, skb);
+ ++cdev->nr_txs_without_submit;
+ if (cdev->nr_txs_without_submit >= cdev->tx_max_coalesced_frames ||
+ !netdev_xmit_more()) {
+ cdev->nr_txs_without_submit = 0;
+ submit = true;
+ } else {
+ submit = false;
+ }
+ m_can_tx_queue_skb(cdev, skb, submit);
return NETDEV_TX_OK;
}
@@ -1992,6 +2024,7 @@ static int m_can_get_coalesce(struct net_device *dev,
ec->rx_max_coalesced_frames_irq = cdev->rx_max_coalesced_frames_irq;
ec->rx_coalesce_usecs_irq = cdev->rx_coalesce_usecs_irq;
+ ec->tx_max_coalesced_frames = cdev->tx_max_coalesced_frames;
ec->tx_max_coalesced_frames_irq = cdev->tx_max_coalesced_frames_irq;
ec->tx_coalesce_usecs_irq = cdev->tx_coalesce_usecs_irq;
@@ -2036,6 +2069,18 @@ static int m_can_set_coalesce(struct net_device *dev,
netdev_err(dev, "tx-frames-irq and tx-usecs-irq can only be set together\n");
return -EINVAL;
}
+ if (ec->tx_max_coalesced_frames > cdev->mcfg[MRAM_TXE].num) {
+ netdev_err(dev, "tx-frames %u greater than the TX event FIFO %u\n",
+ ec->tx_max_coalesced_frames,
+ cdev->mcfg[MRAM_TXE].num);
+ return -EINVAL;
+ }
+ if (ec->tx_max_coalesced_frames > cdev->mcfg[MRAM_TXB].num) {
+ netdev_err(dev, "tx-frames %u greater than the TX FIFO %u\n",
+ ec->tx_max_coalesced_frames,
+ cdev->mcfg[MRAM_TXB].num);
+ return -EINVAL;
+ }
if (ec->rx_coalesce_usecs_irq != 0 && ec->tx_coalesce_usecs_irq != 0 &&
ec->rx_coalesce_usecs_irq != ec->tx_coalesce_usecs_irq) {
netdev_err(dev, "rx-usecs-irq %u needs to be equal to tx-usecs-irq %u if both are enabled\n",
@@ -2046,6 +2091,7 @@ static int m_can_set_coalesce(struct net_device *dev,
cdev->rx_max_coalesced_frames_irq = ec->rx_max_coalesced_frames_irq;
cdev->rx_coalesce_usecs_irq = ec->rx_coalesce_usecs_irq;
+ cdev->tx_max_coalesced_frames = ec->tx_max_coalesced_frames;
cdev->tx_max_coalesced_frames_irq = ec->tx_max_coalesced_frames_irq;
cdev->tx_coalesce_usecs_irq = ec->tx_coalesce_usecs_irq;
@@ -2063,6 +2109,7 @@ static const struct ethtool_ops m_can_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ |
ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ |
ETHTOOL_COALESCE_TX_USECS_IRQ |
+ ETHTOOL_COALESCE_TX_MAX_FRAMES |
ETHTOOL_COALESCE_TX_MAX_FRAMES_IRQ,
.get_ts_info = ethtool_op_get_ts_info,
.get_coalesce = m_can_get_coalesce,
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 5c182aece15c..54af26a94042 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -74,6 +74,7 @@ struct m_can_tx_op {
struct m_can_classdev *cdev;
struct work_struct work;
struct sk_buff *skb;
+ bool submit;
};
struct m_can_classdev {
@@ -103,6 +104,7 @@ struct m_can_classdev {
u32 active_interrupts;
u32 rx_max_coalesced_frames_irq;
u32 rx_coalesce_usecs_irq;
+ u32 tx_max_coalesced_frames;
u32 tx_max_coalesced_frames_irq;
u32 tx_coalesce_usecs_irq;
@@ -117,6 +119,10 @@ struct m_can_classdev {
int tx_fifo_size;
int next_tx_op;
+ int nr_txs_without_submit;
+ /* bitfield of fifo elements that will be submitted together */
+ u32 tx_peripheral_submit;
+
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.40.1
Keep track of the number of transmits in flight.
This patch prepares the driver to control the network interface queue
based on this counter. By itself this counter be
implemented with an atomic, but as we need to do other things in the
critical sections later I am using a spinlock instead.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
drivers/net/can/m_can/m_can.c | 41 ++++++++++++++++++++++++++++++++++-
drivers/net/can/m_can/m_can.h | 4 ++++
2 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 88c8d6a418f0..3353fd9fb3a2 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -465,6 +465,7 @@ static u32 m_can_get_timestamp(struct m_can_classdev *cdev)
static void m_can_clean(struct net_device *net)
{
struct m_can_classdev *cdev = netdev_priv(net);
+ unsigned long irqflags;
for (int i = 0; i != cdev->tx_fifo_size; ++i) {
if (!cdev->tx_ops[i].skb)
@@ -476,6 +477,10 @@ static void m_can_clean(struct net_device *net)
for (int i = 0; i != cdev->can.echo_skb_max; ++i)
can_free_echo_skb(cdev->net, i, NULL);
+
+ spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);
+ cdev->tx_fifo_in_flight = 0;
+ spin_unlock_irqrestore(&cdev->tx_handling_spinlock, irqflags);
}
/* For peripherals, pass skb to rx-offload, which will push skb from
@@ -1046,6 +1051,24 @@ static void m_can_tx_update_stats(struct m_can_classdev *cdev,
stats->tx_packets++;
}
+static void m_can_finish_tx(struct m_can_classdev *cdev, int transmitted)
+{
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);
+ cdev->tx_fifo_in_flight -= transmitted;
+ spin_unlock_irqrestore(&cdev->tx_handling_spinlock, irqflags);
+}
+
+static void m_can_start_tx(struct m_can_classdev *cdev)
+{
+ unsigned long irqflags;
+
+ spin_lock_irqsave(&cdev->tx_handling_spinlock, irqflags);
+ ++cdev->tx_fifo_in_flight;
+ spin_unlock_irqrestore(&cdev->tx_handling_spinlock, irqflags);
+}
+
static int m_can_echo_tx_event(struct net_device *dev)
{
u32 txe_count = 0;
@@ -1055,6 +1078,7 @@ static int m_can_echo_tx_event(struct net_device *dev)
int i = 0;
int err = 0;
unsigned int msg_mark;
+ int processed = 0;
struct m_can_classdev *cdev = netdev_priv(dev);
@@ -1084,12 +1108,15 @@ static int m_can_echo_tx_event(struct net_device *dev)
/* update stats */
m_can_tx_update_stats(cdev, msg_mark, timestamp);
+ ++processed;
}
if (ack_fgi != -1)
m_can_write(cdev, M_CAN_TXEFA, FIELD_PREP(TXEFA_EFAI_MASK,
ack_fgi));
+ m_can_finish_tx(cdev, processed);
+
return err;
}
@@ -1168,6 +1195,7 @@ static irqreturn_t m_can_isr(int irq, void *dev_id)
timestamp = m_can_get_timestamp(cdev);
m_can_tx_update_stats(cdev, 0, timestamp);
netif_wake_queue(dev);
+ m_can_finish_tx(cdev, 1);
}
} else {
if (ir & (IR_TEFN | IR_TEFW)) {
@@ -1854,11 +1882,22 @@ static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
}
netif_stop_queue(cdev->net);
+
+ m_can_start_tx(cdev);
+
m_can_tx_queue_skb(cdev, skb);
return NETDEV_TX_OK;
}
+static netdev_tx_t m_can_start_fast_xmit(struct m_can_classdev *cdev,
+ struct sk_buff *skb)
+{
+ m_can_start_tx(cdev);
+
+ return m_can_tx_handler(cdev, skb);
+}
+
static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
struct net_device *dev)
{
@@ -1870,7 +1909,7 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
if (cdev->is_peripheral)
return m_can_start_peripheral_xmit(cdev, skb);
else
- return m_can_tx_handler(cdev, skb);
+ return m_can_start_fast_xmit(cdev, skb);
}
static int m_can_open(struct net_device *dev)
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 38b154fea04b..5c182aece15c 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -109,6 +109,10 @@ struct m_can_classdev {
// Store this internally to avoid fetch delays on peripheral chips
u32 tx_fifo_putidx;
+ /* Protects shared state between start_xmit and m_can_isr */
+ spinlock_t tx_handling_spinlock;
+ int tx_fifo_in_flight;
+
struct m_can_tx_op *tx_ops;
int tx_fifo_size;
int next_tx_op;
--
2.40.1
Combine header and data before writing to the transmit fifo to reduce
the overhead for peripheral chips.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
---
drivers/net/can/m_can/m_can.c | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a5003435802b..5251073987ee 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -317,6 +317,12 @@ struct id_and_dlc {
u32 dlc;
};
+struct m_can_fifo_element {
+ u32 id;
+ u32 dlc;
+ u8 data[CANFD_MAX_DLEN];
+};
+
static inline u32 m_can_read(struct m_can_classdev *cdev, enum m_can_reg reg)
{
return cdev->ops->read_reg(cdev, reg);
@@ -1622,6 +1628,8 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
{
struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data;
+ u8 len_padded = DIV_ROUND_UP(cf->len, 4);
+ struct m_can_fifo_element fifo_element;
struct net_device *dev = cdev->net;
struct sk_buff *skb = cdev->tx_skb;
struct id_and_dlc fifo_header;
@@ -1635,27 +1643,27 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
/* Generate ID field for TX buffer Element */
/* Common to all supported M_CAN versions */
if (cf->can_id & CAN_EFF_FLAG) {
- fifo_header.id = cf->can_id & CAN_EFF_MASK;
- fifo_header.id |= TX_BUF_XTD;
+ fifo_element.id = cf->can_id & CAN_EFF_MASK;
+ fifo_element.id |= TX_BUF_XTD;
} else {
- fifo_header.id = ((cf->can_id & CAN_SFF_MASK) << 18);
+ fifo_element.id = ((cf->can_id & CAN_SFF_MASK) << 18);
}
if (cf->can_id & CAN_RTR_FLAG)
- fifo_header.id |= TX_BUF_RTR;
+ fifo_element.id |= TX_BUF_RTR;
if (cdev->version == 30) {
netif_stop_queue(dev);
- fifo_header.dlc = can_fd_len2dlc(cf->len) << 16;
+ fifo_element.dlc = can_fd_len2dlc(cf->len) << 16;
/* Write the frame ID, DLC, and payload to the FIFO element. */
- err = m_can_fifo_write(cdev, 0, M_CAN_FIFO_ID, &fifo_header, 2);
+ err = m_can_fifo_write(cdev, 0, M_CAN_FIFO_ID, &fifo_element, 2);
if (err)
goto out_fail;
err = m_can_fifo_write(cdev, 0, M_CAN_FIFO_DATA,
- cf->data, DIV_ROUND_UP(cf->len, 4));
+ cf->data, len_padded);
if (err)
goto out_fail;
@@ -1717,15 +1725,15 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
fdflags |= TX_BUF_BRS;
}
- fifo_header.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) |
+ fifo_element.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) |
FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) |
fdflags | TX_BUF_EFC;
- err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID, &fifo_header, 2);
- if (err)
- goto out_fail;
- err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_DATA,
- cf->data, DIV_ROUND_UP(cf->len, 4));
+ memcpy_and_pad(fifo_element.data, CANFD_MAX_DLEN, &cf->data,
+ cf->len, 0);
+
+ err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID,
+ &fifo_element, 2 + len_padded);
if (err)
goto out_fail;
--
2.40.1
The current implementation uses the workqueue for peripheral chips to
submit work. Only a single work item is queued and used at any time.
To be able to keep more than one transmit in flight at a time, prepare
the workqueue to support multiple transmits at the same time.
Each work item now has a separate storage for a skb and a pointer to
cdev. This assures that each workitem can be processed individually.
The workqueue is replaced by an ordered workqueue which makes sure that
only a single worker processes the items queued on the workqueue. Also
items are ordered by the order they were enqueued. This removes most of
the concurrency the workqueue normally offers. It is not necessary for
this driver.
The cleanup functions have to be adopted a bit to handle this new
mechanism.
Signed-off-by: Markus Schneider-Pargmann <[email protected]>
Reviewed-by: Simon Horman <[email protected]>
---
drivers/net/can/m_can/m_can.c | 109 ++++++++++++++++++++--------------
drivers/net/can/m_can/m_can.h | 14 ++++-
2 files changed, 75 insertions(+), 48 deletions(-)
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 40acd78cc0ed..88c8d6a418f0 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -466,17 +466,16 @@ static void m_can_clean(struct net_device *net)
{
struct m_can_classdev *cdev = netdev_priv(net);
- if (cdev->tx_skb) {
- u32 putidx = 0;
+ for (int i = 0; i != cdev->tx_fifo_size; ++i) {
+ if (!cdev->tx_ops[i].skb)
+ continue;
net->stats.tx_errors++;
- if (cdev->version > 30)
- putidx = FIELD_GET(TXFQS_TFQPI_MASK,
- m_can_read(cdev, M_CAN_TXFQS));
-
- can_free_echo_skb(cdev->net, putidx, NULL);
- cdev->tx_skb = NULL;
+ cdev->tx_ops[i].skb = NULL;
}
+
+ for (int i = 0; i != cdev->can.echo_skb_max; ++i)
+ can_free_echo_skb(cdev->net, i, NULL);
}
/* For peripherals, pass skb to rx-offload, which will push skb from
@@ -1663,8 +1662,9 @@ static int m_can_close(struct net_device *dev)
m_can_clk_stop(cdev);
free_irq(dev->irq, dev);
+ m_can_clean(dev);
+
if (cdev->is_peripheral) {
- cdev->tx_skb = NULL;
destroy_workqueue(cdev->tx_wq);
cdev->tx_wq = NULL;
can_rx_offload_disable(&cdev->offload);
@@ -1691,21 +1691,19 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, u32 putidx)
return !!cdev->can.echo_skb[next_idx];
}
-static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
+static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev,
+ struct sk_buff *skb)
{
- struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data;
+ struct canfd_frame *cf = (struct canfd_frame *)skb->data;
u8 len_padded = DIV_ROUND_UP(cf->len, 4);
struct m_can_fifo_element fifo_element;
struct net_device *dev = cdev->net;
- struct sk_buff *skb = cdev->tx_skb;
struct id_and_dlc fifo_header;
u32 cccr, fdflags;
u32 txfqs;
int err;
u32 putidx;
- cdev->tx_skb = NULL;
-
/* Generate ID field for TX buffer Element */
/* Common to all supported M_CAN versions */
if (cf->can_id & CAN_EFF_FLAG) {
@@ -1829,10 +1827,36 @@ static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
static void m_can_tx_work_queue(struct work_struct *ws)
{
- struct m_can_classdev *cdev = container_of(ws, struct m_can_classdev,
- tx_work);
+ struct m_can_tx_op *op = container_of(ws, struct m_can_tx_op, work);
+ struct m_can_classdev *cdev = op->cdev;
+ struct sk_buff *skb = op->skb;
- m_can_tx_handler(cdev);
+ op->skb = NULL;
+ m_can_tx_handler(cdev, skb);
+}
+
+static void m_can_tx_queue_skb(struct m_can_classdev *cdev, struct sk_buff *skb)
+{
+ cdev->tx_ops[cdev->next_tx_op].skb = skb;
+ queue_work(cdev->tx_wq, &cdev->tx_ops[cdev->next_tx_op].work);
+
+ ++cdev->next_tx_op;
+ if (cdev->next_tx_op >= cdev->tx_fifo_size)
+ cdev->next_tx_op = 0;
+}
+
+static netdev_tx_t m_can_start_peripheral_xmit(struct m_can_classdev *cdev,
+ struct sk_buff *skb)
+{
+ if (cdev->can.state == CAN_STATE_BUS_OFF) {
+ m_can_clean(cdev->net);
+ return NETDEV_TX_OK;
+ }
+
+ netif_stop_queue(cdev->net);
+ m_can_tx_queue_skb(cdev, skb);
+
+ return NETDEV_TX_OK;
}
static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
@@ -1843,30 +1867,10 @@ static netdev_tx_t m_can_start_xmit(struct sk_buff *skb,
if (can_dev_dropped_skb(dev, skb))
return NETDEV_TX_OK;
- if (cdev->is_peripheral) {
- if (cdev->tx_skb) {
- netdev_err(dev, "hard_xmit called while tx busy\n");
- return NETDEV_TX_BUSY;
- }
-
- if (cdev->can.state == CAN_STATE_BUS_OFF) {
- m_can_clean(dev);
- } else {
- /* Need to stop the queue to avoid numerous requests
- * from being sent. Suggested improvement is to create
- * a queueing mechanism that will queue the skbs and
- * process them in order.
- */
- cdev->tx_skb = skb;
- netif_stop_queue(cdev->net);
- queue_work(cdev->tx_wq, &cdev->tx_work);
- }
- } else {
- cdev->tx_skb = skb;
- return m_can_tx_handler(cdev);
- }
-
- return NETDEV_TX_OK;
+ if (cdev->is_peripheral)
+ return m_can_start_peripheral_xmit(cdev, skb);
+ else
+ return m_can_tx_handler(cdev, skb);
}
static int m_can_open(struct net_device *dev)
@@ -1894,15 +1898,17 @@ static int m_can_open(struct net_device *dev)
/* register interrupt handler */
if (cdev->is_peripheral) {
- cdev->tx_skb = NULL;
- cdev->tx_wq = alloc_workqueue("mcan_wq",
- WQ_FREEZABLE | WQ_MEM_RECLAIM, 0);
+ cdev->tx_wq = alloc_ordered_workqueue("mcan_wq",
+ WQ_FREEZABLE | WQ_MEM_RECLAIM);
if (!cdev->tx_wq) {
err = -ENOMEM;
goto out_wq_fail;
}
- INIT_WORK(&cdev->tx_work, m_can_tx_work_queue);
+ for (int i = 0; i != cdev->tx_fifo_size; ++i) {
+ cdev->tx_ops[i].cdev = cdev;
+ INIT_WORK(&cdev->tx_ops[i].work, m_can_tx_work_queue);
+ }
err = request_threaded_irq(dev->irq, NULL, m_can_isr,
IRQF_ONESHOT,
@@ -2172,6 +2178,19 @@ int m_can_class_register(struct m_can_classdev *cdev)
{
int ret;
+ cdev->tx_fifo_size = max(1, min(cdev->mcfg[MRAM_TXB].num,
+ cdev->mcfg[MRAM_TXE].num));
+ if (cdev->is_peripheral) {
+ cdev->tx_ops =
+ devm_kzalloc(cdev->dev,
+ cdev->tx_fifo_size * sizeof(*cdev->tx_ops),
+ GFP_KERNEL);
+ if (!cdev->tx_ops) {
+ dev_err(cdev->dev, "Failed to allocate tx_ops for workqueue\n");
+ return -ENOMEM;
+ }
+ }
+
if (cdev->pm_clock_support) {
ret = m_can_clk_start(cdev);
if (ret)
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 548ae908ac4e..38b154fea04b 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -70,6 +70,12 @@ struct m_can_ops {
int (*init)(struct m_can_classdev *cdev);
};
+struct m_can_tx_op {
+ struct m_can_classdev *cdev;
+ struct work_struct work;
+ struct sk_buff *skb;
+};
+
struct m_can_classdev {
struct can_priv can;
struct can_rx_offload offload;
@@ -80,8 +86,6 @@ struct m_can_classdev {
struct clk *cclk;
struct workqueue_struct *tx_wq;
- struct work_struct tx_work;
- struct sk_buff *tx_skb;
struct phy *transceiver;
struct hrtimer irq_timer;
@@ -103,7 +107,11 @@ struct m_can_classdev {
u32 tx_coalesce_usecs_irq;
// Store this internally to avoid fetch delays on peripheral chips
- int tx_fifo_putidx;
+ u32 tx_fifo_putidx;
+
+ struct m_can_tx_op *tx_ops;
+ int tx_fifo_size;
+ int next_tx_op;
struct mram_cfg mcfg[MRAM_CFG_NUM];
};
--
2.40.1
On Wed, Jun 21, 2023 at 11:23:39AM +0200, Markus Schneider-Pargmann wrote:
> Combine header and data before writing to the transmit fifo to reduce
> the overhead for peripheral chips.
>
> Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> ---
> drivers/net/can/m_can/m_can.c | 34 +++++++++++++++++++++-------------
> 1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index a5003435802b..5251073987ee 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -317,6 +317,12 @@ struct id_and_dlc {
> u32 dlc;
> };
>
> +struct m_can_fifo_element {
> + u32 id;
> + u32 dlc;
> + u8 data[CANFD_MAX_DLEN];
> +};
> +
> static inline u32 m_can_read(struct m_can_classdev *cdev, enum m_can_reg reg)
> {
> return cdev->ops->read_reg(cdev, reg);
> @@ -1622,6 +1628,8 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
> static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
> {
> struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data;
> + u8 len_padded = DIV_ROUND_UP(cf->len, 4);
> + struct m_can_fifo_element fifo_element;
> struct net_device *dev = cdev->net;
> struct sk_buff *skb = cdev->tx_skb;
> struct id_and_dlc fifo_header;
Hi Markus,
GCC 12.3.0 complains that fifo_header is not (no longer) used.
drivers/net/can/m_can/m_can.c:1635:20: warning: unused variable 'fifo_header' [-Wunused-variable]
struct id_and_dlc fifo_header;
--
pw-bot: changes-requested
Hi Markus,
kernel test robot noticed the following build warnings:
[auto build test WARNING on ac9a78681b921877518763ba0e89202254349d1b]
url: https://github.com/intel-lab-lkp/linux/commits/Markus-Schneider-Pargmann/can-m_can-Write-transmit-header-and-data-in-one-transaction/20230621-173848
base: ac9a78681b921877518763ba0e89202254349d1b
patch link: https://lore.kernel.org/r/20230621092350.3130866-2-msp%40baylibre.com
patch subject: [PATCH v4 01/12] can: m_can: Write transmit header and data in one transaction
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20230622/[email protected]/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce: (https://download.01.org/0day-ci/archive/20230622/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/net/can/m_can/m_can.c: In function 'm_can_tx_handler':
>> drivers/net/can/m_can/m_can.c:1635:27: warning: unused variable 'fifo_header' [-Wunused-variable]
1635 | struct id_and_dlc fifo_header;
| ^~~~~~~~~~~
vim +/fifo_header +1635 drivers/net/can/m_can/m_can.c
10c1c3975a6663 Mario Huettel 2017-04-08 1627
441ac340169b79 Dan Murphy 2019-05-09 1628 static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1629 {
441ac340169b79 Dan Murphy 2019-05-09 1630 struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data;
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1631 u8 len_padded = DIV_ROUND_UP(cf->len, 4);
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1632 struct m_can_fifo_element fifo_element;
441ac340169b79 Dan Murphy 2019-05-09 1633 struct net_device *dev = cdev->net;
441ac340169b79 Dan Murphy 2019-05-09 1634 struct sk_buff *skb = cdev->tx_skb;
812270e5445bd1 Matt Kline 2021-08-16 @1635 struct id_and_dlc fifo_header;
812270e5445bd1 Matt Kline 2021-08-16 1636 u32 cccr, fdflags;
c1eaf8b9bd3145 Markus Schneider-Pargmann 2022-12-06 1637 u32 txfqs;
812270e5445bd1 Matt Kline 2021-08-16 1638 int err;
10c1c3975a6663 Mario Huettel 2017-04-08 1639 int putidx;
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1640
e04b2cfe61072c Marc Kleine-Budde 2021-05-05 1641 cdev->tx_skb = NULL;
e04b2cfe61072c Marc Kleine-Budde 2021-05-05 1642
10c1c3975a6663 Mario Huettel 2017-04-08 1643 /* Generate ID field for TX buffer Element */
10c1c3975a6663 Mario Huettel 2017-04-08 1644 /* Common to all supported M_CAN versions */
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1645 if (cf->can_id & CAN_EFF_FLAG) {
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1646 fifo_element.id = cf->can_id & CAN_EFF_MASK;
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1647 fifo_element.id |= TX_BUF_XTD;
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1648 } else {
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1649 fifo_element.id = ((cf->can_id & CAN_SFF_MASK) << 18);
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1650 }
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1651
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1652 if (cf->can_id & CAN_RTR_FLAG)
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1653 fifo_element.id |= TX_BUF_RTR;
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1654
441ac340169b79 Dan Murphy 2019-05-09 1655 if (cdev->version == 30) {
10c1c3975a6663 Mario Huettel 2017-04-08 1656 netif_stop_queue(dev);
10c1c3975a6663 Mario Huettel 2017-04-08 1657
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1658 fifo_element.dlc = can_fd_len2dlc(cf->len) << 16;
80646733f11c2e Dong Aisheng 2014-11-18 1659
812270e5445bd1 Matt Kline 2021-08-16 1660 /* Write the frame ID, DLC, and payload to the FIFO element. */
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1661 err = m_can_fifo_write(cdev, 0, M_CAN_FIFO_ID, &fifo_element, 2);
e39381770ec9ca Matt Kline 2021-08-16 1662 if (err)
e39381770ec9ca Matt Kline 2021-08-16 1663 goto out_fail;
e39381770ec9ca Matt Kline 2021-08-16 1664
812270e5445bd1 Matt Kline 2021-08-16 1665 err = m_can_fifo_write(cdev, 0, M_CAN_FIFO_DATA,
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1666 cf->data, len_padded);
e39381770ec9ca Matt Kline 2021-08-16 1667 if (err)
e39381770ec9ca Matt Kline 2021-08-16 1668 goto out_fail;
80646733f11c2e Dong Aisheng 2014-11-18 1669
441ac340169b79 Dan Murphy 2019-05-09 1670 if (cdev->can.ctrlmode & CAN_CTRLMODE_FD) {
441ac340169b79 Dan Murphy 2019-05-09 1671 cccr = m_can_read(cdev, M_CAN_CCCR);
20779943a080c5 Torin Cooper-Bennun 2021-05-04 1672 cccr &= ~CCCR_CMR_MASK;
80646733f11c2e Dong Aisheng 2014-11-18 1673 if (can_is_canfd_skb(skb)) {
80646733f11c2e Dong Aisheng 2014-11-18 1674 if (cf->flags & CANFD_BRS)
20779943a080c5 Torin Cooper-Bennun 2021-05-04 1675 cccr |= FIELD_PREP(CCCR_CMR_MASK,
20779943a080c5 Torin Cooper-Bennun 2021-05-04 1676 CCCR_CMR_CANFD_BRS);
80646733f11c2e Dong Aisheng 2014-11-18 1677 else
20779943a080c5 Torin Cooper-Bennun 2021-05-04 1678 cccr |= FIELD_PREP(CCCR_CMR_MASK,
20779943a080c5 Torin Cooper-Bennun 2021-05-04 1679 CCCR_CMR_CANFD);
80646733f11c2e Dong Aisheng 2014-11-18 1680 } else {
20779943a080c5 Torin Cooper-Bennun 2021-05-04 1681 cccr |= FIELD_PREP(CCCR_CMR_MASK, CCCR_CMR_CAN);
80646733f11c2e Dong Aisheng 2014-11-18 1682 }
441ac340169b79 Dan Murphy 2019-05-09 1683 m_can_write(cdev, M_CAN_CCCR, cccr);
80646733f11c2e Dong Aisheng 2014-11-18 1684 }
441ac340169b79 Dan Murphy 2019-05-09 1685 m_can_write(cdev, M_CAN_TXBTIE, 0x1);
2e8e79c416aae1 Marc Kleine-Budde 2022-03-17 1686
2e8e79c416aae1 Marc Kleine-Budde 2022-03-17 1687 can_put_echo_skb(skb, dev, 0, 0);
2e8e79c416aae1 Marc Kleine-Budde 2022-03-17 1688
441ac340169b79 Dan Murphy 2019-05-09 1689 m_can_write(cdev, M_CAN_TXBAR, 0x1);
10c1c3975a6663 Mario Huettel 2017-04-08 1690 /* End of xmit function for version 3.0.x */
10c1c3975a6663 Mario Huettel 2017-04-08 1691 } else {
10c1c3975a6663 Mario Huettel 2017-04-08 1692 /* Transmit routine for version >= v3.1.x */
10c1c3975a6663 Mario Huettel 2017-04-08 1693
c1eaf8b9bd3145 Markus Schneider-Pargmann 2022-12-06 1694 txfqs = m_can_read(cdev, M_CAN_TXFQS);
c1eaf8b9bd3145 Markus Schneider-Pargmann 2022-12-06 1695
10c1c3975a6663 Mario Huettel 2017-04-08 1696 /* Check if FIFO full */
c1eaf8b9bd3145 Markus Schneider-Pargmann 2022-12-06 1697 if (_m_can_tx_fifo_full(txfqs)) {
10c1c3975a6663 Mario Huettel 2017-04-08 1698 /* This shouldn't happen */
10c1c3975a6663 Mario Huettel 2017-04-08 1699 netif_stop_queue(dev);
10c1c3975a6663 Mario Huettel 2017-04-08 1700 netdev_warn(dev,
10c1c3975a6663 Mario Huettel 2017-04-08 1701 "TX queue active although FIFO is full.");
441ac340169b79 Dan Murphy 2019-05-09 1702
441ac340169b79 Dan Murphy 2019-05-09 1703 if (cdev->is_peripheral) {
f524f829b75a7d Dan Murphy 2019-05-09 1704 kfree_skb(skb);
f524f829b75a7d Dan Murphy 2019-05-09 1705 dev->stats.tx_dropped++;
f524f829b75a7d Dan Murphy 2019-05-09 1706 return NETDEV_TX_OK;
f524f829b75a7d Dan Murphy 2019-05-09 1707 } else {
10c1c3975a6663 Mario Huettel 2017-04-08 1708 return NETDEV_TX_BUSY;
10c1c3975a6663 Mario Huettel 2017-04-08 1709 }
f524f829b75a7d Dan Murphy 2019-05-09 1710 }
10c1c3975a6663 Mario Huettel 2017-04-08 1711
10c1c3975a6663 Mario Huettel 2017-04-08 1712 /* get put index for frame */
c1eaf8b9bd3145 Markus Schneider-Pargmann 2022-12-06 1713 putidx = FIELD_GET(TXFQS_TFQPI_MASK, txfqs);
812270e5445bd1 Matt Kline 2021-08-16 1714
812270e5445bd1 Matt Kline 2021-08-16 1715 /* Construct DLC Field, with CAN-FD configuration.
812270e5445bd1 Matt Kline 2021-08-16 1716 * Use the put index of the fifo as the message marker,
812270e5445bd1 Matt Kline 2021-08-16 1717 * used in the TX interrupt for sending the correct echo frame.
812270e5445bd1 Matt Kline 2021-08-16 1718 */
10c1c3975a6663 Mario Huettel 2017-04-08 1719
10c1c3975a6663 Mario Huettel 2017-04-08 1720 /* get CAN FD configuration of frame */
10c1c3975a6663 Mario Huettel 2017-04-08 1721 fdflags = 0;
10c1c3975a6663 Mario Huettel 2017-04-08 1722 if (can_is_canfd_skb(skb)) {
10c1c3975a6663 Mario Huettel 2017-04-08 1723 fdflags |= TX_BUF_FDF;
10c1c3975a6663 Mario Huettel 2017-04-08 1724 if (cf->flags & CANFD_BRS)
10c1c3975a6663 Mario Huettel 2017-04-08 1725 fdflags |= TX_BUF_BRS;
10c1c3975a6663 Mario Huettel 2017-04-08 1726 }
10c1c3975a6663 Mario Huettel 2017-04-08 1727
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1728 fifo_element.dlc = FIELD_PREP(TX_BUF_MM_MASK, putidx) |
e39381770ec9ca Matt Kline 2021-08-16 1729 FIELD_PREP(TX_BUF_DLC_MASK, can_fd_len2dlc(cf->len)) |
e39381770ec9ca Matt Kline 2021-08-16 1730 fdflags | TX_BUF_EFC;
10c1c3975a6663 Mario Huettel 2017-04-08 1731
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1732 memcpy_and_pad(fifo_element.data, CANFD_MAX_DLEN, &cf->data,
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1733 cf->len, 0);
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1734
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1735 err = m_can_fifo_write(cdev, putidx, M_CAN_FIFO_ID,
9a10b9d9f7e946 Markus Schneider-Pargmann 2023-06-21 1736 &fifo_element, 2 + len_padded);
e39381770ec9ca Matt Kline 2021-08-16 1737 if (err)
e39381770ec9ca Matt Kline 2021-08-16 1738 goto out_fail;
10c1c3975a6663 Mario Huettel 2017-04-08 1739
10c1c3975a6663 Mario Huettel 2017-04-08 1740 /* Push loopback echo.
10c1c3975a6663 Mario Huettel 2017-04-08 1741 * Will be looped back on TX interrupt based on message marker
10c1c3975a6663 Mario Huettel 2017-04-08 1742 */
1dcb6e57db8334 Vincent Mailhol 2021-01-11 1743 can_put_echo_skb(skb, dev, putidx, 0);
10c1c3975a6663 Mario Huettel 2017-04-08 1744
10c1c3975a6663 Mario Huettel 2017-04-08 1745 /* Enable TX FIFO element to start transfer */
441ac340169b79 Dan Murphy 2019-05-09 1746 m_can_write(cdev, M_CAN_TXBAR, (1 << putidx));
10c1c3975a6663 Mario Huettel 2017-04-08 1747
10c1c3975a6663 Mario Huettel 2017-04-08 1748 /* stop network queue if fifo full */
441ac340169b79 Dan Murphy 2019-05-09 1749 if (m_can_tx_fifo_full(cdev) ||
10c1c3975a6663 Mario Huettel 2017-04-08 1750 m_can_next_echo_skb_occupied(dev, putidx))
10c1c3975a6663 Mario Huettel 2017-04-08 1751 netif_stop_queue(dev);
10c1c3975a6663 Mario Huettel 2017-04-08 1752 }
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1753
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1754 return NETDEV_TX_OK;
e39381770ec9ca Matt Kline 2021-08-16 1755
e39381770ec9ca Matt Kline 2021-08-16 1756 out_fail:
e39381770ec9ca Matt Kline 2021-08-16 1757 netdev_err(dev, "FIFO write returned %d\n", err);
e39381770ec9ca Matt Kline 2021-08-16 1758 m_can_disable_all_interrupts(cdev);
e39381770ec9ca Matt Kline 2021-08-16 1759 return NETDEV_TX_BUSY;
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1760 }
e0d1f4816f2a7e Dong Aisheng 2014-07-16 1761
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Simon,
On Wed, Jun 21, 2023 at 04:19:06PM +0200, Simon Horman wrote:
> On Wed, Jun 21, 2023 at 11:23:39AM +0200, Markus Schneider-Pargmann wrote:
> > Combine header and data before writing to the transmit fifo to reduce
> > the overhead for peripheral chips.
> >
> > Signed-off-by: Markus Schneider-Pargmann <[email protected]>
> > ---
> > drivers/net/can/m_can/m_can.c | 34 +++++++++++++++++++++-------------
> > 1 file changed, 21 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index a5003435802b..5251073987ee 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -317,6 +317,12 @@ struct id_and_dlc {
> > u32 dlc;
> > };
> >
> > +struct m_can_fifo_element {
> > + u32 id;
> > + u32 dlc;
> > + u8 data[CANFD_MAX_DLEN];
> > +};
> > +
> > static inline u32 m_can_read(struct m_can_classdev *cdev, enum m_can_reg reg)
> > {
> > return cdev->ops->read_reg(cdev, reg);
> > @@ -1622,6 +1628,8 @@ static int m_can_next_echo_skb_occupied(struct net_device *dev, int putidx)
> > static netdev_tx_t m_can_tx_handler(struct m_can_classdev *cdev)
> > {
> > struct canfd_frame *cf = (struct canfd_frame *)cdev->tx_skb->data;
> > + u8 len_padded = DIV_ROUND_UP(cf->len, 4);
> > + struct m_can_fifo_element fifo_element;
> > struct net_device *dev = cdev->net;
> > struct sk_buff *skb = cdev->tx_skb;
> > struct id_and_dlc fifo_header;
>
> Hi Markus,
>
> GCC 12.3.0 complains that fifo_header is not (no longer) used.
>
> drivers/net/can/m_can/m_can.c:1635:20: warning: unused variable 'fifo_header' [-Wunused-variable]
> struct id_and_dlc fifo_header;
Yes, I moved everything to fifo_element on purpose and then forgot to
remove fifo_header. Removing it now.
Thanks,
Markus
>
> --
> pw-bot: changes-requested
>