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

2021-10-13 Thread Das, Nirmoy

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 
CC: Maxime Ripard 
CC: Thomas Zimmermann 
CC: David Airlie 
CC: Daniel Vetter 

Signed-off-by: Nirmoy Das 
---
  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



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

2021-10-13 Thread Nirmoy Das
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 
CC: Maxime Ripard 
CC: Thomas Zimmermann 
CC: David Airlie 
CC: Daniel Vetter 

Signed-off-by: Nirmoy Das 
---
 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