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

[PATCH can-next v2 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>
---
Changes in v2:
- Reduced code in tcan4x5x_deinit()
- Taken care of return values from deinit callback
- Link to v1: https://lore.kernel.org/r/20241111-tcan-standby-v1-0-f9337ebaceea@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/init callback when going into suspend/resume

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

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

[PATCH can-next v2 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 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 | 10 +++++++---
 drivers/net/can/m_can/m_can.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index a7b3bc439ae596527493a73d62b4b7a120ae4e49..667c70f8dc5e7e8b15b667119b63dea1fe667e8a 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1750,12 +1750,16 @@ static void m_can_stop(struct net_device *dev)
 
 	/* Set init mode to disengage from the network */
 	ret = m_can_cccr_update_bits(cdev, CCCR_INIT, CCCR_INIT);
-	if (ret)
-		netdev_err(dev, "failed to enter standby mode: %pe\n",
-			   ERR_PTR(ret));
 
 	/* set the state as STOPPED */
 	cdev->can.state = CAN_STATE_STOPPED;
+
+	if (!ret && cdev->ops->deinit)
+		ret = cdev->ops->deinit(cdev);
+
+	if (ret)
+		netdev_err(dev, "failed to enter standby mode: %pe\n",
+			   ERR_PTR(ret));
 }
 
 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

[PATCH can-next v2 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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/can/m_can/tcan4x5x-core.c b/drivers/net/can/m_can/tcan4x5x-core.c
index 2f73bf3abad889c222f15c39a3d43de1a1cf5fbb..7e6eea2c04ee7fbeaa6c8b229f7f64bd04bee309 100644
--- a/drivers/net/can/m_can/tcan4x5x-core.c
+++ b/drivers/net/can/m_can/tcan4x5x-core.c
@@ -270,6 +270,14 @@ 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);
+
+	return regmap_update_bits(tcan4x5x->regmap, TCAN4X5X_CONFIG,
+				  TCAN4X5X_MODE_SEL_MASK, TCAN4X5X_MODE_STANDBY);
+};
+
 static int tcan4x5x_disable_wake(struct m_can_classdev *cdev)
 {
 	struct tcan4x5x_priv *tcan4x5x = cdev_to_priv(cdev);
@@ -359,6 +367,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

[PATCH can-next v2 3/3] can: m_can: call deinit/init callback when going into suspend/resume

[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 | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 667c70f8dc5e7e8b15b667119b63dea1fe667e8a..76a691de89baba1addf24342815c76e1ab32a776 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -2440,6 +2440,7 @@ int m_can_class_suspend(struct device *dev)
 {
 	struct m_can_classdev *cdev = dev_get_drvdata(dev);
 	struct net_device *ndev = cdev->net;
+	int ret = 0;
 
 	if (netif_running(ndev)) {
 		netif_stop_queue(ndev);
@@ -2452,6 +2453,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)
+				ret = cdev->ops->deinit(cdev);
 		} else {
 			m_can_stop(ndev);
 		}
@@ -2463,7 +2467,7 @@ int m_can_class_suspend(struct device *dev)
 
 	cdev->can.state = CAN_STATE_SLEEPING;
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(m_can_class_suspend);
 
@@ -2471,14 +2475,13 @@ int m_can_class_resume(struct device *dev)
 {
 	struct m_can_classdev *cdev = dev_get_drvdata(dev);
 	struct net_device *ndev = cdev->net;
+	int ret = 0;
 
 	pinctrl_pm_select_default_state(dev);
 
 	cdev->can.state = CAN_STATE_ERROR_ACTIVE;
 
 	if (netif_running(ndev)) {
-		int ret;
-
 		ret = m_can_clk_start(cdev);
 		if (ret)
 			return ret;
@@ -2491,6 +2494,10 @@ int m_can_class_resume(struct device *dev)
 			 * again.
 			 */
 			cdev->active_interrupts |= IR_RF0N | IR_TEFN;
+
+			if (cdev->ops->init)
+				ret = cdev->ops->init(cdev);
+
 			m_can_write(cdev, M_CAN_IE, cdev->active_interrupts);
 		} else {
 			ret  = m_can_start(ndev);
@@ -2504,7 +2511,7 @@ int m_can_class_resume(struct device *dev)
 		netif_start_queue(ndev);
 	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(m_can_class_resume);
 

-- 
2.46.2

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

[Vincent Mailhol]

On 15/11/2024 at 17:15, 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 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 | 10 +++++++---
>  drivers/net/can/m_can/m_can.h |  1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> index a7b3bc439ae596527493a73d62b4b7a120ae4e49..667c70f8dc5e7e8b15b667119b63dea1fe667e8a 100644
> --- a/drivers/net/can/m_can/m_can.c
> +++ b/drivers/net/can/m_can/m_can.c
> @@ -1750,12 +1750,16 @@ static void m_can_stop(struct net_device *dev)
>  
>  	/* Set init mode to disengage from the network */
>  	ret = m_can_cccr_update_bits(cdev, CCCR_INIT, CCCR_INIT);
> -	if (ret)
> -		netdev_err(dev, "failed to enter standby mode: %pe\n",
> -			   ERR_PTR(ret));
>  
>  	/* set the state as STOPPED */
>  	cdev->can.state = CAN_STATE_STOPPED;
> +
> +	if (!ret && cdev->ops->deinit)
> +		ret = cdev->ops->deinit(cdev);

Question: is there a reason not to try to deinit() even if the
m_can_cccr_update_bits() failed?

> +	if (ret)
> +		netdev_err(dev, "failed to enter standby mode: %pe\n",
> +			   ERR_PTR(ret));>  }
>  
>  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 {
> 

Yours sincerely,
Vincent Mailhol

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

[Sean Nyekjaer]

On Fri, Nov 15, 2024 at 11:23:49PM +0100, Vincent Mailhol wrote:
> On 15/11/2024 at 17:15, 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 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 | 10 +++++++---
> >  drivers/net/can/m_can/m_can.h |  1 +
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
> > index a7b3bc439ae596527493a73d62b4b7a120ae4e49..667c70f8dc5e7e8b15b667119b63dea1fe667e8a 100644
> > --- a/drivers/net/can/m_can/m_can.c
> > +++ b/drivers/net/can/m_can/m_can.c
> > @@ -1750,12 +1750,16 @@ static void m_can_stop(struct net_device *dev)
> >  
> >  	/* Set init mode to disengage from the network */
> >  	ret = m_can_cccr_update_bits(cdev, CCCR_INIT, CCCR_INIT);
> > -	if (ret)
> > -		netdev_err(dev, "failed to enter standby mode: %pe\n",
> > -			   ERR_PTR(ret));
> >  
> >  	/* set the state as STOPPED */
> >  	cdev->can.state = CAN_STATE_STOPPED;
> > +
> > +	if (!ret && cdev->ops->deinit)
> > +		ret = cdev->ops->deinit(cdev);
> 
> Question: is there a reason not to try to deinit() even if the
> m_can_cccr_update_bits() failed?

Good question.
If the call the m_can core fails, then there bigger problems than setting
the (in nmy case) tcan4x5x in standby mode.

> 
> > +	if (ret)
> > +		netdev_err(dev, "failed to enter standby mode: %pe\n",
> > +			   ERR_PTR(ret));>  }
> >  
> >  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 {
> > 
> 
> Yours sincerely,
> Vincent Mailhol
> 

/Sean