Add Loongson CAN-FD controller driver
Subsystems (2)
v4 cover Add Loongson CAN-FD controller driver
Hi all:
This patchset adds support for the CAN-FD controller found on Loongson CPUs.
Patch 1 introduces the basic CAN-FD controller driver with support for classic CAN and CAN FD, including bit timing, error handling, NAPI-based RX, and multiple TX buffers.
Patch 2 adds optional DMA support for RX path using the Loongson APB CMC DMA engine, which significantly reduces CPU load under high receive throughput.
The driver has been tested on Loongson-2K3000 platforms with various CAN/CAN FD traffic patterns.
Finally, I'd like to thank Bingxiong, the original author of this driver, for his efforts in working on the patch.
Thanks.
Binbin
=========
V4:
- Fix some issues from sashiko.
Patch (1/2):
- Update commit message;
- Use common APIs to calculate DLC;
- Correct CAN_RTR_FLAG judgement;
- Add skb check for CAN_STATE_ERROR_ACTIVE case;
- Use regmap_write() rather than regmap_update_bits() to disable all interrupts;
- Add napi_disable() in error paths;
- Add clock-frequency return value check;
- More comments for macros.
Link to V3:
https://lore.kernel.org/all/[email protected]/
V3:
Patch (1/2):
- Replace the switch-case statement with a calculated mask in loongson_canfd_get_txbuf_sts();
- Change loongson_canfd_reset() return type to void;
- Drop old code in loongson_canfd_set_btr();
- Rename loongson_canfd_set_mode() to loongson_canfd_set_conf_mode();
- Drop cf->__res* usage;
- Drop unnecessary macro definitions;
- Add netif_napi_del() in probe error route path;
- Replace ternary operator with if-else;
- Drop stats->rx_packets and stats->rx_bytes count in CAN error frames;
- Add ethtool_ops;
- Add CAN_CTRLMODE_CC_LEN8_DLC;
Patch (2/2):
- Fix W=2 warning;
- Rename get_rx_frc() -> get_rxbuf_empty();
- Put loongson_canfd_rxdma_remove() before free_candev();
- Adjust the RXDMA probe process.
Link to V2:
https://lore.kernel.org/all/[email protected]/
V2:
Patch (1/2):
- Put all code into one file;
- Add COMPILE_TEST Kconfig option;
- Rewrite Kconfig description;
- Use regmap_test_bits() to simplify bit field checks;
- Drop odd FIELD_GET() usage;
- Don't use FIELD_GET() for a single bit;
- Use an if/else instead of the ternary operator;
- Use the CAN TDC framework to get the SSP value;
- Use guard(spinlock)/scoped_guard(spinlock);
- Use netdev_debug() to be less verboss;
- Check for memory allocation failure;
- Add CAN_ERR_CNT flag;
- Drop unused REG_DATA_xx_yy_W_DATA_yy definition;
- Add more function comment;
Link to V1:
https://lore.kernel.org/all/[email protected]/
Binbin Zhou (2):
can: Add Loongson CAN-FD controller driver
can: loongson_canfd: Add RXDMA support
MAINTAINERS | 7 + drivers/net/can/Kconfig | 11 + drivers/net/can/Makefile | 1 + drivers/net/can/loongson_canfd.c | 1853 ++++++++++++++++++++++++++++++ 4 files changed, 1872 insertions(+) create mode 100644 drivers/net/can/loongson_canfd.c
base-commit: b266bacba796ff5c4dcd2ae2fc08aacf7ab39153
Signature · 1 line
2.52.0
PATCH v4 1/2 can: Add Loongson CAN-FD controller driver
Add a new driver for the Loongson CAN-FD controller, which is integrated into Loongson-2K series SoCs. The controller supports both CAN 2.0 and CAN-FD protocols, with up to 8 TX buffers and a flexible RX buffer.
The driver implements the core CAN device operations:
- NAPI-based receive path with interrupt masking to reduce overhead;
- TX echo support for accurate loopback and echo skb handling;
- Error reporting (warning, passive, bus-off) with detailed status;
- Support for common CAN controller modes: loopback, listen-only, one-shot, FD, and non-ISO FD;
- Auto TDC (Transmitter Delay Compensation) for FD data phase.
--- MAINTAINERS | 7 + drivers/net/can/Kconfig | 11 + drivers/net/can/Makefile | 1 + drivers/net/can/loongson_canfd.c | 1688 ++++++++++++++++++++++++++++++ 4 files changed, 1707 insertions(+) create mode 100644 drivers/net/can/loongson_canfd.c
MAINTAINERS +7 -0
@@ -15042,6 +15042,13 @@ F: arch/loongarch/ F: drivers/*/*loongarch* F: drivers/cpufreq/loongson3_cpufreq.c +LOONGSON CAN FD DRIVER +M: Bingxiong Li <[email protected]> +M: Binbin Zhou <[email protected]> +L: [email protected] +S: Maintained +F: drivers/net/can/loongson_canfd/ + LOONGSON GPIO DRIVER M: Yinbo Zhu <[email protected]> L: [email protected]
drivers/net/can/Kconfig +11 -0
@@ -189,6 +189,17 @@ config CAN_KVASER_PCIEFD Kvaser M.2 PCIe 4xCAN Kvaser PCIe 8xCAN +config CAN_LOONGSON_CANFD + tristate "Loongson CAN-FD controller" + depends on HAS_IOMEM + select REGMAP_MMIO + help + This is a canfd driver switch for the Loongson platform, + integrated with the Loongson-2K series SoCs. + + To compile as a module, choose M here: the module will be + called loongson_canfd. + config CAN_SLCAN tristate "Serial / USB serial CAN Adaptors (slcan)" depends on TTY
drivers/net/can/Makefile +1 -0
@@ -27,6 +27,7 @@ obj-$(CONFIG_CAN_GRCAN) += grcan.o obj-$(CONFIG_CAN_IFI_CANFD) += ifi_canfd/ obj-$(CONFIG_CAN_JANZ_ICAN3) += janz-ican3.o obj-$(CONFIG_CAN_KVASER_PCIEFD) += kvaser_pciefd/ +obj-$(CONFIG_CAN_LOONGSON_CANFD) += loongson_canfd.o obj-$(CONFIG_CAN_MSCAN) += mscan/ obj-$(CONFIG_CAN_M_CAN) += m_can/ obj-$(CONFIG_CAN_PEAK_PCIEFD) += peak_canfd/
drivers/net/can/loongson_canfd.c new file +1688 -0
1,688 lines not on this page · raw message
Signature · 1 line
2.52.0
[email protected]
AI reviewRe: [PATCH v4 1/2] can: Add Loongson CAN-FD controller driver
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider: - [High] Lockless queue stop in `start_xmit` races with queue wake in the TX interrupt, risking a…
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [High] Lockless queue stop in start_xmit races with queue wake in the TX interrupt, risking a permanent deadlock.
- [High] Failing to allocate an SKB in the RX path causes early return without draining the hardware FIFO, permanently desynchronizing the RX stream and hanging NAPI.
- [High] Clearing the TX Hardware Command Interrupt flag at the end of loongson_canfd_tx_interrupt() creates a race condition leading to lost TX interrupts.
- [High] The driver immediately clears hardware error counters upon entering BUS_OFF, bypassing the mandatory software restart delay.
- [High] Overwriting the isr variable in loongson_canfd_err_interrupt() drops concurrent error interrupts and leaves them permanently masked.
- [High] Using the logical cfd->len to pop payload data from the RX FIFO permanently corrupts the hardware FIFO state for CAN 2.0 RTR frames.
- [High] Incorrect constraints and silent truncation for CAN FD data phase timing lead to corrupted bit-timing configurations.
- [High] Missing volatile register definition for LOONGSON_CANFD_TRV_DLY causes stale cached TDCV readings.
- [Medium] The queue space check incorrectly evaluates if ALL transmit buffers are free instead of AT LEAST ONE, severely throttling transmission.
- [Medium] The driver passes a hardcoded 0 frame length to can_put_echo_skb(), preventing the tx_bytes statistic from ever incrementing.
- [Medium] Synchronous polling of hardware status in TX interrupt causes runaway double counting of the tx_packets statistic.
--
--- Patch [1]: [PATCH v4 1/2] can: Add Loongson CAN-FD controller driver --- commit ecb7c7be40459261656c18cd36d4c01fa4e29b1d
Author: Binbin Zhou <[email protected]>
Subject: can: Add Loongson CAN-FD controller driver
This commit adds a new driver for the Loongson CAN-FD controller integrated into Loongson-2K series SoCs. It implements the core CAN device operations, including a NAPI-based receive path, TX echo support, error reporting, and various CAN controller modes.
Quoted 6 lines
drivers/net/can/loongson_canfd.c new file +0
-0@@ -0,0 +1,1688 @@
[ ... ]
drivers/net/can/loongson_canfd.c line 405
Quoted 6 lines
drivers/net/can/loongson_canfd.c new file +6
-0+static const struct can_bittiming_const loongson_canfd_bit_timing_data = { + .name =DEV_NAME , + .tseg1_min =2 , + .tseg1_max =190 , + .tseg2_min =2 , + .tseg2_max =63 ,
[Severity: High]
Are these data phase bounds correct for the FD hardware? The struct loongson_canfd_bit_timing_data advertises tseg1_max as 190 and tseg2_max as 63. However, looking at loongson_canfd_set_btr(), the phase segments are passed to FIELD_PREP(REG_BTR_FD_PH1...) which is bounded by a 5-bit mask (GENMASK(11, 7)). Will values greater than 31 be silently truncated here?
Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5
-0+ .sjw_max = 31, + .brp_min = 1, + .brp_max = 255, + .brp_inc = 1, +};
[ ... ]
drivers/net/can/loongson_canfd.c line 444
Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +4
-0+static bool loongson_canfd_txbuf_freed (struct loongson_canfd_priv *priv) +{ +return !regmap_test_bits (priv->regmap,LOONGSON_CANFD_TX_STAT ,REG_TX_STAT_BRP ); +}
[Severity: Medium]
Does this check properly evaluate if a transmit buffer is free? The use of !regmap_test_bits() requires all 8 Buffer Request Pending bits to be 0 for the function to return true. In loongson_canfd_start_xmit(), this leads to the queue being stopped immediately after a single frame is queued, effectively reducing the 8-buffer queue down to a depth of 1.
[ ... ]
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+static netdev_tx_t loongson_canfd_start_xmit (struct sk_buff *skb,struct net_device *ndev) +{
[ ... ]
drivers/net/can/loongson_canfd.c line 928
Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5
-0+if (unlikely (!loongson_canfd_txbuf_freed (priv))) { +netif_stop_queue (ndev); +netdev_err (ndev,"BUG!, no TXB free when queue awake!\n" ); +return NETDEV_TX_BUSY ; + }
[Severity: High]
Is this check safe from a race condition with the TX interrupt? The queue status is read locklessly outside of tx_lock. If this detects a full queue, but gets preempted before netif_stop_queue() is called, a concurrent TX interrupt could free a buffer and call netif_wake_queue().
When this thread resumes, it would stop the queue despite free space being available, potentially resulting in a permanent stall.
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+ +guard (spinlock_irqsave)(&priv->tx_lock);
[ ... ]
drivers/net/can/loongson_canfd.c line 952
Quoted 1 line
drivers/net/can/loongson_canfd.c new file +1
-0+can_put_echo_skb (skb, ndev, buf_id,0 );
[Severity: Medium]
Is it intentional to pass 0 as the frame length to can_put_echo_skb()? When the TX interrupt later fires, can_get_echo_skb() will return this length of 0, meaning stats->tx_bytes will never increment.
Quoted 9 lines
drivers/net/can/loongson_canfd.c new file +9
-0+ +loongson_canfd_set_txbuf_cmd (ndev,TXT_CMD_ADD , buf_id); + +/* Check if all TX buffers are full */ +if (!loongson_canfd_txbuf_freed (priv)) +netif_stop_queue (ndev); + +return NETDEV_TX_OK ; +}
[ ... ]
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+static int loongson_canfd_rx (struct net_device *ndev) +{
[ ... ]
drivers/net/can/loongson_canfd.c line 1001
Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10
-0+/* Flexible Data-rate Format */ +if (is_can_fd) { + skb =alloc_canfd_skb (ndev, &cfd); + }else { + skb =alloc_can_skb (ndev, &ccf); + cfd = (struct canfd_frame *)ccf; + } + +if (unlikely (!skb)) +return 0 ;
[Severity: High]
Can returning 0 here when skb allocation fails corrupt the hardware FIFO state? This early return skips popping the remaining payload words from the FIFO. The next read operation might then mistakenly interpret payload data as frame headers.
Additionally, returning 0 breaks the NAPI polling loop in loongson_canfd_rx_napi(), which will then exit without calling napi_complete_done() when ret == 0, potentially hanging the receive path.
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+ +/* Extended Identifier Type */
[ ... ]
drivers/net/can/loongson_canfd.c line 1031
Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10
-0+/* Data Length Code */ + dlc =FIELD_GET (REG_FRAME_META1_DLC , meta1); +if (is_can_fd) + cfd->len =can_fd_dlc2len (dlc); +else +can_frame_set_cc_len (ccf, dlc, priv->can.ctrlmode); + +/* Data */ +for (unsigned int i =0 ; i < cfd->len; i +=LOONGSON_CANFD_DW_BYTE ) +regmap_read (priv->regmap,LOONGSON_CANFD_RX_DATA , (u32 *)(cfd->data + i));
[Severity: High]
Does iterating based on cfd->len safely handle CAN 2.0 RTR frames? For Remote Transmission Request frames, cfd->len specifies the requested length (up to 8) even though there is no physical payload on the wire. Reading from the FIFO based on this length could consume data belonging to the next frame, breaking stream synchronization. Would it be safer to read exactly REG_FRAME_META1_RWCNT words?
Quoted 8 lines
drivers/net/can/loongson_canfd.c new file +8
-0+ +if (!(cfd->can_id &CAN_RTR_FLAG )) + stats->rx_bytes += cfd->len; + stats->rx_packets++; +netif_receive_skb (skb); + +return 1 ; +}
[ ... ]
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+static void loongson_canfd_err_interrupt (struct net_device *ndev,u32 isr) +{
[ ... ]
Quoted 4 lines
drivers/net/can/loongson_canfd.c new file +4
-0+/* +* EWLI: error warning limit condition met +* FCSI: fault confinement state changed +* ALI: arbitration lost (just informative)
drivers/net/can/loongson_canfd.c line 1160
Quoted 10 lines
drivers/net/can/loongson_canfd.c new file +10
-0+* BEI: bus error interrupt +*/ +if ((isr ®_INT_STAT_FCSI ) || (isr ®_INT_STAT_EWLI )) { +netdev_info (ndev,"state changes from %s to %s\n" , +can_get_state_str (priv->can.state),can_get_state_str (state)); + +if (priv->can.state == state) +netdev_warn (ndev,"cur and pre state is the same!(miss intr?)\n" ); + + isr =REG_INT_STAT_FCSI |REG_INT_STAT_EWLI ;
[Severity: High]
Does overwriting the isr variable here drop concurrent error interrupts? If multiple errors occur simultaneously, this assignment destroys the other flags (like REG_INT_STAT_ALI).
Because this modified isr variable is later used to write back to LOONGSON_CANFD_INT_MASK at the end of the function, the dropped interrupts could remain permanently masked in the hardware.
drivers/net/can/loongson_canfd.c line 1167
Quoted 7 lines
drivers/net/can/loongson_canfd.c new file +7
-0+ priv->can.state = state; +switch (state) { +case CAN_STATE_BUS_OFF : + priv->can.can_stats.bus_off++; +if (priv->can.restart_ms) +regmap_write (priv->regmap,LOONGSON_CANFD_CMD , +REG_CMD_ERCRST );
[Severity: High]
Does clearing the error counters here bypass the software recovery protocol? The REG_CMD_ERCRST command instantly zeroes the hardware TEC/REC counters, which could cause the controller to immediately rejoin the bus, bypassing the mandatory CAN subsystem recovery delay specified by priv->can.restart_ms.
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+ +can_bus_off (ndev);
[ ... ]
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+static void loongson_canfd_tx_interrupt (struct net_device *ndev) +{
[ ... ]
Quoted 8 lines
drivers/net/can/loongson_canfd.c new file +8
-0+while ((buf_id =loongson_canfd_get_txbuf_id (priv, &sts)) <LOONGSON_CANFD_TXBUF_NUM ) { +switch (sts) { +case TX_BS_VALID : + stats->tx_bytes +=can_get_echo_skb (ndev, buf_id,NULL ); + stats->tx_packets++; +break ; +case TX_BS_FAIL : +u32 cnt;
[ ... ]
drivers/net/can/loongson_canfd.c line 1338
Quoted 8 lines
drivers/net/can/loongson_canfd.c new file +8
-0+ stats->tx_dropped++; +break ; +default : +break ; + } + +loongson_canfd_set_txbuf_cmd (ndev,TXT_CMD_SR_CLEAN , buf_id); + }
[Severity: Medium]
Can this loop sequence artificially inflate the tx_packets statistic? The code issues a TXT_CMD_SR_CLEAN command and immediately continues the loop without verifying the status cleared.
Since hardware register updates take time, the very next regmap_read() in loongson_canfd_get_txbuf_id() could yield the same TX_BS_VALID status. The buffer would then be processed again, safely returning 0 from can_get_echo_skb(), but unconditionally incrementing stats->tx_packets over and over.
drivers/net/can/loongson_canfd.c line 1345
Quoted 7 lines
drivers/net/can/loongson_canfd.c new file +7
-0+ +/* +* Clear the interrupt again. We do not want to receive again interrupt +* for the buffer already handled. If it is the last finished one then +* it would cause log of spurious interrupt. +*/ +regmap_write (priv->regmap,LOONGSON_CANFD_INT_STAT ,REG_INT_STAT_TXBHCI );
[Severity: High]
Is it safe to clear the hardware interrupt flag after processing the buffers instead of before? If a new buffer finishes transmission between the end of the loop and this regmap_write, the newly set interrupt flag would be cleared without getting processed, potentially causing a lost interrupt.
Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5
-0+ +/* Check if at least one TX buffer is free */ +if (loongson_canfd_txbuf_freed (priv)) +netif_wake_queue (ndev); +}
[ ... ]
drivers/net/can/loongson_canfd.c line 1545
Quoted 5 lines
drivers/net/can/loongson_canfd.c new file +5
-0+static bool loongson_canfd_volatile_reg (struct device *dev,unsigned int reg) +{ +switch (reg) { +case LOONGSON_CANFD_MODE : +case LOONGSON_CANFD_CONF :
[Severity: High]
Should LOONGSON_CANFD_TRV_DLY be included in this volatile register list? Since regmap uses REGCACHE_MAPLE, omitting this register causes the first read in loongson_canfd_get_auto_tdcv() to be cached indefinitely. This would continuously feed stale TDCV readings into the auto-TDC logic.
Quoted 2 lines
drivers/net/can/loongson_canfd.c new file +2
-0+case LOONGSON_CANFD_STAT : +case LOONGSON_CANFD_INT_STAT :
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
PATCH v4 2/2 can: loongson_canfd: Add RXDMA support
In reply to Binbin ZhouExtend the driver with an optional DMA engine support for the receive path. When a DMA channel named rx is available (e.g., from the loongson2-apb-cmc-dma driver), the RX buffer is mapped to a cyclic DMA transfer that continuously fills a pre-allocated ring buffer. The driver then readds incoming CAN frames directly from the DMA buffer, reducing the number of MMIO accesses and improving performance under high traffic.
The DMA mode is completely transparent to the under networking stack; the driver automatically selects the appropriate RX data reading method based on the availability of the DMA channel.
--- drivers/net/can/Kconfig | 2 +- drivers/net/can/loongson_canfd.c | 197 ++++++++++++++++++++++++++++--- 2 files changed, 182 insertions(+), 17 deletions(-)
drivers/net/can/Kconfig +1 -1
@@ -191,7 +191,7 @@ config CAN_KVASER_PCIEFD config CAN_LOONGSON_CANFD tristate "Loongson CAN-FD controller" - depends on HAS_IOMEM + depends on HAS_IOMEM && (LOONGSON2_APB_CMC_DMA || COMPILE_TEST) select REGMAP_MMIO help This is a canfd driver switch for the Loongson platform,
drivers/net/can/loongson_canfd.c +181 -16
314 lines not on this page · raw message
Signature · 1 line
2.52.0
[email protected]
AI reviewRe: [PATCH v4 2/2] can: loongson_canfd: Add RXDMA support
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [High] The CPU reads ahead of the DMA write pointer, leading to frame corruption and ring buffer desynchronization.
- [High] A DMA status error causes an infinite NAPI polling loop, leading to a soft lockup.
- [High] Failure of loongson_canfd_rxdma_init leads to a double free of the DMA buffer and completely breaks the RX path.
- [Medium] Kconfig dependency strictly requires the Loongson DMA controller, breaking the driver for polling mode users.
- [Medium] The network device is leaked if dma_request_chan() returns -EPROBE_DEFER.
- [Low] Build failure due to undeclared variable 'cf'.
- [Low] Improper internal access of dma_chan->cookie field violates the DMA engine API.
Pre-existing issues:
- [High] An SKB allocation failure in loongson_canfd_rx leaves payload data in the buffer, permanently desynchronizing the RX stream. --
--- Patch [2]: [PATCH v4 2/2] can: loongson_canfd: Add RXDMA support --- commit 1ade4f6818d2a045c4fbf2074c44add29566cfc4
Author: Binbin Zhou <[email protected]>
Subject: can: loongson_canfd: Add RXDMA support
This commit adds optional DMA engine support for the receive path in the Loongson CAN-FD driver. It configures a cyclic DMA transfer to continuously fill a ring buffer, falling back to polling if DMA is unavailable.
drivers/net/can/Kconfig config CAN_KVASER_PCIEFD · line 194
Quoted 9 lines
drivers/net/can/Kconfig +1
-1@@ -191,7 +191,7 @@ config CAN_KVASER_PCIEFD config CAN_LOONGSON_CANFD tristate "Loongson CAN-FD controller" - depends on HAS_IOMEM + depends on HAS_IOMEM && (LOONGSON2_APB_CMC_DMA || COMPILE_TEST)
[Severity: Medium]
The commit message notes that DMA is optional and the driver will fall back to poll mode. Does modifying the Kconfig to strictly depend on LOONGSON2_APB_CMC_DMA prevent users from building the driver on platforms where they only want polling mode?
Quoted 3 lines
drivers/net/can/loongson_canfd.c +0
-0
[ ... ]
drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 595
Quoted 6 lines
drivers/net/can/loongson_canfd.c +6
-0+static bool loongson_canfd_get_rxbuf_empty_in_dma (struct loongson_canfd_priv *priv) +{ +struct dma_tx_state state; +enum dma_status status; + + status =dmaengine_tx_status (priv->rx_ch, priv->rx_ch->cookie, &state);
[Severity: Low]
Is it safe to directly access the internal cookie field of the dma_chan structure? The DMA engine API typically requires clients to track the cookie returned by dmaengine_submit() rather than dereferencing the internal framework structures.
drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 599
Quoted 4 lines
drivers/net/can/loongson_canfd.c +4
-0+ +return priv->last_res == (state.residue /DMA_SLAVE_BUSWIDTH_4_BYTES ) && + (status ==DMA_IN_PROGRESS ); +}
[Severity: High]
Could this empty check cause the CPU to read ahead of the DMA write pointer?
In loongson_canfd_rx_napi(), the buffer is considered non-empty as soon as the DMA engine transfers a single word (since last_res will no longer equal the residue calculation).
When NAPI calls loongson_canfd_rx(), it unconditionally reads multiple words for the header and payload. Since the DMA engine might have only written one word so far, does this consume stale data and corrupt the stream?
[Severity: High]
What happens if the DMA engine status is not DMA_IN_PROGRESS due to a hardware error or a halted transfer?
This function will return false, indicating the buffer is not empty. The NAPI loop in loongson_canfd_rx_napi() will continuously poll loongson_canfd_rx().
Because the empty check evaluates to false, it skips napi_complete_done() and the networking stack instantly reschedules NAPI. Does this lead to an infinite loop and a CPU soft lockup?
[ ... ]
Quoted 2 lines
drivers/net/can/loongson_canfd.c +2
-0+static int loongson_canfd_rxdma_init (struct net_device *ndev) +{
[ ... ]
Quoted 6 lines
drivers/net/can/loongson_canfd.c +6
-0+/* Push current dma transaction in the pending queue */ + ret =dma_submit_error (dmaengine_submit (desc)); +if (ret) { +dmaengine_terminate_sync (priv->rx_ch); + goto err_config; + }
drivers/net/can/loongson_canfd.c static void loongson_canfd_set_txbuf_cmd(struct net_device *ndev, · line 674
Quoted 10 lines
drivers/net/can/loongson_canfd.c +10
-0+ +/* Issue pending DMA requests */ +dma_async_issue_pending (priv->rx_ch); + +return 0 ; + +err_config: +loongson_canfd_rxdma_free (priv, dev); +return ret; +}
[Severity: High]
If dmaengine_slave_config or dmaengine_prep_dma_cyclic fails, this calls loongson_canfd_rxdma_free() but leaves priv->rx_ch pointing to the freed channel.
Since the probe function ignores the error to fall back to poll mode, will loongson_canfd_chip_start() mistakenly enable the DMADI interrupt instead of RBNEI because priv->rx_ch is not NULL?
Furthermore, when the driver is removed, loongson_canfd_rxdma_remove() checks priv->rx_ch and calls loongson_canfd_rxdma_free() again. Does this result in a fatal double-free?
[ ... ]
Quoted 1 line
drivers/net/can/loongson_canfd.c +0
-0@@ -1028,7 +1171,7 @@ static int loongson_canfd_rx(struct net_device *ndev)
[ ... ]
drivers/net/can/loongson_canfd.c
Quoted 9 lines
drivers/net/can/loongson_canfd.c +0
-0if (is_can_fd) { skb =alloc_canfd_skb (ndev, &cfd); }else { skb =alloc_can_skb (ndev, &ccf); cfd = (struct canfd_frame *)ccf; }if (unlikely (!skb))return 0 ;
[Severity: High]
This is a pre-existing issue, but if alloc_canfd_skb() or alloc_can_skb() fails in loongson_canfd_rx(), the function returns 0 immediately.
It has already consumed the meta0 and meta1 headers but skips consuming the payload words. Since these unread words remain in the hardware FIFO or DMA buffer, will the next RX interrupt misinterpret them as the headers of the next frame, permanently desynchronizing the stream?
[ ... ]
drivers/net/can/loongson_canfd.c
Quoted 11 lines
drivers/net/can/loongson_canfd.c +1
-1/* Data Length Code */ dlc =FIELD_GET (REG_FRAME_META1_DLC , meta1);if (is_can_fd) cfd->len =can_fd_dlc2len (dlc);else can_frame_set_cc_len (ccf, dlc, priv->can.ctrlmode);/* Data */ for (unsigned int i =0 ; i < cfd->len; i +=LOONGSON_CANFD_DW_BYTE ) -regmap_read (priv->regmap,LOONGSON_CANFD_RX_DATA , (u32 *)(cfd->data + i)); + *(u32 *)(cf->data + i) = priv->get_rx_data (priv);
[Severity: Low]
Does this cause a build failure? The variable cf is not declared in loongson_canfd_rx() (the available pointers are cfd and ccf). Writing to cf->data seems to be a typo that prevents the driver from compiling.
[ ... ]
Quoted 1 line
drivers/net/can/loongson_canfd.c +0
-0@@ -1635,6 +1779,24 @@ static int loongson_canfd_probe(struct platform_device *pdev)
[ ... ]
drivers/net/can/loongson_canfd.c static int loongson_canfd_probe(struct platform_device *pdev) · line 1787
Quoted 10 lines
drivers/net/can/loongson_canfd.c +6
-0ndev->netdev_ops = &loongson_canfd_netdev_ops; ndev->ethtool_ops = &loongson_canfd_ethtool_ops;SET_NETDEV_DEV (ndev, dev); + priv->get_rx_data = loongson_canfd_get_rxdata_in_poll; + priv->get_rxbuf_empty = loongson_canfd_get_rxbuf_empty_in_poll; + + priv->rx_ch =dma_request_chan (dev,"rx" ); +if (PTR_ERR (priv->rx_ch) == -EPROBE_DEFER ) +return -EPROBE_DEFER ;
[Severity: Medium]
If dma_request_chan() returns -EPROBE_DEFER, this immediately returns the error code without jumping to the error handling path.
Since alloc_candev() was successfully called earlier in the probe function, does returning directly here permanently leak the net_device and priv structures?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2