[PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Sakari Ailus]

Rename the pad field in __media_pipeline_start() to both better describe
what it is and avoid masking it during the loop.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
since v1:

- Also change the argument name in the prototype.

 drivers/media/mc/mc-entity.c | 8 ++++----
 include/media/media-entity.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
index 96dd0f6ccd0d..0df9fc90cf33 100644
--- a/drivers/media/mc/mc-entity.c
+++ b/drivers/media/mc/mc-entity.c
@@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
 	return ret;
 }
 
-__must_check int __media_pipeline_start(struct media_pad *pad,
+__must_check int __media_pipeline_start(struct media_pad *origin,
 					struct media_pipeline *pipe)
 {
-	struct media_device *mdev = pad->graph_obj.mdev;
+	struct media_device *mdev = origin->graph_obj.mdev;
 	struct media_pipeline_pad *err_ppad;
 	struct media_pipeline_pad *ppad;
 	int ret;
@@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
 	 * If the pad is already part of a pipeline, that pipeline must be the
 	 * same as the pipe given to media_pipeline_start().
 	 */
-	if (WARN_ON(pad->pipe && pad->pipe != pipe))
+	if (WARN_ON(origin->pipe && origin->pipe != pipe))
 		return -EINVAL;
 
 	/*
@@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
 	 * with media_pipeline_pad instances for each pad found during graph
 	 * walk.
 	 */
-	ret = media_pipeline_populate(pipe, pad);
+	ret = media_pipeline_populate(pipe, origin);
 	if (ret)
 		return ret;
 
diff --git a/include/media/media-entity.h b/include/media/media-entity.h
index 0393b23129eb..2fca4556c311 100644
--- a/include/media/media-entity.h
+++ b/include/media/media-entity.h
@@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
 
 /**
  * media_pipeline_start - Mark a pipeline as streaming
- * @pad: Starting pad
+ * @origin: Starting pad
  * @pipe: Media pipeline to be assigned to all pads in the pipeline.
  *
  * Mark all pads connected to a given pad through enabled links, either
@@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
  * pipeline pointer must be identical for all nested calls to
  * media_pipeline_start().
  */
-__must_check int media_pipeline_start(struct media_pad *pad,
+__must_check int media_pipeline_start(struct media_pad *origin,
 				      struct media_pipeline *pipe);
 /**
  * __media_pipeline_start - Mark a pipeline as streaming
-- 
2.39.5

Re: [PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Laurent Pinchart]

Hi Sakari,

Thank you for the patch.

On Tue, Nov 05, 2024 at 08:40:12AM +0200, Sakari Ailus wrote:
> Rename the pad field in __media_pipeline_start() to both better describe
> what it is and avoid masking it during the loop.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> since v1:
> 
> - Also change the argument name in the prototype.
> 
>  drivers/media/mc/mc-entity.c | 8 ++++----
>  include/media/media-entity.h | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> index 96dd0f6ccd0d..0df9fc90cf33 100644
> --- a/drivers/media/mc/mc-entity.c
> +++ b/drivers/media/mc/mc-entity.c
> @@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
>  	return ret;
>  }
>  
> -__must_check int __media_pipeline_start(struct media_pad *pad,
> +__must_check int __media_pipeline_start(struct media_pad *origin,
>  					struct media_pipeline *pipe)
>  {
> -	struct media_device *mdev = pad->graph_obj.mdev;
> +	struct media_device *mdev = origin->graph_obj.mdev;
>  	struct media_pipeline_pad *err_ppad;
>  	struct media_pipeline_pad *ppad;
>  	int ret;
> @@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
>  	 * If the pad is already part of a pipeline, that pipeline must be the
>  	 * same as the pipe given to media_pipeline_start().
>  	 */
> -	if (WARN_ON(pad->pipe && pad->pipe != pipe))
> +	if (WARN_ON(origin->pipe && origin->pipe != pipe))
>  		return -EINVAL;
>  
>  	/*
> @@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
>  	 * with media_pipeline_pad instances for each pad found during graph
>  	 * walk.
>  	 */
> -	ret = media_pipeline_populate(pipe, pad);
> +	ret = media_pipeline_populate(pipe, origin);
>  	if (ret)
>  		return ret;
>  
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index 0393b23129eb..2fca4556c311 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
>  
>  /**
>   * media_pipeline_start - Mark a pipeline as streaming
> - * @pad: Starting pad
> + * @origin: Starting pad
>   * @pipe: Media pipeline to be assigned to all pads in the pipeline.
>   *
>   * Mark all pads connected to a given pad through enabled links, either

s/to a given pad/to the given @starting pad/

> @@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
>   * pipeline pointer must be identical for all nested calls to
>   * media_pipeline_start().
>   */
> -__must_check int media_pipeline_start(struct media_pad *pad,
> +__must_check int media_pipeline_start(struct media_pad *origin,
>  				      struct media_pipeline *pipe);
>  /**
>   * __media_pipeline_start - Mark a pipeline as streaming

The documentation for __media_pipeline_start needs a similar update.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Sakari Ailus]

Hi Laurent,

Thanks for the review.

On Tue, Nov 05, 2024 at 10:03:07AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> Thank you for the patch.
> 
> On Tue, Nov 05, 2024 at 08:40:12AM +0200, Sakari Ailus wrote:
> > Rename the pad field in __media_pipeline_start() to both better describe
> > what it is and avoid masking it during the loop.
> > 
> > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > since v1:
> > 
> > - Also change the argument name in the prototype.
> > 
> >  drivers/media/mc/mc-entity.c | 8 ++++----
> >  include/media/media-entity.h | 4 ++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > index 96dd0f6ccd0d..0df9fc90cf33 100644
> > --- a/drivers/media/mc/mc-entity.c
> > +++ b/drivers/media/mc/mc-entity.c
> > @@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
> >  	return ret;
> >  }
> >  
> > -__must_check int __media_pipeline_start(struct media_pad *pad,
> > +__must_check int __media_pipeline_start(struct media_pad *origin,
> >  					struct media_pipeline *pipe)
> >  {
> > -	struct media_device *mdev = pad->graph_obj.mdev;
> > +	struct media_device *mdev = origin->graph_obj.mdev;
> >  	struct media_pipeline_pad *err_ppad;
> >  	struct media_pipeline_pad *ppad;
> >  	int ret;
> > @@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> >  	 * If the pad is already part of a pipeline, that pipeline must be the
> >  	 * same as the pipe given to media_pipeline_start().
> >  	 */
> > -	if (WARN_ON(pad->pipe && pad->pipe != pipe))
> > +	if (WARN_ON(origin->pipe && origin->pipe != pipe))
> >  		return -EINVAL;
> >  
> >  	/*
> > @@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> >  	 * with media_pipeline_pad instances for each pad found during graph
> >  	 * walk.
> >  	 */
> > -	ret = media_pipeline_populate(pipe, pad);
> > +	ret = media_pipeline_populate(pipe, origin);
> >  	if (ret)
> >  		return ret;
> >  
> > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > index 0393b23129eb..2fca4556c311 100644
> > --- a/include/media/media-entity.h
> > +++ b/include/media/media-entity.h
> > @@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> >  
> >  /**
> >   * media_pipeline_start - Mark a pipeline as streaming
> > - * @pad: Starting pad
> > + * @origin: Starting pad
> >   * @pipe: Media pipeline to be assigned to all pads in the pipeline.
> >   *
> >   * Mark all pads connected to a given pad through enabled links, either
> 
> s/to a given pad/to the given @starting pad/

s/to a given pad/@origin pad/

?

> 
> > @@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> >   * pipeline pointer must be identical for all nested calls to
> >   * media_pipeline_start().
> >   */
> > -__must_check int media_pipeline_start(struct media_pad *pad,
> > +__must_check int media_pipeline_start(struct media_pad *origin,
> >  				      struct media_pipeline *pipe);
> >  /**
> >   * __media_pipeline_start - Mark a pipeline as streaming
> 
> The documentation for __media_pipeline_start needs a similar update.

Too many similarly named functions. :-P

I'll address this in v3.

-- 
Regards,

Sakari Ailus

Re: [PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Laurent Pinchart]

On Tue, Nov 05, 2024 at 08:14:43AM +0000, Sakari Ailus wrote:
> Hi Laurent,
> 
> Thanks for the review.
> 
> On Tue, Nov 05, 2024 at 10:03:07AM +0200, Laurent Pinchart wrote:
> > Hi Sakari,
> > 
> > Thank you for the patch.
> > 
> > On Tue, Nov 05, 2024 at 08:40:12AM +0200, Sakari Ailus wrote:
> > > Rename the pad field in __media_pipeline_start() to both better describe
> > > what it is and avoid masking it during the loop.
> > > 
> > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > > since v1:
> > > 
> > > - Also change the argument name in the prototype.
> > > 
> > >  drivers/media/mc/mc-entity.c | 8 ++++----
> > >  include/media/media-entity.h | 4 ++--
> > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > > index 96dd0f6ccd0d..0df9fc90cf33 100644
> > > --- a/drivers/media/mc/mc-entity.c
> > > +++ b/drivers/media/mc/mc-entity.c
> > > @@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
> > >  	return ret;
> > >  }
> > >  
> > > -__must_check int __media_pipeline_start(struct media_pad *pad,
> > > +__must_check int __media_pipeline_start(struct media_pad *origin,
> > >  					struct media_pipeline *pipe)
> > >  {
> > > -	struct media_device *mdev = pad->graph_obj.mdev;
> > > +	struct media_device *mdev = origin->graph_obj.mdev;
> > >  	struct media_pipeline_pad *err_ppad;
> > >  	struct media_pipeline_pad *ppad;
> > >  	int ret;
> > > @@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > >  	 * If the pad is already part of a pipeline, that pipeline must be the
> > >  	 * same as the pipe given to media_pipeline_start().
> > >  	 */
> > > -	if (WARN_ON(pad->pipe && pad->pipe != pipe))
> > > +	if (WARN_ON(origin->pipe && origin->pipe != pipe))
> > >  		return -EINVAL;
> > >  
> > >  	/*
> > > @@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > >  	 * with media_pipeline_pad instances for each pad found during graph
> > >  	 * walk.
> > >  	 */
> > > -	ret = media_pipeline_populate(pipe, pad);
> > > +	ret = media_pipeline_populate(pipe, origin);
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > > index 0393b23129eb..2fca4556c311 100644
> > > --- a/include/media/media-entity.h
> > > +++ b/include/media/media-entity.h
> > > @@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > >  
> > >  /**
> > >   * media_pipeline_start - Mark a pipeline as streaming
> > > - * @pad: Starting pad
> > > + * @origin: Starting pad
> > >   * @pipe: Media pipeline to be assigned to all pads in the pipeline.
> > >   *
> > >   * Mark all pads connected to a given pad through enabled links, either
> > 
> > s/to a given pad/to the given @starting pad/
> 
> s/to a given pad/@origin pad/
>
> ?

I meant

 * Mark all pads connected to the given @starting pad through enabled links,

> > > @@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > >   * pipeline pointer must be identical for all nested calls to
> > >   * media_pipeline_start().
> > >   */
> > > -__must_check int media_pipeline_start(struct media_pad *pad,
> > > +__must_check int media_pipeline_start(struct media_pad *origin,
> > >  				      struct media_pipeline *pipe);
> > >  /**
> > >   * __media_pipeline_start - Mark a pipeline as streaming
> > 
> > The documentation for __media_pipeline_start needs a similar update.
> 
> Too many similarly named functions. :-P
> 
> I'll address this in v3.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Sakari Ailus]

On Tue, Nov 05, 2024 at 10:27:17AM +0200, Laurent Pinchart wrote:
> On Tue, Nov 05, 2024 at 08:14:43AM +0000, Sakari Ailus wrote:
> > Hi Laurent,
> > 
> > Thanks for the review.
> > 
> > On Tue, Nov 05, 2024 at 10:03:07AM +0200, Laurent Pinchart wrote:
> > > Hi Sakari,
> > > 
> > > Thank you for the patch.
> > > 
> > > On Tue, Nov 05, 2024 at 08:40:12AM +0200, Sakari Ailus wrote:
> > > > Rename the pad field in __media_pipeline_start() to both better describe
> > > > what it is and avoid masking it during the loop.
> > > > 
> > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > ---
> > > > since v1:
> > > > 
> > > > - Also change the argument name in the prototype.
> > > > 
> > > >  drivers/media/mc/mc-entity.c | 8 ++++----
> > > >  include/media/media-entity.h | 4 ++--
> > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > > > index 96dd0f6ccd0d..0df9fc90cf33 100644
> > > > --- a/drivers/media/mc/mc-entity.c
> > > > +++ b/drivers/media/mc/mc-entity.c
> > > > @@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
> > > >  	return ret;
> > > >  }
> > > >  
> > > > -__must_check int __media_pipeline_start(struct media_pad *pad,
> > > > +__must_check int __media_pipeline_start(struct media_pad *origin,
> > > >  					struct media_pipeline *pipe)
> > > >  {
> > > > -	struct media_device *mdev = pad->graph_obj.mdev;
> > > > +	struct media_device *mdev = origin->graph_obj.mdev;
> > > >  	struct media_pipeline_pad *err_ppad;
> > > >  	struct media_pipeline_pad *ppad;
> > > >  	int ret;
> > > > @@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > > >  	 * If the pad is already part of a pipeline, that pipeline must be the
> > > >  	 * same as the pipe given to media_pipeline_start().
> > > >  	 */
> > > > -	if (WARN_ON(pad->pipe && pad->pipe != pipe))
> > > > +	if (WARN_ON(origin->pipe && origin->pipe != pipe))
> > > >  		return -EINVAL;
> > > >  
> > > >  	/*
> > > > @@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > > >  	 * with media_pipeline_pad instances for each pad found during graph
> > > >  	 * walk.
> > > >  	 */
> > > > -	ret = media_pipeline_populate(pipe, pad);
> > > > +	ret = media_pipeline_populate(pipe, origin);
> > > >  	if (ret)
> > > >  		return ret;
> > > >  
> > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > > > index 0393b23129eb..2fca4556c311 100644
> > > > --- a/include/media/media-entity.h
> > > > +++ b/include/media/media-entity.h
> > > > @@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > > >  
> > > >  /**
> > > >   * media_pipeline_start - Mark a pipeline as streaming
> > > > - * @pad: Starting pad
> > > > + * @origin: Starting pad
> > > >   * @pipe: Media pipeline to be assigned to all pads in the pipeline.
> > > >   *
> > > >   * Mark all pads connected to a given pad through enabled links, either
> > > 
> > > s/to a given pad/to the given @starting pad/
> > 
> > s/to a given pad/@origin pad/
> >
> > ?
> 
> I meant
> 
>  * Mark all pads connected to the given @starting pad through enabled links,

There's no "starting" among the arguments.

> 
> > > > @@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > > >   * pipeline pointer must be identical for all nested calls to
> > > >   * media_pipeline_start().
> > > >   */
> > > > -__must_check int media_pipeline_start(struct media_pad *pad,
> > > > +__must_check int media_pipeline_start(struct media_pad *origin,
> > > >  				      struct media_pipeline *pipe);
> > > >  /**
> > > >   * __media_pipeline_start - Mark a pipeline as streaming
> > > 
> > > The documentation for __media_pipeline_start needs a similar update.
> > 
> > Too many similarly named functions. :-P
> > 
> > I'll address this in v3.
> 

-- 
Sakari Ailus

Re: [PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Laurent Pinchart]

On Tue, Nov 05, 2024 at 08:29:00AM +0000, Sakari Ailus wrote:
> On Tue, Nov 05, 2024 at 10:27:17AM +0200, Laurent Pinchart wrote:
> > On Tue, Nov 05, 2024 at 08:14:43AM +0000, Sakari Ailus wrote:
> > > On Tue, Nov 05, 2024 at 10:03:07AM +0200, Laurent Pinchart wrote:
> > > > On Tue, Nov 05, 2024 at 08:40:12AM +0200, Sakari Ailus wrote:
> > > > > Rename the pad field in __media_pipeline_start() to both better describe
> > > > > what it is and avoid masking it during the loop.
> > > > > 
> > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > > ---
> > > > > since v1:
> > > > > 
> > > > > - Also change the argument name in the prototype.
> > > > > 
> > > > >  drivers/media/mc/mc-entity.c | 8 ++++----
> > > > >  include/media/media-entity.h | 4 ++--
> > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > > > > index 96dd0f6ccd0d..0df9fc90cf33 100644
> > > > > --- a/drivers/media/mc/mc-entity.c
> > > > > +++ b/drivers/media/mc/mc-entity.c
> > > > > @@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
> > > > >  	return ret;
> > > > >  }
> > > > >  
> > > > > -__must_check int __media_pipeline_start(struct media_pad *pad,
> > > > > +__must_check int __media_pipeline_start(struct media_pad *origin,
> > > > >  					struct media_pipeline *pipe)
> > > > >  {
> > > > > -	struct media_device *mdev = pad->graph_obj.mdev;
> > > > > +	struct media_device *mdev = origin->graph_obj.mdev;
> > > > >  	struct media_pipeline_pad *err_ppad;
> > > > >  	struct media_pipeline_pad *ppad;
> > > > >  	int ret;
> > > > > @@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > > > >  	 * If the pad is already part of a pipeline, that pipeline must be the
> > > > >  	 * same as the pipe given to media_pipeline_start().
> > > > >  	 */
> > > > > -	if (WARN_ON(pad->pipe && pad->pipe != pipe))
> > > > > +	if (WARN_ON(origin->pipe && origin->pipe != pipe))
> > > > >  		return -EINVAL;
> > > > >  
> > > > >  	/*
> > > > > @@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > > > >  	 * with media_pipeline_pad instances for each pad found during graph
> > > > >  	 * walk.
> > > > >  	 */
> > > > > -	ret = media_pipeline_populate(pipe, pad);
> > > > > +	ret = media_pipeline_populate(pipe, origin);
> > > > >  	if (ret)
> > > > >  		return ret;
> > > > >  
> > > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > > > > index 0393b23129eb..2fca4556c311 100644
> > > > > --- a/include/media/media-entity.h
> > > > > +++ b/include/media/media-entity.h
> > > > > @@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > > > >  
> > > > >  /**
> > > > >   * media_pipeline_start - Mark a pipeline as streaming
> > > > > - * @pad: Starting pad
> > > > > + * @origin: Starting pad
> > > > >   * @pipe: Media pipeline to be assigned to all pads in the pipeline.
> > > > >   *
> > > > >   * Mark all pads connected to a given pad through enabled links, either
> > > > 
> > > > s/to a given pad/to the given @starting pad/
> > > 
> > > s/to a given pad/@origin pad/
> > >
> > > ?
> > 
> > I meant
> > 
> >  * Mark all pads connected to the given @starting pad through enabled links,
> 
> There's no "starting" among the arguments.

Seems I need to wake up.

 * Mark all pads connected to the given @origin pad through enabled links,

> > > > > @@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > > > >   * pipeline pointer must be identical for all nested calls to
> > > > >   * media_pipeline_start().
> > > > >   */
> > > > > -__must_check int media_pipeline_start(struct media_pad *pad,
> > > > > +__must_check int media_pipeline_start(struct media_pad *origin,
> > > > >  				      struct media_pipeline *pipe);
> > > > >  /**
> > > > >   * __media_pipeline_start - Mark a pipeline as streaming
> > > > 
> > > > The documentation for __media_pipeline_start needs a similar update.
> > > 
> > > Too many similarly named functions. :-P
> > > 
> > > I'll address this in v3.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/1] media: mc: Rename pad as origin in __media_pipeline_start()

[Sakari Ailus]

On Tue, Nov 05, 2024 at 10:37:44AM +0200, Laurent Pinchart wrote:
> On Tue, Nov 05, 2024 at 08:29:00AM +0000, Sakari Ailus wrote:
> > On Tue, Nov 05, 2024 at 10:27:17AM +0200, Laurent Pinchart wrote:
> > > On Tue, Nov 05, 2024 at 08:14:43AM +0000, Sakari Ailus wrote:
> > > > On Tue, Nov 05, 2024 at 10:03:07AM +0200, Laurent Pinchart wrote:
> > > > > On Tue, Nov 05, 2024 at 08:40:12AM +0200, Sakari Ailus wrote:
> > > > > > Rename the pad field in __media_pipeline_start() to both better describe
> > > > > > what it is and avoid masking it during the loop.
> > > > > > 
> > > > > > Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > > > > Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > > > > > ---
> > > > > > since v1:
> > > > > > 
> > > > > > - Also change the argument name in the prototype.
> > > > > > 
> > > > > >  drivers/media/mc/mc-entity.c | 8 ++++----
> > > > > >  include/media/media-entity.h | 4 ++--
> > > > > >  2 files changed, 6 insertions(+), 6 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/media/mc/mc-entity.c b/drivers/media/mc/mc-entity.c
> > > > > > index 96dd0f6ccd0d..0df9fc90cf33 100644
> > > > > > --- a/drivers/media/mc/mc-entity.c
> > > > > > +++ b/drivers/media/mc/mc-entity.c
> > > > > > @@ -768,10 +768,10 @@ static int media_pipeline_populate(struct media_pipeline *pipe,
> > > > > >  	return ret;
> > > > > >  }
> > > > > >  
> > > > > > -__must_check int __media_pipeline_start(struct media_pad *pad,
> > > > > > +__must_check int __media_pipeline_start(struct media_pad *origin,
> > > > > >  					struct media_pipeline *pipe)
> > > > > >  {
> > > > > > -	struct media_device *mdev = pad->graph_obj.mdev;
> > > > > > +	struct media_device *mdev = origin->graph_obj.mdev;
> > > > > >  	struct media_pipeline_pad *err_ppad;
> > > > > >  	struct media_pipeline_pad *ppad;
> > > > > >  	int ret;
> > > > > > @@ -782,7 +782,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > > > > >  	 * If the pad is already part of a pipeline, that pipeline must be the
> > > > > >  	 * same as the pipe given to media_pipeline_start().
> > > > > >  	 */
> > > > > > -	if (WARN_ON(pad->pipe && pad->pipe != pipe))
> > > > > > +	if (WARN_ON(origin->pipe && origin->pipe != pipe))
> > > > > >  		return -EINVAL;
> > > > > >  
> > > > > >  	/*
> > > > > > @@ -799,7 +799,7 @@ __must_check int __media_pipeline_start(struct media_pad *pad,
> > > > > >  	 * with media_pipeline_pad instances for each pad found during graph
> > > > > >  	 * walk.
> > > > > >  	 */
> > > > > > -	ret = media_pipeline_populate(pipe, pad);
> > > > > > +	ret = media_pipeline_populate(pipe, origin);
> > > > > >  	if (ret)
> > > > > >  		return ret;
> > > > > >  
> > > > > > diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> > > > > > index 0393b23129eb..2fca4556c311 100644
> > > > > > --- a/include/media/media-entity.h
> > > > > > +++ b/include/media/media-entity.h
> > > > > > @@ -1143,7 +1143,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > > > > >  
> > > > > >  /**
> > > > > >   * media_pipeline_start - Mark a pipeline as streaming
> > > > > > - * @pad: Starting pad
> > > > > > + * @origin: Starting pad
> > > > > >   * @pipe: Media pipeline to be assigned to all pads in the pipeline.
> > > > > >   *
> > > > > >   * Mark all pads connected to a given pad through enabled links, either
> > > > > 
> > > > > s/to a given pad/to the given @starting pad/
> > > > 
> > > > s/to a given pad/@origin pad/
> > > >
> > > > ?
> > > 
> > > I meant
> > > 
> > >  * Mark all pads connected to the given @starting pad through enabled links,
> > 
> > There's no "starting" among the arguments.
> 
> Seems I need to wake up.

I thought so. ;-)

> 
>  * Mark all pads connected to the given @origin pad through enabled links,

The pad is specified here so there's no need for "the given".

> 
> > > > > > @@ -1155,7 +1155,7 @@ struct media_entity *media_graph_walk_next(struct media_graph *graph);
> > > > > >   * pipeline pointer must be identical for all nested calls to
> > > > > >   * media_pipeline_start().
> > > > > >   */
> > > > > > -__must_check int media_pipeline_start(struct media_pad *pad,
> > > > > > +__must_check int media_pipeline_start(struct media_pad *origin,
> > > > > >  				      struct media_pipeline *pipe);
> > > > > >  /**
> > > > > >   * __media_pipeline_start - Mark a pipeline as streaming
> > > > > 
> > > > > The documentation for __media_pipeline_start needs a similar update.
> > > > 
> > > > Too many similarly named functions. :-P
> > > > 
> > > > I'll address this in v3.
> 

-- 
Sakari Ailus