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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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