| Message ID | 20230809185453.40916-7-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Fix wakeup problems on some AMD platforms | expand |
On Wed, Aug 09, 2023 at 01:54:50PM -0500, Mario Limonciello wrote: > Other parts of the kernel may use constraints information to make > decisions on what power state to put a device into. ... > +int acpi_get_lps0_constraint(struct device *dev) > +{ > + int i; > + > + for (i = 0; i < lpi_constraints_table_size; ++i) { > + static struct lpi_constraints *entry; static?! Seems to me with the code in lpi_check_constraints() this actually can be first (separate patch maybe with conversion of the mentioned user) transformed to #define for_each_lpi_constraint(entry) for (int i = 0; entry = &lpi_constraints_table[i], i < lpi_constraints_table_size; i++) > + int val; > + > + entry = &lpi_constraints_table[i]; > + if (!device_match_acpi_handle(dev, entry->handle)) > + continue; > + val = entry->enabled ? entry->min_dstate : 0; > + acpi_handle_debug(entry->handle, > + "ACPI device constraint: %d\n", val); > + return val; > + } So will become struct lpi_constraints *entry; int val; for_each_lpi_constraint(entry) { if (!device_match_acpi_handle(dev, entry->handle)) continue; val = entry->enabled ? entry->min_dstate : 0; acpi_handle_debug(entry->handle, "ACPI device constraint: %d\n", val); return val; } > + return -ENODEV; > +}
On 8/10/2023 10:47 AM, Andy Shevchenko wrote: > On Wed, Aug 09, 2023 at 01:54:50PM -0500, Mario Limonciello wrote: >> Other parts of the kernel may use constraints information to make >> decisions on what power state to put a device into. > > ... > >> +int acpi_get_lps0_constraint(struct device *dev) >> +{ > >> + int i; >> + >> + for (i = 0; i < lpi_constraints_table_size; ++i) { >> + static struct lpi_constraints *entry; > > static?! Whoops! Good catch! > > Seems to me with the code in lpi_check_constraints() this actually can be first > (separate patch maybe with conversion of the mentioned user) transformed to > > #define for_each_lpi_constraint(entry) > for (int i = 0; > entry = &lpi_constraints_table[i], i < lpi_constraints_table_size; > i++) > >> + int val; >> + >> + entry = &lpi_constraints_table[i]; >> + if (!device_match_acpi_handle(dev, entry->handle)) >> + continue; >> + val = entry->enabled ? entry->min_dstate : 0; >> + acpi_handle_debug(entry->handle, >> + "ACPI device constraint: %d\n", val); >> + return val; >> + } > > So will become > > struct lpi_constraints *entry; > int val; > > for_each_lpi_constraint(entry) { > if (!device_match_acpi_handle(dev, entry->handle)) > continue; > val = entry->enabled ? entry->min_dstate : 0; > acpi_handle_debug(entry->handle, > "ACPI device constraint: %d\n", val); > return val; > } > >> + return -ENODEV; >> +} > Much appreciated suggestion. I'll incorporate this in the next version.
On Thu, Aug 10, 2023 at 10:54:08AM -0500, Limonciello, Mario wrote: > On 8/10/2023 10:47 AM, Andy Shevchenko wrote: > > On Wed, Aug 09, 2023 at 01:54:50PM -0500, Mario Limonciello wrote: ... > Much appreciated suggestion. I'll incorporate this in the next version. I just sent a patch into this thread. Feel free to incorporate.
diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c index 0c8101acc92ef..2a1a482f4803a 100644 --- a/drivers/acpi/x86/s2idle.c +++ b/drivers/acpi/x86/s2idle.c @@ -295,6 +295,35 @@ static void lpi_device_get_constraints(void) ACPI_FREE(out_obj); } +/** + * acpi_get_lps0_constraint - get any LPS0 constraint for a device + * @dev: device to get constraints for + * + * Returns: + * - If the constraint is enabled, the value for constraint. + * - If the constraint is disabled, 0. + * - Otherwise, -ENODEV. + */ +int acpi_get_lps0_constraint(struct device *dev) +{ + int i; + + for (i = 0; i < lpi_constraints_table_size; ++i) { + static struct lpi_constraints *entry; + int val; + + entry = &lpi_constraints_table[i]; + if (!device_match_acpi_handle(dev, entry->handle)) + continue; + val = entry->enabled ? entry->min_dstate : 0; + acpi_handle_debug(entry->handle, + "ACPI device constraint: %d\n", val); + return val; + } + + return -ENODEV; +} + static void lpi_check_constraints(void) { int i; diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 13a0fca3539f0..99458502a7510 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -1109,6 +1109,12 @@ struct acpi_s2idle_dev_ops { }; int acpi_register_lps0_dev(struct acpi_s2idle_dev_ops *arg); void acpi_unregister_lps0_dev(struct acpi_s2idle_dev_ops *arg); +int acpi_get_lps0_constraint(struct device *dev); +#else /* CONFIG_ACPI_SLEEP && CONFIG_X86 */ +static inline int acpi_get_lps0_constraint(struct device *dev) +{ + return -ENODEV; +} #endif /* CONFIG_ACPI_SLEEP && CONFIG_X86 */ #ifndef CONFIG_IA64 void arch_reserve_mem_area(acpi_physical_address addr, size_t size);
Other parts of the kernel may use constraints information to make decisions on what power state to put a device into. Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> --- v9->v10: * split from other patches * kerneldoc fixes * move debug statement to this function --- drivers/acpi/x86/s2idle.c | 29 +++++++++++++++++++++++++++++ include/linux/acpi.h | 6 ++++++ 2 files changed, 35 insertions(+)