Re: [PATCH v3] PM / devfreq: Add new name attribute for sysfs

From: Chanwoo Choi
Date: Tue Nov 26 2019 - 04:31:35 EST

Next message: Ingo Molnar: "Re: [GIT PULL] x86/asm changes for v5.5"
Previous message: Oleksij Rempel: "[PATCH v2] net: dsa: sja1105: print info about probed chip only after everything was done."
In reply to: Greg KH: "Re: [PATCH v3] PM / devfreq: Add new name attribute for sysfs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On 11/26/19 6:15 PM, Greg KH wrote:
> On Tue, Nov 26, 2019 at 05:35:28PM +0900, Chanwoo Choi wrote:
>> On 11/26/19 4:53 PM, Greg KH wrote:
>>> On Tue, Nov 26, 2019 at 12:08:18PM +0900, Chanwoo Choi wrote:
>>>> Hi Greg,
>>>>
>>>> On 11/25/19 5:50 PM, Greg KH wrote:
>>>>> On Mon, Nov 25, 2019 at 10:03:57AM +0900, Chanwoo Choi wrote:
>>>>>> The commit 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X) for
>>>>>> sysfs") changed the node name to devfreq(x). After this commit, it is not
>>>>>> possible to get the device name through /sys/class/devfreq/devfreq(X)/*.
>>>>>>
>>>>>> Add new name attribute in order to get device name.
>>>>>>
>>>>>> Cc: stable@xxxxxxxxxxxxxxx
>>>>>> Fixes: 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X) for sysfs")
>>>>>> Signed-off-by: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
>>>>>> ---
>>>>>> Changes from v2:
>>>>>> - Change the order of name_show() according to the sequence in devfreq_attrs[]
>>>>>>
>>>>>> Changes from v1:
>>>>>> - Update sysfs-class-devfreq documentation
>>>>>> - Show device name directly from 'devfreq->dev.parent'
>>>>>>
>>>>>
>>>>> Shouldn't you just revert the original patch here? Why did the sysfs
>>>>> file change?
>>>>
>>>> The initial devfreq code used the parent device name for device name
>>>> corresponding to devfreq object instead of 'devfreq%d' style.
>>>> Before applied The commit 4585fbcb5331 ("PM / devfreq: Modify
>>>> the device name as devfreq(X) for sysfs"), the devfreq sysfs
>>>> showed the parent device name as following:
>>>>
>>>> For example on Odroid-XU3 board before applied the commit 4585fbcb5331,
>>>> /sys/class/devfreq/soc:bus_wcore
>>>> /sys/class/devfreq/soc:bus_noc
>>>> ...(skip)
>>>>
>>>>
>>>> But, I think that devfreq subsystem had to show the consistent
>>>> sysfs entry name for devfreq device like input, thermal, hwmon subsystem.
>>>>
>>>> For example on Odroid-XU3 board,
>>>> - The input subsystem show the 'input%d' style for input device.
>>>> $root@localhost:/# ls /sys/class/input/
>>>> event0 event1 input0 input1 mice mouse0
>>>>
>>>> - The thermal subsystem show the 'cooling_device%d' style for thermal cooling device.
>>>> $ root@localhost:/# ls /sys/class/thermal/
>>>> cooling_device0 cooling_device2 thermal_zone1 thermal_zone3
>>>> cooling_device1 thermal_zone0 thermal_zone2 thermal_zone4
>>>>
>>>> - The hwmon subsystem show the 'hwmon%d' style for h/w monitor device.
>>>> $root@localhost:/# ls /sys/class/hwmon/
>>>> hwmon0
>>>>
>>>>
>>>> So, I tried to make the consistent sysfs entry name for devfreq device
>>>> by contributing commit 4585fbcb5331 ("PM / devfreq: Modify the device name as
>>>> devfreq(X) for sysfs"). But, The commit 4585fbcb5331 have missed that sysfs
>>>> interface had to provide the real device name. Some subsystem like thermal
>>>> and hwmon provide the device type or device name through sysfs interface.
>>>> It is possible to make the user to find their own specific device by iteration
>>>> on user-space.
>>>>
>>>> root@localhost:/# cat /sys/class/thermal/cooling_device0/type
>>>> pwm-fan
>>>> root@localhost:/# cat /sys/class/thermal/cooling_device1/type
>>>> thermal-cpufreq-0
>>>> root@localhost:/# cat /sys/class/thermal/cooling_device2/type
>>>> thermal-cpufreq-1
>>>>
>>>> root@localhost:/# cat /sys/class/hwmon/hwmon0/name
>>>> pwmfan
>>>>
>>>>
>>>> So, I add the new 'name' attribute of sysfs for devfreq device.
>>>>
>>>>>
>>>>>> Documentation/ABI/testing/sysfs-class-devfreq | 7 +++++++
>>>>>> drivers/devfreq/devfreq.c | 9 +++++++++
>>>>>> 2 files changed, 16 insertions(+)
>>>>>>
>>>>>> diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq
>>>>>> index 01196e19afca..75897e2fde43 100644
>>>>>> --- a/Documentation/ABI/testing/sysfs-class-devfreq
>>>>>> +++ b/Documentation/ABI/testing/sysfs-class-devfreq
>>>>>> @@ -7,6 +7,13 @@ Description:
>>>>>> The name of devfreq object denoted as ... is same as the
>>>>>> name of device using devfreq.
>>>>>>
>>>>>> +What: /sys/class/devfreq/.../name
>>>>>> +Date: November 2019
>>>>>> +Contact: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
>>>>>> +Description:
>>>>>> + The /sys/class/devfreq/.../name shows the name of device
>>>>>> + of the corresponding devfreq object.
>>>>>> +
>>>>>> What: /sys/class/devfreq/.../governor
>>>>>> Date: September 2011
>>>>>> Contact: MyungJoo Ham <myungjoo.ham@xxxxxxxxxxx>
>>>>>> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
>>>>>> index 65a4b6cf3fa5..6f4d93d2a651 100644
>>>>>> --- a/drivers/devfreq/devfreq.c
>>>>>> +++ b/drivers/devfreq/devfreq.c
>>>>>> @@ -1169,6 +1169,14 @@ int devfreq_remove_governor(struct devfreq_governor *governor)
>>>>>> }
>>>>>> EXPORT_SYMBOL(devfreq_remove_governor);
>>>>>>
>>>>>> +static ssize_t name_show(struct device *dev,
>>>>>> + struct device_attribute *attr, char *buf)
>>>>>> +{
>>>>>> + struct devfreq *devfreq = to_devfreq(dev);
>>>>>> + return sprintf(buf, "%s\n", dev_name(devfreq->dev.parent));
>>>>>
>>>>> Why is the parent's name being set here, and not the device name?
>>>>
>>>> The device name style in struct devfreq is 'devfreq%d' instead of
>>>> parent device name in order to keep the consistent naming style for devfreq device
>>>> as I mentioned above. 'devfreq%d' name is consistent name style for devfreq device.
>>>> But, it don't show the real h/w device name. So, show the parent device name
>>>> which is specified on device-tree file.
>>>
>>> I'm sorry, but I still do not understand. Can you show me the directory
>>> tree before and after here?
>>>
>>
>> I'm sorry for not enough description. I add the following example on Odroid-XU3 board.
>>
>>
>> 1. Before applied commit 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X),
>>
>> root@localhost:~# ls /sys/class/devfreq
>> soc:bus_disp1 soc:bus_fsys_apb soc:bus_gscl_scaler soc:bus_mscl
>> soc:bus_disp1_fimd soc:bus_g2d soc:bus_jpeg soc:bus_noc
>> soc:bus_fsys soc:bus_g2d_acp soc:bus_jpeg_apb soc:bus_peri
>> soc:bus_fsys2 soc:bus_gen soc:bus_mfc soc:bus_wcore
>>
>> root@localhost:~# ls -al /sys/class/devfreq
>> total 0
>> drwxr-xr-x 2 root root 0 Jan 1 09:00 .
>> drwxr-xr-x 52 root root 0 Jan 1 09:00 ..
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_disp1 -> ../../devices/platform/soc/soc:bus_disp1/devfreq/soc:bus_disp1
>
> Ah, that's odd, ok.
>
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_disp1_fimd -> ../../devices/platform/soc/soc:bus_disp1_fimd/devfreq/soc:bus_did
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_fsys -> ../../devices/platform/soc/soc:bus_fsys/devfreq/soc:bus_fsys
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_fsys2 -> ../../devices/platform/soc/soc:bus_fsys2/devfreq/soc:bus_fsys2
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_fsys_apb -> ../../devices/platform/soc/soc:bus_fsys_apb/devfreq/soc:bus_fsys_ab
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_g2d -> ../../devices/platform/soc/soc:bus_g2d/devfreq/soc:bus_g2d
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_g2d_acp -> ../../devices/platform/soc/soc:bus_g2d_acp/devfreq/soc:bus_g2d_acp
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_gen -> ../../devices/platform/soc/soc:bus_gen/devfreq/soc:bus_gen
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_gscl_scaler -> ../../devices/platform/soc/soc:bus_gscl_scaler/devfreq/soc:bus_r
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_jpeg -> ../../devices/platform/soc/soc:bus_jpeg/devfreq/soc:bus_jpeg
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_jpeg_apb -> ../../devices/platform/soc/soc:bus_jpeg_apb/devfreq/soc:bus_jpeg_ab
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_mfc -> ../../devices/platform/soc/soc:bus_mfc/devfreq/soc:bus_mfc
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_mscl -> ../../devices/platform/soc/soc:bus_mscl/devfreq/soc:bus_mscl
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_noc -> ../../devices/platform/soc/soc:bus_noc/devfreq/soc:bus_noc
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_peri -> ../../devices/platform/soc/soc:bus_peri/devfreq/soc:bus_peri
>> lrwxrwxrwx 1 root root 0 Jan 1 09:00 soc:bus_wcore -> ../../devices/platform/soc/soc:bus_wcore/devfreq/soc:bus_wcore
>>
>>
>>
>> 2. After applied commit 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X),
>>
>> root@localhost:~# ls /sys/class/devfreq
>> devfreq0 devfreq11 devfreq14 devfreq3 devfreq6 devfreq9
>> devfreq1 devfreq12 devfreq15 devfreq4 devfreq7
>> devfreq10 devfreq13 devfreq2 devfreq5 devfreq8
>
> That's better.
>
>>
>> root@localhost:~# ls -al /sys/class/devfreq
>> total 0
>> drwxr-xr-x 2 root root 0 Jan 1 09:02 .
>> drwxr-xr-x 52 root root 0 Jan 1 09:02 ..
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq0 -> ../../devices/platform/soc/soc:bus_wcore/devfreq/devfreq0
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq1 -> ../../devices/platform/soc/soc:bus_noc/devfreq/devfreq1
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq10 -> ../../devices/platform/soc/soc:bus_jpeg/devfreq/devfreq10
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq11 -> ../../devices/platform/soc/soc:bus_jpeg_apb/devfreq/devfreq11
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq12 -> ../../devices/platform/soc/soc:bus_disp1_fimd/devfreq/devfreq12
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq13 -> ../../devices/platform/soc/soc:bus_disp1/devfreq/devfreq13
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq14 -> ../../devices/platform/soc/soc:bus_gscl_scaler/devfreq/devfreq14
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq15 -> ../../devices/platform/soc/soc:bus_mscl/devfreq/devfreq15
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq2 -> ../../devices/platform/soc/soc:bus_fsys_apb/devfreq/devfreq2
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq3 -> ../../devices/platform/soc/soc:bus_fsys/devfreq/devfreq3
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq4 -> ../../devices/platform/soc/soc:bus_fsys2/devfreq/devfreq4
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq5 -> ../../devices/platform/soc/soc:bus_mfc/devfreq/devfreq5
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq6 -> ../../devices/platform/soc/soc:bus_gen/devfreq/devfreq6
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq7 -> ../../devices/platform/soc/soc:bus_peri/devfreq/devfreq7
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq8 -> ../../devices/platform/soc/soc:bus_g2d/devfreq/devfreq8
>> lrwxrwxrwx 1 root root 0 Jan 1 09:02 devfreq9 -> ../../devices/platform/soc/soc:bus_g2d_acp/devfreq/devfreq9
>
> Ok, this looks a bit better, but why is there the extra "devfreq"
> directory in there? That was in the original as well, but that feels
> odd.

What extra directory are you talking about? I didn't create
the any extra directory for devfreq.

If you mention the following 'devfreq' directory,
it is created by basic device driver code in linux kernel.
- /sys/devices/platform/soc/soc\:bus_wcore/devfreq/

or

Each devfreq0~15 directories indicates the each devfreq device.

For example, show the info of each path
root@localhost:~# ls -al /sys/devices/platform/soc/soc\:bus_wcore/
total 0
drwxr-xr-x 4 root root 0 Jan 1 09:56 .
drwxr-xr-x 109 root root 0 Jan 1 09:56 ..
drwxr-xr-x 3 root root 0 Jan 1 09:56 devfreq
lrwxrwxrwx 1 root root 0 Jan 1 09:57 driver -> ../../../../bus/platform/drivers/exynos-bus
-rw-r--r-- 1 root root 4096 Jan 1 09:57 driver_override
-r--r--r-- 1 root root 4096 Jan 1 09:57 modalias
lrwxrwxrwx 1 root root 0 Jan 1 09:57 of_node -> ../../../../firmware/devicetree/base/soc/bus_wcore
drwxr-xr-x 2 root root 0 Jan 1 09:57 power
lrwxrwxrwx 1 root root 0 Jan 1 09:56 subsystem -> ../../../../bus/platform
-rw-r--r-- 1 root root 4096 Jan 1 09:56 uevent

root@localhost:~# ls -al /sys/devices/platform/soc/soc\:bus_wcore/devfreq/
total 0
drwxr-xr-x 3 root root 0 Jan 1 09:56 .
drwxr-xr-x 4 root root 0 Jan 1 09:56 ..
drwxr-xr-x 3 root root 0 Jan 1 09:56 devfreq0

root@localhost:~# ls -al /sys/devices/platform/soc/soc\:bus_wcore/devfreq/devfreq0
drwxr-xr-x 3 root root 0 Jan 1 09:56 .
drwxr-xr-x 3 root root 0 Jan 1 09:56 ..
-r--r--r-- 1 root root 4096 Jan 1 10:03 available_frequencies
-r--r--r-- 1 root root 4096 Jan 1 10:03 available_governors
-r--r--r-- 1 root root 4096 Jan 1 10:03 cur_freq
lrwxrwxrwx 1 root root 0 Jan 1 10:03 device -> ../../../soc:bus_wcore
-rw-r--r-- 1 root root 4096 Jan 1 10:03 governor
-rw-r--r-- 1 root root 4096 Jan 1 10:03 max_freq
-rw-r--r-- 1 root root 4096 Jan 1 10:03 min_freq
-r--r--r-- 1 root root 4096 Jan 1 10:03 name
-rw-r--r-- 1 root root 4096 Jan 1 10:03 polling_interval
drwxr-xr-x 2 root root 0 Jan 1 10:03 power
lrwxrwxrwx 1 root root 0 Jan 1 09:56 subsystem -> ../../../../../../class/devfreq
-r--r--r-- 1 root root 4096 Jan 1 10:03 target_freq
-r--r--r-- 1 root root 4096 Jan 1 10:03 trans_stat
-rw-r--r-- 1 root root 4096 Jan 1 09:56 uevent

--
Best Regards,
Chanwoo Choi
Samsung Electronics

Next message: Ingo Molnar: "Re: [GIT PULL] x86/asm changes for v5.5"
Previous message: Oleksij Rempel: "[PATCH v2] net: dsa: sja1105: print info about probed chip only after everything was done."
In reply to: Greg KH: "Re: [PATCH v3] PM / devfreq: Add new name attribute for sysfs"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]