On Thu, Aug 25, 2022 at 04:13:34PM +0200, 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.
...
> /* 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;
checkpatch might complain for absence of blank line here, btw. Perhaps
you may just move the above one here at the same patch.
> memset(&props, 0, sizeof(struct backlight_properties));
--
With Best Regards,
Andy Shevchenko
