Re: [PATCH v1 3/4] platform profile: Add firmware-neutral aggregate sysfs interface

[Armin Wolf <W_Armin@gmx.de>]

Am 14.10.26 um 15:24 schrieb Daniel Lezcano:

> The per-provider class interface is firmware-independent, but system-
> wide profile selection is only exposed under /sys/firmware/acpi.
>
> Add /sys/kernel/platform_profile/{choices,profile}, sharing the existing
> aggregate callbacks and semantics with the legacy interface. Keep
> attributes hidden until a provider is registered and remove them after
> the last provider.
>
> Notify both aggregate frontends on profile changes. Also notify the
> provider class device on a direct profile write, matching its documented
> POLLPRI ABI. Restore aggregate visibility after a failed registration.

IMHO the aggregate sysfs interface only exists for backwards-compatibility,
so i would be very happy if you could just make the associated code optional
and instead teach platform-profiles-daemon to use the individual class devices
instead.

Thanks,
Armin Wolf

> Assisted-by: OpenCode:gpt-6.1-sol
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> ---
>   drivers/platform/profile/core.c | 60 +++++++++++++++++++++++++++++----
>   1 file changed, 53 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/profile/core.c b/drivers/platform/profile/core.c
> index f18fd96b942c..507b96bdfdb1 100644
> --- a/drivers/platform/profile/core.c
> +++ b/drivers/platform/profile/core.c
> @@ -26,9 +26,11 @@ static DEFINE_MUTEX(profile_lock);
>   static DEFINE_MUTEX(profile_registration_lock);
>   static LIST_HEAD(profile_handlers);
>   static struct kobject *compat_kobj;
> +static struct kobject *profile_kobj;
>   
>   static void profile_notify(void)
>   {
> +	sysfs_notify(profile_kobj, NULL, "profile");
>   	if (compat_kobj)
>   		sysfs_notify(compat_kobj, NULL, "platform_profile");
>   }
> @@ -250,6 +252,7 @@ static ssize_t profile_store(struct device *dev,
>   		ret = _store_class_profile(dev, &index);
>   		if (ret)
>   			return ret;
> +		_notify_class_profile(dev, NULL);
>   	}
>   
>   	profile_notify();
> @@ -324,7 +327,7 @@ static int _remove_hidden_choices(struct device *dev, void *arg)
>   }
>   
>   /**
> - * platform_profile_choices_show - Show the available profile choices for legacy sysfs interface
> + * platform_profile_choices_show - Show the aggregate profile choices
>    * @kobj: The kobject
>    * @attr: The attribute
>    * @buf: The buffer to write to
> @@ -360,7 +363,7 @@ static ssize_t platform_profile_choices_show(struct kobject *kobj,
>   }
>   
>   /**
> - * _aggregate_profiles - Aggregate the profiles for legacy sysfs interface
> + * _aggregate_profiles - Aggregate the profiles across providers
>    * @dev: The device
>    * @data: The profile to return
>    *
> @@ -385,7 +388,7 @@ static int _aggregate_profiles(struct device *dev, void *data)
>   }
>   
>   /**
> - * _store_and_notify - Store and notify a class from legacy sysfs interface
> + * _store_and_notify - Store and notify a provider from an aggregate write
>    * @dev: The device
>    * @data: The profile to return
>    *
> @@ -403,7 +406,7 @@ static int _store_and_notify(struct device *dev, void *data)
>   }
>   
>   /**
> - * platform_profile_show - Show the current profile for legacy sysfs interface
> + * platform_profile_show - Show the current aggregate profile
>    * @kobj: The kobject
>    * @attr: The attribute
>    * @buf: The buffer to write to
> @@ -431,7 +434,7 @@ static ssize_t platform_profile_show(struct kobject *kobj,
>   }
>   
>   /**
> - * platform_profile_store - Set the profile for legacy sysfs interface
> + * platform_profile_store - Set the profile across all providers
>    * @kobj: The kobject
>    * @attr: The attribute
>    * @buf: The buffer to read from
> @@ -493,8 +496,30 @@ static const struct attribute_group platform_profile_group = {
>   	.is_visible = profile_class_is_visible,
>   };
>   
> +static struct kobj_attribute aggregate_choices =
> +	__ATTR(choices, 0444, platform_profile_choices_show, NULL);
> +static struct kobj_attribute aggregate_profile =
> +	__ATTR(profile, 0644, platform_profile_show, platform_profile_store);
> +
> +static struct attribute *aggregate_attrs[] = {
> +	&aggregate_choices.attr,
> +	&aggregate_profile.attr,
> +	NULL
> +};
> +
> +static const struct attribute_group aggregate_group = {
> +	.attrs = aggregate_attrs,
> +	.is_visible = profile_class_is_visible,
> +};
> +
>   static int profile_update(void)
>   {
> +	int err;
> +
> +	err = sysfs_update_group(profile_kobj, &aggregate_group);
> +	if (err)
> +		return err;
> +
>   	if (compat_kobj)
>   		return sysfs_update_group(compat_kobj, &platform_profile_group);
>   
> @@ -502,7 +527,7 @@ static int profile_update(void)
>   }
>   
>   /**
> - * platform_profile_notify - Notify class device and legacy sysfs interface
> + * platform_profile_notify - Notify the provider and aggregate sysfs interfaces
>    * @dev: The class device
>    */
>   void platform_profile_notify(struct device *dev)
> @@ -643,6 +668,7 @@ struct device *platform_profile_register(struct device *dev, const char *name,
>   cleanup_cur:
>   	scoped_guard(mutex, &profile_lock)
>   		list_del(&to_pprof_handler(ppdev)->node);
> +	profile_update();
>   	device_unregister(ppdev);
>   
>   cleanup_ida:
> @@ -726,15 +752,33 @@ static int __init platform_profile_init(void)
>   	if (err)
>   		return err;
>   
> +	profile_kobj = kobject_create_and_add("platform_profile", kernel_kobj);
> +	if (!profile_kobj) {
> +		err = -ENOMEM;
> +		goto unregister_class;
> +	}
> +
> +	err = sysfs_create_group(profile_kobj, &aggregate_group);
> +	if (err)
> +		goto put_kobj;
> +
>   	compat_kobj = platform_profile_compat_kobj();
>   	if (compat_kobj) {
>   		err = sysfs_create_group(compat_kobj, &platform_profile_group);
>   		if (err) {
>   			kobject_put(compat_kobj);
> -			class_unregister(&platform_profile_class);
> +			goto remove_group;
>   		}
>   	}
>   
> +	return 0;
> +
> +remove_group:
> +	sysfs_remove_group(profile_kobj, &aggregate_group);
> +put_kobj:
> +	kobject_put(profile_kobj);
> +unregister_class:
> +	class_unregister(&platform_profile_class);
>   	return err;
>   }
>   
> @@ -744,6 +788,8 @@ static void __exit platform_profile_exit(void)
>   		sysfs_remove_group(compat_kobj, &platform_profile_group);
>   		kobject_put(compat_kobj);
>   	}
> +	sysfs_remove_group(profile_kobj, &aggregate_group);
> +	kobject_put(profile_kobj);
>   	class_unregister(&platform_profile_class);
>   }
>   subsys_initcall_sync(platform_profile_init);