linux-kernel - Re: [PATCH v2 3/7] can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DCOZNFLPYZFH.2EWPVI5X452IJ@baylibre.com>
Date: Wed, 10 Sep 2025 10:47:45 +0200
From: "Markus Schneider-Pargmann" <msp@...libre.com>
To: "Marc Kleine-Budde" <mkl@...gutronix.de>, "Chandrasekar Ramakrishnan"
 <rcsekar@...sung.com>, "Vincent Mailhol" <mailhol.vincent@...adoo.fr>,
 "Patrik Flykt" <patrik.flykt@...ux.intel.com>, "Dong Aisheng"
 <b29396@...escale.com>, "Varka Bhadram" <varkabhadram@...il.com>, "Wu Bo"
 <wubo.oduw@...il.com>, "Philipp Zabel" <p.zabel@...gutronix.de>
Cc: <linux-can@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <kernel@...gutronix.de>
Subject: Re: [PATCH v2 3/7] can: m_can: m_can_handle_state_errors(): fix CAN
 state transition to Error Active

On Tue Sep 9, 2025 at 7:53 PM CEST, Marc Kleine-Budde wrote:
> The CAN Error State is determined by the receive and transmit error
> counters. The CAN error counters decrease when reception/transmission
> is successful, so that a status transition back to the Error Active
> status is possible. This transition is not handled by
> m_can_handle_state_errors().
>
> Add the missing detection of the Error Active state to
> m_can_handle_state_errors() and extend the handling of this state in
> m_can_handle_state_change().
>
> Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support")
> Fixes: cd0d83eab2e0 ("can: m_can: m_can_handle_state_change(): fix state change")
> Signed-off-by: Marc Kleine-Budde <mkl@...gutronix.de>

Reviewed-by: Markus Schneider-Pargmann <msp@...libre.com>

> ---
>  drivers/net/can/m_can/m_can.c | 55 ++++++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 16b38e6c3985..3edf01b098a4 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -812,6 +812,9 @@ static int m_can_handle_state_change(struct net_device *dev,
>  	u32 timestamp = 0;
>  
>  	switch (new_state) {
> +	case CAN_STATE_ERROR_ACTIVE:
> +		cdev->can.state = CAN_STATE_ERROR_ACTIVE;
> +		break;
>  	case CAN_STATE_ERROR_WARNING:
>  		/* error warning state */
>  		cdev->can.can_stats.error_warning++;
> @@ -841,6 +844,12 @@ static int m_can_handle_state_change(struct net_device *dev,
>  	__m_can_get_berr_counter(dev, &bec);
>  
>  	switch (new_state) {
> +	case CAN_STATE_ERROR_ACTIVE:
> +		cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
> +		cf->data[1] = CAN_ERR_CRTL_ACTIVE;
> +		cf->data[6] = bec.txerr;
> +		cf->data[7] = bec.rxerr;
> +		break;
>  	case CAN_STATE_ERROR_WARNING:
>  		/* error warning state */
>  		cf->can_id |= CAN_ERR_CRTL | CAN_ERR_CNT;
> @@ -877,30 +886,33 @@ static int m_can_handle_state_change(struct net_device *dev,
>  	return 1;
>  }
>  
> -static int m_can_handle_state_errors(struct net_device *dev, u32 psr)
> +static enum can_state
> +m_can_state_get_by_psr(struct m_can_classdev *cdev)
> +{
> +	u32 reg_psr;
> +
> +	reg_psr = m_can_read(cdev, M_CAN_PSR);
> +
> +	if (reg_psr & PSR_BO)
> +		return CAN_STATE_BUS_OFF;
> +	if (reg_psr & PSR_EP)
> +		return CAN_STATE_ERROR_PASSIVE;
> +	if (reg_psr & PSR_EW)
> +		return CAN_STATE_ERROR_WARNING;
> +
> +	return CAN_STATE_ERROR_ACTIVE;
> +}
> +
> +static int m_can_handle_state_errors(struct net_device *dev)
>  {
>  	struct m_can_classdev *cdev = netdev_priv(dev);
> -	int work_done = 0;
> +	enum can_state new_state;
>  
> -	if (psr & PSR_EW && cdev->can.state != CAN_STATE_ERROR_WARNING) {
> -		netdev_dbg(dev, "entered error warning state\n");
> -		work_done += m_can_handle_state_change(dev,
> -						       CAN_STATE_ERROR_WARNING);
> -	}
> +	new_state = m_can_state_get_by_psr(cdev);
> +	if (new_state == cdev->can.state)
> +		return 0;
>  
> -	if (psr & PSR_EP && cdev->can.state != CAN_STATE_ERROR_PASSIVE) {
> -		netdev_dbg(dev, "entered error passive state\n");
> -		work_done += m_can_handle_state_change(dev,
> -						       CAN_STATE_ERROR_PASSIVE);
> -	}
> -
> -	if (psr & PSR_BO && cdev->can.state != CAN_STATE_BUS_OFF) {
> -		netdev_dbg(dev, "entered error bus off state\n");
> -		work_done += m_can_handle_state_change(dev,
> -						       CAN_STATE_BUS_OFF);
> -	}
> -
> -	return work_done;
> +	return m_can_handle_state_change(dev, new_state);
>  }
>  
>  static void m_can_handle_other_err(struct net_device *dev, u32 irqstatus)
> @@ -1031,8 +1043,7 @@ static int m_can_rx_handler(struct net_device *dev, int quota, u32 irqstatus)
>  	}
>  
>  	if (irqstatus & IR_ERR_STATE)
> -		work_done += m_can_handle_state_errors(dev,
> -						       m_can_read(cdev, M_CAN_PSR));
> +		work_done += m_can_handle_state_errors(dev);
>  
>  	if (irqstatus & IR_ERR_BUS_30X)
>  		work_done += m_can_handle_bus_errors(dev, irqstatus,


Download attachment "signature.asc" of type "application/pgp-signature" (290 bytes)