Re: [PATCH] ACPI: Support Fujitsu Lifebook S6410 in fujitsu-laptop

Jonathan Woithe
2008-04-15T23:12:34Z

> Here is a modularized version of the patch, it also includes autodetection
> of S6410 to switch between the brightness interface (can be overridden by
> module parameter)
>
> Jonathan can you test it if it doesn't break anything on the S7020?

Will do. A comment on the patch - there seems to be a fair number of
seemingly cosmetic and unnecessary whitespace/ordering changes. Perhaps try
to minimise these as it makes the true impact of the patch much clearer
and thus easier to evaluate and work with.

For example:

> diff --git a/drivers/misc/fujitsu-laptop.c b/drivers/misc/fujitsu-laptop.c
> index e2e7c05..9a80d29 100644
> --- a/drivers/misc/fujitsu-laptop.c
> +++ b/drivers/misc/fujitsu-laptop.c
> @@ -2,6 +2,7 @@
:
> @@ -56,21 +65,36 @@
> #define FUJITSU_LCD_N_LEVELS 8
>
> #define ACPI_FUJITSU_CLASS "fujitsu"
> -#define ACPI_FUJITSU_HID "FUJ02B1"
> -#define ACPI_FUJITSU_DRIVER_NAME "Fujitsu laptop FUJ02B1 ACPI extras driver"
> -#define ACPI_FUJITSU_DEVICE_NAME "Fujitsu FUJ02B1"
> +
> +#define ACPI_FUJITSUB1_DRIVER_NAME "Fujitsu laptop ACPI extras driver"
> +#define ACPI_FUJITSUB1_HID "FUJ02B1"
> +#define ACPI_FUJITSUB1_DEVICE_NAME "Fujitsu FUJ02B1"

As far as I can tell all you've really changed here is
ACPI_FUJITSU_DRIVER_NAME, yet ACPI_FUJITSUB1_HID and
ACPI_FUJITSUB1_DEVICE_NAME are also flagged as changed.

Another one:

> status =
> - acpi_evaluate_integer(fujitsu->acpi_handle, "GBLL", NULL, &state);
> + acpi_evaluate_integer(fujitsu->acpi_handle, "GBLL", NULL,
> + &state);

Even though the acpi_evaluate_integer() call hasn't been altered it appears
in the patch due to a minor and unneeded formatting change.

Regards
jonathan