drm/fb-helper: Use -errno return in restore_mode_unlocked
diff mbox

Message ID 1440516028-11127-1-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Aug. 25, 2015, 3:20 p.m. UTC
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>
---
 drivers/gpu/drm/drm_fb_helper.c | 19 +++++++++++--------
 include/drm/drm_fb_helper.h     |  6 +++---
 2 files changed, 14 insertions(+), 11 deletions(-)

Comments

Rob Clark Aug. 25, 2015, 7:20 p.m. UTC | #1
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
>
Daniel Vetter Aug. 26, 2015, 11:36 a.m. UTC | #2
On Tue, Aug 25, 2015 at 03:20:02PM -0400, Rob Clark wrote:
> 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>

Merged the first 2 patches from this series to drm-misc, thanks for your
review.
-Daniel

> 
> 
> > ---
> >  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
> >

Patch
diff mbox

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 *