diff mbox series

[1/2] PM / devfreq: Add debugfs support with devfreq_summary file

Message ID 20200107090519.3231-2-cw00.choi@samsung.com (mailing list archive)
State Changes Requested
Delegated to: Chanwoo Choi
Headers show
Series PM / devfreq: Add debugfs support | expand

Commit Message

Chanwoo Choi Jan. 7, 2020, 9:05 a.m. UTC
Add debugfs interface to provide debugging information of devfreq device.
It contains 'devfreq_summary' entry to show the summary of registered
devfreq devices as following and the additional debugfs file will be added.
- /sys/kernel/debug/devfreq/devfreq_summary

[Detailed description of each field of 'devfreq_summary' debugfs file]
- dev_name	: Device name of h/w.
- dev		: Device name made by devfreq core.
- parent_dev	: If devfreq device uses the passive governor,
		  show parent devfreq device name.
- governor	: Devfreq governor.
- polling_ms	: If devfreq device uses the simple_ondemand governor,
		  polling_ms is necessary for the period. (unit: millisecond)
- cur_freq_hz	: Current Frequency (unit: hz)
- old_freq_hz	: Frequency before changing. (unit: hz)
- new_freq_hz	: Frequency after changed. (unit: hz)

[For example on Exynos5422-based Odroid-XU3 board]
- In order to show the multiple governors on devfreq_summay result,
change the governor of devfreq0 from simple_ondemand to userspace.

$ cat /sys/kernel/debug/devfreq/devfreq_summary
dev_name                       dev        parent_dev governor        polling_ms cur_freq_hz  min_freq_hz  max_freq_hz
------------------------------ ---------- ---------- --------------- ---------- ------------ ------------ ------------
10c20000.memory-controller     devfreq0              userspace       0          206000000    165000000    825000000
soc:bus_wcore                  devfreq1              simple_ondemand 50         532000000    88700000     532000000
soc:bus_noc                    devfreq2   devfreq1   passive         0          111000000    66600000     111000000
soc:bus_fsys_apb               devfreq3   devfreq1   passive         0          222000000    111000000    222000000
soc:bus_fsys                   devfreq4   devfreq1   passive         0          200000000    75000000     200000000
soc:bus_fsys2                  devfreq5   devfreq1   passive         0          200000000    75000000     200000000
soc:bus_mfc                    devfreq6   devfreq1   passive         0          333000000    83250000     333000000
soc:bus_gen                    devfreq7   devfreq1   passive         0          266000000    88700000     266000000
soc:bus_peri                   devfreq8   devfreq1   passive         0          66600000     66600000     66600000
soc:bus_g2d                    devfreq9   devfreq1   passive         0          0            83250000     333000000
soc:bus_g2d_acp                devfreq10  devfreq1   passive         0          0            66500000     266000000
soc:bus_jpeg                   devfreq11  devfreq1   passive         0          0            75000000     300000000
soc:bus_jpeg_apb               devfreq12  devfreq1   passive         0          0            83250000     166500000
soc:bus_disp1_fimd             devfreq13  devfreq1   passive         0          0            120000000    200000000
soc:bus_disp1                  devfreq14  devfreq1   passive         0          0            120000000    300000000
soc:bus_gscl_scaler            devfreq15  devfreq1   passive         0          0            150000000    300000000
soc:bus_mscl                   devfreq16  devfreq1   passive         0          0            84000000     666000000

Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
---
 drivers/devfreq/devfreq.c | 80 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

Comments

Bjorn Andersson Jan. 7, 2020, 9:15 p.m. UTC | #1
On Tue 07 Jan 01:05 PST 2020, Chanwoo Choi wrote:

> Add debugfs interface to provide debugging information of devfreq device.
> It contains 'devfreq_summary' entry to show the summary of registered
> devfreq devices as following and the additional debugfs file will be added.
> - /sys/kernel/debug/devfreq/devfreq_summary
> 
> [Detailed description of each field of 'devfreq_summary' debugfs file]
> - dev_name	: Device name of h/w.
> - dev		: Device name made by devfreq core.
> - parent_dev	: If devfreq device uses the passive governor,
> 		  show parent devfreq device name.
> - governor	: Devfreq governor.
> - polling_ms	: If devfreq device uses the simple_ondemand governor,
> 		  polling_ms is necessary for the period. (unit: millisecond)
> - cur_freq_hz	: Current Frequency (unit: hz)
> - old_freq_hz	: Frequency before changing. (unit: hz)
> - new_freq_hz	: Frequency after changed. (unit: hz)
> 
> [For example on Exynos5422-based Odroid-XU3 board]
> - In order to show the multiple governors on devfreq_summay result,
> change the governor of devfreq0 from simple_ondemand to userspace.
> 
> $ cat /sys/kernel/debug/devfreq/devfreq_summary
> dev_name                       dev        parent_dev governor        polling_ms cur_freq_hz  min_freq_hz  max_freq_hz
> ------------------------------ ---------- ---------- --------------- ---------- ------------ ------------ ------------
> 10c20000.memory-controller     devfreq0              userspace       0          206000000    165000000    825000000
> soc:bus_wcore                  devfreq1              simple_ondemand 50         532000000    88700000     532000000
> soc:bus_noc                    devfreq2   devfreq1   passive         0          111000000    66600000     111000000
> soc:bus_fsys_apb               devfreq3   devfreq1   passive         0          222000000    111000000    222000000
> soc:bus_fsys                   devfreq4   devfreq1   passive         0          200000000    75000000     200000000
> soc:bus_fsys2                  devfreq5   devfreq1   passive         0          200000000    75000000     200000000
> soc:bus_mfc                    devfreq6   devfreq1   passive         0          333000000    83250000     333000000
> soc:bus_gen                    devfreq7   devfreq1   passive         0          266000000    88700000     266000000
> soc:bus_peri                   devfreq8   devfreq1   passive         0          66600000     66600000     66600000
> soc:bus_g2d                    devfreq9   devfreq1   passive         0          0            83250000     333000000
> soc:bus_g2d_acp                devfreq10  devfreq1   passive         0          0            66500000     266000000
> soc:bus_jpeg                   devfreq11  devfreq1   passive         0          0            75000000     300000000
> soc:bus_jpeg_apb               devfreq12  devfreq1   passive         0          0            83250000     166500000
> soc:bus_disp1_fimd             devfreq13  devfreq1   passive         0          0            120000000    200000000
> soc:bus_disp1                  devfreq14  devfreq1   passive         0          0            120000000    300000000
> soc:bus_gscl_scaler            devfreq15  devfreq1   passive         0          0            150000000    300000000
> soc:bus_mscl                   devfreq16  devfreq1   passive         0          0            84000000     666000000

This looks quite useful.

> 
> Reported-by: kbuild test robot <lkp@intel.com>

May I ask how the build test robot came up with this idea?

> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
> ---
>  drivers/devfreq/devfreq.c | 80 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 80 insertions(+)
> 
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
[..]
> +/**
> + * devfreq_summary_show() - Show the summary of the registered devfreq devices
> + *				via 'devfreq_summary' debugfs file.

Please make this proper kerneldoc, i.e:

 * func() - short description
 * @s:
 * @data:
 * 
 * Long description
 * 
 * Return: foo on bar

[..]
> @@ -1733,6 +1803,16 @@ static int __init devfreq_init(void)
>  	}
>  	devfreq_class->dev_groups = devfreq_groups;
>  
> +	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
> +	if (PTR_ERR(devfreq_debugfs) != -ENODEV && IS_ERR(devfreq_debugfs)) {

Don't PTR_ERR() before IS_ERR().

> +		devfreq_debugfs = NULL;

I don't think you need this, given that debugfs_create_file() will fail
gracefully when passed and IS_ERR()

> +		pr_warn("%s: couldn't create debugfs dir\n", __FILE__);

Afaict debugfs_create_() won't fail without printing a message already.

> +	} else {
> +		debugfs_create_file("devfreq_summary", 0444,
> +				devfreq_debugfs, NULL,
> +				&devfreq_summary_fops);
> +	}
> +
>  	return 0;
>  }
>  subsys_initcall(devfreq_init);

Regards,
Bjorn
Chanwoo Choi Jan. 8, 2020, 7:56 a.m. UTC | #2
On 1/8/20 6:15 AM, Bjorn Andersson wrote:
> On Tue 07 Jan 01:05 PST 2020, Chanwoo Choi wrote:
> 
>> Add debugfs interface to provide debugging information of devfreq device.
>> It contains 'devfreq_summary' entry to show the summary of registered
>> devfreq devices as following and the additional debugfs file will be added.
>> - /sys/kernel/debug/devfreq/devfreq_summary
>>
>> [Detailed description of each field of 'devfreq_summary' debugfs file]
>> - dev_name	: Device name of h/w.
>> - dev		: Device name made by devfreq core.
>> - parent_dev	: If devfreq device uses the passive governor,
>> 		  show parent devfreq device name.
>> - governor	: Devfreq governor.
>> - polling_ms	: If devfreq device uses the simple_ondemand governor,
>> 		  polling_ms is necessary for the period. (unit: millisecond)
>> - cur_freq_hz	: Current Frequency (unit: hz)
>> - old_freq_hz	: Frequency before changing. (unit: hz)
>> - new_freq_hz	: Frequency after changed. (unit: hz)
>>
>> [For example on Exynos5422-based Odroid-XU3 board]
>> - In order to show the multiple governors on devfreq_summay result,
>> change the governor of devfreq0 from simple_ondemand to userspace.
>>
>> $ cat /sys/kernel/debug/devfreq/devfreq_summary
>> dev_name                       dev        parent_dev governor        polling_ms cur_freq_hz  min_freq_hz  max_freq_hz
>> ------------------------------ ---------- ---------- --------------- ---------- ------------ ------------ ------------
>> 10c20000.memory-controller     devfreq0              userspace       0          206000000    165000000    825000000
>> soc:bus_wcore                  devfreq1              simple_ondemand 50         532000000    88700000     532000000
>> soc:bus_noc                    devfreq2   devfreq1   passive         0          111000000    66600000     111000000
>> soc:bus_fsys_apb               devfreq3   devfreq1   passive         0          222000000    111000000    222000000
>> soc:bus_fsys                   devfreq4   devfreq1   passive         0          200000000    75000000     200000000
>> soc:bus_fsys2                  devfreq5   devfreq1   passive         0          200000000    75000000     200000000
>> soc:bus_mfc                    devfreq6   devfreq1   passive         0          333000000    83250000     333000000
>> soc:bus_gen                    devfreq7   devfreq1   passive         0          266000000    88700000     266000000
>> soc:bus_peri                   devfreq8   devfreq1   passive         0          66600000     66600000     66600000
>> soc:bus_g2d                    devfreq9   devfreq1   passive         0          0            83250000     333000000
>> soc:bus_g2d_acp                devfreq10  devfreq1   passive         0          0            66500000     266000000
>> soc:bus_jpeg                   devfreq11  devfreq1   passive         0          0            75000000     300000000
>> soc:bus_jpeg_apb               devfreq12  devfreq1   passive         0          0            83250000     166500000
>> soc:bus_disp1_fimd             devfreq13  devfreq1   passive         0          0            120000000    200000000
>> soc:bus_disp1                  devfreq14  devfreq1   passive         0          0            120000000    300000000
>> soc:bus_gscl_scaler            devfreq15  devfreq1   passive         0          0            150000000    300000000
>> soc:bus_mscl                   devfreq16  devfreq1   passive         0          0            84000000     666000000
> 
> This looks quite useful.
> 
>>
>> Reported-by: kbuild test robot <lkp@intel.com>
> 
> May I ask how the build test robot came up with this idea?

I'm missing the detailed what are the reported by kbuild test robot.
It reported the possible build error. I'll add the following description
on next version:
	[lkp: Reported the build error]

> 
>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> ---
>>  drivers/devfreq/devfreq.c | 80 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 80 insertions(+)
>>
>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> [..]
>> +/**
>> + * devfreq_summary_show() - Show the summary of the registered devfreq devices
>> + *				via 'devfreq_summary' debugfs file.
> 
> Please make this proper kerneldoc, i.e:
> 
>  * func() - short description
>  * @s:
>  * @data:
>  * 
>  * Long description
>  * 
>  * Return: foo on bar

OK.

> 
> [..]
>> @@ -1733,6 +1803,16 @@ static int __init devfreq_init(void)
>>  	}
>>  	devfreq_class->dev_groups = devfreq_groups;
>>  
>> +	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
>> +	if (PTR_ERR(devfreq_debugfs) != -ENODEV && IS_ERR(devfreq_debugfs)) {
> 
> Don't PTR_ERR() before IS_ERR().

Before checking IS_ERR(), have to check the PTR_ERR(devfreq_debugfs)
is -ENODEV or not because debugfs_create_dir return the '-ENODEV'
if DEBUG_FS is disabled. If return the -ENODEV, it is not error.

> 
>> +		devfreq_debugfs = NULL;
> 
> I don't think you need this, given that debugfs_create_file() will fail
> gracefully when passed and IS_ERR()

Right. Jut on patch2 checks the 'devfreq_debugfs' is NULL or not
in order to catch the DEBUG_FS is well working for devfreq.
Anyway, it would be better to add it to patch2 because devfreq_debugfs
is not used when failed to create debugfs dir.

> 
>> +		pr_warn("%s: couldn't create debugfs dir\n", __FILE__);
> 
> Afaict debugfs_create_() won't fail without printing a message already.

It just print the error message when DEBUG_FS is enabled
and debugfs_create_dir() returns the error.

> 
>> +	} else {
>> +		debugfs_create_file("devfreq_summary", 0444,
>> +				devfreq_debugfs, NULL,
>> +				&devfreq_summary_fops);
>> +	}
>> +
>>  	return 0;
>>  }
>>  subsys_initcall(devfreq_init);
> 
> Regards,
> Bjorn
> 
>
Dmitry Osipenko Jan. 8, 2020, 2:14 p.m. UTC | #3
08.01.2020 10:56, Chanwoo Choi пишет:
> On 1/8/20 6:15 AM, Bjorn Andersson wrote:
>> On Tue 07 Jan 01:05 PST 2020, Chanwoo Choi wrote:
>>
>>> Add debugfs interface to provide debugging information of devfreq device.
>>> It contains 'devfreq_summary' entry to show the summary of registered
>>> devfreq devices as following and the additional debugfs file will be added.
>>> - /sys/kernel/debug/devfreq/devfreq_summary
>>>
>>> [Detailed description of each field of 'devfreq_summary' debugfs file]
>>> - dev_name	: Device name of h/w.
>>> - dev		: Device name made by devfreq core.
>>> - parent_dev	: If devfreq device uses the passive governor,
>>> 		  show parent devfreq device name.
>>> - governor	: Devfreq governor.
>>> - polling_ms	: If devfreq device uses the simple_ondemand governor,
>>> 		  polling_ms is necessary for the period. (unit: millisecond)
>>> - cur_freq_hz	: Current Frequency (unit: hz)
>>> - old_freq_hz	: Frequency before changing. (unit: hz)
>>> - new_freq_hz	: Frequency after changed. (unit: hz)
>>>
>>> [For example on Exynos5422-based Odroid-XU3 board]
>>> - In order to show the multiple governors on devfreq_summay result,
>>> change the governor of devfreq0 from simple_ondemand to userspace.
>>>
>>> $ cat /sys/kernel/debug/devfreq/devfreq_summary
>>> dev_name                       dev        parent_dev governor        polling_ms cur_freq_hz  min_freq_hz  max_freq_hz
>>> ------------------------------ ---------- ---------- --------------- ---------- ------------ ------------ ------------
>>> 10c20000.memory-controller     devfreq0              userspace       0          206000000    165000000    825000000
>>> soc:bus_wcore                  devfreq1              simple_ondemand 50         532000000    88700000     532000000
>>> soc:bus_noc                    devfreq2   devfreq1   passive         0          111000000    66600000     111000000
>>> soc:bus_fsys_apb               devfreq3   devfreq1   passive         0          222000000    111000000    222000000
>>> soc:bus_fsys                   devfreq4   devfreq1   passive         0          200000000    75000000     200000000
>>> soc:bus_fsys2                  devfreq5   devfreq1   passive         0          200000000    75000000     200000000
>>> soc:bus_mfc                    devfreq6   devfreq1   passive         0          333000000    83250000     333000000
>>> soc:bus_gen                    devfreq7   devfreq1   passive         0          266000000    88700000     266000000
>>> soc:bus_peri                   devfreq8   devfreq1   passive         0          66600000     66600000     66600000
>>> soc:bus_g2d                    devfreq9   devfreq1   passive         0          0            83250000     333000000
>>> soc:bus_g2d_acp                devfreq10  devfreq1   passive         0          0            66500000     266000000
>>> soc:bus_jpeg                   devfreq11  devfreq1   passive         0          0            75000000     300000000
>>> soc:bus_jpeg_apb               devfreq12  devfreq1   passive         0          0            83250000     166500000
>>> soc:bus_disp1_fimd             devfreq13  devfreq1   passive         0          0            120000000    200000000
>>> soc:bus_disp1                  devfreq14  devfreq1   passive         0          0            120000000    300000000
>>> soc:bus_gscl_scaler            devfreq15  devfreq1   passive         0          0            150000000    300000000
>>> soc:bus_mscl                   devfreq16  devfreq1   passive         0          0            84000000     666000000
>>
>> This looks quite useful.
>>
>>>
>>> Reported-by: kbuild test robot <lkp@intel.com>
>>
>> May I ask how the build test robot came up with this idea?
> 
> I'm missing the detailed what are the reported by kbuild test robot.
> It reported the possible build error. I'll add the following description
> on next version:
> 	[lkp: Reported the build error]
> 
>>
>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> ---
>>>  drivers/devfreq/devfreq.c | 80 +++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>> [..]
>>> +/**
>>> + * devfreq_summary_show() - Show the summary of the registered devfreq devices
>>> + *				via 'devfreq_summary' debugfs file.
>>
>> Please make this proper kerneldoc, i.e:
>>
>>  * func() - short description
>>  * @s:
>>  * @data:
>>  * 
>>  * Long description
>>  * 
>>  * Return: foo on bar
> 
> OK.
> 
>>
>> [..]
>>> @@ -1733,6 +1803,16 @@ static int __init devfreq_init(void)
>>>  	}
>>>  	devfreq_class->dev_groups = devfreq_groups;
>>>  
>>> +	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
>>> +	if (PTR_ERR(devfreq_debugfs) != -ENODEV && IS_ERR(devfreq_debugfs)) {
>>
>> Don't PTR_ERR() before IS_ERR().
> 
> Before checking IS_ERR(), have to check the PTR_ERR(devfreq_debugfs)
> is -ENODEV or not because debugfs_create_dir return the '-ENODEV'
> if DEBUG_FS is disabled. If return the -ENODEV, it is not error.

You could write it this way:

	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
	err = PTR_ERR_OR_ZERO(devfreq_debugfs);
	if (err && err != -ENODEV) {
	...
Chanwoo Choi Jan. 13, 2020, 4:45 a.m. UTC | #4
On 1/8/20 11:14 PM, Dmitry Osipenko wrote:
> 08.01.2020 10:56, Chanwoo Choi пишет:
>> On 1/8/20 6:15 AM, Bjorn Andersson wrote:
>>> On Tue 07 Jan 01:05 PST 2020, Chanwoo Choi wrote:
>>>
>>>> Add debugfs interface to provide debugging information of devfreq device.
>>>> It contains 'devfreq_summary' entry to show the summary of registered
>>>> devfreq devices as following and the additional debugfs file will be added.
>>>> - /sys/kernel/debug/devfreq/devfreq_summary
>>>>
>>>> [Detailed description of each field of 'devfreq_summary' debugfs file]
>>>> - dev_name	: Device name of h/w.
>>>> - dev		: Device name made by devfreq core.
>>>> - parent_dev	: If devfreq device uses the passive governor,
>>>> 		  show parent devfreq device name.
>>>> - governor	: Devfreq governor.
>>>> - polling_ms	: If devfreq device uses the simple_ondemand governor,
>>>> 		  polling_ms is necessary for the period. (unit: millisecond)
>>>> - cur_freq_hz	: Current Frequency (unit: hz)
>>>> - old_freq_hz	: Frequency before changing. (unit: hz)
>>>> - new_freq_hz	: Frequency after changed. (unit: hz)
>>>>
>>>> [For example on Exynos5422-based Odroid-XU3 board]
>>>> - In order to show the multiple governors on devfreq_summay result,
>>>> change the governor of devfreq0 from simple_ondemand to userspace.
>>>>
>>>> $ cat /sys/kernel/debug/devfreq/devfreq_summary
>>>> dev_name                       dev        parent_dev governor        polling_ms cur_freq_hz  min_freq_hz  max_freq_hz
>>>> ------------------------------ ---------- ---------- --------------- ---------- ------------ ------------ ------------
>>>> 10c20000.memory-controller     devfreq0              userspace       0          206000000    165000000    825000000
>>>> soc:bus_wcore                  devfreq1              simple_ondemand 50         532000000    88700000     532000000
>>>> soc:bus_noc                    devfreq2   devfreq1   passive         0          111000000    66600000     111000000
>>>> soc:bus_fsys_apb               devfreq3   devfreq1   passive         0          222000000    111000000    222000000
>>>> soc:bus_fsys                   devfreq4   devfreq1   passive         0          200000000    75000000     200000000
>>>> soc:bus_fsys2                  devfreq5   devfreq1   passive         0          200000000    75000000     200000000
>>>> soc:bus_mfc                    devfreq6   devfreq1   passive         0          333000000    83250000     333000000
>>>> soc:bus_gen                    devfreq7   devfreq1   passive         0          266000000    88700000     266000000
>>>> soc:bus_peri                   devfreq8   devfreq1   passive         0          66600000     66600000     66600000
>>>> soc:bus_g2d                    devfreq9   devfreq1   passive         0          0            83250000     333000000
>>>> soc:bus_g2d_acp                devfreq10  devfreq1   passive         0          0            66500000     266000000
>>>> soc:bus_jpeg                   devfreq11  devfreq1   passive         0          0            75000000     300000000
>>>> soc:bus_jpeg_apb               devfreq12  devfreq1   passive         0          0            83250000     166500000
>>>> soc:bus_disp1_fimd             devfreq13  devfreq1   passive         0          0            120000000    200000000
>>>> soc:bus_disp1                  devfreq14  devfreq1   passive         0          0            120000000    300000000
>>>> soc:bus_gscl_scaler            devfreq15  devfreq1   passive         0          0            150000000    300000000
>>>> soc:bus_mscl                   devfreq16  devfreq1   passive         0          0            84000000     666000000
>>>
>>> This looks quite useful.
>>>
>>>>
>>>> Reported-by: kbuild test robot <lkp@intel.com>
>>>
>>> May I ask how the build test robot came up with this idea?
>>
>> I'm missing the detailed what are the reported by kbuild test robot.
>> It reported the possible build error. I'll add the following description
>> on next version:
>> 	[lkp: Reported the build error]
>>
>>>
>>>> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>> ---
>>>>  drivers/devfreq/devfreq.c | 80 +++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 80 insertions(+)
>>>>
>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>> [..]
>>>> +/**
>>>> + * devfreq_summary_show() - Show the summary of the registered devfreq devices
>>>> + *				via 'devfreq_summary' debugfs file.
>>>
>>> Please make this proper kerneldoc, i.e:
>>>
>>>  * func() - short description
>>>  * @s:
>>>  * @data:
>>>  * 
>>>  * Long description
>>>  * 
>>>  * Return: foo on bar
>>
>> OK.
>>
>>>
>>> [..]
>>>> @@ -1733,6 +1803,16 @@ static int __init devfreq_init(void)
>>>>  	}
>>>>  	devfreq_class->dev_groups = devfreq_groups;
>>>>  
>>>> +	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
>>>> +	if (PTR_ERR(devfreq_debugfs) != -ENODEV && IS_ERR(devfreq_debugfs)) {
>>>
>>> Don't PTR_ERR() before IS_ERR().
>>
>> Before checking IS_ERR(), have to check the PTR_ERR(devfreq_debugfs)
>> is -ENODEV or not because debugfs_create_dir return the '-ENODEV'
>> if DEBUG_FS is disabled. If return the -ENODEV, it is not error.
> 
> You could write it this way:
> 
> 	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
> 	err = PTR_ERR_OR_ZERO(devfreq_debugfs);
> 	if (err && err != -ENODEV) {

It is same result between your suggestion and following statement'
without any separate local variable. 

	if (IS_ERR(devfreq_debugfs) && PTR_ERR(devfreq_debugfs) != -ENODEV)


> 	...
> 
>
diff mbox series

Patch

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 254f11b31824..c7f5e4e06420 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -10,6 +10,7 @@ 
 #include <linux/kernel.h>
 #include <linux/kmod.h>
 #include <linux/sched.h>
+#include <linux/debugfs.h>
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/init.h>
@@ -33,6 +34,7 @@ 
 #define HZ_PER_KHZ	1000
 
 static struct class *devfreq_class;
+static struct dentry *devfreq_debugfs;
 
 /*
  * devfreq core provides delayed work based load monitoring helper
@@ -1717,6 +1719,74 @@  static struct attribute *devfreq_attrs[] = {
 };
 ATTRIBUTE_GROUPS(devfreq);
 
+/**
+ * devfreq_summary_show() - Show the summary of the registered devfreq devices
+ *				via 'devfreq_summary' debugfs file.
+ */
+static int devfreq_summary_show(struct seq_file *s, void *data)
+{
+	struct devfreq *devfreq;
+	struct devfreq *p_devfreq = NULL;
+	unsigned long cur_freq, min_freq, max_freq;
+	unsigned int polling_ms;
+
+	seq_printf(s, "%-30s %-10s %-10s %-15s %-10s %-12s %-12s %-12s\n",
+			"dev_name",
+			"dev",
+			"parent_dev",
+			"governor",
+			"polling_ms",
+			"cur_freq_hz",
+			"min_freq_hz",
+			"max_freq_hz");
+	seq_printf(s, "%-30s %-10s %-10s %-15s %-10s %-12s %-12s %-12s\n",
+			"------------------------------",
+			"----------",
+			"----------",
+			"---------------",
+			"----------",
+			"------------",
+			"------------",
+			"------------");
+
+	mutex_lock(&devfreq_list_lock);
+
+	list_for_each_entry_reverse(devfreq, &devfreq_list, node) {
+#if IS_ENABLED(CONFIG_DEVFREQ_GOV_PASSIVE)
+		if (!strncmp(devfreq->governor_name, DEVFREQ_GOV_PASSIVE,
+							DEVFREQ_NAME_LEN)) {
+			struct devfreq_passive_data *data = devfreq->data;
+
+			if (data)
+				p_devfreq = data->parent;
+		} else {
+			p_devfreq = NULL;
+		}
+#endif
+		mutex_lock(&devfreq->lock);
+		cur_freq = devfreq->previous_freq,
+		get_freq_range(devfreq, &min_freq, &max_freq);
+		polling_ms = devfreq->profile->polling_ms,
+		mutex_unlock(&devfreq->lock);
+
+		seq_printf(s,
+			"%-30s %-10s %-10s %-15s %-10d %-12ld %-12ld %-12ld\n",
+			dev_name(devfreq->dev.parent),
+			dev_name(&devfreq->dev),
+			p_devfreq ? dev_name(&p_devfreq->dev) : "",
+			devfreq->governor_name,
+			polling_ms,
+			cur_freq,
+			min_freq,
+			max_freq);
+	}
+
+	mutex_unlock(&devfreq_list_lock);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(devfreq_summary);
+
 static int __init devfreq_init(void)
 {
 	devfreq_class = class_create(THIS_MODULE, "devfreq");
@@ -1733,6 +1803,16 @@  static int __init devfreq_init(void)
 	}
 	devfreq_class->dev_groups = devfreq_groups;
 
+	devfreq_debugfs = debugfs_create_dir("devfreq", NULL);
+	if (PTR_ERR(devfreq_debugfs) != -ENODEV && IS_ERR(devfreq_debugfs)) {
+		devfreq_debugfs = NULL;
+		pr_warn("%s: couldn't create debugfs dir\n", __FILE__);
+	} else {
+		debugfs_create_file("devfreq_summary", 0444,
+				devfreq_debugfs, NULL,
+				&devfreq_summary_fops);
+	}
+
 	return 0;
 }
 subsys_initcall(devfreq_init);