| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20200227144558.GW2363188@phenom.ffwll.local>
Date: Thu, 27 Feb 2020 15:45:58 +0100
From: Daniel Vetter <daniel@...ll.ch>
To: Wambui Karuga <wambui.karugax@...il.com>
Cc: daniel@...ll.ch, airlied@...ux.ie, Rob Clark <robdclark@...il.com>,
Sean Paul <sean@...rly.run>, linux-kernel@...r.kernel.org,
gregkh@...uxfoundation.org, dri-devel@...ts.freedesktop.org,
linux-arm-msm@...r.kernel.org, freedreno@...ts.freedesktop.org
Subject: Re: [PATCH 12/21] drm/msm: remove checks for return value of
drm_debugfs functions.
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@...il.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
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Powered by blists - more mailing lists