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

[Chanwoo Choi]

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