On Mon, 15 Jun 2015, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verk...@cisco.com>
>
> When the standard changes the VACTIVE and VDELAY values need to be updated.
>
> Signed-off-by: Hans Verkuil <hans.verk...@cisco.com>
> ---
> drivers/media/i2c/soc_camera/tw9910.c | 29 ++++++++++++++++++++++++++++-
> 1 file changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/soc_camera/tw9910.c
> b/drivers/media/i2c/soc_camera/tw9910.c
> index df66417..e939c24 100644
> --- a/drivers/media/i2c/soc_camera/tw9910.c
> +++ b/drivers/media/i2c/soc_camera/tw9910.c
> @@ -510,13 +510,39 @@ static int tw9910_s_std(struct v4l2_subdev *sd,
> v4l2_std_id norm)
> {
> struct i2c_client *client = v4l2_get_subdevdata(sd);
> struct tw9910_priv *priv = to_tw9910(client);
> + const unsigned hact = 720;
> + const unsigned hdelay = 15;
> + unsigned vact;
> + unsigned vdelay;
> + int ret;
>
> if (!(norm & (V4L2_STD_NTSC | V4L2_STD_PAL)))
> return -EINVAL;
>
> priv->norm = norm;
> + if (norm & V4L2_STD_525_60) {
> + vact = 240;
> + vdelay = 18;
> + ret = tw9910_mask_set(client, VVBI, 0x10, 0x10);
> + } else {
> + vact = 288;
> + vdelay = 24;
> + ret = tw9910_mask_set(client, VVBI, 0x10, 0x00);
> + }
> + if (!ret)
> + ret = i2c_smbus_write_byte_data(client, CROP_HI,
> + ((vdelay >> 2) & 0xc0) |
> + ((vact >> 4) & 0x30) |
> + ((hdelay >> 6) & 0x0c) |
> + ((hact >> 8) & 0x03));
I personally would find ((x & 0xc0) >> {2,4,6,8}) a bit easier for the
eyes, but this works as well for me:)
> + if (!ret)
> + ret = i2c_smbus_write_byte_data(client, VDELAY_LO,
> + vdelay & 0xff);
> + if (!ret)
> + ret = i2c_smbus_write_byte_data(client, VACTIVE_LO,
> + vact & 0xff);
>
> - return 0;
> + return ret;
> }
>
> #ifdef CONFIG_VIDEO_ADV_DEBUG
> @@ -820,6 +846,7 @@ static int tw9910_video_probe(struct i2c_client *client)
> "tw9910 Product ID %0x:%0x\n", id, priv->revision);
>
> priv->norm = V4L2_STD_NTSC;
> + priv->scale = &tw9910_ntsc_scales[0];
Why do you need this? So far everywhere in the code priv->scale is either
checked or set before use. Don't see why an additional initialisation is
needed.
Thanks
Guennadi
>
> done:
> tw9910_s_power(&priv->subdev, 0);
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
