[PATCH v4] can: tcan4x5x: put tcan into sleep when removing driver

[PATCH v4] can: tcan4x5x: put tcan into sleep when removing driver

[Sean Nyekjaer]

Put the tcan4x5x transceiver into sleep mode when the driver is
removed, instead of leaving it in its current operating mode.
This reduces power consumption(3mA@12V) once the driver is
no longer bound to the device.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Changes since v1:
 - Moved enter sleep mode into tcan4x5x_power_enable()

Changes since v2:
 - Added comment about RST pin
 - Fixed all calls to tcan4x5x_power_enable()

Changes since v3:
 - When powering off, always put the device into sleep mode (if the RST pin is
   present). This will ensure the device is in sleep mode even if the
   power regulator is shared.

 drivers/net/can/m_can/tcan4x5x-core.c | 29 +++++++++++++++++++++++----
 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 31cc9d0abd45..d8fd219a27cc 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -211,8 +211,29 @@ static int tcan4x5x_write_fifo(struct m_can_classdev *cdev,
 	return regmap_bulk_write(priv->regmap, TCAN4X5X_MRAM_START + addr_offset, val, val_count);
 }
 
-static int tcan4x5x_power_enable(struct regulator *reg, int enable)
+static int tcan4x5x_power_enable(struct tcan4x5x_priv *priv, int enable)
 {
+	struct regulator *reg = priv->power;
+	int ret;
+
+	/*
+	 * Put the device into sleep mode if the RST pin is available,
+	 * since a wake-up event, RST pin toggle, or power cycle are the only
+	 * ways to exit sleep mode.
+	 * Redundant if the regulator is exclusive to this device, but that
+	 * can't be determined here.
+	 *
+	 * Datasheet: TCAN4550, section "8.4.3 Sleep Mode"
+	 * https://www.ti.com/lit/gpn/tcan4550
+	 */
+	if (priv->reset_gpio && !enable) {
+		ret = regmap_update_bits(priv->regmap, TCAN4X5X_CONFIG,
+					 TCAN4X5X_MODE_SEL_MASK,
+					 TCAN4X5X_MODE_SLEEP);
+		if (ret)
+			return ret;
+	}
+
 	if (IS_ERR_OR_NULL(reg))
 		return 0;
 
@@ -476,7 +497,7 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
 		goto out_m_can_class_free_dev;
 	}
 
-	ret = tcan4x5x_power_enable(priv->power, 1);
+	ret = tcan4x5x_power_enable(priv, 1);
 	if (ret) {
 		dev_err(&spi->dev, "Enabling regulator failed %pe\n",
 			ERR_PTR(ret));
@@ -531,7 +552,7 @@ static int tcan4x5x_can_probe(struct spi_device *spi)
 	return 0;
 
 out_power:
-	tcan4x5x_power_enable(priv->power, 0);
+	tcan4x5x_power_enable(priv, 0);
  out_m_can_class_free_dev:
 	m_can_class_free_dev(mcan_class->net);
 	return ret;
@@ -543,7 +564,7 @@ static void tcan4x5x_can_remove(struct spi_device *spi)
 
 	m_can_class_unregister(&priv->cdev);
 
-	tcan4x5x_power_enable(priv->power, 0);
+	tcan4x5x_power_enable(priv, 0);
 
 	m_can_class_free_dev(priv->cdev.net);
 }
-- 
2.55.0

Re: [PATCH v4] can: tcan4x5x: put tcan into sleep when removing driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Early return on SPI error in `tcan4x5x_power_enable(priv, 0)` bypasses `regulator_disable()`, causing a regulator enable count leak.
--

--- Patch [1]: [PATCH v4] can: tcan4x5x: put tcan into sleep when removing driver ---
commit 277a5e1f34c213275f7c42975ff590a923bc1148
Author: Sean Nyekjaer <sean@geanix.com>

can: tcan4x5x: put tcan into sleep when removing driver

This commit modifies the driver to put the TCAN4x5x transceiver into sleep
mode upon removal. It issues a sleep command via SPI to reduce power
consumption when the driver unbinds. The sleep command is integrated into
tcan4x5x_power_enable().

> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 31cc9d0abd453..d8fd219a27cc5 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c

[ ... ]

> -static int tcan4x5x_power_enable(struct regulator *reg, int enable)
> +static int tcan4x5x_power_enable(struct tcan4x5x_priv *priv, int enable)
>  {
> +	struct regulator *reg = priv->power;
> +	int ret;
> +
> +	/*
> +	 * Put the device into sleep mode if the RST pin is available,
> +	 * since a wake-up event, RST pin toggle, or power cycle are the only
> +	 * ways to exit sleep mode.
> +	 * Redundant if the regulator is exclusive to this device, but that
> +	 * can't be determined here.
> +	 *
> +	 * Datasheet: TCAN4550, section "8.4.3 Sleep Mode"
> +	 * https://www.ti.com/lit/gpn/tcan4550
> +	 */
> +	if (priv->reset_gpio && !enable) {
> +		ret = regmap_update_bits(priv->regmap, TCAN4X5X_CONFIG,
> +					 TCAN4X5X_MODE_SEL_MASK,
> +					 TCAN4X5X_MODE_SLEEP);
> +		if (ret)
> +			return ret;

[Severity: Medium]
If this regmap_update_bits() fails over SPI during the power-off sequence,
could this early return bypass the regulator_disable() call below?

It looks like this might leak the regulator enable count, which would
prevent the regulator from ever being disabled and defeat the power-saving
purpose of the patch.

> +	}
> +
>  	if (IS_ERR_OR_NULL(reg))
>  		return 0;
>  
>  	if (enable)
>  		return regulator_enable(reg);
>  	else
>  		return regulator_disable(reg);
>  }

[ ... ]

> @@ -543,7 +564,7 @@ static void tcan4x5x_can_remove(struct spi_device *spi)
>  
>  	m_can_class_unregister(&priv->cdev);
>  
> -	tcan4x5x_power_enable(priv->power, 0);
> +	tcan4x5x_power_enable(priv, 0);

[Severity: Medium]
Because tcan4x5x_can_remove() ignores the return value of
tcan4x5x_power_enable(), any error returned from the regmap_update_bits()
failure mentioned above would be silently dropped. 

Would it be safer to ensure the regulator is still disabled even if the
SPI command fails, so the failure doesn't silently keep the power on?

>  
>  	m_can_class_free_dev(priv->cdev.net);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260804113027.1626442-1-sean@geanix.com?part=1

Re: [PATCH v4] can: tcan4x5x: put tcan into sleep when removing driver

[Marc Kleine-Budde]

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

On 04.08.2026 13:30:26, Sean Nyekjaer wrote:
> Put the tcan4x5x transceiver into sleep mode when the driver is
> removed, instead of leaving it in its current operating mode.
> This reduces power consumption(3mA@12V) once the driver is
> no longer bound to the device.
>
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
> Changes since v1:
>  - Moved enter sleep mode into tcan4x5x_power_enable()
>
> Changes since v2:
>  - Added comment about RST pin
>  - Fixed all calls to tcan4x5x_power_enable()
>
> Changes since v3:
>  - When powering off, always put the device into sleep mode (if the RST pin is
>    present). This will ensure the device is in sleep mode even if the
>    power regulator is shared.
>
>  drivers/net/can/m_can/tcan4x5x-core.c | 29 +++++++++++++++++++++++----
>  1 file changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 31cc9d0abd45..d8fd219a27cc 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
> @@ -211,8 +211,29 @@ static int tcan4x5x_write_fifo(struct m_can_classdev *cdev,
>  	return regmap_bulk_write(priv->regmap, TCAN4X5X_MRAM_START + addr_offset, val, val_count);
>  }
>
> -static int tcan4x5x_power_enable(struct regulator *reg, int enable)
> +static int tcan4x5x_power_enable(struct tcan4x5x_priv *priv, int enable)
>  {
> +	struct regulator *reg = priv->power;
> +	int ret;
> +
> +	/*
> +	 * Put the device into sleep mode if the RST pin is available,
> +	 * since a wake-up event, RST pin toggle, or power cycle are the only
> +	 * ways to exit sleep mode.
> +	 * Redundant if the regulator is exclusive to this device, but that
> +	 * can't be determined here.
> +	 *
> +	 * Datasheet: TCAN4550, section "8.4.3 Sleep Mode"
> +	 * https://www.ti.com/lit/gpn/tcan4550
> +	 */
> +	if (priv->reset_gpio && !enable) {
> +		ret = regmap_update_bits(priv->regmap, TCAN4X5X_CONFIG,
> +					 TCAN4X5X_MODE_SEL_MASK,
> +					 TCAN4X5X_MODE_SLEEP);
> +		if (ret)
> +			return ret;

As sashiko pointed out, maybe only log a error here and continue.

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: 228 bytes --]