linux-kernel - Re: [PATCH 2/2] platform/loongarch: laptop: Support backlight power control

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhV-H7RBcaAP8WjjrX20cvuMixarqyeTLoMPdb8QMztz_648g@mail.gmail.com>
Date: Tue, 3 Jun 2025 12:16:57 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Yao Zi <ziyao@...root.org>
Cc: Jianmin Lv <lvjianmin@...ngson.cn>, WANG Xuerui <kernel@...0n.name>, 
	linux-kernel@...r.kernel.org, loongarch@...ts.linux.dev, 
	Mingcong Bai <jeffbai@...c.io>, Kexy Biscuit <kexybiscuit@...c.io>
Subject: Re: [PATCH 2/2] platform/loongarch: laptop: Support backlight power control

On Sat, May 31, 2025 at 7:39 PM Yao Zi <ziyao@...root.org> wrote:
>
> loongson_laptop_turn_{on,off}_backlight() are designed for controlling
> power of the backlight, but they aren't really used in the driver
> previously.
>
> Unify these two functions since they only differ in arguments passed to
> ACPI method, and wire up loongson_laptop_backlight_update() to update
> power state of the backlight as well. Tested on TongFang L860-T2 3A5000
> laptop.
>
> Signed-off-by: Yao Zi <ziyao@...root.org>
> ---
>  drivers/platform/loongarch/loongson-laptop.c | 53 +++++++-------------
>  1 file changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/platform/loongarch/loongson-laptop.c b/drivers/platform/loongarch/loongson-laptop.c
> index 828bd62e3596..f01e53b1c84d 100644
> --- a/drivers/platform/loongarch/loongson-laptop.c
> +++ b/drivers/platform/loongarch/loongson-laptop.c
> @@ -56,8 +56,6 @@ static struct input_dev *generic_inputdev;
>  static acpi_handle hotkey_handle;
>  static struct key_entry hotkey_keycode_map[GENERIC_HOTKEY_MAP_MAX];
>
> -int loongson_laptop_turn_on_backlight(void);
> -int loongson_laptop_turn_off_backlight(void);
>  static int loongson_laptop_backlight_update(struct backlight_device *bd);
>
>  /* 2. ACPI Helpers and device model */
> @@ -354,6 +352,22 @@ static int ec_backlight_level(u8 level)
>         return level;
>  }
>
> +static int ec_backlight_set_power(bool state)
> +{
> +       int status;
> +       union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> +       struct acpi_object_list args = { 1, &arg0 };
> +
> +       arg0.integer.value = state;
> +       status = acpi_evaluate_object(NULL, "\\BLSW", &args, NULL);
> +       if (ACPI_FAILURE(status)) {
> +               pr_info("Loongson lvds error: 0x%x\n", status);
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}
> +
>  static int loongson_laptop_backlight_update(struct backlight_device *bd)
>  {
>         int lvl = ec_backlight_level(bd->props.brightness);
> @@ -363,6 +377,8 @@ static int loongson_laptop_backlight_update(struct backlight_device *bd)
>         if (ec_set_brightness(lvl))
>                 return -EIO;
>
> +       ec_backlight_set_power(bd->props.power == BACKLIGHT_POWER_ON ? true : false);
It is better to check the status before setting, because the EC
firmware may not be as robust as needed, a checking can reduce
interactions between kernel and EC.

There is an example: dp_aux_backlight_update_status() in
drivers/gpu/drm/display/drm_dp_helper.c.

> +
>         return 0;
>  }
>
> @@ -394,6 +410,7 @@ static int laptop_backlight_register(void)
>
>         props.brightness = ec_get_brightness();
>         props.max_brightness = status;
> +       props.power = BACKLIGHT_POWER_ON;
>         props.type = BACKLIGHT_PLATFORM;
>
>         backlight_device_register("loongson_laptop",
> @@ -402,38 +419,6 @@ static int laptop_backlight_register(void)
>         return 0;
>  }
>
> -int loongson_laptop_turn_on_backlight(void)
> -{
> -       int status;
> -       union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> -       struct acpi_object_list args = { 1, &arg0 };
> -
> -       arg0.integer.value = 1;
> -       status = acpi_evaluate_object(NULL, "\\BLSW", &args, NULL);
> -       if (ACPI_FAILURE(status)) {
> -               pr_info("Loongson lvds error: 0x%x\n", status);
> -               return -ENODEV;
> -       }
> -
> -       return 0;
> -}
> -
> -int loongson_laptop_turn_off_backlight(void)
> -{
> -       int status;
> -       union acpi_object arg0 = { ACPI_TYPE_INTEGER };
> -       struct acpi_object_list args = { 1, &arg0 };
> -
> -       arg0.integer.value = 0;
> -       status = acpi_evaluate_object(NULL, "\\BLSW", &args, NULL);
> -       if (ACPI_FAILURE(status)) {
> -               pr_info("Loongson lvds error: 0x%x\n", status);
> -               return -ENODEV;
> -       }
> -
> -       return 0;
> -}
I prefer to keep them, in downstream kernels there are users of them,
I don't want to add them back if one day those users are upstream.

Huacai

> -
>  static int __init event_init(struct generic_sub_driver *sub_driver)
>  {
>         int ret;
> --
> 2.49.0
>
>