Hi Sam,
On Sun, 17 May 2020 at 20:02, Sam Ravnborg <sam@xxxxxxxxxxxx> wrote:
>
> Increase readability of fb_notifier_callback() by removing
> a few indent levels.
> No functional change.
>
> Signed-off-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> Cc: Lee Jones <lee.jones@xxxxxxxxxx>
> Cc: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> Cc: Jingoo Han <jingoohan1@xxxxxxxxx>
> ---
> 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
