Re: [PATCH] v4l: vsp1: Move subdev operations from HGO to common histogram code

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Niklas,

On Tuesday 06 Sep 2016 12:28:00 Niklas Söderlund wrote:
> On 2016-09-05 18:13:39 +0300, Laurent Pinchart wrote:
> > The code will be shared with the HGT entity, move it to the generic
> > histogram implementation.
> > 
> > Signed-off-by: Laurent Pinchart
> > <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_drv.c   |   7 +-
> >  drivers/media/platform/vsp1/vsp1_hgo.c   | 308 ++-----------------------
> >  drivers/media/platform/vsp1/vsp1_hgo.h   |   7 +-
> >  drivers/media/platform/vsp1/vsp1_histo.c | 334 +++++++++++++++++++++++--
> >  drivers/media/platform/vsp1/vsp1_histo.h |  25 ++-
> >  5 files changed, 355 insertions(+), 326 deletions(-)

[snip]

> > @@ -268,7 +559,16 @@ int vsp1_histogram_init(struct vsp1_device *vsp1,
> > struct vsp1_histogram *histo,> 
> >  	INIT_LIST_HEAD(&histo->irqqueue);
> >  	init_waitqueue_head(&histo->wait_queue);
> > 
> > -	/* Initialize the media entity... */
> > +	/* Initialize the VSP entity... */
> > +	histo->entity.ops = ops;
> > +	histo->entity.type = type;
> > +
> > +	ret = vsp1_entity_init(vsp1, &histo->entity, name, 2, &histo_ops,
> > +			       MEDIA_ENT_F_PROC_VIDEO_STATISTICS);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	/* ... and the media entity... */
> >  	ret = media_entity_pads_init(&histo->video.entity, 1, &histo->pad);
> >  	if (ret < 0)
> >  		return ret;
> 
> You forgot to update the histo video device name to match the subdevice
> name here. Something like this makes vsp-tests work for me again.
> 
> @@ -577,7 +577,7 @@ int vsp1_histogram_init(struct vsp1_device *vsp1, struct
> vsp1_histogram *histo, histo->video.v4l2_dev = &vsp1->v4l2_dev;
>         histo->video.fops = &histo_v4l2_fops;
>         snprintf(histo->video.name, sizeof(histo->video.name),
> -                "%s histo", name);
> +                "%s histo", histo->entity.subdev.name);
>         histo->video.vfl_type = VFL_TYPE_GRABBER;
>         histo->video.release = video_device_release_empty;
>         histo->video.ioctl_ops = &histo_v4l2_ioctl_ops;
> 
> Without this fix the names listed using media-ctl -p show:
> 
> - entity 1: hgo histo (1 pad, 1 link)
> 
> Instead as it did before:
> 
> - entity 1: fe928000.vsp1 hgo histo (1 pad, 1 link)
> 
> Other then that
> 
> Tested-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

Thank you. I'll fix that in the next version.

-- 
Regards,

Laurent Pinchart