Re: [PATCH V4] can: flexcan: implement can Runtime PM
On 11/30/18 9:53 AM, Joakim Zhang wrote: > From: Aisheng Dong > > Flexcan will be disabled during suspend if no wakeup function required and > enabled after resume accordingly. During this period, we could explicitly > disable clocks. > Since PM is optional, the clock is enabled at probe to guarante the > clock is running when PM is not enabled in the kernel. > > Implement Runtime PM which will: > 1) Without CONFIG_PM, clock is running whether Flexcan up or down. > 2) With CONFIG_PM, clock enabled while Flexcan up and disabled when >Flexcan down. > 3) Disable clock when do system suspend and enable clock while system >resume. > 4) Make Power Domain framework be able to shutdown the corresponding >power domain of this device. > > Signed-off-by: Aisheng Dong > Signed-off-by: Joakim Zhang > --- > ChangeLog: > V1->V2: > *rebased on patch "can: flexcan: add self wakeup support". > V2->V3: > *fix device fails to probe without CONFIG_PM. > V3->V4: > *runtime pm enable should ahead of registering device. > *disable device even if keeping the clocks on. > --- > drivers/net/can/flexcan.c | 111 +- > 1 file changed, 73 insertions(+), 38 deletions(-) > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > index 0f36eafe3ac1..cad42f20cfe5 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -277,6 +278,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; > @@ -444,6 +446,27 @@ static inline void flexcan_error_irq_disable(const > struct flexcan_priv *priv) > priv->write(reg_ctrl, >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); The original code first disabled the per then the ipg. > +} > + > static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv) > { > if (!priv->reg_xceiver) > @@ -570,19 +593,13 @@ 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) > + err = pm_runtime_get_sync(priv->dev); > + if (err < 0) > return err; > > - err = clk_prepare_enable(priv->clk_per); > - if (err) > - goto out_disable_ipg; > - > 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; > } > @@ -1215,17 +1232,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); > - if (err) > + err = pm_runtime_get_sync(priv->dev); > + if (err < 0) > 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_disable_clks; nitpick: you do a pm_runtime_put, so rename the label... > > err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev); > if (err) > @@ -1288,10 +1301,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_disable_clks: > + pm_runtime_put(priv->dev); ...to out_runtime_put > > return err; > } > @@ -1306,10 +1317,9 @@ static int flexcan_close(struct net_device *dev) > > can_rx_offload_del(>offload); > 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); > > @@ -1349,18 +1359,14 @@ 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); > + err =
RE: [PATCH V4] can: flexcan: implement can Runtime PM
Kindly Ping... Best Regards, Joakim Zhang > -Original Message- > From: Joakim Zhang > Sent: 2019年1月17日 14:23 > To: m...@pengutronix.de; linux-...@vger.kernel.org > Cc: w...@grandegger.com; net...@vger.kernel.org; > linux-kernel@vger.kernel.org; dl-linux-imx ; Aisheng > Dong > Subject: RE: [PATCH V4] can: flexcan: implement can Runtime PM > > > Kindly Ping... > > Best Regards, > Joakim Zhang > > > -Original Message- > > From: Joakim Zhang > > Sent: 2018年11月30日 16:53 > > To: m...@pengutronix.de; linux-...@vger.kernel.org > > Cc: w...@grandegger.com; net...@vger.kernel.org; > > linux-kernel@vger.kernel.org; dl-linux-imx ; > > Aisheng DONG ; Joakim Zhang > > > > Subject: [PATCH V4] can: flexcan: implement can Runtime PM > > > > From: Aisheng Dong > > > > Flexcan will be disabled during suspend if no wakeup function required > > and enabled after resume accordingly. During this period, we could > > explicitly disable clocks. > > Since PM is optional, the clock is enabled at probe to guarante the > > clock is running when PM is not enabled in the kernel. > > > > Implement Runtime PM which will: > > 1) Without CONFIG_PM, clock is running whether Flexcan up or down. > > 2) With CONFIG_PM, clock enabled while Flexcan up and disabled when > >Flexcan down. > > 3) Disable clock when do system suspend and enable clock while system > >resume. > > 4) Make Power Domain framework be able to shutdown the corresponding > >power domain of this device. > > > > Signed-off-by: Aisheng Dong > > Signed-off-by: Joakim Zhang > > --- > > ChangeLog: > > V1->V2: > > *rebased on patch "can: flexcan: add self wakeup support". > > V2->V3: > > *fix device fails to probe without CONFIG_PM. > > V3->V4: > > *runtime pm enable should ahead of registering device. > > *disable device even if keeping the clocks on. > > --- > > drivers/net/can/flexcan.c | 111 > > +- > > 1 file changed, 73 insertions(+), 38 deletions(-) > > > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c > > index > > 0f36eafe3ac1..cad42f20cfe5 100644 > > --- a/drivers/net/can/flexcan.c > > +++ b/drivers/net/can/flexcan.c > > @@ -24,6 +24,7 @@ > > #include > > #include > > #include > > +#include > > #include #include > > > > @@ -277,6 +278,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; @@ -444,6 +446,27 > > @@ static inline void flexcan_error_irq_disable(const struct flexcan_priv > *priv) > > priv->write(reg_ctrl, >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) > > @@ -570,19 +593,13 @@ 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) > > + err = pm_runtime_get_sync(priv->dev); > > + if (err < 0) > > return err; > > > > - err = clk_prepare_enable(priv->clk_per); > > - if (err) > > - goto out_disable_ipg; > > - > > 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; > > } > > @@ -1215,17 +1232,13 @@ static int flexcan_open(struct net_device *dev) > > struct flexcan_priv *priv = netdev_priv(d
RE: [PATCH V4] can: flexcan: implement can Runtime PM
Kindly Ping... Best Regards, Joakim Zhang > -Original Message- > From: Joakim Zhang > Sent: 2018年11月30日 16:53 > To: m...@pengutronix.de; linux-...@vger.kernel.org > Cc: w...@grandegger.com; net...@vger.kernel.org; > linux-kernel@vger.kernel.org; dl-linux-imx ; Aisheng > DONG ; Joakim Zhang > Subject: [PATCH V4] can: flexcan: implement can Runtime PM > > From: Aisheng Dong > > Flexcan will be disabled during suspend if no wakeup function required and > enabled after resume accordingly. During this period, we could explicitly > disable clocks. > Since PM is optional, the clock is enabled at probe to guarante the clock is > running when PM is not enabled in the kernel. > > Implement Runtime PM which will: > 1) Without CONFIG_PM, clock is running whether Flexcan up or down. > 2) With CONFIG_PM, clock enabled while Flexcan up and disabled when >Flexcan down. > 3) Disable clock when do system suspend and enable clock while system >resume. > 4) Make Power Domain framework be able to shutdown the corresponding >power domain of this device. > > Signed-off-by: Aisheng Dong > Signed-off-by: Joakim Zhang > --- > ChangeLog: > V1->V2: > *rebased on patch "can: flexcan: add self wakeup support". > V2->V3: > *fix device fails to probe without CONFIG_PM. > V3->V4: > *runtime pm enable should ahead of registering device. > *disable device even if keeping the clocks on. > --- > drivers/net/can/flexcan.c | 111 +- > 1 file changed, 73 insertions(+), 38 deletions(-) > > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c index > 0f36eafe3ac1..cad42f20cfe5 100644 > --- a/drivers/net/can/flexcan.c > +++ b/drivers/net/can/flexcan.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -277,6 +278,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; @@ -444,6 +446,27 > @@ static inline void flexcan_error_irq_disable(const struct flexcan_priv > *priv) > priv->write(reg_ctrl, >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) > @@ -570,19 +593,13 @@ 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) > + err = pm_runtime_get_sync(priv->dev); > + if (err < 0) > return err; > > - err = clk_prepare_enable(priv->clk_per); > - if (err) > - goto out_disable_ipg; > - > 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; > } > @@ -1215,17 +1232,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); > - if (err) > + err = pm_runtime_get_sync(priv->dev); > + if (err < 0) > 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_disable_clks; > > err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev); > if (err) > @@ -1288,10 +1301,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_disable_clks: > + pm_runtime_put(priv->dev); > > return err; > } > @@ -1306,10 +1317,9 @@ static int flexcan_close(struct net_device *dev) > > can_rx_offload_del(>offload); > 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); > > @@ -1349,18 +1359,14 @@ static int register_flexcandev(struct