Re: [PATCH 1/3] platform/x86: msi-laptop: Fix old-ec check for backlight registering

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 8/25/22 16:13, Hans de Goede wrote:
> Commit 2cc6c717799f ("msi-laptop: Port to new backlight interface
> selection API") replaced this check:
> 
> 	if (!quirks->old_ec_model || acpi_video_backlight_support())
> 		pr_info("Brightness ignored, ...");
> 	else
> 		do_register();
> 
> With:
> 
> 	if (quirks->old_ec_model ||
> 	    acpi_video_get_backlight_type() == acpi_backlight_vendor)
> 		do_register();
> 
> But since the do_register() part was part of the else branch, the entire
> condition should be inverted.  So not only the 2 statements on either
> side of the || should be inverted, but the || itself should be replaced
> with a &&.
> 
> In practice this has likely not been an issue because the new-ec models
> (old_ec_model==false) likely all support ACPI video backlight control,
> making acpi_video_get_backlight_type() return acpi_backlight_video
> turning the second part of the || also false when old_ec_model == false.
> 
> Fixes: 2cc6c717799f ("msi-laptop: Port to new backlight interface selection API")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I've added this series to my review-hans (soon to be for-next) branch now.

Regards,

Hans

> ---
>  drivers/platform/x86/msi-laptop.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/msi-laptop.c b/drivers/platform/x86/msi-laptop.c
> index 93ef8851b93e..54170172a666 100644
> --- a/drivers/platform/x86/msi-laptop.c
> +++ b/drivers/platform/x86/msi-laptop.c
> @@ -1047,8 +1047,7 @@ static int __init msi_init(void)
>  		return -EINVAL;
>  
>  	/* Register backlight stuff */
> -
> -	if (quirks->old_ec_model ||
> +	if (quirks->old_ec_model &&
>  	    acpi_video_get_backlight_type() == acpi_backlight_vendor) {
>  		struct backlight_properties props;
>  		memset(&props, 0, sizeof(struct backlight_properties));