Message ID | 20220712145847.3438544-2-Shyam-sundar.S-k@amd.com (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | platform/x86/amd/pmf: Introduce AMD PMF Driver | expand |
[Public] > -----Original Message----- > From: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com> > Sent: Tuesday, July 12, 2022 09:59 > To: hdegoede@redhat.com; markgross@kernel.org > Cc: platform-driver-x86@vger.kernel.org; Patil Rajesh > <Patil.Reddy@amd.com>; Limonciello, Mario > <Mario.Limonciello@amd.com>; S-k, Shyam-sundar <Shyam-sundar.S- > k@amd.com> > Subject: [PATCH v1 01/15] ACPI: platform_profile: Add support for > notification chains > > From: Mario Limonciello <mario.limonciello@amd.com> > > Allow other drivers to react to determine current active profile > and react to platform profile changes. > The original patch this came from had notification chains, but as this was pared down to just export the get method, this commit message and title should be updated. > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > --- > drivers/acpi/platform_profile.c | 26 ++++++++++++++++++++++++++ > include/linux/platform_profile.h | 1 + > 2 files changed, 27 insertions(+) > > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c > index d418462ab791..7e12a1f30f06 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -49,6 +49,32 @@ static ssize_t platform_profile_choices_show(struct > device *dev, > return len; > } > > +int platform_profile_get(enum platform_profile_option *profile) > +{ > + int err; > + > + err = mutex_lock_interruptible(&profile_lock); > + if (err) > + return err; > + > + if (!cur_profile) { > + mutex_unlock(&profile_lock); > + return -ENODEV; > + } > + > + 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)))) > + 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) > diff --git a/include/linux/platform_profile.h > b/include/linux/platform_profile.h > index e5cbb6841f3a..2395be670dfd 100644 > --- a/include/linux/platform_profile.h > +++ b/include/linux/platform_profile.h > @@ -37,5 +37,6 @@ 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); > > #endif /*_PLATFORM_PROFILE_H_*/ > -- > 2.25.1
Hi, On 7/12/22 17:03, Limonciello, Mario wrote: > [Public] > >> -----Original Message----- >> From: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com> >> Sent: Tuesday, July 12, 2022 09:59 >> To: hdegoede@redhat.com; markgross@kernel.org >> Cc: platform-driver-x86@vger.kernel.org; Patil Rajesh >> <Patil.Reddy@amd.com>; Limonciello, Mario >> <Mario.Limonciello@amd.com>; S-k, Shyam-sundar <Shyam-sundar.S- >> k@amd.com> >> Subject: [PATCH v1 01/15] ACPI: platform_profile: Add support for >> notification chains >> >> From: Mario Limonciello <mario.limonciello@amd.com> >> >> Allow other drivers to react to determine current active profile >> and react to platform profile changes. >> > > The original patch this came from had notification chains, but as this was > pared down to just export the get method, this commit message and title > should be updated. > >> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> >> --- >> drivers/acpi/platform_profile.c | 26 ++++++++++++++++++++++++++ >> include/linux/platform_profile.h | 1 + >> 2 files changed, 27 insertions(+) >> >> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c >> index d418462ab791..7e12a1f30f06 100644 >> --- a/drivers/acpi/platform_profile.c >> +++ b/drivers/acpi/platform_profile.c >> @@ -49,6 +49,32 @@ static ssize_t platform_profile_choices_show(struct >> device *dev, >> return len; >> } >> >> +int platform_profile_get(enum platform_profile_option *profile) >> +{ >> + int err; >> + >> + err = mutex_lock_interruptible(&profile_lock); Besides Mario'r remark about the commit message, this must be mutex_lock() not mutex_lock_interruptible() since this function is intended to be called by other kernel code, rather then from userspace. Regards, Hans >> + if (err) >> + return err; >> + >> + if (!cur_profile) { >> + mutex_unlock(&profile_lock); >> + return -ENODEV; >> + } >> + >> + 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)))) >> + 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) >> diff --git a/include/linux/platform_profile.h >> b/include/linux/platform_profile.h >> index e5cbb6841f3a..2395be670dfd 100644 >> --- a/include/linux/platform_profile.h >> +++ b/include/linux/platform_profile.h >> @@ -37,5 +37,6 @@ 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); >> >> #endif /*_PLATFORM_PROFILE_H_*/ >> -- >> 2.25.1 >
Hi, On 7/12/22 16:58, Shyam Sundar S K wrote: > From: Mario Limonciello <mario.limonciello@amd.com> > > Allow other drivers to react to determine current active profile > and react to platform profile changes. > > Signed-off-by: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> This new helper is not necessary, as mentioned in my review of 8/15, its single caller, the code there can just use: current_profile = READ_ONCE(dev->current_profile); Please drop this patch for the next version of the series. Regards, Hans > --- > drivers/acpi/platform_profile.c | 26 ++++++++++++++++++++++++++ > include/linux/platform_profile.h | 1 + > 2 files changed, 27 insertions(+) > > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c > index d418462ab791..7e12a1f30f06 100644 > --- a/drivers/acpi/platform_profile.c > +++ b/drivers/acpi/platform_profile.c > @@ -49,6 +49,32 @@ static ssize_t platform_profile_choices_show(struct device *dev, > return len; > } > > +int platform_profile_get(enum platform_profile_option *profile) > +{ > + int err; > + > + err = mutex_lock_interruptible(&profile_lock); > + if (err) > + return err; > + > + if (!cur_profile) { > + mutex_unlock(&profile_lock); > + return -ENODEV; > + } > + > + 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)))) > + 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) > diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h > index e5cbb6841f3a..2395be670dfd 100644 > --- a/include/linux/platform_profile.h > +++ b/include/linux/platform_profile.h > @@ -37,5 +37,6 @@ 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); > > #endif /*_PLATFORM_PROFILE_H_*/
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index d418462ab791..7e12a1f30f06 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -49,6 +49,32 @@ static ssize_t platform_profile_choices_show(struct device *dev, return len; } +int platform_profile_get(enum platform_profile_option *profile) +{ + int err; + + err = mutex_lock_interruptible(&profile_lock); + if (err) + return err; + + if (!cur_profile) { + mutex_unlock(&profile_lock); + return -ENODEV; + } + + 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)))) + 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) diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h index e5cbb6841f3a..2395be670dfd 100644 --- a/include/linux/platform_profile.h +++ b/include/linux/platform_profile.h @@ -37,5 +37,6 @@ 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); #endif /*_PLATFORM_PROFILE_H_*/