[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20191126075333.GA1231308@kroah.com>
Date: Tue, 26 Nov 2019 08:53:33 +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 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?
thanks,
greg k-h
Powered by blists - more mailing lists