Re: [PATCH 07/18] media: v4l: async: Clean up list heads and entries

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

Hi Sakari,

Thank you for the patch.

On Thu, Mar 30, 2023 at 02:58:42PM +0300, Sakari Ailus wrote:
> The naming of list heads and list entries is confusing as they're named
> similarly. Use _head for list head and _list for list entries.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  drivers/media/pci/intel/ipu3/ipu3-cio2-main.c |  2 +-
>  .../platform/renesas/rcar-vin/rcar-core.c     |  2 +-
>  .../platform/renesas/rzg2l-cru/rzg2l-core.c   |  2 +-
>  drivers/media/platform/xilinx/xilinx-vipp.c   | 10 +--
>  drivers/media/v4l2-core/v4l2-async.c          | 66 +++++++++----------
>  .../staging/media/imx/imx-media-dev-common.c  |  2 +-
>  drivers/staging/media/tegra-video/vi.c        |  6 +-
>  include/media/v4l2-async.h                    | 21 +++---
>  8 files changed, 56 insertions(+), 55 deletions(-)

[snip]

> diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
> index 0c4cffd081c9..425280b4d387 100644
> --- a/include/media/v4l2-async.h
> +++ b/include/media/v4l2-async.h
> @@ -68,7 +68,7 @@ struct v4l2_async_match {
>   * @match:	struct of match type and per-bus type matching data sets
>   * @asd_list:	used to add struct v4l2_async_subdev objects to the
>   *		master notifier @asd_list

It's now called @asd_head.

> - * @list:	used to link struct v4l2_async_subdev objects, waiting to be
> + * @waiting_list: used to link struct v4l2_async_subdev objects, waiting to be
>   *		probed, to a notifier->waiting list

It's now called notifier->waiting_head.

Please double-check comments and documentation to catch other
occurrences.

>   *
>   * When this struct is used as a member in a driver specific struct,
> @@ -77,9 +77,10 @@ struct v4l2_async_match {
>   */
>  struct v4l2_async_subdev {
>  	struct v4l2_async_match match;
> +
>  	/* v4l2-async core private: not to be used by drivers */
> -	struct list_head list;
>  	struct list_head asd_list;
> +	struct list_head waiting_list;
>  };
>  
>  /**
> @@ -108,20 +109,20 @@ struct v4l2_async_notifier_operations {
>   * @v4l2_dev:	v4l2_device of the root notifier, NULL otherwise
>   * @sd:		sub-device that registered the notifier, NULL otherwise
>   * @parent:	parent notifier
> - * @asd_list:	master list of struct v4l2_async_subdev
> - * @waiting:	list of struct v4l2_async_subdev, waiting for their drivers
> - * @done:	list of struct v4l2_subdev, already probed
> - * @list:	member in a global list of notifiers
> + * @asd_head:	master list of struct v4l2_async_subdev
> + * @waiting_list: list of struct v4l2_async_subdev, waiting for their drivers
> + * @done_head:	list of struct v4l2_subdev, already probed
> + * @notifier_list: member in a global list of notifiers
>   */
>  struct v4l2_async_notifier {
>  	const struct v4l2_async_notifier_operations *ops;
>  	struct v4l2_device *v4l2_dev;
>  	struct v4l2_subdev *sd;
>  	struct v4l2_async_notifier *parent;
> -	struct list_head asd_list;
> -	struct list_head waiting;
> -	struct list_head done;
> -	struct list_head list;
> +	struct list_head asd_head;
> +	struct list_head waiting_head;
> +	struct list_head done_head;
> +	struct list_head notifier_list;

I find the _head suffix to still be confusing. How about the following ?

	struct {
		struct list_head all;
		struct list_head waiting;
		struct list_head done;
	} asds;

>  };
>  
>  /**

-- 
Regards,

Laurent Pinchart