RE: [PATCH 3/3] can: rcar_canfd: Handle Bus-Off recovery interrupt

[Biju Das]

Hi Marc,

Thanks for the feedback.

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@xxxxxxxxxxxxxx>
> Sent: 03 April 2026 22:33
> Subject: Re: [PATCH 3/3] can: rcar_canfd: Handle Bus-Off recovery interrupt
> 
> On 03.04.2026 10:50:00, Biju wrote:
> > From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> >
> > Add handling for the Bus-Off recovery interrupt in the error handler.
> > When the channel bus-off recovery interrupt is received, transition
> > the CAN state back to CAN_STATE_ERROR_ACTIVE to reflect that the
> > controller has successfully recovered from a bus-off event.
> 
> This looks wrong, better change the CAN state in rcar_canfd_state_change(), so that can_change_state()
> is called and the user gets informed.
> 
> BTW: The transition to bus-off should also be done via this function for the same reason. This would be
> a proper fixes patch, that should go in first.

I will replace the direct assignments to priv->can.state and manual can_stats
counter increments with the can_change_state() helper in the error
handler.

Please let me know, is the below changes ok for you?

	if (cerfl & RCANFD_CERFL_EWF) {
 		netdev_dbg(ndev, "Error warning interrupt\n");
-		priv->can.state = CAN_STATE_ERROR_WARNING;
-		priv->can.can_stats.error_warning++;
-		cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
-		cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_WARNING :
-			CAN_ERR_CRTL_RX_WARNING;
+		new_state = CAN_STATE_ERROR_WARNING;
+		cf->can_id |= CAN_ERR_CNT;
 		cf->data[6] = txerr;
 		cf->data[7] = rxerr;
 	}


	if (cerfl & RCANFD_CERFL_EPF) {
 		netdev_dbg(ndev, "Error passive interrupt\n");
-		priv->can.state = CAN_STATE_ERROR_PASSIVE;
-		priv->can.can_stats.error_passive++;
-		cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
-		cf->data[1] = txerr > rxerr ? CAN_ERR_CRTL_TX_PASSIVE :
-			CAN_ERR_CRTL_RX_PASSIVE;
+		new_state = CAN_STATE_ERROR_PASSIVE;
+		cf->can_id |= CAN_ERR_CNT;
 		cf->data[6] = txerr;
 		cf->data[7] = rxerr;
 	}
 	if (cerfl & RCANFD_CERFL_BOEF) {
 		netdev_dbg(ndev, "Bus-off entry interrupt\n");
 		rcar_canfd_tx_failure_cleanup(ndev);
-		priv->can.state = CAN_STATE_BUS_OFF;
-		priv->can.can_stats.bus_off++;
+		new_state = CAN_STATE_BUS_OFF;
+		txerr = CAN_BUS_OFF_THRESHOLD;  --> The IP sets error counters to zero during bus off.
 		can_bus_off(ndev);
-		cf->can_id |= CAN_ERR_BUSOFF;
 	}


+
+	if (unlikely(current_state != new_state)) {
+		enum can_state rx_state, tx_state;
+
+		tx_state = txerr >= rxerr ? new_state : 0;
+		rx_state = txerr <= rxerr ? new_state : 0;
+
+		can_change_state(ndev, cf, tx_state, rx_state);
+	}
+


Cheers,
Biju