2012-06-13 04:43:34

by Xiaoguang Chen

[permalink] [raw]
Subject: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

Devfreq framework don't have a frequency table, add it
for easy use.

Signed-off-by: Xiaoguang Chen <[email protected]>
---
drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++
include/linux/devfreq.h | 12 ++++++++++++
2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 70c31d4..2144200 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -460,6 +460,17 @@ int devfreq_remove_device(struct devfreq *devfreq)
return 0;
}

+/*
+ * devfreq_set_freq_table()- Set frequency table for devfreq
+ * @devfreq The devfreq instance
+ * @table The frequency table that device supports
+ */
+void devfreq_set_freq_table(struct devfreq *devfreq,
+ struct devfreq_frequency_table *table)
+{
+ devfreq->freq_table = table;
+}
+
static ssize_t show_governor(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -472,6 +483,20 @@ static ssize_t show_freq(struct device *dev,
return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq);
}

+static ssize_t show_avail_freq(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int len = 0, i;
+ struct devfreq *devfreq = to_devfreq(dev);
+ if (devfreq->freq_table)
+ for (i = 0; devfreq->freq_table[i].frequency != DEVFREQ_TABLE_END; i++)
+ len += sprintf(buf + len, "%lu\n",
+ devfreq->freq_table[i].frequency);
+ if (len == 0)
+ len += sprintf(buf + len, "No frequency table is provided\n");
+ return len;
+}
+
static ssize_t show_polling_interval(struct device *dev,
struct device_attribute *attr, char *buf)
{
@@ -595,6 +620,7 @@ static struct device_attribute devfreq_attrs[] = {
store_polling_interval),
__ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
__ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
+ __ATTR(available_freqs, S_IRUGO, show_avail_freq, NULL),
{ },
};

diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
index 281c72a..e5e4036 100644
--- a/include/linux/devfreq.h
+++ b/include/linux/devfreq.h
@@ -52,6 +52,14 @@ struct devfreq_dev_status {
*/
#define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1

+#define DEVFREQ_ENTRY_INVALID (~0)
+#define DEVFREQ_TABLE_END (~1)
+
+struct devfreq_frequency_table {
+ unsigned int index;
+ unsigned long frequency;
+};
+
/**
* struct devfreq_dev_profile - Devfreq's user device profile
* @initial_freq The operating frequency when devfreq_add_device() is
@@ -130,6 +138,7 @@ struct devfreq_governor {
* "devfreq_monitor" executions to reevaluate
* frequency/voltage of the device. Set by
* profile's polling_ms interval.
+ * @freq_table The frequency table that device supports
* @data Private data of the governor. The devfreq framework does not
* touch this.
* @being_removed a flag to mark that this object is being removed in
@@ -157,6 +166,7 @@ struct devfreq {
unsigned long polling_jiffies;
unsigned long previous_freq;
unsigned int next_polling;
+ struct devfreq_frequency_table *freq_table;

void *data; /* private data for governors */

@@ -180,6 +190,8 @@ extern int devfreq_register_opp_notifier(struct device *dev,
struct devfreq *devfreq);
extern int devfreq_unregister_opp_notifier(struct device *dev,
struct devfreq *devfreq);
+extern void devfreq_set_freq_table(struct devfreq *devfreq,
+ struct devfreq_frequency_table *table);

#ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
extern const struct devfreq_governor devfreq_powersave;
--
1.7.0.4


2012-06-13 04:53:50

by MyungJoo Ham

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> Devfreq framework don't have a frequency table, add it
> for easy use.
>
> Signed-off-by: Xiaoguang Chen <[email protected]>

If you need a predefined data structure to support frequency table,
you can simply use OPP, which has helper functions implemented in
devfreq subsystem. Is there any reason not to use OPP and to implement
another data structure to store a frequency table attached to a device?


Cheers!
MyungJoo.

> ---
> drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++
> include/linux/devfreq.h | 12 ++++++++++++
> 2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 70c31d4..2144200 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -460,6 +460,17 @@ int devfreq_remove_device(struct devfreq *devfreq)
> return 0;
> }
>
> +/*
> + * devfreq_set_freq_table()- Set frequency table for devfreq
> + * @devfreq The devfreq instance
> + * @table The frequency table that device supports
> + */
> +void devfreq_set_freq_table(struct devfreq *devfreq,
> + struct devfreq_frequency_table *table)
> +{
> + devfreq->freq_table = table;
> +}
> +
> static ssize_t show_governor(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -472,6 +483,20 @@ static ssize_t show_freq(struct device *dev,
> return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq);
> }
>
> +static ssize_t show_avail_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int len = 0, i;
> + struct devfreq *devfreq = to_devfreq(dev);
> + if (devfreq->freq_table)
> + for (i = 0; devfreq->freq_table[i].frequency != DEVFREQ_TABLE_END; i++)
> + len += sprintf(buf + len, "%lu\n",
> + devfreq->freq_table[i].frequency);
> + if (len == 0)
> + len += sprintf(buf + len, "No frequency table is provided\n");
> + return len;
> +}
> +
> static ssize_t show_polling_interval(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -595,6 +620,7 @@ static struct device_attribute devfreq_attrs[] = {
> store_polling_interval),
> __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
> __ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
> + __ATTR(available_freqs, S_IRUGO, show_avail_freq, NULL),
> { },
> };
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 281c72a..e5e4036 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -52,6 +52,14 @@ struct devfreq_dev_status {
> */
> #define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1
>
> +#define DEVFREQ_ENTRY_INVALID (~0)
> +#define DEVFREQ_TABLE_END (~1)
> +
> +struct devfreq_frequency_table {
> + unsigned int index;
> + unsigned long frequency;
> +};
> +
> /**
> * struct devfreq_dev_profile - Devfreq's user device profile
> * @initial_freq The operating frequency when devfreq_add_device() is
> @@ -130,6 +138,7 @@ struct devfreq_governor {
> * "devfreq_monitor" executions to reevaluate
> * frequency/voltage of the device. Set by
> * profile's polling_ms interval.
> + * @freq_table The frequency table that device supports
> * @data Private data of the governor. The devfreq framework does not
> * touch this.
> * @being_removed a flag to mark that this object is being removed in
> @@ -157,6 +166,7 @@ struct devfreq {
> unsigned long polling_jiffies;
> unsigned long previous_freq;
> unsigned int next_polling;
> + struct devfreq_frequency_table *freq_table;
>
> void *data; /* private data for governors */
>
> @@ -180,6 +190,8 @@ extern int devfreq_register_opp_notifier(struct device *dev,
> struct devfreq *devfreq);
> extern int devfreq_unregister_opp_notifier(struct device *dev,
> struct devfreq *devfreq);
> +extern void devfreq_set_freq_table(struct devfreq *devfreq,
> + struct devfreq_frequency_table *table);
>
> #ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
> extern const struct devfreq_governor devfreq_powersave;
> --
> 1.7.0.4
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-06-14 04:43:52

by MyungJoo Ham

[permalink] [raw]
Subject: Re: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> Hi, Myungjoo
>
>
> what's your opinion?

Hello Xiaoguang,

Still, I don't think we need additional API and ABI for a simple frequency table. Why a devfreq device driver would want to register a table in struct devfreq while it can hold one either with its dev-data, private data of devfreq, or even OPP.

1. Devfreq is not "combined" with OPP. OPP is optional.

2. I guess filling voltage column with some arbitrary values in OPP table won't hurt anything if the device does not care voltage values. (just a suggestion and speculation) Thus, you can still use OPP in your case as long as the frequency values are discrete and not too many.

3. Devfreq and its governors recommends the base frequency to devfreq drivers. Frequency table is only needed to be visible to devfreq drivers, not to governors or devfreq itself. The frequency table you've suggested is not need to be visible to devfreq subsystem.


I still object to adding a frequency table (which is already supported by OPP by not specifying voltage or specifying arbitrary voltage values). However, even if I don't, we won't need that API (devfreq_set_freq_table), which should've been added in device profile at devfreq_add_device() time.


Cheers!
MyungJoo.

>
>
> Thanks
> Xiaoguang
>
>
> > 2012/6/13 Xiaoguang Chen <[email protected]>
> >
> > I think Devfreq should not be combined with OPP,
> > OPP framework does contain one frequency table, but the frequency is combined with voltage. some platforms may don't want to use this but handling voltage seperately in their clock driver.
> >
> >
> > and some platforms don't use OPP, and they want a frequency list.
> > then this is necessary. also devfreq should contain a frequency list even without any other frameworks, don't you think so ?
> >
> >
> > Thanks
> > Xiaoguang
> >
> >
> >
> > 2012/6/13 MyungJoo Ham <[email protected]>
> > >
> > > > Devfreq framework don't have a frequency table, add it
> > > > for easy use.
> > > >
> > > > Signed-off-by: Xiaoguang Chen <[email protected]>
> > >
> > >
> > > If you need a predefined data structure to support frequency table,
> > > you can simply use OPP, which has helper functions implemented in
> > > devfreq subsystem. Is there any reason not to use OPP and to implement
> > > another data structure to store a frequency table attached to a device?
> > >
> > >
> > > Cheers!
> > > MyungJoo.
> > > ????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-06-14 05:40:08

by Xiaoguang Chen

[permalink] [raw]
Subject: RE: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

Hi, Myungjoo

the API is optional, but I think one frequency table is better to have.
if user space want to see what is the supported frequencies for the specific devfreq driver, then where do you think we can see this interface?
do we have to go to OPP framework to get it ? or we can just add it in our devfreq ? for example: sys/class/devfreq/xxx-devfreq/available_freqs
I think it is best for us to see this in the same sysfs path.

Thanks
Xiaoguang


-----Original Message-----
From: ?Ը??? [mailto:[email protected]]
Sent: 2012Ҵ6??14?? 12:44
To: Xiaoguang Chen
Cc: Xiaoguang Chen; [email protected]; ?ڰ???; [email protected]
Subject: Re: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> Hi, Myungjoo
>
>
> what's your opinion?

Hello Xiaoguang,

Still, I don't think we need additional API and ABI for a simple frequency table. Why a devfreq device driver would want to register a table in struct devfreq while it can hold one either with its dev-data, private data of devfreq, or even OPP.

1. Devfreq is not "combined" with OPP. OPP is optional.

2. I guess filling voltage column with some arbitrary values in OPP table won't hurt anything if the device does not care voltage values. (just a suggestion and speculation) Thus, you can still use OPP in your case as long as the frequency values are discrete and not too many.

3. Devfreq and its governors recommends the base frequency to devfreq drivers. Frequency table is only needed to be visible to devfreq drivers, not to governors or devfreq itself. The frequency table you've suggested is not need to be visible to devfreq subsystem.


I still object to adding a frequency table (which is already supported by OPP by not specifying voltage or specifying arbitrary voltage values). However, even if I don't, we won't need that API (devfreq_set_freq_table), which should've been added in device profile at devfreq_add_device() time.


Cheers!
MyungJoo.

>
>
> Thanks
> Xiaoguang
>
>
> > 2012/6/13 Xiaoguang Chen <[email protected]>
> >
> > I think Devfreq should not be combined with OPP, OPP framework does
> > contain one frequency table, but the frequency is combined with voltage. some platforms may don't want to use this but handling voltage seperately in their clock driver.
> >
> >
> > and some platforms don't use OPP, and they want a frequency list.
> > then this is necessary. also devfreq should contain a frequency list even without any other frameworks, don't you think so ?
> >
> >
> > Thanks
> > Xiaoguang
> >
> >
> >
> > 2012/6/13 MyungJoo Ham <[email protected]>
> > >
> > > > Devfreq framework don't have a frequency table, add it for easy
> > > > use.
> > > >
> > > > Signed-off-by: Xiaoguang Chen <[email protected]>
> > >
> > >
> > > If you need a predefined data structure to support frequency
> > > table, you can simply use OPP, which has helper functions
> > > implemented in devfreq subsystem. Is there any reason not to use
> > > OPP and to implement another data structure to store a frequency table attached to a device?
> > >
> > >
> > > Cheers!
> > > MyungJoo.
> > >
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-06-19 05:04:55

by MyungJoo Ham

[permalink] [raw]
Subject: Re: RE: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> Hi, Myungjoo
>
> the API is optional, but I think one frequency table is better to have.
> if user space want to see what is the supported frequencies for the specific devfreq driver, then where do you think we can see this interface?
> do we have to go to OPP framework to get it ? or we can just add it in our devfreq ? for example: sys/class/devfreq/xxx-devfreq/available_freqs
> I think it is best for us to see this in the same sysfs path.
>
> Thanks
> Xiaoguang

Hi Xiaoguang,

First of all, both conceptually and practically, devfreq subsystem does not need to hold the list of available frequencies. Devfreq subsystem does not care whether a devfreq device has only two (or even one) frequencies available or several billions frequencies available, which is why OPP is dropped from devfreq subsystem and only helper functions are remaining.

The available frequencies information may be in OPP, clock subsystem, or the device driver itself. The listing of available frequencies should be provided by the one who has the information at first.

Anyway, except for the pure curiosity of users, why do we need to provide the list of available frequencies at the devfreq subsystem? If we are adding it in devfreq subsystem, we are adding an ABI and supporting data structure that are never used by the subsystem, but for printing out via ABI which only satisfies the curiosity of users that may already know what it would list up. There are even cases where the devfreq device driver itself also does not aware of avilable frequencies; when OPP is given outside.


Cheers!
MyungJoo.

>
>
> -----Original Message-----
> From: ?Ը??? [mailto:[email protected]]
> Sent: 2012Ҵ6??14?? 12:44
> To: Xiaoguang Chen
> Cc: Xiaoguang Chen; [email protected]; ?ڰ???; [email protected]
> Subject: Re: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs
>
> > Hi, Myungjoo
> >
> >
> > what's your opinion?
>
> Hello Xiaoguang,
>
> Still, I don't think we need additional API and ABI for a simple frequency table. Why a devfreq device driver would want to register a table in struct devfreq while it can hold one either with its dev-data, private data of devfreq, or even OPP.
>
> 1. Devfreq is not "combined" with OPP. OPP is optional.
>
> 2. I guess filling voltage column with some arbitrary values in OPP table won't hurt anything if the device does not care voltage values. (just a suggestion and speculation) Thus, you can still use OPP in your case as long as the frequency values are discrete and not too many.
>
> 3. Devfreq and its governors recommends the base frequency to devfreq drivers. Frequency table is only needed to be visible to devfreq drivers, not to governors or devfreq itself. The frequency table you've suggested is not need to be visible to devfreq subsystem.
>
>
> I still object to adding a frequency table (which is already supported by OPP by not specifying voltage or specifying arbitrary voltage values). However, even if I don't, we won't need that API (devfreq_set_freq_table), which should've been added in device profile at devfreq_add_device() time.
>
>
> Cheers!
> MyungJoo.
>
> >
> >
> > Thanks
> > Xiaoguang
> >
> >
> > > 2012/6/13 Xiaoguang Chen <[email protected]>
> > >
> > > I think Devfreq should not be combined with OPP, OPP framework does
> > > contain one frequency table, but the frequency is combined with voltage. some platforms may don't want to use this but handling voltage seperately in their clock driver.
> > >
> > >
> > > and some platforms don't use OPP, and they want a frequency list.
> > > then this is necessary. also devfreq should contain a frequency list even without any other frameworks, don't you think so ?
> > >
> > >
> > > Thanks
> > > Xiaoguang
> > >
> > >
> > >
> > > 2012/6/13 MyungJoo Ham <[email protected]>
> > > >
> > > > > Devfreq framework don't have a frequency table, add it for easy
> > > > > use.
> > > > >
> > > > > Signed-off-by: Xiaoguang Chen <[email protected]>
> > > >
> > > >
> > > > If you need a predefined data structure to support frequency
> > > > table, you can simply use OPP, which has helper functions
> > > > implemented in devfreq subsystem. Is there any reason not to use
> > > > OPP and to implement another data structure to store a frequency table attached to a device?
> > > >
> > > >
> > > > Cheers!
> > > > MyungJoo.
> > > >
>
>
>
>
>
>
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-06-19 05:34:30

by Xiaoguang Chen

[permalink] [raw]
Subject: RE: RE: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

OK, I get it.
Another question, do you guys ever want to add the devfreq governor dynamic change feature?

I mean we can switch governors for one devfreq driver dynamically in devfreq framework. current framework only supports statically compile the governor and can't change it dynamically.

Thanks
Xiaoguang


-----Original Message-----
From: ?Ը??? [mailto:[email protected]]
Sent: 2012Ҵ6??19?? 13:05
To: Xiaoguang Chen; Xiaoguang Chen
Cc: [email protected]; ?ڰ???; [email protected]; [email protected]
Subject: Re: RE: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> Hi, Myungjoo
>
> the API is optional, but I think one frequency table is better to have.
> if user space want to see what is the supported frequencies for the specific devfreq driver, then where do you think we can see this interface?
> do we have to go to OPP framework to get it ? or we can just add it in
> our devfreq ? for example:
> sys/class/devfreq/xxx-devfreq/available_freqs
> I think it is best for us to see this in the same sysfs path.
>
> Thanks
> Xiaoguang

Hi Xiaoguang,

First of all, both conceptually and practically, devfreq subsystem does not need to hold the list of available frequencies. Devfreq subsystem does not care whether a devfreq device has only two (or even one) frequencies available or several billions frequencies available, which is why OPP is dropped from devfreq subsystem and only helper functions are remaining.

The available frequencies information may be in OPP, clock subsystem, or the device driver itself. The listing of available frequencies should be provided by the one who has the information at first.

Anyway, except for the pure curiosity of users, why do we need to provide the list of available frequencies at the devfreq subsystem? If we are adding it in devfreq subsystem, we are adding an ABI and supporting data structure that are never used by the subsystem, but for printing out via ABI which only satisfies the curiosity of users that may already know what it would list up. There are even cases where the devfreq device driver itself also does not aware of avilable frequencies; when OPP is given outside.


Cheers!
MyungJoo.

>
>
> -----Original Message-----
> From: ?Ը??? [mailto:[email protected]]
> Sent: 2012Ҵ6??14?? 12:44
> To: Xiaoguang Chen
> Cc: Xiaoguang Chen; [email protected]; ?ڰ???;
> [email protected]
> Subject: Re: Re: [PATCH 1/2] PM: devfreq: add freq table and
> available_freqs
>
> > Hi, Myungjoo
> >
> >
> > what's your opinion?
>
> Hello Xiaoguang,
>
> Still, I don't think we need additional API and ABI for a simple frequency table. Why a devfreq device driver would want to register a table in struct devfreq while it can hold one either with its dev-data, private data of devfreq, or even OPP.
>
> 1. Devfreq is not "combined" with OPP. OPP is optional.
>
> 2. I guess filling voltage column with some arbitrary values in OPP table won't hurt anything if the device does not care voltage values. (just a suggestion and speculation) Thus, you can still use OPP in your case as long as the frequency values are discrete and not too many.
>
> 3. Devfreq and its governors recommends the base frequency to devfreq drivers. Frequency table is only needed to be visible to devfreq drivers, not to governors or devfreq itself. The frequency table you've suggested is not need to be visible to devfreq subsystem.
>
>
> I still object to adding a frequency table (which is already supported by OPP by not specifying voltage or specifying arbitrary voltage values). However, even if I don't, we won't need that API (devfreq_set_freq_table), which should've been added in device profile at devfreq_add_device() time.
>
>
> Cheers!
> MyungJoo.
>
> >
> >
> > Thanks
> > Xiaoguang
> >
> >
> > > 2012/6/13 Xiaoguang Chen <[email protected]>
> > >
> > > I think Devfreq should not be combined with OPP, OPP framework
> > > does contain one frequency table, but the frequency is combined with voltage. some platforms may don't want to use this but handling voltage seperately in their clock driver.
> > >
> > >
> > > and some platforms don't use OPP, and they want a frequency list.
> > > then this is necessary. also devfreq should contain a frequency list even without any other frameworks, don't you think so ?
> > >
> > >
> > > Thanks
> > > Xiaoguang
> > >
> > >
> > >
> > > 2012/6/13 MyungJoo Ham <[email protected]>
> > > >
> > > > > Devfreq framework don't have a frequency table, add it for
> > > > > easy use.
> > > > >
> > > > > Signed-off-by: Xiaoguang Chen <[email protected]>
> > > >
> > > >
> > > > If you need a predefined data structure to support frequency
> > > > table, you can simply use OPP, which has helper functions
> > > > implemented in devfreq subsystem. Is there any reason not to use
> > > > OPP and to implement another data structure to store a frequency table attached to a device?
> > > >
> > > >
> > > > Cheers!
> > > > MyungJoo.
> > > >
>
>
>
>
>
>
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-06-19 05:50:04

by MyungJoo Ham

[permalink] [raw]
Subject: Re: RE: RE: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> OK, I get it.
> Another question, do you guys ever want to add the devfreq governor dynamic change feature?
>
> I mean we can switch governors for one devfreq driver dynamically in devfreq framework. current framework only supports statically compile the governor and can't change it dynamically.
>
> Thanks
> Xiaoguang

Yes, that's on the TODO list.

But, we have done nothing to support it, yet, as you can see in git.infradead.org repository.


Cheers!
MyungJoo

????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-08-23 07:21:26

by MyungJoo Ham

[permalink] [raw]
Subject: Re: [PATCH 1/2] PM: devfreq: add freq table and available_freqs

> Devfreq framework don't have a frequency table, add it
> for easy use.
>
> Signed-off-by: Xiaoguang Chen <[email protected]>

As we are going to have transition statistics (similar with trans_table
and time_in_state of CPUfreq), we are going to have a data structure
that your suggested code may use.

Could you please rephrase your code to be based on Jonghwa's patch?
"devfreq: Add sysfs node for representing frequency transition information."

Cheers!
MyungJoo

> ---
> drivers/devfreq/devfreq.c | 26 ++++++++++++++++++++++++++
> include/linux/devfreq.h | 12 ++++++++++++
> 2 files changed, 38 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
> index 70c31d4..2144200 100644
> --- a/drivers/devfreq/devfreq.c
> +++ b/drivers/devfreq/devfreq.c
> @@ -460,6 +460,17 @@ int devfreq_remove_device(struct devfreq *devfreq)
> return 0;
> }
>
> +/*
> + * devfreq_set_freq_table()- Set frequency table for devfreq
> + * @devfreq The devfreq instance
> + * @table The frequency table that device supports
> + */
> +void devfreq_set_freq_table(struct devfreq *devfreq,
> + struct devfreq_frequency_table *table)
> +{
> + devfreq->freq_table = table;
> +}
> +
> static ssize_t show_governor(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -472,6 +483,20 @@ static ssize_t show_freq(struct device *dev,
> return sprintf(buf, "%lu\n", to_devfreq(dev)->previous_freq);
> }
>
> +static ssize_t show_avail_freq(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + int len = 0, i;
> + struct devfreq *devfreq = to_devfreq(dev);
> + if (devfreq->freq_table)
> + for (i = 0; devfreq->freq_table[i].frequency != DEVFREQ_TABLE_END; i++)
> + len += sprintf(buf + len, "%lu\n",
> + devfreq->freq_table[i].frequency);
> + if (len == 0)
> + len += sprintf(buf + len, "No frequency table is provided\n");
> + return len;
> +}
> +
> static ssize_t show_polling_interval(struct device *dev,
> struct device_attribute *attr, char *buf)
> {
> @@ -595,6 +620,7 @@ static struct device_attribute devfreq_attrs[] = {
> store_polling_interval),
> __ATTR(min_freq, S_IRUGO | S_IWUSR, show_min_freq, store_min_freq),
> __ATTR(max_freq, S_IRUGO | S_IWUSR, show_max_freq, store_max_freq),
> + __ATTR(available_freqs, S_IRUGO, show_avail_freq, NULL),
> { },
> };
>
> diff --git a/include/linux/devfreq.h b/include/linux/devfreq.h
> index 281c72a..e5e4036 100644
> --- a/include/linux/devfreq.h
> +++ b/include/linux/devfreq.h
> @@ -52,6 +52,14 @@ struct devfreq_dev_status {
> */
> #define DEVFREQ_FLAG_LEAST_UPPER_BOUND 0x1
>
> +#define DEVFREQ_ENTRY_INVALID (~0)
> +#define DEVFREQ_TABLE_END (~1)
> +
> +struct devfreq_frequency_table {
> + unsigned int index;
> + unsigned long frequency;
> +};
> +
> /**
> * struct devfreq_dev_profile - Devfreq's user device profile
> * @initial_freq The operating frequency when devfreq_add_device() is
> @@ -130,6 +138,7 @@ struct devfreq_governor {
> * "devfreq_monitor" executions to reevaluate
> * frequency/voltage of the device. Set by
> * profile's polling_ms interval.
> + * @freq_table The frequency table that device supports
> * @data Private data of the governor. The devfreq framework does not
> * touch this.
> * @being_removed a flag to mark that this object is being removed in
> @@ -157,6 +166,7 @@ struct devfreq {
> unsigned long polling_jiffies;
> unsigned long previous_freq;
> unsigned int next_polling;
> + struct devfreq_frequency_table *freq_table;
>
> void *data; /* private data for governors */
>
> @@ -180,6 +190,8 @@ extern int devfreq_register_opp_notifier(struct device *dev,
> struct devfreq *devfreq);
> extern int devfreq_unregister_opp_notifier(struct device *dev,
> struct devfreq *devfreq);
> +extern void devfreq_set_freq_table(struct devfreq *devfreq,
> + struct devfreq_frequency_table *table);
>
> #ifdef CONFIG_DEVFREQ_GOV_POWERSAVE
> extern const struct devfreq_governor devfreq_powersave;
> --
> 1.7.0.4
>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?