diff mbox series

[v2,02/16] backlight: refactor fb_notifier_callback()

Message ID 20200517190139.740249-3-sam@ravnborg.org (mailing list archive)
State New, archived
Headers show
Series backlight updates | expand

Commit Message

Sam Ravnborg May 17, 2020, 7:01 p.m. UTC
Increase readability of fb_notifier_callback() by removing
a few indent levels.
No functional change.

Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
Cc: Lee Jones <lee.jones@linaro.org>
Cc: Daniel Thompson <daniel.thompson@linaro.org>
Cc: Jingoo Han <jingoohan1@gmail.com>
---
 drivers/video/backlight/backlight.c | 43 +++++++++++++++--------------
 1 file changed, 22 insertions(+), 21 deletions(-)

Comments

Daniel Thompson May 18, 2020, 2:54 p.m. UTC | #1
On Sun, May 17, 2020 at 09:01:25PM +0200, Sam Ravnborg wrote:
> Increase readability of fb_notifier_callback() by removing
> a few indent levels.
> No functional change.
> 
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>

Reviewed-by: Daniel Thompson <daniel.thompson@linaro.org>


> ---
>  drivers/video/backlight/backlight.c | 43 +++++++++++++++--------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index cac3e35d7630..17f04cff50ab 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -58,28 +58,29 @@ static int fb_notifier_callback(struct notifier_block *self,
>  
>  	bd = container_of(self, struct backlight_device, fb_notif);
>  	mutex_lock(&bd->ops_lock);
> -	if (bd->ops)
> -		if (!bd->ops->check_fb ||
> -		    bd->ops->check_fb(bd, evdata->info)) {
> -			fb_blank = *(int *)evdata->data;
> -			if (fb_blank == FB_BLANK_UNBLANK &&
> -			    !bd->fb_bl_on[node]) {
> -				bd->fb_bl_on[node] = true;
> -				if (!bd->use_count++) {
> -					bd->props.state &= ~BL_CORE_FBBLANK;
> -					bd->props.fb_blank = FB_BLANK_UNBLANK;
> -					backlight_update_status(bd);
> -				}
> -			} else if (fb_blank != FB_BLANK_UNBLANK &&
> -				   bd->fb_bl_on[node]) {
> -				bd->fb_bl_on[node] = false;
> -				if (!(--bd->use_count)) {
> -					bd->props.state |= BL_CORE_FBBLANK;
> -					bd->props.fb_blank = fb_blank;
> -					backlight_update_status(bd);
> -				}
> -			}
> +
> +	if (!bd->ops)
> +		goto out;
> +	if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
> +		goto out;
> +
> +	fb_blank = *(int *)evdata->data;
> +	if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
> +		bd->fb_bl_on[node] = true;
> +		if (!bd->use_count++) {
> +			bd->props.state &= ~BL_CORE_FBBLANK;
> +			bd->props.fb_blank = FB_BLANK_UNBLANK;
> +			backlight_update_status(bd);
> +		}
> +	} else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
> +		bd->fb_bl_on[node] = false;
> +		if (!(--bd->use_count)) {
> +			bd->props.state |= BL_CORE_FBBLANK;
> +			bd->props.fb_blank = fb_blank;
> +			backlight_update_status(bd);
>  		}
> +	}
> +out:
>  	mutex_unlock(&bd->ops_lock);
>  	return 0;
>  }
> -- 
> 2.25.1
>
Emil Velikov May 20, 2020, 10:41 a.m. UTC | #2
Hi Sam,

On Sun, 17 May 2020 at 20:02, Sam Ravnborg <sam@ravnborg.org> wrote:
>
> Increase readability of fb_notifier_callback() by removing
> a few indent levels.
> No functional change.
>
> Signed-off-by: Sam Ravnborg <sam@ravnborg.org>
> Cc: Lee Jones <lee.jones@linaro.org>
> Cc: Daniel Thompson <daniel.thompson@linaro.org>
> Cc: Jingoo Han <jingoohan1@gmail.com>
> ---
>  drivers/video/backlight/backlight.c | 43 +++++++++++++++--------------
>  1 file changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index cac3e35d7630..17f04cff50ab 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -58,28 +58,29 @@ static int fb_notifier_callback(struct notifier_block *self,
>
>         bd = container_of(self, struct backlight_device, fb_notif);
>         mutex_lock(&bd->ops_lock);
> -       if (bd->ops)
> -               if (!bd->ops->check_fb ||
> -                   bd->ops->check_fb(bd, evdata->info)) {
> -                       fb_blank = *(int *)evdata->data;
> -                       if (fb_blank == FB_BLANK_UNBLANK &&
> -                           !bd->fb_bl_on[node]) {
> -                               bd->fb_bl_on[node] = true;
> -                               if (!bd->use_count++) {
> -                                       bd->props.state &= ~BL_CORE_FBBLANK;
> -                                       bd->props.fb_blank = FB_BLANK_UNBLANK;
> -                                       backlight_update_status(bd);
> -                               }
> -                       } else if (fb_blank != FB_BLANK_UNBLANK &&
> -                                  bd->fb_bl_on[node]) {
> -                               bd->fb_bl_on[node] = false;
> -                               if (!(--bd->use_count)) {
> -                                       bd->props.state |= BL_CORE_FBBLANK;
> -                                       bd->props.fb_blank = fb_blank;
> -                                       backlight_update_status(bd);
> -                               }
> -                       }
> +
> +       if (!bd->ops)
> +               goto out;
> +       if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
Mildly related: Would be a nice to define which ops are mandatory and
which aren't.
That plus enforcement in backlight_device_register.

But that's for another patchset.

> +               goto out;
> +
> +       fb_blank = *(int *)evdata->data;
> +       if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
> +               bd->fb_bl_on[node] = true;
> +               if (!bd->use_count++) {
> +                       bd->props.state &= ~BL_CORE_FBBLANK;
> +                       bd->props.fb_blank = FB_BLANK_UNBLANK;
> +                       backlight_update_status(bd);
> +               }
> +       } else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
> +               bd->fb_bl_on[node] = false;
> +               if (!(--bd->use_count)) {
> +                       bd->props.state |= BL_CORE_FBBLANK;
> +                       bd->props.fb_blank = fb_blank;
> +                       backlight_update_status(bd);
>                 }
Something like the following reads better, plus one could simplify it
with follow-on patch.

if (fb_blank == FB_BLANK_UNBLANK)
    if (!bd->fb_bl_on[node] && !bd->use_count++) {
        bd->props.state &= ~BL_CORE_FBBLANK;
        bd->props.fb_blank = FB_BLANK_UNBLANK;
        backlight_update_status(bd);
        // above is backlight_enable()
    }
    bd->fb_bl_on[node] = true;
} else {
    if (bd->fb_bl_on[node] && !(--bd->use_count)) {
        bd->props.state |= BL_CORE_FBBLANK;
        bd->props.fb_blank = fb_blank;
        backlight_update_status(bd);
        // above is backlight_disable()
   }
    bd->fb_bl_on[node] = false;
}

As-is, one cannot use the backlight helpers indicated, since it
touches .power. First one should ensure the drivers honour .power - by
using the helper introduced later.

-Emil
diff mbox series

Patch

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index cac3e35d7630..17f04cff50ab 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -58,28 +58,29 @@  static int fb_notifier_callback(struct notifier_block *self,
 
 	bd = container_of(self, struct backlight_device, fb_notif);
 	mutex_lock(&bd->ops_lock);
-	if (bd->ops)
-		if (!bd->ops->check_fb ||
-		    bd->ops->check_fb(bd, evdata->info)) {
-			fb_blank = *(int *)evdata->data;
-			if (fb_blank == FB_BLANK_UNBLANK &&
-			    !bd->fb_bl_on[node]) {
-				bd->fb_bl_on[node] = true;
-				if (!bd->use_count++) {
-					bd->props.state &= ~BL_CORE_FBBLANK;
-					bd->props.fb_blank = FB_BLANK_UNBLANK;
-					backlight_update_status(bd);
-				}
-			} else if (fb_blank != FB_BLANK_UNBLANK &&
-				   bd->fb_bl_on[node]) {
-				bd->fb_bl_on[node] = false;
-				if (!(--bd->use_count)) {
-					bd->props.state |= BL_CORE_FBBLANK;
-					bd->props.fb_blank = fb_blank;
-					backlight_update_status(bd);
-				}
-			}
+
+	if (!bd->ops)
+		goto out;
+	if (bd->ops->check_fb && !bd->ops->check_fb(bd, evdata->info))
+		goto out;
+
+	fb_blank = *(int *)evdata->data;
+	if (fb_blank == FB_BLANK_UNBLANK && !bd->fb_bl_on[node]) {
+		bd->fb_bl_on[node] = true;
+		if (!bd->use_count++) {
+			bd->props.state &= ~BL_CORE_FBBLANK;
+			bd->props.fb_blank = FB_BLANK_UNBLANK;
+			backlight_update_status(bd);
+		}
+	} else if (fb_blank != FB_BLANK_UNBLANK && bd->fb_bl_on[node]) {
+		bd->fb_bl_on[node] = false;
+		if (!(--bd->use_count)) {
+			bd->props.state |= BL_CORE_FBBLANK;
+			bd->props.fb_blank = fb_blank;
+			backlight_update_status(bd);
 		}
+	}
+out:
 	mutex_unlock(&bd->ops_lock);
 	return 0;
 }