From: Dong Aisheng <[email protected]>
Flexcan will be disabled during suspend if no wakeup function required
and enabled after resume accordingly. During this period, we could
explicitly disable clocks.
Implement Runtime PM which will:
1) Keep device in suspend state (clocks disabled) if it's not openned
2) Make Power Domain framework be able to shutdown the corresponding power
domain of this device.
Signed-off-by: Dong Aisheng <[email protected]>
Signed-off-by: Joakim Zhang <[email protected]>
---
drivers/net/can/flexcan.c | 125 +++++++++++++++++++++++++-------------
1 file changed, 82 insertions(+), 43 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 8e972ef08637..3813f6708201 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -23,6 +23,7 @@
#include <linux/of.h>
#include <linux/of_device.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/regulator/consumer.h>
#define DRV_NAME "flexcan"
@@ -266,6 +267,7 @@ struct flexcan_priv {
u32 reg_imask1_default;
u32 reg_imask2_default;
+ struct device *dev;
struct clk *clk_ipg;
struct clk *clk_per;
const struct flexcan_devtype_data *devtype_data;
@@ -369,6 +371,27 @@ static inline void flexcan_error_irq_disable(const struct flexcan_priv *priv)
priv->write(reg_ctrl, ®s->ctrl);
}
+static int flexcan_clks_enable(const struct flexcan_priv *priv)
+{
+ int err;
+
+ err = clk_prepare_enable(priv->clk_ipg);
+ if (err)
+ return err;
+
+ err = clk_prepare_enable(priv->clk_per);
+ if (err)
+ clk_disable_unprepare(priv->clk_ipg);
+
+ return err;
+}
+
+static void flexcan_clks_disable(const struct flexcan_priv *priv)
+{
+ clk_disable_unprepare(priv->clk_ipg);
+ clk_disable_unprepare(priv->clk_per);
+}
+
static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
{
if (!priv->reg_xceiver)
@@ -495,19 +518,11 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
const struct flexcan_priv *priv = netdev_priv(dev);
int err;
- err = clk_prepare_enable(priv->clk_ipg);
- if (err)
- return err;
-
- err = clk_prepare_enable(priv->clk_per);
- if (err)
- goto out_disable_ipg;
+ pm_runtime_get_sync(priv->dev);
err = __flexcan_get_berr_counter(dev, bec);
- clk_disable_unprepare(priv->clk_per);
- out_disable_ipg:
- clk_disable_unprepare(priv->clk_ipg);
+ pm_runtime_put(priv->dev);
return err;
}
@@ -1096,17 +1111,13 @@ static int flexcan_open(struct net_device *dev)
struct flexcan_priv *priv = netdev_priv(dev);
int err;
- err = clk_prepare_enable(priv->clk_ipg);
+ err = pm_runtime_get_sync(priv->dev);
if (err)
return err;
- err = clk_prepare_enable(priv->clk_per);
- if (err)
- goto out_disable_ipg;
-
err = open_candev(dev);
if (err)
- goto out_disable_per;
+ goto out_pm_runtime;
err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
if (err)
@@ -1128,10 +1139,8 @@ static int flexcan_open(struct net_device *dev)
free_irq(dev->irq, dev);
out_close:
close_candev(dev);
- out_disable_per:
- clk_disable_unprepare(priv->clk_per);
- out_disable_ipg:
- clk_disable_unprepare(priv->clk_ipg);
+ out_pm_runtime:
+ pm_runtime_put(priv->dev);
return err;
}
@@ -1145,11 +1154,11 @@ static int flexcan_close(struct net_device *dev)
flexcan_chip_stop(dev);
free_irq(dev->irq, dev);
- clk_disable_unprepare(priv->clk_per);
- clk_disable_unprepare(priv->clk_ipg);
close_candev(dev);
+ pm_runtime_put(priv->dev);
+
can_led_event(dev, CAN_LED_EVENT_STOP);
return 0;
@@ -1188,18 +1197,10 @@ static int register_flexcandev(struct net_device *dev)
struct flexcan_regs __iomem *regs = priv->regs;
u32 reg, err;
- err = clk_prepare_enable(priv->clk_ipg);
- if (err)
- return err;
-
- err = clk_prepare_enable(priv->clk_per);
- if (err)
- goto out_disable_ipg;
-
/* select "bus clock", chip must be disabled */
err = flexcan_chip_disable(priv);
if (err)
- goto out_disable_per;
+ return err;
reg = priv->read(®s->ctrl);
reg |= FLEXCAN_CTRL_CLK_SRC;
priv->write(reg, ®s->ctrl);
@@ -1228,13 +1229,8 @@ static int register_flexcandev(struct net_device *dev)
err = register_candev(dev);
- /* disable core and turn off clocks */
out_chip_disable:
flexcan_chip_disable(priv);
- out_disable_per:
- clk_disable_unprepare(priv->clk_per);
- out_disable_ipg:
- clk_disable_unprepare(priv->clk_ipg);
return err;
}
@@ -1342,6 +1338,7 @@ static int flexcan_probe(struct platform_device *pdev)
priv->write = flexcan_write_le;
}
+ priv->dev = &pdev->dev;
priv->can.clock.freq = clock_freq;
priv->can.bittiming_const = &flexcan_bittiming_const;
priv->can.do_set_mode = flexcan_set_mode;
@@ -1388,22 +1385,35 @@ static int flexcan_probe(struct platform_device *pdev)
if (err)
goto failed_offload;
+ pm_runtime_enable(&pdev->dev);
+ err = pm_runtime_get_sync(&pdev->dev);
+ if (err < 0) {
+ dev_err(&pdev->dev, "pm_runtime_get failed(%d)\n", err);
+ goto failed_rpm_disable;
+ }
+
err = register_flexcandev(dev);
if (err) {
dev_err(&pdev->dev, "registering netdev failed\n");
- goto failed_register;
+ goto failed_rpm_put;
}
devm_can_led_init(dev);
+ pm_runtime_put(&pdev->dev);
+
dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
priv->regs, dev->irq);
return 0;
+ failed_rpm_put:
+ pm_runtime_put(&pdev->dev);
+ failed_rpm_disable:
+ pm_runtime_disable(&pdev->dev);
failed_offload:
- failed_register:
free_candev(dev);
+
return err;
}
@@ -1413,6 +1423,7 @@ static int flexcan_remove(struct platform_device *pdev)
struct flexcan_priv *priv = netdev_priv(dev);
unregister_flexcandev(dev);
+ pm_runtime_disable(&pdev->dev);
can_rx_offload_del(&priv->offload);
free_candev(dev);
@@ -1423,38 +1434,66 @@ static int __maybe_unused flexcan_suspend(struct device *device)
{
struct net_device *dev = dev_get_drvdata(device);
struct flexcan_priv *priv = netdev_priv(dev);
- int err;
+ int err = 0;
if (netif_running(dev)) {
err = flexcan_chip_disable(priv);
if (err)
return err;
+ err = pm_runtime_force_suspend(device);
+
netif_stop_queue(dev);
netif_device_detach(dev);
}
priv->can.state = CAN_STATE_SLEEPING;
- return 0;
+ return err;
}
static int __maybe_unused flexcan_resume(struct device *device)
{
struct net_device *dev = dev_get_drvdata(device);
struct flexcan_priv *priv = netdev_priv(dev);
- int err;
+ int err = 0;
priv->can.state = CAN_STATE_ERROR_ACTIVE;
if (netif_running(dev)) {
netif_device_attach(dev);
netif_start_queue(dev);
- err = flexcan_chip_enable(priv);
+
+ err = pm_runtime_force_resume(device);
if (err)
return err;
+ err = flexcan_chip_enable(priv);
}
+ return err;
+}
+
+static int __maybe_unused flexcan_runtime_suspend(struct device *device)
+{
+ struct net_device *dev = dev_get_drvdata(device);
+ struct flexcan_priv *priv = netdev_priv(dev);
+
+ flexcan_clks_disable(priv);
+
return 0;
}
-static SIMPLE_DEV_PM_OPS(flexcan_pm_ops, flexcan_suspend, flexcan_resume);
+static int __maybe_unused flexcan_runtime_resume(struct device *device)
+{
+ struct net_device *dev = dev_get_drvdata(device);
+ struct flexcan_priv *priv = netdev_priv(dev);
+
+ flexcan_clks_enable(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)
+};
static struct platform_driver flexcan_driver = {
.driver = {
--
2.17.1
On 10/19/18 11:45 AM, Joakim Zhang wrote:
> From: Dong Aisheng <[email protected]>
>
> Flexcan will be disabled during suspend if no wakeup function required
> and enabled after resume accordingly. During this period, we could
> explicitly disable clocks.
>
> Implement Runtime PM which will:
> 1) Keep device in suspend state (clocks disabled) if it's not openned
> 2) Make Power Domain framework be able to shutdown the corresponding power
> domain of this device.
Without CONFIG_PM the device fails to probe:
> [ 214.420606] flexcan 2090000.flexcan: 2090000.flexcan supply xceiver not found, using dummy regulator
> [ 214.445952] flexcan 2090000.flexcan: Linked as a consumer to regulator.0
> [ 214.453448] flexcan 2090000.flexcan: registering netdev failed
> [ 214.459666] flexcan 2090000.flexcan: Dropping the link to regulator.0
> [ 214.472066] flexcan: probe of 2090000.flexcan failed with error -110
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 |
Hi Marc,
Best Regards,
Joakim Zhang
> -----Original Message-----
> From: Marc Kleine-Budde <[email protected]>
> Sent: 2018年11月21日 22:36
> To: Joakim Zhang <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; Aisheng
> DONG <[email protected]>
> Subject: Re: [PATCH] can: flexcan: Implement CAN Runtime PM
>
> On 10/19/18 11:45 AM, Joakim Zhang wrote:
> > From: Dong Aisheng <[email protected]>
> >
> > Flexcan will be disabled during suspend if no wakeup function required
> > and enabled after resume accordingly. During this period, we could
> > explicitly disable clocks.
> >
> > Implement Runtime PM which will:
> > 1) Keep device in suspend state (clocks disabled) if it's not openned
> > 2) Make Power Domain framework be able to shutdown the corresponding
> > power domain of this device.
>
> Without CONFIG_PM the device fails to probe:
>
> > [ 214.420606] flexcan 2090000.flexcan: 2090000.flexcan supply xceiver
> > not found, using dummy regulator [ 214.445952] flexcan
> > 2090000.flexcan: Linked as a consumer to regulator.0 [ 214.453448]
> > flexcan 2090000.flexcan: registering netdev failed [ 214.459666]
> > flexcan 2090000.flexcan: Dropping the link to regulator.0 [
> > 214.472066] flexcan: probe of 2090000.flexcan failed with error -110
I will firstly send the patch to do FlexCAN wakeup upstream, you could review the patch "can: flexcan: add self wakeup support" when you are free.
After you have reviewed, then I will send patch to do FlexCAN runtime PM upstream. It will be convenient for you to review the patch.
Thanks a lot for your review.
Best Regards,
Joakim Zhang
> 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 |
> -----Original Message-----
> From: Marc Kleine-Budde <[email protected]>
> Sent: 2018年11月21日 22:36
> To: Joakim Zhang <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; Aisheng
> DONG <[email protected]>
> Subject: Re: [PATCH] can: flexcan: Implement CAN Runtime PM
>
> On 10/19/18 11:45 AM, Joakim Zhang wrote:
> > From: Dong Aisheng <[email protected]>
> >
> > Flexcan will be disabled during suspend if no wakeup function required
> > and enabled after resume accordingly. During this period, we could
> > explicitly disable clocks.
> >
> > Implement Runtime PM which will:
> > 1) Keep device in suspend state (clocks disabled) if it's not openned
> > 2) Make Power Domain framework be able to shutdown the corresponding
> > power domain of this device.
>
> Without CONFIG_PM the device fails to probe:
>
> > [ 214.420606] flexcan 2090000.flexcan: 2090000.flexcan supply xceiver
> > not found, using dummy regulator [ 214.445952] flexcan
> > 2090000.flexcan: Linked as a consumer to regulator.0 [ 214.453448]
> > flexcan 2090000.flexcan: registering netdev failed [ 214.459666]
> > flexcan 2090000.flexcan: Dropping the link to regulator.0 [
> > 214.472066] flexcan: probe of 2090000.flexcan failed with error -110
>
> Marc
Hi Marc,
I would sent V2 rebased on patch "can: flexcan: add self wakeup support", and runtime pm works normally on MX6SX-SDB and MX7D-SDB. So, could you tell me which board you tested on that causes the device probe failed?
And that CONFIG_PM has set in imx_v6_v7_defconfig which config file I used.
Best Regards,
Joakim Zhang
> --
> 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 |
On 11/27/18 8:44 AM, Joakim Zhang wrote:
>>> Flexcan will be disabled during suspend if no wakeup function required
>>> and enabled after resume accordingly. During this period, we could
>>> explicitly disable clocks.
>>>
>>> Implement Runtime PM which will:
>>> 1) Keep device in suspend state (clocks disabled) if it's not openned
>>> 2) Make Power Domain framework be able to shutdown the corresponding
>>> power domain of this device.
>>
>> Without CONFIG_PM the device fails to probe:
>>
>>> [ 214.420606] flexcan 2090000.flexcan: 2090000.flexcan supply xceiver
>>> not found, using dummy regulator [ 214.445952] flexcan
>>> 2090000.flexcan: Linked as a consumer to regulator.0 [ 214.453448]
>>> flexcan 2090000.flexcan: registering netdev failed [ 214.459666]
>>> flexcan 2090000.flexcan: Dropping the link to regulator.0 [
>>> 214.472066] flexcan: probe of 2090000.flexcan failed with error -110
> I would sent V2 rebased on patch "can: flexcan: add self wakeup
> support", and runtime pm works normally on MX6SX-SDB and MX7D-SDB.
> So, could you tell me which board you tested on that causes the
> device probe failed?
I tested on an imx6dl riotboard
(https://elixir.bootlin.com/linux/latest/source/arch/arm/boot/dts/imx6dl-riotboard.dts
+ patches to enable CAN1) with an external PHY.
> And that CONFIG_PM has set in imx_v6_v7_defconfig which config file I used.
I disabled CONFIG_PM on purpose.
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 |
> -----Original Message-----
> From: Marc Kleine-Budde [mailto:[email protected]]
> Sent: Tuesday, November 27, 2018 3:52 PM
> To: Joakim Zhang <[email protected]>; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; dl-linux-imx <[email protected]>; Aisheng
> DONG <[email protected]>
> Subject: Re: [PATCH] can: flexcan: Implement CAN Runtime PM
>
> On 11/27/18 8:44 AM, Joakim Zhang wrote:
> >>> Flexcan will be disabled during suspend if no wakeup function
> >>> required and enabled after resume accordingly. During this period,
> >>> we could explicitly disable clocks.
> >>>
> >>> Implement Runtime PM which will:
> >>> 1) Keep device in suspend state (clocks disabled) if it's not
> >>> openned
> >>> 2) Make Power Domain framework be able to shutdown the corresponding
> >>> power domain of this device.
> >>
> >> Without CONFIG_PM the device fails to probe:
> >>
> >>> [ 214.420606] flexcan 2090000.flexcan: 2090000.flexcan supply
> >>> xceiver not found, using dummy regulator [ 214.445952] flexcan
> >>> 2090000.flexcan: Linked as a consumer to regulator.0 [ 214.453448]
> >>> flexcan 2090000.flexcan: registering netdev failed [ 214.459666]
> >>> flexcan 2090000.flexcan: Dropping the link to regulator.0 [
> >>> 214.472066] flexcan: probe of 2090000.flexcan failed with error -110
>
> > I would sent V2 rebased on patch "can: flexcan: add self wakeup
> > support", and runtime pm works normally on MX6SX-SDB and MX7D-SDB.
> > So, could you tell me which board you tested on that causes the device
> > probe failed?
>
> I tested on an imx6dl riotboard
> (https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.
> bootlin.com%2Flinux%2Flatest%2Fsource%2Farch%2Farm%2Fboot%2Fdts%2Fi
> mx6dl-riotboard.dts&data=02%7C01%7Caisheng.dong%40nxp.com%7C8
> 4cfd3f3a08b4c7953fd08d6543d42c1%7C686ea1d3bc2b4c6fa92cd99c5c3016
> 35%7C0%7C0%7C636789019420569534&sdata=rugKpmVaNn0ajTdctyi%
> 2F%2Bnwr%2FqqIBcikAP7eXLp6fcA%3D&reserved=0
> + patches to enable CAN1) with an external PHY.
>
> > And that CONFIG_PM has set in imx_v6_v7_defconfig which config file I
> used.
>
> I disabled CONFIG_PM on purpose.
Joakim,
You need run the same test as Marc.
I guess It might because without CONFIG_PM, Runtime PM API
Simply return positive but the clock may still not enabled, thus cause
a CAN enable timeout.
You can check it.
Regards
Dong Aisheng
>
> 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 |
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.
> pengutronix.de&data=02%7C01%7Caisheng.dong%40nxp.com%7C84cfd3
> f3a08b4c7953fd08d6543d42c1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7
> C0%7C0%7C636789019420569534&sdata=Us6cO9QfYkw%2B6Nhj68zLr
> wrNxUCRjcC0WDdmr9MOzHM%3D&reserved=0 |