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 <jav...@osg.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.pinch...@ideasonboard.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-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to