Re: [PATCH 02/13] drm/debugfs: Create helper to create debugfs files from list

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 11 Jan 2023, Maíra Canal <mcanal@igalia.com> wrote:
> Create a helper to encapsulate the code that creates a new debugfs file
> from a linked list related to an object. Moreover, the helper also provides
> more flexibily on the type of the object.
>
> Signed-off-by: Maíra Canal <mcanal@igalia.com>
> ---
>  drivers/gpu/drm/drm_debugfs.c | 22 ++++++++++++----------
>  1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 255d2068ac16..23f6ed7b5d68 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -218,6 +218,16 @@ void drm_debugfs_create_files(const struct drm_info_list *files, int count,
>  }
>  EXPORT_SYMBOL(drm_debugfs_create_files);
>  
> +#define drm_create_file_from_list(component) do {					\
> +		list_for_each_entry_safe(entry, tmp, &(component)->debugfs_list,	\
> +					 list) {					\
> +			debugfs_create_file(entry->file.name, 0444,			\
> +					    minor->debugfs_root, entry,			\
> +					    &drm_debugfs_entry_fops);			\
> +			list_del(&entry->list);						\
> +		}									\
> +	} while (0)
> +

Same here as in the previous patch. I really think you should try to
figure out how to break this into useful functions, and avoid macros
like this.

BR,
Jani.

>  int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  		     struct dentry *root)
>  {
> @@ -245,11 +255,7 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  	if (dev->driver->debugfs_init)
>  		dev->driver->debugfs_init(minor);
>  
> -	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
> -		debugfs_create_file(entry->file.name, 0444,
> -				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
> -		list_del(&entry->list);
> -	}
> +	drm_create_file_from_list(dev);
>  
>  	return 0;
>  }
> @@ -262,11 +268,7 @@ void drm_debugfs_late_register(struct drm_device *dev)
>  	if (!minor)
>  		return;
>  
> -	list_for_each_entry_safe(entry, tmp, &dev->debugfs_list, list) {
> -		debugfs_create_file(entry->file.name, 0444,
> -				    minor->debugfs_root, entry, &drm_debugfs_entry_fops);
> -		list_del(&entry->list);
> -	}
> +	drm_create_file_from_list(dev);
>  }
>  
>  int drm_debugfs_remove_files(const struct drm_info_list *files, int count,

-- 
Jani Nikula, Intel Open Source Graphics Center