linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/powercap: Implement powercap_set_enabled()
@ 2025-05-10 18:47 Suchit Karunakaran
  2025-05-13 22:03 ` Shuah Khan
  0 siblings, 1 reply; 4+ messages in thread
From: Suchit Karunakaran @ 2025-05-10 18:47 UTC (permalink / raw)
  To: trenn, shuah, jwyatt, jkacur, linux-pm
  Cc: linux-kernel-mentees, linux-kernel, Suchit Karunakaran

The powercap_set_enabled() function previously returned a dummy value
and was marked with a TODO comment. This patch implements the function
by writing the desired mode (0 or 1) to /sys/class/powercap/intel-rapl/enabled

Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
---
 tools/power/cpupower/lib/powercap.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tools/power/cpupower/lib/powercap.c b/tools/power/cpupower/lib/powercap.c
index 94a0c69e55ef..7947b9809239 100644
--- a/tools/power/cpupower/lib/powercap.c
+++ b/tools/power/cpupower/lib/powercap.c
@@ -70,6 +70,22 @@ static int sysfs_get_enabled(char *path, int *mode)
 	return ret;
 }
 
+static int sysfs_set_enabled(const char *path, int mode)
+{
+	int fd;
+	char buf[2] = { mode ? '1' : '0', '\n' };
+	ssize_t ret;
+
+	fd = open(path, O_WRONLY);
+	if (fd == -1)
+		return -1;
+
+	ret = write(fd, buf, sizeof(buf));
+	close(fd);
+
+	return ret == sizeof(buf) ? 0 : -1;
+}
+
 int powercap_get_enabled(int *mode)
 {
 	char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
@@ -77,12 +93,10 @@ int powercap_get_enabled(int *mode)
 	return sysfs_get_enabled(path, mode);
 }
 
-/*
- * TODO: implement function. Returns dummy 0 for now.
- */
 int powercap_set_enabled(int mode)
 {
-	return 0;
+	char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
+	return sysfs_set_enabled(path, mode);
 }
 
 /*
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] tools/powercap: Implement powercap_set_enabled()
  2025-05-10 18:47 [PATCH] tools/powercap: Implement powercap_set_enabled() Suchit Karunakaran
@ 2025-05-13 22:03 ` Shuah Khan
  2025-05-14  1:20   ` Suchit K
  0 siblings, 1 reply; 4+ messages in thread
From: Shuah Khan @ 2025-05-13 22:03 UTC (permalink / raw)
  To: Suchit Karunakaran, trenn, shuah, jwyatt, jkacur, linux-pm
  Cc: linux-kernel-mentees, linux-kernel, Shuah Khan

On 5/10/25 12:47, Suchit Karunakaran wrote:
> The powercap_set_enabled() function previously returned a dummy value
> and was marked with a TODO comment. This patch implements the function
> by writing the desired mode (0 or 1) to /sys/class/powercap/intel-rapl/enabled

The short summary should say cpupower: Implement powercap_set_enabled()
> 
> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> ---
>   tools/power/cpupower/lib/powercap.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/power/cpupower/lib/powercap.c b/tools/power/cpupower/lib/powercap.c
> index 94a0c69e55ef..7947b9809239 100644
> --- a/tools/power/cpupower/lib/powercap.c
> +++ b/tools/power/cpupower/lib/powercap.c
> @@ -70,6 +70,22 @@ static int sysfs_get_enabled(char *path, int *mode)
>   	return ret;
>   }
>   
> +static int sysfs_set_enabled(const char *path, int mode)
> +{
> +	int fd;
> +	char buf[2] = { mode ? '1' : '0', '\n' };
> +	ssize_t ret;
> +
> +	fd = open(path, O_WRONLY);
> +	if (fd == -1)
> +		return -1;
> +
> +	ret = write(fd, buf, sizeof(buf));
> +	close(fd);
> +
> +	return ret == sizeof(buf) ? 0 : -1;
> +}
> +
>   int powercap_get_enabled(int *mode)
>   {
>   	char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
> @@ -77,12 +93,10 @@ int powercap_get_enabled(int *mode)
>   	return sysfs_get_enabled(path, mode);
>   }
>   
> -/*
> - * TODO: implement function. Returns dummy 0 for now.
> - */
>   int powercap_set_enabled(int mode)
>   {
> -	return 0;
> +	char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
> +	return sysfs_set_enabled(path, mode);

Did you compile this?

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tools/powercap: Implement powercap_set_enabled()
  2025-05-13 22:03 ` Shuah Khan
@ 2025-05-14  1:20   ` Suchit K
  2025-05-14 19:53     ` Shuah Khan
  0 siblings, 1 reply; 4+ messages in thread
From: Suchit K @ 2025-05-14  1:20 UTC (permalink / raw)
  To: Shuah Khan
  Cc: trenn, shuah, jwyatt, jkacur, linux-pm, linux-kernel-mentees,
	linux-kernel

Thanks for the suggestion, Shuah. And I did compile the kernel after
making the changes.


On Wed, 14 May 2025 at 03:33, Shuah Khan <skhan@linuxfoundation.org> wrote:
>
> On 5/10/25 12:47, Suchit Karunakaran wrote:
> > The powercap_set_enabled() function previously returned a dummy value
> > and was marked with a TODO comment. This patch implements the function
> > by writing the desired mode (0 or 1) to /sys/class/powercap/intel-rapl/enabled
>
> The short summary should say cpupower: Implement powercap_set_enabled()
> >
> > Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
> > ---
> >   tools/power/cpupower/lib/powercap.c | 22 ++++++++++++++++++----
> >   1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/power/cpupower/lib/powercap.c b/tools/power/cpupower/lib/powercap.c
> > index 94a0c69e55ef..7947b9809239 100644
> > --- a/tools/power/cpupower/lib/powercap.c
> > +++ b/tools/power/cpupower/lib/powercap.c
> > @@ -70,6 +70,22 @@ static int sysfs_get_enabled(char *path, int *mode)
> >       return ret;
> >   }
> >
> > +static int sysfs_set_enabled(const char *path, int mode)
> > +{
> > +     int fd;
> > +     char buf[2] = { mode ? '1' : '0', '\n' };
> > +     ssize_t ret;
> > +
> > +     fd = open(path, O_WRONLY);
> > +     if (fd == -1)
> > +             return -1;
> > +
> > +     ret = write(fd, buf, sizeof(buf));
> > +     close(fd);
> > +
> > +     return ret == sizeof(buf) ? 0 : -1;
> > +}
> > +
> >   int powercap_get_enabled(int *mode)
> >   {
> >       char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
> > @@ -77,12 +93,10 @@ int powercap_get_enabled(int *mode)
> >       return sysfs_get_enabled(path, mode);
> >   }
> >
> > -/*
> > - * TODO: implement function. Returns dummy 0 for now.
> > - */
> >   int powercap_set_enabled(int mode)
> >   {
> > -     return 0;
> > +     char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
> > +     return sysfs_set_enabled(path, mode);
>
> Did you compile this?
>
> thanks,
> -- Shuah

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] tools/powercap: Implement powercap_set_enabled()
  2025-05-14  1:20   ` Suchit K
@ 2025-05-14 19:53     ` Shuah Khan
  0 siblings, 0 replies; 4+ messages in thread
From: Shuah Khan @ 2025-05-14 19:53 UTC (permalink / raw)
  To: Suchit K
  Cc: trenn, shuah, jwyatt, jkacur, linux-pm, linux-kernel-mentees,
	linux-kernel, Shuah Khan

On 5/13/25 19:20, Suchit K wrote:
> Thanks for the suggestion, Shuah. And I did compile the kernel after
> making the changes.
> 
> 

Please don't top post in kernel patch responses.

> On Wed, 14 May 2025 at 03:33, Shuah Khan <skhan@linuxfoundation.org> wrote:
>>
>> On 5/10/25 12:47, Suchit Karunakaran wrote:
>>> The powercap_set_enabled() function previously returned a dummy value
>>> and was marked with a TODO comment. This patch implements the function
>>> by writing the desired mode (0 or 1) to /sys/class/powercap/intel-rapl/enabled
>>
>> The short summary should say cpupower: Implement powercap_set_enabled()

I am not clear on how this can be used. Can you elaborate on the use-case
for the set?

Send me v2 fixing the change log.

>>>
>>> Signed-off-by: Suchit Karunakaran <suchitkarunakaran@gmail.com>
>>> ---
>>>    tools/power/cpupower/lib/powercap.c | 22 ++++++++++++++++++----
>>>    1 file changed, 18 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/tools/power/cpupower/lib/powercap.c b/tools/power/cpupower/lib/powercap.c
>>> index 94a0c69e55ef..7947b9809239 100644
>>> --- a/tools/power/cpupower/lib/powercap.c
>>> +++ b/tools/power/cpupower/lib/powercap.c
>>> @@ -70,6 +70,22 @@ static int sysfs_get_enabled(char *path, int *mode)
>>>        return ret;
>>>    }
>>>
>>> +static int sysfs_set_enabled(const char *path, int mode)
>>> +{
>>> +     int fd;
>>> +     char buf[2] = { mode ? '1' : '0', '\n' };
>>> +     ssize_t ret;
>>> +
>>> +     fd = open(path, O_WRONLY);
>>> +     if (fd == -1)
>>> +             return -1;
>>> +
>>> +     ret = write(fd, buf, sizeof(buf));
>>> +     close(fd);
>>> +
>>> +     return ret == sizeof(buf) ? 0 : -1;
>>> +}
>>> +
>>>    int powercap_get_enabled(int *mode)
>>>    {
>>>        char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
>>> @@ -77,12 +93,10 @@ int powercap_get_enabled(int *mode)
>>>        return sysfs_get_enabled(path, mode);
>>>    }
>>>
>>> -/*
>>> - * TODO: implement function. Returns dummy 0 for now.
>>> - */
>>>    int powercap_set_enabled(int mode)
>>>    {
>>> -     return 0;
>>> +     char path[SYSFS_PATH_MAX] = PATH_TO_POWERCAP "/intel-rapl/enabled";
>>> +     return sysfs_set_enabled(path, mode);
>>
>> Did you compile this?
>>
>> thanks,
>> -- Shuah


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-05-14 19:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-10 18:47 [PATCH] tools/powercap: Implement powercap_set_enabled() Suchit Karunakaran
2025-05-13 22:03 ` Shuah Khan
2025-05-14  1:20   ` Suchit K
2025-05-14 19:53     ` Shuah Khan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).