Re: [PATCH v9 1/2] can: Add Loongson CAN-FD controller driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 16 potential
issues to consider:
- [High] can: loongson_canfd: use-after-free in
  loongson_canfd_start_xmit() after echo
- [Medium] can: loongson_canfd: use of can_dropped_invalid_skb() in
  start_xmit()
- [Medium] can: loongson_canfd: CANFD_FDF cleared in
  loongson_canfd_rx_deliver()
- [Medium] can: loongson_canfd: incomplete free check in
  loongson_canfd_txbuf_freed()
- [Medium] can: loongson_canfd: TOCTOU race waking TX queue outside
  priv->tx_lock
- [Medium] can: loongson_canfd: broken error frame flags and stats in
  state/overrun paths
- [Medium] can: loongson_canfd: missing TX queue and chip stop on
  CAN_STATE_BUS_OFF
- [Medium] can: loongson_canfd: bogus error skb stats and race on
  ndev->stats
- [Medium] can: loongson_canfd: race enabling IRQs before state init in
  loongson_canfd_chip_start()
- [Medium] can: loongson_canfd: teardown race with IRQ and restart_work
  in loongson_canfd_close()
- [Medium] can: loongson_canfd: hwts capabilities hidden when down in
  loongson_canfd_get_ts_info()
- [Medium] can: loongson_canfd: broken REGCACHE_MAPLE config on banked
  TX and FIFO registers
- [Medium] can: loongson_canfd: prop_seg > 127 truncated in
  loongson_canfd_set_btr()
- [Medium] can: loongson_canfd: RX FIFO desync and tautological check in
  rx_hdr_valid()
- [Medium] can: loongson_canfd: missing mode checks in
  loongson_canfd_start_xmit()
- [Low] can: loongson_canfd: tautological SFF ID check in
  loongson_canfd_hdr_plausible()
--

commit 081a5a8aea730be1bb8b197fafbcd027960619e3
Author: Binbin Zhou <zhoubinbin@loongson.cn>

can: Add Loongson CAN-FD controller driver

This commit adds a driver for the CAN-FD controller integrated in the
Loongson-2K series SoCs, supporting CAN 2.0B, CAN FD, NAPI RX, TX echo,
error reporting, and hardware timestamps.

> diff --git a/drivers/net/can/loongson_canfd.c b/drivers/net/can/loongson_canfd.c
> new file mode 100644
> index 0000000000000..7a4d8df7815ac
> --- /dev/null
> +++ b/drivers/net/can/loongson_canfd.c

[ ... ]

> +static void loongson_canfd_set_btr(struct net_device *ndev,
> +				   struct can_bittiming *bt, bool nominal)
> +{
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +	u32 prop = bt->prop_seg, phase1 = bt->phase_seg1;
> +	u32 ph1_max = FIELD_MAX(REG_BTR_PH1);
> +	u32 btr = 0;
> +
> +	/* PROP is 7 bits but PH1 only 6; move the excess into PROP */
> +	if (phase1 > ph1_max) {
> +		prop += phase1 - ph1_max;
> +		phase1 = ph1_max;
> +	}
> +
> +	if (nominal) {
> +		btr = FIELD_PREP(REG_BTR_PROP, prop) |

[Severity: Medium]
Can FIELD_PREP(REG_BTR_PROP, prop) and FIELD_PREP(REG_BTR_FD_PROP, prop)
truncate prop when manual bit-timing parameters specify prop_seg > 127?

loongson_canfd_bit_timing and loongson_canfd_bit_timing_data advertise
tseg1_max = 190, and can_fixup_bittiming() only enforces
prop_seg + phase_seg1 <= tseg1_max.  If userspace sets prop_seg in
128..189, the excess above FIELD_MAX(REG_BTR_PROP) (127) is not moved
into phase1 and is masked off by FIELD_PREP(), programming the wrong bit
rate and sample point.

[ ... ]

> +static int loongson_canfd_chip_start(struct net_device *ndev)
> +{
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +	u32 int_ena, int_msk;
> +	int ret;
> +
> +	ret = loongson_canfd_set_bittiming(ndev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* Programs BTR_FD and SSP_CFG together */
> +	ret = loongson_canfd_set_data_bittiming(ndev);
> +	if (ret < 0)
> +		return ret;
> +
> +	loongson_canfd_apply_ctrlmode(priv);
> +
> +	int_ena = REG_INT_STAT_RBNEI | REG_INT_STAT_TXBHCI |
> +		  REG_INT_STAT_EWLI  | REG_INT_STAT_FCSI   |
> +		  REG_INT_STAT_DOI;
> +
> +	if (priv->can.ctrlmode & CAN_CTRLMODE_BERR_REPORTING)
> +		int_ena |= REG_INT_STAT_ALI | REG_INT_STAT_BEI;
> +
> +	int_msk = REG_INT_MASK_SET & ~int_ena;
> +
> +	/* Clear stale mask/ena bits left by disable_interrupts() */
> +	regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, REG_INT_MASK_CLR);
> +	regmap_write(priv->regmap, LOONGSON_CANFD_INT_ENA, REG_INT_ENA_CLR);
> +
> +	regmap_write(priv->regmap, LOONGSON_CANFD_INT_MASK, int_msk);
> +	regmap_write(priv->regmap, LOONGSON_CANFD_INT_ENA, int_ena);
> +
> +	loongson_canfd_hwts_setup(ndev);
> +
> +	priv->can.state = CAN_STATE_ERROR_ACTIVE;
> +
> +	regmap_update_bits(priv->regmap, LOONGSON_CANFD_CONF, REG_CONF_ENA,
> +			   REG_CONF_ENA);
> +
> +	return 0;
> +}

[Severity: Medium]
Can enabling LOONGSON_CANFD_INT_ENA before setting priv->can.state =
CAN_STATE_ERROR_ACTIVE trigger a spurious state change report?

CPU0                              <hardirq>
loongson_canfd_chip_start()
  regmap_write(..., INT_MASK, int_msk)
  regmap_write(..., INT_ENA, int_ena)
                                  loongson_canfd_interrupt()
                                    loongson_canfd_err_interrupt()
                                      priv->can.state != state
                                      loongson_canfd_report_state_change()
  loongson_canfd_hwts_setup()
  priv->can.state = CAN_STATE_ERROR_ACTIVE;

loongson_canfd_reset() latches REG_INT_STAT_FCSI on reset to Error Active,
and loongson_canfd_err_interrupt() relies on priv->can.state ==
CAN_STATE_ERROR_ACTIVE to ignore that initial interrupt.  Because the IRQ
is already registered in loongson_canfd_open() and
loongson_canfd_do_set_mode(), FCSI can fire while priv->can.state is still
CAN_STATE_STOPPED or CAN_STATE_BUS_OFF and before hwts is initialized.

[ ... ]

> +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 loongson_canfd_txbuf_freed() return true even when no TX buffer is
writable?

loongson_canfd_find_free_txbuf() requires a slot's BRP bit to be clear,
its 2-bit BS field to be TX_BS_IDLE, and priv->can.echo_skb[i] to be NULL.
When a frame finishes on the wire, hardware clears its BRP bit and sets BS
to TX_BS_VALID or TX_BS_FAIL until loongson_canfd_tx_interrupt() runs
TXT_CMD_SR_CLEAN.  Checking only REG_TX_STAT_BRP leaves the queue awake in
loongson_canfd_start_xmit() when the last writable slot is used while
another slot awaits ISR cleanup, causing the next start_xmit() call to
return NETDEV_TX_BUSY.

[ ... ]

> +	for (unsigned int i = 0; i < cf->len; i += LOONGSON_CANFD_DW_BYTE) {
> +		unsigned int remain = cf->len - i;
> +		u32 word;
> +
> +		if (remain >= LOONGSON_CANFD_DW_BYTE) {
> +			word = get_unaligned_le32(cf->data + i);

[Severity: Medium]
Can an ETH_P_CANXL skb cause loongson_canfd_insert_frame() to read past
skb->data and write past LOONGSON_CANFD_TX_DATA_18?

loongson_canfd_start_xmit() validates frames with
can_dropped_invalid_skb() instead of can_dev_dropped_skb().  That skips
the CAN_CTRLMODE_LISTENONLY and CAN_CTRLMODE_FD checks, allowing frames
from non-RAW sockets (such as can-bcm or can-isotp) to fill the hardware
TX buffers while in listen-only mode.  In addition, if an ETH_P_CANXL skb
is sent via AF_PACKET, cf->len aliases canxl_frame::flags (which has
CANXL_XLF = 0x80 set), so this loop reads out-of-bounds past small CAN XL
skbs and writes registers past LOONGSON_CANFD_TX_DATA_18.

[ ... ]

> +static bool loongson_canfd_hdr_plausible(u32 hdr0, u32 hdr1)
> +{
> +	u32 rwcnt  = FIELD_GET(REG_FRAME_META1_RWCNT, hdr1);
> +	u32 dlc    = FIELD_GET(REG_FRAME_META1_DLC,   hdr1);
> +	bool isfd  = !!(hdr1 & REG_FRAME_META1_FDF);
> +	bool isrtr = !!(hdr0 & REG_FRAME_META0_RTR);
> +	u32 payload_len, expected_rwcnt;
> +
> +	/* 2 header words + payload, within CAN FD limits */
> +	if (rwcnt < 2 || rwcnt > 2 + CANFD_MAX_DLEN / LOONGSON_CANFD_DW_BYTE)
> +		return false;
> +
> +	/* RTR is a Classic CAN concept */
> +	if (isfd && isrtr)
> +		return false;
> +
> +	/* Standard ID fits in 11 bits */
> +	if (!(hdr0 & REG_FRAME_META0_XDT) &&
> +	    FIELD_GET(REG_FRAME_META0_ID_BASE, hdr0) > CAN_SFF_MASK)
> +		return false;

[Severity: Low]
Can FIELD_GET(REG_FRAME_META0_ID_BASE, hdr0) ever exceed CAN_SFF_MASK?
REG_FRAME_META0_ID_BASE is GENMASK(28, 18), an 11-bit field whose maximum
value is already CAN_SFF_MASK (0x7ff), whereas REG_FRAME_META0_ID_EXT
(GENMASK(17, 0)) is not checked when REG_FRAME_META0_XDT is clear.

[ ... ]

> +static void loongson_canfd_rx_note_bad(struct net_device *ndev, u32 rwcnt)
> +{
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +	u32 words, data;
> +
> +	ndev->stats.rx_dropped++;
> +
> +	words = rwcnt > 2 ?
> +		min_t(u32, rwcnt - 2,
> +		      CANFD_MAX_DLEN / LOONGSON_CANFD_DW_BYTE) : 0;
> +	while (words--)
> +		regmap_read(priv->regmap, LOONGSON_CANFD_RX_DATA, &data);
> +}
> +
> +/**
> + * loongson_canfd_rx_hdr_valid() - Validate a frame header pair
> + * @ndev: Network device
> + * @d: Output descriptor, filled with all parsed fields
> + *
> + * Reads and sanity-checks the two header words. The caller must drain
> + * the remaining payload words when false is returned.
> + *
> + * Return: true if the header is valid, false otherwise.
> + */
> +static bool loongson_canfd_rx_hdr_valid(struct net_device *ndev,
> +					struct loongson_canfd_rx_desc *d)
> +{
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +
> +	d->hdr0 = loongson_canfd_read_rx_word_fifo(priv);
> +	d->hdr1 = loongson_canfd_read_rx_word_fifo(priv);
> +
> +	d->rwcnt = FIELD_GET(REG_FRAME_META1_RWCNT, d->hdr1);
> +	if (d->rwcnt < 2) {
> +		/* Hand the caller a worst-case drain length */
> +		d->rwcnt = 2 + CANFD_MAX_DLEN / LOONGSON_CANFD_DW_BYTE;
> +		return false;
> +	}

[Severity: Medium]
Does setting d->rwcnt to 18 when d->rwcnt < 2 desynchronize the 80-byte
hardware RX FIFO?

When d->rwcnt < 2, loongson_canfd_rx_note_bad() unconditionally reads 16
words from LOONGSON_CANFD_RX_DATA without checking
loongson_canfd_rx_is_empty_fifo(), which consumes following valid frames in
the FIFO and leaves the next read mid-frame.  Also, the standard-ID check
in loongson_canfd_hdr_plausible() compares the 11-bit
REG_FRAME_META0_ID_BASE field against 0x7ff instead of checking whether
REG_FRAME_META0_ID_EXT is non-zero.

[ ... ]

> +	cfd->flags = 0;
> +
> +	if (d->is_can_fd) {
> +		if (d->hdr1 & REG_FRAME_META1_BRS)
> +			cfd->flags |= CANFD_BRS;
> +		if (d->hdr0 & REG_FRAME_META0_ESI)
> +			cfd->flags |= CANFD_ESI;
> +	} else if (d->is_rtr) {
> +		cfd->can_id |= CAN_RTR_FLAG;
> +	}

[Severity: Medium]
Does setting cfd->flags = 0 clear the mandatory CANFD_FDF flag that
alloc_canfd_skb() initialized on received CAN FD frames?

[ ... ]

> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (skb) {
> +		cf->can_id |= CAN_ERR_CRTL;
> +		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
> +
> +		scoped_guard(spinlock_irqsave, &priv->stats_lock) {
> +			stats->rx_packets++;
> +			stats->rx_bytes += cf->len;
> +		}
> +
> +		/* Hard IRQ: use netif_rx(), not netif_receive_skb() */
> +		netif_rx(skb);
> +	}

[Severity: Medium]
As reported by Vincent Mailhol on v2 [1], should synthetic SocketCAN error
frames be excluded from stats->rx_packets and stats->rx_bytes here?  Also,
updating stats->rx_packets and stats->rx_bytes under priv->stats_lock in
hardirq context races with the unlocked updates to the same counters in
loongson_canfd_rx_deliver() during NAPI polling.

[ ... ]

> +	/* Stats and BUS_OFF handling must not depend on skb allocation */
> +	switch (new_state) {
> +	case CAN_STATE_BUS_OFF:
> +		priv->can.can_stats.bus_off++;
> +		can_bus_off(ndev);
> +		break;

[Severity: Medium]
Does entering CAN_STATE_BUS_OFF without stopping the TX queue and
controller allow loongson_canfd_start_xmit() and
loongson_canfd_tx_interrupt() to race with bus-off recovery?

CPU0                              CPU1
can_restart_work()
  can_restart()
    can_flush_echo_skb()
      kfree_skb(priv->echo_skb[i])
                                  loongson_canfd_tx_interrupt()
                                    can_free_echo_skb()  <-- double free
    netif_carrier_on()
    loongson_canfd_do_set_mode()
      loongson_canfd_reset()
                                  loongson_canfd_start_xmit()

can_bus_off() only calls netif_carrier_off(), which defers qdisc
deactivation to linkwatch_work, and can_restart() flushes echo_skb[]
without holding priv->tx_lock before calling netif_carrier_on() and
do_set_mode(CAN_MODE_START).

> +	case CAN_STATE_ERROR_PASSIVE:
> +		priv->can.can_stats.error_passive++;
> +		break;
> +	case CAN_STATE_ERROR_WARNING:
> +		priv->can.can_stats.error_warning++;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	skb = alloc_can_err_skb(ndev, &cf);
> +	if (!skb)
> +		return;
> +
> +	switch (new_state) {
> +	case CAN_STATE_BUS_OFF:
> +		cf->can_id |= CAN_ERR_BUSOFF;
> +		goto send;
> +	case CAN_STATE_ERROR_ACTIVE:
> +		cf->can_id |= CAN_ERR_CNT;
> +		cf->data[1] = CAN_ERR_CRTL_ACTIVE;
> +		break;

[Severity: Medium]
Should CAN_STATE_ERROR_ACTIVE also set CAN_ERR_CRTL in cf->can_id when
reporting CAN_ERR_CRTL_ACTIVE in cf->data[1], and should error_passive and
error_warning only be incremented when transitioning to a worse state?

Overwriting priv->can.state before checking the transition direction
counts recoveries (such as CAN_STATE_ERROR_PASSIVE down to
CAN_STATE_ERROR_WARNING) as new error events.  In addition,
loongson_canfd_rx_overrun_event() adds cf->len of an error skb to
stats->rx_bytes under priv->stats_lock while loongson_canfd_rx_deliver()
updates rx_packets and rx_bytes without priv->stats_lock.

[ ... ]

> +			case TX_BS_VALID:
> +				u32 dummy_len;
> +
> +				/*
> +				 * The controller does not latch a per-frame
> +				 * TX stamp, so the instant the handler reads
> +				 * the counter is the best estimate of the
> +				 * end of the frame.
> +				 */
> +				skb = priv->can.echo_skb[buf_id];
> +				if (skb &&
> +				    (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS) &&
> +				    READ_ONCE(priv->hwts.ready))
> +					skb_hwtstamps(skb)->hwtstamp =
> +							loongson_canfd_hwts_now(priv);
> +
> +				dummy_len = can_get_echo_skb(ndev, buf_id, NULL);
> +				(void)dummy_len;
> +				stats->tx_packets++;
> +				break;

[ ... ]

> +		if (!processed)
> +			netdev_warn_once(ndev, "Spurious TX interrupt\n");
> +
> +		wake = loongson_canfd_txbuf_freed(priv);
> +	}
> +
> +	if (wake)
> +		netif_wake_queue(ndev);
> +}

[Severity: Medium]
Can calling netif_wake_queue() after dropping priv->tx_lock re-wake the TX
queue after loongson_canfd_start_xmit() has just filled the last buffer
and stopped it?

CPU0                              CPU1
loongson_canfd_tx_interrupt()
  lock(&priv->tx_lock)
  wake = loongson_canfd_txbuf_freed()
  unlock(&priv->tx_lock)
                                  loongson_canfd_start_xmit()
                                    lock(&priv->tx_lock)
                                    /* consumes last free TX buffer */
                                    netif_stop_queue(ndev)
                                    unlock(&priv->tx_lock)
  netif_wake_queue(ndev)  <-- wakes full queue

[ ... ]

> +static netdev_tx_t loongson_canfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> +{
> +	struct canfd_frame *cf = (struct canfd_frame *)skb->data;
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +	struct net_device_stats *stats = &ndev->stats;
> +	int buf_id, ret;
> +
> +	if (can_dropped_invalid_skb(ndev, skb))
> +		return NETDEV_TX_OK;

[Severity: Medium]
Should loongson_canfd_start_xmit() call can_dev_dropped_skb() instead of
can_dropped_invalid_skb()?

can_dropped_invalid_skb() is for virtual CAN drivers and does not check
CAN_CTRLMODE_LISTENONLY or CAN_CTRLMODE_FD, allowing frames sent via
non-RAW sockets in listen-only mode or CAN FD/XL frames sent via AF_PACKET
to reach loongson_canfd_insert_frame().

[ ... ]

> +	can_put_echo_skb(skb, ndev, buf_id, 0);
> +
> +	if (!(cf->can_id & CAN_RTR_FLAG))
> +		stats->tx_bytes += cf->len;

[Severity: High]
Does reading cf->can_id and cf->len after can_put_echo_skb() cause a
use-after-free of skb?

can_put_echo_skb() consumes or frees skb (for example via kfree_skb() when
IFF_ECHO is cleared or when skb_clone() fails in can_create_echo_skb()),
and cf points into skb->data.  Also, incrementing stats->tx_bytes here
instead of adding the length returned by can_get_echo_skb() in
loongson_canfd_tx_interrupt() counts bytes for frames that later fail, are
cancelled, or are flushed.

[ ... ]

> +static int loongson_canfd_close(struct net_device *ndev)
> +{
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +
> +	netif_stop_queue(ndev);
> +	napi_disable(&priv->napi);
> +
> +	/*
> +	 * Cancel a pending bus-off restart before tearing down IRQ/DMA:
> +	 * do_set_mode() re-arms both, so a restart firing after
> +	 * free_irq() would touch released resources. close_candev()\
> +	 * does the same but only after free_irq().
> +	 */
> +	cancel_delayed_work_sync(&priv->can.restart_work);
> +
> +	loongson_canfd_chip_stop(ndev);
> +	free_irq(ndev->irq, ndev);
> +	close_candev(ndev);
> +
> +	return 0;
> +}

[Severity: Medium]
Can an interrupt firing before loongson_canfd_chip_stop() and free_irq()
re-schedule restart_work or schedule NAPI after napi_disable()?

CPU0                              <hardirq>
loongson_canfd_close()
  napi_disable(&priv->napi)
  cancel_delayed_work_sync(&restart_work)
                                  loongson_canfd_interrupt()
                                    loongson_canfd_err_interrupt()
                                      can_bus_off()
                                        schedule_delayed_work(&restart_work)
  loongson_canfd_chip_stop()
                                  can_restart_work()
                                    loongson_canfd_do_set_mode(CAN_MODE_START)
                                      loongson_canfd_chip_start()  <-- restarts stopped chip
  free_irq()

Stopping the chip and freeing the IRQ before cancelling restart_work and
disabling NAPI avoids the ISR re-arming either during teardown.

[ ... ]

> +static int loongson_canfd_get_ts_info(struct net_device *ndev,
> +				      struct kernel_ethtool_ts_info *info)
> +{
> +	struct loongson_canfd_priv *priv = netdev_priv(ndev);
> +
> +	info->so_timestamping = SOF_TIMESTAMPING_SOFTWARE |
> +				SOF_TIMESTAMPING_TX_SOFTWARE |
> +				SOF_TIMESTAMPING_RX_SOFTWARE;
> +
> +	info->phc_index = -1;
> +
> +	if (READ_ONCE(priv->hwts.ready)) {
> +		info->so_timestamping |= SOF_TIMESTAMPING_TX_HARDWARE |
> +					 SOF_TIMESTAMPING_RX_HARDWARE |
> +					 SOF_TIMESTAMPING_RAW_HARDWARE;
> +		info->tx_types = BIT(HWTSTAMP_TX_ON);
> +		info->rx_filters = BIT(HWTSTAMP_FILTER_ALL);
> +	}
> +
> +	return 0;
> +}

[Severity: Medium]
Does checking priv->hwts.ready in loongson_canfd_get_ts_info() hide the
device's hardware timestamping capabilities whenever the interface is down?

priv->hwts.ready is only true between loongson_canfd_chip_start() and
loongson_canfd_chip_stop(), so querying ethtool timestamping info while the
interface is down omits the hardware timestamping flags and filters even
though can_hwtstamp_get() and can_hwtstamp_set() are unconditionally wired
up (see can_ethtool_op_get_ts_info_hwts()).

[ ... ]

> +static bool loongson_canfd_volatile_reg(struct device *dev, unsigned int reg)
> +{
> +	switch (reg) {
> +	case LOONGSON_CANFD_MODE:
> +	case LOONGSON_CANFD_CONF:
> +	case LOONGSON_CANFD_STAT:
> +	case LOONGSON_CANFD_CMD:
> +	case LOONGSON_CANFD_INT_STAT:
> +	case LOONGSON_CANFD_INT_ENA:
> +	case LOONGSON_CANFD_INT_MASK:
> +	case LOONGSON_CANFD_ERL:
> +	case LOONGSON_CANFD_FSTAT:
> +	case LOONGSON_CANFD_ERC:
> +	case LOONGSON_CANFD_ERR_CAPT:
> +	case LOONGSON_CANFD_ALC:
> +	case LOONGSON_CANFD_TRV_DLY:
> +	case LOONGSON_CANFD_TX_FR_CNT:
> +	case LOONGSON_CANFD_TS:
> +	case LOONGSON_CANFD_RX_STAT:
> +	case LOONGSON_CANFD_RX_DATA:
> +	case LOONGSON_CANFD_TX_STAT:
> +	case LOONGSON_CANFD_TX_CMD:
> +	case LOONGSON_CANFD_TX_SEL:
> +		return true;
> +	default:
> +		return false;
> +	};
> +}
> +
> +static const struct regmap_config loongson_canfd_regmap = {
> +	.reg_bits	= 32,
> +	.reg_stride	= 4,
> +	.val_bits	= 32,
> +	.wr_table	= &loongson_canfd_reg_table_wr,
> +	.rd_table	= &loongson_canfd_reg_table_rd,
> +	.volatile_reg	= loongson_canfd_volatile_reg,
> +	.max_register	= LOONGSON_CANFD_TX_DATA_18,
> +	.cache_type	= REGCACHE_MAPLE,
> +};

[Severity: Medium]
Why does loongson_canfd_regmap enable REGCACHE_MAPLE when the banked TX
data registers (LOONGSON_CANFD_TX_DATA_1..LOONGSON_CANFD_TX_DATA_18) are
not marked volatile?

Those offsets are a banked window into 8 TX buffers selected by
LOONGSON_CANFD_TX_SEL, and caching them with REGCACHE_MAPLE causes
regmap_write() in loongson_canfd_insert_frame() to perform GFP_ATOMIC
maple-tree allocations under priv->tx_lock (dropping the frame if an
allocation fails).  Also, LOONGSON_CANFD_RX_DATA has no precious_reg
callback, so reading regmap debugfs pops words from the live RX FIFO.

[1] https://lore.kernel.org/r/c9598f85-5b17-4d38-855e-c3840a01452f@kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794213427.git.zhoubinbin@loongson.cn?part=1