drivers: can: mcp251xfd: Add support for sleep mode #100520

tomridl · 2025-12-04T12:13:42Z

Add support for MCP251xFD sleep mode using device power management. When in sleep mode, controller will wake automatically on CAN activity via WAKIF.
Sleep mode is entered by calling
pm_device_action_run(can_dev, PM_DEVICE_ACTION_SUSPEND);
Sleep mode can be exited by calling
pm_device_action_run(can_dev, PM_DEVICE_ACTION_RESUME);
Add config options to enable power management support as well as an option to enable management of transceiver standby automatically when sleep mode is entered.

Add support for MCP251xFD sleep mode using device power management. When in sleep mode, controller will wake automatically on CAN activity via WAKIF. Enter sleep mode by calling: pm_device_action_run(can_dev, PM_DEVICE_ACTION_SUSPEND); Leave sleep mode by calling: pm_device_action_run(can_dev, PM_DEVICE_ACTION_RESUME); Add config options to enable power management support as well as an option to enable management of transceiver standby automatically when sleep mode is entered. Signed-off-by: Tomas Ridl <[email protected]>

github-actions · 2025-12-04T12:14:22Z

Hello @tomridl, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

henrikbrixandersen

Thanks you for submitting this. A few initial comments below:

henrikbrixandersen · 2025-12-04T12:24:19Z

drivers/can/Kconfig.mcp251xfd

+config CAN_MCP251XFD_PM_DEVICE
+	bool "Enable power management support"
+	default n
+	depends on PM_DEVICE
+	help
+	  Enable power management support for the MCP251XFD CAN controller.
+	  When enabled, the driver implements suspend and resume callbacks
+	  that transition the device to sleep mode on suspend and restore
+	  the previous operational mode on resume.


This should not be a dedicated, driver-specific Kconfig. Enabling CONFIG_PM_DEVICE should be sufficient.

Will do - the reason for the extra config was that entering sleep mode could cause CAN packets to be missed on wakeup, so devices that implement CONFIG_PM for other reasons could optionally prevent the CAN controller going to sleep if it is critical to receive every CAN packet. Could an option perhaps be added to the dts instead?
Edit: Doh, if they don't want to use sleep mode then just don't call pm_device_action_run(can_dev, PM_DEVICE_ACTION_SUSPEND);

henrikbrixandersen · 2025-12-04T12:25:16Z

drivers/can/can_mcp251xfd.c

-		if (requested_mode ==  MCP251XFD_REG_CON_MODE_CAN2_0 ||
-		    requested_mode ==  MCP251XFD_REG_CON_MODE_EXT_LOOPBACK ||
+		if (requested_mode == MCP251XFD_REG_CON_MODE_CAN2_0 ||
+		    requested_mode == MCP251XFD_REG_CON_MODE_EXT_LOOPBACK ||


Please do not mix unrelated whitespace changes with functional changes. Same goes for the rest of the file.

henrikbrixandersen · 2025-12-04T12:26:09Z

drivers/can/can_mcp251xfd.c

@@ -1099,6 +1111,24 @@ static void mcp251xfd_handle_interrupts(const struct device *dev)
 			}
 		}

+#if defined(CONFIG_CAN_MCP251XFD_PM_DEVICE)
+		if ((reg_int & MCP251XFD_REG_INT_WAKIF) != 0) {
+			LOG_INF("WAKIF Interrupt - CAN bus activity wake");


Please reduce the log level here to DBG.

henrikbrixandersen · 2025-12-04T12:29:16Z

drivers/can/can_mcp251xfd.c

+	/* If in operational mode (NORMAL/LISTENONLY), first abort any pending TX
+	 * and transition to CONFIG mode for clean sleep entry.
+	 */


Please invert this to disallow entering sleep mode while in operational mode (see discussion in #96039 for details).

Will the MCP251xFD retain CAN RX filter configuration while in sleep mode?

Yes, the MCP251xFD retains CAN RX filter configuration while in sleep mode (the same is not true for LP mode, however LP mode has not been implemented here.
According to the MCP25XXFD family reference manual, sleep mode can be entered from normal mode however in testing I found it more reliable to switch to config mode first. I will change this to match the implementation for tcan4x5x and only allow entry from configuration mode.

henrikbrixandersen · 2025-12-04T12:30:09Z

drivers/can/can_mcp251xfd.c

@@ -1688,6 +1721,274 @@ static int mcp251xfd_init(const struct device *dev)
 	return ret;
 }

+#if defined(CONFIG_CAN_MCP251XFD_PM_DEVICE)
+static int mcp251xfd_enter_sleep_mode(const struct device *dev)


This function is very long and complicated. Please consider refactoring it for readability.

henrikbrixandersen · 2025-12-04T12:31:34Z

drivers/can/can_mcp251xfd.c

+		ret = mcp251xfd_reg_check_value_wtimeout(
+			dev, MCP251XFD_REG_CON,
+			FIELD_PREP(MCP251XFD_REG_CON_OPMOD_MASK, MCP251XFD_REG_CON_MODE_CONFIG),
+			MCP251XFD_REG_CON_OPMOD_MASK, MCP251XFD_MODE_CHANGE_TIMEOUT_USEC,
+			MCP251XFD_MODE_CHANGE_RETRIES, true);


Please avoid formatting like this (Python-like) where a function call ends with ( on a line. Put the arguments on the same line up to 100 characters in width.

henrikbrixandersen · 2025-12-04T12:34:41Z

drivers/can/can_mcp251xfd.c

+#if defined(CONFIG_CAN_MCP251XFD_TRANSCEIVER_STANDBY_ON_SLEEP)
+			if (dev_cfg->common.phy != NULL) {
+				/* Put transceiver in standby for lowest power */
+				(void)can_transceiver_disable(dev_cfg->common.phy);
+			}
+#endif
+		}
+		break;
+
+	case PM_DEVICE_ACTION_RESUME:
+		LOG_DBG("PM resume requested");
+
+#if defined(CONFIG_CAN_MCP251XFD_TRANSCEIVER_STANDBY_ON_SLEEP)
+		if (dev_cfg->common.phy != NULL) {
+			/* Bring transceiver out of standby */
+			(void)can_transceiver_enable(dev_cfg->common.phy, dev_data->common.mode);
+		}
+#endif


Suspending/disabling the CAN transceiver due to PM is the task of the CAN transceiver driver (not the individual CAN controller driver), which needs to implement device PM by itself.

It appears that the existing mcp251xfd_stop and mcp251xfd_start functions already control the transceiver standby, so switching to configuration mode using can_stop (putting the device in configuration mode) will already put the transceiver to sleep. I will switch the PM_DEVICE_ACTION_RESUME to use mcp251xfd_start which will then switch the transceiver out of standby.

Since the CAN controller will be in stopped mode when entering sleep mode, resuming should not put it into started mode - that's up to the application.

Got it - will restore to config mode after wake from sleep and leave it up to the app to start.

Although then I'm not sure how to restart the controller when waking from CAN activity - the state_changed_callback doesn't have a defined state for WAKIF so how should this be handled?

Add support for MCP251xFD sleep mode using device power management. When in sleep mode, controller will wake automatically on CAN activity via WAKIF. Enter sleep mode by calling: pm_device_action_run(can_dev, PM_DEVICE_ACTION_SUSPEND); Leave sleep mode by calling: pm_device_action_run(can_dev, PM_DEVICE_ACTION_RESUME); Signed-off-by: Tomas Ridl <[email protected]>

Remove transceiver interactions from PM handler. Signed-off-by: Tomas Ridl <[email protected]>

Removed unused variable. Signed-off-by: Tomas Ridl <[email protected]>

Removed unused variable dev_cfg. Signed-off-by: Tomas Ridl <[email protected]>

sonarqubecloud · 2025-12-05T12:58:19Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

zephyrbot added the area: CAN label Dec 4, 2025

zephyrbot requested review from alexanderwachter, henrikbrixandersen, martinjaeger and str4t0m December 4, 2025 12:15

zephyrbot assigned henrikbrixandersen Dec 4, 2025

henrikbrixandersen requested changes Dec 4, 2025

View reviewed changes

tomridl added 4 commits December 5, 2025 13:08

drivers: can: mcp251xfd: Add support for sleep mode

52c0722

Remove transceiver interactions from PM handler. Signed-off-by: Tomas Ridl <[email protected]>

drivers: can: mcp251xfd: Add support for sleep mode

06a210f

Removed unused variable. Signed-off-by: Tomas Ridl <[email protected]>

drivers: can: mcp251xfd: Add support for sleep mode

d396dbf

Removed unused variable dev_cfg. Signed-off-by: Tomas Ridl <[email protected]>

drivers: can: mcp251xfd: Add support for sleep mode #100520

Are you sure you want to change the base?

drivers: can: mcp251xfd: Add support for sleep mode #100520

Uh oh!

Conversation

tomridl commented Dec 4, 2025

Uh oh!

github-actions bot commented Dec 4, 2025

Uh oh!

henrikbrixandersen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tomridl Dec 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Dec 5, 2025

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tomridl Dec 4, 2025 •

edited

Loading