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