On Fri, Mar 23, 2018 at 11:08:11PM +0900, Tomasz Figa wrote: > On Fri, Mar 23, 2018 at 10:50 PM, Sakari Ailus > <sakari.ailus@xxxxxxxxxxxxxxx> wrote: > > Hi Tomasz, > > > > On Fri, Mar 23, 2018 at 08:43:50PM +0900, Tomasz Figa wrote: > >> Hi Andy, > >> > >> Some issues found when reviewing cherry pick of this patch to Chrome > >> OS kernel. Please see inline. > >> > >> On Sat, Mar 17, 2018 at 1:38 AM, Andy Yeh <andy.yeh@xxxxxxxxx> wrote: > >> > >> [snip] > >> > >> > + case V4L2_CID_VBLANK: > >> > + /* > >> > + * Auto Frame Length Line Control is enabled by default. > >> > + * Not need control Vblank Register. > >> > + */ > >> > >> What is the meaning of this control then? Should it be read-only? > > > > The read-only flag is for the uAPI; the control framework still passes > > through changes to the control value done using kAPI to the driver. > > The read-only flag is not even set in current code. Ah, you're right, it's just hblank... but if the driver doesn't support setting this control, then it should most likely be read-only. It would seem like that the driver just updates the control to convey the value to the user. > > Also, I'm not sure about the control framework setting read-only > control. According to the code, it doesn't: > https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-ctrls.c#L2477 If you set the control using e.g. v4l2_ctrl_s_ctrl(), it should end up to the driver's s_ctrl callback. -- Regards, Sakari Ailus e-mail: sakari.ailus@xxxxxx
