| Message ID | 20241119171739.77028-15-mario.limonciello@amd.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add support for binding ACPI platform profile to multiple drivers | expand |
On Tue, 19 Nov 2024, Mario Limonciello wrote: > As multiple platform profile handlers may come and go, send a notification > to userspace each time that a platform profile handler is registered or > unregistered. > > Reviewed-by: Armin Wolf <W_Armin@gmx.de> > Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> > Tested-by: Matthew Schwartz <matthew.schwartz@linux.dev> > Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca> > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > --- > v7: > * Add Armin's tag > --- > drivers/acpi/platform_profile.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c > index 1530e6096cd39..de0804305b02c 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -363,6 +363,8 @@ int platform_profile_register(struct platform_profile_handler *pprof) > goto cleanup_ida; > } > > + sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + > cur_profile = pprof; > > err = sysfs_update_group(acpi_kobj, &platform_profile_group); Is the ordering problematic here, how long userspace has to wait for the update to become visible? > @@ -393,6 +395,8 @@ int platform_profile_remove(struct platform_profile_handler *pprof) > device_unregister(pprof->class_dev); > ida_free(&platform_profile_ida, id); > > + sysfs_notify(acpi_kobj, NULL, "platform_profile"); > + > sysfs_update_group(acpi_kobj, &platform_profile_group);
On 11/20/2024 09:09, Ilpo Järvinen wrote: > On Tue, 19 Nov 2024, Mario Limonciello wrote: > >> As multiple platform profile handlers may come and go, send a notification >> to userspace each time that a platform profile handler is registered or >> unregistered. >> >> Reviewed-by: Armin Wolf <W_Armin@gmx.de> >> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> Tested-by: Matthew Schwartz <matthew.schwartz@linux.dev> >> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca> >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> --- >> v7: >> * Add Armin's tag >> --- >> drivers/acpi/platform_profile.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c >> index 1530e6096cd39..de0804305b02c 100644 >> --- a/drivers/acpi/platform_profile.c >> +++ b/drivers/acpi/platform_profile.c >> @@ -363,6 +363,8 @@ int platform_profile_register(struct platform_profile_handler *pprof) >> goto cleanup_ida; >> } >> >> + sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + >> cur_profile = pprof; >> >> err = sysfs_update_group(acpi_kobj, &platform_profile_group); > > Is the ordering problematic here, how long userspace has to wait for the > update to become visible? TBH - this feels like an artifact of the earlier patches. I don't know that we really need the notify anymore since calling sysfs_update_group(). I'm tending to think drop this patch entirely. > >> @@ -393,6 +395,8 @@ int platform_profile_remove(struct platform_profile_handler *pprof) >> device_unregister(pprof->class_dev); >> ida_free(&platform_profile_ida, id); >> >> + sysfs_notify(acpi_kobj, NULL, "platform_profile"); >> + >> sysfs_update_group(acpi_kobj, &platform_profile_group); > >
Am 20.11.24 um 16:37 schrieb Mario Limonciello: > On 11/20/2024 09:09, Ilpo Järvinen wrote: >> On Tue, 19 Nov 2024, Mario Limonciello wrote: >> >>> As multiple platform profile handlers may come and go, send a >>> notification >>> to userspace each time that a platform profile handler is registered or >>> unregistered. >>> >>> Reviewed-by: Armin Wolf <W_Armin@gmx.de> >>> Tested-by: Mark Pearson <mpearson-lenovo@squebb.ca> >>> Tested-by: Matthew Schwartz <matthew.schwartz@linux.dev> >>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca> >>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >>> --- >>> v7: >>> * Add Armin's tag >>> --- >>> drivers/acpi/platform_profile.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/acpi/platform_profile.c >>> b/drivers/acpi/platform_profile.c >>> index 1530e6096cd39..de0804305b02c 100644 >>> --- a/drivers/acpi/platform_profile.c >>> +++ b/drivers/acpi/platform_profile.c >>> @@ -363,6 +363,8 @@ int platform_profile_register(struct >>> platform_profile_handler *pprof) >>> goto cleanup_ida; >>> } >>> + sysfs_notify(acpi_kobj, NULL, "platform_profile"); >>> + >>> cur_profile = pprof; >>> err = sysfs_update_group(acpi_kobj, &platform_profile_group); >> >> Is the ordering problematic here, how long userspace has to wait for the >> update to become visible? > > TBH - this feels like an artifact of the earlier patches. I don't > know that we really need the notify anymore since calling > sysfs_update_group(). > > I'm tending to think drop this patch entirely. > I do not think so. A new platform profile handler might cause the platform profile choices to change. It might also cause the platform profile to switch to "custom" in some cases. So i think we still have to keep this patch. Thanks >> >>> @@ -393,6 +395,8 @@ int platform_profile_remove(struct >>> platform_profile_handler *pprof) >>> device_unregister(pprof->class_dev); >>> ida_free(&platform_profile_ida, id); >>> + sysfs_notify(acpi_kobj, NULL, "platform_profile"); >>> + >>> sysfs_update_group(acpi_kobj, &platform_profile_group); >> >> > >
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index 1530e6096cd39..de0804305b02c 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -363,6 +363,8 @@ int platform_profile_register(struct platform_profile_handler *pprof) goto cleanup_ida; } + sysfs_notify(acpi_kobj, NULL, "platform_profile"); + cur_profile = pprof; err = sysfs_update_group(acpi_kobj, &platform_profile_group); @@ -393,6 +395,8 @@ int platform_profile_remove(struct platform_profile_handler *pprof) device_unregister(pprof->class_dev); ida_free(&platform_profile_ida, id); + sysfs_notify(acpi_kobj, NULL, "platform_profile"); + sysfs_update_group(acpi_kobj, &platform_profile_group); return 0;