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

[PATCH v3] 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()

 drivers/net/can/m_can/tcan4x5x-core.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 31cc9d0abd45..a318f1874b35 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -211,10 +211,26 @@ 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)
 {
-	if (IS_ERR_OR_NULL(reg))
+	struct regulator *reg = priv->power;
+
+	/*
+	 * Check if the power regulator is configured. If it is, just power
+	 * the device on/off. If not, put the device into sleep mode here 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.
+	 * Datasheet: TCAN4550, section "8.4.3 Sleep Mode"
+	 * https://www.ti.com/lit/gpn/tcan4550
+	 */
+	if (IS_ERR_OR_NULL(reg)) {
+		if (priv->reset_gpio && !enable)
+			return regmap_update_bits(priv->regmap, TCAN4X5X_CONFIG,
+						  TCAN4X5X_MODE_SEL_MASK,
+						  TCAN4X5X_MODE_SLEEP);
+
 		return 0;
+	}
 
 	if (enable)
 		return regulator_enable(reg);
@@ -476,7 +492,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 +547,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 +559,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 v3] 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] The transceiver is not put into sleep mode if a regulator is configured, defeating the power-saving goal for shared or always-on regulators.
--

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

can: tcan4x5x: put tcan into sleep when removing driver

This commit attempts to put the tcan4x5x transceiver into sleep mode when
the driver is removed to reduce power consumption. It updates
tcan4x5x_power_enable to send the sleep command via SPI if the RST pin is
available and no regulator is present.

> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 31cc9d0abd453..a318f1874b351 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
> @@ -211,10 +211,26 @@ 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)
>  {
> -	if (IS_ERR_OR_NULL(reg))
> +	struct regulator *reg = priv->power;
> +
> +	/*
> +	 * Check if the power regulator is configured. If it is, just power
> +	 * the device on/off. If not, put the device into sleep mode here 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.
> +	 * Datasheet: TCAN4550, section "8.4.3 Sleep Mode"
> +	 * https://www.ti.com/lit/gpn/tcan4550
> +	 */
> +	if (IS_ERR_OR_NULL(reg)) {
> +		if (priv->reset_gpio && !enable)
> +			return regmap_update_bits(priv->regmap, TCAN4X5X_CONFIG,
> +						  TCAN4X5X_MODE_SEL_MASK,
> +						  TCAN4X5X_MODE_SLEEP);
> +
>  		return 0;
> +	}

[Severity: Medium]
Will this miss the power savings if the regulator is shared or marked as
always-on?

Because the sleep command is nested inside the IS_ERR_OR_NULL(reg) check,
the device is not put into sleep mode if a regulator is configured. Instead,
the function continues to the else block and calls regulator_disable().

If the regulator is shared with other active devices or is marked always-on in
the device tree, regulator_disable() will only decrement the reference count
without physically turning off the power. The transceiver will stay fully
powered in its active operating mode.

Should the driver issue the sleep command via SPI whenever priv->reset_gpio
is available, before disabling the regulator, to ensure power is saved regardless
of the physical regulator state?

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

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

[Marc Kleine-Budde]

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

On 03.08.2026 14:09:16, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The transceiver is not put into sleep mode if a regulator is configured, defeating the power-saving goal for shared or always-on regulators.
> --
>
> --- Patch [1]: [PATCH v3] can: tcan4x5x: put tcan into sleep when removing driver ---
> commit 153dfa0940acefa28cd89e367eedf818096ade5b
> Author: Sean Nyekjaer <sean@geanix.com>
>
> can: tcan4x5x: put tcan into sleep when removing driver
>
> This commit attempts to put the tcan4x5x transceiver into sleep mode when
> the driver is removed to reduce power consumption. It updates
> tcan4x5x_power_enable to send the sleep command via SPI if the RST pin is
> available and no regulator is present.
>
> > diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> > index 31cc9d0abd453..a318f1874b351 100644
> > --- a/drivers/net/can/m_can/tcan4x5x-core.c
> > +++ b/drivers/net/can/m_can/tcan4x5x-core.c
> > @@ -211,10 +211,26 @@ 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)
> >  {
> > -	if (IS_ERR_OR_NULL(reg))
> > +	struct regulator *reg = priv->power;
> > +
> > +	/*
> > +	 * Check if the power regulator is configured. If it is, just power
> > +	 * the device on/off. If not, put the device into sleep mode here 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.
> > +	 * Datasheet: TCAN4550, section "8.4.3 Sleep Mode"
> > +	 * https://www.ti.com/lit/gpn/tcan4550
> > +	 */
> > +	if (IS_ERR_OR_NULL(reg)) {
> > +		if (priv->reset_gpio && !enable)
> > +			return regmap_update_bits(priv->regmap, TCAN4X5X_CONFIG,
> > +						  TCAN4X5X_MODE_SEL_MASK,
> > +						  TCAN4X5X_MODE_SLEEP);
> > +
> >  		return 0;
> > +	}
>
> [Severity: Medium]
> Will this miss the power savings if the regulator is shared or marked as
> always-on?
>
> Because the sleep command is nested inside the IS_ERR_OR_NULL(reg) check,
> the device is not put into sleep mode if a regulator is configured. Instead,
> the function continues to the else block and calls regulator_disable().
>
> If the regulator is shared with other active devices or is marked always-on in
> the device tree, regulator_disable() will only decrement the reference count
> without physically turning off the power. The transceiver will stay fully
> powered in its active operating mode.
>
> Should the driver issue the sleep command via SPI whenever priv->reset_gpio
> is available, before disabling the regulator, to ensure power is saved regardless
> of the physical regulator state?

Sean, sounds like a valid point to me, what do you think? The additional
SPI message, even if the device is powered off, should not hurt.

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