Re: [External] [PATCH v3 3/3] ACPI: platform_profile: Add support for notification chains

[Mark Pearson <markpearson@xxxxxxxxxx>]

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@xxxxxxx>
> ---
>   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_*/