| Message ID | 20241015-topic-mcan-wakeup-source-v6-12-v4-3-fdac1d1e7aa6@baylibre.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | can: m_can: Add am62 wakeup support | expand |
On Wed. 16 Oct. 2024 at 04:18, Markus Schneider-Pargmann <msp@baylibre.com> wrote: > In some devices the pins of the m_can module can act as a wakeup source. > This patch helps do that by connecting the PHY_WAKE WoL option to > device_set_wakeup_enable. By marking this device as being wakeup > enabled, this setting can be used by platform code to decide which > sleep or poweroff mode to use. > > Also this prepares the driver for the next patch in which the pinctrl > settings are changed depending on the desired wakeup source. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> I left a nitpick below. Regardless: Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > --- > drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > index a978b960f1f1e1e8273216ff330ab789d0fd6d51..d427645a5b3baf7d0a648e3b008d7d7de7f23374 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -2185,6 +2185,36 @@ static int m_can_set_coalesce(struct net_device *dev, > return 0; > } > > +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol) > +{ > + struct m_can_classdev *cdev = netdev_priv(dev); > + > + wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0; > + wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0; > +} > + > +static int m_can_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) > +{ > + struct m_can_classdev *cdev = netdev_priv(dev); > + bool wol_enable = !!(wol->wolopts & WAKE_PHY); > + int ret; > + > + if ((wol->wolopts & WAKE_PHY) != wol->wolopts) Here, you want to check if a bit other than WAKE_PHY is set, isn't it? What about doing this: if (wol->wolopts & ~WAKE_PHY) instead? > + return -EINVAL; > + > + if (wol_enable == device_may_wakeup(cdev->dev)) > + return 0; > + > + ret = device_set_wakeup_enable(cdev->dev, wol_enable); > + if (ret) { > + netdev_err(cdev->net, "Failed to set wakeup enable %pE\n", > + ERR_PTR(ret)); > + return ret; > + } > + > + return 0; > +} > + > static const struct ethtool_ops m_can_ethtool_ops_coalescing = { > .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ | > ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ | > @@ -2194,10 +2224,14 @@ static const struct ethtool_ops m_can_ethtool_ops_coalescing = { > .get_ts_info = ethtool_op_get_ts_info, > .get_coalesce = m_can_get_coalesce, > .set_coalesce = m_can_set_coalesce, > + .get_wol = m_can_get_wol, > + .set_wol = m_can_set_wol, > }; > > static const struct ethtool_ops m_can_ethtool_ops = { > .get_ts_info = ethtool_op_get_ts_info, > + .get_wol = m_can_get_wol, > + .set_wol = m_can_set_wol, > }; > > static int register_m_can_dev(struct m_can_classdev *cdev) > @@ -2324,6 +2358,9 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, > goto out; > } > > + if (dev->of_node && of_property_read_bool(dev->of_node, "wakeup-source")) > + device_set_wakeup_capable(dev, true); > + > /* Get TX FIFO size > * Defines the total amount of echo buffers for loopback > */ > > -- > 2.45.2 > >
Hi Vincent, On Wed, Oct 16, 2024 at 11:32:06PM GMT, Vincent MAILHOL wrote: > On Wed. 16 Oct. 2024 at 04:18, Markus Schneider-Pargmann > <msp@baylibre.com> wrote: > > In some devices the pins of the m_can module can act as a wakeup source. > > This patch helps do that by connecting the PHY_WAKE WoL option to > > device_set_wakeup_enable. By marking this device as being wakeup > > enabled, this setting can be used by platform code to decide which > > sleep or poweroff mode to use. > > > > Also this prepares the driver for the next patch in which the pinctrl > > settings are changed depending on the desired wakeup source. > > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > > I left a nitpick below. Regardless: > > Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > > --- > > drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++++ > > 1 file changed, 37 insertions(+) > > > > diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c > > index a978b960f1f1e1e8273216ff330ab789d0fd6d51..d427645a5b3baf7d0a648e3b008d7d7de7f23374 100644 > > --- a/drivers/net/can/m_can/m_can.c > > +++ b/drivers/net/can/m_can/m_can.c > > @@ -2185,6 +2185,36 @@ static int m_can_set_coalesce(struct net_device *dev, > > return 0; > > } > > > > +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol) > > +{ > > + struct m_can_classdev *cdev = netdev_priv(dev); > > + > > + wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0; > > + wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0; > > +} > > + > > +static int m_can_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) > > +{ > > + struct m_can_classdev *cdev = netdev_priv(dev); > > + bool wol_enable = !!(wol->wolopts & WAKE_PHY); > > + int ret; > > + > > + if ((wol->wolopts & WAKE_PHY) != wol->wolopts) > > Here, you want to check if a bit other than WAKE_PHY is set, isn't it? > What about doing this: > > if (wol->wolopts & ~WAKE_PHY) > > instead? Yes, thanks, that is better. Thank you for your reviews! Best Markus > > > + return -EINVAL; > > + > > + if (wol_enable == device_may_wakeup(cdev->dev)) > > + return 0; > > + > > + ret = device_set_wakeup_enable(cdev->dev, wol_enable); > > + if (ret) { > > + netdev_err(cdev->net, "Failed to set wakeup enable %pE\n", > > + ERR_PTR(ret)); > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > static const struct ethtool_ops m_can_ethtool_ops_coalescing = { > > .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ | > > ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ | > > @@ -2194,10 +2224,14 @@ static const struct ethtool_ops m_can_ethtool_ops_coalescing = { > > .get_ts_info = ethtool_op_get_ts_info, > > .get_coalesce = m_can_get_coalesce, > > .set_coalesce = m_can_set_coalesce, > > + .get_wol = m_can_get_wol, > > + .set_wol = m_can_set_wol, > > }; > > > > static const struct ethtool_ops m_can_ethtool_ops = { > > .get_ts_info = ethtool_op_get_ts_info, > > + .get_wol = m_can_get_wol, > > + .set_wol = m_can_set_wol, > > }; > > > > static int register_m_can_dev(struct m_can_classdev *cdev) > > @@ -2324,6 +2358,9 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, > > goto out; > > } > > > > + if (dev->of_node && of_property_read_bool(dev->of_node, "wakeup-source")) > > + device_set_wakeup_capable(dev, true); > > + > > /* Get TX FIFO size > > * Defines the total amount of echo buffers for loopback > > */ > > > > -- > > 2.45.2 > > > >
diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index a978b960f1f1e1e8273216ff330ab789d0fd6d51..d427645a5b3baf7d0a648e3b008d7d7de7f23374 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -2185,6 +2185,36 @@ static int m_can_set_coalesce(struct net_device *dev, return 0; } +static void m_can_get_wol(struct net_device *dev, struct ethtool_wolinfo *wol) +{ + struct m_can_classdev *cdev = netdev_priv(dev); + + wol->supported = device_can_wakeup(cdev->dev) ? WAKE_PHY : 0; + wol->wolopts = device_may_wakeup(cdev->dev) ? WAKE_PHY : 0; +} + +static int m_can_set_wol(struct net_device *dev, struct ethtool_wolinfo *wol) +{ + struct m_can_classdev *cdev = netdev_priv(dev); + bool wol_enable = !!(wol->wolopts & WAKE_PHY); + int ret; + + if ((wol->wolopts & WAKE_PHY) != wol->wolopts) + return -EINVAL; + + if (wol_enable == device_may_wakeup(cdev->dev)) + return 0; + + ret = device_set_wakeup_enable(cdev->dev, wol_enable); + if (ret) { + netdev_err(cdev->net, "Failed to set wakeup enable %pE\n", + ERR_PTR(ret)); + return ret; + } + + return 0; +} + static const struct ethtool_ops m_can_ethtool_ops_coalescing = { .supported_coalesce_params = ETHTOOL_COALESCE_RX_USECS_IRQ | ETHTOOL_COALESCE_RX_MAX_FRAMES_IRQ | @@ -2194,10 +2224,14 @@ static const struct ethtool_ops m_can_ethtool_ops_coalescing = { .get_ts_info = ethtool_op_get_ts_info, .get_coalesce = m_can_get_coalesce, .set_coalesce = m_can_set_coalesce, + .get_wol = m_can_get_wol, + .set_wol = m_can_set_wol, }; static const struct ethtool_ops m_can_ethtool_ops = { .get_ts_info = ethtool_op_get_ts_info, + .get_wol = m_can_get_wol, + .set_wol = m_can_set_wol, }; static int register_m_can_dev(struct m_can_classdev *cdev) @@ -2324,6 +2358,9 @@ struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, goto out; } + if (dev->of_node && of_property_read_bool(dev->of_node, "wakeup-source")) + device_set_wakeup_capable(dev, true); + /* Get TX FIFO size * Defines the total amount of echo buffers for loopback */
In some devices the pins of the m_can module can act as a wakeup source. This patch helps do that by connecting the PHY_WAKE WoL option to device_set_wakeup_enable. By marking this device as being wakeup enabled, this setting can be used by platform code to decide which sleep or poweroff mode to use. Also this prepares the driver for the next patch in which the pinctrl settings are changed depending on the desired wakeup source. Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> --- drivers/net/can/m_can/m_can.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)