Am Mon, Sep 12, 2022 at 12:23:55PM -0500 schrieb Mario Limonciello:
> Right now the information about which cases to use for what are in a
> comment, but this is error prone. Instead move all information into
> a dedicated structure.
>
> Tested-by: catalin@xxxxxxxxxxx
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> drivers/acpi/x86/s2idle.c | 63 ++++++++++++++++++++++++++++-----------
> 1 file changed, 46 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
> index f9ac12b778e6..a7757551f750 100644
> --- a/drivers/acpi/x86/s2idle.c
> +++ b/drivers/acpi/x86/s2idle.c
> @@ -363,6 +363,39 @@ static int validate_dsm(acpi_handle handle, const char *uuid, int rev, guid_t *d
> return ret;
> }
>
> +struct amd_lps0_hid_device_data {
> + const unsigned int rev_id;
> + const bool check_off_by_one;
> + const bool prefer_amd_guid;
> +};
> +
> +static const struct amd_lps0_hid_device_data amd_picasso = {
> + .rev_id = 0,
> + .check_off_by_one = true,
> + .prefer_amd_guid = false,
> +};
> +
> +static const struct amd_lps0_hid_device_data amd_cezanne = {
> + .rev_id = 0,
> + .check_off_by_one = false,
> + .prefer_amd_guid = false,
> +};
> +
> +static const struct amd_lps0_hid_device_data amd_rembrandt = {
> + .rev_id = 2,
> + .check_off_by_one = false,
> + .prefer_amd_guid = true,
> +};
> +
> +static const struct acpi_device_id amd_hid_ids[] = {
> + {"AMD0004", (kernel_ulong_t)&amd_picasso, },
> + {"AMD0005", (kernel_ulong_t)&amd_picasso, },
> + {"AMDI0005", (kernel_ulong_t)&amd_picasso, },
> + {"AMDI0006", (kernel_ulong_t)&amd_cezanne, },
> + {"AMDI0007", (kernel_ulong_t)&amd_rembrandt, },
> + {}
> +};
> +
> static int lps0_device_attach(struct acpi_device *adev,
> const struct acpi_device_id *not_used)
> {
> @@ -370,31 +403,27 @@ static int lps0_device_attach(struct acpi_device *adev,
> return 0;
>
> if (acpi_s2idle_vendor_amd()) {
> - /* AMD0004, AMD0005, AMDI0005:
> - * - Should use rev_id 0x0
> - * - function mask > 0x3: Should use AMD method, but has off by one bug
> - * - function mask = 0x3: Should use Microsoft method
> - * AMDI0006:
> - * - should use rev_id 0x0
> - * - function mask = 0x3: Should use Microsoft method
> - * AMDI0007:
> - * - Should use rev_id 0x2
> - * - Should only use AMD method
> - */
> - const char *hid = acpi_device_hid(adev);
> - rev_id = strcmp(hid, "AMDI0007") ? 0 : 2;
> + static const struct acpi_device_id *dev_id;
> + const struct amd_lps0_hid_device_data *data;
> +
> + for (dev_id = &amd_hid_ids[0]; dev_id->id[0]; dev_id++)
> + if (acpi_dev_hid_uid_match(adev, dev_id->id, NULL))
> + break;
> + if (dev_id != NULL)
> + data = (const struct amd_lps0_hid_device_data *) dev_id->driver_data;
> + else
> + return 0;
The "!= NULL" seems unnecessary, I would change this to:
+ if (!dev_id)
+ return 0;
+ data = (const struct amd_lps0_hid_device_data *) dev_id->driver_data;
But either way,
Reviewed-by: Philipp Zabel <philipp.zabel@xxxxxxxxx>
Tested-by: Philipp Zabel <philipp.zabel@xxxxxxxxx> # GA402RJ
regards
Philipp
