On 06/21/2015 07:23 PM, Guennadi Liakhovetski wrote: > On Mon, 15 Jun 2015, Hans Verkuil wrote: > >> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> >> >> When the standard changes the VACTIVE and VDELAY values need to be updated. >> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> >> --- >> 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. If you just start streaming without explicitly setting up formats (which is allowed), then priv->scale is still NULL. V4L2 always assumes that there is some initial format configured, and this line enables that for this driver (NTSC). Regards, Hans > > Thanks > Guennadi > >> >> done: >> tw9910_s_power(&priv->subdev, 0); >> -- >> 2.1.4 >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in