[PATCH] drm/arc: make arcpgu_debugfs_init return 0

[PATCH] drm/arc: make arcpgu_debugfs_init return 0

[Wambui Karuga]

As drm_debugfs_create_files should return void, remove its use as the
return value of arcpgu_debugfs_init and have the latter function
return 0 directly.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/arc/arcpgu_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index d6a6692db0ac..660b25f9588e 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -139,8 +139,10 @@ static struct drm_info_list arcpgu_debugfs_list[] = {
 
 static int arcpgu_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(arcpgu_debugfs_list,
-		ARRAY_SIZE(arcpgu_debugfs_list), minor->debugfs_root, minor);
+	drm_debugfs_create_files(arcpgu_debugfs_list,
+				 ARRAY_SIZE(arcpgu_debugfs_list),
+				 minor->debugfs_root, minor);
+	return 0;
 }
 #endif
 
-- 
2.25.0

[PATCH] drm/arm: make hdlcd_debugfs_init return 0

[Wambui Karuga]

As drm_debugfs_create_files should return void, remove its use as a
return value in hdlcd_debugfs_init and have the latter function return 0
directly.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 2e053815b54a..bd0ad6f46a97 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -226,8 +226,10 @@ static struct drm_info_list hdlcd_debugfs_list[] = {
 
 static int hdlcd_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(hdlcd_debugfs_list,
-		ARRAY_SIZE(hdlcd_debugfs_list),	minor->debugfs_root, minor);
+	drm_debugfs_create_files(hdlcd_debugfs_list,
+				 ARRAY_SIZE(hdlcd_debugfs_list),
+				 minor->debugfs_root, minor);
+	return 0;
 }
 #endif
 
-- 
2.25.0

[PATCH 1/2] drm/debugfs: remove checks for return value of drm_debugfs functions.

[Wambui Karuga]

As there is no need to check the return value of
drm_debugfs_create_files, remove unnecessary checks and error handling
statement blocks.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/drm_debugfs.c | 28 +++++-----------------------
 1 file changed, 5 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 4e673d318503..6a2f141b6a38 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -215,35 +215,17 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
 	sprintf(name, "%d", minor_id);
 	minor->debugfs_root = debugfs_create_dir(name, root);
 
-	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
-				       minor->debugfs_root, minor);
-	if (ret) {
-		debugfs_remove(minor->debugfs_root);
-		minor->debugfs_root = NULL;
-		DRM_ERROR("Failed to create core drm debugfs files\n");
-		return ret;
-	}
+	drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
+				 minor->debugfs_root, minor);
 
 	if (drm_drv_uses_atomic_modeset(dev)) {
-		ret = drm_atomic_debugfs_init(minor);
-		if (ret) {
-			DRM_ERROR("Failed to create atomic debugfs files\n");
-			return ret;
-		}
+		drm_atomic_debugfs_init(minor);
 	}
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		ret = drm_framebuffer_debugfs_init(minor);
-		if (ret) {
-			DRM_ERROR("Failed to create framebuffer debugfs file\n");
-			return ret;
-		}
+		drm_framebuffer_debugfs_init(minor);
 
-		ret = drm_client_debugfs_init(minor);
-		if (ret) {
-			DRM_ERROR("Failed to create client debugfs file\n");
-			return ret;
-		}
+		drm_client_debugfs_init(minor);
 	}
 
 	if (dev->driver->debugfs_init) {
-- 
2.25.0

[PATCH] drm/etnaviv: remove check for return value of drm_debugfs function

[Wambui Karuga]

As there is no need to check the return value if
drm_debugfs_create_files, remove the check and error handling in
etnaviv_debugfs_init and have the function return 0 directly.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index 6b43c1c94e8f..a65d30a48a9d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -233,19 +233,11 @@ static struct drm_info_list etnaviv_debugfs_list[] = {
 
 static int etnaviv_debugfs_init(struct drm_minor *minor)
 {
-	struct drm_device *dev = minor->dev;
-	int ret;
-
-	ret = drm_debugfs_create_files(etnaviv_debugfs_list,
-			ARRAY_SIZE(etnaviv_debugfs_list),
-			minor->debugfs_root, minor);
+	drm_debugfs_create_files(etnaviv_debugfs_list,
+				 ARRAY_SIZE(etnaviv_debugfs_list),
+				 minor->debugfs_root, minor);
 
-	if (ret) {
-		dev_err(dev->dev, "could not install etnaviv_debugfs_list\n");
-		return ret;
-	}
-
-	return ret;
+	return 0;
 }
 #endif
 
-- 
2.25.0

[PATCH] drm/i915: make i915_debugfs_register return void.

[Wambui Karuga]

As drm_debugfs_create_files should return void, remove its use as the
return value of i915_debugfs_register and have i915_debugfs_register
return void.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 8 ++++----
 drivers/gpu/drm/i915/i915_debugfs.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e5eea915bd0d..4a3c58f9fc1e 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2391,7 +2391,7 @@ static const struct i915_debugfs_files {
 	{"i915_guc_log_relay", &i915_guc_log_relay_fops},
 };
 
-int i915_debugfs_register(struct drm_i915_private *dev_priv)
+void i915_debugfs_register(struct drm_i915_private *dev_priv)
 {
 	struct drm_minor *minor = dev_priv->drm.primary;
 	int i;
@@ -2408,7 +2408,7 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
 				    i915_debugfs_files[i].fops);
 	}
 
-	return drm_debugfs_create_files(i915_debugfs_list,
-					I915_DEBUGFS_ENTRIES,
-					minor->debugfs_root, minor);
+	drm_debugfs_create_files(i915_debugfs_list,
+				 I915_DEBUGFS_ENTRIES,
+				 minor->debugfs_root, minor);
 }
diff --git a/drivers/gpu/drm/i915/i915_debugfs.h b/drivers/gpu/drm/i915/i915_debugfs.h
index 6da39c76ab5e..1de2736f1248 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.h
+++ b/drivers/gpu/drm/i915/i915_debugfs.h
@@ -12,10 +12,10 @@ struct drm_i915_private;
 struct seq_file;
 
 #ifdef CONFIG_DEBUG_FS
-int i915_debugfs_register(struct drm_i915_private *dev_priv);
+void i915_debugfs_register(struct drm_i915_private *dev_priv);
 void i915_debugfs_describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj);
 #else
-static inline int i915_debugfs_register(struct drm_i915_private *dev_priv) { return 0; }
+static inline void i915_debugfs_register(struct drm_i915_private *dev_priv) {}
 static inline void i915_debugfs_describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj) {}
 #endif
 
-- 
2.25.0

[PATCH] drm/nouveau: remove checks for return value of debugfs functions

[Wambui Karuga]

As there is no need to check for the return value of debugfs_create_file
and drm_debugfs_create_files, remove unnecessary checks and error
handling in nouveau_drm_debugfs_init.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 7dfbbbc1beea..15a3d40edf02 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -222,22 +222,18 @@ nouveau_drm_debugfs_init(struct drm_minor *minor)
 {
 	struct nouveau_drm *drm = nouveau_drm(minor->dev);
 	struct dentry *dentry;
-	int i, ret;
+	int i;
 
 	for (i = 0; i < ARRAY_SIZE(nouveau_debugfs_files); i++) {
-		dentry = debugfs_create_file(nouveau_debugfs_files[i].name,
-					     S_IRUGO | S_IWUSR,
-					     minor->debugfs_root, minor->dev,
-					     nouveau_debugfs_files[i].fops);
-		if (!dentry)
-			return -ENOMEM;
+		debugfs_create_file(nouveau_debugfs_files[i].name,
+				    S_IRUGO | S_IWUSR,
+				    minor->debugfs_root, minor->dev,
+				    nouveau_debugfs_files[i].fops);
 	}
 
-	ret = drm_debugfs_create_files(nouveau_debugfs_list,
-				       NOUVEAU_DEBUGFS_ENTRIES,
-				       minor->debugfs_root, minor);
-	if (ret)
-		return ret;
+	drm_debugfs_create_files(nouveau_debugfs_list,
+				 NOUVEAU_DEBUGFS_ENTRIES,
+				 minor->debugfs_root, minor);
 
 	/* Set the size of the vbios since we know it, and it's confusing to
 	 * userspace if it wants to seek() but the file has a length of 0
-- 
2.25.0

[PATCH] drm/v3d: make v3d_debugfs_init return 0

[Wambui Karuga]

As drm_debugfs_create_files should return void, remove its use as the
return value of v3d_debugfs_init and have the function return 0
directly.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/v3d/v3d_debugfs.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
index 9e953ce64ef7..57dded6a3957 100644
--- a/drivers/gpu/drm/v3d/v3d_debugfs.c
+++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
@@ -261,7 +261,8 @@ static const struct drm_info_list v3d_debugfs_list[] = {
 int
 v3d_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(v3d_debugfs_list,
-					ARRAY_SIZE(v3d_debugfs_list),
-					minor->debugfs_root, minor);
+	drm_debugfs_create_files(v3d_debugfs_list,
+				 ARRAY_SIZE(v3d_debugfs_list),
+				 minor->debugfs_root, minor);
+	return 0;
 }
-- 
2.25.0

[PATCH] drm/vc4: remove check of return value of drm_debugfs functions

[Wambui Karuga]

Remove unnecessary check and error handling for the return value of
drm_debugfs_create_files in vc4_debugfs_init.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/vc4/vc4_debugfs.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index b61b2d3407b5..1835f12337ec 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -30,11 +30,8 @@ vc4_debugfs_init(struct drm_minor *minor)
 			    minor->debugfs_root, &vc4->load_tracker_enabled);
 
 	list_for_each_entry(entry, &vc4->debugfs_list, link) {
-		int ret = drm_debugfs_create_files(&entry->info, 1,
-						   minor->debugfs_root, minor);
-
-		if (ret)
-			return ret;
+		drm_debugfs_create_files(&entry->info, 1,
+					 minor->debugfs_root, minor);
 	}
 
 	return 0;
-- 
2.25.0

[PATCH] drm/vram-helper: make drm_vram_mm_debugfs_init return 0

[Wambui Karuga]

As drm_debugfs_create_files() should return 0, remove its use as the
return value of drm_vram_mm_debugfs_init(), and have the function return
0 directly.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 92a11bb42365..77b36a2286f9 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -1043,19 +1043,16 @@ static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
  * @minor: drm minor device.
  *
  * Returns:
- * 0 on success, or
- * a negative error code otherwise.
+ * 0
  */
 int drm_vram_mm_debugfs_init(struct drm_minor *minor)
 {
-	int ret = 0;
-
 #if defined(CONFIG_DEBUG_FS)
-	ret = drm_debugfs_create_files(drm_vram_mm_debugfs_list,
-				       ARRAY_SIZE(drm_vram_mm_debugfs_list),
-				       minor->debugfs_root, minor);
+	drm_debugfs_create_files(drm_vram_mm_debugfs_list,
+				 ARRAY_SIZE(drm_vram_mm_debugfs_list),
+				 minor->debugfs_root, minor);
 #endif
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
 
-- 
2.25.0

[PATCH 2/2] drm: convert drm_debugfs functions to return void

[Wambui Karuga]

As drm_debug_create_files will be converted to return void,
drop return value from various drm_debugfs functions that return
drm_debug_create_files and convert the functions to return void.

Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
---
 drivers/gpu/drm/drm_atomic.c        | 8 ++++----
 drivers/gpu/drm/drm_client.c        | 8 ++++----
 drivers/gpu/drm/drm_crtc_internal.h | 2 +-
 drivers/gpu/drm/drm_framebuffer.c   | 8 ++++----
 drivers/gpu/drm/drm_internal.h      | 2 +-
 include/drm/drm_client.h            | 2 +-
 6 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 65c46ed049c5..d619f3340084 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1640,10 +1640,10 @@ static const struct drm_info_list drm_atomic_debugfs_list[] = {
 	{"state", drm_state_info, 0},
 };
 
-int drm_atomic_debugfs_init(struct drm_minor *minor)
+void drm_atomic_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(drm_atomic_debugfs_list,
-			ARRAY_SIZE(drm_atomic_debugfs_list),
-			minor->debugfs_root, minor);
+	drm_debugfs_create_files(drm_atomic_debugfs_list,
+				 ARRAY_SIZE(drm_atomic_debugfs_list),
+				 minor->debugfs_root, minor);
 }
 #endif
diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
index b031b45aa8ef..2a147d1c3a13 100644
--- a/drivers/gpu/drm/drm_client.c
+++ b/drivers/gpu/drm/drm_client.c
@@ -457,10 +457,10 @@ static const struct drm_info_list drm_client_debugfs_list[] = {
 	{ "internal_clients", drm_client_debugfs_internal_clients, 0 },
 };
 
-int drm_client_debugfs_init(struct drm_minor *minor)
+void drm_client_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(drm_client_debugfs_list,
-					ARRAY_SIZE(drm_client_debugfs_list),
-					minor->debugfs_root, minor);
+	drm_debugfs_create_files(drm_client_debugfs_list,
+				 ARRAY_SIZE(drm_client_debugfs_list),
+				 minor->debugfs_root, minor);
 }
 #endif
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 16f2413403aa..1b620ba9244b 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -224,7 +224,7 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
 /* drm_atomic.c */
 #ifdef CONFIG_DEBUG_FS
 struct drm_minor;
-int drm_atomic_debugfs_init(struct drm_minor *minor);
+void drm_atomic_debugfs_init(struct drm_minor *minor);
 #endif
 
 int __drm_atomic_helper_disable_plane(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index 57ac94ce9b9e..0375b3d7f8d0 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -1207,10 +1207,10 @@ static const struct drm_info_list drm_framebuffer_debugfs_list[] = {
 	{ "framebuffer", drm_framebuffer_info, 0 },
 };
 
-int drm_framebuffer_debugfs_init(struct drm_minor *minor)
+void drm_framebuffer_debugfs_init(struct drm_minor *minor)
 {
-	return drm_debugfs_create_files(drm_framebuffer_debugfs_list,
-				ARRAY_SIZE(drm_framebuffer_debugfs_list),
-				minor->debugfs_root, minor);
+	drm_debugfs_create_files(drm_framebuffer_debugfs_list,
+				 ARRAY_SIZE(drm_framebuffer_debugfs_list),
+				 minor->debugfs_root, minor);
 }
 #endif
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
index aeec2e68d772..f0c99d00b201 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -235,7 +235,7 @@ int drm_syncobj_query_ioctl(struct drm_device *dev, void *data,
 /* drm_framebuffer.c */
 void drm_framebuffer_print_info(struct drm_printer *p, unsigned int indent,
 				const struct drm_framebuffer *fb);
-int drm_framebuffer_debugfs_init(struct drm_minor *minor);
+void drm_framebuffer_debugfs_init(struct drm_minor *minor);
 
 /* drm_hdcp.c */
 int drm_setup_hdcp_srm(struct class *drm_class);
diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
index 3ed5dee899fd..7402f852d3c4 100644
--- a/include/drm/drm_client.h
+++ b/include/drm/drm_client.h
@@ -188,6 +188,6 @@ int drm_client_modeset_dpms(struct drm_client_dev *client, int mode);
 	drm_for_each_connector_iter(connector, iter) \
 		if (connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK)
 
-int drm_client_debugfs_init(struct drm_minor *minor);
+void drm_client_debugfs_init(struct drm_minor *minor);
 
 #endif
-- 
2.25.0

Re: [PATCH] drm/etnaviv: remove check for return value of drm_debugfs function

[Lucas Stach]

On Di, 2020-02-18 at 20:28 +0300, Wambui Karuga wrote:
> As there is no need to check the return value if
> drm_debugfs_create_files,

And here is where the commit message skips a very important
information: Since 987d65d01356 (drm: debugfs: make
drm_debugfs_create_files() never fail) this function never returns
anything other than 0, so there is no point in checking. This
information should be in the commit message, so the reviewer doesn't
need to look up this fact in the git history.

Regards,
Lucas

>  remove the check and error handling in
> etnaviv_debugfs_init and have the function return 0 directly.
> 
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index 6b43c1c94e8f..a65d30a48a9d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -233,19 +233,11 @@ static struct drm_info_list
> etnaviv_debugfs_list[] = {
>  
>  static int etnaviv_debugfs_init(struct drm_minor *minor)
>  {
> -	struct drm_device *dev = minor->dev;
> -	int ret;
> -
> -	ret = drm_debugfs_create_files(etnaviv_debugfs_list,
> -			ARRAY_SIZE(etnaviv_debugfs_list),
> -			minor->debugfs_root, minor);
> +	drm_debugfs_create_files(etnaviv_debugfs_list,
> +				 ARRAY_SIZE(etnaviv_debugfs_list),
> +				 minor->debugfs_root, minor);
>  
> -	if (ret) {
> -		dev_err(dev->dev, "could not install
> etnaviv_debugfs_list\n");
> -		return ret;
> -	}
> -
> -	return ret;
> +	return 0;
>  }
>  #endif
>  

Re: [PATCH] drm/etnaviv: remove check for return value of drm_debugfs function

[Wambui Karuga]

On Tue, 18 Feb 2020, Lucas Stach wrote:

> On Di, 2020-02-18 at 20:28 +0300, Wambui Karuga wrote:
>> As there is no need to check the return value if
>> drm_debugfs_create_files,
>
> And here is where the commit message skips a very important
> information: Since 987d65d01356 (drm: debugfs: make
> drm_debugfs_create_files() never fail) this function never returns
> anything other than 0, so there is no point in checking. This
> information should be in the commit message, so the reviewer doesn't
> need to look up this fact in the git history.
>
Okay, I can add that to the commit message and resend.
Thanks.

> Regards,
> Lucas
>
>>  remove the check and error handling in
>> etnaviv_debugfs_init and have the function return 0 directly.
>>
>> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
>> ---
>>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 16 ++++------------
>>  1 file changed, 4 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> index 6b43c1c94e8f..a65d30a48a9d 100644
>> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
>> @@ -233,19 +233,11 @@ static struct drm_info_list
>> etnaviv_debugfs_list[] = {
>>
>>  static int etnaviv_debugfs_init(struct drm_minor *minor)
>>  {
>> -	struct drm_device *dev = minor->dev;
>> -	int ret;
>> -
>> -	ret = drm_debugfs_create_files(etnaviv_debugfs_list,
>> -			ARRAY_SIZE(etnaviv_debugfs_list),
>> -			minor->debugfs_root, minor);
>> +	drm_debugfs_create_files(etnaviv_debugfs_list,
>> +				 ARRAY_SIZE(etnaviv_debugfs_list),
>> +				 minor->debugfs_root, minor);
>>
>> -	if (ret) {
>> -		dev_err(dev->dev, "could not install
>> etnaviv_debugfs_list\n");
>> -		return ret;
>> -	}
>> -
>> -	return ret;
>> +	return 0;
>>  }
>>  #endif
>>
>
>

Re: [PATCH] drm/arc: make arcpgu_debugfs_init return 0

[Greg KH]

On Tue, Feb 18, 2020 at 08:28:12PM +0300, Wambui Karuga wrote:
> As drm_debugfs_create_files should return void, remove its use as the
> return value of arcpgu_debugfs_init and have the latter function
> return 0 directly.
> 
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> ---
>  drivers/gpu/drm/arc/arcpgu_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

What order does all of these patches apply in?

You have two of them numbered, but not all, so this is really confusing
as to how to review this...

Can you fix all that up and resend?

thanks,

greg k-h

Re: [PATCH] drm/arc: make arcpgu_debugfs_init return 0

[Daniel Vetter]

On Tue, Feb 18, 2020 at 6:28 PM Wambui Karuga <wambui.karugax@gmail.com> wrote:
>
> As drm_debugfs_create_files should return void, remove its use as the
> return value of arcpgu_debugfs_init and have the latter function
> return 0 directly.
>
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> ---
>  drivers/gpu/drm/arc/arcpgu_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> index d6a6692db0ac..660b25f9588e 100644
> --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> @@ -139,8 +139,10 @@ static struct drm_info_list arcpgu_debugfs_list[] = {
>
>  static int arcpgu_debugfs_init(struct drm_minor *minor)
>  {
> -       return drm_debugfs_create_files(arcpgu_debugfs_list,
> -               ARRAY_SIZE(arcpgu_debugfs_list), minor->debugfs_root, minor);
> +       drm_debugfs_create_files(arcpgu_debugfs_list,
> +                                ARRAY_SIZE(arcpgu_debugfs_list),
> +                                minor->debugfs_root, minor);
> +       return 0;

For cases like these I think we should go a step further and also
remove the return value from the driver wrapper. And that all the way
up until there's something that actually needs to return a value for
some other reason than debugfs.
-Daniel

>  }
>  #endif
>
> --
> 2.25.0
>


-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/2] drm/debugfs: remove checks for return value of drm_debugfs functions.

[Daniel Vetter]

On Tue, Feb 18, 2020 at 08:28:14PM +0300, Wambui Karuga wrote:
> As there is no need to check the return value of
> drm_debugfs_create_files, remove unnecessary checks and error handling
> statement blocks.
> 
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>

I'd split this up a bit differently, with a patch per function. But then
including in that patch the prototype change (at least for the
drm-internal ones that aren't used by drivers). This way reviewers don't
have to check the entire series to make sure that we drop the now
pointeless return value from the implementation of these functions.

I guess you could also just squash this function into the last one, it's a
fairly small patch still.
-Daniel

> ---
>  drivers/gpu/drm/drm_debugfs.c | 28 +++++-----------------------
>  1 file changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 4e673d318503..6a2f141b6a38 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -215,35 +215,17 @@ int drm_debugfs_init(struct drm_minor *minor, int minor_id,
>  	sprintf(name, "%d", minor_id);
>  	minor->debugfs_root = debugfs_create_dir(name, root);
>  
> -	ret = drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
> -				       minor->debugfs_root, minor);
> -	if (ret) {
> -		debugfs_remove(minor->debugfs_root);
> -		minor->debugfs_root = NULL;
> -		DRM_ERROR("Failed to create core drm debugfs files\n");
> -		return ret;
> -	}
> +	drm_debugfs_create_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES,
> +				 minor->debugfs_root, minor);
>  
>  	if (drm_drv_uses_atomic_modeset(dev)) {
> -		ret = drm_atomic_debugfs_init(minor);
> -		if (ret) {
> -			DRM_ERROR("Failed to create atomic debugfs files\n");
> -			return ret;
> -		}
> +		drm_atomic_debugfs_init(minor);
>  	}
>  
>  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> -		ret = drm_framebuffer_debugfs_init(minor);
> -		if (ret) {
> -			DRM_ERROR("Failed to create framebuffer debugfs file\n");
> -			return ret;
> -		}
> +		drm_framebuffer_debugfs_init(minor);
>  
> -		ret = drm_client_debugfs_init(minor);
> -		if (ret) {
> -			DRM_ERROR("Failed to create client debugfs file\n");
> -			return ret;
> -		}
> +		drm_client_debugfs_init(minor);
>  	}
>  
>  	if (dev->driver->debugfs_init) {
> -- 
> 2.25.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH] drm/arc: make arcpgu_debugfs_init return 0

[Daniel Vetter]

On Tue, Feb 18, 2020 at 08:00:30PM +0100, Daniel Vetter wrote:
> On Tue, Feb 18, 2020 at 6:28 PM Wambui Karuga <wambui.karugax@gmail.com> wrote:
> >
> > As drm_debugfs_create_files should return void, remove its use as the
> > return value of arcpgu_debugfs_init and have the latter function
> > return 0 directly.
> >
> > Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> > ---
> >  drivers/gpu/drm/arc/arcpgu_drv.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
> > index d6a6692db0ac..660b25f9588e 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_drv.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_drv.c
> > @@ -139,8 +139,10 @@ static struct drm_info_list arcpgu_debugfs_list[] = {
> >
> >  static int arcpgu_debugfs_init(struct drm_minor *minor)
> >  {
> > -       return drm_debugfs_create_files(arcpgu_debugfs_list,
> > -               ARRAY_SIZE(arcpgu_debugfs_list), minor->debugfs_root, minor);
> > +       drm_debugfs_create_files(arcpgu_debugfs_list,
> > +                                ARRAY_SIZE(arcpgu_debugfs_list),
> > +                                minor->debugfs_root, minor);
> > +       return 0;
> 
> For cases like these I think we should go a step further and also
> remove the return value from the driver wrapper. And that all the way
> up until there's something that actually needs to return a value for
> some other reason than debugfs.

This ofc applies to almost all the driver patches for this series, most
can be cleaned up quite a bit more.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [Nouveau] [PATCH] drm/nouveau: remove checks for return value of debugfs functions

[Ben Skeggs]

On Wed, 19 Feb 2020 at 03:28, Wambui Karuga <wambui.karugax@gmail.com> wrote:
>
> As there is no need to check for the return value of debugfs_create_file
> and drm_debugfs_create_files, remove unnecessary checks and error
> handling in nouveau_drm_debugfs_init.
Thanks!

>
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> ---
>  drivers/gpu/drm/nouveau/nouveau_debugfs.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> index 7dfbbbc1beea..15a3d40edf02 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
> @@ -222,22 +222,18 @@ nouveau_drm_debugfs_init(struct drm_minor *minor)
>  {
>         struct nouveau_drm *drm = nouveau_drm(minor->dev);
>         struct dentry *dentry;
> -       int i, ret;
> +       int i;
>
>         for (i = 0; i < ARRAY_SIZE(nouveau_debugfs_files); i++) {
> -               dentry = debugfs_create_file(nouveau_debugfs_files[i].name,
> -                                            S_IRUGO | S_IWUSR,
> -                                            minor->debugfs_root, minor->dev,
> -                                            nouveau_debugfs_files[i].fops);
> -               if (!dentry)
> -                       return -ENOMEM;
> +               debugfs_create_file(nouveau_debugfs_files[i].name,
> +                                   S_IRUGO | S_IWUSR,
> +                                   minor->debugfs_root, minor->dev,
> +                                   nouveau_debugfs_files[i].fops);
>         }
>
> -       ret = drm_debugfs_create_files(nouveau_debugfs_list,
> -                                      NOUVEAU_DEBUGFS_ENTRIES,
> -                                      minor->debugfs_root, minor);
> -       if (ret)
> -               return ret;
> +       drm_debugfs_create_files(nouveau_debugfs_list,
> +                                NOUVEAU_DEBUGFS_ENTRIES,
> +                                minor->debugfs_root, minor);
>
>         /* Set the size of the vbios since we know it, and it's confusing to
>          * userspace if it wants to seek() but the file has a length of 0
> --
> 2.25.0
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH] drm/vram-helper: make drm_vram_mm_debugfs_init return 0

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1637 bytes --]



Am 18.02.20 um 18:28 schrieb Wambui Karuga:
> As drm_debugfs_create_files() should return 0, remove its use as the
> return value of drm_vram_mm_debugfs_init(), and have the function return
> 0 directly.
> 
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>

Acked-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 92a11bb42365..77b36a2286f9 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -1043,19 +1043,16 @@ static const struct drm_info_list drm_vram_mm_debugfs_list[] = {
>   * @minor: drm minor device.
>   *
>   * Returns:
> - * 0 on success, or
> - * a negative error code otherwise.
> + * 0
>   */
>  int drm_vram_mm_debugfs_init(struct drm_minor *minor)
>  {
> -	int ret = 0;
> -
>  #if defined(CONFIG_DEBUG_FS)
> -	ret = drm_debugfs_create_files(drm_vram_mm_debugfs_list,
> -				       ARRAY_SIZE(drm_vram_mm_debugfs_list),
> -				       minor->debugfs_root, minor);
> +	drm_debugfs_create_files(drm_vram_mm_debugfs_list,
> +				 ARRAY_SIZE(drm_vram_mm_debugfs_list),
> +				 minor->debugfs_root, minor);
>  #endif
> -	return ret;
> +	return 0;
>  }
>  EXPORT_SYMBOL(drm_vram_mm_debugfs_init);
>  
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] drm/v3d: make v3d_debugfs_init return 0

[Eric Anholt]

On Tue, Feb 18, 2020 at 9:28 AM Wambui Karuga <wambui.karugax@gmail.com> wrote:
>
> As drm_debugfs_create_files should return void, remove its use as the
> return value of v3d_debugfs_init and have the function return 0
> directly.

If we're going this route, then this function should be returning void, too.

>
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> ---
>  drivers/gpu/drm/v3d/v3d_debugfs.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_debugfs.c b/drivers/gpu/drm/v3d/v3d_debugfs.c
> index 9e953ce64ef7..57dded6a3957 100644
> --- a/drivers/gpu/drm/v3d/v3d_debugfs.c
> +++ b/drivers/gpu/drm/v3d/v3d_debugfs.c
> @@ -261,7 +261,8 @@ static const struct drm_info_list v3d_debugfs_list[] = {
>  int
>  v3d_debugfs_init(struct drm_minor *minor)
>  {
> -       return drm_debugfs_create_files(v3d_debugfs_list,
> -                                       ARRAY_SIZE(v3d_debugfs_list),
> -                                       minor->debugfs_root, minor);
> +       drm_debugfs_create_files(v3d_debugfs_list,
> +                                ARRAY_SIZE(v3d_debugfs_list),
> +                                minor->debugfs_root, minor);
> +       return 0;
>  }
> --
> 2.25.0
>

Re: [PATCH] drm/arm: make hdlcd_debugfs_init return 0

[Liviu Dudau]

Hi,

On Tue, Feb 18, 2020 at 08:28:13PM +0300, Wambui Karuga wrote:
> As drm_debugfs_create_files should return void, remove its use as a
> return value in hdlcd_debugfs_init and have the latter function return 0
> directly.
> 
> Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com>
> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 2e053815b54a..bd0ad6f46a97 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -226,8 +226,10 @@ static struct drm_info_list hdlcd_debugfs_list[] = {
>  
>  static int hdlcd_debugfs_init(struct drm_minor *minor)
>  {
> -	return drm_debugfs_create_files(hdlcd_debugfs_list,
> -		ARRAY_SIZE(hdlcd_debugfs_list),	minor->debugfs_root, minor);
> +	drm_debugfs_create_files(hdlcd_debugfs_list,
> +				 ARRAY_SIZE(hdlcd_debugfs_list),
> +				 minor->debugfs_root, minor);
> +	return 0;
>  }
>  #endif
>
> -- 
> 2.25.0
>

Thanks for your patch! I had to go into the ML and find out the series where this
patch belongs to and seen that you have already received some feedback, but I think
a summary is worth:

- you should look if it is possible to make .debugfs_init hook return void, it would
  simplify the cleanup in the drivers by not having to return 0.
- you should put in each driver patch a note that drm_debugfs_create_files() always
  returns 0 since 987d65d01356 (drm: debugfs: make drm_debugfs_create_files() never fail)
  so that people don't have to hunt in the git history for clues.
- Make all the changes into a series

With that, you can have my Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Best regards,
Liviu

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯