* [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling
@ 2026-01-23 10:16 Marc Kleine-Budde
2026-01-23 10:16 ` [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters Marc Kleine-Budde
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2026-01-23 10:16 UTC (permalink / raw)
To: Vincent Mailhol, Michael Tretter
Cc: Achim Baumgartner, linux-can, linux-kernel, kernel,
Marc Kleine-Budde
The CAN controller triggers an EPI interrupt when it enters the error
passive state or transitions back to error active. Rather than tracking
state in the driver, the CAN controller state should be derived from the
TX/RX error counters using can_state_get_by_berr_counter().
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Changes in v2:
- patch#1: new
- patch#2: replace evaluation of error warning and error passive IRQ, by
can_state_get_by_berr_counter() to determine CAN state
- Link to v1: https://patch.msgid.link/20260123-can-sja1000-error-passive-v1-1-01e5d454ab5f@pengutronix.de
---
Marc Kleine-Budde (1):
can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters
Michael Tretter (1):
can: sja1000: sja1000_err(): use error counter for error state
drivers/net/can/sja1000/sja1000.c | 42 ++++++++++-----------------------------
1 file changed, 11 insertions(+), 31 deletions(-)
---
base-commit: b00a7b3a612925faa7362f5c61065e3e5f393fff
change-id: 20260123-sja1000-state-handling-b18fbddf102e
Best regards,
--
Marc Kleine-Budde <mkl@pengutronix.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters
2026-01-23 10:16 [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
@ 2026-01-23 10:16 ` Marc Kleine-Budde
2026-01-23 13:01 ` Michael Tretter
2026-01-23 10:16 ` [PATCH can-next v2 2/2] can: sja1000: sja1000_err(): use error counter for error state Marc Kleine-Budde
2026-01-23 13:40 ` [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
2 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2026-01-23 10:16 UTC (permalink / raw)
To: Vincent Mailhol, Michael Tretter
Cc: Achim Baumgartner, linux-can, linux-kernel, kernel,
Marc Kleine-Budde
This is a preparation patch to make use of can_state_get_by_berr_counter()
which works on a struct can_berr_counter.
Reuse the existing function sja1000_get_berr_counter() to read the error
counters into a struct can_berr_counter.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/sja1000.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index a8fa0d6516b9..ae3244b63b65 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -411,14 +411,13 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
struct sk_buff *skb;
enum can_state state = priv->can.state;
enum can_state rx_state, tx_state;
- unsigned int rxerr, txerr;
+ struct can_berr_counter bec;
uint8_t ecc, alc;
int ret = 0;
skb = alloc_can_err_skb(dev, &cf);
- txerr = priv->read_reg(priv, SJA1000_TXERR);
- rxerr = priv->read_reg(priv, SJA1000_RXERR);
+ sja1000_get_berr_counter(dev, &bec);
if (isrc & IRQ_DOI) {
/* data overrun interrupt */
@@ -455,8 +454,8 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
}
if (state != CAN_STATE_BUS_OFF && skb) {
cf->can_id |= CAN_ERR_CNT;
- cf->data[6] = txerr;
- cf->data[7] = rxerr;
+ cf->data[6] = bec.txerr;
+ cf->data[7] = bec.rxerr;
}
if (isrc & IRQ_BEI) {
/* bus error interrupt */
@@ -515,8 +514,8 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
}
if (state != priv->can.state) {
- tx_state = txerr >= rxerr ? state : 0;
- rx_state = txerr <= rxerr ? state : 0;
+ tx_state = bec.txerr >= bec.rxerr ? state : 0;
+ rx_state = bec.txerr <= bec.rxerr ? state : 0;
can_change_state(dev, cf, tx_state, rx_state);
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH can-next v2 2/2] can: sja1000: sja1000_err(): use error counter for error state
2026-01-23 10:16 [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
2026-01-23 10:16 ` [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters Marc Kleine-Budde
@ 2026-01-23 10:16 ` Marc Kleine-Budde
2026-01-23 13:05 ` Michael Tretter
2026-01-23 13:40 ` [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
2 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2026-01-23 10:16 UTC (permalink / raw)
To: Vincent Mailhol, Michael Tretter
Cc: Achim Baumgartner, linux-can, linux-kernel, kernel,
Marc Kleine-Budde
From: Michael Tretter <m.tretter@pengutronix.de>
The CAN controller sends the EPI interrupt whenever it reaches the error
passive status or enters the error active status from the error passive
status.
Instead of keeping track of the controller status in the driver, read the
txerr and rxerr counters and use can_state_get_by_berr_counter() to
determine the state of the CAN controller.
Suggested-by: Achim Baumgartner <abaumgartner@topcon.com>
Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/sja1000/sja1000.c | 33 +++++++--------------------------
1 file changed, 7 insertions(+), 26 deletions(-)
diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
index ae3244b63b65..67fccc15ed20 100644
--- a/drivers/net/can/sja1000/sja1000.c
+++ b/drivers/net/can/sja1000/sja1000.c
@@ -407,10 +407,9 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
{
struct sja1000_priv *priv = netdev_priv(dev);
struct net_device_stats *stats = &dev->stats;
+ enum can_state state, rx_state, tx_state;
struct can_frame *cf;
struct sk_buff *skb;
- enum can_state state = priv->can.state;
- enum can_state rx_state, tx_state;
struct can_berr_counter bec;
uint8_t ecc, alc;
int ret = 0;
@@ -418,6 +417,12 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
skb = alloc_can_err_skb(dev, &cf);
sja1000_get_berr_counter(dev, &bec);
+ can_state_get_by_berr_counter(dev, &bec, &tx_state, &rx_state);
+
+ if (status & SR_BS)
+ rx_state = CAN_STATE_BUS_OFF;
+
+ state = max(tx_state, rx_state);
if (isrc & IRQ_DOI) {
/* data overrun interrupt */
@@ -440,18 +445,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
if (priv->flags & SJA1000_QUIRK_RESET_ON_OVERRUN)
ret = IRQ_WAKE_THREAD;
}
-
- if (isrc & IRQ_EI) {
- /* error warning interrupt */
- netdev_dbg(dev, "error warning interrupt\n");
-
- if (status & SR_BS)
- state = CAN_STATE_BUS_OFF;
- else if (status & SR_ES)
- state = CAN_STATE_ERROR_WARNING;
- else
- state = CAN_STATE_ERROR_ACTIVE;
- }
if (state != CAN_STATE_BUS_OFF && skb) {
cf->can_id |= CAN_ERR_CNT;
cf->data[6] = bec.txerr;
@@ -493,15 +486,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
stats->rx_errors++;
}
}
- if (isrc & IRQ_EPI) {
- /* error passive interrupt */
- netdev_dbg(dev, "error passive interrupt\n");
-
- if (state == CAN_STATE_ERROR_PASSIVE)
- state = CAN_STATE_ERROR_WARNING;
- else
- state = CAN_STATE_ERROR_PASSIVE;
- }
if (isrc & IRQ_ALI) {
/* arbitration lost interrupt */
netdev_dbg(dev, "arbitration lost interrupt\n");
@@ -514,9 +498,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
}
if (state != priv->can.state) {
- tx_state = bec.txerr >= bec.rxerr ? state : 0;
- rx_state = bec.txerr <= bec.rxerr ? state : 0;
-
can_change_state(dev, cf, tx_state, rx_state);
if(state == CAN_STATE_BUS_OFF)
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters
2026-01-23 10:16 ` [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters Marc Kleine-Budde
@ 2026-01-23 13:01 ` Michael Tretter
0 siblings, 0 replies; 6+ messages in thread
From: Michael Tretter @ 2026-01-23 13:01 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Vincent Mailhol, Achim Baumgartner, linux-can, linux-kernel,
kernel
On Fri, 23 Jan 2026 11:16:26 +0100, Marc Kleine-Budde wrote:
> This is a preparation patch to make use of can_state_get_by_berr_counter()
> which works on a struct can_berr_counter.
>
> Reuse the existing function sja1000_get_berr_counter() to read the error
> counters into a struct can_berr_counter.
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Reviewed-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> drivers/net/can/sja1000/sja1000.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index a8fa0d6516b9..ae3244b63b65 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -411,14 +411,13 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> struct sk_buff *skb;
> enum can_state state = priv->can.state;
> enum can_state rx_state, tx_state;
> - unsigned int rxerr, txerr;
> + struct can_berr_counter bec;
> uint8_t ecc, alc;
> int ret = 0;
>
> skb = alloc_can_err_skb(dev, &cf);
>
> - txerr = priv->read_reg(priv, SJA1000_TXERR);
> - rxerr = priv->read_reg(priv, SJA1000_RXERR);
> + sja1000_get_berr_counter(dev, &bec);
>
> if (isrc & IRQ_DOI) {
> /* data overrun interrupt */
> @@ -455,8 +454,8 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> }
> if (state != CAN_STATE_BUS_OFF && skb) {
> cf->can_id |= CAN_ERR_CNT;
> - cf->data[6] = txerr;
> - cf->data[7] = rxerr;
> + cf->data[6] = bec.txerr;
> + cf->data[7] = bec.rxerr;
> }
> if (isrc & IRQ_BEI) {
> /* bus error interrupt */
> @@ -515,8 +514,8 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> }
>
> if (state != priv->can.state) {
> - tx_state = txerr >= rxerr ? state : 0;
> - rx_state = txerr <= rxerr ? state : 0;
> + tx_state = bec.txerr >= bec.rxerr ? state : 0;
> + rx_state = bec.txerr <= bec.rxerr ? state : 0;
>
> can_change_state(dev, cf, tx_state, rx_state);
>
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH can-next v2 2/2] can: sja1000: sja1000_err(): use error counter for error state
2026-01-23 10:16 ` [PATCH can-next v2 2/2] can: sja1000: sja1000_err(): use error counter for error state Marc Kleine-Budde
@ 2026-01-23 13:05 ` Michael Tretter
0 siblings, 0 replies; 6+ messages in thread
From: Michael Tretter @ 2026-01-23 13:05 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Vincent Mailhol, Achim Baumgartner, linux-can, linux-kernel,
kernel
On Fri, 23 Jan 2026 11:16:27 +0100, Marc Kleine-Budde wrote:
> From: Michael Tretter <m.tretter@pengutronix.de>
>
> The CAN controller sends the EPI interrupt whenever it reaches the error
> passive status or enters the error active status from the error passive
> status.
>
> Instead of keeping track of the controller status in the driver, read the
> txerr and rxerr counters and use can_state_get_by_berr_counter() to
> determine the state of the CAN controller.
>
> Suggested-by: Achim Baumgartner <abaumgartner@topcon.com>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
> drivers/net/can/sja1000/sja1000.c | 33 +++++++--------------------------
> 1 file changed, 7 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/can/sja1000/sja1000.c b/drivers/net/can/sja1000/sja1000.c
> index ae3244b63b65..67fccc15ed20 100644
> --- a/drivers/net/can/sja1000/sja1000.c
> +++ b/drivers/net/can/sja1000/sja1000.c
> @@ -407,10 +407,9 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> {
> struct sja1000_priv *priv = netdev_priv(dev);
> struct net_device_stats *stats = &dev->stats;
> + enum can_state state, rx_state, tx_state;
> struct can_frame *cf;
> struct sk_buff *skb;
> - enum can_state state = priv->can.state;
> - enum can_state rx_state, tx_state;
> struct can_berr_counter bec;
> uint8_t ecc, alc;
> int ret = 0;
> @@ -418,6 +417,12 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> skb = alloc_can_err_skb(dev, &cf);
>
> sja1000_get_berr_counter(dev, &bec);
> + can_state_get_by_berr_counter(dev, &bec, &tx_state, &rx_state);
> +
> + if (status & SR_BS)
> + rx_state = CAN_STATE_BUS_OFF;
I was wondering why you dropped the check for (status & SR_ES). SR_ES
just indicates that one of the error counters exceeded the warning
limit, which is handled by can_state_get_by_berr_counter(). Thus, it's
fine to drop the check.
Thanks!
Michael
> +
> + state = max(tx_state, rx_state);
>
> if (isrc & IRQ_DOI) {
> /* data overrun interrupt */
> @@ -440,18 +445,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> if (priv->flags & SJA1000_QUIRK_RESET_ON_OVERRUN)
> ret = IRQ_WAKE_THREAD;
> }
> -
> - if (isrc & IRQ_EI) {
> - /* error warning interrupt */
> - netdev_dbg(dev, "error warning interrupt\n");
> -
> - if (status & SR_BS)
> - state = CAN_STATE_BUS_OFF;
> - else if (status & SR_ES)
> - state = CAN_STATE_ERROR_WARNING;
> - else
> - state = CAN_STATE_ERROR_ACTIVE;
> - }
> if (state != CAN_STATE_BUS_OFF && skb) {
> cf->can_id |= CAN_ERR_CNT;
> cf->data[6] = bec.txerr;
> @@ -493,15 +486,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> stats->rx_errors++;
> }
> }
> - if (isrc & IRQ_EPI) {
> - /* error passive interrupt */
> - netdev_dbg(dev, "error passive interrupt\n");
> -
> - if (state == CAN_STATE_ERROR_PASSIVE)
> - state = CAN_STATE_ERROR_WARNING;
> - else
> - state = CAN_STATE_ERROR_PASSIVE;
> - }
> if (isrc & IRQ_ALI) {
> /* arbitration lost interrupt */
> netdev_dbg(dev, "arbitration lost interrupt\n");
> @@ -514,9 +498,6 @@ static int sja1000_err(struct net_device *dev, uint8_t isrc, uint8_t status)
> }
>
> if (state != priv->can.state) {
> - tx_state = bec.txerr >= bec.rxerr ? state : 0;
> - rx_state = bec.txerr <= bec.rxerr ? state : 0;
> -
> can_change_state(dev, cf, tx_state, rx_state);
>
> if(state == CAN_STATE_BUS_OFF)
>
> --
> 2.51.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling
2026-01-23 10:16 [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
2026-01-23 10:16 ` [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters Marc Kleine-Budde
2026-01-23 10:16 ` [PATCH can-next v2 2/2] can: sja1000: sja1000_err(): use error counter for error state Marc Kleine-Budde
@ 2026-01-23 13:40 ` Marc Kleine-Budde
2 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2026-01-23 13:40 UTC (permalink / raw)
To: Vincent Mailhol, Michael Tretter
Cc: Achim Baumgartner, linux-can, linux-kernel, kernel
[-- Attachment #1: Type: text/plain, Size: 712 bytes --]
On 23.01.2026 11:16:25, Marc Kleine-Budde wrote:
> The CAN controller triggers an EPI interrupt when it enters the error
> passive state or transitions back to error active. Rather than tracking
> state in the driver, the CAN controller state should be derived from the
> TX/RX error counters using can_state_get_by_berr_counter().
>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
Applied to linux-can-next.
Thanks,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-01-23 13:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-23 10:16 [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
2026-01-23 10:16 ` [PATCH can-next v2 1/2] can: sja1000: sja1000_err(): make use of sja1000_get_berr_counter() to read error counters Marc Kleine-Budde
2026-01-23 13:01 ` Michael Tretter
2026-01-23 10:16 ` [PATCH can-next v2 2/2] can: sja1000: sja1000_err(): use error counter for error state Marc Kleine-Budde
2026-01-23 13:05 ` Michael Tretter
2026-01-23 13:40 ` [PATCH can-next v2 0/2] can: sja1000: clean up CAN state handling Marc Kleine-Budde
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox