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
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
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
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) {
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
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
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 installetnaviv_debugfs_list\n");
return ret;- }
- return ret;
- return 0;
} #endif
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 installetnaviv_debugfs_list\n");
return ret;- }
- return ret;
- return 0;
} #endif
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
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
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
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; }
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
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;
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);
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);
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
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
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
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
dri-devel@lists.freedesktop.org
-
Ben Skeggs -
Daniel Vetter -
Eric Anholt -
Greg KH -
Liviu Dudau -
Lucas Stach -
Thomas Zimmermann -
Wambui Karuga