All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mailbox: imx: Support runtime PM
@ 2020-04-13 12:25 ` Anson Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Anson Huang @ 2020-04-13 12:25 UTC (permalink / raw)
  To: jassisinghbrar, shawnguo, s.hauer, kernel, festevam,
	linux-kernel, linux-arm-kernel
  Cc: Linux-imx

Some power hungry sub-systems like VPU has its own MUs which also
use mailbox driver, current mailbox driver uses platform driver
model and MU's power will be ON after driver probed and left ON
there, it may cause the whole sub-system can NOT enter lower power
mode, take VPU driver for example, it has runtime PM support, but
due to its MU always ON, the VPU sub-system will be always ON and
consume many power during kernel idle.

To save power in kernel idle, mailbox driver needs to support
runtime PM in order to power off MU when it is unused. However,
the runtime suspend/resume can ONLY be implemented in mailbox's
.shutdown/.startup callback, so its consumer needs to call
mbox_request_channel()/mbox_free_channel() in consumer driver's
runtime PM callback, then the MU's power will be ON/OFF along with
consumer's runtime PM status.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 7906624..97bf0ac 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -12,6 +12,7 @@
 #include <linux/mailbox_controller.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
@@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
 	struct imx_mu_con_priv *cp = chan->con_priv;
 	int ret;
 
+	pm_runtime_get_sync(priv->dev);
 	if (cp->type == IMX_MU_TYPE_TXDB) {
 		/* Tx doorbell don't have ACK support */
 		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
@@ -323,6 +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 
 	if (cp->type == IMX_MU_TYPE_TXDB) {
 		tasklet_kill(&cp->txdb_tasklet);
+		pm_runtime_put_sync(priv->dev);
 		return;
 	}
 
@@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 	}
 
 	free_irq(priv->irq, chan);
+	pm_runtime_put_sync(priv->dev);
 }
 
 static const struct mbox_chan_ops imx_mu_ops = {
@@ -508,7 +512,27 @@ static int imx_mu_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
-	return devm_mbox_controller_register(dev, &priv->mbox);
+	ret = devm_mbox_controller_register(dev, &priv->mbox);
+	if (ret)
+		return ret;
+
+	pm_runtime_enable(dev);
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		goto disable_runtime_pm;
+	}
+
+	ret = pm_runtime_put_sync(dev);
+	if (ret < 0)
+		goto disable_runtime_pm;
+
+	return 0;
+
+disable_runtime_pm:
+	pm_runtime_disable(dev);
+	return ret;
 }
 
 static int imx_mu_remove(struct platform_device *pdev)
@@ -516,6 +540,7 @@ static int imx_mu_remove(struct platform_device *pdev)
 	struct imx_mu_priv *priv = platform_get_drvdata(pdev);
 
 	clk_disable_unprepare(priv->clk);
+	pm_runtime_disable(priv->dev);
 
 	return 0;
 }
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH] mailbox: imx: Support runtime PM
@ 2020-04-13 12:25 ` Anson Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Anson Huang @ 2020-04-13 12:25 UTC (permalink / raw)
  To: jassisinghbrar, shawnguo, s.hauer, kernel, festevam,
	linux-kernel, linux-arm-kernel
  Cc: Linux-imx

Some power hungry sub-systems like VPU has its own MUs which also
use mailbox driver, current mailbox driver uses platform driver
model and MU's power will be ON after driver probed and left ON
there, it may cause the whole sub-system can NOT enter lower power
mode, take VPU driver for example, it has runtime PM support, but
due to its MU always ON, the VPU sub-system will be always ON and
consume many power during kernel idle.

To save power in kernel idle, mailbox driver needs to support
runtime PM in order to power off MU when it is unused. However,
the runtime suspend/resume can ONLY be implemented in mailbox's
.shutdown/.startup callback, so its consumer needs to call
mbox_request_channel()/mbox_free_channel() in consumer driver's
runtime PM callback, then the MU's power will be ON/OFF along with
consumer's runtime PM status.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
index 7906624..97bf0ac 100644
--- a/drivers/mailbox/imx-mailbox.c
+++ b/drivers/mailbox/imx-mailbox.c
@@ -12,6 +12,7 @@
 #include <linux/mailbox_controller.h>
 #include <linux/module.h>
 #include <linux/of_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/slab.h>
 
 #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
@@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
 	struct imx_mu_con_priv *cp = chan->con_priv;
 	int ret;
 
+	pm_runtime_get_sync(priv->dev);
 	if (cp->type == IMX_MU_TYPE_TXDB) {
 		/* Tx doorbell don't have ACK support */
 		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
@@ -323,6 +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 
 	if (cp->type == IMX_MU_TYPE_TXDB) {
 		tasklet_kill(&cp->txdb_tasklet);
+		pm_runtime_put_sync(priv->dev);
 		return;
 	}
 
@@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
 	}
 
 	free_irq(priv->irq, chan);
+	pm_runtime_put_sync(priv->dev);
 }
 
 static const struct mbox_chan_ops imx_mu_ops = {
@@ -508,7 +512,27 @@ static int imx_mu_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, priv);
 
-	return devm_mbox_controller_register(dev, &priv->mbox);
+	ret = devm_mbox_controller_register(dev, &priv->mbox);
+	if (ret)
+		return ret;
+
+	pm_runtime_enable(dev);
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret < 0) {
+		pm_runtime_put_noidle(dev);
+		goto disable_runtime_pm;
+	}
+
+	ret = pm_runtime_put_sync(dev);
+	if (ret < 0)
+		goto disable_runtime_pm;
+
+	return 0;
+
+disable_runtime_pm:
+	pm_runtime_disable(dev);
+	return ret;
 }
 
 static int imx_mu_remove(struct platform_device *pdev)
@@ -516,6 +540,7 @@ static int imx_mu_remove(struct platform_device *pdev)
 	struct imx_mu_priv *priv = platform_get_drvdata(pdev);
 
 	clk_disable_unprepare(priv->clk);
+	pm_runtime_disable(priv->dev);
 
 	return 0;
 }
-- 
2.7.4


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mailbox: imx: Support runtime PM
  2020-04-13 12:25 ` Anson Huang
@ 2020-04-13 12:52   ` Marc Kleine-Budde
  -1 siblings, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2020-04-13 12:52 UTC (permalink / raw)
  To: Anson Huang, jassisinghbrar, shawnguo, s.hauer, kernel, festevam,
	linux-kernel, linux-arm-kernel
  Cc: Linux-imx

On 4/13/20 2:25 PM, Anson Huang wrote:
> Some power hungry sub-systems like VPU has its own MUs which also
> use mailbox driver, current mailbox driver uses platform driver
> model and MU's power will be ON after driver probed and left ON
> there, it may cause the whole sub-system can NOT enter lower power
> mode, take VPU driver for example, it has runtime PM support, but
> due to its MU always ON, the VPU sub-system will be always ON and
> consume many power during kernel idle.
> 
> To save power in kernel idle, mailbox driver needs to support
> runtime PM in order to power off MU when it is unused. However,
> the runtime suspend/resume can ONLY be implemented in mailbox's
> .shutdown/.startup callback, so its consumer needs to call
> mbox_request_channel()/mbox_free_channel() in consumer driver's
> runtime PM callback, then the MU's power will be ON/OFF along with
> consumer's runtime PM status.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 7906624..97bf0ac 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -12,6 +12,7 @@
>  #include <linux/mailbox_controller.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  
>  #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
> @@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
>  	struct imx_mu_con_priv *cp = chan->con_priv;
>  	int ret;
>  
> +	pm_runtime_get_sync(priv->dev);
>  	if (cp->type == IMX_MU_TYPE_TXDB) {
>  		/* Tx doorbell don't have ACK support */
>  		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
> @@ -323,6 +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
>  
>  	if (cp->type == IMX_MU_TYPE_TXDB) {
>  		tasklet_kill(&cp->txdb_tasklet);
> +		pm_runtime_put_sync(priv->dev);
>  		return;
>  	}
>  
> @@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
>  	}
>  
>  	free_irq(priv->irq, chan);
> +	pm_runtime_put_sync(priv->dev);
>  }
>  
>  static const struct mbox_chan_ops imx_mu_ops = {
> @@ -508,7 +512,27 @@ static int imx_mu_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  
> -	return devm_mbox_controller_register(dev, &priv->mbox);
> +	ret = devm_mbox_controller_register(dev, &priv->mbox);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_enable(dev);
> +

First registering at the system and then setting up the power management
looks racy. Don't know if this is serialized by some other means.

> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(dev);
> +		goto disable_runtime_pm;
> +	}
> +
> +	ret = pm_runtime_put_sync(dev);
> +	if (ret < 0)
> +		goto disable_runtime_pm;
> +
> +	return 0;

Marc

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mailbox: imx: Support runtime PM
@ 2020-04-13 12:52   ` Marc Kleine-Budde
  0 siblings, 0 replies; 8+ messages in thread
From: Marc Kleine-Budde @ 2020-04-13 12:52 UTC (permalink / raw)
  To: Anson Huang, jassisinghbrar, shawnguo, s.hauer, kernel, festevam,
	linux-kernel, linux-arm-kernel
  Cc: Linux-imx

On 4/13/20 2:25 PM, Anson Huang wrote:
> Some power hungry sub-systems like VPU has its own MUs which also
> use mailbox driver, current mailbox driver uses platform driver
> model and MU's power will be ON after driver probed and left ON
> there, it may cause the whole sub-system can NOT enter lower power
> mode, take VPU driver for example, it has runtime PM support, but
> due to its MU always ON, the VPU sub-system will be always ON and
> consume many power during kernel idle.
> 
> To save power in kernel idle, mailbox driver needs to support
> runtime PM in order to power off MU when it is unused. However,
> the runtime suspend/resume can ONLY be implemented in mailbox's
> .shutdown/.startup callback, so its consumer needs to call
> mbox_request_channel()/mbox_free_channel() in consumer driver's
> runtime PM callback, then the MU's power will be ON/OFF along with
> consumer's runtime PM status.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mailbox/imx-mailbox.c b/drivers/mailbox/imx-mailbox.c
> index 7906624..97bf0ac 100644
> --- a/drivers/mailbox/imx-mailbox.c
> +++ b/drivers/mailbox/imx-mailbox.c
> @@ -12,6 +12,7 @@
>  #include <linux/mailbox_controller.h>
>  #include <linux/module.h>
>  #include <linux/of_device.h>
> +#include <linux/pm_runtime.h>
>  #include <linux/slab.h>
>  
>  #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
> @@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
>  	struct imx_mu_con_priv *cp = chan->con_priv;
>  	int ret;
>  
> +	pm_runtime_get_sync(priv->dev);
>  	if (cp->type == IMX_MU_TYPE_TXDB) {
>  		/* Tx doorbell don't have ACK support */
>  		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet,
> @@ -323,6 +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
>  
>  	if (cp->type == IMX_MU_TYPE_TXDB) {
>  		tasklet_kill(&cp->txdb_tasklet);
> +		pm_runtime_put_sync(priv->dev);
>  		return;
>  	}
>  
> @@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
>  	}
>  
>  	free_irq(priv->irq, chan);
> +	pm_runtime_put_sync(priv->dev);
>  }
>  
>  static const struct mbox_chan_ops imx_mu_ops = {
> @@ -508,7 +512,27 @@ static int imx_mu_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, priv);
>  
> -	return devm_mbox_controller_register(dev, &priv->mbox);
> +	ret = devm_mbox_controller_register(dev, &priv->mbox);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_enable(dev);
> +

First registering at the system and then setting up the power management
looks racy. Don't know if this is serialized by some other means.

> +	ret = pm_runtime_get_sync(dev);
> +	if (ret < 0) {
> +		pm_runtime_put_noidle(dev);
> +		goto disable_runtime_pm;
> +	}
> +
> +	ret = pm_runtime_put_sync(dev);
> +	if (ret < 0)
> +		goto disable_runtime_pm;
> +
> +	return 0;

Marc

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] mailbox: imx: Support runtime PM
  2020-04-13 12:52   ` Marc Kleine-Budde
@ 2020-04-13 13:13     ` Anson Huang
  -1 siblings, 0 replies; 8+ messages in thread
From: Anson Huang @ 2020-04-13 13:13 UTC (permalink / raw)
  To: Marc Kleine-Budde, jassisinghbrar, shawnguo, s.hauer, kernel,
	festevam, linux-kernel, linux-arm-kernel
  Cc: dl-linux-imx

Hi, Marc

> Subject: Re: [PATCH] mailbox: imx: Support runtime PM
> 
> On 4/13/20 2:25 PM, Anson Huang wrote:
> > Some power hungry sub-systems like VPU has its own MUs which also use
> > mailbox driver, current mailbox driver uses platform driver model and
> > MU's power will be ON after driver probed and left ON there, it may
> > cause the whole sub-system can NOT enter lower power mode, take VPU
> > driver for example, it has runtime PM support, but due to its MU
> > always ON, the VPU sub-system will be always ON and consume many power
> > during kernel idle.
> >
> > To save power in kernel idle, mailbox driver needs to support runtime
> > PM in order to power off MU when it is unused. However, the runtime
> > suspend/resume can ONLY be implemented in mailbox's .shutdown/.startup
> > callback, so its consumer needs to call
> > mbox_request_channel()/mbox_free_channel() in consumer driver's
> > runtime PM callback, then the MU's power will be ON/OFF along with
> > consumer's runtime PM status.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
> >  1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c
> > b/drivers/mailbox/imx-mailbox.c index 7906624..97bf0ac 100644
> > --- a/drivers/mailbox/imx-mailbox.c
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/mailbox_controller.h>  #include <linux/module.h>
> > #include <linux/of_device.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/slab.h>
> >
> >  #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
> > @@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
> >  	struct imx_mu_con_priv *cp = chan->con_priv;
> >  	int ret;
> >
> > +	pm_runtime_get_sync(priv->dev);
> >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> >  		/* Tx doorbell don't have ACK support */
> >  		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet, @@ -323,6
> > +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
> >
> >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> >  		tasklet_kill(&cp->txdb_tasklet);
> > +		pm_runtime_put_sync(priv->dev);
> >  		return;
> >  	}
> >
> > @@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan
> *chan)
> >  	}
> >
> >  	free_irq(priv->irq, chan);
> > +	pm_runtime_put_sync(priv->dev);
> >  }
> >
> >  static const struct mbox_chan_ops imx_mu_ops = { @@ -508,7 +512,27
> @@
> > static int imx_mu_probe(struct platform_device *pdev)
> >
> >  	platform_set_drvdata(pdev, priv);
> >
> > -	return devm_mbox_controller_register(dev, &priv->mbox);
> > +	ret = devm_mbox_controller_register(dev, &priv->mbox);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pm_runtime_enable(dev);
> > +
> 
> First registering at the system and then setting up the power management
> looks racy. Don't know if this is serialized by some other means.

Don't have other means, just refer to
drivers/mailbox/omap-mailbox.c, it has same sequence, NOT sure if there is
special requirement about this sequence.

Thanks,
Anson

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] mailbox: imx: Support runtime PM
@ 2020-04-13 13:13     ` Anson Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Anson Huang @ 2020-04-13 13:13 UTC (permalink / raw)
  To: Marc Kleine-Budde, jassisinghbrar, shawnguo, s.hauer, kernel,
	festevam, linux-kernel, linux-arm-kernel
  Cc: dl-linux-imx

Hi, Marc

> Subject: Re: [PATCH] mailbox: imx: Support runtime PM
> 
> On 4/13/20 2:25 PM, Anson Huang wrote:
> > Some power hungry sub-systems like VPU has its own MUs which also use
> > mailbox driver, current mailbox driver uses platform driver model and
> > MU's power will be ON after driver probed and left ON there, it may
> > cause the whole sub-system can NOT enter lower power mode, take VPU
> > driver for example, it has runtime PM support, but due to its MU
> > always ON, the VPU sub-system will be always ON and consume many power
> > during kernel idle.
> >
> > To save power in kernel idle, mailbox driver needs to support runtime
> > PM in order to power off MU when it is unused. However, the runtime
> > suspend/resume can ONLY be implemented in mailbox's .shutdown/.startup
> > callback, so its consumer needs to call
> > mbox_request_channel()/mbox_free_channel() in consumer driver's
> > runtime PM callback, then the MU's power will be ON/OFF along with
> > consumer's runtime PM status.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
> >  1 file changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mailbox/imx-mailbox.c
> > b/drivers/mailbox/imx-mailbox.c index 7906624..97bf0ac 100644
> > --- a/drivers/mailbox/imx-mailbox.c
> > +++ b/drivers/mailbox/imx-mailbox.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/mailbox_controller.h>  #include <linux/module.h>
> > #include <linux/of_device.h>
> > +#include <linux/pm_runtime.h>
> >  #include <linux/slab.h>
> >
> >  #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
> > @@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan *chan)
> >  	struct imx_mu_con_priv *cp = chan->con_priv;
> >  	int ret;
> >
> > +	pm_runtime_get_sync(priv->dev);
> >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> >  		/* Tx doorbell don't have ACK support */
> >  		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet, @@ -323,6
> > +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
> >
> >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> >  		tasklet_kill(&cp->txdb_tasklet);
> > +		pm_runtime_put_sync(priv->dev);
> >  		return;
> >  	}
> >
> > @@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan
> *chan)
> >  	}
> >
> >  	free_irq(priv->irq, chan);
> > +	pm_runtime_put_sync(priv->dev);
> >  }
> >
> >  static const struct mbox_chan_ops imx_mu_ops = { @@ -508,7 +512,27
> @@
> > static int imx_mu_probe(struct platform_device *pdev)
> >
> >  	platform_set_drvdata(pdev, priv);
> >
> > -	return devm_mbox_controller_register(dev, &priv->mbox);
> > +	ret = devm_mbox_controller_register(dev, &priv->mbox);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pm_runtime_enable(dev);
> > +
> 
> First registering at the system and then setting up the power management
> looks racy. Don't know if this is serialized by some other means.

Don't have other means, just refer to
drivers/mailbox/omap-mailbox.c, it has same sequence, NOT sure if there is
special requirement about this sequence.

Thanks,
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] mailbox: imx: Support runtime PM
  2020-04-13 13:13     ` Anson Huang
@ 2020-05-28  2:01       ` Anson Huang
  -1 siblings, 0 replies; 8+ messages in thread
From: Anson Huang @ 2020-05-28  2:01 UTC (permalink / raw)
  To: Marc Kleine-Budde, jassisinghbrar, shawnguo, s.hauer, kernel,
	festevam, linux-kernel, linux-arm-kernel
  Cc: dl-linux-imx

Gentle ping...


> Subject: RE: [PATCH] mailbox: imx: Support runtime PM
> 
> Hi, Marc
> 
> > Subject: Re: [PATCH] mailbox: imx: Support runtime PM
> >
> > On 4/13/20 2:25 PM, Anson Huang wrote:
> > > Some power hungry sub-systems like VPU has its own MUs which also
> > > use mailbox driver, current mailbox driver uses platform driver
> > > model and MU's power will be ON after driver probed and left ON
> > > there, it may cause the whole sub-system can NOT enter lower power
> > > mode, take VPU driver for example, it has runtime PM support, but
> > > due to its MU always ON, the VPU sub-system will be always ON and
> > > consume many power during kernel idle.
> > >
> > > To save power in kernel idle, mailbox driver needs to support
> > > runtime PM in order to power off MU when it is unused. However, the
> > > runtime suspend/resume can ONLY be implemented in mailbox's
> > > .shutdown/.startup callback, so its consumer needs to call
> > > mbox_request_channel()/mbox_free_channel() in consumer driver's
> > > runtime PM callback, then the MU's power will be ON/OFF along with
> > > consumer's runtime PM status.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > >  drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
> > >  1 file changed, 26 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mailbox/imx-mailbox.c
> > > b/drivers/mailbox/imx-mailbox.c index 7906624..97bf0ac 100644
> > > --- a/drivers/mailbox/imx-mailbox.c
> > > +++ b/drivers/mailbox/imx-mailbox.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/mailbox_controller.h>  #include <linux/module.h>
> > > #include <linux/of_device.h>
> > > +#include <linux/pm_runtime.h>
> > >  #include <linux/slab.h>
> > >
> > >  #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
> > > @@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan
> *chan)
> > >  	struct imx_mu_con_priv *cp = chan->con_priv;
> > >  	int ret;
> > >
> > > +	pm_runtime_get_sync(priv->dev);
> > >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> > >  		/* Tx doorbell don't have ACK support */
> > >  		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet, @@
> -323,6
> > > +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
> > >
> > >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> > >  		tasklet_kill(&cp->txdb_tasklet);
> > > +		pm_runtime_put_sync(priv->dev);
> > >  		return;
> > >  	}
> > >
> > > @@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan
> > *chan)
> > >  	}
> > >
> > >  	free_irq(priv->irq, chan);
> > > +	pm_runtime_put_sync(priv->dev);
> > >  }
> > >
> > >  static const struct mbox_chan_ops imx_mu_ops = { @@ -508,7 +512,27
> > @@
> > > static int imx_mu_probe(struct platform_device *pdev)
> > >
> > >  	platform_set_drvdata(pdev, priv);
> > >
> > > -	return devm_mbox_controller_register(dev, &priv->mbox);
> > > +	ret = devm_mbox_controller_register(dev, &priv->mbox);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pm_runtime_enable(dev);
> > > +
> >
> > First registering at the system and then setting up the power
> > management looks racy. Don't know if this is serialized by some other
> means.
> 
> Don't have other means, just refer to
> drivers/mailbox/omap-mailbox.c, it has same sequence, NOT sure if there is
> special requirement about this sequence.
> 
> Thanks,
> Anson

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [PATCH] mailbox: imx: Support runtime PM
@ 2020-05-28  2:01       ` Anson Huang
  0 siblings, 0 replies; 8+ messages in thread
From: Anson Huang @ 2020-05-28  2:01 UTC (permalink / raw)
  To: Marc Kleine-Budde, jassisinghbrar, shawnguo, s.hauer, kernel,
	festevam, linux-kernel, linux-arm-kernel
  Cc: dl-linux-imx

Gentle ping...


> Subject: RE: [PATCH] mailbox: imx: Support runtime PM
> 
> Hi, Marc
> 
> > Subject: Re: [PATCH] mailbox: imx: Support runtime PM
> >
> > On 4/13/20 2:25 PM, Anson Huang wrote:
> > > Some power hungry sub-systems like VPU has its own MUs which also
> > > use mailbox driver, current mailbox driver uses platform driver
> > > model and MU's power will be ON after driver probed and left ON
> > > there, it may cause the whole sub-system can NOT enter lower power
> > > mode, take VPU driver for example, it has runtime PM support, but
> > > due to its MU always ON, the VPU sub-system will be always ON and
> > > consume many power during kernel idle.
> > >
> > > To save power in kernel idle, mailbox driver needs to support
> > > runtime PM in order to power off MU when it is unused. However, the
> > > runtime suspend/resume can ONLY be implemented in mailbox's
> > > .shutdown/.startup callback, so its consumer needs to call
> > > mbox_request_channel()/mbox_free_channel() in consumer driver's
> > > runtime PM callback, then the MU's power will be ON/OFF along with
> > > consumer's runtime PM status.
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > >  drivers/mailbox/imx-mailbox.c | 27 ++++++++++++++++++++++++++-
> > >  1 file changed, 26 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mailbox/imx-mailbox.c
> > > b/drivers/mailbox/imx-mailbox.c index 7906624..97bf0ac 100644
> > > --- a/drivers/mailbox/imx-mailbox.c
> > > +++ b/drivers/mailbox/imx-mailbox.c
> > > @@ -12,6 +12,7 @@
> > >  #include <linux/mailbox_controller.h>  #include <linux/module.h>
> > > #include <linux/of_device.h>
> > > +#include <linux/pm_runtime.h>
> > >  #include <linux/slab.h>
> > >
> > >  #define IMX_MU_xSR_GIPn(x)	BIT(28 + (3 - (x)))
> > > @@ -287,6 +288,7 @@ static int imx_mu_startup(struct mbox_chan
> *chan)
> > >  	struct imx_mu_con_priv *cp = chan->con_priv;
> > >  	int ret;
> > >
> > > +	pm_runtime_get_sync(priv->dev);
> > >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> > >  		/* Tx doorbell don't have ACK support */
> > >  		tasklet_init(&cp->txdb_tasklet, imx_mu_txdb_tasklet, @@
> -323,6
> > > +325,7 @@ static void imx_mu_shutdown(struct mbox_chan *chan)
> > >
> > >  	if (cp->type == IMX_MU_TYPE_TXDB) {
> > >  		tasklet_kill(&cp->txdb_tasklet);
> > > +		pm_runtime_put_sync(priv->dev);
> > >  		return;
> > >  	}
> > >
> > > @@ -341,6 +344,7 @@ static void imx_mu_shutdown(struct mbox_chan
> > *chan)
> > >  	}
> > >
> > >  	free_irq(priv->irq, chan);
> > > +	pm_runtime_put_sync(priv->dev);
> > >  }
> > >
> > >  static const struct mbox_chan_ops imx_mu_ops = { @@ -508,7 +512,27
> > @@
> > > static int imx_mu_probe(struct platform_device *pdev)
> > >
> > >  	platform_set_drvdata(pdev, priv);
> > >
> > > -	return devm_mbox_controller_register(dev, &priv->mbox);
> > > +	ret = devm_mbox_controller_register(dev, &priv->mbox);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pm_runtime_enable(dev);
> > > +
> >
> > First registering at the system and then setting up the power
> > management looks racy. Don't know if this is serialized by some other
> means.
> 
> Don't have other means, just refer to
> drivers/mailbox/omap-mailbox.c, it has same sequence, NOT sure if there is
> special requirement about this sequence.
> 
> Thanks,
> Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-05-28  2:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-13 12:25 [PATCH] mailbox: imx: Support runtime PM Anson Huang
2020-04-13 12:25 ` Anson Huang
2020-04-13 12:52 ` Marc Kleine-Budde
2020-04-13 12:52   ` Marc Kleine-Budde
2020-04-13 13:13   ` Anson Huang
2020-04-13 13:13     ` Anson Huang
2020-05-28  2:01     ` Anson Huang
2020-05-28  2:01       ` Anson Huang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.