Re: [PATCH 5/6] fbdev: sh_mobile_meram: Add clock enable/disble hooks

[Magnus Damm <magnus.damm@gmail.com>]

On Wed, May 18, 2011 at 8:10 PM, Damian Hobson-Garcia
<dhobsong@igel.co.jp> wrote:
> Signed-off-by: Damian Hobson-Garcia <dhobsong@igel.co.jp>
> ---
>  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..5e4ce98 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;
>  }
>
> +int sh_mobile_meram_clk_on(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(&pdata->pdev->dev);

This should be pm_runtime_get_sync() to force block until the clock is enabled.

> diff --git a/include/video/sh_mobile_meram.h b/include/video/sh_mobile_meram.h
> index af602d6..3605874 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_clk_on)(struct sh_mobile_meram_info *meram_dev);
> +
> +       /* disable meram clock */
> +       int (*meram_clk_off)(struct sh_mobile_meram_info *meram_dev);

Hm, we need more than just clock control. Runtime PM is used for both
clock and power domain control. This means that after pm_runtime_put()
the power to the MERAM may be turned off. So you probably want to add
some context save/restore code to make sure the MERAM settings are
re-initialized after power-up. At this point there are no patches
upstream for this, but I think we should give it a try after
2.6.40-rc1 or rc2 is out.

Thanks,

/ magnus