[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7559062.XA9lTlmQ7K@avalon>
Date: Sun, 06 Dec 2015 04:51:04 +0200
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Javier Martinez Canillas <javier@....samsung.com>
Cc: linux-kernel@...r.kernel.org,
Hans Verkuil <hans.verkuil@...co.com>,
Mauro Carvalho Chehab <mchehab@....samsung.com>,
linux-sh@...r.kernel.org, linux-media@...r.kernel.org
Subject: Re: [PATCH 3/5] [media] v4l: vsp1: separate links creation from entities init
Hi Javier,
Thank you for the patch.
On Thursday 03 September 2015 18:00:34 Javier Martinez Canillas wrote:
> The vsp1 driver initializes the entities and creates the pads links
> before the entities are registered with the media device. This doesn't
> work now that object IDs are used to create links so the media_device
> has to be set.
>
> Split out the pads links creation from the entity initialization so are
> made after the entities registration.
>
> Signed-off-by: Javier Martinez Canillas <javier@....samsung.com>
> ---
>
> drivers/media/platform/vsp1/vsp1_drv.c | 14 ++++++++++--
> drivers/media/platform/vsp1/vsp1_rpf.c | 29 ++++++++++++++++--------
> drivers/media/platform/vsp1/vsp1_rwpf.h | 5 +++++
> drivers/media/platform/vsp1/vsp1_wpf.c | 40 +++++++++++++++++-------------
> 4 files changed, 62 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drv.c
> b/drivers/media/platform/vsp1/vsp1_drv.c index 2aa427d3ff39..8f995d267646
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_drv.c
> +++ b/drivers/media/platform/vsp1/vsp1_drv.c
> @@ -260,9 +260,19 @@ static int vsp1_create_entities(struct vsp1_device
> *vsp1)
>
> /* Create links. */
> list_for_each_entry(entity, &vsp1->entities, list_dev) {
> - if (entity->type == VSP1_ENTITY_LIF ||
> - entity->type == VSP1_ENTITY_RPF)
> + if (entity->type == VSP1_ENTITY_LIF) {
> + ret = vsp1_wpf_create_pads_links(vsp1, entity);
Could you please s/pads_links/links/ ? There's no other type of links handled
by the driver.
> + if (ret < 0)
> + goto done;
> + continue;
I would use
} else if (...) {
instead of a continue.
> + }
> +
> + if (entity->type == VSP1_ENTITY_RPF) {
> + ret = vsp1_rpf_create_pads_links(vsp1, entity);
> + if (ret < 0)
> + goto done;
> continue;
Same here.
Apart from that,
Acked-by: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> + }
>
> ret = vsp1_create_links(vsp1, entity);
> if (ret < 0)
> diff --git a/drivers/media/platform/vsp1/vsp1_rpf.c
> b/drivers/media/platform/vsp1/vsp1_rpf.c index b60a528a8fe8..38aebdf691b5
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_rpf.c
> @@ -277,18 +277,29 @@ struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device
> *vsp1, unsigned int index)
>
> rpf->entity.video = video;
>
> - /* Connect the video device to the RPF. */
> - ret = media_create_pad_link(&rpf->video.video.entity, 0,
> - &rpf->entity.subdev.entity,
> - RWPF_PAD_SINK,
> - MEDIA_LNK_FL_ENABLED |
> - MEDIA_LNK_FL_IMMUTABLE);
> - if (ret < 0)
> - goto error;
> -
> return rpf;
>
> error:
> vsp1_entity_destroy(&rpf->entity);
> return ERR_PTR(ret);
> }
> +
> +/*
> + * vsp1_rpf_create_pads_links_create_pads_links() - RPF pads links creation
> + * @vsp1: Pointer to VSP1 device
> + * @entity: Pointer to VSP1 entity
> + *
> + * return negative error code or zero on success
> + */
> +int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
> + struct vsp1_entity *entity)
> +{
> + struct vsp1_rwpf *rpf = to_rwpf(&entity->subdev);
> +
> + /* Connect the video device to the RPF. */
> + return media_create_pad_link(&rpf->video.video.entity, 0,
> + &rpf->entity.subdev.entity,
> + RWPF_PAD_SINK,
> + MEDIA_LNK_FL_ENABLED |
> + MEDIA_LNK_FL_IMMUTABLE);
> +}
> diff --git a/drivers/media/platform/vsp1/vsp1_rwpf.h
> b/drivers/media/platform/vsp1/vsp1_rwpf.h index f452dce1a931..6638b3587369
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_rwpf.h
> +++ b/drivers/media/platform/vsp1/vsp1_rwpf.h
> @@ -50,6 +50,11 @@ static inline struct vsp1_rwpf *to_rwpf(struct
> v4l2_subdev *subdev) struct vsp1_rwpf *vsp1_rpf_create(struct vsp1_device
> *vsp1, unsigned int index); struct vsp1_rwpf *vsp1_wpf_create(struct
> vsp1_device *vsp1, unsigned int index);
>
> +int vsp1_rpf_create_pads_links(struct vsp1_device *vsp1,
> + struct vsp1_entity *entity);
> +int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
> + struct vsp1_entity *entity);
> +
> int vsp1_rwpf_enum_mbus_code(struct v4l2_subdev *subdev,
> struct v4l2_subdev_pad_config *cfg,
> struct v4l2_subdev_mbus_code_enum *code);
> diff --git a/drivers/media/platform/vsp1/vsp1_wpf.c
> b/drivers/media/platform/vsp1/vsp1_wpf.c index d39aa4b8aea1..1be363e4f741
> 100644
> --- a/drivers/media/platform/vsp1/vsp1_wpf.c
> +++ b/drivers/media/platform/vsp1/vsp1_wpf.c
> @@ -220,7 +220,6 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device
> *vsp1, unsigned int index) struct v4l2_subdev *subdev;
> struct vsp1_video *video;
> struct vsp1_rwpf *wpf;
> - unsigned int flags;
> int ret;
>
> wpf = devm_kzalloc(vsp1->dev, sizeof(*wpf), GFP_KERNEL);
> @@ -276,20 +275,6 @@ struct vsp1_rwpf *vsp1_wpf_create(struct vsp1_device
> *vsp1, unsigned int index) goto error;
>
> wpf->entity.video = video;
> -
> - /* Connect the video device to the WPF. All connections are immutable
> - * except for the WPF0 source link if a LIF is present.
> - */
> - flags = MEDIA_LNK_FL_ENABLED;
> - if (!(vsp1->pdata.features & VSP1_HAS_LIF) || index != 0)
> - flags |= MEDIA_LNK_FL_IMMUTABLE;
> -
> - ret = media_create_pad_link(&wpf->entity.subdev.entity,
> - RWPF_PAD_SOURCE,
> - &wpf->video.video.entity, 0, flags);
> - if (ret < 0)
> - goto error;
> -
> wpf->entity.sink = &wpf->video.video.entity;
>
> return wpf;
> @@ -298,3 +283,28 @@ error:
> vsp1_entity_destroy(&wpf->entity);
> return ERR_PTR(ret);
> }
> +
> +/*
> + * vsp1_wpf_create_pads_links_create_pads_links() - RPF pads links creation
> + * @vsp1: Pointer to VSP1 device
> + * @entity: Pointer to VSP1 entity
> + *
> + * return negative error code or zero on success
> + */
> +int vsp1_wpf_create_pads_links(struct vsp1_device *vsp1,
> + struct vsp1_entity *entity)
> +{
> + struct vsp1_rwpf *wpf = to_rwpf(&entity->subdev);
> + unsigned int flags;
> +
> + /* Connect the video device to the WPF. All connections are immutable
> + * except for the WPF0 source link if a LIF is present.
> + */
> + flags = MEDIA_LNK_FL_ENABLED;
> + if (!(vsp1->pdata.features & VSP1_HAS_LIF) || entity->index != 0)
> + flags |= MEDIA_LNK_FL_IMMUTABLE;
> +
> + return media_create_pad_link(&wpf->entity.subdev.entity,
> + RWPF_PAD_SOURCE,
> + &wpf->video.video.entity, 0, flags);
> +}
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists