platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] Let other drivers react to platform profile changes
@ 2021-10-26 18:05 Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 1/3] platform/x86: hp-wmi: rename platform_profile_* function symbols Mario Limonciello
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Mario Limonciello @ 2021-10-26 18:05 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, markpearson, linux-acpi,
	Mario Limonciello

Currently only one driver can register as a handler for a platform profile.

This limitation means that if multiple drivers want to react to platform
profile changes that they will need to directly interact with individual
drivers.

Instead introduce a notification flow that drivers can register for event
changes.  The idea is that any driver that wants to can:
1. read the current profile
2. set up initial values
3. register for changes
4. react to changes

Note: currently this is accomplished by changing platform_profile_get to a
symbol that all drivers can get during initialization.  Another idea for
this may be to change:

`int platform_profile_register_notifier(struct notifier_block *nb)`

into:

`int platform_profile_register_notifier(struct notifier_block *nb,
					enum *profile)`

IOW return the current profile value as an out argument as part of
registration. I don't have a strong opinion.

Changes from v2->v3:
 * Add patches to avoid collisions in hp-wmi and asus-wmi symbols

Mario Limonciello (3):
  platform/x86: hp-wmi: rename platform_profile_* function symbols
  platform/x86: asus-wmi: rename platform_profile_* function symbols
  ACPI: platform_profile: Add support for notification chains

 drivers/acpi/platform_profile.c  | 48 ++++++++++++++++++++++++++++----
 drivers/platform/x86/asus-wmi.c  | 12 ++++----
 drivers/platform/x86/hp-wmi.c    | 12 ++++----
 include/linux/platform_profile.h | 10 +++++++
 4 files changed, 64 insertions(+), 18 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/3] platform/x86: hp-wmi: rename platform_profile_* function symbols
  2021-10-26 18:05 [PATCH v3 0/3] Let other drivers react to platform profile changes Mario Limonciello
@ 2021-10-26 18:05 ` Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 2/3] platform/x86: asus-wmi: " Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains Mario Limonciello
  2 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2021-10-26 18:05 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, markpearson, linux-acpi,
	Mario Limonciello

An upcoming change to platform profiles will export `platform_profile_get`
as a symbol that can be used by other drivers. Avoid the collision.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/hp-wmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 8e31ffadf894..48a46466f086 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -1061,8 +1061,8 @@ static int thermal_profile_set(int thermal_profile)
 							   sizeof(thermal_profile), 0);
 }
 
-static int platform_profile_get(struct platform_profile_handler *pprof,
-				enum platform_profile_option *profile)
+static int hp_wmi_platform_profile_get(struct platform_profile_handler *pprof,
+					enum platform_profile_option *profile)
 {
 	int tp;
 
@@ -1087,8 +1087,8 @@ static int platform_profile_get(struct platform_profile_handler *pprof,
 	return 0;
 }
 
-static int platform_profile_set(struct platform_profile_handler *pprof,
-				enum platform_profile_option profile)
+static int hp_wmi_platform_profile_set(struct platform_profile_handler *pprof,
+					enum platform_profile_option profile)
 {
 	int err, tp;
 
@@ -1147,8 +1147,8 @@ static int thermal_profile_setup(void)
 		if (err)
 			return err;
 
-		platform_profile_handler.profile_get = platform_profile_get;
-		platform_profile_handler.profile_set = platform_profile_set;
+		platform_profile_handler.profile_get = hp_wmi_platform_profile_get;
+		platform_profile_handler.profile_set = hp_wmi_platform_profile_set;
 	}
 
 	set_bit(PLATFORM_PROFILE_COOL, platform_profile_handler.choices);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/3] platform/x86: asus-wmi: rename platform_profile_* function symbols
  2021-10-26 18:05 [PATCH v3 0/3] Let other drivers react to platform profile changes Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 1/3] platform/x86: hp-wmi: rename platform_profile_* function symbols Mario Limonciello
@ 2021-10-26 18:05 ` Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains Mario Limonciello
  2 siblings, 0 replies; 8+ messages in thread
From: Mario Limonciello @ 2021-10-26 18:05 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, markpearson, linux-acpi,
	Mario Limonciello

An upcoming change to platform profiles will export `platform_profile_get`
as a symbol that can be used by other drivers. Avoid the collision.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/asus-wmi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index e14fb5fa7324..8f067ac4e952 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -2169,8 +2169,8 @@ static ssize_t throttle_thermal_policy_store(struct device *dev,
 static DEVICE_ATTR_RW(throttle_thermal_policy);
 
 /* Platform profile ***********************************************************/
-static int platform_profile_get(struct platform_profile_handler *pprof,
-				enum platform_profile_option *profile)
+static int asus_wmi_platform_profile_get(struct platform_profile_handler *pprof,
+					enum platform_profile_option *profile)
 {
 	struct asus_wmi *asus;
 	int tp;
@@ -2196,8 +2196,8 @@ static int platform_profile_get(struct platform_profile_handler *pprof,
 	return 0;
 }
 
-static int platform_profile_set(struct platform_profile_handler *pprof,
-				enum platform_profile_option profile)
+static int asus_wmi_platform_profile_set(struct platform_profile_handler *pprof,
+					enum platform_profile_option profile)
 {
 	struct asus_wmi *asus;
 	int tp;
@@ -2236,8 +2236,8 @@ static int platform_profile_setup(struct asus_wmi *asus)
 
 	dev_info(dev, "Using throttle_thermal_policy for platform_profile support\n");
 
-	asus->platform_profile_handler.profile_get = platform_profile_get;
-	asus->platform_profile_handler.profile_set = platform_profile_set;
+	asus->platform_profile_handler.profile_get = asus_wmi_platform_profile_get;
+	asus->platform_profile_handler.profile_set = asus_wmi_platform_profile_set;
 
 	set_bit(PLATFORM_PROFILE_QUIET, asus->platform_profile_handler.choices);
 	set_bit(PLATFORM_PROFILE_BALANCED,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains
  2021-10-26 18:05 [PATCH v3 0/3] Let other drivers react to platform profile changes Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 1/3] platform/x86: hp-wmi: rename platform_profile_* function symbols Mario Limonciello
  2021-10-26 18:05 ` [PATCH v3 2/3] platform/x86: asus-wmi: " Mario Limonciello
@ 2021-10-26 18:05 ` Mario Limonciello
  2021-10-26 18:36   ` [External] " Mark Pearson
  2 siblings, 1 reply; 8+ messages in thread
From: Mario Limonciello @ 2021-10-26 18:05 UTC (permalink / raw)
  To: Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, markpearson, linux-acpi,
	Mario Limonciello

Allow other drivers to initialize relative to current active
profile and react to platform profile changes.

Drivers wishing to utilize this should register for notification
at module load and unregister when unloading.

Notifications will come in the from a notifier call.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/platform_profile.c  | 48 ++++++++++++++++++++++++++++----
 include/linux/platform_profile.h | 10 +++++++
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index d418462ab791..225247efa55f 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -21,6 +21,24 @@ static const char * const profile_names[] = {
 	[PLATFORM_PROFILE_PERFORMANCE] = "performance",
 };
 static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
+static BLOCKING_NOTIFIER_HEAD(platform_profile_chain_head);
+
+int platform_profile_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&platform_profile_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(platform_profile_register_notifier);
+
+int platform_profile_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&platform_profile_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(platform_profile_unregister_notifier);
+
+static void platform_profile_call_notifier(unsigned long action, void *data)
+{
+	blocking_notifier_call_chain(&platform_profile_chain_head, action, data);
+}
 
 static ssize_t platform_profile_choices_show(struct device *dev,
 					struct device_attribute *attr,
@@ -49,11 +67,8 @@ static ssize_t platform_profile_choices_show(struct device *dev,
 	return len;
 }
 
-static ssize_t platform_profile_show(struct device *dev,
-					struct device_attribute *attr,
-					char *buf)
+int platform_profile_get(enum platform_profile_option *profile)
 {
-	enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
 	int err;
 
 	err = mutex_lock_interruptible(&profile_lock);
@@ -65,15 +80,28 @@ static ssize_t platform_profile_show(struct device *dev,
 		return -ENODEV;
 	}
 
-	err = cur_profile->profile_get(cur_profile, &profile);
+	err = cur_profile->profile_get(cur_profile, profile);
 	mutex_unlock(&profile_lock);
 	if (err)
 		return err;
 
 	/* Check that profile is valid index */
-	if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names))))
+	if (WARN_ON((*profile < 0) || (*profile >= ARRAY_SIZE(profile_names))))
 		return -EIO;
 
+	return 0;
+}
+EXPORT_SYMBOL_GPL(platform_profile_get);
+
+static ssize_t platform_profile_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
+	int ret = platform_profile_get(&profile);
+
+	if (ret)
+		return ret;
 	return sysfs_emit(buf, "%s\n", profile_names[profile]);
 }
 
@@ -130,9 +158,17 @@ static const struct attribute_group platform_profile_group = {
 
 void platform_profile_notify(void)
 {
+	enum platform_profile_option profile;
+	int ret;
+
 	if (!cur_profile)
 		return;
 	sysfs_notify(acpi_kobj, NULL, "platform_profile");
+	ret = platform_profile_get(&profile);
+	if (ret)
+		return;
+	platform_profile_call_notifier(PLATFORM_PROFILE_CHANGED, &profile);
+
 }
 EXPORT_SYMBOL_GPL(platform_profile_notify);
 
diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
index e5cbb6841f3a..05ba3403509a 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -11,6 +11,8 @@
 
 #include <linux/bitops.h>
 
+struct notifier_block;
+
 /*
  * If more options are added please update profile_names array in
  * platform_profile.c and sysfs-platform_profile documentation.
@@ -37,5 +39,13 @@ struct platform_profile_handler {
 int platform_profile_register(struct platform_profile_handler *pprof);
 int platform_profile_remove(void);
 void platform_profile_notify(void);
+int platform_profile_get(enum platform_profile_option *profile);
+
+int platform_profile_register_notifier(struct notifier_block *nb);
+int platform_profile_unregister_notifier(struct notifier_block *nb);
+
+enum platform_profile_notifier_actions {
+	PLATFORM_PROFILE_CHANGED,
+};
 
 #endif  /*_PLATFORM_PROFILE_H_*/
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains
  2021-10-26 18:05 ` [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains Mario Limonciello
@ 2021-10-26 18:36   ` Mark Pearson
  2021-10-26 18:38     ` Limonciello, Mario
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Pearson @ 2021-10-26 18:36 UTC (permalink / raw)
  To: Mario Limonciello, Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi

Thanks Mario,

Patch series looks good. One minor suggestion below.

Mark

On 2021-10-26 14:05, Mario Limonciello wrote:
> Allow other drivers to initialize relative to current active
> profile and react to platform profile changes.
> 
> Drivers wishing to utilize this should register for notification
> at module load and unregister when unloading.
> 
> Notifications will come in the from a notifier call.
> 
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   drivers/acpi/platform_profile.c  | 48 ++++++++++++++++++++++++++++----
>   include/linux/platform_profile.h | 10 +++++++
>   2 files changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index d418462ab791..225247efa55f 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -21,6 +21,24 @@ static const char * const profile_names[] = {
>   	[PLATFORM_PROFILE_PERFORMANCE] = "performance",
>   };
>   static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
> +static BLOCKING_NOTIFIER_HEAD(platform_profile_chain_head);
> +
> +int platform_profile_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&platform_profile_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_register_notifier);
> +
> +int platform_profile_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&platform_profile_chain_head, nb);
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_unregister_notifier);
> +
> +static void platform_profile_call_notifier(unsigned long action, void *data)
> +{
> +	blocking_notifier_call_chain(&platform_profile_chain_head, action, data);
> +}
>   
>   static ssize_t platform_profile_choices_show(struct device *dev,
>   					struct device_attribute *attr,
> @@ -49,11 +67,8 @@ static ssize_t platform_profile_choices_show(struct device *dev,
>   	return len;
>   }
>   
> -static ssize_t platform_profile_show(struct device *dev,
> -					struct device_attribute *attr,
> -					char *buf)
> +int platform_profile_get(enum platform_profile_option *profile)
>   {
> -	enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
>   	int err;
>   
>   	err = mutex_lock_interruptible(&profile_lock);
> @@ -65,15 +80,28 @@ static ssize_t platform_profile_show(struct device *dev,
>   		return -ENODEV;
>   	}
>   
> -	err = cur_profile->profile_get(cur_profile, &profile);
> +	err = cur_profile->profile_get(cur_profile, profile);
>   	mutex_unlock(&profile_lock);
>   	if (err)
>   		return err;
>   
>   	/* Check that profile is valid index */
> -	if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names))))
> +	if (WARN_ON((*profile < 0) || (*profile >= ARRAY_SIZE(profile_names))))
>   		return -EIO;
>   
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(platform_profile_get);
> +
> +static ssize_t platform_profile_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED;
> +	int ret = platform_profile_get(&profile);
> +
> +	if (ret)
> +		return ret;
>   	return sysfs_emit(buf, "%s\n", profile_names[profile]);
>   }
>   
> @@ -130,9 +158,17 @@ static const struct attribute_group platform_profile_group = {
>   
>   void platform_profile_notify(void)
>   {
> +	enum platform_profile_option profile;
> +	int ret;
> +
>   	if (!cur_profile)
>   		return;
>   	sysfs_notify(acpi_kobj, NULL, "platform_profile");
> +	ret = platform_profile_get(&profile);
> +	if (ret)
> +		return;

As no return value to function then simplify to:
	if (platform_profile_get(&profile))
		return;

> +	platform_profile_call_notifier(PLATFORM_PROFILE_CHANGED, &profile);
> +
>   }
>   EXPORT_SYMBOL_GPL(platform_profile_notify);
>   
> diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> index e5cbb6841f3a..05ba3403509a 100644
> --- a/include/linux/platform_profile.h
> +++ b/include/linux/platform_profile.h
> @@ -11,6 +11,8 @@
>   
>   #include <linux/bitops.h>
>   
> +struct notifier_block;
> +
>   /*
>    * If more options are added please update profile_names array in
>    * platform_profile.c and sysfs-platform_profile documentation.
> @@ -37,5 +39,13 @@ struct platform_profile_handler {
>   int platform_profile_register(struct platform_profile_handler *pprof);
>   int platform_profile_remove(void);
>   void platform_profile_notify(void);
> +int platform_profile_get(enum platform_profile_option *profile);
> +
> +int platform_profile_register_notifier(struct notifier_block *nb);
> +int platform_profile_unregister_notifier(struct notifier_block *nb);
> +
> +enum platform_profile_notifier_actions {
> +	PLATFORM_PROFILE_CHANGED,
> +};
>   
>   #endif  /*_PLATFORM_PROFILE_H_*/
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains
  2021-10-26 18:36   ` [External] " Mark Pearson
@ 2021-10-26 18:38     ` Limonciello, Mario
  2021-10-26 18:50       ` Mark Pearson
  0 siblings, 1 reply; 8+ messages in thread
From: Limonciello, Mario @ 2021-10-26 18:38 UTC (permalink / raw)
  To: Mark Pearson, Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi

[Public]

> -----Original Message-----
> From: Mark Pearson <markpearson@lenovo.com>
> Sent: Tuesday, October 26, 2021 13:36
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Hans de Goede
> <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; Rafael J .
> Wysocki <rjw@rjwysocki.net>
> Cc: open list:X86 PLATFORM DRIVERS <platform-driver-x86@vger.kernel.org>;
> linux-acpi@vger.kernel.org
> Subject: Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for
> notification chains
> 
> Thanks Mario,
> 
> Patch series looks good. One minor suggestion below.

Thanks Mark.  What do you think of the other idea I had in my cover letter?
I think that's another way to do this, that might mean less surgery to this
source file and other function call.

I'll re-spin it either to accept your suggestion below or the other idea I put
in the cover letter.

> 
> Mark
> 
> On 2021-10-26 14:05, Mario Limonciello wrote:
> > Allow other drivers to initialize relative to current active
> > profile and react to platform profile changes.
> >
> > Drivers wishing to utilize this should register for notification
> > at module load and unregister when unloading.
> >
> > Notifications will come in the from a notifier call.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> > ---
> >   drivers/acpi/platform_profile.c  | 48 ++++++++++++++++++++++++++++----
> >   include/linux/platform_profile.h | 10 +++++++
> >   2 files changed, 52 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> > index d418462ab791..225247efa55f 100644
> > --- a/drivers/acpi/platform_profile.c
> > +++ b/drivers/acpi/platform_profile.c
> > @@ -21,6 +21,24 @@ static const char * const profile_names[] = {
> >   	[PLATFORM_PROFILE_PERFORMANCE] = "performance",
> >   };
> >   static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
> > +static BLOCKING_NOTIFIER_HEAD(platform_profile_chain_head);
> > +
> > +int platform_profile_register_notifier(struct notifier_block *nb)
> > +{
> > +	return blocking_notifier_chain_register(&platform_profile_chain_head,
> nb);
> > +}
> > +EXPORT_SYMBOL_GPL(platform_profile_register_notifier);
> > +
> > +int platform_profile_unregister_notifier(struct notifier_block *nb)
> > +{
> > +	return
> blocking_notifier_chain_unregister(&platform_profile_chain_head, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(platform_profile_unregister_notifier);
> > +
> > +static void platform_profile_call_notifier(unsigned long action, void *data)
> > +{
> > +	blocking_notifier_call_chain(&platform_profile_chain_head, action,
> data);
> > +}
> >
> >   static ssize_t platform_profile_choices_show(struct device *dev,
> >   					struct device_attribute *attr,
> > @@ -49,11 +67,8 @@ static ssize_t platform_profile_choices_show(struct
> device *dev,
> >   	return len;
> >   }
> >
> > -static ssize_t platform_profile_show(struct device *dev,
> > -					struct device_attribute *attr,
> > -					char *buf)
> > +int platform_profile_get(enum platform_profile_option *profile)
> >   {
> > -	enum platform_profile_option profile =
> PLATFORM_PROFILE_BALANCED;
> >   	int err;
> >
> >   	err = mutex_lock_interruptible(&profile_lock);
> > @@ -65,15 +80,28 @@ static ssize_t platform_profile_show(struct device
> *dev,
> >   		return -ENODEV;
> >   	}
> >
> > -	err = cur_profile->profile_get(cur_profile, &profile);
> > +	err = cur_profile->profile_get(cur_profile, profile);
> >   	mutex_unlock(&profile_lock);
> >   	if (err)
> >   		return err;
> >
> >   	/* Check that profile is valid index */
> > -	if (WARN_ON((profile < 0) || (profile >= ARRAY_SIZE(profile_names))))
> > +	if (WARN_ON((*profile < 0) || (*profile >= ARRAY_SIZE(profile_names))))
> >   		return -EIO;
> >
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(platform_profile_get);
> > +
> > +static ssize_t platform_profile_show(struct device *dev,
> > +					struct device_attribute *attr,
> > +					char *buf)
> > +{
> > +	enum platform_profile_option profile =
> PLATFORM_PROFILE_BALANCED;
> > +	int ret = platform_profile_get(&profile);
> > +
> > +	if (ret)
> > +		return ret;
> >   	return sysfs_emit(buf, "%s\n", profile_names[profile]);
> >   }
> >
> > @@ -130,9 +158,17 @@ static const struct attribute_group
> platform_profile_group = {
> >
> >   void platform_profile_notify(void)
> >   {
> > +	enum platform_profile_option profile;
> > +	int ret;
> > +
> >   	if (!cur_profile)
> >   		return;
> >   	sysfs_notify(acpi_kobj, NULL, "platform_profile");
> > +	ret = platform_profile_get(&profile);
> > +	if (ret)
> > +		return;
> 
> As no return value to function then simplify to:
> 	if (platform_profile_get(&profile))
> 		return;
> 
> > +	platform_profile_call_notifier(PLATFORM_PROFILE_CHANGED,
> &profile);
> > +
> >   }
> >   EXPORT_SYMBOL_GPL(platform_profile_notify);
> >
> > diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> > index e5cbb6841f3a..05ba3403509a 100644
> > --- a/include/linux/platform_profile.h
> > +++ b/include/linux/platform_profile.h
> > @@ -11,6 +11,8 @@
> >
> >   #include <linux/bitops.h>
> >
> > +struct notifier_block;
> > +
> >   /*
> >    * If more options are added please update profile_names array in
> >    * platform_profile.c and sysfs-platform_profile documentation.
> > @@ -37,5 +39,13 @@ struct platform_profile_handler {
> >   int platform_profile_register(struct platform_profile_handler *pprof);
> >   int platform_profile_remove(void);
> >   void platform_profile_notify(void);
> > +int platform_profile_get(enum platform_profile_option *profile);
> > +
> > +int platform_profile_register_notifier(struct notifier_block *nb);
> > +int platform_profile_unregister_notifier(struct notifier_block *nb);
> > +
> > +enum platform_profile_notifier_actions {
> > +	PLATFORM_PROFILE_CHANGED,
> > +};
> >
> >   #endif  /*_PLATFORM_PROFILE_H_*/
> >

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains
  2021-10-26 18:38     ` Limonciello, Mario
@ 2021-10-26 18:50       ` Mark Pearson
  2021-10-26 19:02         ` Limonciello, Mario
  0 siblings, 1 reply; 8+ messages in thread
From: Mark Pearson @ 2021-10-26 18:50 UTC (permalink / raw)
  To: Limonciello, Mario, Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi


Hi Mario

On 2021-10-26 14:38, Limonciello, Mario wrote:
> [Public]
> 
>> -----Original Message-----
>> From: Mark Pearson <markpearson@lenovo.com>
>> Sent: Tuesday, October 26, 2021 13:36
>> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Hans de Goede
>> <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; Rafael J .
>> Wysocki <rjw@rjwysocki.net>
>> Cc: open list:X86 PLATFORM DRIVERS <platform-driver-x86@vger.kernel.org>;
>> linux-acpi@vger.kernel.org
>> Subject: Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for
>> notification chains
>>
>> Thanks Mario,
>>
>> Patch series looks good. One minor suggestion below.
> 
> Thanks Mark.  What do you think of the other idea I had in my cover letter?
> I think that's another way to do this, that might mean less surgery to this
> source file and other function call.
> 
> I'll re-spin it either to accept your suggestion below or the other idea I put
> in the cover letter.

I don't have a strong opinion either I'm afraid :) Getting the
profile when you register seems neat to me - I have no objections.

Mark

^ permalink raw reply	[flat|nested] 8+ messages in thread

* RE: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains
  2021-10-26 18:50       ` Mark Pearson
@ 2021-10-26 19:02         ` Limonciello, Mario
  0 siblings, 0 replies; 8+ messages in thread
From: Limonciello, Mario @ 2021-10-26 19:02 UTC (permalink / raw)
  To: Mark Pearson, Hans de Goede, Mark Gross, Rafael J . Wysocki
  Cc: open list:X86 PLATFORM DRIVERS, linux-acpi

[Public]

> -----Original Message-----
> From: Mark Pearson <markpearson@lenovo.com>
> Sent: Tuesday, October 26, 2021 13:50
> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Hans de Goede
> <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; Rafael J .
> Wysocki <rjw@rjwysocki.net>
> Cc: open list:X86 PLATFORM DRIVERS <platform-driver-x86@vger.kernel.org>;
> linux-acpi@vger.kernel.org
> Subject: Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for
> notification chains
> 
> 
> Hi Mario
> 
> On 2021-10-26 14:38, Limonciello, Mario wrote:
> > [Public]
> >
> >> -----Original Message-----
> >> From: Mark Pearson <markpearson@lenovo.com>
> >> Sent: Tuesday, October 26, 2021 13:36
> >> To: Limonciello, Mario <Mario.Limonciello@amd.com>; Hans de Goede
> >> <hdegoede@redhat.com>; Mark Gross <mgross@linux.intel.com>; Rafael J .
> >> Wysocki <rjw@rjwysocki.net>
> >> Cc: open list:X86 PLATFORM DRIVERS <platform-driver-
> x86@vger.kernel.org>;
> >> linux-acpi@vger.kernel.org
> >> Subject: Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support
> for
> >> notification chains
> >>
> >> Thanks Mario,
> >>
> >> Patch series looks good. One minor suggestion below.
> >
> > Thanks Mark.  What do you think of the other idea I had in my cover letter?
> > I think that's another way to do this, that might mean less surgery to this
> > source file and other function call.
> >
> > I'll re-spin it either to accept your suggestion below or the other idea I put
> > in the cover letter.
> 
> I don't have a strong opinion either I'm afraid :) Getting the
> profile when you register seems neat to me - I have no objections.
> 
> Mark

OK I slightly lean towards keeping it as is to prevent racing around driver load
order.  I'll adopt your suggestion and re-send.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-10-26 19:03 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-26 18:05 [PATCH v3 0/3] Let other drivers react to platform profile changes Mario Limonciello
2021-10-26 18:05 ` [PATCH v3 1/3] platform/x86: hp-wmi: rename platform_profile_* function symbols Mario Limonciello
2021-10-26 18:05 ` [PATCH v3 2/3] platform/x86: asus-wmi: " Mario Limonciello
2021-10-26 18:05 ` [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains Mario Limonciello
2021-10-26 18:36   ` [External] " Mark Pearson
2021-10-26 18:38     ` Limonciello, Mario
2021-10-26 18:50       ` Mark Pearson
2021-10-26 19:02         ` Limonciello, Mario

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).