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

From: Chanwoo Choi
Date: Tue Nov 26 2019 - 03:29:39 EST


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
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

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


Best Regards,
Chanwoo Choi