On Mon, Jan 7, 2019 at 5:08 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>
> This commit refactors the chassis-type detection introduced by
> commit 53fa1f6e8a59 ("ACPI / video: Only default only_lcd to true on
> Win8-ready _desktops_") (where desktop means anything without a builtin
> screen).
>
> The DMI chassis_type is an unsigned integer, so rather then doing a
> whole bunch of string-compares on it, convert it to an int and feed
> the result to a switch case.
>
> Note the switch case uses hex values, this is done because the spec
> uses hex values too. This changes the check for "Main Server Chassis"
> from checking for 11 decimal to 11 hexadecimal, this is a bug fix,
> the original check for 11 decimal was wrong.
>
> Fixes: 53fa1f6e8a59 ("ACPI / video: Only default only_lcd to true ...")
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/acpi/acpi_video.c | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/acpi/acpi_video.c b/drivers/acpi/acpi_video.c
> index f0b52266b3ac..ba5c2f70babe 100644
> --- a/drivers/acpi/acpi_video.c
> +++ b/drivers/acpi/acpi_video.c
> @@ -2124,21 +2124,27 @@ static int __init intel_opregion_present(void)
> return opregion;
> }
>
> +/* Check if the chassis-type indicates there is no builtin LCD panel */
> static bool dmi_is_desktop(void)
> {
> const char *chassis_type;
> + unsigned long type;
>
> chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE);
> if (!chassis_type)
> return false;
>
> - if (!strcmp(chassis_type, "3") || /* 3: Desktop */
> - !strcmp(chassis_type, "4") || /* 4: Low Profile Desktop */
> - !strcmp(chassis_type, "5") || /* 5: Pizza Box */
> - !strcmp(chassis_type, "6") || /* 6: Mini Tower */
> - !strcmp(chassis_type, "7") || /* 7: Tower */
> - !strcmp(chassis_type, "11")) /* 11: Main Server Chassis */
> - return true;
> + if (kstrtoul(chassis_type, 10, &type) != 0)
> + return false;
> +
Why don't you write the code below as a single line:
return (type >= 0x03 && type <= 0x07) || type == 0x11;
> + switch (type) {
> + case 0x03: return true; /* Desktop */
> + case 0x04: return true; /* Low Profile Desktop */
> + case 0x05: return true; /* Pizza Box */
> + case 0x06: return true; /* Mini Tower */
> + case 0x07: return true; /* Tower */
> + case 0x11: return true; /* Main Server Chassis */
> + }
>
> return false;
> }
> --
And you can add a comment like "Return true for the Desktop (0x03),
Low Profile Desktop (0x04), ... Main Server Chassis (0x11) form
factors." next to it to describe the meaning of the values.
