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@xxxxxxxxxxxxxxx>
> ---
> 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;
