| Message ID | 20201007050703.20759-1-cw00.choi@samsung.com |
|---|---|
| Headers | show |
| Series | PM / devfreq: Add governor feature and attribute flag | expand |
... > diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq > index deefffb3bbe4..67af3f31e17c 100644 > --- a/Documentation/ABI/testing/sysfs-class-devfreq > +++ b/Documentation/ABI/testing/sysfs-class-devfreq > @@ -37,20 +37,6 @@ Description: > The /sys/class/devfreq/.../target_freq shows the next governor > predicted target frequency of the corresponding devfreq object. > > -What: /sys/class/devfreq/.../polling_interval > -Date: September 2011 > -Contact: MyungJoo Ham <myungjoo.ham@samsung.com> > -Description: > - The /sys/class/devfreq/.../polling_interval shows and sets > - the requested polling interval of the corresponding devfreq > - object. The values are represented in ms. If the value is > - less than 1 jiffy, it is considered to be 0, which means > - no polling. This value is meaningless if the governor is > - not polling; thus. If the governor is not using > - devfreq-provided central polling > - (/sys/class/devfreq/.../central_polling is 0), this value > - may be useless. > - > What: /sys/class/devfreq/.../trans_stat > Date: October 2012 > Contact: MyungJoo Ham <myungjoo.ham@samsung.com> > @@ -65,14 +51,6 @@ Description: > as following: > echo 0 > /sys/class/devfreq/.../trans_stat > > -What: /sys/class/devfreq/.../userspace/set_freq > -Date: September 2011 > -Contact: MyungJoo Ham <myungjoo.ham@samsung.com> > -Description: > - The /sys/class/devfreq/.../userspace/set_freq shows and > - sets the requested frequency for the devfreq object if > - userspace governor is in effect. > - > What: /sys/class/devfreq/.../available_frequencies > Date: October 2012 > Contact: Nishanth Menon <nm@ti.com> > @@ -109,6 +87,35 @@ Description: > The max_freq overrides min_freq because max_freq may be > used to throttle devices to avoid overheating. > > +What: /sys/class/devfreq/.../polling_interval > +Date: September 2011 > +Contact: MyungJoo Ham <myungjoo.ham@samsung.com> > +Description: > + The /sys/class/devfreq/.../polling_interval shows and sets > + the requested polling interval of the corresponding devfreq > + object. The values are represented in ms. If the value is > + less than 1 jiffy, it is considered to be 0, which means > + no polling. This value is meaningless if the governor is > + not polling; thus. If the governor is not using > + devfreq-provided central polling > + (/sys/class/devfreq/.../central_polling is 0), this value > + may be useless. > + > + A list of governors that support the node: > + - simple_ondmenad > + - tegra_actmon > + > +What: /sys/class/devfreq/.../userspace/set_freq > +Date: September 2011 > +Contact: MyungJoo Ham <myungjoo.ham@samsung.com> > +Description: > + The /sys/class/devfreq/.../userspace/set_freq shows and > + sets the requested frequency for the devfreq object if > + userspace governor is in effect. > + > + A list of governors that support the node: > + - userspace > + > What: /sys/class/devfreq/.../timer > Date: July 2020 > Contact: Chanwoo Choi <cw00.choi@samsung.com> > @@ -120,3 +127,6 @@ Description: > as following: > echo deferrable > /sys/class/devfreq/.../timer > echo delayed > /sys/class/devfreq/.../timer > + > + A list of governors that support the node: > + - simple_ondemand Hello, Chanwoo! Could you please explain the reason of changing the doc? It looks like you only added the lists of governors, but is it a really useful change? Are you going to keep these lists up-to-date?
... > @@ -1361,6 +1373,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, > goto out; > } > > + remove_sysfs_files(df, df->governor); > + create_sysfs_files(df, governor); > + > prev_governor = df->governor; > df->governor = governor; > strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN); > @@ -1460,39 +1475,6 @@ static ssize_t target_freq_show(struct device *dev, > } The further code may revert df->governor to the prev_governor or set it to NULL. The create_sysfs_files(df->governor) should be invoked at the very end of the governor_store() and only in a case of success.
On 10/19/20 9:38 AM, Dmitry Osipenko wrote: > ... >> diff --git a/Documentation/ABI/testing/sysfs-class-devfreq b/Documentation/ABI/testing/sysfs-class-devfreq >> index deefffb3bbe4..67af3f31e17c 100644 >> --- a/Documentation/ABI/testing/sysfs-class-devfreq >> +++ b/Documentation/ABI/testing/sysfs-class-devfreq >> @@ -37,20 +37,6 @@ Description: >> The /sys/class/devfreq/.../target_freq shows the next governor >> predicted target frequency of the corresponding devfreq object. >> >> -What: /sys/class/devfreq/.../polling_interval >> -Date: September 2011 >> -Contact: MyungJoo Ham <myungjoo.ham@samsung.com> >> -Description: >> - The /sys/class/devfreq/.../polling_interval shows and sets >> - the requested polling interval of the corresponding devfreq >> - object. The values are represented in ms. If the value is >> - less than 1 jiffy, it is considered to be 0, which means >> - no polling. This value is meaningless if the governor is >> - not polling; thus. If the governor is not using >> - devfreq-provided central polling >> - (/sys/class/devfreq/.../central_polling is 0), this value >> - may be useless. >> - >> What: /sys/class/devfreq/.../trans_stat >> Date: October 2012 >> Contact: MyungJoo Ham <myungjoo.ham@samsung.com> >> @@ -65,14 +51,6 @@ Description: >> as following: >> echo 0 > /sys/class/devfreq/.../trans_stat >> >> -What: /sys/class/devfreq/.../userspace/set_freq >> -Date: September 2011 >> -Contact: MyungJoo Ham <myungjoo.ham@samsung.com> >> -Description: >> - The /sys/class/devfreq/.../userspace/set_freq shows and >> - sets the requested frequency for the devfreq object if >> - userspace governor is in effect. >> - >> What: /sys/class/devfreq/.../available_frequencies >> Date: October 2012 >> Contact: Nishanth Menon <nm@ti.com> >> @@ -109,6 +87,35 @@ Description: >> The max_freq overrides min_freq because max_freq may be >> used to throttle devices to avoid overheating. >> >> +What: /sys/class/devfreq/.../polling_interval >> +Date: September 2011 >> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com> >> +Description: >> + The /sys/class/devfreq/.../polling_interval shows and sets >> + the requested polling interval of the corresponding devfreq >> + object. The values are represented in ms. If the value is >> + less than 1 jiffy, it is considered to be 0, which means >> + no polling. This value is meaningless if the governor is >> + not polling; thus. If the governor is not using >> + devfreq-provided central polling >> + (/sys/class/devfreq/.../central_polling is 0), this value >> + may be useless. >> + >> + A list of governors that support the node: >> + - simple_ondmenad >> + - tegra_actmon >> + >> +What: /sys/class/devfreq/.../userspace/set_freq >> +Date: September 2011 >> +Contact: MyungJoo Ham <myungjoo.ham@samsung.com> >> +Description: >> + The /sys/class/devfreq/.../userspace/set_freq shows and >> + sets the requested frequency for the devfreq object if >> + userspace governor is in effect. >> + >> + A list of governors that support the node: >> + - userspace >> + >> What: /sys/class/devfreq/.../timer >> Date: July 2020 >> Contact: Chanwoo Choi <cw00.choi@samsung.com> >> @@ -120,3 +127,6 @@ Description: >> as following: >> echo deferrable > /sys/class/devfreq/.../timer >> echo delayed > /sys/class/devfreq/.../timer >> + >> + A list of governors that support the node: >> + - simple_ondemand > > Hello, Chanwoo! > > Could you please explain the reason of changing the doc? It looks like > you only added the lists of governors, but is it a really useful change? > Are you going to keep these lists up-to-date? I think that is is useful. Because user cannot know why specific sysfs node (like 'timer') is absence according to governor. So, in order to remove the user confusion, better to add the information to documentation.
On 10/19/20 9:39 AM, Dmitry Osipenko wrote: > ... >> @@ -1361,6 +1373,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, >> goto out; >> } >> >> + remove_sysfs_files(df, df->governor); >> + create_sysfs_files(df, governor); >> + >> prev_governor = df->governor; >> df->governor = governor; >> strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN); >> @@ -1460,39 +1475,6 @@ static ssize_t target_freq_show(struct device *dev, >> } > > The further code may revert df->governor to the prev_governor or set it prev_governor is better. I'll change it. > to NULL. The create_sysfs_files(df->governor) should be invoked at the > very end of the governor_store() and only in a case of success. OK. I'll add more exception handling code.
On 10/19/20 1:11 PM, Chanwoo Choi wrote: > On 10/19/20 9:39 AM, Dmitry Osipenko wrote: >> ... >>> @@ -1361,6 +1373,9 @@ static ssize_t governor_store(struct device *dev, struct device_attribute *attr, >>> goto out; >>> } >>> >>> + remove_sysfs_files(df, df->governor); >>> + create_sysfs_files(df, governor); >>> + >>> prev_governor = df->governor; >>> df->governor = governor; >>> strncpy(df->governor_name, governor->name, DEVFREQ_NAME_LEN); >>> @@ -1460,39 +1475,6 @@ static ssize_t target_freq_show(struct device *dev, >>> } >> >> The further code may revert df->governor to the prev_governor or set it > > prev_governor is better. I'll change it. > >> to NULL. The create_sysfs_files(df->governor) should be invoked at the Also, when creating and removing the sysfs files, devfreq instance is needed because of df->dev.kobj. So, *_sysfs_files need the two parameters. >> very end of the governor_store() and only in a case of success. > > OK. I'll add more exception handling code. > > -- Best Regards, Chanwoo Choi Samsung Electronics
Each devfreq governor can have the different sysfs attributes and features. In order to provide the only available sysfs attribute to user-space, add governor attribute flag with DEVFREQ_GOV_ATTR_[attribute name] defintion. Also, each governor is able to have the specific flag in order to support specific feature with DEVFREQ_GOV_FLAG_[feature name] defintion like immutable governor. According to each governor, can initiate the governor feature and attribute flags. [Common sysfs attributes for devfreq class] And all devfreq governors have to support the following common attributes. The common attributes are added to devfreq class by default. - governor - available_governors - available_frequencies - cur_freq - target_freq - min_freq - max_freq - trans_stat [Definition for governor attribute flag] - DEVFREQ_GOV_ATTR_POLLING_INTERVAL to update polling interval for timer. : /sys/class/devfreq/[devfreq dev name]/polling_interval - DEVFREQ_GOV_ATTR_TIMER to change the type of timer on either deferrable or dealyed timer. : /sys/class/devfreq/[devfreq dev name]/timer [Definition for governor feature flag] - DEVFREQ_GOV_FLAG_IMMUTABLE : If immutable flag is set, governor is never changeable to other governors. - DEVFREQ_GOV_FLAG_IRQ_DRIVEN : Devfreq core won't schedule polling work for this governor if value is set. [Table of governor attribute flags for evfreq governors] ----------------------------------------------------------------------------- | simple | perfor | power | user | passive | tegra30 | ondemand | mance | save | space| | ------------------------------------------------------------------------------ governor | O | O | O | O | O | O available_governors | O | O | O | O | O | O available_frequencies | O | O | O | O | O | O cur_freq | O | O | O | O | O | O target_freq | O | O | O | O | O | O min_freq | O | O | O | O | O | O max_freq | O | O | O | O | O | O trans_stat | O | O | O | O | O | O ------------------------------------------------------------------------------ polling_interval | O | X | X | X | X | O timer | O | X | X | X | X | X ------------------------------------------------------------------------------ immutable | X | X | X | X | O | O interrupt_driven | X(polling)| X | X | X | X | O (irq) ------------------------------------------------------------------------------ Changes from v2: - https://lkml.org/lkml/2020/7/13/285 ("[PATCH v2 0/2] PM / devfreq: Add governor flags") - Hide unsupported sysfs node to user-space instead of checking the permission of sysfs node. Chanwoo Choi (2): PM / devfreq: Add governor feature flag PM / devfreq: Add governor attribute flag for specifc sysfs nodes Documentation/ABI/testing/sysfs-class-devfreq | 54 +++--- drivers/devfreq/devfreq.c | 170 +++++++++++------- drivers/devfreq/governor.h | 30 +++- drivers/devfreq/governor_passive.c | 2 +- drivers/devfreq/governor_simpleondemand.c | 2 + drivers/devfreq/tegra30-devfreq.c | 5 +- 6 files changed, 171 insertions(+), 92 deletions(-)