| Message ID | 20220912172401.22301-2-mario.limonciello@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fixups for s2idle on various Rembrandt laptops | expand |
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@antebit.com > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > 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@gmail.com> Tested-by: Philipp Zabel <philipp.zabel@gmail.com> # GA402RJ regards Philipp
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; + rev_id = data->rev_id; lps0_dsm_func_mask = validate_dsm(adev->handle, ACPI_LPS0_DSM_UUID_AMD, rev_id, &lps0_dsm_guid); lps0_dsm_func_mask_microsoft = validate_dsm(adev->handle, ACPI_LPS0_DSM_UUID_MICROSOFT, 0, &lps0_dsm_guid_microsoft); - if (lps0_dsm_func_mask > 0x3 && (!strcmp(hid, "AMD0004") || - !strcmp(hid, "AMD0005") || - !strcmp(hid, "AMDI0005"))) { + if (lps0_dsm_func_mask > 0x3 && data->check_off_by_one) { lps0_dsm_func_mask = (lps0_dsm_func_mask << 1) | 0x1; acpi_handle_debug(adev->handle, "_DSM UUID %s: Adjusted function mask: 0x%x\n", ACPI_LPS0_DSM_UUID_AMD, lps0_dsm_func_mask); - } else if (lps0_dsm_func_mask_microsoft > 0 && + } else if (lps0_dsm_func_mask_microsoft > 0 && data->prefer_amd_guid && (!strcmp(hid, "AMDI0007") || !strcmp(hid, "AMDI0008"))) { lps0_dsm_func_mask_microsoft = -EINVAL;