Re: [RFC 2/7] media: v4l: subdev: Support INTERNAL_SOURCE pads in routing IOCTLs

[Hans Verkuil <hverkuil@xs4all.nl>]

On 05/05/2023 23:52, Sakari Ailus wrote:
> Take the new INTERNAL_SOURCE pad flag into account in validating routing
> IOCTL argument. Effectively this is a SINK pad in this respect. Due to the
> union there's no need to check for the particular name.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/v4l2-core/v4l2-subdev.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 2ec179cd1264..6312fc2bf1f0 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -906,7 +906,8 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  				return -EINVAL;
>  
>  			if (!(pads[route->sink_pad].flags &
> -			      MEDIA_PAD_FL_SINK))
> +			      (MEDIA_PAD_FL_SINK |
> +			       MEDIA_PAD_FL_INTERNAL_SOURCE)))
>  				return -EINVAL;
>  
>  			if (route->source_pad >= sd->entity.num_pads)
> @@ -1787,7 +1788,8 @@ int v4l2_subdev_routing_validate(struct v4l2_subdev *sd,
>  
>  		/* Validate the sink and source pad numbers. */
>  		if (route->sink_pad >= sd->entity.num_pads ||
> -		    !(sd->entity.pads[route->sink_pad].flags & MEDIA_PAD_FL_SINK)) {
> +		    !(sd->entity.pads[route->sink_pad].flags &
> +		      (MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_INTERNAL_SOURCE))) {

So this relates to my reply to the previous patch: calling a sink pad
an INTERNAL_SOURCE is really confusing.

Some suggestions for alternative names:

1) MEDIA_PAD_FL_SINK_INTERNAL
2) MEDIA_PAD_FL_SINK_INTERNAL_SOURCE (or _SRC)
3) MEDIA_PAD_FL_SINK_FROM_INTERNAL_SOURCE (or _SRC)

IMHO the name *has* to start with MEDIA_PAD_FL_SINK_ to indicate
that it is a sink pad, followed by something that indicates that
it is fed from an internal source.

Regards,

	Hans

>  			dev_dbg(sd->dev, "route %u sink (%u) is not a sink pad\n",
>  				i, route->sink_pad);
>  			goto out;