| Message ID | 20200227120232.19413-13-wambui.karugax@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Thu, Feb 27, 2020 at 03:02:23PM +0300, Wambui Karuga wrote: > Since commit 987d65d01356 (drm: debugfs: make > drm_debugfs_create_files() never fail), drm_debugfs_create_files() does > not fail, and should return void. This change therefore removes the > checks of its return value in drm/msm and subsequent error handling. > > These changes also enable the changing of various debugfs_init() > functions to return void. > > Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com> I think msm_kms_funcs->debugfs_init could also be made to return void. But that's quite a bit more involved so doesn't make much sense to do that as part of this patch series. Also the debug/core_perf/ files look kinda funny, if I'd bet this is used by the qualcomm hwc somewhere to make it's decisions :-) That's at least what's been the case everywhere else I spotted something like that. -Daniel > --- > drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 18 +++++------------- > drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 +++----------- > drivers/gpu/drm/msm/msm_debugfs.c | 21 ++++++--------------- > drivers/gpu/drm/msm/msm_debugfs.h | 2 +- > drivers/gpu/drm/msm/msm_gpu.h | 2 +- > 5 files changed, 16 insertions(+), 41 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c > index 075ecce4b5e0..8cae2ca4af6b 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c > @@ -148,27 +148,19 @@ reset_set(void *data, u64 val) > DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set, "%llx\n"); > > > -int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor) > +void a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor) > { > struct drm_device *dev; > - int ret; > > if (!minor) > - return 0; > + return; > > dev = minor->dev; > > - ret = drm_debugfs_create_files(a5xx_debugfs_list, > - ARRAY_SIZE(a5xx_debugfs_list), > - minor->debugfs_root, minor); > - > - if (ret) { > - DRM_DEV_ERROR(dev->dev, "could not install a5xx_debugfs_list\n"); > - return ret; > - } > + drm_debugfs_create_files(a5xx_debugfs_list, > + ARRAY_SIZE(a5xx_debugfs_list), > + minor->debugfs_root, minor); > > debugfs_create_file("reset", S_IWUGO, minor->debugfs_root, dev, > &reset_fops); > - > - return 0; > } > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > index 6650f478b226..41b461128bbc 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c > @@ -259,17 +259,9 @@ static struct drm_info_list mdp5_debugfs_list[] = { > > static int mdp5_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor) > { > - struct drm_device *dev = minor->dev; > - int ret; > - > - ret = drm_debugfs_create_files(mdp5_debugfs_list, > - ARRAY_SIZE(mdp5_debugfs_list), > - minor->debugfs_root, minor); > - > - if (ret) { > - DRM_DEV_ERROR(dev->dev, "could not install mdp5_debugfs_list\n"); > - return ret; > - } > + drm_debugfs_create_files(mdp5_debugfs_list, > + ARRAY_SIZE(mdp5_debugfs_list), > + minor->debugfs_root, minor); > > return 0; > } > diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c > index 1c74381a4fc9..3c958f311bbc 100644 > --- a/drivers/gpu/drm/msm/msm_debugfs.c > +++ b/drivers/gpu/drm/msm/msm_debugfs.c > @@ -214,31 +214,22 @@ int msm_debugfs_late_init(struct drm_device *dev) > return ret; > } > > -int msm_debugfs_init(struct drm_minor *minor) > +void msm_debugfs_init(struct drm_minor *minor) > { > struct drm_device *dev = minor->dev; > struct msm_drm_private *priv = dev->dev_private; > - int ret; > + int ret = 0; > > - ret = drm_debugfs_create_files(msm_debugfs_list, > - ARRAY_SIZE(msm_debugfs_list), > - minor->debugfs_root, minor); > - > - if (ret) { > - DRM_DEV_ERROR(dev->dev, "could not install msm_debugfs_list\n"); > - return ret; > - } > + drm_debugfs_create_files(msm_debugfs_list, > + ARRAY_SIZE(msm_debugfs_list), > + minor->debugfs_root, minor); > > debugfs_create_file("gpu", S_IRUSR, minor->debugfs_root, > dev, &msm_gpu_fops); > > if (priv->kms && priv->kms->funcs->debugfs_init) { > - ret = priv->kms->funcs->debugfs_init(priv->kms, minor); > - if (ret) > - return ret; > + priv->kms->funcs->debugfs_init(priv->kms, minor); > } > - > - return ret; > } > #endif > > diff --git a/drivers/gpu/drm/msm/msm_debugfs.h b/drivers/gpu/drm/msm/msm_debugfs.h > index 2b91f8c178ad..ef58f66abbb3 100644 > --- a/drivers/gpu/drm/msm/msm_debugfs.h > +++ b/drivers/gpu/drm/msm/msm_debugfs.h > @@ -8,7 +8,7 @@ > #define __MSM_DEBUGFS_H__ > > #ifdef CONFIG_DEBUG_FS > -int msm_debugfs_init(struct drm_minor *minor); > +void msm_debugfs_init(struct drm_minor *minor); > #endif > > #endif /* __MSM_DEBUGFS_H__ */ > diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h > index be5bc2e8425c..6ccae4ba905c 100644 > --- a/drivers/gpu/drm/msm/msm_gpu.h > +++ b/drivers/gpu/drm/msm/msm_gpu.h > @@ -57,7 +57,7 @@ struct msm_gpu_funcs { > void (*show)(struct msm_gpu *gpu, struct msm_gpu_state *state, > struct drm_printer *p); > /* for generation specific debugfs: */ > - int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor); > + void (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor); > #endif > unsigned long (*gpu_busy)(struct msm_gpu *gpu); > struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu); > -- > 2.25.0 >
Hi Wambui,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on drm-intel/for-linux-next]
[also build test ERROR on linus/master v5.6-rc3 next-20200227]
[cannot apply to tegra/for-next anholt/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Wambui-Karuga/drm-subsytem-wide-debugfs-functions-refactor/20200228-102633
base: git://anongit.freedesktop.org/drm-intel for-linux-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=arm64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> drivers/gpu/drm/msm/adreno/a5xx_gpu.c:1437:19: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
.debugfs_init = a5xx_debugfs_init,
^~~~~~~~~~~~~~~~~
drivers/gpu/drm/msm/adreno/a5xx_gpu.c:1437:19: note: (near initialization for 'funcs.base.debugfs_init')
cc1: some warnings being treated as errors
--
>> drivers/gpu/drm/msm/adreno/a5xx_debugfs.c:151:6: error: conflicting types for 'a5xx_debugfs_init'
void a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
^~~~~~~~~~~~~~~~~
In file included from drivers/gpu/drm/msm/adreno/a5xx_debugfs.c:12:0:
drivers/gpu/drm/msm/adreno/a5xx_gpu.h:44:5: note: previous declaration of 'a5xx_debugfs_init' was here
int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor);
^~~~~~~~~~~~~~~~~
vim +/a5xx_debugfs_init +151 drivers/gpu/drm/msm/adreno/a5xx_debugfs.c
149
150
> 151 void a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor)
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c index 075ecce4b5e0..8cae2ca4af6b 100644 --- a/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c +++ b/drivers/gpu/drm/msm/adreno/a5xx_debugfs.c @@ -148,27 +148,19 @@ reset_set(void *data, u64 val) DEFINE_SIMPLE_ATTRIBUTE(reset_fops, NULL, reset_set, "%llx\n"); -int a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor) +void a5xx_debugfs_init(struct msm_gpu *gpu, struct drm_minor *minor) { struct drm_device *dev; - int ret; if (!minor) - return 0; + return; dev = minor->dev; - ret = drm_debugfs_create_files(a5xx_debugfs_list, - ARRAY_SIZE(a5xx_debugfs_list), - minor->debugfs_root, minor); - - if (ret) { - DRM_DEV_ERROR(dev->dev, "could not install a5xx_debugfs_list\n"); - return ret; - } + drm_debugfs_create_files(a5xx_debugfs_list, + ARRAY_SIZE(a5xx_debugfs_list), + minor->debugfs_root, minor); debugfs_create_file("reset", S_IWUGO, minor->debugfs_root, dev, &reset_fops); - - return 0; } diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c index 6650f478b226..41b461128bbc 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c @@ -259,17 +259,9 @@ static struct drm_info_list mdp5_debugfs_list[] = { static int mdp5_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor) { - struct drm_device *dev = minor->dev; - int ret; - - ret = drm_debugfs_create_files(mdp5_debugfs_list, - ARRAY_SIZE(mdp5_debugfs_list), - minor->debugfs_root, minor); - - if (ret) { - DRM_DEV_ERROR(dev->dev, "could not install mdp5_debugfs_list\n"); - return ret; - } + drm_debugfs_create_files(mdp5_debugfs_list, + ARRAY_SIZE(mdp5_debugfs_list), + minor->debugfs_root, minor); return 0; } diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c index 1c74381a4fc9..3c958f311bbc 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.c +++ b/drivers/gpu/drm/msm/msm_debugfs.c @@ -214,31 +214,22 @@ int msm_debugfs_late_init(struct drm_device *dev) return ret; } -int msm_debugfs_init(struct drm_minor *minor) +void msm_debugfs_init(struct drm_minor *minor) { struct drm_device *dev = minor->dev; struct msm_drm_private *priv = dev->dev_private; - int ret; + int ret = 0; - ret = drm_debugfs_create_files(msm_debugfs_list, - ARRAY_SIZE(msm_debugfs_list), - minor->debugfs_root, minor); - - if (ret) { - DRM_DEV_ERROR(dev->dev, "could not install msm_debugfs_list\n"); - return ret; - } + drm_debugfs_create_files(msm_debugfs_list, + ARRAY_SIZE(msm_debugfs_list), + minor->debugfs_root, minor); debugfs_create_file("gpu", S_IRUSR, minor->debugfs_root, dev, &msm_gpu_fops); if (priv->kms && priv->kms->funcs->debugfs_init) { - ret = priv->kms->funcs->debugfs_init(priv->kms, minor); - if (ret) - return ret; + priv->kms->funcs->debugfs_init(priv->kms, minor); } - - return ret; } #endif diff --git a/drivers/gpu/drm/msm/msm_debugfs.h b/drivers/gpu/drm/msm/msm_debugfs.h index 2b91f8c178ad..ef58f66abbb3 100644 --- a/drivers/gpu/drm/msm/msm_debugfs.h +++ b/drivers/gpu/drm/msm/msm_debugfs.h @@ -8,7 +8,7 @@ #define __MSM_DEBUGFS_H__ #ifdef CONFIG_DEBUG_FS -int msm_debugfs_init(struct drm_minor *minor); +void msm_debugfs_init(struct drm_minor *minor); #endif #endif /* __MSM_DEBUGFS_H__ */ diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h index be5bc2e8425c..6ccae4ba905c 100644 --- a/drivers/gpu/drm/msm/msm_gpu.h +++ b/drivers/gpu/drm/msm/msm_gpu.h @@ -57,7 +57,7 @@ struct msm_gpu_funcs { void (*show)(struct msm_gpu *gpu, struct msm_gpu_state *state, struct drm_printer *p); /* for generation specific debugfs: */ - int (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor); + void (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor); #endif unsigned long (*gpu_busy)(struct msm_gpu *gpu); struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);
Since commit 987d65d01356 (drm: debugfs: make drm_debugfs_create_files() never fail), drm_debugfs_create_files() does not fail, and should return void. This change therefore removes the checks of its return value in drm/msm and subsequent error handling. These changes also enable the changing of various debugfs_init() functions to return void. Signed-off-by: Wambui Karuga <wambui.karugax@gmail.com> --- drivers/gpu/drm/msm/adreno/a5xx_debugfs.c | 18 +++++------------- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 14 +++----------- drivers/gpu/drm/msm/msm_debugfs.c | 21 ++++++--------------- drivers/gpu/drm/msm/msm_debugfs.h | 2 +- drivers/gpu/drm/msm/msm_gpu.h | 2 +- 5 files changed, 16 insertions(+), 41 deletions(-)