[PATCH 0/3] can: tcan4x5x/m_can: use standby mode when down and in suspend

[PATCH 0/3] can: tcan4x5x/m_can: use standby mode when down and in suspend

[Sean Nyekjaer]

When downing the tcan4x5x there is no reason to keep the tcan4x5x in
"normal" mode and waste power.
So set standby mode when the interface is down and normal mode when
interface is up.

Also when going into suspend, set the tcan4x5x into standby mode. The
tcan4x5x can still be used as a wake-source when in standby as low power
rx is enabled.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
Sean Nyekjaer (3):
      can: m_can: add deinit callback
      can: tcan4x5x: add deinit callback to set standby mode
      can: m_can: call deinit callback when going into suspend.

 drivers/net/can/m_can/m_can.c         | 10 ++++++++++
 drivers/net/can/m_can/m_can.h         |  1 +
 drivers/net/can/m_can/tcan4x5x-core.c | 12 ++++++++++++
 3 files changed, 23 insertions(+)
---
base-commit: 2b2a9a08f8f0b904ea2bc61db3374421b0f944a6
change-id: 20241107-tcan-standby-def358771b2b

Best regards,
-- 
Sean Nyekjaer <sean@geanix.com>

[PATCH 1/3] can: m_can: add deinit callback

[Sean Nyekjaer]

This is added in preparation for calling standby mode in the tcan4x5x
driver or other users of m_can.
For the tcan4x5x; If Vsup is 12V, standby mode will save 7-8mA, when
the interface is down.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 drivers/net/can/m_can/m_can.c | 3 +++
 drivers/net/can/m_can/m_can.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a7b3bc439ae596527493a73d62b4b7a120ae4e49..a171ff860b7c6992846ae8d615640a40b623e0cb 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1756,6 +1756,9 @@ static void m_can_stop(struct net_device *dev)
 
 	/* set the state as STOPPED */
 	cdev->can.state = CAN_STATE_STOPPED;
+
+	if (cdev->ops->deinit)
+		cdev->ops->deinit(cdev);
 }
 
 static int m_can_close(struct net_device *dev)
diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
index 92b2bd8628e6b31370f4accbc2e28f3b2257a71d..6206535341a22a68d7c5570f619e6c4d05e6fcf4 100644
--- a/drivers/net/can/m_can/m_can.h
+++ b/drivers/net/can/m_can/m_can.h
@@ -68,6 +68,7 @@ struct m_can_ops {
 	int (*write_fifo)(struct m_can_classdev *cdev, int addr_offset,
 			  const void *val, size_t val_count);
 	int (*init)(struct m_can_classdev *cdev);
+	int (*deinit)(struct m_can_classdev *cdev);
 };
 
 struct m_can_tx_op {

-- 
2.46.2

Re: [PATCH 1/3] can: m_can: add deinit callback

[Simon Horman]

On Mon, Nov 11, 2024 at 11:51:23AM +0100, Sean Nyekjaer wrote:
> This is added in preparation for calling standby mode in the tcan4x5x
> driver or other users of m_can.
> For the tcan4x5x; If Vsup is 12V, standby mode will save 7-8mA, when
> the interface is down.
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
>  drivers/net/can/m_can/m_can.c | 3 +++
>  drivers/net/can/m_can/m_can.h | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index a7b3bc439ae596527493a73d62b4b7a120ae4e49..a171ff860b7c6992846ae8d615640a40b623e0cb 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1756,6 +1756,9 @@ static void m_can_stop(struct net_device *dev)
>  
>  	/* set the state as STOPPED */
>  	cdev->can.state = CAN_STATE_STOPPED;
> +
> +	if (cdev->ops->deinit)
> +		cdev->ops->deinit(cdev);

Hi Sean,

Perhaps this implementation is in keeping with other m_can code, but
I am wondering if either the return value of the callback be returned to
the caller, or the return type of the callback be changed to void?

Similarly for calls to callbacks in in patch 3/3.

>  }
>  
>  static int m_can_close(struct net_device *dev)
> diff --git a/drivers/net/can/m_can/m_can.h b/drivers/net/can/m_can/m_can.h
> index 92b2bd8628e6b31370f4accbc2e28f3b2257a71d..6206535341a22a68d7c5570f619e6c4d05e6fcf4 100644
> --- a/drivers/net/can/m_can/m_can.h
> +++ b/drivers/net/can/m_can/m_can.h
> @@ -68,6 +68,7 @@ struct m_can_ops {
>  	int (*write_fifo)(struct m_can_classdev *cdev, int addr_offset,
>  			  const void *val, size_t val_count);
>  	int (*init)(struct m_can_classdev *cdev);
> +	int (*deinit)(struct m_can_classdev *cdev);
>  };
>  
>  struct m_can_tx_op {
> 
> -- 
> 2.46.2
> 
> 

Re: [PATCH 1/3] can: m_can: add deinit callback

[Marc Kleine-Budde]

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

On 12.11.2024 14:46:03, Simon Horman wrote:
> On Mon, Nov 11, 2024 at 11:51:23AM +0100, Sean Nyekjaer wrote:
> > This is added in preparation for calling standby mode in the tcan4x5x
> > driver or other users of m_can.
> > For the tcan4x5x; If Vsup is 12V, standby mode will save 7-8mA, when
> > the interface is down.
> > 
> > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > ---
> >  drivers/net/can/m_can/m_can.c | 3 +++
> >  drivers/net/can/m_can/m_can.h | 1 +
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index a7b3bc439ae596527493a73d62b4b7a120ae4e49..a171ff860b7c6992846ae8d615640a40b623e0cb 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -1756,6 +1756,9 @@ static void m_can_stop(struct net_device *dev)
> >  
> >  	/* set the state as STOPPED */
> >  	cdev->can.state = CAN_STATE_STOPPED;
> > +
> > +	if (cdev->ops->deinit)
> > +		cdev->ops->deinit(cdev);
> 
> Hi Sean,
> 
> Perhaps this implementation is in keeping with other m_can code, but
> I am wondering if either the return value of the callback be returned to
> the caller, or the return type of the callback be changed to void?
> 
> Similarly for calls to callbacks in in patch 3/3.

please take care of errors/return values.

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 1/3] can: m_can: add deinit callback

[Sean Nyekjaer]

On Thu, Nov 14, 2024 at 10:34:23AM +0100, Marc Kleine-Budde wrote:
> On 12.11.2024 14:46:03, Simon Horman wrote:
> > On Mon, Nov 11, 2024 at 11:51:23AM +0100, Sean Nyekjaer wrote:
> > > This is added in preparation for calling standby mode in the tcan4x5x
> > > driver or other users of m_can.
> > > For the tcan4x5x; If Vsup is 12V, standby mode will save 7-8mA, when
> > > the interface is down.
> > > 
> > > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > > ---
> > >  drivers/net/can/m_can/m_can.c | 3 +++
> > >  drivers/net/can/m_can/m_can.h | 1 +
> > >  2 files changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > > index a7b3bc439ae596527493a73d62b4b7a120ae4e49..a171ff860b7c6992846ae8d615640a40b623e0cb 100644
> > > --- a/drivers/net/can/m_can/m_can.c
> > > +++ b/drivers/net/can/m_can/m_can.c
> > > @@ -1756,6 +1756,9 @@ static void m_can_stop(struct net_device *dev)
> > >  
> > >  	/* set the state as STOPPED */
> > >  	cdev->can.state = CAN_STATE_STOPPED;
> > > +
> > > +	if (cdev->ops->deinit)
> > > +		cdev->ops->deinit(cdev);
> > 
> > Hi Sean,
> > 
> > Perhaps this implementation is in keeping with other m_can code, but
> > I am wondering if either the return value of the callback be returned to
> > the caller, or the return type of the callback be changed to void?
> > 
> > Similarly for calls to callbacks in in patch 3/3.
> 
> please take care of errors/return values.
> 

Will do.
It's also missing for the init callback. Would you like this series to
fix that?

/Sean

Re: [PATCH 1/3] can: m_can: add deinit callback

[Marc Kleine-Budde]

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

On 14.11.2024 10:36:48, Sean Nyekjaer wrote:
> On Thu, Nov 14, 2024 at 10:34:23AM +0100, Marc Kleine-Budde wrote:
> > On 12.11.2024 14:46:03, Simon Horman wrote:
> > > On Mon, Nov 11, 2024 at 11:51:23AM +0100, Sean Nyekjaer wrote:
> > > > This is added in preparation for calling standby mode in the tcan4x5x
> > > > driver or other users of m_can.
> > > > For the tcan4x5x; If Vsup is 12V, standby mode will save 7-8mA, when
> > > > the interface is down.
> > > > 
> > > > Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> > > > ---
> > > >  drivers/net/can/m_can/m_can.c | 3 +++
> > > >  drivers/net/can/m_can/m_can.h | 1 +
> > > >  2 files changed, 4 insertions(+)
> > > > 
> > > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > > > index a7b3bc439ae596527493a73d62b4b7a120ae4e49..a171ff860b7c6992846ae8d615640a40b623e0cb 100644
> > > > --- a/drivers/net/can/m_can/m_can.c
> > > > +++ b/drivers/net/can/m_can/m_can.c
> > > > @@ -1756,6 +1756,9 @@ static void m_can_stop(struct net_device *dev)
> > > >  
> > > >  	/* set the state as STOPPED */
> > > >  	cdev->can.state = CAN_STATE_STOPPED;
> > > > +
> > > > +	if (cdev->ops->deinit)
> > > > +		cdev->ops->deinit(cdev);
> > > 
> > > Hi Sean,
> > > 
> > > Perhaps this implementation is in keeping with other m_can code, but
> > > I am wondering if either the return value of the callback be returned to
> > > the caller, or the return type of the callback be changed to void?
> > > 
> > > Similarly for calls to callbacks in in patch 3/3.
> > 
> > please take care of errors/return values.
> > 
> 
> Will do.
> It's also missing for the init callback. Would you like this series to
> fix that?

If the patches don't conflict, please make it a separate patch.

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

[PATCH 2/3] can: tcan4x5x: add deinit callback to set standby mode

[Sean Nyekjaer]

At Vsup 12V, standby mode will save 7-8mA, when the interface is
down.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 drivers/net/can/m_can/tcan4x5x-core.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 2f73bf3abad889c222f15c39a3d43de1a1cf5fbb..c8336750cdc276b539dde7555b2510fba0d0da75 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -270,6 +270,17 @@ static int tcan4x5x_init(struct m_can_classdev *cdev)
 	return ret;
 }
 
+static int tcan4x5x_deinit(struct m_can_classdev *cdev)
+{
+	struct tcan4x5x_priv *tcan4x5x = cdev_to_priv(cdev);
+	int ret = 0;
+
+	ret = regmap_update_bits(tcan4x5x->regmap, TCAN4X5X_CONFIG,
+				 TCAN4X5X_MODE_SEL_MASK, TCAN4X5X_MODE_STANDBY);
+
+	return ret;
+};
+
 static int tcan4x5x_disable_wake(struct m_can_classdev *cdev)
 {
 	struct tcan4x5x_priv *tcan4x5x = cdev_to_priv(cdev);
@@ -359,6 +370,7 @@ static int tcan4x5x_get_gpios(struct m_can_classdev *cdev,
 
 static const struct m_can_ops tcan4x5x_ops = {
 	.init = tcan4x5x_init,
+	.deinit = tcan4x5x_deinit,
 	.read_reg = tcan4x5x_read_reg,
 	.write_reg = tcan4x5x_write_reg,
 	.write_fifo = tcan4x5x_write_fifo,

-- 
2.46.2

Re: [PATCH 2/3] can: tcan4x5x: add deinit callback to set standby mode

[Marc Kleine-Budde]

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

On 11.11.2024 11:51:24, Sean Nyekjaer wrote:
> At Vsup 12V, standby mode will save 7-8mA, when the interface is
> down.
> 
> Signed-off-by: Sean Nyekjaer <sean@geanix.com>
> ---
>  drivers/net/can/m_can/tcan4x5x-core.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
> index 2f73bf3abad889c222f15c39a3d43de1a1cf5fbb..c8336750cdc276b539dde7555b2510fba0d0da75 100644
> --- a/drivers/net/can/m_can/tcan4x5x-core.c
> +++ b/drivers/net/can/m_can/tcan4x5x-core.c
> @@ -270,6 +270,17 @@ static int tcan4x5x_init(struct m_can_classdev *cdev)
>  	return ret;
>  }
>  
> +static int tcan4x5x_deinit(struct m_can_classdev *cdev)
> +{
> +	struct tcan4x5x_priv *tcan4x5x = cdev_to_priv(cdev);
> +	int ret = 0;
> +
> +	ret = regmap_update_bits(tcan4x5x->regmap, TCAN4X5X_CONFIG,
> +				 TCAN4X5X_MODE_SEL_MASK, TCAN4X5X_MODE_STANDBY);

return regmap_update_bits();

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

[PATCH 3/3] can: m_can: call deinit callback when going into suspend.

[Sean Nyekjaer]

m_can user like the tcan4x5x device, can go into standby mode.
Low power RX mode is enabled to allow wake on can.

Signed-off-by: Sean Nyekjaer <sean@geanix.com>
---
 drivers/net/can/m_can/m_can.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a171ff860b7c6992846ae8d615640a40b623e0cb..d9f820b5609ea3e8a98dc8a9f4d9948c09aa85c6 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2451,6 +2451,9 @@ int m_can_class_suspend(struct device *dev)
 		if (cdev->pm_wake_source) {
 			hrtimer_cancel(&cdev->hrtimer);
 			m_can_write(cdev, M_CAN_IE, IR_RF0N);
+
+			if (cdev->ops->deinit)
+				cdev->ops->deinit(cdev);
 		} else {
 			m_can_stop(ndev);
 		}
@@ -2490,6 +2493,10 @@ int m_can_class_resume(struct device *dev)
 			 * again.
 			 */
 			cdev->active_interrupts |= IR_RF0N | IR_TEFN;
+
+			if (cdev->ops->init)
+				cdev->ops->init(cdev);
+
 			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
 		} else {
 			ret  = m_can_start(ndev);

-- 
2.46.2