Hi Janusz On Fri, 9 Sep 2011, Janusz Krzysztofik wrote: > On Thu, 8 Sep 2011 at 10:44:01 Guennadi Liakhovetski wrote: > > From: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > > > Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> > > [g.liakhovetski@xxxxxx: simplified pointer arithmetic] > > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx> > > Hi, > I've successfully tested this one for you, to the extent possible using > mplayer, i.e., only saturation, hue and brightness controls, and an > overall functionality. Thanks for testing and reviewing! > Modifications to other (not runtime tested) controls look good to me, > except for one copy-paste mistake, see below. With that erratic REG_BLUE > corrected: > > Acked-by: Janusz Krzysztofik <jkrzyszt@xxxxxxxxxxxx> > > There are also a few minor comments for you to consider. Well, some of them are not so minor, I would say;-) But I personally would be happy to have this just as an incremental patch. Would you like to prepare one or should I do it? I basically agree with all your comments apart from maybe [snip] > > @@ -1176,9 +1021,11 @@ static int ov6650_probe(struct i2c_client *client, > > priv->colorspace = V4L2_COLORSPACE_JPEG; > > > > ret = ov6650_video_probe(icd, client); > > + if (!ret) > > + ret = v4l2_ctrl_handler_setup(&priv->hdl); > > Are you sure the probe function should fail if v4l2_ctrl_handler_setup() > fails? Its usage is documented as optional. Not sure what the standard really meant, but it looks like this is done in all patches in this series. So, we'd have to change this everywhere. Most other drivers indeed do not care. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
