Re: [PATCH v5 11/20] ACPI: platform_profile: Add choices attribute for class interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/8/2024 12:06, Armin Wolf wrote:
Am 07.11.24 um 23:09 schrieb Mario Limonciello:

On 11/7/2024 02:28, Armin Wolf wrote:
Am 07.11.24 um 07:02 schrieb Mario Limonciello:

The `choices` file will show all possible choices that a given platform
profile handler can support.

Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
v5:
  * Fix kdoc
  * Add tag
  * Fix whitespace
  * Adjust mutex use
---
  drivers/acpi/platform_profile.c | 65
+++++++++++++++++++++++++++++++++
  1 file changed, 65 insertions(+)

diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/
platform_profile.c
index f605c2bd35c68..5e0bb91c5f451 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -25,6 +25,46 @@ static_assert(ARRAY_SIZE(profile_names) ==
PLATFORM_PROFILE_LAST);

  static DEFINE_IDA(platform_profile_ida);

+/**
+ * _commmon_choices_show - Show the available profile choices
+ * @choices: The available profile choices
+ * @buf: The buffer to write to
+ * Return: The number of bytes written
+ */
+static ssize_t _commmon_choices_show(unsigned long choices, char *buf)
+{
+    int i, len = 0;
+
+    for_each_set_bit(i, &choices, PLATFORM_PROFILE_LAST) {
+        if (len == 0)
+            len += sysfs_emit_at(buf, len, "%s", profile_names[i]);
+        else
+            len += sysfs_emit_at(buf, len, " %s", profile_names[i]);
+    }
+    len += sysfs_emit_at(buf, len, "\n");
+
+    return len;
+}
+
+/**
+ * _get_class_choices - Get the available profile choices for a
class device
+ * @dev: The class device
+ * @choices: Pointer to return the available profile choices
+ * Return: The available profile choices
+ */
+static int _get_class_choices(struct device *dev, unsigned long
*choices)
+{
+    struct platform_profile_handler *handler;
+    int i;
+
+    lockdep_assert_held(&profile_lock);
+    handler = dev_get_drvdata(dev);
+    for_each_set_bit(i, handler->choices, PLATFORM_PROFILE_LAST)
+        *choices |= BIT(i);

Maybe just copying the bitmask would be enough here? In this case we
could also drop
this function as well.

Right now this could work, but choices and the use of it has gone
through great lengths to ensure that once there are too many profiles
it automatically becomes a bigger variable.

    unsigned long choices[BITS_TO_LONGS(PLATFORM_PROFILE_LAST)];

So I would rather keep this as is.

I think users of this function can do the locking themself and instead use the functions from bitmap.h. Because _get_class_choices() will break once "choices" becomes bigger.


I am thinking it's better to just switch explicitly to an 'unsigned long' and add a static check that all profiles fit. If ever they don't fit, the code can be overhauled at that point.

And yes _get_class_choices can go away then, and this is a lot simpler.




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux