* [PATCH V2 0/2] can: flexcan: Add CAN self wakeup support
@ 2018-10-26 9:00 Joakim Zhang
2018-10-26 9:00 ` [PATCH V2 1/2] can: flexcan: Add " Joakim Zhang
2018-10-26 9:00 ` [PATCH V2 2/2] Documentation: can: flexcan: Add stop mode property to device tree Joakim Zhang
0 siblings, 2 replies; 6+ messages in thread
From: Joakim Zhang @ 2018-10-26 9:00 UTC (permalink / raw)
To: linux-can
Cc: wg, mkl, robh+dt, mark.rutland, devicetree, linux-kernel,
dl-linux-imx, Joakim Zhang
This patch series intends to add CAN self wakeup support. The CAN
controller can parse stop mode property from device tree to enable self
wakeup feature.
ChangeLog:
V1->V2:
*add a vendor prefix in property (stop-mode -> fsl,stop-mode).
Dong Aisheng (2):
can: flexcan: Add self wakeup support
Documentation: can: flexcan: Add stop mode property to device tree
.../bindings/net/can/fsl-flexcan.txt | 8 +
drivers/net/can/flexcan.c | 141 ++++++++++++++++--
2 files changed, 140 insertions(+), 9 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH V2 1/2] can: flexcan: Add self wakeup support
2018-10-26 9:00 [PATCH V2 0/2] can: flexcan: Add CAN self wakeup support Joakim Zhang
@ 2018-10-26 9:00 ` Joakim Zhang
2018-10-30 13:44 ` Marc Kleine-Budde
2018-10-26 9:00 ` [PATCH V2 2/2] Documentation: can: flexcan: Add stop mode property to device tree Joakim Zhang
1 sibling, 1 reply; 6+ messages in thread
From: Joakim Zhang @ 2018-10-26 9:00 UTC (permalink / raw)
To: linux-can
Cc: wg, mkl, robh+dt, mark.rutland, devicetree, linux-kernel,
dl-linux-imx, A.s. Dong, Joakim Zhang
From: Dong Aisheng <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: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
drivers/net/can/flexcan.c | 141 +++++++++++++++++++++++++++++++++++---
1 file changed, 132 insertions(+), 9 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 3813f6708201..d4708ba28c44 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -19,12 +19,17 @@
#include <linux/delay.h>
#include <linux/interrupt.h>
#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/list.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/pm_runtime.h>
#include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
#define DRV_NAME "flexcan"
@@ -132,7 +137,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 */
@@ -255,6 +261,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;
@@ -272,6 +286,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);
@@ -392,6 +407,22 @@ static void flexcan_clks_disable(const struct flexcan_priv *priv)
clk_disable_unprepare(priv->clk_per);
}
+static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
+{
+ /* enable stop request */
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
+ 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)
+{
+ /* remove stop request */
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
+ regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
+ 1 << priv->stm.req_bit, 0);
+}
+
static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
{
if (!priv->reg_xceiver)
@@ -955,6 +986,10 @@ static int flexcan_chip_start(struct net_device *dev)
reg_mcr |= FLEXCAN_MCR_FEN |
FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
}
+
+ /* enable self wakeup */
+ reg_mcr |= FLEXCAN_MCR_WAK_MSK | FLEXCAN_MCR_SLF_WAK;
+
netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
priv->write(reg_mcr, ®s->mcr);
@@ -1240,6 +1275,56 @@ static void unregister_flexcandev(struct net_device *dev)
unregister_candev(dev);
}
+static int flexcan_of_parse_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 *node;
+ 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;
+
+ node = of_find_node_by_phandle(phandle);
+ if (!node) {
+ dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
+ return PTR_ERR(node);
+ }
+
+ priv = netdev_priv(dev);
+ priv->stm.gpr = syscon_node_to_regmap(node);
+ if (IS_ERR(priv->stm.gpr)) {
+ dev_dbg(&pdev->dev, "could not find gpr regmap\n");
+ return PTR_ERR(priv->stm.gpr);
+ }
+
+ of_node_put(node);
+
+ 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",
+ node->full_name, priv->stm.req_gpr,
+ priv->stm.req_bit, priv->stm.ack_gpr,
+ priv->stm.ack_bit);
+ 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, },
@@ -1271,6 +1356,7 @@ static int flexcan_probe(struct platform_device *pdev)
struct flexcan_regs __iomem *regs;
int err, irq;
u32 clock_freq = 0;
+ int wakeup = 1;
reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
@@ -1400,6 +1486,16 @@ static int flexcan_probe(struct platform_device *pdev)
devm_can_led_init(dev);
+ if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
+ err = flexcan_of_parse_stop_mode(pdev);
+ if (err) {
+ wakeup = 0;
+ dev_dbg(&pdev->dev, "failed to parse stop-mode\n");
+ }
+ }
+
+ device_set_wakeup_capable(&pdev->dev, wakeup);
+
pm_runtime_put(&pdev->dev);
dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
@@ -1437,10 +1533,18 @@ static int __maybe_unused flexcan_suspend(struct device *device)
int err = 0;
if (netif_running(dev)) {
- err = flexcan_chip_disable(priv);
- if (err)
- return err;
- err = pm_runtime_force_suspend(device);
+ /* 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;
+ err = pm_runtime_force_suspend(device);
+ }
netif_stop_queue(dev);
netif_device_detach(dev);
@@ -1461,10 +1565,12 @@ static int __maybe_unused flexcan_resume(struct device *device)
netif_device_attach(dev);
netif_start_queue(dev);
- err = pm_runtime_force_resume(device);
- if (err)
- return err;
- err = flexcan_chip_enable(priv);
+ if (!device_may_wakeup(device)) {
+ err = pm_runtime_force_resume(device);
+ if (err)
+ return err;
+ err = flexcan_chip_enable(priv);
+ }
}
return err;
}
@@ -1489,10 +1595,27 @@ static int __maybe_unused flexcan_runtime_resume(struct device *device)
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);
+
+ /* exit stop mode during noirq stage avoid continuously entering
+ * wakeup ISR before CAN resume back.
+ */
+ if (netif_running(dev) && device_may_wakeup(device)) {
+ disable_irq_wake(dev->irq);
+ 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_RUNTIME_PM_OPS(flexcan_runtime_suspend, flexcan_runtime_resume,
NULL)
+ SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, flexcan_noirq_resume)
};
static struct platform_driver flexcan_driver = {
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V2 2/2] Documentation: can: flexcan: Add stop mode property to device tree
2018-10-26 9:00 [PATCH V2 0/2] can: flexcan: Add CAN self wakeup support Joakim Zhang
2018-10-26 9:00 ` [PATCH V2 1/2] can: flexcan: Add " Joakim Zhang
@ 2018-10-26 9:00 ` Joakim Zhang
2018-10-30 21:48 ` Rob Herring
1 sibling, 1 reply; 6+ messages in thread
From: Joakim Zhang @ 2018-10-26 9:00 UTC (permalink / raw)
To: linux-can
Cc: wg, mkl, robh+dt, mark.rutland, devicetree, linux-kernel,
dl-linux-imx, A.s. Dong, Joakim Zhang
From: Dong Aisheng <aisheng.dong@nxp.com>
The FlexCAN controller can parse the stop mode property to enable CAN
self wakeup feature.
Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
---
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..23d56f7ebe3d 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2 1/2] can: flexcan: Add self wakeup support
2018-10-26 9:00 ` [PATCH V2 1/2] can: flexcan: Add " Joakim Zhang
@ 2018-10-30 13:44 ` Marc Kleine-Budde
2018-11-12 1:47 ` Joakim Zhang
0 siblings, 1 reply; 6+ messages in thread
From: Marc Kleine-Budde @ 2018-10-30 13:44 UTC (permalink / raw)
To: Joakim Zhang, linux-can
Cc: wg, robh+dt, mark.rutland, devicetree, linux-kernel,
dl-linux-imx, A.s. Dong
[-- Attachment #1.1: Type: text/plain, Size: 8991 bytes --]
On 10/26/2018 11:00 AM, Joakim Zhang wrote:
> From: Dong Aisheng <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: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> drivers/net/can/flexcan.c | 141 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 132 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 3813f6708201..d4708ba28c44 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -19,12 +19,17 @@
> #include <linux/delay.h>
> #include <linux/interrupt.h>
> #include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/list.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/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> +#include <linux/regmap.h>
>
> #define DRV_NAME "flexcan"
>
> @@ -132,7 +137,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 */
> @@ -255,6 +261,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;
> @@ -272,6 +286,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);
> @@ -392,6 +407,22 @@ static void flexcan_clks_disable(const struct flexcan_priv *priv)
> clk_disable_unprepare(priv->clk_per);
> }
>
> +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
> +{
> + /* enable stop request */
> + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
Why do you check the quirk here? This should only be called if the
device is wakeup capable.
> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> + 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
Please align with the opening bracket of "regmap_update_bits(".
> +}
> +
> +static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv)
> +{
> + /* remove stop request */
> + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
same here
> + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> + 1 << priv->stm.req_bit, 0);
same here
> +}
> +
> static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
> {
> if (!priv->reg_xceiver)
> @@ -955,6 +986,10 @@ static int flexcan_chip_start(struct net_device *dev)
> reg_mcr |= FLEXCAN_MCR_FEN |
> FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
> }
> +
> + /* enable self wakeup */
> + reg_mcr |= FLEXCAN_MCR_WAK_MSK | FLEXCAN_MCR_SLF_WAK;
> +
> netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
> priv->write(reg_mcr, ®s->mcr);
>
> @@ -1240,6 +1275,56 @@ static void unregister_flexcandev(struct net_device *dev)
> unregister_candev(dev);
> }
>
> +static int flexcan_of_parse_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 *node;
> + 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;
> +
> + node = of_find_node_by_phandle(phandle);
> + if (!node) {
> + dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
> + return PTR_ERR(node);
> + }
> +
> + priv = netdev_priv(dev);
> + priv->stm.gpr = syscon_node_to_regmap(node);
> + if (IS_ERR(priv->stm.gpr)) {
> + dev_dbg(&pdev->dev, "could not find gpr regmap\n");
> + return PTR_ERR(priv->stm.gpr);
> + }
> +
> + of_node_put(node);
> +
> + 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",
> + node->full_name, priv->stm.req_gpr,
> + priv->stm.req_bit, priv->stm.ack_gpr,
> + priv->stm.ack_bit);
> + 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, },
> @@ -1271,6 +1356,7 @@ static int flexcan_probe(struct platform_device *pdev)
> struct flexcan_regs __iomem *regs;
> int err, irq;
> u32 clock_freq = 0;
> + int wakeup = 1;
Make it a bool.
Why is is default enabled?
See below.
>
> reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
> if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER)
> @@ -1400,6 +1486,16 @@ static int flexcan_probe(struct platform_device *pdev)
>
> devm_can_led_init(dev);
>
> + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP) {
Please add another QUIRK.
> + err = flexcan_of_parse_stop_mode(pdev);
> + if (err) {
> + wakeup = 0;
> + dev_dbg(&pdev->dev, "failed to parse stop-mode\n");
> + }
> + }
> +
> + device_set_wakeup_capable(&pdev->dev, wakeup);
What about moving this into "flexcan_of_parse_stop_mode(pdev);", and
rename this function into ..._setup_stop_mode() or something similar.
> +
> pm_runtime_put(&pdev->dev);
>
> dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
> @@ -1437,10 +1533,18 @@ static int __maybe_unused flexcan_suspend(struct device *device)
> int err = 0;
>
> if (netif_running(dev)) {
> - err = flexcan_chip_disable(priv);
> - if (err)
> - return err;
> - err = pm_runtime_force_suspend(device);
> + /* 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;
> + err = pm_runtime_force_suspend(device);
> + }
>
> netif_stop_queue(dev);
> netif_device_detach(dev);
> @@ -1461,10 +1565,12 @@ static int __maybe_unused flexcan_resume(struct device *device)
> netif_device_attach(dev);
> netif_start_queue(dev);
>
> - err = pm_runtime_force_resume(device);
> - if (err)
> - return err;
> - err = flexcan_chip_enable(priv);
> + if (!device_may_wakeup(device)) {
> + err = pm_runtime_force_resume(device);
> + if (err)
> + return err;
> + err = flexcan_chip_enable(priv);
> + }
> }
> return err;
> }
> @@ -1489,10 +1595,27 @@ static int __maybe_unused flexcan_runtime_resume(struct device *device)
> 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);
> +
> + /* exit stop mode during noirq stage avoid continuously entering
> + * wakeup ISR before CAN resume back.
> + */
> + if (netif_running(dev) && device_may_wakeup(device)) {
> + disable_irq_wake(dev->irq);
> + 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_RUNTIME_PM_OPS(flexcan_runtime_suspend, flexcan_runtime_resume,
> NULL)
> + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, flexcan_noirq_resume)
> };
>
> static struct platform_driver flexcan_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 --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2 2/2] Documentation: can: flexcan: Add stop mode property to device tree
2018-10-26 9:00 ` [PATCH V2 2/2] Documentation: can: flexcan: Add stop mode property to device tree Joakim Zhang
@ 2018-10-30 21:48 ` Rob Herring
0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2018-10-30 21:48 UTC (permalink / raw)
To: Joakim Zhang
Cc: linux-can, wg, mkl, robh+dt, mark.rutland, devicetree,
linux-kernel, dl-linux-imx, A.s. Dong, Joakim Zhang
On Fri, 26 Oct 2018 09:00:40 +0000, Joakim Zhang wrote:
> From: Dong Aisheng <aisheng.dong@nxp.com>
>
> The FlexCAN controller can parse the stop mode property to enable CAN
> self wakeup feature.
>
> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> Documentation/devicetree/bindings/net/can/fsl-flexcan.txt | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH V2 1/2] can: flexcan: Add self wakeup support
2018-10-30 13:44 ` Marc Kleine-Budde
@ 2018-11-12 1:47 ` Joakim Zhang
0 siblings, 0 replies; 6+ messages in thread
From: Joakim Zhang @ 2018-11-12 1:47 UTC (permalink / raw)
To: Marc Kleine-Budde, linux-can
Cc: wg, robh+dt, mark.rutland, devicetree, linux-kernel,
dl-linux-imx, A.s. Dong
> -----Original Message-----
> From: Marc Kleine-Budde <mkl@pengutronix.de>
> Sent: 2018年10月30日 21:45
> To: Joakim Zhang <qiangqing.zhang@nxp.com>; linux-can@vger.kernel.org
> Cc: wg@grandegger.com; robh+dt@kernel.org; mark.rutland@arm.com;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; dl-linux-imx
> <linux-imx@nxp.com>; A.s. Dong <aisheng.dong@nxp.com>
> Subject: Re: [PATCH V2 1/2] can: flexcan: Add self wakeup support
>
> On 10/26/2018 11:00 AM, Joakim Zhang wrote:
> > From: Dong Aisheng <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: Dong Aisheng <aisheng.dong@nxp.com>
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> > ---
> > drivers/net/can/flexcan.c | 141
> > +++++++++++++++++++++++++++++++++++---
> > 1 file changed, 132 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index 3813f6708201..d4708ba28c44 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -19,12 +19,17 @@
> > #include <linux/delay.h>
> > #include <linux/interrupt.h>
> > #include <linux/io.h>
> > +#include <linux/kernel.h>
> > +#include <linux/list.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/pm_runtime.h>
> > #include <linux/regulator/consumer.h>
> > +#include <linux/regmap.h>
> >
> > #define DRV_NAME "flexcan"
> >
> > @@ -132,7 +137,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 */ @@ -255,6
> > +261,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;
> > @@ -272,6 +286,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);
> > @@ -392,6 +407,22 @@ static void flexcan_clks_disable(const struct
> flexcan_priv *priv)
> > clk_disable_unprepare(priv->clk_per);
> > }
> >
> > +static inline void flexcan_enter_stop_mode(struct flexcan_priv *priv)
> > +{
> > + /* enable stop request */
> > + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
>
> Why do you check the quirk here? This should only be called if the device is
> wakeup capable.
Yes, you are right. I have moved out this check.
> > + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> > + 1 << priv->stm.req_bit, 1 << priv->stm.req_bit);
>
> Please align with the opening bracket of "regmap_update_bits(".
Okay.
> > +}
> > +
> > +static inline void flexcan_exit_stop_mode(struct flexcan_priv *priv)
> > +{
> > + /* remove stop request */
> > + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
>
> same here
Have done.
> > + regmap_update_bits(priv->stm.gpr, priv->stm.req_gpr,
> > + 1 << priv->stm.req_bit, 0);
>
> same here
Have done.
> > +}
> > +
> > static inline int flexcan_transceiver_enable(const struct
> > flexcan_priv *priv) {
> > if (!priv->reg_xceiver)
> > @@ -955,6 +986,10 @@ static int flexcan_chip_start(struct net_device
> *dev)
> > reg_mcr |= FLEXCAN_MCR_FEN |
> > FLEXCAN_MCR_MAXMB(priv->tx_mb_idx);
> > }
> > +
> > + /* enable self wakeup */
> > + reg_mcr |= FLEXCAN_MCR_WAK_MSK | FLEXCAN_MCR_SLF_WAK;
> > +
> > netdev_dbg(dev, "%s: writing mcr=0x%08x", __func__, reg_mcr);
> > priv->write(reg_mcr, ®s->mcr);
> >
> > @@ -1240,6 +1275,56 @@ static void unregister_flexcandev(struct
> net_device *dev)
> > unregister_candev(dev);
> > }
> >
> > +static int flexcan_of_parse_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 *node;
> > + 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;
> > +
> > + node = of_find_node_by_phandle(phandle);
> > + if (!node) {
> > + dev_dbg(&pdev->dev, "could not find gpr node by phandle\n");
> > + return PTR_ERR(node);
> > + }
> > +
> > + priv = netdev_priv(dev);
> > + priv->stm.gpr = syscon_node_to_regmap(node);
> > + if (IS_ERR(priv->stm.gpr)) {
> > + dev_dbg(&pdev->dev, "could not find gpr regmap\n");
> > + return PTR_ERR(priv->stm.gpr);
> > + }
> > +
> > + of_node_put(node);
> > +
> > + 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",
> > + node->full_name, priv->stm.req_gpr,
> > + priv->stm.req_bit, priv->stm.ack_gpr,
> > + priv->stm.ack_bit);
> > + 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, }, @@ -1271,6 +1356,7 @@ static int
> flexcan_probe(struct platform_device *pdev)
> > struct flexcan_regs __iomem *regs;
> > int err, irq;
> > u32 clock_freq = 0;
> > + int wakeup = 1;
>
> Make it a bool.
> Why is is default enabled?
> See below.
>
> >
> > reg_xceiver = devm_regulator_get(&pdev->dev, "xceiver");
> > if (PTR_ERR(reg_xceiver) == -EPROBE_DEFER) @@ -1400,6 +1486,16 @@
> > static int flexcan_probe(struct platform_device *pdev)
> >
> > devm_can_led_init(dev);
> >
> > + if (priv->devtype_data->quirks & FLEXCAN_QUIRK_USE_OFF_TIMESTAMP)
> {
>
> Please add another QUIRK.
>
> > + err = flexcan_of_parse_stop_mode(pdev);
> > + if (err) {
> > + wakeup = 0;
> > + dev_dbg(&pdev->dev, "failed to parse stop-mode\n");
> > + }
> > + }
> > +
> > + device_set_wakeup_capable(&pdev->dev, wakeup);
>
> What about moving this into "flexcan_of_parse_stop_mode(pdev);", and
> rename this function into ..._setup_stop_mode() or something similar.
I have renamed the function and moved "device_set_wakeup_capable()" into this function.
Best Regards,
Joakim Zhang
> > +
> > pm_runtime_put(&pdev->dev);
> >
> > dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n", @@
> > -1437,10 +1533,18 @@ static int __maybe_unused flexcan_suspend(struct
> device *device)
> > int err = 0;
> >
> > if (netif_running(dev)) {
> > - err = flexcan_chip_disable(priv);
> > - if (err)
> > - return err;
> > - err = pm_runtime_force_suspend(device);
> > + /* 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;
> > + err = pm_runtime_force_suspend(device);
> > + }
> >
> > netif_stop_queue(dev);
> > netif_device_detach(dev);
> > @@ -1461,10 +1565,12 @@ static int __maybe_unused
> flexcan_resume(struct device *device)
> > netif_device_attach(dev);
> > netif_start_queue(dev);
> >
> > - err = pm_runtime_force_resume(device);
> > - if (err)
> > - return err;
> > - err = flexcan_chip_enable(priv);
> > + if (!device_may_wakeup(device)) {
> > + err = pm_runtime_force_resume(device);
> > + if (err)
> > + return err;
> > + err = flexcan_chip_enable(priv);
> > + }
> > }
> > return err;
> > }
> > @@ -1489,10 +1595,27 @@ static int __maybe_unused
> flexcan_runtime_resume(struct device *device)
> > 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);
> > +
> > + /* exit stop mode during noirq stage avoid continuously entering
> > + * wakeup ISR before CAN resume back.
> > + */
> > + if (netif_running(dev) && device_may_wakeup(device)) {
> > + disable_irq_wake(dev->irq);
> > + 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_RUNTIME_PM_OPS(flexcan_runtime_suspend,
> flexcan_runtime_resume,
> > NULL)
> > + SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(NULL, flexcan_noirq_resume)
> > };
> >
> > static struct platform_driver flexcan_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 |
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-11-12 1:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-26 9:00 [PATCH V2 0/2] can: flexcan: Add CAN self wakeup support Joakim Zhang
2018-10-26 9:00 ` [PATCH V2 1/2] can: flexcan: Add " Joakim Zhang
2018-10-30 13:44 ` Marc Kleine-Budde
2018-11-12 1:47 ` Joakim Zhang
2018-10-26 9:00 ` [PATCH V2 2/2] Documentation: can: flexcan: Add stop mode property to device tree Joakim Zhang
2018-10-30 21:48 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).