Re: [PATCH] ACPI: platform_profile: Add support for notification chains

["Limonciello, Mario" <mario.limonciello@xxxxxxx>]

On 10/24/2021 08:40, Barnabás Pőcze wrote:
> Hi
>
>
> Am I right in thinking that this adds functionality that has no users in the
> kernel at the moment?

That's correct.  I have a few ideas for consumers of it, but I wanted to 
make sure that this approach is acceptable before spending too much 
effort building on top of it.

> 2021. október 22., péntek 20:16 keltezéssel, Mario Limonciello írta:
>
> > Allow other drivers to react to determine 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 form of a notifier call.
> >
> > Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> > ---
> >   drivers/acpi/platform_profile.c  | 44 +++++++++++++++++++++++++++-----
> >   include/linux/platform_profile.h | 11 ++++++++
> >   2 files changed, 49 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> > index dd2fbf38e414..964e0c9bf70d 100644
> > --- a/drivers/acpi/platform_profile.c
> > +++ b/drivers/acpi/platform_profile.c
> > @@ -21,6 +21,25 @@ 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);
> > +
> > +void platform_profile_call_notifier(unsigned long action, void *data)
> > +{
> > +	blocking_notifier_call_chain(&platform_profile_chain_head, action, data);
> > +}
> > +EXPORT_SYMBOL_GPL(platform_profile_call_notifier);
>
> What is the reason for exporting this function?

Thanks for catching this.  You're right - this doesn't need to be 
exported, it's only needed to be used by platform_profile_notify and can 
be static to platform_profile.c

Will fix up on the next version.

> >   static ssize_t platform_profile_choices_show(struct device *dev,
> >   					struct device_attribute *attr,
> > @@ -49,11 +68,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 +81,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 +159,12 @@ void platform_profile_notify(void)
> >   	if (!cur_profile)
> >   		return;
> >   	sysfs_notify(acpi_kobj, NULL, "platform_profile");
> > +	platform_profile_call_notifier(PLATFORM_PROFILE_CHANGED, NULL);
> > +
> >   }
> >   EXPORT_SYMBOL_GPL(platform_profile_notify);
> >
> > +
> >   int platform_profile_register(struct platform_profile_handler *pprof)
> >   {
> >   	int err;
> > diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> > index e5cbb6841f3a..390d95d47e07 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,14 @@ 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);
> > +void platform_profile_call_notifier(unsigned long action, void *data);
> > +
> > +enum platform_profile_notifier_actions {
> > +	PLATFORM_PROFILE_CHANGED,
> > +};
> >
> >   #endif  /*_PLATFORM_PROFILE_H_*/
> > --
> > 2.25.1
>
>
> Regards,
> Barnabás Pőcze