Re: [PATCHv3 09/10] [media] rcar-vin: rework how subdeivce is found and bound

[Sergei Shtylyov <sergei.shtylyov@xxxxxxxxxxxxxxxxxx>]

Hello.

On 08/15/2016 06:06 PM, Niklas Söderlund wrote:

> The original drivers code to find a subdevice by looking in the DT grpah

   s/grpah/graph/.

> and how the callbacks to the v4l2 async bind framework where poorly
> written. The most obvious example of badness was the duplication of data
> in the struct rvin_graph_entity.
>
> This patch removes the data duplication, simplifies the parsing of the
> DT graph and add checks to the v4l2 callbacks.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
> ---
>  drivers/media/platform/rcar-vin/rcar-core.c | 232 +++++++++++++---------------
>  drivers/media/platform/rcar-vin/rcar-vin.h  |   8 +-
>  2 files changed, 111 insertions(+), 129 deletions(-)
>
> diff --git a/drivers/media/platform/rcar-vin/rcar-core.c b/drivers/media/platform/rcar-vin/rcar-core.c
> index 3941134..39bf6fc 100644
> --- a/drivers/media/platform/rcar-vin/rcar-core.c
> +++ b/drivers/media/platform/rcar-vin/rcar-core.c
[...]
> @@ -94,89 +102,111 @@ static int rvin_digital_notify_bound(struct v4l2_async_notifier *notifier,
>  {
>  	struct rvin_dev *vin = notifier_to_vin(notifier);
>
> -	vin_dbg(vin, "subdev %s bound\n", subdev->name);
> +	v4l2_set_subdev_hostdata(subdev, vin);
>
> -	vin->digital.entity = &subdev->entity;
> -	vin->digital.subdev = subdev;
> +	if (vin->digital.asd.match.of.node == subdev->dev->of_node) {
> +		vin_dbg(vin, "bound digital subdev %s\n", subdev->name);
> +		vin->digital.subdev = subdev;
> +		return 0;
> +	}
>
> -	return 0;
> +	vin_err(vin, "no entity for subdev %s to bind\n", subdev->name);
> +	return -EINVAL;
>  }
>
> -static int rvin_digital_parse(struct rvin_dev *vin,
> -			      struct device_node *node)
> +static int rvin_digitial_parse_v4l2(struct rvin_dev *vin,

   s/digitial/digital/.

[...]
> diff --git a/drivers/media/platform/rcar-vin/rcar-vin.h b/drivers/media/platform/rcar-vin/rcar-vin.h
> index 93daa05..edfe658 100644
> --- a/drivers/media/platform/rcar-vin/rcar-vin.h
> +++ b/drivers/media/platform/rcar-vin/rcar-vin.h
> @@ -70,10 +70,12 @@ struct rvin_video_format {
>  	u8 bpp;
>  };
>
> +/**
> + * struct rvin_graph_entity - Video endpoint from async framework
> + * @asd:	sub-device descriptor for async framework
> + * @subdev:	subdevice matched using async framework
> + */

    Looks like a materia for a separate patch...

>  struct rvin_graph_entity {
> -	struct device_node *node;
> -	struct media_entity *entity;
> -
>  	struct v4l2_async_subdev asd;
>  	struct v4l2_subdev *subdev;
>  };

MBR, Sergei