| Message ID | 1426858247-25746-1-git-send-email-ricardo.ribalda@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Ricardo, Thank you for the patch, and sorry for the late review (so late that the patch has already been merged). On Friday 20 March 2015 14:30:46 Ricardo Ribalda Delgado wrote: > Volatile controls should not generate CH_VALUE events. What's the rationale for that ? I would actually expect the value change events to be more useful for volatile controls than non-volatile controls. Volatile controls can have their value changed by the hardware without software intervention, and it makes sense to me to report that to userspace. > Set has_changed to false to prevent this happening. > > Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> > --- > v2: By Sakari Ailus <sakari.ailus@linux.intel.com> > > Fix CodeStyle (sorry :S) > drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c > b/drivers/media/v4l2-core/v4l2-ctrls.c index 45c5b47..2ebc33e 100644 > --- a/drivers/media/v4l2-core/v4l2-ctrls.c > +++ b/drivers/media/v4l2-core/v4l2-ctrls.c > @@ -1609,6 +1609,15 @@ static int cluster_changed(struct v4l2_ctrl *master) > > if (ctrl == NULL) > continue; > + /* > + * Set has_changed to false to avoid generating > + * the event V4L2_EVENT_CTRL_CH_VALUE > + */ > + if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) { > + ctrl->has_changed = false; > + continue; > + } > + > for (idx = 0; !ctrl_changed && idx < ctrl->elems; idx++) > ctrl_changed = !ctrl->type_ops->equal(ctrl, idx, > ctrl->p_cur, ctrl->p_new);
Hello Laurent On Tue, Apr 21, 2015 at 7:44 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > Hi Ricardo, > > Thank you for the patch, and sorry for the late review (so late that the patch > has already been merged). No worries. > > On Friday 20 March 2015 14:30:46 Ricardo Ribalda Delgado wrote: >> Volatile controls should not generate CH_VALUE events. > > What's the rationale for that ? I would actually expect the value change > events to be more useful for volatile controls than non-volatile controls. > Volatile controls can have their value changed by the hardware without > software intervention, and it makes sense to me to report that to userspace. Imagine a temperature register on the sensor. It is changing constantly, resolution 10 milidegrees: Do you want to get an event for every change? Who will poll the temperature? The driver? The hardware will irq the driver....? So I guess the less wrong solution is not throwing the ch_value event. This is just my two cents, probably Hans has a much better global view :) Regards. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 45c5b47..2ebc33e 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1609,6 +1609,15 @@ static int cluster_changed(struct v4l2_ctrl *master) if (ctrl == NULL) continue; + /* + * Set has_changed to false to avoid generating + * the event V4L2_EVENT_CTRL_CH_VALUE + */ + if (ctrl->flags & V4L2_CTRL_FLAG_VOLATILE) { + ctrl->has_changed = false; + continue; + } + for (idx = 0; !ctrl_changed && idx < ctrl->elems; idx++) ctrl_changed = !ctrl->type_ops->equal(ctrl, idx, ctrl->p_cur, ctrl->p_new);
Volatile controls should not generate CH_VALUE events. Set has_changed to false to prevent this happening. Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com> --- v2: By Sakari Ailus <sakari.ailus@linux.intel.com> Fix CodeStyle (sorry :S) drivers/media/v4l2-core/v4l2-ctrls.c | 9 +++++++++ 1 file changed, 9 insertions(+)