Re: [PATCH] drm/fb-helper: Use -errno return in restore_mode_unlocked

Rob Clark
2015-08-25T19:20:02Z

On Tue, Aug 25, 2015 at 11:20 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Using bool and returning true upon error is very uncommon. Also an int
> return value is actually what all the callers which did check it seem
> to have expected.
>
> v2: Restore hunk misplaced in a rebase, spotted by Rob.
>
> Cc: Rob Clark <robdclark@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Reviewed-by: Rob Clark <robdclark@gmail.com>

> ---
> drivers/gpu/drm/drm_fb_helper.c | 19 +++++++++++--------
> include/drm/drm_fb_helper.h | 6 +++---
> 2 files changed, 14 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 418d299f3b12..859134e0d72d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -320,11 +320,10 @@ int drm_fb_helper_debug_leave(struct fb_info *info)
> }
> EXPORT_SYMBOL(drm_fb_helper_debug_leave);
>
> -static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> +static int restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> struct drm_plane *plane;
> - bool error = false;
> int i;
>
> drm_warn_on_modeset_not_all_locked(dev);
> @@ -348,14 +347,15 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> if (crtc->funcs->cursor_set) {
> ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0);
> if (ret)
> - error = true;
> + return ret;
> }
>
> ret = drm_mode_set_config_internal(mode_set);
> if (ret)
> - error = true;
> + return ret;
> }
> - return error;
> +
> + return 0;
> }
>
> /**
> @@ -365,12 +365,15 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper)
> * This should be called from driver's drm ->lastclose callback
> * when implementing an fbcon on top of kms using this helper. This ensures that
> * the user isn't greeted with a black screen when e.g. X dies.
> + *
> + * RETURNS:
> + * Zero if everything went ok, negative error code otherwise.
> */
> -bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> +int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> {
> struct drm_device *dev = fb_helper->dev;
> - bool ret;
> - bool do_delayed = false;
> + bool do_delayed;
> + int ret;
>
> drm_modeset_lock_all(dev);
> ret = restore_fbdev_mode(fb_helper);
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index dbab4622b58f..67de1f10008e 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -136,7 +136,7 @@ int drm_fb_helper_set_par(struct fb_info *info);
> int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
> struct fb_info *info);
>
> -bool drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper);
> +int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper);
>
> struct fb_info *drm_fb_helper_alloc_fbi(struct drm_fb_helper *fb_helper);
> void drm_fb_helper_unregister_fbi(struct drm_fb_helper *fb_helper);
> @@ -226,10 +226,10 @@ static inline int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
> return 0;
> }
>
> -static inline bool
> +static inline int
> drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper)
> {
> - return true;
> + return 0;
> }
>
> static inline struct fb_info *
> --
> 1.8.3.1