Re: [PATCH v4 19/29] media: mc: Separate single link validation into a new function

Laurent Pinchart
2026-04-16T16:35:44Z

Hi Sakari,

Thank you for the patch.

On Wed, Apr 08, 2026 at 06:39:28PM +0300, Sakari Ailus wrote:
> Add a new function __media_pipeline_validate_one() to validate a single
> link in a pipeline. This will soon be used for performing validation in
> multiple phases.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Michael Riesch <michael.riesch@collabora.com>
> ---
> drivers/media/mc/mc-entity.c | 74 ++++++++++++++++++++----------------
> 1 file changed, 42 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 6bf4730b89d2..717569bd1a8c 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -768,6 +768,45 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
> return ret;
> }
>
> +static int
> +__media_pipeline_validate_one(struct media_pad *origin,
> + struct media_pad *pad, struct media_link *link,
> + bool *has_enabled_link)

I'd drop the __ prefix unless you plan to add a
media_pipeline_validate_one().

As I think 22/29 should be postpone to a series that introduces its
first user, I'd postpone this patch too.

> +{
> + struct media_device *mdev = origin->graph_obj.mdev;
> + struct media_entity *entity = pad->entity;
> + int ret;
> +
> + /* Return here if the link is disabled. */
> + if (!(link->flags & MEDIA_LNK_FL_ENABLED))
> + return 0;
> +
> + if (has_enabled_link)
> + *has_enabled_link = true;
> +
> + /* Skip validation if the current pad isn't the sink pad of the link. */
> + if (link->sink != pad)
> + return 0;
> +
> + if (!entity->ops || !entity->ops->link_validate)
> + return 0;
> +
> + ret = entity->ops->link_validate(link);
> + if (ret) {
> + dev_dbg(mdev->dev,
> + "Link '%s':%u -> '%s':%u failed validation: %d\n",
> + link->source->entity->name, link->source->index,
> + link->sink->entity->name, link->sink->index, ret);
> + return ret;
> + }
> +
> + dev_dbg(mdev->dev, "Link '%s':%u -> '%s':%u is valid\n",
> + link->source->entity->name, link->source->index,
> + link->sink->entity->name, link->sink->index);
> +
> + return 0;
> +}
> +
> __must_check int __media_pipeline_start(struct media_pad *origin,
> struct media_pipeline *pipe)
> {
> @@ -838,39 +877,10 @@ __must_check int __media_pipeline_start(struct media_pad *origin,
> if (link->sink != pad && link->source != pad)
> continue;
>
> - /*
> - * Ensure the link is enabled and if so, record
> - * it. Proceed to the next link if the current pad isn't
> - * the sink pad of the link.
> - */
> - if (!(link->flags & MEDIA_LNK_FL_ENABLED))
> - continue;
> -
> - has_enabled_link = true;
> -
> - if (link->sink != pad)
> - continue;
> -
> - if (!entity->ops || !entity->ops->link_validate)
> - continue;
> -
> - ret = entity->ops->link_validate(link);
> - if (ret) {
> - dev_dbg(mdev->dev,
> - "Link '%s':%u -> '%s':%u failed validation: %d\n",
> - link->source->entity->name,
> - link->source->index,
> - link->sink->entity->name,
> - link->sink->index, ret);
> + ret = __media_pipeline_validate_one(origin, pad, link,
> + &has_enabled_link);
> + if (ret)
> goto error;
> - }
> -
> - dev_dbg(mdev->dev,
> - "Link '%s':%u -> '%s':%u is valid\n",
> - link->source->entity->name,
> - link->source->index,
> - link->sink->entity->name,
> - link->sink->index);
> }
>
> /*

--
Regards,

Laurent Pinchart