[PATCH RESEND v4 3/8] can: flexcan: split rx/tx masks per mailbox IRQ line
Ciprian Marian Costea
ciprianmarian.costea at oss.nxp.com
Tue Jun 9 04:20:17 PDT 2026
On 6/9/2026 12:24 PM, Vincent Mailhol wrote:
> On 03/06/2026 at 09:13, Ciprian Costea wrote:
>> From: Ciprian Marian Costea <ciprianmarian.costea at oss.nxp.com>
>>
>> On S32G2, which has two mailbox IRQ lines (mb-0 for MBs 0-7, mb-1
>> for MBs 8-127), both handlers currently process the full rx_mask/tx_mask
>> range,
>>
>> Introduce struct flexcan_mb_irq to hold per-IRQ-line rx and tx masks.
>>
>> In flexcan_irq_mb(), the irq argument selects the correct mask set: the
>> primary MB IRQ uses mb_irq[0] and the secondary uses mb_irq[1].
>>
>> For single-IRQ platforms, mb_irq[0] holds the full combined masks with no
>> functional change.
>>
>> Signed-off-by: Ciprian Marian Costea <ciprianmarian.costea at oss.nxp.com>
>> ---
>> drivers/net/can/flexcan/flexcan-core.c | 61 +++++++++++++++++++-------
>> drivers/net/can/flexcan/flexcan.h | 10 ++++-
>> 2 files changed, 52 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
>> index 7dde2e623def..32e4d4da00a1 100644
>> --- a/drivers/net/can/flexcan/flexcan-core.c
>> +++ b/drivers/net/can/flexcan/flexcan-core.c
>> @@ -957,14 +957,16 @@ static inline void flexcan_write64(struct flexcan_priv *priv, u64 val, void __io
>> priv->write(lower_32_bits(val), addr);
>> }
>>
>> -static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv)
>> +static inline u64 flexcan_read_reg_iflag_rx(struct flexcan_priv *priv,
>> + u64 rx_mask)
>> {
>> - return flexcan_read64_mask(priv, &priv->regs->iflag1, priv->rx_mask);
>> + return flexcan_read64_mask(priv, &priv->regs->iflag1, rx_mask);
>> }
>>
>> -static inline u64 flexcan_read_reg_iflag_tx(struct flexcan_priv *priv)
>> +static inline u64 flexcan_read_reg_iflag_tx(struct flexcan_priv *priv,
>> + u64 tx_mask)
>> {
>> - return flexcan_read64_mask(priv, &priv->regs->iflag1, priv->tx_mask);
>> + return flexcan_read64_mask(priv, &priv->regs->iflag1, tx_mask);
>> }
>>
>> static inline struct flexcan_priv *rx_offload_to_priv(struct can_rx_offload *offload)
>> @@ -1071,7 +1073,8 @@ static struct sk_buff *flexcan_mailbox_read(struct can_rx_offload *offload,
>> }
>>
>> /* Process mailbox (RX + TX) events */
>> -static irqreturn_t flexcan_do_mb(struct net_device *dev)
>> +static irqreturn_t flexcan_do_mb(struct net_device *dev,
>> + const struct flexcan_mb_irq *mb_irq)
>> {
>> struct net_device_stats *stats = &dev->stats;
>> struct flexcan_priv *priv = netdev_priv(dev);
>> @@ -1084,7 +1087,8 @@ static irqreturn_t flexcan_do_mb(struct net_device *dev)
>> u64 reg_iflag_rx;
>> int ret;
>>
>> - while ((reg_iflag_rx = flexcan_read_reg_iflag_rx(priv))) {
>> + while ((reg_iflag_rx = flexcan_read_reg_iflag_rx(priv,
>> + mb_irq->rx_mask))) {
>> handled = IRQ_HANDLED;
>> ret = can_rx_offload_irq_offload_timestamp(&priv->offload,
>> reg_iflag_rx);
>> @@ -1110,10 +1114,10 @@ static irqreturn_t flexcan_do_mb(struct net_device *dev)
>> }
>> }
>>
>> - reg_iflag_tx = flexcan_read_reg_iflag_tx(priv);
>> + reg_iflag_tx = flexcan_read_reg_iflag_tx(priv, mb_irq->tx_mask);
>>
>> /* transmission complete interrupt */
>> - if (reg_iflag_tx & priv->tx_mask) {
>> + if (reg_iflag_tx & mb_irq->tx_mask) {
>> u32 reg_ctrl = priv->read(&priv->tx_mb->can_ctrl);
>>
>> handled = IRQ_HANDLED;
>> @@ -1125,7 +1129,7 @@ static irqreturn_t flexcan_do_mb(struct net_device *dev)
>> /* after sending a RTR frame MB is in RX mode */
>> priv->write(FLEXCAN_MB_CODE_TX_INACTIVE,
>> &priv->tx_mb->can_ctrl);
>> - flexcan_write64(priv, priv->tx_mask, ®s->iflag1);
>> + flexcan_write64(priv, mb_irq->tx_mask, ®s->iflag1);
>> netif_wake_queue(dev);
>> }
>>
>> @@ -1228,7 +1232,7 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id)
>> struct flexcan_priv *priv = netdev_priv(dev);
>> irqreturn_t handled;
>>
>> - handled = flexcan_do_mb(dev);
>> + handled = flexcan_do_mb(dev, &priv->mb_irq[0]);
>> handled |= flexcan_do_state(dev);
>> handled |= flexcan_do_berr(dev);
>>
>> @@ -1243,9 +1247,15 @@ static irqreturn_t flexcan_irq_mb(int irq, void *dev_id)
>> {
>> struct net_device *dev = dev_id;
>> struct flexcan_priv *priv = netdev_priv(dev);
>> + const struct flexcan_mb_irq *mb_irq;
>> irqreturn_t handled;
>> + int idx;
>>
>> - handled = flexcan_do_mb(dev);
>> + idx = (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ &&
>> + irq == priv->irq_secondary_mb) ? 1 : 0;
>> + mb_irq = &priv->mb_irq[idx];
>> +
>> + handled = flexcan_do_mb(dev, mb_irq);
>>
>> if (handled)
>> can_rx_offload_irq_finish(&priv->offload);
>> @@ -1473,6 +1483,7 @@ static void flexcan_ram_init(struct net_device *dev)
>> static int flexcan_rx_offload_setup(struct net_device *dev)
>> {
>> struct flexcan_priv *priv = netdev_priv(dev);
>> + u64 rx_mask, tx_mask;
>> int err;
>>
>> if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
>> @@ -1494,20 +1505,35 @@ static int flexcan_rx_offload_setup(struct net_device *dev)
>> flexcan_get_mb(priv, FLEXCAN_TX_MB_RESERVED_RX_FIFO);
>> priv->tx_mb_idx = priv->mb_count - 1;
>> priv->tx_mb = flexcan_get_mb(priv, priv->tx_mb_idx);
>> - priv->tx_mask = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
>> -
>> priv->offload.mailbox_read = flexcan_mailbox_read;
>>
>> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_USE_RX_MAILBOX) {
>> priv->offload.mb_first = FLEXCAN_RX_MB_RX_MAILBOX_FIRST;
>> priv->offload.mb_last = priv->mb_count - 2;
>>
>> - priv->rx_mask = GENMASK_ULL(priv->offload.mb_last,
>> - priv->offload.mb_first);
>> + rx_mask = GENMASK_ULL(priv->offload.mb_last,
>> + priv->offload.mb_first);
>> + tx_mask = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
>> +
>> + if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ) {
>> + /* S32G2 has two MB IRQ lines with the split at MB 8:
>> + * mb-0 IRQ handles MBs 0-7,
>> + * mb-1 IRQ handles MBs 8-127.
> ^^^
> Your comment says 8-127 but the code uses GENMASK_ULL(63, 8). Is this
> intentional?
Hello Vincent,
Thank you for taking time in reviewing this series.
This is not intentional. The driver currently supports up to 64 MBs
(bounded by the iflag1 + iflag2 register layout and the u64 mask).
I will update the comment accordingly in V5.
>
>> + */
>> + priv->mb_irq[0].rx_mask = rx_mask & GENMASK_ULL(7, 0);
>> + priv->mb_irq[0].tx_mask = tx_mask & GENMASK_ULL(7, 0);
>> + priv->mb_irq[1].rx_mask = rx_mask & GENMASK_ULL(63, 8);
>> + priv->mb_irq[1].tx_mask = tx_mask & GENMASK_ULL(63, 8);
>> + } else {
>> + priv->mb_irq[0].rx_mask = rx_mask;
>> + priv->mb_irq[0].tx_mask = tx_mask;
>> + }
>> +
>
> The introduction of the struct flexcan_mb_irq seems a bit overkill.
> Can't you just define two new masks and keep the existing struct
> flexcan_stop_mode untouched:
>
> #define FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK GENMASK_U64(7, 0)
> #define FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK GENMASK_U64(63, 8)
>
> and when you need to access the MB, just select the correct mask. For
> example, flexcan_irq_mb() becomes something like this:
>
> u64 mb_mask;
>
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_SECONDARY_MB_IRQ
> && irq == priv->irq_secondary_mb)
> mb_mask = FLEXCAN_SECONDARY_MB_IRQ_MB0_MASK;
> else
> mb_mask = FLEXCAN_SECONDARY_MB_IRQ_MB1_MASK;
>
> handled = flexcan_do_mb(dev, mb_mask);
>
Sounds like a good simplification indeed. I will update it in V5.
Regards,
Ciprian
>> err = can_rx_offload_add_timestamp(dev, &priv->offload);
>> } else {
>> - priv->rx_mask = FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
>> + priv->mb_irq[0].rx_mask = FLEXCAN_IFLAG_RX_FIFO_OVERFLOW |
>> FLEXCAN_IFLAG_RX_FIFO_AVAILABLE;
>> + priv->mb_irq[0].tx_mask = FLEXCAN_IFLAG_MB(priv->tx_mb_idx);
>> err = can_rx_offload_add_fifo(dev, &priv->offload,
>> FLEXCAN_NAPI_WEIGHT);
>> }
>> @@ -1531,7 +1557,8 @@ static void flexcan_chip_interrupts_enable(const struct net_device *dev)
>> disable_irq(priv->irq_secondary_mb);
>>
>> priv->write(priv->reg_ctrl_default, ®s->ctrl);
>> - reg_imask = priv->rx_mask | priv->tx_mask;
>> + reg_imask = priv->mb_irq[0].rx_mask | priv->mb_irq[0].tx_mask |
>> + priv->mb_irq[1].rx_mask | priv->mb_irq[1].tx_mask;
>> priv->write(upper_32_bits(reg_imask), ®s->imask2);
>> priv->write(lower_32_bits(reg_imask), ®s->imask1);
>> enable_irq(dev->irq);
>> diff --git a/drivers/net/can/flexcan/flexcan.h b/drivers/net/can/flexcan/flexcan.h
>> index 16692a2502eb..22aa097ec3c0 100644
>> --- a/drivers/net/can/flexcan/flexcan.h
>> +++ b/drivers/net/can/flexcan/flexcan.h
>> @@ -75,10 +75,17 @@
>> */
>> #define FLEXCAN_QUIRK_SECONDARY_MB_IRQ BIT(18)
>>
>> +#define FLEXCAN_NR_MB_IRQS 2
>> +
>> struct flexcan_devtype_data {
>> u32 quirks; /* quirks needed for different IP cores */
>> };
>>
>> +struct flexcan_mb_irq {
>> + u64 rx_mask;
>> + u64 tx_mask;
>> +};
>> +
>> struct flexcan_stop_mode {
>> struct regmap *gpr;
>> u8 req_gpr;
>> @@ -99,8 +106,7 @@ struct flexcan_priv {
>> u8 clk_src; /* clock source of CAN Protocol Engine */
>> u8 scu_idx;
>>
>> - u64 rx_mask;
>> - u64 tx_mask;
>> + struct flexcan_mb_irq mb_irq[FLEXCAN_NR_MB_IRQS];
>> u32 reg_ctrl_default;
>>
>> struct clk *clk_ipg;
>
>
> Yours sincerely,
> Vincent Mailhol
>
More information about the linux-arm-kernel
mailing list