diff mbox series

[2/5] input/rmi4/rmi_f54: fix various V4L2 compliance problems

Message ID 20191119105118.54285-3-hverkuil-cisco@xs4all.nl (mailing list archive)
State New, archived
Headers show
Series input: rmi4/synaptics fixes | expand

Commit Message

Hans Verkuil Nov. 19, 2019, 10:51 a.m. UTC
The v4l2-compliance utility reported several V4L2 API compliance
issues:

- the sequence counter wasn't filled in
- the sequence counter wasn't reset to 0 at the start of streaming
- the returned field value wasn't set to V4L2_FIELD_NONE
- the timestamp wasn't set
- the payload size was undefined if an error was returned
- min_buffers_needed doesn't need to be initialized

Fix these issues.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/input/rmi4/rmi_f54.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Lucas Stach Nov. 19, 2019, 11:42 a.m. UTC | #1
On Di, 2019-11-19 at 11:51 +0100, Hans Verkuil wrote:
> The v4l2-compliance utility reported several V4L2 API compliance
> issues:
> 
> - the sequence counter wasn't filled in
> - the sequence counter wasn't reset to 0 at the start of streaming
> - the returned field value wasn't set to V4L2_FIELD_NONE
> - the timestamp wasn't set
> - the payload size was undefined if an error was returned
> - min_buffers_needed doesn't need to be initialized
> 
> Fix these issues.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de

> ---
>  drivers/input/rmi4/rmi_f54.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/input/rmi4/rmi_f54.c
> b/drivers/input/rmi4/rmi_f54.c
> index 710b02595486..ebccab7a4834 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -116,6 +116,7 @@ struct f54_data {
>  	struct video_device vdev;
>  	struct vb2_queue queue;
>  	struct mutex lock;
> +	u32 sequence;
>  	int input;
>  	enum rmi_f54_report_type inputs[F54_MAX_REPORT_TYPE];
>  };
> @@ -290,6 +291,7 @@ static int rmi_f54_queue_setup(struct vb2_queue
> *q, unsigned int *nbuffers,
>  
>  static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
>  {
> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>  	struct f54_data *f54 = vb2_get_drv_priv(vb->vb2_queue);
>  	u16 *ptr;
>  	enum vb2_buffer_state state;
> @@ -298,6 +300,7 @@ static void rmi_f54_buffer_queue(struct
> vb2_buffer *vb)
>  
>  	mutex_lock(&f54->status_mutex);
>  
> +	vb2_set_plane_payload(vb, 0, 0);
>  	reptype = rmi_f54_get_reptype(f54, f54->input);
>  	if (reptype == F54_REPORT_NONE) {
>  		state = VB2_BUF_STATE_ERROR;
> @@ -344,14 +347,25 @@ static void rmi_f54_buffer_queue(struct
> vb2_buffer *vb)
>  data_done:
>  	mutex_unlock(&f54->data_mutex);
>  done:
> +	vb->timestamp = ktime_get_ns();
> +	vbuf->field = V4L2_FIELD_NONE;
> +	vbuf->sequence = f54->sequence++;
>  	vb2_buffer_done(vb, state);
>  	mutex_unlock(&f54->status_mutex);
>  }
>  
> +static void rmi_f54_stop_streaming(struct vb2_queue *q)
> +{
> +	struct f54_data *f54 = vb2_get_drv_priv(q);
> +
> +	f54->sequence = 0;
> +}
> +
>  /* V4L2 structures */
>  static const struct vb2_ops rmi_f54_queue_ops = {
>  	.queue_setup            = rmi_f54_queue_setup,
>  	.buf_queue              = rmi_f54_buffer_queue,
> +	.stop_streaming		= rmi_f54_stop_streaming,
>  	.wait_prepare           = vb2_ops_wait_prepare,
>  	.wait_finish            = vb2_ops_wait_finish,
>  };
> @@ -363,7 +377,6 @@ static const struct vb2_queue rmi_f54_queue = {
>  	.ops = &rmi_f54_queue_ops,
>  	.mem_ops = &vb2_vmalloc_memops,
>  	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
> -	.min_buffers_needed = 1,
>  };
>  
>  static int rmi_f54_vidioc_querycap(struct file *file, void *priv,
Dmitry Torokhov Nov. 23, 2019, 12:19 a.m. UTC | #2
On Tue, Nov 19, 2019 at 12:42:46PM +0100, Lucas Stach wrote:
> On Di, 2019-11-19 at 11:51 +0100, Hans Verkuil wrote:
> > The v4l2-compliance utility reported several V4L2 API compliance
> > issues:
> > 
> > - the sequence counter wasn't filled in
> > - the sequence counter wasn't reset to 0 at the start of streaming
> > - the returned field value wasn't set to V4L2_FIELD_NONE
> > - the timestamp wasn't set
> > - the payload size was undefined if an error was returned
> > - min_buffers_needed doesn't need to be initialized
> > 
> > Fix these issues.
> > 
> > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> 
> Reviewed-by: Lucas Stach <l.stach@pengutronix.de

Applied, thank you.

> 
> > ---
> >  drivers/input/rmi4/rmi_f54.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/input/rmi4/rmi_f54.c
> > b/drivers/input/rmi4/rmi_f54.c
> > index 710b02595486..ebccab7a4834 100644
> > --- a/drivers/input/rmi4/rmi_f54.c
> > +++ b/drivers/input/rmi4/rmi_f54.c
> > @@ -116,6 +116,7 @@ struct f54_data {
> >  	struct video_device vdev;
> >  	struct vb2_queue queue;
> >  	struct mutex lock;
> > +	u32 sequence;
> >  	int input;
> >  	enum rmi_f54_report_type inputs[F54_MAX_REPORT_TYPE];
> >  };
> > @@ -290,6 +291,7 @@ static int rmi_f54_queue_setup(struct vb2_queue
> > *q, unsigned int *nbuffers,
> >  
> >  static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
> >  {
> > +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> >  	struct f54_data *f54 = vb2_get_drv_priv(vb->vb2_queue);
> >  	u16 *ptr;
> >  	enum vb2_buffer_state state;
> > @@ -298,6 +300,7 @@ static void rmi_f54_buffer_queue(struct
> > vb2_buffer *vb)
> >  
> >  	mutex_lock(&f54->status_mutex);
> >  
> > +	vb2_set_plane_payload(vb, 0, 0);
> >  	reptype = rmi_f54_get_reptype(f54, f54->input);
> >  	if (reptype == F54_REPORT_NONE) {
> >  		state = VB2_BUF_STATE_ERROR;
> > @@ -344,14 +347,25 @@ static void rmi_f54_buffer_queue(struct
> > vb2_buffer *vb)
> >  data_done:
> >  	mutex_unlock(&f54->data_mutex);
> >  done:
> > +	vb->timestamp = ktime_get_ns();
> > +	vbuf->field = V4L2_FIELD_NONE;
> > +	vbuf->sequence = f54->sequence++;
> >  	vb2_buffer_done(vb, state);
> >  	mutex_unlock(&f54->status_mutex);
> >  }
> >  
> > +static void rmi_f54_stop_streaming(struct vb2_queue *q)
> > +{
> > +	struct f54_data *f54 = vb2_get_drv_priv(q);
> > +
> > +	f54->sequence = 0;
> > +}
> > +
> >  /* V4L2 structures */
> >  static const struct vb2_ops rmi_f54_queue_ops = {
> >  	.queue_setup            = rmi_f54_queue_setup,
> >  	.buf_queue              = rmi_f54_buffer_queue,
> > +	.stop_streaming		= rmi_f54_stop_streaming,
> >  	.wait_prepare           = vb2_ops_wait_prepare,
> >  	.wait_finish            = vb2_ops_wait_finish,
> >  };
> > @@ -363,7 +377,6 @@ static const struct vb2_queue rmi_f54_queue = {
> >  	.ops = &rmi_f54_queue_ops,
> >  	.mem_ops = &vb2_vmalloc_memops,
> >  	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
> > -	.min_buffers_needed = 1,
> >  };
> >  
> >  static int rmi_f54_vidioc_querycap(struct file *file, void *priv,
>
diff mbox series

Patch

diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
index 710b02595486..ebccab7a4834 100644
--- a/drivers/input/rmi4/rmi_f54.c
+++ b/drivers/input/rmi4/rmi_f54.c
@@ -116,6 +116,7 @@  struct f54_data {
 	struct video_device vdev;
 	struct vb2_queue queue;
 	struct mutex lock;
+	u32 sequence;
 	int input;
 	enum rmi_f54_report_type inputs[F54_MAX_REPORT_TYPE];
 };
@@ -290,6 +291,7 @@  static int rmi_f54_queue_setup(struct vb2_queue *q, unsigned int *nbuffers,
 
 static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
 {
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
 	struct f54_data *f54 = vb2_get_drv_priv(vb->vb2_queue);
 	u16 *ptr;
 	enum vb2_buffer_state state;
@@ -298,6 +300,7 @@  static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
 
 	mutex_lock(&f54->status_mutex);
 
+	vb2_set_plane_payload(vb, 0, 0);
 	reptype = rmi_f54_get_reptype(f54, f54->input);
 	if (reptype == F54_REPORT_NONE) {
 		state = VB2_BUF_STATE_ERROR;
@@ -344,14 +347,25 @@  static void rmi_f54_buffer_queue(struct vb2_buffer *vb)
 data_done:
 	mutex_unlock(&f54->data_mutex);
 done:
+	vb->timestamp = ktime_get_ns();
+	vbuf->field = V4L2_FIELD_NONE;
+	vbuf->sequence = f54->sequence++;
 	vb2_buffer_done(vb, state);
 	mutex_unlock(&f54->status_mutex);
 }
 
+static void rmi_f54_stop_streaming(struct vb2_queue *q)
+{
+	struct f54_data *f54 = vb2_get_drv_priv(q);
+
+	f54->sequence = 0;
+}
+
 /* V4L2 structures */
 static const struct vb2_ops rmi_f54_queue_ops = {
 	.queue_setup            = rmi_f54_queue_setup,
 	.buf_queue              = rmi_f54_buffer_queue,
+	.stop_streaming		= rmi_f54_stop_streaming,
 	.wait_prepare           = vb2_ops_wait_prepare,
 	.wait_finish            = vb2_ops_wait_finish,
 };
@@ -363,7 +377,6 @@  static const struct vb2_queue rmi_f54_queue = {
 	.ops = &rmi_f54_queue_ops,
 	.mem_ops = &vb2_vmalloc_memops,
 	.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC,
-	.min_buffers_needed = 1,
 };
 
 static int rmi_f54_vidioc_querycap(struct file *file, void *priv,