| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20191126091541.GB1371943@kroah.com>
Date: Tue, 26 Nov 2019 10:15:41 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: Chanwoo Choi <cw00.choi@...sung.com>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
rafael.j.wysocki@...el.com, myungjoo.ham@...sung.com,
kyungmin.park@...sung.com, chanwoo@...nel.org,
stable@...r.kernel.org
Subject: Re: [PATCH v3] PM / devfreq: Add new name attribute for sysfs
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@...r.kernel.org
> >>>> Fixes: 4585fbcb5331 ("PM / devfreq: Modify the device name as devfreq(X) for sysfs")
> >>>> Signed-off-by: Chanwoo Choi <cw00.choi@...sung.com>
> >>>> ---
> >>>> 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@...alhost:/# ls /sys/class/input/
> >> event0 event1 input0 input1 mice mouse0
> >>
> >> - The thermal subsystem show the 'cooling_device%d' style for thermal cooling device.
> >> $ root@...alhost:/# 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@...alhost:/# 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@...alhost:/# cat /sys/class/thermal/cooling_device0/type
> >> pwm-fan
> >> root@...alhost:/# cat /sys/class/thermal/cooling_device1/type
> >> thermal-cpufreq-0
> >> root@...alhost:/# cat /sys/class/thermal/cooling_device2/type
> >> thermal-cpufreq-1
> >>
> >> root@...alhost:/# 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@...sung.com>
> >>>> +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@...sung.com>
> >>>> 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@...alhost:~# 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@...alhost:~# 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@...alhost:~# ls /sys/class/devfreq
> devfreq0 devfreq11 devfreq14 devfreq3 devfreq6 devfreq9
> devfreq1 devfreq12 devfreq15 devfreq4 devfreq7
> devfreq10 devfreq13 devfreq2 devfreq5 devfreq8
That's better.
>
> root@...alhost:~# 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.
thanks,
greg k-h
Powered by blists - more mailing lists