Re: [PATCH v7 13/22] ACPI: platform_profile: Add profile attribute for class interface

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

 



Am 21.11.24 um 15:27 schrieb Mark Pearson:

On Thu, Nov 21, 2024, at 6:10 AM, Ilpo Järvinen wrote:
I don't know why you dropped Mario and the list, I reinstanstated those
two.
Unintentional - sorry. Hit the reply button instead of reply-all and didn't notice. Doh.

On Wed, 20 Nov 2024, Mark Pearson wrote:
On Wed, Nov 20, 2024, at 9:56 AM, Ilpo Järvinen wrote:
On Tue, 19 Nov 2024, Mario Limonciello wrote:

Reading and writing the `profile` sysfs file will use the callbacks for
the platform profile handler to read or set the given profile.

Tested-by: Mark Pearson <mpearson-lenovo@xxxxxxxxx>
Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
v7:
  * Remove extra handler set
  * Remove err variable
v6:
  * Fix return
v5:
  * Drop recovery flow
  * Don't get profile before setting (not needed)
  * Simplify casting for call to _store_class_profile()
  * Only notify legacy interface of changes
  * Adjust mutex use
---
  drivers/acpi/platform_profile.c | 100 ++++++++++++++++++++++++++++++++
  1 file changed, 100 insertions(+)

diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index 9d6ead043994c..1530e6096cd39 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
  static struct attribute *profile_attrs[] = {
  	&dev_attr_name.attr,
  	&dev_attr_choices.attr,
+	&dev_attr_profile.attr,
I started to wonder if "choices" is good name for the other attribute as
it is the set of values "profile" accepts? Should they be bound by the
naming too like "profile_choices" or something along those lines so the
connection between the two is very evident?

Wouldn't it be weird to not have it in sync with the main sysfs entry
(which I don't think we can change at that point without messing up
userspace).

I think it would be more confusing to have different naming as it would
imply they're different things.
Ah, I didn't realize there's a pre-existing convention. Then just
disregard what I suggested.

No idea if it's a convention - I just would think it would be confusing for users.

Thanks
Mark

I personally would prefer the attribute name "choices", but i would also accept if the attribute was
named "profile_choices". I think adding the "profile" prefix to the attribute when it is already handled
by the platform-profile class is silly.

Either way:
Reviewed-by: Armin Wolf <W_Armin@xxxxxx>






[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux