Re: [PATCH v2 1/4] dri: do not check for NULL debugfs dentry

["Das, Nirmoy" <nirmoy.das@amd.com>]

Ah there are three typos :/

s/deference/dereference for this one and for the 2nd patch as well.


Regards,

Nirmoy

On 10/13/2021 8:35 PM, Nirmoy Das wrote:
> Debugfs APIs returns encoded error on failure instead of NULL
> and for drm primary/minor debugfs directories, we save the
> returned value in the dentry pointer and pass it on to drm
> drivers to further create debugfs files/directories. Error
> conditions are handled by debugfs APIs, so no need to check
> for NULL, as saved dentry pointers will either contain a
> valid pointer or an error code.
>
> Also document this for future reference.
>
> CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> CC: Maxime Ripard <mripard@kernel.org>
> CC: Thomas Zimmermann <tzimmermann@suse.de>
> CC: David Airlie <airlied@linux.ie>
> CC: Daniel Vetter <daniel@ffwll.ch>
>
> Signed-off-by: Nirmoy Das <nirmoy.das@amd.com>
> ---
>   drivers/gpu/drm/drm_debugfs.c |  9 ---------
>   drivers/gpu/drm/drm_drv.c     |  1 +
>   include/drm/drm_file.h        | 28 ++++++++++++++++++++++++----
>   3 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index b0a826489488..0073854a4383 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -272,9 +272,6 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
>
>   void drm_debugfs_cleanup(struct drm_minor *minor)
>   {
> -	if (!minor->debugfs_root)
> -		return;
> -
>   	drm_debugfs_remove_all_files(minor);
>
>   	debugfs_remove_recursive(minor->debugfs_root);
> @@ -419,9 +416,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector)
>   	struct drm_minor *minor = connector->dev->primary;
>   	struct dentry *root;
>
> -	if (!minor->debugfs_root)
> -		return;
> -
>   	root = debugfs_create_dir(connector->name, minor->debugfs_root);
>   	connector->debugfs_entry = root;
>
> @@ -440,9 +434,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector)
>
>   void drm_debugfs_connector_remove(struct drm_connector *connector)
>   {
> -	if (!connector->debugfs_entry)
> -		return;
> -
>   	debugfs_remove_recursive(connector->debugfs_entry);
>
>   	connector->debugfs_entry = NULL;
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7a5097467ba5..918f302d9c43 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -64,6 +64,7 @@ static struct idr drm_minors_idr;
>    */
>   static bool drm_core_init_complete;
>
> +/* Do not deference this pointer as it will contain ERR_PTR on error. */
>   static struct dentry *drm_debugfs_root;
>
>   DEFINE_STATIC_SRCU(drm_unplug_srcu);
> diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h
> index a3acb7ac3550..3a30fc4d8905 100644
> --- a/include/drm/drm_file.h
> +++ b/include/drm/drm_file.h
> @@ -69,15 +69,35 @@ enum drm_minor_type {
>    */
>   struct drm_minor {
>   	/* private: */
> -	int index;			/* Minor device number */
> -	int type;                       /* Control or render */
> -	struct device *kdev;		/* Linux device */
> +	/** @index: minor device number. */
> +	int index;
> +
> +	/** @type: minor device type: primary, control, render. */
> +	int type;
> +
> +	/** @kdev: Linux device pointer. */
> +	struct device *kdev;
> +
> +	/** @dev: drm device pointer. */
>   	struct drm_device *dev;
>
> +
> +	/** @debugfs_root:
> +	 *
> +	 * Dentry for /sys/kernel/debug/dri/@index debugfs dir. Do not
> +	 * deference this pointer as it will contain ERR_PTR on error.
> +	 */
>   	struct dentry *debugfs_root;
>
> +	/** @debugfs_list:
> +	 *
> +	 * A list to keep track of debugfs dentries created using
> +	 * drm_debugfs_create_files() by drm drivers.
> +	 */
>   	struct list_head debugfs_list;
> -	struct mutex debugfs_lock; /* Protects debugfs_list. */
> +
> +	/** @debugfs_lock: Protects debugfs_list. */
> +	struct mutex debugfs_lock;
>   };
>
>   /**
> --
> 2.32.0
>