| Message ID | 20241111-tcan-standby-v1-1-f9337ebaceea@geanix.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | can: tcan4x5x/m_can: use standby mode when down and in suspend | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Series ignored based on subject |
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 > >
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
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
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
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 {
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(+)