diff mbox

[1/5,v3] fbdev: sh_mobile_meram: Add enable/disble hooks for LCDC

Message ID 1308728992-9660-2-git-send-email-dhobsong@igel.co.jp (mailing list archive)
State New, archived
Headers show

Commit Message

Damian Hobson-Garcia June 22, 2011, 7:49 a.m. UTC 
Add hooks to allow the LCDC to increase/decrease the MERAM PM reference
counts

Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
---
Changes from V2

Comments

Paul Mundt June 24, 2011, 5:34 a.m. UTC | #1 
On Wed, Jun 22, 2011 at 04:49:48PM +0900, Damian Hobson-Garcia wrote:
> +static int sh_mobile_meram_pm_get_sync(struct sh_mobile_meram_info *pdata)
> +{
> +	if (!pdata || !pdata->pdev)
> +		return -EINVAL;
> +
> +	dev_dbg(&pdata->pdev->dev, "Enabling sh_mobile_meram clock.");
> +	pm_runtime_get_sync(&pdata->pdev->dev);
> +	return 0;
> +}
> +

On Wed, Jun 22, 2011 at 04:49:49PM +0900, Damian Hobson-Garcia wrote:
> @@ -259,6 +259,11 @@ static void sh_mobile_lcdc_clk_on(struct sh_mobile_lcdc_priv *priv)
>  		pm_runtime_get_sync(priv->dev);
>  		if (priv->dot_clk)
>  			clk_enable(priv->dot_clk);
> +		if (priv->meram_dev && priv->meram_dev->ops) {
> +			struct sh_mobile_meram_info *mdev;
> +			mdev = priv->meram_dev;
> +			mdev->ops->meram_pm_get_sync(mdev);
> +		}
>  	}
>  }
>  
I'm not sure that I really see the point in the callbacks. The callbacks
would make sense in the case where you're dealing with opaque types that
you don't wish to have knowledge of in the other drivers, but when all
you're doing is fetching the pointer and wrapping verbatim in to the
runtime pm calls, it just seems like a pointless layer of indirection.

You could easily just do this as:

	if (priv->meram_dev)
		pm_runtime_get_sync(&priv->meram_dev->pdev->dev);

and be done with it.

This will also save you from having to add additional callbacks should
you decide that you suddenly require async behaviour or so in other
cases, too.
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Damian Hobson-Garcia June 29, 2011, 5:26 a.m. UTC | #2 
Hi Paul,

On 2011/06/24 14:34, Paul Mundt wrote:
> On Wed, Jun 22, 2011 at 04:49:48PM +0900, Damian Hobson-Garcia wrote:
>> +static int sh_mobile_meram_pm_get_sync(struct sh_mobile_meram_info *pdata)
>> +{
>> +	if (!pdata || !pdata->pdev)
>> +		return -EINVAL;
>> +
>> +	dev_dbg(&pdata->pdev->dev, "Enabling sh_mobile_meram clock.");
>> +	pm_runtime_get_sync(&pdata->pdev->dev);
>> +	return 0;
>> +}
>> +
> 
> On Wed, Jun 22, 2011 at 04:49:49PM +0900, Damian Hobson-Garcia wrote:
>> @@ -259,6 +259,11 @@ static void sh_mobile_lcdc_clk_on(struct sh_mobile_lcdc_priv *priv)
>>  		pm_runtime_get_sync(priv->dev);
>>  		if (priv->dot_clk)
>>  			clk_enable(priv->dot_clk);
>> +		if (priv->meram_dev && priv->meram_dev->ops) {
>> +			struct sh_mobile_meram_info *mdev;
>> +			mdev = priv->meram_dev;
>> +			mdev->ops->meram_pm_get_sync(mdev);
>> +		}
>>  	}
>>  }
>>  
> I'm not sure that I really see the point in the callbacks. The callbacks
> would make sense in the case where you're dealing with opaque types that
> you don't wish to have knowledge of in the other drivers, but when all
> you're doing is fetching the pointer and wrapping verbatim in to the
> runtime pm calls, it just seems like a pointless layer of indirection.
> 
> You could easily just do this as:
> 
> 	if (priv->meram_dev)
> 		pm_runtime_get_sync(&priv->meram_dev->pdev->dev);
> 
> and be done with it.
> 
> This will also save you from having to add additional callbacks should
> you decide that you suddenly require async behaviour or so in other
> cases, too.
Thanks for your comment.

You raise a good point here.  I'll get rid of the useless call.

Damian
--
To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

===============

* Change the names of the clk_on/clk_off callbacks to pm_get_sync/pm_put_sync
  to better reflect their actual functionality

* Make these callback functions static

 drivers/video/sh_mobile_meram.c |   27 +++++++++++++++++++++++++++
 include/video/sh_mobile_meram.h |    6 ++++++
 2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/video/sh_mobile_meram.c b/drivers/video/sh_mobile_meram.c
index 9170c82..371f129 100644
--- a/drivers/video/sh_mobile_meram.c
+++ b/drivers/video/sh_mobile_meram.c
@@ -12,6 +12,7 @@ 
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/device.h>
+#include <linux/pm_runtime.h>
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/platform_device.h>
@@ -460,11 +461,33 @@  static int sh_mobile_meram_update(struct sh_mobile_meram_info *pdata,
 	return 0;
 }
 
+static int sh_mobile_meram_pm_get_sync(struct sh_mobile_meram_info *pdata)
+{
+	if (!pdata || !pdata->pdev)
+		return -EINVAL;
+
+	dev_dbg(&pdata->pdev->dev, "Enabling sh_mobile_meram clock.");
+	pm_runtime_get_sync(&pdata->pdev->dev);
+	return 0;
+}
+
+static int sh_mobile_meram_pm_put_sync(struct sh_mobile_meram_info *pdata)
+{
+	if (!pdata || !pdata->pdev)
+		return -EINVAL;
+
+	dev_dbg(&pdata->pdev->dev, "Disabling sh_mobile_meram clock.");
+	pm_runtime_put_sync(&pdata->pdev->dev);
+	return 0;
+}
+
 static struct sh_mobile_meram_ops sh_mobile_meram_ops = {
 	.module			= THIS_MODULE,
 	.meram_register		= sh_mobile_meram_register,
 	.meram_unregister	= sh_mobile_meram_unregister,
 	.meram_update		= sh_mobile_meram_update,
+	.meram_pm_put_sync	= sh_mobile_meram_pm_put_sync,
+	.meram_pm_get_sync	= sh_mobile_meram_pm_get_sync,
 };
 
 /*
@@ -515,6 +538,8 @@  static int __devinit sh_mobile_meram_probe(struct platform_device *pdev)
 	if (pdata->addr_mode == SH_MOBILE_MERAM_MODE1)
 		meram_write_reg(priv->base, MEVCR1, 1 << 29);
 
+	pm_runtime_enable(&pdev->dev);
+
 	dev_info(&pdev->dev, "sh_mobile_meram initialized.");
 
 	return 0;
@@ -530,6 +555,8 @@  static int sh_mobile_meram_remove(struct platform_device *pdev)
 {
 	struct sh_mobile_meram_priv *priv = platform_get_drvdata(pdev);
 
+	pm_runtime_disable(&pdev->dev);
+
 	if (priv->base)
 		iounmap(priv->base);
 
diff --git a/include/video/sh_mobile_meram.h b/include/video/sh_mobile_meram.h
index af602d6..f213b6d 100644
--- a/include/video/sh_mobile_meram.h
+++ b/include/video/sh_mobile_meram.h
@@ -63,6 +63,12 @@  struct sh_mobile_meram_ops {
 			    unsigned long base_addr_c,
 			    unsigned long *icb_addr_y,
 			    unsigned long *icb_addr_c);
+
+	/* enable meram clock */
+	int (*meram_pm_get_sync)(struct sh_mobile_meram_info *meram_dev);
+
+	/* disable meram clock */
+	int (*meram_pm_put_sync)(struct sh_mobile_meram_info *meram_dev);
 };
 
 #endif /* __VIDEO_SH_MOBILE_MERAM_H__  */