linux-can.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling
@ 2025-09-29  8:40 Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 1/4] can: m_can: m_can_plat_remove(): add missing pm_runtime_disable() Marc Kleine-Budde
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-09-29  8:40 UTC (permalink / raw)
  To: Chandrasekar Ramakrishnan, Vincent Mailhol, Patrik Flykt,
	Dong Aisheng, Varka Bhadram, Wu Bo, Markus Schneider-Pargmann,
	Philipp Zabel
  Cc: linux-can, kernel, 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 "Error Warning" back to "Error
  Active" (patch 2)
- 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 3+4)

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 v4:
- 5+6: drop patch
- Link to v3: https://patch.msgid.link/20250923-m_can-fix-state-handling-v3-0-06d8baccadbf@pengutronix.de

Changes in v3:
- 2: drop patch
- 3: add Markus's R-b
- 4: add Markus's R-b
- 7: add dev_err_probe() to error path (thanks Markus)
- Link to v2: https://patch.msgid.link/20250909-m_can-fix-state-handling-v2-0-af9fa240b68a@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 (4):
      can: m_can: m_can_plat_remove(): add missing pm_runtime_disable()
      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

 drivers/net/can/m_can/m_can.c          | 64 ++++++++++++++++++++--------------
 drivers/net/can/m_can/m_can_platform.c |  2 +-
 2 files changed, 38 insertions(+), 28 deletions(-)
---
base-commit: 012ea489aedab1a4c08efbd936bb7be91a06d236
change-id: 20250811-m_can-fix-state-handling-0ba4bda6cb8e

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/4] can: m_can: m_can_plat_remove(): add missing pm_runtime_disable()
  2025-09-29  8:40 [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
@ 2025-09-29  8:40 ` Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 2/4] can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active Marc Kleine-Budde
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-09-29  8:40 UTC (permalink / raw)
  To: Chandrasekar Ramakrishnan, Vincent Mailhol, Patrik Flykt,
	Dong Aisheng, Varka Bhadram, Wu Bo, Markus Schneider-Pargmann,
	Philipp Zabel
  Cc: linux-can, kernel, 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/4] can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active
  2025-09-29  8:40 [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 1/4] can: m_can: m_can_plat_remove(): add missing pm_runtime_disable() Marc Kleine-Budde
@ 2025-09-29  8:40 ` Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 3/4] can: m_can: m_can_chip_config(): bring up interface in correct state Marc Kleine-Budde
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-09-29  8:40 UTC (permalink / raw)
  To: Chandrasekar Ramakrishnan, Vincent Mailhol, Patrik Flykt,
	Dong Aisheng, Varka Bhadram, Wu Bo, Markus Schneider-Pargmann,
	Philipp Zabel
  Cc: linux-can, kernel, 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")
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 | 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 fe74dbd2c966..4826b8dcad0f 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 3/4] can: m_can: m_can_chip_config(): bring up interface in correct state
  2025-09-29  8:40 [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 1/4] can: m_can: m_can_plat_remove(): add missing pm_runtime_disable() Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 2/4] can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active Marc Kleine-Budde
@ 2025-09-29  8:40 ` Marc Kleine-Budde
  2025-09-29  8:40 ` [PATCH v4 4/4] can: m_can: fix CAN state in system PM Marc Kleine-Budde
  2025-10-08  8:23 ` [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-09-29  8:40 UTC (permalink / raw)
  To: Chandrasekar Ramakrishnan, Vincent Mailhol, Patrik Flykt,
	Dong Aisheng, Varka Bhadram, Wu Bo, Markus Schneider-Pargmann,
	Philipp Zabel
  Cc: linux-can, kernel, 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")
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 | 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 4826b8dcad0f..9eafd135fcb4 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1617,7 +1617,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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 4/4] can: m_can: fix CAN state in system PM
  2025-09-29  8:40 [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
                   ` (2 preceding siblings ...)
  2025-09-29  8:40 ` [PATCH v4 3/4] can: m_can: m_can_chip_config(): bring up interface in correct state Marc Kleine-Budde
@ 2025-09-29  8:40 ` Marc Kleine-Budde
  2025-10-08  8:23 ` [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-09-29  8:40 UTC (permalink / raw)
  To: Chandrasekar Ramakrishnan, Vincent Mailhol, Patrik Flykt,
	Dong Aisheng, Varka Bhadram, Wu Bo, Markus Schneider-Pargmann,
	Philipp Zabel
  Cc: linux-can, kernel, 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 9eafd135fcb4..c82ea6043d40 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2505,12 +2505,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);
@@ -2523,8 +2522,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)
@@ -2542,6 +2539,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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling
  2025-09-29  8:40 [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
                   ` (3 preceding siblings ...)
  2025-09-29  8:40 ` [PATCH v4 4/4] can: m_can: fix CAN state in system PM Marc Kleine-Budde
@ 2025-10-08  8:23 ` Marc Kleine-Budde
  4 siblings, 0 replies; 6+ messages in thread
From: Marc Kleine-Budde @ 2025-10-08  8:23 UTC (permalink / raw)
  To: Chandrasekar Ramakrishnan, Vincent Mailhol, Patrik Flykt,
	Varka Bhadram, Wu Bo, Markus Schneider-Pargmann, Philipp Zabel
  Cc: linux-can, kernel

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

On 29.09.2025 10:40:12, Marc Kleine-Budde wrote:
> 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 "Error Warning" back to "Error
>   Active" (patch 2)
> - 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 3+4)
> 
> 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>

Added to linux-can. Removed mentioning the DT bindings update from cover
letter as these patches were removed from this series.

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:[~2025-10-08  8:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-29  8:40 [PATCH v4 0/4] can: m_can: fix pm_runtime and CAN state handling Marc Kleine-Budde
2025-09-29  8:40 ` [PATCH v4 1/4] can: m_can: m_can_plat_remove(): add missing pm_runtime_disable() Marc Kleine-Budde
2025-09-29  8:40 ` [PATCH v4 2/4] can: m_can: m_can_handle_state_errors(): fix CAN state transition to Error Active Marc Kleine-Budde
2025-09-29  8:40 ` [PATCH v4 3/4] can: m_can: m_can_chip_config(): bring up interface in correct state Marc Kleine-Budde
2025-09-29  8:40 ` [PATCH v4 4/4] can: m_can: fix CAN state in system PM Marc Kleine-Budde
2025-10-08  8:23 ` [PATCH v4 0/4] can: m_can: fix pm_runtime and 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;
as well as URLs for NNTP newsgroup(s).