[PATCH V6 0/2] can: flexcan: add can self wakeup support

[PATCH V6 0/2] can: flexcan: add can self wakeup support

[Joakim Zhang]

This patch series intends to add CAN self wakeup support. The CAN controller
can parse "fsl,stop-mode" property from device tree to enable self wakeup feature.

Aisheng Dong (2):
  Documentation: can: flexcan: add stop mode property to device tree
  can: flexcan: add self wakeup support

 .../bindings/net/can/fsl-flexcan.txt          |   8 +
 drivers/net/can/flexcan.c                     | 172 +++++++++++++++++-
 2 files changed, 171 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH V6 1/2] Documentation: can: flexcan: add stop mode property to device tree

[Joakim Zhang]

From: Aisheng Dong <aisheng.dong@nxp.com>

The FlexCAN controller can parse the stop mode property to enable CAN
self wakeup feature.

Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
ChangeLog:
V1->V2:
	*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
V2->V6:
	*no change.
---
 Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
index bfc0c433654f..bc77477c6878 100644
--- a/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
+++ b/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
@@ -24,6 +24,14 @@ Optional properties:
               if this property is present then controller is assumed to be big
               endian.
 
+- fsl,stop-mode: register bits of stop mode control, the format is
+		 <&gpr req_gpr req_bit ack_gpr ack_bit>.
+		 gpr is the phandle to general purpose register node.
+		 req_gpr is the gpr register offset of CAN stop request.
+		 req_bit is the bit offset of CAN stop request.
+		 ack_gpr is the gpr register offset of CAN stop acknowledge.
+		 ack_bit is the bit offset of CAN stop acknowledge.
+
 Example:
 
 	can@1c000 {
-- 
2.17.1

Re: [PATCH V6 1/2] Documentation: can: flexcan: add stop mode property to device tree

[Rob Herring]

On Fri, 23 Nov 2018 08:35:29 +0000, Joakim Zhang wrote:
> From: Aisheng Dong <aisheng.dong@nxp.com>
> 
> The FlexCAN controller can parse the stop mode property to enable CAN
> self wakeup feature.
> 
> Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> ChangeLog:
> V1->V2:
> 	*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
> V2->V6:
> 	*no change.
> ---
>  Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH V6 1/2] Documentation: can: flexcan: add stop mode property to device tree

[Rob Herring]

On Fri, 23 Nov 2018 08:35:29 +0000, Joakim Zhang wrote:
> From: Aisheng Dong <aisheng.dong@nxp.com>
> 
> The FlexCAN controller can parse the stop mode property to enable CAN
> self wakeup feature.
> 
> Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> ChangeLog:
> V1->V2:
> 	*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
> V2->V6:
> 	*no change.
> ---
>  Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH V6 2/2] can: flexcan: add self wakeup support

[Joakim Zhang]

From: Aisheng Dong <aisheng.dong@nxp.com>

If wakeup is enabled, enter stop mode, else enter disabled mode. Self wake
can only work on stop mode.

Starting from IMX6, the flexcan stop mode control bits is SoC specific,
move it out of IP driver and parse it from devicetree.

Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
ChangeLog:
V1->V2:
	*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
V2->V3:
	*add FLEXCAN_QUIRK_SETUP_STOP_MODE quirk.
	*rename function.
	*fix system can't be wakeuped during suspend.
V3->V4:
	*normalize the code following Aisheng Dong's comments.
V4->V5:
	*move enable/disable self wakeup feature into
	 enter/exit_stop_mode() function.
V5->V6:
	*exchange the order of flexcan_enable_wakeup_irq(priv, false)
	 and disable_irq_wake(dev->irq).
---
 drivers/net/can/flexcan.c | 172 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 163 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8e972ef08637..da08a8c113d2 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -19,11 +19,14 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/mfd/syscon.h>
+#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
 
 #define DRV_NAME			"flexcan"
 
@@ -131,7 +134,8 @@
 	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
 #define FLEXCAN_ESR_ALL_INT \
 	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
-	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
+	FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
+	FLEXCAN_ESR_WAK_INT)
 
 /* FLEXCAN interrupt flag register (IFLAG) bits */
 /* Errata ERR005829 step7: Reserve first valid MB */
@@ -190,6 +194,7 @@
 #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP	BIT(5) /* Use timestamp based offloading */
 #define FLEXCAN_QUIRK_BROKEN_PERR_STATE	BIT(6) /* No interrupt for error passive */
 #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN	BIT(7) /* default to BE register access */
+#define FLEXCAN_QUIRK_SETUP_STOP_MODE		BIT(8) /* Setup stop mode to support wakeup */
 
 /* Structure of the message buffer */
 struct flexcan_mb {
@@ -254,6 +259,14 @@ struct flexcan_devtype_data {
 	u32 quirks;		/* quirks needed for different IP cores */
 };
 
+struct flexcan_stop_mode {
+	struct regmap *gpr;
+	u8 req_gpr;
+	u8 req_bit;
+	u8 ack_gpr;
+	u8 ack_bit;
+};
+
 struct flexcan_priv {
 	struct can_priv can;
 	struct can_rx_offload offload;
@@ -270,6 +283,7 @@ struct flexcan_priv {
 	struct clk *clk_per;
 	const struct flexcan_devtype_data *devtype_data;
 	struct regulator *reg_xceiver;
+	struct flexcan_stop_mode stm;
 
 	/* Read and Write APIs */
 	u32 (*read)(void __iomem *addr);
@@ -293,7 +307,8 @@ static const struct flexcan_devtype_data fsl_imx28_devtype_data = {
 
 static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
 	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
-		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE,
+		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
+		FLEXCAN_QUIRK_SETUP_STOP_MODE,
 };
 
 static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
@@ -353,6 +368,49 @@ static inline void flexcan_write_le(u32 val, void __iomem *addr)
 	iowrite32(val, addr);
 }
 
+static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
+{
+	struct flexcan_regs __iomem *regs = priv->regs;
+	u32 reg_mcr;
+
+	reg_mcr = priv->read(&regs->mcr);
+
+	if (enable)
+		reg_mcr |= FLEXCAN_MCR_WAK_MSK;
+	else
+		reg_mcr &= ~FLEXCAN_MCR_WAK_MSK;
+
+	priv->write(reg_mcr, &regs->mcr);
+}
+
+static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->regs;
+	u32 reg_mcr;
+
+	reg_mcr = priv->read(&regs->mcr);
+	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
+	priv->write(reg_mcr, &regs->mcr);
+
+	/* enable stop request */
+	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
+}
+
+static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv)
+{
+	struct flexcan_regs __iomem *regs = priv->regs;
+	u32 reg_mcr;
+
+	/* remove stop request */
+	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+			   1 << priv->stm.req_bit, 0);
+
+	reg_mcr = priv->read(&regs->mcr);
+	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
+	priv->write(reg_mcr, &regs->mcr);
+}
+
 static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
 {
 	struct flexcan_regs __iomem *regs = priv->regs;
@@ -1244,6 +1302,57 @@ static void unregister_flexcandev(struct net_device *dev)
 	unregister_candev(dev);
 }
 
+static int flexcan_setup_stop_mode(struct platform_device *pdev)
+{
+	struct net_device *dev = platform_get_drvdata(pdev);
+	struct device_node *np = pdev->dev.of_node;
+	struct device_node *gpr_np;
+	struct flexcan_priv *priv;
+	phandle phandle;
+	u32 out_val[5];
+	int ret;
+
+	if (!np)
+		return -EINVAL;
+
+	/* stop mode property format is:
+	 * <&gpr req_gpr req_bit ack_gpr ack_bit>.
+	 */
+	ret = of_property_read_u32_array(np, "fsl,stop-mode", out_val, 5);
+	if (ret) {
+		dev_dbg(&pdev->dev, "no stop-mode property\n");
+		return ret;
+	}
+	phandle = *out_val;
+
+	gpr_np = of_find_node_by_phandle(phandle);
+	if (!gpr_np) {
+		dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
+		return PTR_ERR(gpr_np);
+	}
+
+	priv = netdev_priv(dev);
+	priv->stm.gpr = syscon_node_to_regmap(gpr_np);
+	of_node_put(gpr_np);
+	if (IS_ERR(priv->stm.gpr)) {
+		dev_dbg(&pdev->dev, "could not find gpr regmap\n");
+		return PTR_ERR(priv->stm.gpr);
+	}
+
+	priv->stm.req_gpr = out_val[1];
+	priv->stm.req_bit = out_val[2];
+	priv->stm.ack_gpr = out_val[3];
+	priv->stm.ack_bit = out_val[4];
+
+	dev_dbg(&pdev->dev, "gpr %s req_gpr 0x%x req_bit %u ack_gpr 0x%x ack_bit %u\n",
+		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit, priv->stm.ack_gpr,
+		priv->stm.ack_bit);
+
+	device_set_wakeup_capable(&pdev->dev, true);
+
+	return 0;
+}
+
 static const struct of_device_id flexcan_of_match[] = {
 	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
 	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
@@ -1396,6 +1505,12 @@ static int flexcan_probe(struct platform_device *pdev)
 
 	devm_can_led_init(dev);
 
+	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
+		err = flexcan_setup_stop_mode(pdev);
+		if (err)
+			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
+	}
+
 	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
 		 priv->regs, dev->irq);
 
@@ -1426,9 +1541,17 @@ static int __maybe_unused flexcan_suspend(struct device *device)
 	int err;
 
 	if (netif_running(dev)) {
-		err = flexcan_chip_disable(priv);
-		if (err)
-			return err;
+		/* if wakeup is enabled, enter stop mode
+		 * else enter disabled mode.
+		 */
+		if (device_may_wakeup(device)) {
+			enable_irq_wake(dev->irq);
+			flexcan_enter_stop_mode(priv);
+		} else {
+			err = flexcan_chip_disable(priv);
+			if (err)
+				return err;
+		}
 		netif_stop_queue(dev);
 		netif_device_detach(dev);
 	}
@@ -1447,14 +1570,45 @@ static int __maybe_unused flexcan_resume(struct device *device)
 	if (netif_running(dev)) {
 		netif_device_attach(dev);
 		netif_start_queue(dev);
-		err = flexcan_chip_enable(priv);
-		if (err)
-			return err;
+		if (device_may_wakeup(device)) {
+			disable_irq_wake(dev->irq);
+		} else {
+			err = flexcan_chip_enable(priv);
+			if (err)
+				return err;
+		}
 	}
 	return 0;
 }
 
-static SIMPLE_DEV_PM_OPS(flexcan_pm_ops, flexcan_suspend, flexcan_resume);
+static int __maybe_unused flexcan_noirq_suspend(struct device *device)
+{
+	struct net_device *dev = dev_get_drvdata(device);
+	struct flexcan_priv *priv = netdev_priv(dev);
+
+	if (netif_running(dev) && device_may_wakeup(device))
+		flexcan_enable_wakeup_irq(priv, true);
+
+	return 0;
+}
+
+static int __maybe_unused flexcan_noirq_resume(struct device *device)
+{
+	struct net_device *dev = dev_get_drvdata(device);
+	struct flexcan_priv *priv = netdev_priv(dev);
+
+	if (netif_running(dev) && device_may_wakeup(device)) {
+		flexcan_enable_wakeup_irq(priv, false);
+		flexcan_exit_stop_mode(priv);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops flexcan_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(flexcan_suspend, flexcan_resume)
+	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(flexcan_noirq_suspend, flexcan_noirq_resume)
+};
 
 static struct platform_driver flexcan_driver = {
 	.driver = {
-- 
2.17.1

Re: [PATCH V6 2/2] can: flexcan: add self wakeup support

[Marc Kleine-Budde]

[-- Attachment #1.1: Type: text/plain, Size: 10074 bytes --]

On 11/23/18 9:35 AM, Joakim Zhang wrote:
> From: Aisheng Dong <aisheng.dong@nxp.com>
> 
> If wakeup is enabled, enter stop mode, else enter disabled mode. Self wake
> can only work on stop mode.
> 
> Starting from IMX6, the flexcan stop mode control bits is SoC specific,
> move it out of IP driver and parse it from devicetree.
> 
> Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> ChangeLog:
> V1->V2:
> 	*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
> V2->V3:
> 	*add FLEXCAN_QUIRK_SETUP_STOP_MODE quirk.
> 	*rename function.
> 	*fix system can't be wakeuped during suspend.
> V3->V4:
> 	*normalize the code following Aisheng Dong's comments.
> V4->V5:
> 	*move enable/disable self wakeup feature into
> 	 enter/exit_stop_mode() function.
> V5->V6:
> 	*exchange the order of flexcan_enable_wakeup_irq(priv, false)
> 	 and disable_irq_wake(dev->irq).
> ---
>  drivers/net/can/flexcan.c | 172 ++++++++++++++++++++++++++++++++++++--
>  1 file changed, 163 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 8e972ef08637..da08a8c113d2 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -19,11 +19,14 @@
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
>  #include <linux/of_device.h>
>  #include <linux/platform_device.h>
>  #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
>  
>  #define DRV_NAME			"flexcan"
>  
> @@ -131,7 +134,8 @@
>  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)
>  #define FLEXCAN_ESR_ALL_INT \
>  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> +	FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> +	FLEXCAN_ESR_WAK_INT)

I've fixed the indention while applying the patch.

>  
>  /* FLEXCAN interrupt flag register (IFLAG) bits */
>  /* Errata ERR005829 step7: Reserve first valid MB */
> @@ -190,6 +194,7 @@
>  #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP	BIT(5) /* Use timestamp based offloading */
>  #define FLEXCAN_QUIRK_BROKEN_PERR_STATE	BIT(6) /* No interrupt for error passive */
>  #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN	BIT(7) /* default to BE register access */
> +#define FLEXCAN_QUIRK_SETUP_STOP_MODE		BIT(8) /* Setup stop mode to support wakeup */
>  
>  /* Structure of the message buffer */
>  struct flexcan_mb {
> @@ -254,6 +259,14 @@ struct flexcan_devtype_data {
>  	u32 quirks;		/* quirks needed for different IP cores */
>  };
>  
> +struct flexcan_stop_mode {
> +	struct regmap *gpr;
> +	u8 req_gpr;
> +	u8 req_bit;
> +	u8 ack_gpr;
> +	u8 ack_bit;
> +};
> +
>  struct flexcan_priv {
>  	struct can_priv can;
>  	struct can_rx_offload offload;
> @@ -270,6 +283,7 @@ struct flexcan_priv {
>  	struct clk *clk_per;
>  	const struct flexcan_devtype_data *devtype_data;
>  	struct regulator *reg_xceiver;
> +	struct flexcan_stop_mode stm;
>  
>  	/* Read and Write APIs */
>  	u32 (*read)(void __iomem *addr);
> @@ -293,7 +307,8 @@ static const struct flexcan_devtype_data fsl_imx28_devtype_data = {
>  
>  static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
>  	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG | FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> -		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> +		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP | FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> +		FLEXCAN_QUIRK_SETUP_STOP_MODE,
>  };
>  
>  static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
> @@ -353,6 +368,49 @@ static inline void flexcan_write_le(u32 val, void __iomem *addr)
>  	iowrite32(val, addr);
>  }
>  
> +static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool enable)
> +{
> +	struct flexcan_regs __iomem *regs = priv->regs;
> +	u32 reg_mcr;
> +
> +	reg_mcr = priv->read(&regs->mcr);
> +
> +	if (enable)
> +		reg_mcr |= FLEXCAN_MCR_WAK_MSK;
> +	else
> +		reg_mcr &= ~FLEXCAN_MCR_WAK_MSK;
> +
> +	priv->write(reg_mcr, &regs->mcr);
> +}
> +
> +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
> +{
> +	struct flexcan_regs __iomem *regs = priv->regs;
> +	u32 reg_mcr;
> +
> +	reg_mcr = priv->read(&regs->mcr);
> +	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
> +	priv->write(reg_mcr, &regs->mcr);
> +
> +	/* enable stop request */
> +	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
> +}
> +
> +static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv)
> +{
> +	struct flexcan_regs __iomem *regs = priv->regs;
> +	u32 reg_mcr;
> +
> +	/* remove stop request */
> +	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> +			   1 << priv->stm.req_bit, 0);
> +
> +	reg_mcr = priv->read(&regs->mcr);
> +	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
> +	priv->write(reg_mcr, &regs->mcr);
> +}
> +
>  static inline void flexcan_error_irq_enable(const struct flexcan_priv *priv)
>  {
>  	struct flexcan_regs __iomem *regs = priv->regs;
> @@ -1244,6 +1302,57 @@ static void unregister_flexcandev(struct net_device *dev)
>  	unregister_candev(dev);
>  }
>  
> +static int flexcan_setup_stop_mode(struct platform_device *pdev)
> +{
> +	struct net_device *dev = platform_get_drvdata(pdev);
> +	struct device_node *np = pdev->dev.of_node;
> +	struct device_node *gpr_np;
> +	struct flexcan_priv *priv;
> +	phandle phandle;
> +	u32 out_val[5];
> +	int ret;
> +
> +	if (!np)
> +		return -EINVAL;
> +
> +	/* stop mode property format is:
> +	 * <&gpr req_gpr req_bit ack_gpr ack_bit>.
> +	 */
> +	ret = of_property_read_u32_array(np, "fsl,stop-mode", out_val, 5);

I've changed the 5 to ARRAY_SIZE() while applying the patch + rewrap to
80 cols.

> +	if (ret) {
> +		dev_dbg(&pdev->dev, "no stop-mode property\n");
> +		return ret;
> +	}
> +	phandle = *out_val;
> +
> +	gpr_np = of_find_node_by_phandle(phandle);
> +	if (!gpr_np) {
> +		dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
> +		return PTR_ERR(gpr_np);
> +	}
> +
> +	priv = netdev_priv(dev);
> +	priv->stm.gpr = syscon_node_to_regmap(gpr_np);
> +	of_node_put(gpr_np);
> +	if (IS_ERR(priv->stm.gpr)) {
> +		dev_dbg(&pdev->dev, "could not find gpr regmap\n");
> +		return PTR_ERR(priv->stm.gpr);
> +	}
> +
> +	priv->stm.req_gpr = out_val[1];
> +	priv->stm.req_bit = out_val[2];
> +	priv->stm.ack_gpr = out_val[3];
> +	priv->stm.ack_bit = out_val[4];
> +
> +	dev_dbg(&pdev->dev, "gpr %s req_gpr 0x%x req_bit %u ack_gpr 0x%x ack_bit %u\n",
> +		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit, priv->stm.ack_gpr,
> +		priv->stm.ack_bit);

For the hex values I've changed to 0x%x to 0x%02x + rewarp to 80 cols.

> +
> +	device_set_wakeup_capable(&pdev->dev, true);
> +
> +	return 0;
> +}
> +
>  static const struct of_device_id flexcan_of_match[] = {
>  	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
>  	{ .compatible = "fsl,imx28-flexcan", .data = &fsl_imx28_devtype_data, },
> @@ -1396,6 +1505,12 @@ static int flexcan_probe(struct platform_device *pdev)
>  
>  	devm_can_led_init(dev);
>  
> +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> +		err = flexcan_setup_stop_mode(pdev);
> +		if (err)
> +			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> +	}
> +
>  	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
>  		 priv->regs, dev->irq);
>  
> @@ -1426,9 +1541,17 @@ static int __maybe_unused flexcan_suspend(struct device *device)
>  	int err;
>  
>  	if (netif_running(dev)) {
> -		err = flexcan_chip_disable(priv);
> -		if (err)
> -			return err;
> +		/* if wakeup is enabled, enter stop mode
> +		 * else enter disabled mode.
> +		 */
> +		if (device_may_wakeup(device)) {
> +			enable_irq_wake(dev->irq);
> +			flexcan_enter_stop_mode(priv);
> +		} else {
> +			err = flexcan_chip_disable(priv);
> +			if (err)
> +				return err;
> +		}
>  		netif_stop_queue(dev);
>  		netif_device_detach(dev);
>  	}
> @@ -1447,14 +1570,45 @@ static int __maybe_unused flexcan_resume(struct device *device)
>  	if (netif_running(dev)) {
>  		netif_device_attach(dev);
>  		netif_start_queue(dev);
> -		err = flexcan_chip_enable(priv);
> -		if (err)
> -			return err;
> +		if (device_may_wakeup(device)) {
> +			disable_irq_wake(dev->irq);
> +		} else {
> +			err = flexcan_chip_enable(priv);
> +			if (err)
> +				return err;
> +		}
>  	}
>  	return 0;
>  }
>  
> -static SIMPLE_DEV_PM_OPS(flexcan_pm_ops, flexcan_suspend, flexcan_resume);
> +static int __maybe_unused flexcan_noirq_suspend(struct device *device)
> +{
> +	struct net_device *dev = dev_get_drvdata(device);
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +
> +	if (netif_running(dev) && device_may_wakeup(device))
> +		flexcan_enable_wakeup_irq(priv, true);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused flexcan_noirq_resume(struct device *device)
> +{
> +	struct net_device *dev = dev_get_drvdata(device);
> +	struct flexcan_priv *priv = netdev_priv(dev);
> +
> +	if (netif_running(dev) && device_may_wakeup(device)) {
> +		flexcan_enable_wakeup_irq(priv, false);
> +		flexcan_exit_stop_mode(priv);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops flexcan_pm_ops = {
> +	SET_SYSTEM_SLEEP_PM_OPS(flexcan_suspend, flexcan_resume)
> +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(flexcan_noirq_suspend, flexcan_noirq_resume)
> +};
>  
>  static struct platform_driver flexcan_driver = {
>  	.driver = {
> 

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

RE: [PATCH V6 2/2] can: flexcan: add self wakeup support

[Joakim Zhang]

Hi Marc,

Firstly, thanks for reviewing the patch. Then, I am confused to do upstream on "linux-can" or "linux-can-next" due to I find "linux-can" is more updated than "linux-can-next".

Best Regards,
Joakim Zhang

> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2018年11月23日 23:04
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org;
> robh+dt@kernel.org
> Cc: wg@grandegger.com; mark.rutland@arm.com;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; Aisheng DONG <aisheng.dong@nxp.com>
> Subject: Re: [PATCH V6 2/2] can: flexcan: add self wakeup support
> 
> On 11/23/18 9:35 AM, Joakim Zhang wrote:
> > From: Aisheng Dong <aisheng.dong@nxp.com>
> >
> > If wakeup is enabled, enter stop mode, else enter disabled mode. Self
> > wake can only work on stop mode.
> >
> > Starting from IMX6, the flexcan stop mode control bits is SoC
> > specific, move it out of IP driver and parse it from devicetree.
> >
> > Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> > ChangeLog:
> > V1->V2:
> > 	*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
> > V2->V3:
> > 	*add FLEXCAN_QUIRK_SETUP_STOP_MODE quirk.
> > 	*rename function.
> > 	*fix system can't be wakeuped during suspend.
> > V3->V4:
> > 	*normalize the code following Aisheng Dong's comments.
> > V4->V5:
> > 	*move enable/disable self wakeup feature into
> > 	 enter/exit_stop_mode() function.
> > V5->V6:
> > 	*exchange the order of flexcan_enable_wakeup_irq(priv, false)
> > 	 and disable_irq_wake(dev->irq).
> > ---
> >  drivers/net/can/flexcan.c | 172
> > ++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 163 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 8e972ef08637..da08a8c113d2 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -19,11 +19,14 @@
> >  #include <linux/delay.h>
> >  #include <linux/interrupt.h>
> >  #include <linux/io.h>
> > +#include <linux/mfd/syscon.h>
> > +#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
> >  #include <linux/module.h>
> >  #include <linux/of.h>
> >  #include <linux/of_device.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/regulator/consumer.h>
> > +#include <linux/regmap.h>
> >
> >  #define DRV_NAME			"flexcan"
> >
> > @@ -131,7 +134,8 @@
> >  	(FLEXCAN_ESR_ERR_BUS | FLEXCAN_ESR_ERR_STATE)  #define
> > FLEXCAN_ESR_ALL_INT \
> >  	(FLEXCAN_ESR_TWRN_INT | FLEXCAN_ESR_RWRN_INT | \
> > -	 FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT)
> > +	FLEXCAN_ESR_BOFF_INT | FLEXCAN_ESR_ERR_INT | \
> > +	FLEXCAN_ESR_WAK_INT)
> 
> I've fixed the indention while applying the patch.
> 
> >
> >  /* FLEXCAN interrupt flag register (IFLAG) bits */
> >  /* Errata ERR005829 step7: Reserve first valid MB */ @@ -190,6 +194,7
> > @@
> >  #define FLEXCAN_QUIRK_USE_OFF_TIMESTAMP	BIT(5) /* Use timestamp
> based offloading */
> >  #define FLEXCAN_QUIRK_BROKEN_PERR_STATE	BIT(6) /* No interrupt
> for error passive */
> >  #define FLEXCAN_QUIRK_DEFAULT_BIG_ENDIAN	BIT(7) /* default to BE
> register access */
> > +#define FLEXCAN_QUIRK_SETUP_STOP_MODE		BIT(8) /* Setup stop
> mode to support wakeup */
> >
> >  /* Structure of the message buffer */  struct flexcan_mb { @@ -254,6
> > +259,14 @@ struct flexcan_devtype_data {
> >  	u32 quirks;		/* quirks needed for different IP cores */
> >  };
> >
> > +struct flexcan_stop_mode {
> > +	struct regmap *gpr;
> > +	u8 req_gpr;
> > +	u8 req_bit;
> > +	u8 ack_gpr;
> > +	u8 ack_bit;
> > +};
> > +
> >  struct flexcan_priv {
> >  	struct can_priv can;
> >  	struct can_rx_offload offload;
> > @@ -270,6 +283,7 @@ struct flexcan_priv {
> >  	struct clk *clk_per;
> >  	const struct flexcan_devtype_data *devtype_data;
> >  	struct regulator *reg_xceiver;
> > +	struct flexcan_stop_mode stm;
> >
> >  	/* Read and Write APIs */
> >  	u32 (*read)(void __iomem *addr);
> > @@ -293,7 +307,8 @@ static const struct flexcan_devtype_data
> > fsl_imx28_devtype_data = {
> >
> >  static const struct flexcan_devtype_data fsl_imx6q_devtype_data = {
> >  	.quirks = FLEXCAN_QUIRK_DISABLE_RXFG |
> FLEXCAN_QUIRK_ENABLE_EACEN_RRS |
> > -		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE,
> > +		FLEXCAN_QUIRK_USE_OFF_TIMESTAMP |
> FLEXCAN_QUIRK_BROKEN_PERR_STATE |
> > +		FLEXCAN_QUIRK_SETUP_STOP_MODE,
> >  };
> >
> >  static const struct flexcan_devtype_data fsl_vf610_devtype_data = {
> > @@ -353,6 +368,49 @@ static inline void flexcan_write_le(u32 val, void
> __iomem *addr)
> >  	iowrite32(val, addr);
> >  }
> >
> > +static void flexcan_enable_wakeup_irq(struct flexcan_priv *priv, bool
> > +enable) {
> > +	struct flexcan_regs __iomem *regs = priv->regs;
> > +	u32 reg_mcr;
> > +
> > +	reg_mcr = priv->read(&regs->mcr);
> > +
> > +	if (enable)
> > +		reg_mcr |= FLEXCAN_MCR_WAK_MSK;
> > +	else
> > +		reg_mcr &= ~FLEXCAN_MCR_WAK_MSK;
> > +
> > +	priv->write(reg_mcr, &regs->mcr);
> > +}
> > +
> > +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
> > +{
> > +	struct flexcan_regs __iomem *regs = priv->regs;
> > +	u32 reg_mcr;
> > +
> > +	reg_mcr = priv->read(&regs->mcr);
> > +	reg_mcr |= FLEXCAN_MCR_SLF_WAK;
> > +	priv->write(reg_mcr, &regs->mcr);
> > +
> > +	/* enable stop request */
> > +	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> > +			   1 << priv->stm.req_bit, 1 << priv->stm.req_bit); }
> > +
> > +static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv)
> > +{
> > +	struct flexcan_regs __iomem *regs = priv->regs;
> > +	u32 reg_mcr;
> > +
> > +	/* remove stop request */
> > +	regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> > +			   1 << priv->stm.req_bit, 0);
> > +
> > +	reg_mcr = priv->read(&regs->mcr);
> > +	reg_mcr &= ~FLEXCAN_MCR_SLF_WAK;
> > +	priv->write(reg_mcr, &regs->mcr);
> > +}
> > +
> >  static inline void flexcan_error_irq_enable(const struct flexcan_priv
> > *priv)  {
> >  	struct flexcan_regs __iomem *regs = priv->regs; @@ -1244,6 +1302,57
> > @@ static void unregister_flexcandev(struct net_device *dev)
> >  	unregister_candev(dev);
> >  }
> >
> > +static int flexcan_setup_stop_mode(struct platform_device *pdev) {
> > +	struct net_device *dev = platform_get_drvdata(pdev);
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct device_node *gpr_np;
> > +	struct flexcan_priv *priv;
> > +	phandle phandle;
> > +	u32 out_val[5];
> > +	int ret;
> > +
> > +	if (!np)
> > +		return -EINVAL;
> > +
> > +	/* stop mode property format is:
> > +	 * <&gpr req_gpr req_bit ack_gpr ack_bit>.
> > +	 */
> > +	ret = of_property_read_u32_array(np, "fsl,stop-mode", out_val, 5);
> 
> I've changed the 5 to ARRAY_SIZE() while applying the patch + rewrap to
> 80 cols.
> 
> > +	if (ret) {
> > +		dev_dbg(&pdev->dev, "no stop-mode property\n");
> > +		return ret;
> > +	}
> > +	phandle = *out_val;
> > +
> > +	gpr_np = of_find_node_by_phandle(phandle);
> > +	if (!gpr_np) {
> > +		dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
> > +		return PTR_ERR(gpr_np);
> > +	}
> > +
> > +	priv = netdev_priv(dev);
> > +	priv->stm.gpr = syscon_node_to_regmap(gpr_np);
> > +	of_node_put(gpr_np);
> > +	if (IS_ERR(priv->stm.gpr)) {
> > +		dev_dbg(&pdev->dev, "could not find gpr regmap\n");
> > +		return PTR_ERR(priv->stm.gpr);
> > +	}
> > +
> > +	priv->stm.req_gpr = out_val[1];
> > +	priv->stm.req_bit = out_val[2];
> > +	priv->stm.ack_gpr = out_val[3];
> > +	priv->stm.ack_bit = out_val[4];
> > +
> > +	dev_dbg(&pdev->dev, "gpr %s req_gpr 0x%x req_bit %u ack_gpr 0x%x
> ack_bit %u\n",
> > +		gpr_np->full_name, priv->stm.req_gpr, priv->stm.req_bit,
> priv->stm.ack_gpr,
> > +		priv->stm.ack_bit);
> 
> For the hex values I've changed to 0x%x to 0x%02x + rewarp to 80 cols.
> 
> > +
> > +	device_set_wakeup_capable(&pdev->dev, true);
> > +
> > +	return 0;
> > +}
> > +
> >  static const struct of_device_id flexcan_of_match[] = {
> >  	{ .compatible = "fsl,imx6q-flexcan", .data = &fsl_imx6q_devtype_data, },
> >  	{ .compatible = "fsl,imx28-flexcan", .data =
> > &fsl_imx28_devtype_data, }, @@ -1396,6 +1505,12 @@ static int
> > flexcan_probe(struct platform_device *pdev)
> >
> >  	devm_can_led_init(dev);
> >
> > +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SETUP_STOP_MODE) {
> > +		err = flexcan_setup_stop_mode(pdev);
> > +		if (err)
> > +			dev_dbg(&pdev->dev, "failed to setup stop-mode\n");
> > +	}
> > +
> >  	dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
> >  		 priv->regs, dev->irq);
> >
> > @@ -1426,9 +1541,17 @@ static int __maybe_unused
> flexcan_suspend(struct device *device)
> >  	int err;
> >
> >  	if (netif_running(dev)) {
> > -		err = flexcan_chip_disable(priv);
> > -		if (err)
> > -			return err;
> > +		/* if wakeup is enabled, enter stop mode
> > +		 * else enter disabled mode.
> > +		 */
> > +		if (device_may_wakeup(device)) {
> > +			enable_irq_wake(dev->irq);
> > +			flexcan_enter_stop_mode(priv);
> > +		} else {
> > +			err = flexcan_chip_disable(priv);
> > +			if (err)
> > +				return err;
> > +		}
> >  		netif_stop_queue(dev);
> >  		netif_device_detach(dev);
> >  	}
> > @@ -1447,14 +1570,45 @@ static int __maybe_unused
> flexcan_resume(struct device *device)
> >  	if (netif_running(dev)) {
> >  		netif_device_attach(dev);
> >  		netif_start_queue(dev);
> > -		err = flexcan_chip_enable(priv);
> > -		if (err)
> > -			return err;
> > +		if (device_may_wakeup(device)) {
> > +			disable_irq_wake(dev->irq);
> > +		} else {
> > +			err = flexcan_chip_enable(priv);
> > +			if (err)
> > +				return err;
> > +		}
> >  	}
> >  	return 0;
> >  }
> >
> > -static SIMPLE_DEV_PM_OPS(flexcan_pm_ops, flexcan_suspend,
> > flexcan_resume);
> > +static int __maybe_unused flexcan_noirq_suspend(struct device
> > +*device) {
> > +	struct net_device *dev = dev_get_drvdata(device);
> > +	struct flexcan_priv *priv = netdev_priv(dev);
> > +
> > +	if (netif_running(dev) && device_may_wakeup(device))
> > +		flexcan_enable_wakeup_irq(priv, true);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __maybe_unused flexcan_noirq_resume(struct device *device)
> > +{
> > +	struct net_device *dev = dev_get_drvdata(device);
> > +	struct flexcan_priv *priv = netdev_priv(dev);
> > +
> > +	if (netif_running(dev) && device_may_wakeup(device)) {
> > +		flexcan_enable_wakeup_irq(priv, false);
> > +		flexcan_exit_stop_mode(priv);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct dev_pm_ops flexcan_pm_ops = {
> > +	SET_SYSTEM_SLEEP_PM_OPS(flexcan_suspend, flexcan_resume)
> > +	SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(flexcan_noirq_suspend,
> > +flexcan_noirq_resume) };
> >
> >  static struct platform_driver flexcan_driver = {
> >  	.driver = {
> >
> 
> Marc
> 
> --
> Pengutronix e.K.                  | Marc Kleine-Budde           |
> Industrial Linux Solutions        | Phone: +49-231-2826-924     |
> Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

Re: [PATCH V6 2/2] can: flexcan: add self wakeup support

[Marc Kleine-Budde]

On 11/26/18 2:12 AM, Joakim Zhang wrote:
> Firstly, thanks for reviewing the patch. Then, I am confused to do
> upstream on "linux-can" or "linux-can-next" due to I find "linux-can"
> is more updated than "linux-can-next".
linux-can is for fixes aginat the current net/master.
linux-can-next is for new features against net-next/master.

I just updates both branches and pushed linux-can-next/testing, too,
which is going the be the next pull request.

regards,
Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |

RE: [PATCH V6 0/2] can: flexcan: add can self wakeup support

[Aisheng DONG]

> -----Original Message-----
> From: Joakim Zhang
> Sent: Friday, November 23, 2018 4:35 PM
> To: linux-can@vger.kernel.org; mkl@pengutronix.de; robh+dt@kernel.org
> Cc: wg@grandegger.com; mark.rutland@arm.com; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; Joakim
> Zhang <qiangqing.zhang@nxp.com>
> Subject: [PATCH V6 0/2] can: flexcan: add can self wakeup support
> 
> This patch series intends to add CAN self wakeup support. The CAN controller
> can parse "fsl,stop-mode" property from device tree to enable self wakeup
> feature.
> 

As I'm already authored, this is just to tell this version looks good to me.
Reviewed-by: Dong Aisheng <aisheng.dong@nxp.com>

Regards
Dong Aisheng