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(+)

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);
-- 
2.34.1

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;
> +}

-- 
With Best Regards,
Andy Shevchenko

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.

-- 
With Best Regards,
Andy Shevchenko

We have one existing and one coming user of this macro.
Introduce a helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/acpi/x86/s2idle.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/x86/s2idle.c b/drivers/acpi/x86/s2idle.c
index ce62e61a9605..a8aa7299039e 100644
--- a/drivers/acpi/x86/s2idle.c
+++ b/drivers/acpi/x86/s2idle.c
@@ -94,6 +94,11 @@ static struct lpi_constraints *lpi_constraints_table;
 static int lpi_constraints_table_size;
 static int rev_id;
 
+#define for_each_lpi_constraint(entry)						\
+	for (int i = 0;								\
+	     entry = &lpi_constraints_table[i], i < lpi_constraints_table_size;	\
+	     i++)
+
 static void lpi_device_get_constraints_amd(void)
 {
 	union acpi_object *out_obj;
@@ -293,30 +298,29 @@ static void lpi_device_get_constraints(void)
 
 static void lpi_check_constraints(void)
 {
-	int i;
+	struct lpi_constraints *entry;
 
-	for (i = 0; i < lpi_constraints_table_size; ++i) {
-		acpi_handle handle = lpi_constraints_table[i].handle;
-		struct acpi_device *adev = acpi_fetch_acpi_dev(handle);
+	for_each_lpi_constraint(entry) {
+		struct acpi_device *adev = acpi_fetch_acpi_dev(entry->handle);
 
 		if (!adev)
 			continue;
 
-		acpi_handle_debug(handle,
+		acpi_handle_debug(entry->handle,
 			"LPI: required min power state:%s current power state:%s\n",
-			acpi_power_state_string(lpi_constraints_table[i].min_dstate),
+			acpi_power_state_string(entry->min_dstate),
 			acpi_power_state_string(adev->power.state));
 
 		if (!adev->flags.power_manageable) {
-			acpi_handle_info(handle, "LPI: Device not power manageable\n");
-			lpi_constraints_table[i].handle = NULL;
+			acpi_handle_info(entry->handle, "LPI: Device not power manageable\n");
+			entry->handle = NULL;
 			continue;
 		}
 
-		if (adev->power.state < lpi_constraints_table[i].min_dstate)
-			acpi_handle_info(handle,
+		if (adev->power.state < entry->min_dstate)
+			acpi_handle_info(entry->handle,
 				"LPI: Constraint not met; min power state:%s current power state:%s\n",
-				acpi_power_state_string(lpi_constraints_table[i].min_dstate),
+				acpi_power_state_string(entry->min_dstate),
 				acpi_power_state_string(adev->power.state));
 	}
 }
-- 
2.40.0.1.gaa8946217a0b