Re: [PATCH 5/5 v2] [media] ov9740: Add suspend/resume

Guennadi Liakhovetski
2011-05-29T10:58:33Z

On Wed, 25 May 2011, achew@nvidia.com wrote:

> From: Andrew Chew <achew@nvidia.com>
>
> On suspend, remember whether we are streaming or not, and at what frame format,
> so that on resume, we can start streaming again.
>
> Signed-off-by: Andrew Chew <achew@nvidia.com>
> ---
> drivers/media/video/ov9740.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/ov9740.c b/drivers/media/video/ov9740.c
> index 6c28ae8..4abe943 100644
> --- a/drivers/media/video/ov9740.c
> +++ b/drivers/media/video/ov9740.c
> @@ -201,6 +201,10 @@ struct ov9740_priv {
>
> bool flag_vflip;
> bool flag_hflip;
> +
> + /* For suspend/resume. */
> + struct v4l2_mbus_framefmt current_mf;
> + int current_enable;

bool?

> };
>
> static const struct ov9740_reg ov9740_defaults[] = {
> @@ -551,6 +555,8 @@ static int ov9740_s_stream(struct v4l2_subdev *sd, int enable)
> 0x00);
> }
>
> + priv->current_enable = enable;
> +
> return ret;
> }
>
> @@ -786,6 +792,7 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
> struct v4l2_mbus_framefmt *mf)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> + struct ov9740_priv *priv = to_ov9740(sd);
> enum v4l2_colorspace cspace;
> enum v4l2_mbus_pixelcode code = mf->code;
> int ret;
> @@ -812,6 +819,8 @@ static int ov9740_s_fmt(struct v4l2_subdev *sd,
> mf->code = code;
> mf->colorspace = cspace;
>
> + memcpy(&priv->current_mf, mf, sizeof(struct v4l2_mbus_framefmt));
> +
> return ret;
> }
>
> @@ -922,7 +931,37 @@ err:
> return ret;
> }
>
> +static int ov9740_suspend(struct soc_camera_device *icd, pm_message_t state)
> +{
> + struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> + struct ov9740_priv *priv = to_ov9740(sd);
> +
> + if (priv->current_enable) {
> + int current_enable = priv->current_enable;
> +
> + ov9740_s_stream(sd, 0);
> + priv->current_enable = current_enable;

You don't need the local variable, just set

priv->current_enable = true;

> + }
> +
> + return 0;
> +}
> +
> +static int ov9740_resume(struct soc_camera_device *icd)
> +{
> + struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
> + struct ov9740_priv *priv = to_ov9740(sd);
> +
> + if (priv->current_enable) {
> + ov9740_s_fmt(sd, &priv->current_mf);
> + ov9740_s_stream(sd, priv->current_enable);
> + }
> +
> + return 0;
> +}
> +
> static struct soc_camera_ops ov9740_ops = {
> + .suspend = ov9740_suspend,
> + .resume = ov9740_resume,

No, we don't want to use these, whey should disappear some time... Please,
use .s_power() from struct v4l2_subdev_core_ops, you can check
http://article.gmane.org/gmane.linux.drivers.video-input-infrastructure/33105
for an example. If your host is not using these ops, it has to be fixed.
So far in the mainline only one soc-camera host driver is using these
callbacks: pxa_camera.c, which, looking at your email address, I doubt is
the driver, that you're using;)

> .set_bus_param = ov9740_set_bus_param,
> .query_bus_param = ov9740_query_bus_param,
> .controls = ov9740_controls,
> --
> 1.7.5.2
>

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer