| Message ID | 20240729074135.3850634-1-msp@baylibre.com |
|---|---|
| Headers | show |
| Series | can: m_can: Add am62 wakeup support | expand |
Hi Markus, On Mon, 2024-07-29 at 09:41 +0200, Markus Schneider-Pargmann wrote: > am62 requires a wakeup flag being set in pinctrl when mcan pins acts > as > a wakeup source. Add support to select the wakeup state if WOL is > enabled. > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com> > --- > drivers/net/can/m_can/m_can.c | 57 > +++++++++++++++++++++++++++++++++++ > drivers/net/can/m_can/m_can.h | 4 +++ > 2 files changed, 61 insertions(+) > > diff --git a/drivers/net/can/m_can/m_can.c > b/drivers/net/can/m_can/m_can.c > index 5b80a7d1f9a1..b71e7f8e9727 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -2193,6 +2193,7 @@ static void m_can_get_wol(struct net_device > *dev, struct ethtool_wolinfo *wol) > static int m_can_set_wol(struct net_device *dev, struct > ethtool_wolinfo *wol) > { > struct m_can_classdev *cdev = netdev_priv(dev); > + struct pinctrl_state *new_pinctrl_state = NULL; > bool wol_enable = !!wol->wolopts & WAKE_PHY; > int ret; > > @@ -2209,7 +2210,27 @@ static int m_can_set_wol(struct net_device > *dev, struct ethtool_wolinfo *wol) > return ret; > } > > + if (wol_enable) > + new_pinctrl_state = cdev->pinctrl_state_wakeup; > + else > + new_pinctrl_state = cdev->pinctrl_state_default; > + > + if (!IS_ERR_OR_NULL(new_pinctrl_state)) { Why not do if (IS_ERR_OR_NULL(new_pinctrl_state)) return 0; ? // Martin > + ret = pinctrl_select_state(cdev->pinctrl, > new_pinctrl_state); > + if (ret) { > + netdev_err(cdev->net, "Failed to select > pinctrl state %pE\n", > + ERR_PTR(ret)); > + goto err_wakeup_enable; > + } > + } > + > return 0; > + > +err_wakeup_enable: > + /* Revert wakeup enable */ > + device_set_wakeup_enable(cdev->dev, !wol_enable); > + > + return ret; > } > > static const struct ethtool_ops m_can_ethtool_ops_coalescing = { > @@ -2377,7 +2398,43 @@ struct m_can_classdev > *m_can_class_allocate_dev(struct device *dev, > > m_can_of_parse_mram(class_dev, mram_config_vals); > > + class_dev->pinctrl = devm_pinctrl_get(dev); > + if (IS_ERR(class_dev->pinctrl)) { > + ret = PTR_ERR(class_dev->pinctrl); > + > + if (ret != -ENODEV) { > + dev_err_probe(dev, ret, "Failed to get > pinctrl\n"); > + goto err_free_candev; > + } > + > + class_dev->pinctrl = NULL; > + } else { > + class_dev->pinctrl_state_wakeup = > pinctrl_lookup_state(class_dev->pinctrl, "wakeup"); > + if (IS_ERR(class_dev->pinctrl_state_wakeup)) { > + ret = PTR_ERR(class_dev- > >pinctrl_state_wakeup); > + ret = -EIO; > + > + if (ret != -ENODEV) { > + dev_err_probe(dev, ret, "Failed to > lookup pinctrl wakeup state\n"); > + goto err_free_candev; > + } > + > + class_dev->pinctrl_state_wakeup = NULL; > + } else { > + class_dev->pinctrl_state_default = > pinctrl_lookup_state(class_dev->pinctrl, "default"); > + if (IS_ERR(class_dev- > >pinctrl_state_default)) { > + ret = PTR_ERR(class_dev- > >pinctrl_state_default); > + dev_err_probe(dev, ret, "Failed to > lookup pinctrl default state\n"); > + goto err_free_candev; > + } > + } > + } > + > return class_dev; > + > +err_free_candev: > + free_candev(net_dev); > + return ERR_PTR(ret); > } > EXPORT_SYMBOL_GPL(m_can_class_allocate_dev); > > diff --git a/drivers/net/can/m_can/m_can.h > b/drivers/net/can/m_can/m_can.h > index 92b2bd8628e6..b75b0dd6ccc9 100644 > --- a/drivers/net/can/m_can/m_can.h > +++ b/drivers/net/can/m_can/m_can.h > @@ -126,6 +126,10 @@ struct m_can_classdev { > struct mram_cfg mcfg[MRAM_CFG_NUM]; > > struct hrtimer hrtimer; > + > + struct pinctrl *pinctrl; > + struct pinctrl_state *pinctrl_state_default; > + struct pinctrl_state *pinctrl_state_wakeup; > }; > > struct m_can_classdev *m_can_class_allocate_dev(struct device *dev, > int sizeof_priv);
On Mon, Jul 29, 2024 at 09:41:30AM +0200, Markus Schneider-Pargmann 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> > --- > 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 81e05746d7d4..2e4ccf05e162 100644 > --- a/drivers/net/can/m_can/m_can.c > +++ b/drivers/net/can/m_can/m_can.c > @@ -2182,6 +2182,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; > +} It is nice to see Ethernet WoL mapped to CAN :-) So will any activity on the CAN BUS wake the device? Or does it need to be addresses to this device? Andrew
On 29.07.2024 21:27:04, Andrew Lunn wrote: > On Mon, Jul 29, 2024 at 09:41:30AM +0200, Markus Schneider-Pargmann 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> > > --- > > 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 81e05746d7d4..2e4ccf05e162 100644 > > --- a/drivers/net/can/m_can/m_can.c > > +++ b/drivers/net/can/m_can/m_can.c > > @@ -2182,6 +2182,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; > > +} > > It is nice to see Ethernet WoL mapped to CAN :-) > > So will any activity on the CAN BUS wake the device? Or does it need > to be addresses to this device? Unless you have a special filtering transceiver, which is the CAN equivalent of a PHY, CAN interfaces usually wake up on the first message on the bus. That message is usually lost. Note: The details of the m_can IP core might be different. regards, Marc
> > > +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; > > > +} > > > > It is nice to see Ethernet WoL mapped to CAN :-) > > > > So will any activity on the CAN BUS wake the device? Or does it need > > to be addresses to this device? > > Unless you have a special filtering transceiver, which is the CAN > equivalent of a PHY, CAN interfaces usually wake up on the first > message on the bus. That message is usually lost. Thanks for the info. WAKE_PHY does seem the most appropriate then. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
Hi, On Mon, Jul 29, 2024 at 09:37:56PM GMT, Andrew Lunn wrote: > > > > +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; > > > > +} > > > > > > It is nice to see Ethernet WoL mapped to CAN :-) > > > > > > So will any activity on the CAN BUS wake the device? Or does it need > > > to be addresses to this device? > > > > Unless you have a special filtering transceiver, which is the CAN > > equivalent of a PHY, CAN interfaces usually wake up on the first > > message on the bus. That message is usually lost. > > Thanks for the info. WAKE_PHY does seem the most appropriate then. > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> Thank you. Just to extend on Marc's explanation specifically for m_can: For this very low power mode 'Partial-IO' the m_can IP is not active. The m_can pins will trigger a wakeup for any activity. Also as the SoC needs to do a normal boot, I would guess there are more messages lost when waking up from Partial-IO. Other low power modes that will be upstreamed in the future will not need as much time to be able to receive CAN messages again. Best Markus
Hi, v2 -- In v2 I mostly fixed error handling in this series and rebased to v6.11-rc1. I also dropped the omap serial series for Partial-IO as it can't be tested with Partial-IO at the moment. The code was tested with other low power modes but they will be upstreamed later on. Series ------ am62, am62a and am62p support Partial-IO, a poweroff SoC state with a few pin groups being active for wakeup. To support mcu_mcan0 and mcu_mcan1 wakeup for the mentioned SoCs, the series introduces a notion of wake-on-lan for m_can. If the user decides to enable wake-on-lan for a m_can device, the device is set to wakeup enabled. A 'wakeup' pinctrl state is selected to enable wakeup flags for the relevant pins. If wake-on-lan is disabled the default pinctrl is selected. It is based on v6.11-rc1 with this series applied: https://lore.kernel.org/lkml/20240726195944.2414812-1-msp@baylibre.com/ Also available at https://gitlab.baylibre.com/msp8/linux/-/tree/topic/mcan-fixes/v6.11?ref_type=heads Partial-IO ---------- This series is part of a bigger topic to support Partial-IO on am62, am62a and am62p. Partial-IO is a poweroff state in which some pins are able to wakeup the SoC. In detail MCU m_can and two serial port pins can trigger the wakeup. A documentation can also be found in section 6.2.4 in the TRM: https://www.ti.com/lit/pdf/spruiv7 This other series is relevant for the support of Partial-IO: - firmware: ti_sci: Partial-IO support https://gitlab.baylibre.com/msp8/linux/-/tree/topic/am62-partialio/v6.11?ref_type=heads Testing ------- A test branch is available here that includes all patches required to test Partial-IO: https://gitlab.baylibre.com/msp8/linux/-/tree/integration/am62-lp-sk-partialio/v6.11?ref_type=heads After enabling Wake-on-LAN the system can be powered off and will enter the Partial-IO state in which it can be woken up by activity on the specific pins: ethtool -s can0 wol p ethtool -s can1 wol p poweroff I tested these patches on am62-lp-sk. Best, Markus Previous versions: v1: https://lore.kernel.org/lkml/20240523075347.1282395-1-msp@baylibre.com/ Changes in v2: - Rebase to v6.11-rc1 - Squash these two patches for the binding into one: dt-bindings: can: m_can: Add wakeup-source property dt-bindings: can: m_can: Add wakeup pinctrl state - Add error handling to multiple patches of the m_can driver - Add error handling in m_can_class_allocate_dev(). This also required to add a new patch to return error pointers from m_can_class_allocate_dev(). Markus Schneider-Pargmann (6): dt-bindings: can: m_can: Add wakeup properties can: m_can: Map WoL to device_set_wakeup_enable can: m_can: Return ERR_PTR on error in allocation can: m_can: Support pinctrl wakeup state arm64: dts: ti: k3-am62: Mark mcu_mcan0/1 as wakeup-source arm64: dts: ti: k3-am62a-mcu: Mark mcu_mcan0/1 as wakeup-source Vibhore Vardhan (1): arm64: dts: ti: k3-am62p-mcu: Mark mcu_mcan0/1 as wakeup-source .../bindings/net/can/bosch,m_can.yaml | 20 ++++ arch/arm64/boot/dts/ti/k3-am62-mcu.dtsi | 2 + arch/arm64/boot/dts/ti/k3-am62a-mcu.dtsi | 2 + .../dts/ti/k3-am62p-j722s-common-mcu.dtsi | 2 + drivers/net/can/m_can/m_can.c | 100 +++++++++++++++++- drivers/net/can/m_can/m_can.h | 4 + drivers/net/can/m_can/m_can_pci.c | 4 +- drivers/net/can/m_can/m_can_platform.c | 4 +- drivers/net/can/m_can/tcan4x5x-core.c | 4 +- 9 files changed, 133 insertions(+), 9 deletions(-)