[PATCH v2 0/7] can: m_can: fix pm_runtime and CAN state handling

[PATCH v2 0/7] can: m_can: fix pm_runtime and CAN state handling

[Marc Kleine-Budde]

The first patch fixes a pm_runtime imbalance in the m_can_platform
driver.

The rest of this series fixes the CAN state handling in the m_can
driver:
- add the missing state transition from from Error Warning back to
  Error Active (Patches 2+3)
- address the fact that in some SoCs (observed on the STM32MP15) the
  M_CAN IP core keeps the CAN state and CAN error counters over an
  internal reset cycle. Set the correct CAN state during ifup and
  system resume (Patches 4+5)
- add support for optional shared external reset, to properly reset
  the IP core (Patches 6+7)

The update of the DT binding for the reset property is upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/atorgue/stm32.git/commit/?h=stm32-next&id=cfd856da6cf561f7e1dc6b16f3453814cde1058e

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
Changes in v2:
- cover: added link to DT bindings update (thanks Philipp)
- 1: add Markus's R-b
- 2: always mask active interrupts (thanks Markus)
- 3: remove not needed comments (thanks Markus)
- 3: rename m_can_can_state_get_by_psr() -> m_can_state_get_by_psr() (thanks Markus)
- 3: read PSR inside m_can_state_get_by_psr() (thanks Markus)
- 5: add Markus's R-b
- 7: fix typo (thanks Philipp)
- 7: rename struct m_can_classdev::rsts -> rst (thanks Philipp)
- Link to v1: https://patch.msgid.link/20250812-m_can-fix-state-handling-v1-0-b739e06c0a3b@pengutronix.de

---
Marc Kleine-Budde (7):
      can: m_can: m_can_plat_remove(): add missing pm_runtime_disable()
      can: m_can: only handle active interrupts
      can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active
      can: m_can: m_can_chip_config(): bring up interface in correct state
      can: m_can: fix CAN state in system PM
      can: m_can: m_can_get_berr_counter(): don't wake up controller if interface is down
      can: m_can: add optional support for reset

 drivers/net/can/m_can/m_can.c          | 97 +++++++++++++++++++++++-----------
 drivers/net/can/m_can/m_can.h          |  1 +
 drivers/net/can/m_can/m_can_platform.c |  2 +-
 3 files changed, 69 insertions(+), 31 deletions(-)
---
base-commit: d3b28612bc5500133260aaf36794a0a0c287d61b
change-id: 20250811-m_can-fix-state-handling-0ba4bda6cb8e

Best regards,
--  
Marc Kleine-Budde <mkl@pengutronix.de>

[PATCH v2 1/7] can: m_can: m_can_plat_remove(): add missing pm_runtime_disable()

[Marc Kleine-Budde]

Commit 227619c3ff7c ("can: m_can: move runtime PM enable/disable to
m_can_platform") moved the PM runtime enable from the m_can core
driver into the m_can_platform.

That patch forgot to move the pm_runtime_disable() to
m_can_plat_remove(), so that unloading the m_can_platform driver
causes an "Unbalanced pm_runtime_enable!" error message.

Add the missing pm_runtime_disable() to m_can_plat_remove() to fix the
problem.

Cc: Patrik Flykt <patrik.flykt@linux.intel.com>
Fixes: 227619c3ff7c ("can: m_can: move runtime PM enable/disable to m_can_platform")
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can_platform.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can_platform.c b/drivers/net/can/m_can/m_can_platform.c
index b832566efda0..057eaa7b8b4b 100644
--- a/drivers/net/can/m_can/m_can_platform.c
+++ b/drivers/net/can/m_can/m_can_platform.c
@@ -180,7 +180,7 @@ static void m_can_plat_remove(struct platform_device *pdev)
 	struct m_can_classdev *mcan_class = &priv->cdev;
 
 	m_can_class_unregister(mcan_class);
-
+	pm_runtime_disable(mcan_class->dev);
 	m_can_class_free_dev(mcan_class->net);
 }
 

-- 
2.51.0

[PATCH v2 2/7] can: m_can: only handle active interrupts

[Marc Kleine-Budde]

The M_CAN IP core has an Interrupt Register (IR) and an Interrupt
Enable (IE) register. An interrupt is triggered if at least 1 bit is
set in the bitwise and of IR and IE.

Depending on the configuration not all interrupts are enabled in the
IE register. However the m_can_rx_handler() IRQ handler looks at all
interrupts not just the enabled ones. This may lead to handling of not
activated interrupts.

Fix the problem and mask the irqstatus (IR register) with the
active_interrupts (cached value of IE register).

Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index fe74dbd2c966..16b38e6c3985 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1057,6 +1057,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
 	u32 irqstatus;
 
 	irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);
+	irqstatus &= cdev->active_interrupts;
 
 	work_done = m_can_rx_handler(dev, quota, irqstatus);
 
@@ -1243,6 +1244,8 @@ static int m_can_interrupt_handler(struct m_can_classdev *cdev)
 	}
 
 	m_can_coalescing_update(cdev, ir);
+
+	ir &= cdev->active_interrupts;
 	if (!ir)
 		return IRQ_NONE;
 

-- 
2.51.0

Re: [PATCH v2 2/7] can: m_can: only handle active interrupts

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

On Tue Sep 9, 2025 at 7:53 PM CEST, Marc Kleine-Budde wrote:
> The M_CAN IP core has an Interrupt Register (IR) and an Interrupt
> Enable (IE) register. An interrupt is triggered if at least 1 bit is
> set in the bitwise and of IR and IE.
>
> Depending on the configuration not all interrupts are enabled in the
> IE register. However the m_can_rx_handler() IRQ handler looks at all
> interrupts not just the enabled ones. This may lead to handling of not
> activated interrupts.
>
> Fix the problem and mask the irqstatus (IR register) with the
> active_interrupts (cached value of IE register).
>
> Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/m_can/m_can.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index fe74dbd2c966..16b38e6c3985 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1057,6 +1057,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
>  	u32 irqstatus;
>  
>  	irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);
> +	irqstatus &= cdev->active_interrupts;
>  
>  	work_done = m_can_rx_handler(dev, quota, irqstatus);
>  
> @@ -1243,6 +1244,8 @@ static int m_can_interrupt_handler(struct m_can_classdev *cdev)
>  	}
>  
>  	m_can_coalescing_update(cdev, ir);
> +
> +	ir &= cdev->active_interrupts;

m_can_coalescing_update() can change active_interrupts, meaning the
interrupt that caused the interrupt handler to run may be disabled in
active_interrupts above and then masked in this added line. Would that
still work or does it confuse the hardware?

Best
Markus

>  	if (!ir)
>  		return IRQ_NONE;
>  


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

Re: [PATCH v2 2/7] can: m_can: only handle active interrupts

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3408 bytes --]

On 10.09.2025 10:41:28, Markus Schneider-Pargmann wrote:
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index fe74dbd2c966..16b38e6c3985 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -1057,6 +1057,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
> >  	u32 irqstatus;
> >  
> >  	irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);
> > +	irqstatus &= cdev->active_interrupts;
> >  
> >  	work_done = m_can_rx_handler(dev, quota, irqstatus);
> >  
> > @@ -1243,6 +1244,8 @@ static int m_can_interrupt_handler(struct m_can_classdev *cdev)
> >  	}
> >  
> >  	m_can_coalescing_update(cdev, ir);
> > +
> > +	ir &= cdev->active_interrupts;
> 
> m_can_coalescing_update() can change active_interrupts, meaning the
> interrupt that caused the interrupt handler to run may be disabled in
> active_interrupts above and then masked in this added line. Would that
> still work or does it confuse the hardware?

I think m_can_coalescing_update() expects the RX/TX will be cleared. Are
the following comments OK...

| diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
| index 16b38e6c3985..8cb9cc1cddbf 100644
| --- a/drivers/net/can/m_can/m_can.c
| +++ b/drivers/net/can/m_can/m_can.c
| @@ -1188,28 +1188,39 @@ static int m_can_echo_tx_event(struct net_device *dev)
|  
|  static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
|  {
|          u32 new_interrupts = cdev->active_interrupts;
|          bool enable_rx_timer = false;
|          bool enable_tx_timer = false;
|  
|          if (!cdev->net->irq)
|                  return;
|  
| +        /* If there is a packet in the FIFO then:
| +         * - start timer
| +         * - disable not empty IRQ
| +         * - handle FIFO
                ^^^^^^^^^^^

...especially this one?

| +         */
|          if (cdev->rx_coalesce_usecs_irq > 0 && (ir & (IR_RF0N | IR_RF0W))) {
|                  enable_rx_timer = true;
|                  new_interrupts &= ~IR_RF0N;
|          }
|          if (cdev->tx_coalesce_usecs_irq > 0 && (ir & (IR_TEFN | IR_TEFW))) {
|                  enable_tx_timer = true;
|                  new_interrupts &= ~IR_TEFN;
|          }
| +
| +        /* If:
| +         * - timer is not going to be start
| +         * - and timer is not active
| +         * -> then enable FIFO empty IRQ
| +         */
|          if (!enable_rx_timer && !hrtimer_active(&cdev->hrtimer))
|                  new_interrupts |= IR_RF0N;
|          if (!enable_tx_timer && !hrtimer_active(&cdev->hrtimer))
|                  new_interrupts |= IR_TEFN;
|  
|          m_can_interrupt_enable(cdev, new_interrupts);
|          if (enable_rx_timer | enable_tx_timer)
|                  hrtimer_start(&cdev->hrtimer, cdev->irq_timer_wait,
|                                HRTIMER_MODE_REL);
|  }

Currently the m_can_coalescing_update() is called at the beginning of
the IRQ handler. Does it make sense to move it to the end and pass the
unmasked M_CAN_IR?

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 --]

Re: [PATCH v2 2/7] can: m_can: only handle active interrupts

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 3814 bytes --]

On 10.09.2025 16:28:54, Marc Kleine-Budde wrote:
> On 10.09.2025 10:41:28, Markus Schneider-Pargmann wrote:
> > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > > index fe74dbd2c966..16b38e6c3985 100644
> > > --- a/drivers/net/can/m_can/m_can.c
> > > +++ b/drivers/net/can/m_can/m_can.c
> > > @@ -1057,6 +1057,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
> > >  	u32 irqstatus;
> > >  
> > >  	irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);
> > > +	irqstatus &= cdev->active_interrupts;
> > >  
> > >  	work_done = m_can_rx_handler(dev, quota, irqstatus);
> > >  
> > > @@ -1243,6 +1244,8 @@ static int m_can_interrupt_handler(struct m_can_classdev *cdev)
> > >  	}
> > >  
> > >  	m_can_coalescing_update(cdev, ir);
> > > +
> > > +	ir &= cdev->active_interrupts;
> > 
> > m_can_coalescing_update() can change active_interrupts, meaning the
> > interrupt that caused the interrupt handler to run may be disabled in
> > active_interrupts above and then masked in this added line. Would that
> > still work or does it confuse the hardware?
> 
> I think m_can_coalescing_update() expects the RX/TX will be cleared. Are
> the following comments OK...
> 
> | diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> | index 16b38e6c3985..8cb9cc1cddbf 100644
> | --- a/drivers/net/can/m_can/m_can.c
> | +++ b/drivers/net/can/m_can/m_can.c
> | @@ -1188,28 +1188,39 @@ static int m_can_echo_tx_event(struct net_device *dev)
> |  
> |  static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
> |  {
> |          u32 new_interrupts = cdev->active_interrupts;
> |          bool enable_rx_timer = false;
> |          bool enable_tx_timer = false;
> |  
> |          if (!cdev->net->irq)
> |                  return;
> |  
> | +        /* If there is a packet in the FIFO then:
> | +         * - start timer
> | +         * - disable not empty IRQ
> | +         * - handle FIFO
>                 ^^^^^^^^^^^
> 
> ...especially this one?
> 
> | +         */
> |          if (cdev->rx_coalesce_usecs_irq > 0 && (ir & (IR_RF0N | IR_RF0W))) {
> |                  enable_rx_timer = true;
> |                  new_interrupts &= ~IR_RF0N;
> |          }
> |          if (cdev->tx_coalesce_usecs_irq > 0 && (ir & (IR_TEFN | IR_TEFW))) {
> |                  enable_tx_timer = true;
> |                  new_interrupts &= ~IR_TEFN;
> |          }
> | +
> | +        /* If:
> | +         * - timer is not going to be start
> | +         * - and timer is not active
> | +         * -> then enable FIFO empty IRQ
> | +         */
> |          if (!enable_rx_timer && !hrtimer_active(&cdev->hrtimer))
> |                  new_interrupts |= IR_RF0N;
> |          if (!enable_tx_timer && !hrtimer_active(&cdev->hrtimer))
> |                  new_interrupts |= IR_TEFN;
> |  
> |          m_can_interrupt_enable(cdev, new_interrupts);
> |          if (enable_rx_timer | enable_tx_timer)
> |                  hrtimer_start(&cdev->hrtimer, cdev->irq_timer_wait,
> |                                HRTIMER_MODE_REL);
> |  }

I can't reproduce the problem I had before. I will drop this patch for
now.

In an upcoming series, however, I would still like to move
can_coalescing_update() to the end of the IRQ handler.

> Currently the m_can_coalescing_update() is called at the beginning of
> the IRQ handler. Does it make sense to move it to the end and pass the
> unmasked M_CAN_IR?

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 --]

Re: [PATCH v2 2/7] can: m_can: only handle active interrupts

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 3952 bytes --]

On Wed Sep 10, 2025 at 5:06 PM CEST, Marc Kleine-Budde wrote:
> On 10.09.2025 16:28:54, Marc Kleine-Budde wrote:
>> On 10.09.2025 10:41:28, Markus Schneider-Pargmann wrote:
>> > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> > > index fe74dbd2c966..16b38e6c3985 100644
>> > > --- a/drivers/net/can/m_can/m_can.c
>> > > +++ b/drivers/net/can/m_can/m_can.c
>> > > @@ -1057,6 +1057,7 @@ static int m_can_poll(struct napi_struct *napi, int quota)
>> > >  	u32 irqstatus;
>> > >  
>> > >  	irqstatus = cdev->irqstatus | m_can_read(cdev, M_CAN_IR);
>> > > +	irqstatus &= cdev->active_interrupts;
>> > >  
>> > >  	work_done = m_can_rx_handler(dev, quota, irqstatus);
>> > >  
>> > > @@ -1243,6 +1244,8 @@ static int m_can_interrupt_handler(struct m_can_classdev *cdev)
>> > >  	}
>> > >  
>> > >  	m_can_coalescing_update(cdev, ir);
>> > > +
>> > > +	ir &= cdev->active_interrupts;
>> > 
>> > m_can_coalescing_update() can change active_interrupts, meaning the
>> > interrupt that caused the interrupt handler to run may be disabled in
>> > active_interrupts above and then masked in this added line. Would that
>> > still work or does it confuse the hardware?
>> 
>> I think m_can_coalescing_update() expects the RX/TX will be cleared. Are
>> the following comments OK...
>> 
>> | diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
>> | index 16b38e6c3985..8cb9cc1cddbf 100644
>> | --- a/drivers/net/can/m_can/m_can.c
>> | +++ b/drivers/net/can/m_can/m_can.c
>> | @@ -1188,28 +1188,39 @@ static int m_can_echo_tx_event(struct net_device *dev)
>> |  
>> |  static void m_can_coalescing_update(struct m_can_classdev *cdev, u32 ir)
>> |  {
>> |          u32 new_interrupts = cdev->active_interrupts;
>> |          bool enable_rx_timer = false;
>> |          bool enable_tx_timer = false;
>> |  
>> |          if (!cdev->net->irq)
>> |                  return;
>> |  
>> | +        /* If there is a packet in the FIFO then:
>> | +         * - start timer
>> | +         * - disable not empty IRQ
>> | +         * - handle FIFO
>>                 ^^^^^^^^^^^
>> 
>> ...especially this one?
>> 
>> | +         */
>> |          if (cdev->rx_coalesce_usecs_irq > 0 && (ir & (IR_RF0N | IR_RF0W))) {
>> |                  enable_rx_timer = true;
>> |                  new_interrupts &= ~IR_RF0N;
>> |          }
>> |          if (cdev->tx_coalesce_usecs_irq > 0 && (ir & (IR_TEFN | IR_TEFW))) {
>> |                  enable_tx_timer = true;
>> |                  new_interrupts &= ~IR_TEFN;
>> |          }
>> | +
>> | +        /* If:
>> | +         * - timer is not going to be start
>> | +         * - and timer is not active
>> | +         * -> then enable FIFO empty IRQ
>> | +         */
>> |          if (!enable_rx_timer && !hrtimer_active(&cdev->hrtimer))
>> |                  new_interrupts |= IR_RF0N;
>> |          if (!enable_tx_timer && !hrtimer_active(&cdev->hrtimer))
>> |                  new_interrupts |= IR_TEFN;
>> |  
>> |          m_can_interrupt_enable(cdev, new_interrupts);
>> |          if (enable_rx_timer | enable_tx_timer)
>> |                  hrtimer_start(&cdev->hrtimer, cdev->irq_timer_wait,
>> |                                HRTIMER_MODE_REL);
>> |  }
>
> I can't reproduce the problem I had before. I will drop this patch for
> now.
>
> In an upcoming series, however, I would still like to move
> can_coalescing_update() to the end of the IRQ handler.

The interrupts are acked just before calling m_can_coalescing_update().
I think ideally the unwanted interrupts should be disabled quick,
especially if there are SPI transfers for handling packets which take
some time.

Another point is to refresh the timer consistently so it doesn't trigger
because we are late refreshing it. The runtime of the interrupt handler
depends on the amount of work in the fifo.

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

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

[Marc Kleine-Budde]

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@pengutronix.de>
---
 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,

-- 
2.51.0

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

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 4099 bytes --]

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@pengutronix.de>

Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.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,


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

[PATCH v2 4/7] can: m_can: m_can_chip_config(): bring up interface in correct state

[Marc Kleine-Budde]

In some SoCs (observed on the STM32MP15) the M_CAN IP core keeps the
CAN state and CAN error counters over an internal reset cycle. An
external reset is not always possible, due to the shared reset with
the other CAN core. This caused the core not always be in Error Active
state when bringing up the controller.

Instead of always setting the CAN state to Error Active in
m_can_chip_config(), fix this by reading and decoding the Protocol
Status Regitser (PSR) and set the CAN state accordingly.

Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support")
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 3edf01b098a4..efd9c23edd4a 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1620,7 +1620,7 @@ static int m_can_start(struct net_device *dev)
 	netdev_queue_set_dql_min_limit(netdev_get_tx_queue(cdev->net, 0),
 				       cdev->tx_max_coalesced_frames);
 
-	cdev->can.state = CAN_STATE_ERROR_ACTIVE;
+	cdev->can.state = m_can_state_get_by_psr(cdev);
 
 	m_can_enable_all_interrupts(cdev);
 

-- 
2.51.0

Re: [PATCH v2 4/7] can: m_can: m_can_chip_config(): bring up interface in correct state

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 1429 bytes --]

On Tue Sep 9, 2025 at 7:53 PM CEST, Marc Kleine-Budde wrote:
> In some SoCs (observed on the STM32MP15) the M_CAN IP core keeps the
> CAN state and CAN error counters over an internal reset cycle. An
> external reset is not always possible, due to the shared reset with
> the other CAN core. This caused the core not always be in Error Active
> state when bringing up the controller.
>
> Instead of always setting the CAN state to Error Active in
> m_can_chip_config(), fix this by reading and decoding the Protocol
> Status Regitser (PSR) and set the CAN state accordingly.
>
> Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support")
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>

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

> ---
>  drivers/net/can/m_can/m_can.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 3edf01b098a4..efd9c23edd4a 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1620,7 +1620,7 @@ static int m_can_start(struct net_device *dev)
>  	netdev_queue_set_dql_min_limit(netdev_get_tx_queue(cdev->net, 0),
>  				       cdev->tx_max_coalesced_frames);
>  
> -	cdev->can.state = CAN_STATE_ERROR_ACTIVE;
> +	cdev->can.state = m_can_state_get_by_psr(cdev);
>  
>  	m_can_enable_all_interrupts(cdev);
>  


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

[PATCH v2 5/7] can: m_can: fix CAN state in system PM

[Marc Kleine-Budde]

A suspend/resume cycle on a down interface results in the interface
coming up in Error Active state. A suspend/resume cycle on an Up
interface will always result in Error Active state, regardless of the
actual CAN state.

During suspend, only set running interfaces to CAN_STATE_SLEEPING.
During resume only touch the CAN state of running interfaces. For
wakeup sources, set the CAN state depending on the Protocol Status
Regitser (PSR), for non wakeup source interfaces m_can_start() will do
the same.

Fixes: e0d1f4816f2a ("can: m_can: add Bosch M_CAN controller support")
Reviewed-by: Markus Schneider-Pargmann <msp@baylibre.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index efd9c23edd4a..d44e348f2417 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2508,12 +2508,11 @@ int m_can_class_suspend(struct device *dev)
 		}
 
 		m_can_clk_stop(cdev);
+		cdev->can.state = CAN_STATE_SLEEPING;
 	}
 
 	pinctrl_pm_select_sleep_state(dev);
 
-	cdev->can.state = CAN_STATE_SLEEPING;
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(m_can_class_suspend);
@@ -2526,8 +2525,6 @@ int m_can_class_resume(struct device *dev)
 
 	pinctrl_pm_select_default_state(dev);
 
-	cdev->can.state = CAN_STATE_ERROR_ACTIVE;
-
 	if (netif_running(ndev)) {
 		ret = m_can_clk_start(cdev);
 		if (ret)
@@ -2545,6 +2542,8 @@ int m_can_class_resume(struct device *dev)
 			if (cdev->ops->init)
 				ret = cdev->ops->init(cdev);
 
+			cdev->can.state = m_can_state_get_by_psr(cdev);
+
 			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
 		} else {
 			ret  = m_can_start(ndev);

-- 
2.51.0

[PATCH v2 6/7] can: m_can: m_can_get_berr_counter(): don't wake up controller if interface is down

[Marc Kleine-Budde]

If the interface is down, the CAN controller might be powered down,
the clock disabled, and/or it's external reset asserted.

Don't wake up the controller to read the CAN bus error counters, if
the interface is down.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index d44e348f2417..9528af8500af 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -790,6 +790,10 @@ static int m_can_get_berr_counter(const struct net_device *dev,
 	struct m_can_classdev *cdev = netdev_priv(dev);
 	int err;
 
+	/* Avoid waking up the controller if the interface is down */
+	if (!(dev->flags & IFF_UP))
+		return 0;
+
 	err = m_can_clk_start(cdev);
 	if (err)
 		return err;

-- 
2.51.0

[PATCH v2 7/7] can: m_can: add optional support for reset

[Marc Kleine-Budde]

In some SoCs (observed on the STM32MP15) the M_CAN IP core keeps the
CAN state and CAN error counters over an internal reset cycle. The
STM32MP15 SoC provides an external reset, which is shared between both
M_CAN cores.

Add support for an optional external reset. Take care of shared
resets, de-assert reset during the probe phase in
m_can_class_register() and while the interface is up, assert the reset
otherwise.

Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 26 +++++++++++++++++++++++---
 drivers/net/can/m_can/m_can.h |  1 +
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 9528af8500af..93085bf1c267 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -23,6 +23,7 @@
 #include <linux/pinctrl/consumer.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
+#include <linux/reset.h>
 
 #include "m_can.h"
 
@@ -1834,6 +1835,7 @@ static int m_can_close(struct net_device *dev)
 
 	close_candev(dev);
 
+	reset_control_assert(cdev->rst);
 	m_can_clk_stop(cdev);
 	phy_power_off(cdev->transceiver);
 
@@ -2076,11 +2078,15 @@ static int m_can_open(struct net_device *dev)
 	if (err)
 		goto out_phy_power_off;
 
+	err = reset_control_deassert(cdev->rst);
+	if (err)
+		goto exit_disable_clks;
+
 	/* open the can device */
 	err = open_candev(dev);
 	if (err) {
 		netdev_err(dev, "failed to open can device\n");
-		goto exit_disable_clks;
+		goto out_reset_control_assert;
 	}
 
 	if (cdev->is_peripheral)
@@ -2136,6 +2142,8 @@ static int m_can_open(struct net_device *dev)
 	else
 		napi_disable(&cdev->napi);
 	close_candev(dev);
+out_reset_control_assert:
+	reset_control_assert(cdev->rst);
 exit_disable_clks:
 	m_can_clk_stop(cdev);
 out_phy_power_off:
@@ -2426,15 +2434,23 @@ int m_can_class_register(struct m_can_classdev *cdev)
 		}
 	}
 
+	cdev->rst = devm_reset_control_get_optional_shared(cdev->dev, NULL);
+	if (IS_ERR(cdev->rst))
+		return PTR_ERR(cdev->rst);
+
 	ret = m_can_clk_start(cdev);
 	if (ret)
 		return ret;
 
+	ret = reset_control_deassert(cdev->rst);
+	if (ret)
+		goto clk_disable;
+
 	if (cdev->is_peripheral) {
 		ret = can_rx_offload_add_manual(cdev->net, &cdev->offload,
 						NAPI_POLL_WEIGHT);
 		if (ret)
-			goto clk_disable;
+			goto out_reset_control_assert;
 	}
 
 	if (!cdev->net->irq) {
@@ -2463,8 +2479,10 @@ int m_can_class_register(struct m_can_classdev *cdev)
 		 KBUILD_MODNAME, cdev->net->irq, cdev->version);
 
 	/* Probe finished
-	 * Stop clocks. They will be reactivated once the M_CAN device is opened
+	 * Assert reset and stop clocks.
+	 * They will be reactivated once the M_CAN device is opened
 	 */
+	reset_control_assert(cdev->rst);
 	m_can_clk_stop(cdev);
 
 	return 0;
@@ -2472,6 +2490,8 @@ int m_can_class_register(struct m_can_classdev *cdev)
 rx_offload_del:
 	if (cdev->is_peripheral)
 		can_rx_offload_del(&cdev->offload);
+out_reset_control_assert:
+	reset_control_assert(cdev->rst);
 clk_disable:
 	m_can_clk_stop(cdev);
 
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index bd4746c63af3..7b7600697c6b 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -86,6 +86,7 @@ struct m_can_classdev {
 	struct device *dev;
 	struct clk *hclk;
 	struct clk *cclk;
+	struct reset_control *rst;
 
 	struct workqueue_struct *tx_wq;
 	struct phy *transceiver;

-- 
2.51.0

Re: [PATCH v2 7/7] can: m_can: add optional support for reset

[Markus Schneider-Pargmann]

[-- Attachment #1: Type: text/plain, Size: 3990 bytes --]

On Tue Sep 9, 2025 at 7:53 PM CEST, Marc Kleine-Budde wrote:
> In some SoCs (observed on the STM32MP15) the M_CAN IP core keeps the
> CAN state and CAN error counters over an internal reset cycle. The
> STM32MP15 SoC provides an external reset, which is shared between both
> M_CAN cores.
>
> Add support for an optional external reset. Take care of shared
> resets, de-assert reset during the probe phase in
> m_can_class_register() and while the interface is up, assert the reset
> otherwise.
>
> Reviewed-by: Philipp Zabel <p.zabel@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
> ---
>  drivers/net/can/m_can/m_can.c | 26 +++++++++++++++++++++++---
>  drivers/net/can/m_can/m_can.h |  1 +
>  2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index 9528af8500af..93085bf1c267 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -23,6 +23,7 @@
>  #include <linux/pinctrl/consumer.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
> +#include <linux/reset.h>
>  
>  #include "m_can.h"
>  
> @@ -1834,6 +1835,7 @@ static int m_can_close(struct net_device *dev)
>  
>  	close_candev(dev);
>  
> +	reset_control_assert(cdev->rst);
>  	m_can_clk_stop(cdev);
>  	phy_power_off(cdev->transceiver);
>  
> @@ -2076,11 +2078,15 @@ static int m_can_open(struct net_device *dev)
>  	if (err)
>  		goto out_phy_power_off;
>  
> +	err = reset_control_deassert(cdev->rst);
> +	if (err)
> +		goto exit_disable_clks;
> +
>  	/* open the can device */
>  	err = open_candev(dev);
>  	if (err) {
>  		netdev_err(dev, "failed to open can device\n");
> -		goto exit_disable_clks;
> +		goto out_reset_control_assert;
>  	}
>  
>  	if (cdev->is_peripheral)
> @@ -2136,6 +2142,8 @@ static int m_can_open(struct net_device *dev)
>  	else
>  		napi_disable(&cdev->napi);
>  	close_candev(dev);
> +out_reset_control_assert:
> +	reset_control_assert(cdev->rst);
>  exit_disable_clks:
>  	m_can_clk_stop(cdev);
>  out_phy_power_off:
> @@ -2426,15 +2434,23 @@ int m_can_class_register(struct m_can_classdev *cdev)
>  		}
>  	}
>  
> +	cdev->rst = devm_reset_control_get_optional_shared(cdev->dev, NULL);
> +	if (IS_ERR(cdev->rst))
> +		return PTR_ERR(cdev->rst);

Should this print an error message here?

Best
Markus

> +
>  	ret = m_can_clk_start(cdev);
>  	if (ret)
>  		return ret;
>  
> +	ret = reset_control_deassert(cdev->rst);
> +	if (ret)
> +		goto clk_disable;
> +
>  	if (cdev->is_peripheral) {
>  		ret = can_rx_offload_add_manual(cdev->net, &cdev->offload,
>  						NAPI_POLL_WEIGHT);
>  		if (ret)
> -			goto clk_disable;
> +			goto out_reset_control_assert;
>  	}
>  
>  	if (!cdev->net->irq) {
> @@ -2463,8 +2479,10 @@ int m_can_class_register(struct m_can_classdev *cdev)
>  		 KBUILD_MODNAME, cdev->net->irq, cdev->version);
>  
>  	/* Probe finished
> -	 * Stop clocks. They will be reactivated once the M_CAN device is opened
> +	 * Assert reset and stop clocks.
> +	 * They will be reactivated once the M_CAN device is opened
>  	 */
> +	reset_control_assert(cdev->rst);
>  	m_can_clk_stop(cdev);
>  
>  	return 0;
> @@ -2472,6 +2490,8 @@ int m_can_class_register(struct m_can_classdev *cdev)
>  rx_offload_del:
>  	if (cdev->is_peripheral)
>  		can_rx_offload_del(&cdev->offload);
> +out_reset_control_assert:
> +	reset_control_assert(cdev->rst);
>  clk_disable:
>  	m_can_clk_stop(cdev);
>  
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index bd4746c63af3..7b7600697c6b 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -86,6 +86,7 @@ struct m_can_classdev {
>  	struct device *dev;
>  	struct clk *hclk;
>  	struct clk *cclk;
> +	struct reset_control *rst;
>  
>  	struct workqueue_struct *tx_wq;
>  	struct phy *transceiver;


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]

Re: [PATCH v2 7/7] can: m_can: add optional support for reset

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 766 bytes --]

On 10.09.2025 11:32:04, Markus Schneider-Pargmann wrote:
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index 9528af8500af..93085bf1c267 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c

[...]

> > +	cdev->rst = devm_reset_control_get_optional_shared(cdev->dev, NULL);
> > +	if (IS_ERR(cdev->rst))
> > +		return PTR_ERR(cdev->rst);
> 
> Should this print an error message here?

makes sense

regards,
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 --]