Re: [PATCH] omap3: pm: cpufreq: populate l3 opp1 again
Menon, Nishanth wrote:
> We had removed the frequency for OPP1 L3 when we used to use frequency
> to enable/disable frequencies. It is better to populate the same
> instead of confusing future readers of the code. The OPP1 remains
> disabled as explained in the discussion.
IMHO the best way to unconfuse readers is to remove the entry.
>
> Discussion: http://marc.info/?t=12645382191&r=1&w=2
>
> Cc: Andrew Murray
> Cc: Benoit Cousson
> Cc: Kevin Hilman
>
> Signed-off-by: Nishanth Menon
> ---
> arch/arm/mach-omap2/cpufreq34xx.c |2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpufreq34xx.c
> b/arch/arm/mach-omap2/cpufreq34xx.c
> index 209af2b..aec1ffe 100644
> --- a/arch/arm/mach-omap2/cpufreq34xx.c
> +++ b/arch/arm/mach-omap2/cpufreq34xx.c
> @@ -43,7 +43,7 @@ static struct omap_opp_def __initdata
> omap34xx_mpu_rate_table[] = {
>
> static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
> /* OPP1 */
> - OMAP_OPP_DEF(false, 0, 975000),
> + OMAP_OPP_DEF(false, 4150, 975000),
> /* OPP2 */
> OMAP_OPP_DEF(true, 8300, 105),
> /* OPP3 */
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PM-WIP-OPP] [PATCH v3 1/1] Introducing enum for OPP types
Using omap_opp * to refer to domain types entails opp implementation
into maintaining pointers outside the opp layer.
as:
Since all we need is an identifier to a specific domain for
query/operational purposes, we introduce enum for identifying
OPP types instead of using opp layer's internal data structure
pointer.
Currently, OMAP3 is the only silicon supporting the OPP layer, hence
mpu_opps, l3_opps and dsp_opps are deprecated and replaced with
OPP_MPU, OPP_L3 and OPP_DSP respectively.
Signed-off-by: Romit Dasgupta
---
arch/arm/mach-omap2/pm34xx.c | 15 --
arch/arm/mach-omap2/resource34xx.c| 84 ++--
arch/arm/mach-omap2/smartreflex.c | 33 +-
arch/arm/plat-omap/common.c |6 -
arch/arm/plat-omap/cpu-omap.c | 12 +-
arch/arm/plat-omap/include/plat/opp.h | 60 +--
arch/arm/plat-omap/omap-pm-noop.c |4
arch/arm/plat-omap/omap-pm-srf.c |4
arch/arm/plat-omap/opp.c | 124
9 files changed, 172 insertions(+), 170 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5c751c1..0930aef 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1347,7 +1347,6 @@ static void __init configure_vc(void)
void __init omap3_pm_init_opp_table(void)
{
- int i;
struct omap_opp_def **omap3_opp_def_list;
struct omap_opp_def *omap34xx_opp_def_list[] = {
omap34xx_mpu_rate_table,
@@ -1359,19 +1358,13 @@ void __init omap3_pm_init_opp_table(void)
omap36xx_l3_rate_table,
omap36xx_dsp_rate_table
};
- struct omap_opp **omap3_rate_tables[] = {
- &mpu_opps,
- &l3_opps,
- &dsp_opps
- };
omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
omap34xx_opp_def_list;
- for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
- *omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
- /* We dont want half configured system at the moment */
- BUG_ON(IS_ERR(omap3_rate_tables[i]));
- }
+
+ BUG_ON(opp_init_list(OPP_MPU, omap3_opp_def_list[0]));
+ BUG_ON(opp_init_list(OPP_L3, omap3_opp_def_list[1]));
+ BUG_ON(opp_init_list(OPP_DSP, omap3_opp_def_list[2]));
}
static int __init omap3_pm_early_init(void)
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 157b38e..3604a38 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -161,7 +161,7 @@ static DEFINE_MUTEX(dvfs_mutex);
/**
* opp_to_freq - convert OPPID to frequency (DEPRECATED)
* @freq: return frequency back to caller
- * @opps: opp list
+ * @opp_type: OPP type where we need to look.
* @opp_id: OPP ID we are searching for
*
* return 0 and freq is populated if we find the opp_id, else,
@@ -169,14 +169,14 @@ static DEFINE_MUTEX(dvfs_mutex);
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated opp_to_freq(unsigned long *freq,
- const struct omap_opp *opps, u8 opp_id)
+static int __deprecated opp_to_freq(unsigned long *freq, enum opp_t opp_type,
+u8 opp_id)
{
struct omap_opp *opp;
- BUG_ON(!freq || !opps);
+ BUG_ON(!freq || opp_type >= OPP_TYPES_MAX);
- opp = opp_find_by_opp_id(opps, opp_id);
+ opp = opp_find_by_opp_id(opp_type, opp_id);
if (!opp)
return -EINVAL;
@@ -188,20 +188,20 @@ static int __deprecated opp_to_freq(unsigned long *freq,
/**
* freq_to_opp - convert a frequency back to OPP ID (DEPRECATED)
* @opp_id: opp ID returned back to caller
- * @opps: opp list
+ * @opp_type: OPP type where we need to look.
* @freq: frequency we are searching for
*
* return 0 and opp_id is populated if we find the freq, else, we return error
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
+static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t opp_type,
unsigned long freq)
{
struct omap_opp *opp;
- BUG_ON(!opp_id || !opps);
- opp = opp_find_freq_ceil(opps, &freq);
+ BUG_ON(opp_type >= OPP_TYPES_MAX);
+ opp = opp_find_freq_ceil(opp_type, &freq);
if (IS_ERR(opp))
return -EINVAL;
*opp_id = opp_get_opp_id(opp);
@@ -218,9 +218,6 @@ void init_opp(struct shared_resource *resp)
u8 opp_id;
resp->no_of_users = 0;
- if (!mpu_opps || !dsp_opps || !l3_opps)
- return;
-
/* Initialize the current level of the OPP resource
* to the opp set by u-boot.
*/
@@ -228,14 +225,14 @@ void init_opp(struct shared_reso
[PM-WIP-OPP] [PATCH v3 0/1] Introducing enum for OPP types
Resending after correcting the changelog. Thanks to Nishanth Menon for the correction. -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PM-WIP-OPP] [PATCH v2] Introducing enum for OPP types
Using omap_opp * to refer to domain types entails opp implementation
into maintaining pointers outside the opp layer.
Since all we need a identifying a specific domain for query/operational
purposes, we introduce enum for identifying OPP types instead of using
opp layer's internal data structure pointer.
Currently, OMAP3 is the only silicon supporting the OPP layer, hence
mpu_opps, l3_opps and dsp_opps are deprecated and replaced with
OPP_MPU, OPP_L3 and OPP_DSP respectively.
Signed-off-by: Romit Dasgupta
---
arch/arm/mach-omap2/pm34xx.c | 15 --
arch/arm/mach-omap2/resource34xx.c| 84 ++--
arch/arm/mach-omap2/smartreflex.c | 33 +-
arch/arm/plat-omap/common.c |6 -
arch/arm/plat-omap/cpu-omap.c | 12 +-
arch/arm/plat-omap/include/plat/opp.h | 60 +--
arch/arm/plat-omap/omap-pm-noop.c |4
arch/arm/plat-omap/omap-pm-srf.c |4
arch/arm/plat-omap/opp.c | 124
9 files changed, 172 insertions(+), 170 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5c751c1..0930aef 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1347,7 +1347,6 @@ static void __init configure_vc(void)
void __init omap3_pm_init_opp_table(void)
{
- int i;
struct omap_opp_def **omap3_opp_def_list;
struct omap_opp_def *omap34xx_opp_def_list[] = {
omap34xx_mpu_rate_table,
@@ -1359,19 +1358,13 @@ void __init omap3_pm_init_opp_table(void)
omap36xx_l3_rate_table,
omap36xx_dsp_rate_table
};
- struct omap_opp **omap3_rate_tables[] = {
- &mpu_opps,
- &l3_opps,
- &dsp_opps
- };
omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
omap34xx_opp_def_list;
- for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
- *omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
- /* We dont want half configured system at the moment */
- BUG_ON(IS_ERR(omap3_rate_tables[i]));
- }
+
+ BUG_ON(opp_init_list(OPP_MPU, omap3_opp_def_list[0]));
+ BUG_ON(opp_init_list(OPP_L3, omap3_opp_def_list[1]));
+ BUG_ON(opp_init_list(OPP_DSP, omap3_opp_def_list[2]));
}
static int __init omap3_pm_early_init(void)
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 157b38e..3604a38 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -161,7 +161,7 @@ static DEFINE_MUTEX(dvfs_mutex);
/**
* opp_to_freq - convert OPPID to frequency (DEPRECATED)
* @freq: return frequency back to caller
- * @opps: opp list
+ * @opp_type: OPP type where we need to look.
* @opp_id: OPP ID we are searching for
*
* return 0 and freq is populated if we find the opp_id, else,
@@ -169,14 +169,14 @@ static DEFINE_MUTEX(dvfs_mutex);
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated opp_to_freq(unsigned long *freq,
- const struct omap_opp *opps, u8 opp_id)
+static int __deprecated opp_to_freq(unsigned long *freq, enum opp_t opp_type,
+u8 opp_id)
{
struct omap_opp *opp;
- BUG_ON(!freq || !opps);
+ BUG_ON(!freq || opp_type >= OPP_TYPES_MAX);
- opp = opp_find_by_opp_id(opps, opp_id);
+ opp = opp_find_by_opp_id(opp_type, opp_id);
if (!opp)
return -EINVAL;
@@ -188,20 +188,20 @@ static int __deprecated opp_to_freq(unsigned long *freq,
/**
* freq_to_opp - convert a frequency back to OPP ID (DEPRECATED)
* @opp_id: opp ID returned back to caller
- * @opps: opp list
+ * @opp_type: OPP type where we need to look.
* @freq: frequency we are searching for
*
* return 0 and opp_id is populated if we find the freq, else, we return error
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
+static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t opp_type,
unsigned long freq)
{
struct omap_opp *opp;
- BUG_ON(!opp_id || !opps);
- opp = opp_find_freq_ceil(opps, &freq);
+ BUG_ON(opp_type >= OPP_TYPES_MAX);
+ opp = opp_find_freq_ceil(opp_type, &freq);
if (IS_ERR(opp))
return -EINVAL;
*opp_id = opp_get_opp_id(opp);
@@ -218,9 +218,6 @@ void init_opp(struct shared_resource *resp)
u8 opp_id;
resp->no_of_users = 0;
- if (!mpu_opps || !dsp_opps || !l3_opps)
- return;
-
/* Initialize the current level of the OPP resource
* to the opp set by u-boot.
*/
@@ -228,14 +225,14 @@ void init_opp(struct shared_resource *r
Re: [RFC] [PATCH 2/2] DSPBRIDGE: pm: use pm-wip-opp APIs for opp list
Menon, Nishanth wrote:
> Use the PM-WIP-OPP branch apis for generating the opp list for DSP.
> This allows for scaling accross 3430,3440,3450 and 3630 series.
>
> @@ -42,7 +56,90 @@ static struct dspbridge_platform_data dspbridge_pdata
> __initdata = {
> static int get_opp_table(struct dspbridge_platform_data *pdata)
> {
Is get_opp_table used anywhere other than dspbridge_init? I dont see any usage
outside this now.
Can't you make this __init?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq
> @@ -67,6 +92,9 @@ module_init(dspbridge_init);
>
> static void __exit dspbridge_exit(void)
> {
> + struct dspbridge_platform_data *pdata = &dspbridge_pdata;
> + kfree(pdata->mpu_speeds);
> + kfree(pdata->dsp_freq_table);
> platform_device_unregister(dspbridge_pdev);
> }
> module_exit(dspbridge_exit);
Ok if pdata->mpu_speeds, pdata->dsp_freq_table are NULL to start with, then you
need to make them NULL after kfree as well. Otherwise next time you insert the
module it may fail if pdata_mpu_speeds is initialized but pdata->dsp_freq_table
is not.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq
Romit Dasgupta wrote:
>> diff --git a/arch/arm/mach-omap2/dspbridge.c
>> b/arch/arm/mach-omap2/dspbridge.c
>> +
>> static int __init dspbridge_init(void)
>> {
>> struct platform_device *pdev;
>> @@ -48,6 +65,10 @@ static int __init dspbridge_init(void)
>> if (!pdev)
>> goto err_out;
>>
>> +err = get_opp_table(pdata);
>> +if (err)
>> +goto err_out;
>> +
>> err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
>> if (err)
>> goto err_out;
>> @@ -60,6 +81,10 @@ static int __init dspbridge_init(void)
>> return 0;
>>
>> err_out:
>> +kfree(pdata->mpu_speeds);
>> +kfree(pdata->dsp_freq_table);
> Are we sure that pdata->dsp_freq_table is NULL before initialization?
> Looking at your second part of the patch. You seem to invoke kfree for
> pdata->dsp_freq_table even if pdata->dsp_freq_table is not allocated.
> So my question is :
missed the last part of the mail. If pdata->dsp_freq_table is NULL to start
with. This is ok. Otherwise this needs to be changed.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] DSPBRIDGE: remove dependency of mpu freq
>
> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
> +
> static int __init dspbridge_init(void)
> {
> struct platform_device *pdev;
> @@ -48,6 +65,10 @@ static int __init dspbridge_init(void)
> if (!pdev)
> goto err_out;
>
> + err = get_opp_table(pdata);
> + if (err)
> + goto err_out;
> +
> err = platform_device_add_data(pdev, pdata, sizeof(*pdata));
> if (err)
> goto err_out;
> @@ -60,6 +81,10 @@ static int __init dspbridge_init(void)
> return 0;
>
> err_out:
> + kfree(pdata->mpu_speeds);
> + kfree(pdata->dsp_freq_table);
Are we sure that pdata->dsp_freq_table is NULL before initialization?
Looking at your second part of the patch. You seem to invoke kfree for
pdata->dsp_freq_table even if pdata->dsp_freq_table is not allocated.
So my question is :
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] DSPBRIDGE: pm: use old implementation for opps
>
> arch/arm/mach-omap2/dspbridge.c | 59
> ++-
> 1 files changed, 58 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/dspbridge.c b/arch/arm/mach-omap2/dspbridge.c
> index 26b860f..120d8a2 100644
> --- a/arch/arm/mach-omap2/dspbridge.c
> +++ b/arch/arm/mach-omap2/dspbridge.c
> @@ -42,7 +42,64 @@ static struct dspbridge_platform_data dspbridge_pdata
> __initdata = {
> static int get_opp_table(struct dspbridge_platform_data *pdata)
> {
> #ifdef CONFIG_BRIDGE_DVFS
> - /* Do nothing now - fill based on PM implementation */
> + /* legacy values for 3430 */
> + u32 vdd1_dsp_freq[6][4] = {
> + {0, 0, 0, 0},
> + /*OPP1*/
> + {0, 9, 0, 86000},
> + /*OPP2*/
> + {0, 18, 8, 17},
> + /*OPP3*/
> + {0, 36, 16, 34},
> + /*OPP4*/
> + {0, 396000, 325000, 376000},
> + /*OPP5*/
> + {0, 43, 355000, 43},
> + };
> + struct omap_opp vdd1_rate_table_bridge[] = {
> + {0, 0, 0},
> + /*OPP1*/
> + {S125M, VDD1_OPP1, 0},
> + /*OPP2*/
> + {S250M, VDD1_OPP2, 0},
> + /*OPP3*/
> + {S500M, VDD1_OPP3, 0},
> + /*OPP4*/
> + {S550M, VDD1_OPP4, 0},
> + /*OPP5*/
> + {S600M, VDD1_OPP5, 0},
> + };
> + pdata->dsp_num_speeds = VDD1_OPP5;
Why dont you use ARRAY_SIZE - 1 ?
> + pdata->mpu_speeds = kzalloc(sizeof(u32) * pdata->dsp_num_speeds,
> + GFP_KERNEL);
I understand pdata->dsp_num_speeds == pdata->mpu_num_speeds. But don't you think
passing pdata->mpu_num_speeds makes more sense here?
> + if (!pdata->mpu_speeds) {
> + pr_err("unable to allocate memory for the mpu"
> + "frequencies\n");
> + return -ENOMEM;
> + }
As I mentioned in my earlier email. You return to the caller here but you free
pdata->dsp_freq_table in the caller even if pdata->dsp_freq_table is not
allocated.
> + pdata->dsp_freq_table = kzalloc(
> + sizeof(struct dsp_shm_freq_table) *
> + (pdata->dsp_num_speeds + 1), GFP_KERNEL);
> + if (!pdata->dsp_freq_table) {
> + pr_err("unable to allocate memory for the dsp"
> + "frequencies\n");
> + return -ENOMEM;
> + }
> + for (i = 0; i < 6; i++)
Why are you hard coding numeric 6 here?
> + pdata->mpu_speed[i] = vdd1_rate_table_bridge[i].rate;
> + pdata->mpu_max_speed = pdata->mpu_speed[VDD1_OPP5];
You can use ARRAY_SIZE.
> + pdata->mpu_min_speed = pdata->mpu_speed[VDD1_OPP1];
> + pdata->dsp_num_speeds = VDD1_OPP5;
Same..ARRAY_SIZE...
> + for (i = 0; i <= pdata->dsp_num_speeds; i++) {
> + pdata->dsp_freq_table[i].u_volts =
> + vdd1_dsp_freq[i][0];
> + frequency = pdata->dsp_freq_table[i].dsp_freq =
> + frequency = vdd1_dsp_freq[i][1];
> + pdata->dsp_freq_table[i].thresh_min_freq =
> + vdd1_dsp_freq[i][2];
> + pdata->dsp_freq_table[i].thresh_max_freq =
> + vdd1_dsp_freq[i][3];
> + }
> #endif
> return 0;
> }
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
> Actually, system boots. It does not break the system. What I've tried is: > > - rx51 board > - omap3_pm_defconfig without CPU_FREQ > - linux-omap-pm/pm-wip-opp branch + this patch > > System boots, but smartreflex disables itself: > > ... > TCP cubic registered > NET: Registered protocol family 17 > NET: Registered protocol family 15 > Power Management for TI OMAP3. > SR: OPP rate tables not defined for platform, not enabling SmartReflex > VFP support v0.3: implementor 41 architecture 3 part 30 variant c rev 1 > ,.. > > But it does not break booting procedure. > > So, I don't see why this would be a reason for doing this compilation fix > into two steps. Unless you see another issue. Ok sorry about the misunderstanding then. Its ok from my side. ACKed. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
Eduardo Valentin wrote:
> On Wed, Jan 20, 2010 at 01:44:31PM +0100, ext Romit Dasgupta wrote:
>> Eduardo Valentin wrote:
>>> On Wed, Jan 20, 2010 at 01:00:05PM +0100, ext Romit Dasgupta wrote:
>>>> Eduardo Valentin wrote:
>>>>> On Wed, Jan 20, 2010 at 12:14:59PM +0100, ext Romit Dasgupta wrote:
>>>>>>> From: Eduardo Valentin
>>>>>>>
>>>>>>> OMAP OPP layer functions now have dependencies of CONFIG_CPU_FREQ only.
>>>>>>>
>>>>>>> With this patch, omap opp layer now has its compilation flags
>>>>>>> bound to CONFIG_CPU_FREQ. Also its code has been removed from pm34xx.c.
>>>>>>>
>>>>>>> A new file has been created to contain cpu freq code related to
>>>>>>> OMAP3: cpufreq34xx.c.
>>>>>>>
>>>>>>> Signed-off-by: Eduardo Valentin
>>>>>> NAK also for the following non-working kernel (smartreflex without
>>>>>> cpufreq).
>>>>> Then this is a problem of dependency with smartreflex and cpufreq. In
>>>>> this case
>>>>> better to solve this other problem with another patch. This patch is to
>>>>> rip off
>>>>> cpufreq code from pm34xx, as stated in the above description.
>>>> In that case the right fix should
>>> OK
>>>
>>>> 1) __not__ include opp.h for non cpufreq builds
>>> I guess the patch is "more or less" this option, as it maps
>>> all functions inside opp.h to nops if it is a cpufreq build.
>>> And all related real code is not compiled.
>>>
>>> Basically removes the opp layer code if cpufreq is disabled.
>>>
>> IMHO, making functions return 0 will give you run time issues when opp
>> related
>> APIs are called in non cpufreq path. So your patch will for the time being
>> solve
>> the build issue but not runtime issue. Hence I suggested to compile out
>> opp.h if
>> you are not using cpufreq. Solving a build issue is not the right fix for
>> this.
>
> No, they are cleaned up by compiler. The following:
>
> +static inline unsigned long omap_twl_vsel_to_uv(const u8 vsel)
> +{
> + return 0;
> +}
>
> will be translated to a nop in final kernel image.
>
> That's the whole idea of this patch.
>
>
Ok, what I mean is that it still solves build issue only. It does not produce a
bootable kernel without CPU_FREQ but with SMARTREFLEX. Smartreflex is almost
always there for a OMAP kernel. It is not a rarely used feature. So this patch
should have another part that fixes the Smartreflex issue!
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
Eduardo Valentin wrote: > On Wed, Jan 20, 2010 at 01:00:05PM +0100, ext Romit Dasgupta wrote: >> Eduardo Valentin wrote: >>> On Wed, Jan 20, 2010 at 12:14:59PM +0100, ext Romit Dasgupta wrote: >>>>> From: Eduardo Valentin >>>>> >>>>> OMAP OPP layer functions now have dependencies of CONFIG_CPU_FREQ only. >>>>> >>>>> With this patch, omap opp layer now has its compilation flags >>>>> bound to CONFIG_CPU_FREQ. Also its code has been removed from pm34xx.c. >>>>> >>>>> A new file has been created to contain cpu freq code related to >>>>> OMAP3: cpufreq34xx.c. >>>>> >>>>> Signed-off-by: Eduardo Valentin >>>> NAK also for the following non-working kernel (smartreflex without >>>> cpufreq). >>> Then this is a problem of dependency with smartreflex and cpufreq. In this >>> case >>> better to solve this other problem with another patch. This patch is to rip >>> off >>> cpufreq code from pm34xx, as stated in the above description. >> In that case the right fix should > > OK > >> 1) __not__ include opp.h for non cpufreq builds > > I guess the patch is "more or less" this option, as it maps > all functions inside opp.h to nops if it is a cpufreq build. > And all related real code is not compiled. > > Basically removes the opp layer code if cpufreq is disabled. > IMHO, making functions return 0 will give you run time issues when opp related APIs are called in non cpufreq path. So your patch will for the time being solve the build issue but not runtime issue. Hence I suggested to compile out opp.h if you are not using cpufreq. Solving a build issue is not the right fix for this. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
Eduardo Valentin wrote: > On Wed, Jan 20, 2010 at 12:14:59PM +0100, ext Romit Dasgupta wrote: >>> From: Eduardo Valentin >>> >>> OMAP OPP layer functions now have dependencies of CONFIG_CPU_FREQ only. >>> >>> With this patch, omap opp layer now has its compilation flags >>> bound to CONFIG_CPU_FREQ. Also its code has been removed from pm34xx.c. >>> >>> A new file has been created to contain cpu freq code related to >>> OMAP3: cpufreq34xx.c. >>> >>> Signed-off-by: Eduardo Valentin >> NAK also for the following non-working kernel (smartreflex without cpufreq). > > Then this is a problem of dependency with smartreflex and cpufreq. In this > case > better to solve this other problem with another patch. This patch is to rip > off > cpufreq code from pm34xx, as stated in the above description. In that case the right fix should 1) __not__ include opp.h for non cpufreq builds OR 2) If it includes opp.h the functions for getting current opp should abstract out the right mechanism to get current frequency/voltage from the chip/power IC. > > And sorry, I didn't get your boot log (if you intended to put it, as you said > "following non-working kernel" > Actually I meant for the following non-working kernel __configuration__. I do not have boot logs! -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
>>> Signed-off-by: Eduardo Valentin >>> --- >>> arch/arm/mach-omap2/Makefile |5 + >> NAK. Your kernel will not boot if you build it without CONFIG_CPU_FREQ. > > I guess you mean without CONFIG_CPU_FREQ and SMART_REFLEX > Just without CONFIG_CPU_FREQ. Did it boot? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
> From: Eduardo Valentin > > OMAP OPP layer functions now have dependencies of CONFIG_CPU_FREQ only. > > With this patch, omap opp layer now has its compilation flags > bound to CONFIG_CPU_FREQ. Also its code has been removed from pm34xx.c. > > A new file has been created to contain cpu freq code related to > OMAP3: cpufreq34xx.c. > > Signed-off-by: Eduardo Valentin NAK also for the following non-working kernel (smartreflex without cpufreq). -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv3 1/1] OMAP3: PM: move omap opp layer from pm34xx.c
> From: Eduardo Valentin > > OMAP OPP layer functions now have dependencies of CONFIG_CPU_FREQ only. > > With this patch, omap opp layer now has its compilation flags > bound to CONFIG_CPU_FREQ. Also its code has been removed from pm34xx.c. > > A new file has been created to contain cpu freq code related to > OMAP3: cpufreq34xx.c. > > Signed-off-by: Eduardo Valentin > --- > arch/arm/mach-omap2/Makefile |5 + NAK. Your kernel will not boot if you build it without CONFIG_CPU_FREQ. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] OMAP3: PM: Fix compilation issue of omap3_pm_init_opp_table
Menon, Nishanth wrote: > Dasgupta, Romit had written, on 01/19/2010 08:43 AM, the following: 3430 opps, and we should move it to opp34xx.c(I hate having new files :( ).. It should be only #ifdef CONFIG_CPU_FREQ. OPP has nothing to do with CONFIG_PM. Why do you need CPU_FREQ for suspend/resume?? >>> voltage control - SR needs to query for voltage? >>> >> Why should suspend/resume be dependent on SR? > please see the code logic -> when SR is enabled and OFF/RET happens, you > need the voltage for the current frequency so that you can disable SR, > set the nominal voltage for the current OPP then go to WFI. > You do not need the OPP table for querying voltage alone. You can read that from the OMAP chip registers directly. So OPP layer is not necessary when we do not use cpufreq! -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] OMAP3: PM: Fix compilation issue of omap3_pm_init_opp_table
>> 3430 opps, and we should move it to opp34xx.c(I hate having new files :( ).. >>> >> It should be only >> #ifdef CONFIG_CPU_FREQ. OPP has nothing to do with CONFIG_PM. >> >> Why do you need CPU_FREQ for suspend/resume?? >> > voltage control - SR needs to query for voltage? > Why should suspend/resume be dependent on SR? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] OMAP3: PM: Fix compilation issue of omap3_pm_init_opp_table
Menon, Nishanth wrote: > Romit Dasgupta had written, on 01/19/2010 08:09 AM, the following: >>>>> >>>> Err... NAK.. I think you missed >>>> http://marc.info/?t=12635611971&r=1&w=2 ? >>>> there seems to be an issue else where, I have not dug at it yet.. >>> Yeah. OK, I couldn't see the logs as the dumps has been removed already >>> from that thread. >>> But if I got the problem correctly, the problem is when CONFIG_PM is not >>> set but cpu freq is. >>> And if there is any call to new omap opp layer helper functions, then it >>> will BUG the system. >>> Causing hangs. >>> >>> I guess one way to solve this is to bind compilation of omap opp layer to >>> CONFIG_PM and CONFIG_CPU_FREQ. >>> If either is disabled, then omap opp layer must be nops. >>> >>> What do you think? >>> >>> I am sending a patch to do the above. >> No. That is incorrect. CONFIG_CPU_FREQ, CONFIG_CPU_IDLE and CONFIG_PM are >> independent. None of the features should be dependent on the other two! >> -Romit > OPP layer is required by CPU_FREQ & CONFIG_PM, not CPU_IDLE. > > if we modify Eduardo's patch from: > > if defined(CONFIG_PM) && defined(CONFIG_CPU_FREQ) > To > > if defined(CONFIG_PM) || defined(CONFIG_CPU_FREQ) > > wont that ensure the independence is maintained for OPP layer? then, > probably pm34xx.c maynot be the right place for opp registration for > 3430 opps, and we should move it to opp34xx.c(I hate having new files :( ).. > It should be only #ifdef CONFIG_CPU_FREQ. OPP has nothing to do with CONFIG_PM. Why do you need CPU_FREQ for suspend/resume?? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] OMAP3: PM: Fix compilation issue of omap3_pm_init_opp_table
>>> >> Err... NAK.. I think you missed >> http://marc.info/?t=12635611971&r=1&w=2 ? >> there seems to be an issue else where, I have not dug at it yet.. > > > Yeah. OK, I couldn't see the logs as the dumps has been removed already from > that thread. > But if I got the problem correctly, the problem is when CONFIG_PM is not set > but cpu freq is. > And if there is any call to new omap opp layer helper functions, then it will > BUG the system. > Causing hangs. > > I guess one way to solve this is to bind compilation of omap opp layer to > CONFIG_PM and CONFIG_CPU_FREQ. > If either is disabled, then omap opp layer must be nops. > > What do you think? > > I am sending a patch to do the above. No. That is incorrect. CONFIG_CPU_FREQ, CONFIG_CPU_IDLE and CONFIG_PM are independent. None of the features should be dependent on the other two! -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2 1/1] OMAP3: PM: Fix compilation issue of omap opp layer functions
Eduardo Valentin wrote: > From: Eduardo Valentin > > This patch removes the compilation error when compiling > kernel with CONFIG_PM=N. The problem was that omap3_pm_init_opp_table > was not defined if CONFIG_PM=N. NAK. See reply to your other thread. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] pm: omap3: fix build error for PM disabled
>>> index d257225..5fc056f 100644
>>> --- a/arch/arm/mach-omap2/pm.h
>>> +++ b/arch/arm/mach-omap2/pm.h
>>> @@ -69,7 +69,13 @@ static inline void omap3_pm_init_vc(struct prm_setup_vc
>>> *setup_vc)
>>> * Initialize the basic opp table here, board files could choose to modify
>>> opp
>>> * table after the basic initialization
>>> */
>>> +#ifdef CONFIG_PM
>>> extern void omap3_pm_init_opp_table(void);
>>> +#else
>>> +static inline void omap3_pm_init_opp_table(void)
>>> +{
>>> +}
>>> +#endif
>>>
>>> extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
>>> extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int
>>> state);
>> This patch IMHO just solves the build issue. The runtime behaviour is a nasty
>> crash. OMAP architecture has tied CONFIG_PM with CONFIG_CPU_FREQ. So I think
>> we
>> need a fix that solves the runtime behavior too.
> hmm.. thanks for pointing it out (dropping the patch) - even though that
> was not the reason for this patch - I had send this patch after booting
> the kernel using omap3_pm_defconfig (disable PM) on SDP3630:
> SDP3630: http://pastebin.mozilla.org/697292 - complete boot
> SDP3430: http://pastebin.mozilla.org/697294 - hang
As I mentioned there is a __fundamental__ problem in OMAP3 PM tree today.
CONFIG_CPU_FREQ, CONFIG_CPU_IDLE are made dependent on CONFIG_PM. You can have
cpufreq, cpuidle without suspend resume. So IMHO making these features
independent is the right fix.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] pm: omap3: fix build error for PM disabled
Romit Dasgupta wrote:
> Nishanth Menon wrote:
>> omap3_pm_init_opp_table should be under #ifdef CONFIG_PM
>> else build fails when PM is disabled. Reported by Paul originally.
>>
>> Cc: Kevin Hilman
>> Cc: Paul Walmsley
>> Reported-by: Paul Walmsley
>>
>> Signed-off-by: Nishanth Menon
>> ---
>> arch/arm/mach-omap2/pm.h |6 ++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
>> index d257225..5fc056f 100644
>> --- a/arch/arm/mach-omap2/pm.h
>> +++ b/arch/arm/mach-omap2/pm.h
>> @@ -69,7 +69,13 @@ static inline void omap3_pm_init_vc(struct prm_setup_vc
>> *setup_vc)
>> * Initialize the basic opp table here, board files could choose to modify
>> opp
>> * table after the basic initialization
>> */
>> +#ifdef CONFIG_PM
>> extern void omap3_pm_init_opp_table(void);
>> +#else
>> +static inline void omap3_pm_init_opp_table(void)
>> +{
>> +}
>> +#endif
>>
>> extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
>> extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int state);
>
> This patch IMHO just solves the build issue. The runtime behaviour is a nasty
> crash. OMAP architecture has tied CONFIG_PM with CONFIG_CPU_FREQ. So I think
> we
> need a fix that solves the runtime behavior too.
What I meant to say is that CONFIG_PM & CONFIG_CPU_FREQ are independent. So this
is not correct.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] pm: omap3: fix build error for PM disabled
Nishanth Menon wrote:
> omap3_pm_init_opp_table should be under #ifdef CONFIG_PM
> else build fails when PM is disabled. Reported by Paul originally.
>
> Cc: Kevin Hilman
> Cc: Paul Walmsley
> Reported-by: Paul Walmsley
>
> Signed-off-by: Nishanth Menon
> ---
> arch/arm/mach-omap2/pm.h |6 ++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
> index d257225..5fc056f 100644
> --- a/arch/arm/mach-omap2/pm.h
> +++ b/arch/arm/mach-omap2/pm.h
> @@ -69,7 +69,13 @@ static inline void omap3_pm_init_vc(struct prm_setup_vc
> *setup_vc)
> * Initialize the basic opp table here, board files could choose to modify
> opp
> * table after the basic initialization
> */
> +#ifdef CONFIG_PM
> extern void omap3_pm_init_opp_table(void);
> +#else
> +static inline void omap3_pm_init_opp_table(void)
> +{
> +}
> +#endif
>
> extern int omap3_pm_get_suspend_state(struct powerdomain *pwrdm);
> extern int omap3_pm_set_suspend_state(struct powerdomain *pwrdm, int state);
This patch IMHO just solves the build issue. The runtime behaviour is a nasty
crash. OMAP architecture has tied CONFIG_PM with CONFIG_CPU_FREQ. So I think we
need a fix that solves the runtime behavior too.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PM-WIP-OPP] [PATCH 1/2]: Introducing enum for OPP types
Using omap_opp * to refer to domain types restricts opp implementation
into maintaining pointers outside the opp layer. This causes issues such
as:
a) Describing cross domain dependencies (e.g. dsp vs mpu)
b) Ease of transitioning/supporting to multiple silicon variants and
families
c) Choice of varied options in implementing opp layer internals.
Since all we need a identifying a specific domain for query/operational
purposes, we introduce enum for identifying OPP types instead of using
opp layer's internal data structure pointer.
Currently, OMAP3 is the only silicon supporting the OPP layer, hence
mpu_opps, l3_opps and dsp_opps are deprecated and replaced with
OPP_MPU,
OPP_L3 and OPP_DSP respectively.
Signed-off-by: Romit Dasgupta
---
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5c751c1..0930aef 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1347,7 +1347,6 @@ static void __init configure_vc(void)
void __init omap3_pm_init_opp_table(void)
{
- int i;
struct omap_opp_def **omap3_opp_def_list;
struct omap_opp_def *omap34xx_opp_def_list[] = {
omap34xx_mpu_rate_table,
@@ -1359,19 +1358,13 @@ void __init omap3_pm_init_opp_table(void)
omap36xx_l3_rate_table,
omap36xx_dsp_rate_table
};
- struct omap_opp **omap3_rate_tables[] = {
- &mpu_opps,
- &l3_opps,
- &dsp_opps
- };
omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
omap34xx_opp_def_list;
- for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
- *omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
- /* We dont want half configured system at the moment */
- BUG_ON(IS_ERR(omap3_rate_tables[i]));
- }
+
+ BUG_ON(opp_init_list(OPP_MPU, omap3_opp_def_list[0]));
+ BUG_ON(opp_init_list(OPP_L3, omap3_opp_def_list[1]));
+ BUG_ON(opp_init_list(OPP_DSP, omap3_opp_def_list[2]));
}
static int __init omap3_pm_early_init(void)
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 157b38e..5ec072e 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -161,7 +161,7 @@ static DEFINE_MUTEX(dvfs_mutex);
/**
* opp_to_freq - convert OPPID to frequency (DEPRECATED)
* @freq: return frequency back to caller
- * @opps: opp list
+ * @opp_t: OPP type where we need to look.
* @opp_id: OPP ID we are searching for
*
* return 0 and freq is populated if we find the opp_id, else,
@@ -169,14 +169,14 @@ static DEFINE_MUTEX(dvfs_mutex);
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated opp_to_freq(unsigned long *freq,
- const struct omap_opp *opps, u8 opp_id)
+static int __deprecated opp_to_freq(unsigned long *freq, enum opp_t opp_t,
+u8 opp_id)
{
struct omap_opp *opp;
- BUG_ON(!freq || !opps);
+ BUG_ON(!freq || opp_t >= OPP_TYPES_MAX);
- opp = opp_find_by_opp_id(opps, opp_id);
+ opp = opp_find_by_opp_id(opp_t, opp_id);
if (!opp)
return -EINVAL;
@@ -188,20 +188,20 @@ static int __deprecated opp_to_freq(unsigned long *freq,
/**
* freq_to_opp - convert a frequency back to OPP ID (DEPRECATED)
* @opp_id: opp ID returned back to caller
- * @opps: opp list
+ * @opp_t: OPP type where we need to look.
* @freq: frequency we are searching for
*
* return 0 and opp_id is populated if we find the freq, else, we return error
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
+static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t opp_t,
unsigned long freq)
{
struct omap_opp *opp;
- BUG_ON(!opp_id || !opps);
- opp = opp_find_freq_ceil(opps, &freq);
+ BUG_ON(opp_t >= OPP_TYPES_MAX);
+ opp = opp_find_freq_ceil(opp_t, &freq);
if (IS_ERR(opp))
return -EINVAL;
*opp_id = opp_get_opp_id(opp);
@@ -218,9 +218,6 @@ void init_opp(struct shared_resource *resp)
u8 opp_id;
resp->no_of_users = 0;
- if (!mpu_opps || !dsp_opps || !l3_opps)
- return;
-
/* Initialize the current level of the OPP resource
* to the opp set by u-boot.
*/
@@ -228,14 +225,14 @@ void init_opp(struct shared_resource *resp)
vdd1_resp = resp;
dpll1_clk = clk_get(NULL, "dpll1_ck");
dpll2_clk = clk_get(NULL, "dpll2_ck");
- ret = freq_to_opp(&opp_id, mpu_opps, dpll1_clk->rate);
+ ret = freq_to_opp(&opp_id, OPP_MPU, dp
[PM-WIP-OPP] [PATCH 2/2]: Change return value from ERR_PTR(..) to NULL in opp layer
Returning NULL pointer from the OPP APIs instead of ERR_PTR where
return struct omap_opp *. This is because there is no inherent value in
returning ERR_PTR from the opp layer. Returning NULL serves the purpose.
Signed-off-by: Romit Dasgupta
---
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 5ec072e..9572062 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -202,7 +202,7 @@ static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t
opp_t,
BUG_ON(opp_t >= OPP_TYPES_MAX);
opp = opp_find_freq_ceil(opp_t, &freq);
- if (IS_ERR(opp))
+ if (!opp)
return -EINVAL;
*opp_id = opp_get_opp_id(opp);
return 0;
diff --git a/arch/arm/plat-omap/opp.c b/arch/arm/plat-omap/opp.c
index 8fd9366..7835b5d 100644
--- a/arch/arm/plat-omap/opp.c
+++ b/arch/arm/plat-omap/opp.c
@@ -129,7 +129,7 @@ struct omap_opp *opp_find_freq_exact(enum opp_t opp_t,
if (unlikely(opp_t >= OPP_TYPES_MAX)) {
pr_err("%s: Invalid parameters being passed\n", __func__);
- return ERR_PTR(-EINVAL);
+ return NULL;
}
oppl = _opp_list[opp_t];
@@ -143,7 +143,7 @@ struct omap_opp *opp_find_freq_exact(enum opp_t opp_t,
oppl++;
}
- return OPP_TERM(oppl) ? ERR_PTR(-ENOENT) : oppl;
+ return OPP_TERM(oppl) ? NULL : oppl;
}
struct omap_opp *opp_find_freq_ceil(enum opp_t opp_t, unsigned long *freq)
@@ -153,7 +153,7 @@ struct omap_opp *opp_find_freq_ceil(enum opp_t opp_t,
unsigned long *freq)
if (unlikely(opp_t >= OPP_TYPES_MAX || !freq ||
IS_ERR(freq))) {
pr_err("%s: Invalid parameters being passed\n", __func__);
- return ERR_PTR(-EINVAL);
+ return NULL;
}
oppl = _opp_list[opp_t];
@@ -169,7 +169,7 @@ struct omap_opp *opp_find_freq_ceil(enum opp_t opp_t,
unsigned long *freq)
}
if (OPP_TERM(oppl))
- return ERR_PTR(-ENOENT);
+ return NULL;
*freq = oppl->rate;
@@ -183,7 +183,7 @@ struct omap_opp *opp_find_freq_floor(enum opp_t opp_t,
unsigned long *freq)
if (unlikely(opp_t >= OPP_TYPES_MAX || !freq ||
IS_ERR(freq))) {
pr_err("%s: Invalid parameters being passed\n", __func__);
- return ERR_PTR(-EINVAL);
+ return NULL;
}
oppl = prev_opp = _opp_list[opp_t];
@@ -202,7 +202,7 @@ struct omap_opp *opp_find_freq_floor(enum opp_t opp_t,
unsigned long *freq)
}
if (prev_opp->rate > *freq)
- return ERR_PTR(-ENOENT);
+ return NULL;
*freq = prev_opp->rate;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PM-WIP-OPP] [PATCH 0/2]: OPP layer changes (introducing enum for OPP types and returning NULL on failure from some OPP APIs)
Based on the discussion in the mailing list, I have taken some suggestions and am reposting the patch. In this two part patchset: a) the first patch introduces a new enum type for identifying different OPP types. This helps in encapsulating the actual pointers to the respective OPP lists inside the OPP layer itself. One can now interact with the OPP layer by passing the appropriate OPP enum type instead of the actual pointer to the OPP list. b) the second patch is for the OPP layer to return NULL pointer (instead of ERR_PTR) when the return type of the API is struct omap_opp * Signed-off-by: Romit Dasgupta --- arch/arm/mach-omap2/pm34xx.c | 15 -- arch/arm/mach-omap2/resource34xx.c| 86 ++-- arch/arm/mach-omap2/smartreflex.c | 33 +- arch/arm/plat-omap/common.c |6 - arch/arm/plat-omap/cpu-omap.c | 12 +- arch/arm/plat-omap/include/plat/opp.h | 58 +- arch/arm/plat-omap/omap-pm-noop.c |4 arch/arm/plat-omap/omap-pm-srf.c |4 arch/arm/plat-omap/opp.c | 127 9 files changed, 170 insertions(+), 175 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types
Nishanth,
I was about to post a re-worked patch. Anyways, please see below:
> Here is a sample commit message I can think of:
>
> Using omap_opp * to refer to domain types restricts opp implementation
> into maintaining pointers outside the opp layer. This causes issues such as:
> a) Describing cross domain dependencies (e.g. dsp vs mpu)
> b) Ease of transitioning/supporting to multiple silicon variants and
> families
> c) Choice of varied options in implementing opp layer internals.
>
> Since all we need a identifying a specific domain for query/operational
> purposes, we introduce enum for identifying OPP types instead of using
> opp layer's internal data structure pointer.
>
> Currently, OMAP3 is the only silicon supporting the OPP layer, hence
> mpu_opps, l3_opps and dsp_opps are deprecated and replaced with OPP_MPU,
> OPP_L3 and OPP_DSP respectively.
I like this message. I will include it.
>>> definition of enum and the implicit usage of enums are in two different
>>> files. there is a distinct possibility of some one modifying the header
>>> without actually knowing that .c depends on the exact values of the enum
>>> definition.
>> As I said before one needs to make changes in the kernel by knowing what they
>> are doing.
>>> pm34xx.c has no right to depend on opp.h definition values -> if it does
>>> it ties it down and a constraint for future flexibility. please change.
>> The right approach should be to take out the loop in pm34xx.c for now and
>> explicitly call the opp_init_list function after passing OPP_MPU, OPP_L3,
>> OPP_DSP in any order. So pm34xx.c needs to change not opp.[ch]. What do you
>> think?
>
> I did dig into this a few mins ago.. and yes I can see similar example
> in drivers/mfd/twl4030-core.c
>
> The intent of my comment is this: when someone else, few months from
> now, is focusing on adding/changing opp logic, they will focus on opp.c
> and .h. we have two choices to handle this:
> a) Ensure that users of opp.h do not know how it works internally ->
> e.g. ordering of opp list for example.
> b) add
> /* WARNING: See file:arch/arm/mach-omap2/pm34xx.c before modifying the
> sequence of these enums */ to opp.h
> and
> /* WARNING: See file:arch/arm/plat-omap/include/plat/opp.h before
> modifying this */ in pm34xx.c
>
> now, I was recommending doing a, till a thought a little more on the
> implementation(array based) and how long that implementation might
> last(we might potentially move opp.c to a list implementation). the
> effort would be to complicate the opp_init,add functions for a very
> short lifetime. This effort maynot be worth it.
I understand your concern. I have made some changes in the code. Please look at
the reposted patch (in few mins from now I shall post them).
> Enum type and variable have the same name :( mebbe a rename of variable is
> appropriate
>
Not sure why you say this. Did you see the compiler throwing up any
warning?
>>> The usage later in the code is opp_t -> this is a readability issue not
>>> a compiler warning.
>> What is the readability issue? Why cant we declare something like enum opp_t
>> opp_t?
>
> Let me try to explain this clearly. assume we have a struct opp_t (not
> enum) for the time being.
> void some_func(struct opp_t *opp_t)
> {
>struct opp_t *opp;
>
> ..
> 200 line of code (>one page full)
>
> /* point 1 */
> BUG_ON(opp_t.xyz)
> ...
> 200 lines of more code
> ..
> /* point 2 */
> BUG_ON(opp.xyz)
> ...
>
> }
>
> lets say this is compiled by some non follower of this mail chain,
> compiler throws an error for point 1: filex:liney
> so the guy/gal fires up vim and opens the filex, goes to line y
> he/she cannot see the start of the function, knows that there is a
> struct opp_t
If a function is that big then the fault lies there to start with! What do you
say?
Nevertheless, your suggestion is cosmetic but I think we should not assume that
developers are so ignorant. For now I will do away with your suggestion. Please
feel free to change the code if you think what you say is the right thing.
Regards,
-Romit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types
Nishanth Menon wrote:
> Romit Dasgupta said the following on 01/13/2010 04:41 AM:
>> Menon, Nishanth wrote:
>>
>>> General comment: might be good to state the enum types you are introducing
>>> for OMAP3 in the commit message
>>>
>> Actually the introduction of enum type itself is the heart of the patch. The
>> details are irrelevant.
>>
> could you be a little more verbose as to what is irrelevant? the OMAP3
> enums descriptions in commit message?
Yes, because they are going to be SoC specific.
>
>>>> Signed-off-by: Romit Dasgupta
>>>> ---
>>>>
>>>> omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
>>>> omap34xx_opp_def_list;
>>>> - for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
>>>> - *omap3_rate_tables[i] =
>>>> opp_init_list(omap3_opp_def_list[i]);
>>>> + entries = cpu_is_omap3630() ? ARRAY_SIZE(omap34xx_opp_def_list) :
>>>> + ARRAY_SIZE(omap36xx_opp_def_list);
>>>> + for (i = 1; i <= entries; i++) {
>>>> + ret = opp_init_list(i, omap3_opp_def_list[i - 1]);
>>>>
>>> a) if you remove OPP_NONE, i-1 is not needed (same everywhere in the patch)
>>>
>> Frankly, I did not want to introduce OPP_NONE but did so as you are checking
>> all
>> parameters passed to the OPP APIs.
>>
> Lets remove it then.
OK
>>
>>
> definition of enum and the implicit usage of enums are in two different
> files. there is a distinct possibility of some one modifying the header
> without actually knowing that .c depends on the exact values of the enum
> definition.
As I said before one needs to make changes in the kernel by knowing what they
are doing.
>
> pm34xx.c has no right to depend on opp.h definition values -> if it does
> it ties it down and a constraint for future flexibility. please change.
The right approach should be to take out the loop in pm34xx.c for now and
explicitly call the opp_init_list function after passing OPP_MPU, OPP_L3,
OPP_DSP in any order. So pm34xx.c needs to change not opp.[ch]. What do you
think?
>>>> */
>>>> -static int __deprecated opp_to_freq(unsigned long *freq,
>>>> - const struct omap_opp *opps, u8 opp_id)
>>>> +static int __deprecated opp_to_freq(unsigned long *freq, enum opp_t opp_t,
>>>>
>>> Enum type and variable have the same name :( mebbe a rename of variable is
>>> appropriate
>>>
>> Not sure why you say this. Did you see the compiler throwing up any warning?
>>
> The usage later in the code is opp_t -> this is a readability issue not
> a compiler warning.
What is the readability issue? Why cant we declare something like enum opp_t
opp_t?
>>> the original intent of this check is lost here - if the initializations did
>>> not
>>> take place, we will not proceed. An equivalent check might be good to
>>> maintain
>>> at this point.
>>>
>> You are partially correct. I took off the checks because we have a BUG_ON()
>> call
>> in the beginning of the boot code right after we initialize the OPP tables.
>> So
>> we should not hit this check.
>>
> hmm.. interesting.. can you elaborate with exact functions?
omap3_pm_init_opp_table
>
> if you are removing this check, please do a follow up patch and maintain
> equivalent one for this so that the patch does exactly what the commit
> message says "introduce enums" - not do something more.
>
How on earth?? I have removed mpu_opps, l3_opps, dsp_opps from the code so how
do you think I should preserve the checking of the variables when they don't
exist.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types
Menon, Nishanth wrote:
>
> General comment: might be good to state the enum types you are introducing
> for OMAP3 in the commit message
Actually the introduction of enum type itself is the heart of the patch. The
details are irrelevant.
>
>> Signed-off-by: Romit Dasgupta
>> ---
>>
>> omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
>> omap34xx_opp_def_list;
>> - for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
>> - *omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
>> + entries = cpu_is_omap3630() ? ARRAY_SIZE(omap34xx_opp_def_list) :
>> + ARRAY_SIZE(omap36xx_opp_def_list);
>> + for (i = 1; i <= entries; i++) {
>> + ret = opp_init_list(i, omap3_opp_def_list[i - 1]);
> a) if you remove OPP_NONE, i-1 is not needed (same everywhere in the patch)
Frankly, I did not want to introduce OPP_NONE but did so as you are checking all
parameters passed to the OPP APIs.
> b) if we modify the ENUMS or the sequence of definitions in opp_t the logic
> here becomes fault. it might be good to retain an equivalent of
> omap3_rate_table with enum equivalents and register by indexing off that.
You are right but this is a kernel level API and user level code is not going to
use this. Having said this there is no scope for a programmer to introduce new
sequences without understanding the consequences.
>
>> /* We dont want half configured system at the moment */
>> - BUG_ON(IS_ERR(omap3_rate_tables[i]));
>> + BUG_ON(ret);
>> }
>> }
>>
>> diff --git a/arch/arm/mach-omap2/resource34xx.c
>> b/arch/arm/mach-omap2/resource34xx.c
>> index 157b38e..38c44ee 100644
>> --- a/arch/arm/mach-omap2/resource34xx.c
>> +++ b/arch/arm/mach-omap2/resource34xx.c
>> @@ -161,7 +161,7 @@ static DEFINE_MUTEX(dvfs_mutex);
>> /**
>> * opp_to_freq - convert OPPID to frequency (DEPRECATED)
>> * @freq: return frequency back to caller
>> - * @opps: opp list
>> + * @opp_t: OPP type where we need to look.
>> * @opp_id: OPP ID we are searching for
>> *
>> * return 0 and freq is populated if we find the opp_id, else,
>> @@ -169,14 +169,14 @@ static DEFINE_MUTEX(dvfs_mutex);
>> *
>> * NOTE: this function is a standin for the timebeing as opp_id is
>> deprecated
>> */
>> -static int __deprecated opp_to_freq(unsigned long *freq,
>> - const struct omap_opp *opps, u8 opp_id)
>> +static int __deprecated opp_to_freq(unsigned long *freq, enum opp_t opp_t,
>
> Enum type and variable have the same name :( mebbe a rename of variable is
> appropriate
Not sure why you say this. Did you see the compiler throwing up any warning?
>> @@ -188,20 +188,20 @@ static int __deprecated opp_to_freq(unsigned long
>> *freq,
>> -static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
>> +static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t opp_t,
> Re: enum type and variable have the same name :( mebbe a rename of variable is
> appropriate
>> unsigned long freq)
>> {
>> struct omap_opp *opp;
>>
>> - BUG_ON(!opp_id || !opps);
>> - opp = opp_find_freq_ceil(opps, &freq);
>> + BUG_ON(opp_t == OPP_NONE || opp_t > OPP_TYPES);
>> + opp = opp_find_freq_ceil(opp_t, &freq);
>> if (IS_ERR(opp))
>> return -EINVAL;
>> *opp_id = opp_get_opp_id(opp);
>> @@ -218,9 +218,6 @@ void init_opp(struct shared_resource *resp)
>> u8 opp_id;
>> resp->no_of_users = 0;
>>
>> - if (!mpu_opps || !dsp_opps || !l3_opps)
>> - return;
>> -
> the original intent of this check is lost here - if the initializations did
> not
> take place, we will not proceed. An equivalent check might be good to maintain
> at this point.
You are partially correct. I took off the checks because we have a BUG_ON() call
in the beginning of the boot code right after we initialize the OPP tables. So
we should not hit this check.
>> @@ -519,19 +513,16 @@ void init_freq(struct shared_resource *resp)
>> unsigned long freq;
>> resp->no_of_users = 0;
>>
>> - if (!mpu_opps || !dsp_opps)
>> - return;
>> -
> again the initial intent is lost -> to handle cases where the initialization
> was
> not being done.
Same comment as before.
Thanks,
-Romit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types
>
> I like this idea... but I have some questions about how we should
> cleanly handle SMP and future SoCs.
Actually for OMAP4 AFAICT MPU's share only one set of OPPs (their clocks are
tied). So it should not be a problem there. OTOH, the OPP layer itself is not
thread safe today(I tried to make it thread + SMP safe in the version of the
code I posted earlier). You can look into opp_add and it shows that it is not
thread safe.
If you are talking about future SOCs we need to define the OPP list for the new
SoCs. The exact mechanism can be clear once we have the SRF replacement.
>
>> diff --git a/arch/arm/plat-omap/include/plat/opp.h
>> b/arch/arm/plat-omap/include/plat/opp.h
>> index 9f91ad3..c4d5bf9 100644
>> --- a/arch/arm/plat-omap/include/plat/opp.h
>> +++ b/arch/arm/plat-omap/include/plat/opp.h
>> @@ -13,9 +13,18 @@
>> #ifndef __ASM_ARM_OMAP_OPP_H
>> #define __ASM_ARM_OMAP_OPP_H
>>
>> -extern struct omap_opp *mpu_opps;
>> -extern struct omap_opp *dsp_opps;
>> -extern struct omap_opp *l3_opps;
>> +#ifdef CONFIG_ARCH_OMAP3
>> +#define OPP_TYPES 3
>> +#else
>> +#error "You need to put the number of OPP types for OMAP chip type."
>> +#endif
>
> Rather than the #ifdef...
>> +enum opp_t {
>> +OPP_NONE,
>> +OPP_MPU,
>> +OPP_L3,
>> +OPP_DSP
>
> add OPP_MAX_TYPES here
>
Yes you are right. I will change the code and re-post.
>> +};
>
Thanks,
-Romit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PM-WIP-OPP][PATCH] OPP: Introduces enum for addressing different OPP types
Introduces enum for identifying OPP types. This helps in querying the OPP
layer by passing the type of OPP (enum types) and gets away from maintaining
the pointer to the OPP data list outside the OPP layer.
Signed-off-by: Romit Dasgupta
---
arch/arm/mach-omap2/pm34xx.c | 15 +--
arch/arm/mach-omap2/resource34xx.c| 84 +
arch/arm/mach-omap2/smartreflex.c | 33 ++--
arch/arm/plat-omap/common.c |6 -
arch/arm/plat-omap/cpu-omap.c | 12 +--
arch/arm/plat-omap/include/plat/opp.h | 60 +++
arch/arm/plat-omap/omap-pm-noop.c |4 -
arch/arm/plat-omap/omap-pm-srf.c |4 -
arch/arm/plat-omap/opp.c | 96 +++-
9 files changed, 148 insertions(+), 166 deletions(-)
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 5c751c1..8c73e0e 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -1347,7 +1347,7 @@ static void __init configure_vc(void)
void __init omap3_pm_init_opp_table(void)
{
- int i;
+ int i, ret, entries;
struct omap_opp_def **omap3_opp_def_list;
struct omap_opp_def *omap34xx_opp_def_list[] = {
omap34xx_mpu_rate_table,
@@ -1359,18 +1359,15 @@ void __init omap3_pm_init_opp_table(void)
omap36xx_l3_rate_table,
omap36xx_dsp_rate_table
};
- struct omap_opp **omap3_rate_tables[] = {
- &mpu_opps,
- &l3_opps,
- &dsp_opps
- };
omap3_opp_def_list = cpu_is_omap3630() ? omap36xx_opp_def_list :
omap34xx_opp_def_list;
- for (i = 0; i < ARRAY_SIZE(omap3_rate_tables); i++) {
- *omap3_rate_tables[i] = opp_init_list(omap3_opp_def_list[i]);
+ entries = cpu_is_omap3630() ? ARRAY_SIZE(omap34xx_opp_def_list) :
+ ARRAY_SIZE(omap36xx_opp_def_list);
+ for (i = 1; i <= entries; i++) {
+ ret = opp_init_list(i, omap3_opp_def_list[i - 1]);
/* We dont want half configured system at the moment */
- BUG_ON(IS_ERR(omap3_rate_tables[i]));
+ BUG_ON(ret);
}
}
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 157b38e..38c44ee 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -161,7 +161,7 @@ static DEFINE_MUTEX(dvfs_mutex);
/**
* opp_to_freq - convert OPPID to frequency (DEPRECATED)
* @freq: return frequency back to caller
- * @opps: opp list
+ * @opp_t: OPP type where we need to look.
* @opp_id: OPP ID we are searching for
*
* return 0 and freq is populated if we find the opp_id, else,
@@ -169,14 +169,14 @@ static DEFINE_MUTEX(dvfs_mutex);
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated opp_to_freq(unsigned long *freq,
- const struct omap_opp *opps, u8 opp_id)
+static int __deprecated opp_to_freq(unsigned long *freq, enum opp_t opp_t,
+u8 opp_id)
{
struct omap_opp *opp;
- BUG_ON(!freq || !opps);
+ BUG_ON(!freq || opp_t == OPP_NONE || opp_t > OPP_TYPES);
- opp = opp_find_by_opp_id(opps, opp_id);
+ opp = opp_find_by_opp_id(opp_t, opp_id);
if (!opp)
return -EINVAL;
@@ -188,20 +188,20 @@ static int __deprecated opp_to_freq(unsigned long *freq,
/**
* freq_to_opp - convert a frequency back to OPP ID (DEPRECATED)
* @opp_id: opp ID returned back to caller
- * @opps: opp list
+ * @opp_t: OPP type where we need to look.
* @freq: frequency we are searching for
*
* return 0 and opp_id is populated if we find the freq, else, we return error
*
* NOTE: this function is a standin for the timebeing as opp_id is deprecated
*/
-static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
+static int __deprecated freq_to_opp(u8 *opp_id, enum opp_t opp_t,
unsigned long freq)
{
struct omap_opp *opp;
- BUG_ON(!opp_id || !opps);
- opp = opp_find_freq_ceil(opps, &freq);
+ BUG_ON(opp_t == OPP_NONE || opp_t > OPP_TYPES);
+ opp = opp_find_freq_ceil(opp_t, &freq);
if (IS_ERR(opp))
return -EINVAL;
*opp_id = opp_get_opp_id(opp);
@@ -218,9 +218,6 @@ void init_opp(struct shared_resource *resp)
u8 opp_id;
resp->no_of_users = 0;
- if (!mpu_opps || !dsp_opps || !l3_opps)
- return;
-
/* Initialize the current level of the OPP resource
* to the opp set by u-boot.
*/
@@ -228,14 +225,14 @@ void init_opp(struct shared_resource *resp)
vdd1_resp = resp;
dpll1_clk = clk_get(NULL, "dpll1_ck");
dpll2_clk = clk_get(NULL, "dpll2_ck");
-
Re: [PM-WIP-OPP] [PATCH] cleaner ceil function for uv to vsel conversion
>> The description of the problem is "Cleaner ceil function for uv to vsel >> conversion". >> >> I think this patch is simple enough for people on this list to understand the >> optimization. I am sorry I cant be more descriptive. >> >> > here is a try: > - > > From patchwork Thu Dec 31 13:29:05 2009 > Content-Type: text/plain; charset="utf-8" > MIME-Version: 1.0 > Content-Transfer-Encoding: 7bit > Subject: [pm-wip-opp] [PATCH] opp: twl/tps: optimize uv to vsel function > Date: Thu, 31 Dec 2009 13:29:05 - > From: Dasgupta, Romit > X-Patchwork-Id: 70374 > > > For integer values x and y; int div x / y causes truncation. Current > omap_twl_uv_to_vsel function implements an equivalent of ceil which > is based on an if condition to check truncation and round up. > We can do this in a more optimal manner without the if condition. > The round up is handled by adding the round off factor prior to > truncation as: > (x + (y - 1)) / y > Wow that is nice... Please send it if you can and add your Signed-off for the work! -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PM-WIP-OPP] [PATCH] cleaner ceil function for uv to vsel conversion
Kevin Hilman wrote: > Romit Dasgupta writes: > >> Cleaner ceil function. > > Needs better subject and better changelog. > > Subject should probably be something like: OMAP: > > [PATCH] OPP: TWL/TPS: optimize uv to vsel function > > And changelog should describe the motiviation for the patch or a > description of the problem you're trying to solve. The description of the problem is "Cleaner ceil function for uv to vsel conversion". I think this patch is simple enough for people on this list to understand the optimization. I am sorry I cant be more descriptive. > IOW, is this a > correctness fix, or an optimization, etc. Looks to me like it's tring > to compensate for some rounding issues, but please describe in detail > in the changelog. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PM-WIP-OPP] [PATCH] cleaner ceil function for uv to vsel conversion
Cleaner ceil function.
Signed-off-by: Romit Dasgupta
---
diff --git a/arch/arm/plat-omap/opp_twl_tps.c b/arch/arm/plat-omap/opp_twl_tps.c
index e0db39b..1caa414 100644
--- a/arch/arm/plat-omap/opp_twl_tps.c
+++ b/arch/arm/plat-omap/opp_twl_tps.c
@@ -36,14 +36,7 @@ unsigned long omap_twl_vsel_to_uv(const u8 vsel)
*/
u8 omap_twl_uv_to_vsel(unsigned long uv)
{
- u8 vsel;
- vsel = ((uv / 100) - 6000) / 125;
+ return (((uv + 99) / 100 - 6000) + 124) / 125;
- /* round off to higher voltage */
- /* XXX Surely not the best way to handle this. */
- if (uv > omap_twl_vsel_to_uv(vsel))
- vsel++;
-
- return vsel;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/10] OPP layer and additional cleanups.
> > lets make the list implementation as a seperate series and discuss this. > I am guessing that there could be wrapper apis which would could > optimize the implementation while maintaining the overall APIs allowing > other dependent users to continue. I will reserve my comments till we > see the series on this. > Based on the OPP nano-summit!! we had last Friday I will post two sets of patches to start with 1. Miscellaneous changes that was part of the earlier patch. 2. Patch that will remove maintaining pointers to the beginning of the array (OPP array). Thanks, -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/10] OPP layer and additional cleanups.
>>
>> Only point I see that may disfavor list based implementation is the fact
>> that we
>> do not expect high number of OPPs.
> yes + overhead of CPU cycles walking thru the list Vs indexing off an array.
True there is overhead but the downside of an array is reallocing (as is the
case in the previous patch).
>
>> Having said this, I have tried to encapsulate the implementation within the
>> OPP
>> layer. So moving to array/list/any other fancy data structure should be
>> hidden
>> within OPP. So the patchset is not only about moving to a list based
>> implementation. It also to change the semantics of the OPP layer APIs with a
>> deliberate intent to hide/encapsulate the implementation details.
> opp.h:
> +struct omap_opp {
> + struct list_head opp_node;
> + unsigned int enabled:1;
> + unsigned long rate;
> + unsigned long uvolt;
> +};
> this is exactly what we have been trying to avoid in the first place
> (see numerous discussions in the last few months in l-o ML). This allows
> for users of opp.h to write their own direct handling mechanisms and we
> want to prevent it by forcing callers to use only accessor apis. opp.h
> is meant as in include file for all users of opp layer and it's inner
> workings/ inner structures should be isolated to opp.c OR a private
> header file alone.
I am not sure what you say is correct. If you see the opp.h file in my patches
you will find that it has accessor APIs. One does not have to do any
dereferencing to access any of the struct omap_opp members.
On the other hand if you look into opp.c you will find a struct opp_list. This
is the internal details that users do not want to care about and thus I have put
it in opp.c. OTOH when you define the opp lists (e.g. for 3630, 3430) we do it
in an array of struct omap_opp. Hence we need to define this in opp.h So I think
your concern is taken care of.
>
* The OPP layer APIs have been changed. The search APIs have been
reduced to one instead of opp_find_{exact|floor|ceil}.
>>> Could you let us know the benefit of merging this? the split is aligned
>>> in the community as such after the very first implementation which had
>>> all three merged as a single function, but was split after multiple
>>> review comments on readability aspects.
>> Actually I wanted to minimize the number of interfaces to OPP Layer. What was
>> shouting at me was the fact that OPP layer at the heart of it is a in memory
>> data list. So all we need to poke about OPP is to
>> add/delete/enable/disable/search.
>> for search I thought only a single interface like
>> 'find_opp_by_freq' is enough. The flags passed to this function should
>> dictate
>> the mode of the search.
>
> yes, I am aware of this(my first series was motivated by the same
> though), but the alignment with the community is for:
> split search into search_exact, search_ceil, search_floor for
> readability purposes. I dont deny that this is a data list and the basic
> APIs u mentioned are what is enough, but functionally, search is split
> as the above instead of taking params to denote the variations in search
> techniques - hence the community consensus.
>
I wanted to reduce the interfaces. Imagine designing a car with two steerings
(one for going for driving back and the other for going forward). Instead it is
better to have one steering with a control to decide if you want to go forward
or backward.
> I really dont care if struct omap_opp * or enum is used (they are both
> 32bit and have to be dereferenced at the end of the day) to refer to a
> voltage domain. In fact using enum might allow us to do cross opp
> dependency queries too if such an infrastructure is being introduced,
> but that can be done also with struct omap_opp albiet in an obtuse way.
Slight difference than what you say. When you maintain a pointer to the head of
your data structure (be it an array or a list) and expect the APIs to pass it
around, it is different from passing an enum to identify which list you want to
search. You do not need to store a handle to the initiliazed list anymore.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/10] OPP layer and additional cleanups.
Menon, Nishanth wrote: > > > > Could you please start using git format-patch -s -M -o to generate patches? > > > > And you exposed struct omap_opp back to the public defeating the > > previous work :( Exposing struct omap_opp is ok. In fact I need this for adding/deleting and getting a handle on a particular OPP. My actual intent is to have the book-keeping done only in OPP layer. Now you just need to search by saying find_opp_by_freq(OPP_MPU, required_freq, flags /* flag will define the search criteria */); > > > > mostly I like what you are trying to do here, but just not exactly what > > you are doing :( Please let me know how to improvise this. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/10] OPP layer and additional cleanups.
>> diff --git a/arch/arm/plat-omap/omap-pm-srf.c
>> b/arch/arm/plat-omap/omap-pm-srf.c
>> index f7bf353..0736d6a 100644
>> --- a/arch/arm/plat-omap/omap-pm-srf.c
>> +++ b/arch/arm/plat-omap/omap-pm-srf.c
>> @@ -25,10 +25,6 @@
>> #include
>> #include
>>
>> -struct omap_opp *dsp_opps;
>> -struct omap_opp *mpu_opps;
>> -struct omap_opp *l3_opps;
>> -
>> #define LAT_RES_POSTAMBLE "_latency"
>> #define MAX_LATENCY_RES_NAME 30
>>
>> @@ -78,16 +74,17 @@ void omap_pm_set_min_bus_tput(struct device *dev, u8
>> agent_id, unsigned long r)
>> WARN_ON(1);
>> return;
>> };
>> +#warning "Convert throughput to L3 frequency before invoking
>> resource_request"
>>
>> if (r == 0) {
>> pr_debug("OMAP PM: remove min bus tput constraint: "
>> "dev %s for agent_id %d\n", dev_name(dev), agent_id);
>> -resource_release("vdd2_opp", dev);
>> +resource_release("l3_freq", dev);
>> } else {
>> pr_debug("OMAP PM: add min bus tput constraint: "
>> "dev %s for agent_id %d: rate %ld KiB\n",
>> dev_name(dev), agent_id, r);
>> -resource_request("vdd2_opp", dev, r);
>> +resource_request("l3_freq", dev, r);
>> }
>> }
>>
>> @@ -168,42 +165,27 @@ void omap_pm_set_max_sdma_lat(struct device *dev, long
>> t)
>>
>
> you may want to split this out.
Yes, there is still a TODO on this patch. I will split it out once this is done.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/10] OPP layer and additional cleanups
Sripathy, Vishwanath wrote: > Romit, > With VDD1 OPP resource being removed, how are you maitaning link between MPU > and DSP frequencies? I could not find such a link in the below code. May be I > am missing something?? I have not linked DSP and MPU frequencies as I thought it is possible to run DSP and MPU with different frequencies as long as voltages are satisfied. If this is incorrect then this can be done from the cpufreq notifiers as I have tried to do it for L3. Thanks, -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] omap3: pm: sr: replace get_opp with freq_to_opp
Sripathy, Vishwanath wrote: > Nishant, > Have you tested your opp patches on 3630? Apparently opp changes in wip > branch does not even boot on ZOOM3 board. > > Vishwa Vishwa, did you try on top of my OPP patches? Thanks, -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/10] OPP layer and additional cleanups.
>> * OPP layer internals have moved to list based implementation.
>
> Is there a benefit of list based implementation?
Actually, this is a question I have asked myself several times. The motivation
behind list based implementation is to accommodate introduction and revocation
of OPPs.(Not just enabling and disabling). Today's CPUFREQ layer and OPP layer
are disjoint (meaning we prepare the OPPs at boot time and then cpufreq copies
them to its own internal arrays). I want this to be united.
Only point I see that may disfavor list based implementation is the fact that we
do not expect high number of OPPs.
Having said this, I have tried to encapsulate the implementation within the OPP
layer. So moving to array/list/any other fancy data structure should be hidden
within OPP. So the patchset is not only about moving to a list based
implementation. It also to change the semantics of the OPP layer APIs with a
deliberate intent to hide/encapsulate the implementation details.
>
>> * The OPP layer APIs have been changed. The search APIs have been
>> reduced to one instead of opp_find_{exact|floor|ceil}.
>
> Could you let us know the benefit of merging this? the split is aligned
> in the community as such after the very first implementation which had
> all three merged as a single function, but was split after multiple
> review comments on readability aspects.
Actually I wanted to minimize the number of interfaces to OPP Layer. What was
shouting at me was the fact that OPP layer at the heart of it is a in memory
data list. So all we need to poke about OPP is to
add/delete/enable/disable/search.
for search I thought only a single interface like
'find_opp_by_freq' is enough. The flags passed to this function should dictate
the mode of the search.
>
>> * OPP book-keeping is now done inside OPP layer. We do not have to
>> maintain pointers to {mpu|dsp|l3}_opp, outside this layer.
> nice idea, BUT, you need some sort of domain reference mechanism,
> introducing a enum (as done in enum volt_rail - patch 6/10) is the same
> as providing named struct pointers, they perform the same function =
> identifying the voltage domain for the operation. what is the benefit of
> using enum?
The introduction of enum volt_rail is a totally different thing. It is to make
the voltage scaling function generic. On the other hand identifying the OPP list
is also enum based (like MPU_OPP, DSP_OPP, L3_OPP). This is to identify the opp
list I am interested in. Note that doing this enables me to get away from
maintaining struct omap_opp *.
>
>> * removed omap_opp_def as this is very similar to omap_opp.
> yes, but the original intent was that registration mechanism will use a
> structure that is visible to the caller, this allows for isolated
> modification of structure and handling mechanism keeping the overall
> system impact minimal.
Moving to struct omap_opp reduces one more data structure. I am sorry, I did not
follow the later part of your above comment.
>> Verified this on zoom2 with and without lock debugging.
> zoom3/sdp3630?
Not yet verified. Any help on this from anyone in the list is appreciated.
>>
>>
> minor comment:
> Might be good if your patches 1/10 to 9/10 had different subjects.
Yes, Santosh pointed the same to me few days back. I agree this can be done.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/10] OPP layer and additional cleanups
1. Honor the CPUFREQ_RELATION{H|L} flags.
2. Introduce the L3 frequency change notifier call back so that L3 frequency
can be cleanly handled along with MPU.
diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index 78986f0..26fcae1 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -33,6 +33,8 @@
#if defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
#include
#include
+#include
+#include "../mach-omap2/pm.h"
#endif
#define VERY_HI_RATE 9
@@ -48,6 +50,7 @@ static struct cpufreq_frequency_table *freq_table;
#endif
static struct clk *mpu_clk;
+static struct device l3_dev;
/* TODO: Add support for SDRAM timing changes */
@@ -87,6 +90,10 @@ static int omap_target(struct cpufreq_policy *policy,
#ifdef CONFIG_ARCH_OMAP1
struct cpufreq_freqs freqs;
#endif
+#ifdef CONFIG_ARCH_OMAP3
+ struct omap_opp *opp;
+ unsigned long freq;
+#endif
int ret = 0;
/* Ensure desired rate is within allowed range. Some govenors
@@ -111,15 +118,66 @@ static int omap_target(struct cpufreq_policy *policy,
ret = clk_set_rate(mpu_clk, freqs.new * 1000);
cpufreq_notify_transition(&freqs, CPUFREQ_POSTCHANGE);
#elif defined(CONFIG_ARCH_OMAP3) && !defined(CONFIG_OMAP_PM_NONE)
- if (mpu_opps) {
- unsigned long freq = target_freq * 1000;
- if (!IS_ERR(opp_find_freq_ceil(mpu_opps, &freq)))
- omap_pm_cpu_set_freq(freq);
+ freq = target_freq * 1000;
+ opp = find_opp_by_freq(OPP_MPU, freq, OPP_EQ | OPP_ENABLED);
+ if (!opp) {
+ if (relation == CPUFREQ_RELATION_H)
+ opp = find_opp_by_freq(OPP_MPU, freq,
+ OPP_H | OPP_ENABLED);
+ else if (relation == CPUFREQ_RELATION_L)
+ opp = find_opp_by_freq(OPP_MPU, freq,
+ OPP_L | OPP_ENABLED);
}
+
+ if (opp)
+ omap_pm_cpu_set_freq(opp_to_freq(opp));
+ else
+ ret = -EPERM;
#endif
return ret;
}
+static int l3_cpufreq_notifier(struct notifier_block *nb,
+ unsigned long val, void *data)
+{
+ struct cpufreq_freqs *freq = data;
+ struct omap_opp *mpu_target_opp;
+ unsigned long l3_target_freq;
+
+ switch (val) {
+ case CPUFREQ_PRECHANGE:
+ mpu_target_opp = find_opp_by_freq(OPP_MPU, freq->new * 1000,
+ OPP_EQ | OPP_ENABLED);
+ l3_target_freq = get_l3_target_freq(mpu_target_opp);
+ if (resource_get_level("l3_freq") != l3_target_freq)
+ resource_request("l3_freq", &l3_dev, l3_target_freq);
+ break;
+
+ case CPUFREQ_POSTCHANGE:
+ break;
+
+ case CPUFREQ_RESUMECHANGE:
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static struct notifier_block l3_cpufreq_notifier_block = {
+ .notifier_call = l3_cpufreq_notifier
+};
+
+static void __init l3_cpufreq_init(void)
+{
+ if (cpufreq_register_notifier(&l3_cpufreq_notifier_block,
+ CPUFREQ_TRANSITION_NOTIFIER))
+ pr_err("omap-cpufreq: L3 cpufreq registration failed\n");
+ return;
+}
+
static int __init omap_cpu_init(struct cpufreq_policy *policy)
{
int result = 0;
@@ -131,12 +189,10 @@ static int __init omap_cpu_init(struct cpufreq_policy
*policy)
if (policy->cpu != 0)
return -EINVAL;
- policy->cur = policy->min = policy->max = omap_getspeed(0);
-
if (!cpu_is_omap34xx())
clk_init_cpufreq_table(&freq_table);
else
- opp_init_cpufreq_table(mpu_opps, &freq_table);
+ opp_init_cpufreq_table(&freq_table);
if (freq_table) {
result = cpufreq_frequency_table_cpuinfo(policy, freq_table);
@@ -151,10 +207,11 @@ static int __init omap_cpu_init(struct cpufreq_policy
*policy)
policy->min = policy->cpuinfo.min_freq;
policy->max = policy->cpuinfo.max_freq;
- policy->cur = omap_getspeed(0);
+ policy->cur = omap_getspeed(smp_processor_id());
/* FIXME: what's the actual transition time? */
policy->cpuinfo.transition_latency = 300 * 1000;
+ l3_cpufreq_init();
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/10] OPP layer and additional cleanups
1. Localized the OPP id to smartreflex driver. This is still very ugly but it
should be cleaned up during the smartreflex driver overhaul.
2. Register the SR voltage scaling function to the system so that it can use
this for voltage scaling.
diff --git a/arch/arm/mach-omap2/smartreflex.c
b/arch/arm/mach-omap2/smartreflex.c
index d341857..8c33bb8 100644
--- a/arch/arm/mach-omap2/smartreflex.c
+++ b/arch/arm/mach-omap2/smartreflex.c
@@ -3,6 +3,9 @@
*
* OMAP34XX SmartReflex Voltage Control
*
+ * Copyright (C) 2009 Texas Instruments, Inc.
+ * Romit Dasgupta
+ *
* Copyright (C) 2008 Nokia Corporation
* Kalle Jokiniemi
*
@@ -30,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
@@ -44,9 +48,7 @@ struct omap_sr {
int is_sr_reset;
int is_autocomp_active;
struct clk *clk;
- struct clk *vdd_opp_clk;
u32 clk_length;
- u32 req_opp_no;
u32 opp1_nvalue, opp2_nvalue, opp3_nvalue, opp4_nvalue;
u32 opp5_nvalue;
u32 senp_mod, senn_mod;
@@ -54,6 +56,58 @@ struct omap_sr {
void __iomem*vpbase_addr;
};
+/*
+ *XXX: !!! Ugly Alert !!!
+ * Note: OPP id is a concept local to Smart reflex. So we want this to be
+ * only in Smartreflex driver.
+ * The hard coding should away when the SR driver is overhauled with a platform
+ * specific part. We can then supply the OPP id on a per platform
+ * basis. I would emphasize here that OPP id is a concept that only
+ * Smartreflex is concerned.
+ */
+
+static unsigned int freq_to_oppid(unsigned int sr, unsigned long freq)
+{
+ if (sr == SR1) {
+ if (cpu_is_omap3630()) {
+ if (freq > 0 && freq <= 3)
+ return 1;
+ if (freq > 3 && freq <= 6)
+ return 2;
+ if (freq > 6 && freq <= 8)
+ return 3;
+ if (freq > 8 && freq <= 10)
+ return 4;
+ } else {
+ if (freq > 0 && freq <= 12500)
+ return 1;
+ if (freq > 12500 && freq <= 25000)
+ return 2;
+ if (freq > 25000 && freq <= 5)
+ return 3;
+ if (freq > 5 && freq <= 55000)
+ return 4;
+ if (freq > 55000 && freq <= 6)
+ return 5;
+ }
+ } else if (sr == SR2) {
+ if (cpu_is_omap3630()) {
+ if (freq > 0 && freq <= 1)
+ return 1;
+ if (freq > 1 && freq <= 2)
+ return 2;
+ } else {
+ if (freq > 0 && freq <= 8300)
+ return 2;
+ if (freq > 8300 && freq <= 16600)
+ return 3;
+ }
+ }
+
+ return 0;
+}
+
+
#define SR_REGADDR(offs) (sr->srbase_addr + offset)
static inline void sr_write_reg(struct omap_sr *sr, unsigned offset, u32 value)
@@ -147,37 +201,6 @@ static u32 cal_test_nvalue(u32 sennval, u32 senpval)
(rnsenn << NVALUERECIPROCAL_RNSENN_SHIFT);
}
-static u8 get_vdd1_opp(void)
-{
- struct omap_opp *opp;
-
- if (sr1.vdd_opp_clk == NULL || IS_ERR(sr1.vdd_opp_clk) ||
- mpu_opps == NULL)
- return 0;
-
- opp = opp_find_freq_exact(mpu_opps, sr1.vdd_opp_clk->rate, true);
- if (IS_ERR(opp))
- return 0;
-
- return opp_get_opp_id(opp);
-}
-
-static u8 get_vdd2_opp(void)
-{
- struct omap_opp *opp;
-
- if (sr2.vdd_opp_clk == NULL || IS_ERR(sr2.vdd_opp_clk) ||
- l3_opps == NULL)
- return 0;
-
- opp = opp_find_freq_exact(l3_opps, sr2.vdd_opp_clk->rate, true);
- if (IS_ERR(opp))
- return 0;
-
- return opp_get_opp_id(opp);
-}
-
-
static void sr_set_clk_length(struct omap_sr *sr)
{
struct clk *sys_ck;
@@ -281,22 +304,17 @@ static void sr_set_nvalues(struct omap_sr *sr)
static void sr_configure_vp(int srid)
{
- u32 vpconfig;
- u32 vsel;
- int uvdc;
- u32 target_opp_no;
- struct omap_opp *opp;
+ u32 vpconfig, curr_freq;
+ u8 vsel;
+
[PATCH 8/10] OPP layer and additional cleanups
1. Removed the OPP resources and instead introduced voltage resources.
This leads to a leaner implementation of DVFS. It still has scope for
cleanup and this will be done soon.
2. Introduced a L3 frequency resource a.k.a. l3_freq.
3. L3 frequency changes are now handled through CPUFREQ notifiers.
4. The frequency resources use the new OPP layer APIs.
5. Removed hardcoded call to vcbypass function of the smartreflex driver.
Now we invoke the generic voltage scaling function.
diff --git a/arch/arm/mach-omap2/resource34xx.c
b/arch/arm/mach-omap2/resource34xx.c
index 157b38e..59b8faa 100644
--- a/arch/arm/mach-omap2/resource34xx.c
+++ b/arch/arm/mach-omap2/resource34xx.c
@@ -37,6 +37,8 @@
#warning MPU latency constraints require CONFIG_CPU_IDLE to function!
#endif
+static struct device vdd1_volt_dev, vdd2_volt_dev;
+
/**
* init_latency - Initializes the mpu/core latency resource.
* @resp: Latency resource to be initalized
@@ -146,115 +148,6 @@ int set_pd_latency(struct shared_resource *resp, u32
latency)
return 0;
}
-static struct shared_resource *vdd1_resp;
-static struct shared_resource *vdd2_resp;
-static struct device dummy_mpu_dev;
-static struct device dummy_dsp_dev;
-static struct device vdd2_dev;
-static int vdd1_lock;
-static int vdd2_lock;
-static struct clk *dpll1_clk, *dpll2_clk, *dpll3_clk;
-static int curr_vdd1_opp;
-static int curr_vdd2_opp;
-static DEFINE_MUTEX(dvfs_mutex);
-
-/**
- * opp_to_freq - convert OPPID to frequency (DEPRECATED)
- * @freq: return frequency back to caller
- * @opps: opp list
- * @opp_id: OPP ID we are searching for
- *
- * return 0 and freq is populated if we find the opp_id, else,
- * we return error
- *
- * NOTE: this function is a standin for the timebeing as opp_id is deprecated
- */
-static int __deprecated opp_to_freq(unsigned long *freq,
- const struct omap_opp *opps, u8 opp_id)
-{
- struct omap_opp *opp;
-
- BUG_ON(!freq || !opps);
-
- opp = opp_find_by_opp_id(opps, opp_id);
- if (!opp)
- return -EINVAL;
-
- *freq = opp_get_freq(opp);
-
- return 0;
-}
-
-/**
- * freq_to_opp - convert a frequency back to OPP ID (DEPRECATED)
- * @opp_id: opp ID returned back to caller
- * @opps: opp list
- * @freq: frequency we are searching for
- *
- * return 0 and opp_id is populated if we find the freq, else, we return error
- *
- * NOTE: this function is a standin for the timebeing as opp_id is deprecated
- */
-static int __deprecated freq_to_opp(u8 *opp_id, struct omap_opp *opps,
- unsigned long freq)
-{
- struct omap_opp *opp;
-
- BUG_ON(!opp_id || !opps);
- opp = opp_find_freq_ceil(opps, &freq);
- if (IS_ERR(opp))
- return -EINVAL;
- *opp_id = opp_get_opp_id(opp);
- return 0;
-}
-
-/**
- * init_opp - Initialize the OPP resource
- */
-void init_opp(struct shared_resource *resp)
-{
- struct clk *l3_clk;
- int ret;
- u8 opp_id;
- resp->no_of_users = 0;
-
- if (!mpu_opps || !dsp_opps || !l3_opps)
- return;
-
- /* Initialize the current level of the OPP resource
- * to the opp set by u-boot.
- */
- if (strcmp(resp->name, "vdd1_opp") == 0) {
- vdd1_resp = resp;
- dpll1_clk = clk_get(NULL, "dpll1_ck");
- dpll2_clk = clk_get(NULL, "dpll2_ck");
- ret = freq_to_opp(&opp_id, mpu_opps, dpll1_clk->rate);
- BUG_ON(ret); /* TBD Cleanup handling */
- curr_vdd1_opp = opp_id;
- } else if (strcmp(resp->name, "vdd2_opp") == 0) {
- vdd2_resp = resp;
- dpll3_clk = clk_get(NULL, "dpll3_m2_ck");
- l3_clk = clk_get(NULL, "l3_ick");
- ret = freq_to_opp(&opp_id, l3_opps, l3_clk->rate);
- BUG_ON(ret); /* TBD Cleanup handling */
- curr_vdd2_opp = opp_id;
- }
- resp->curr_level = opp_id;
- return;
-}
-
-int resource_access_opp_lock(int res, int delta)
-{
- if (res == VDD1_OPP) {
- vdd1_lock += delta;
- return vdd1_lock;
- } else if (res == VDD2_OPP) {
- vdd2_lock += delta;
- return vdd2_lock;
- }
- return -EINVAL;
-}
-
#ifndef CONFIG_CPU_FREQ
static unsigned long compute_lpj(unsigned long ref, u_int div, u_int mult)
{
@@ -277,235 +170,60 @@ static unsigned long compute_lpj(unsigned long ref,
u_int div, u_int mult)
}
#endif
-static int program_opp_freq(int res, int target_level, int current_level)
-{
- int ret = 0, l3_div;
- int *curr_opp;
- unsigned long mpu_freq, dsp_freq, l3_freq;
-#ifndef CONFIG_CPU_FREQ
- unsigned long mpu_cur_freq;
-#endif
-
- /* Check if I can actually switch or not */
- if (res == VDD1_OPP) {
- ret = opp_to_freq(&mpu_freq, mpu_opps, target_level);
- ret |= opp_to_freq(&dsp_freq, dsp_opps, target_le
[PATCH 7/10] OPP layer and additional cleanups
Adapted the OMAP3 specific PM layer with the new OPP layer. The following
have been done
1. Remove struct omap_opp_def and use only struct omap_opp as the previous
was almost similar to struct omap_opp.
2. Introduce a function 'get_l3_target_freq' to obtain the L3 frequency
corresponding to MPU frequency.(This needs to be done neatly). I agree
that this is ugly.
3. Defined a voltage scaling registration function.
4. Invoke the new OPP layer APIs for registering OPPs.
diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index 9744a35..04265f5 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -113,7 +113,7 @@ static struct prm_setup_vc prm_setup = {
.vdd1_off = 0x00, /* 0.6v */
};
-static struct omap_opp_def __initdata omap34xx_mpu_rate_table[] = {
+static struct omap_opp __initdata omap34xx_mpu_rate_table[] = {
/* OPP1 */
OMAP_OPP_DEF(true, 12500, 975000),
/* OPP2 */
@@ -128,7 +128,7 @@ static struct omap_opp_def __initdata
omap34xx_mpu_rate_table[] = {
OMAP_OPP_DEF(0, 0, 0)
};
-static struct omap_opp_def __initdata omap34xx_l3_rate_table[] = {
+static struct omap_opp __initdata omap34xx_l3_rate_table[] = {
/* OPP1 */
OMAP_OPP_DEF(false, 0, 975000),
/* OPP2 */
@@ -139,7 +139,7 @@ static struct omap_opp_def __initdata
omap34xx_l3_rate_table[] = {
OMAP_OPP_DEF(0, 0, 0)
};
-static struct omap_opp_def __initdata omap34xx_dsp_rate_table[] = {
+static struct omap_opp __initdata omap34xx_dsp_rate_table[] = {
/* OPP1 */
OMAP_OPP_DEF(true, 9000, 975000),
/* OPP2 */
@@ -154,7 +154,7 @@ static struct omap_opp_def __initdata
omap34xx_dsp_rate_table[] = {
OMAP_OPP_DEF(0, 0, 0)
};
-static struct omap_opp_def __initdata omap36xx_mpu_rate_table[] = {
+static struct omap_opp __initdata omap36xx_mpu_rate_table[] = {
/* OPP1 - OPP50 */
OMAP_OPP_DEF(true, 3, 93),
/* OPP2 - OPP100 */
@@ -167,7 +167,7 @@ static struct omap_opp_def __initdata
omap36xx_mpu_rate_table[] = {
OMAP_OPP_DEF(0, 0, 0)
};
-static struct omap_opp_def __initdata omap36xx_l3_rate_table[] = {
+static struct omap_opp __initdata omap36xx_l3_rate_table[] = {
/* OPP1 - OPP50 */
OMAP_OPP_DEF(true, 1, 93),
/* OPP2 - OPP100, OPP-Turbo, OPP-SB */
@@ -176,7 +176,7 @@ static struct omap_opp_def __initdata
omap36xx_l3_rate_table[] = {
OMAP_OPP_DEF(0, 0, 0)
};
-static struct omap_opp_def __initdata omap36xx_dsp_rate_table[] = {
+static struct omap_opp __initdata omap36xx_dsp_rate_table[] = {
/* OPP1 - OPP50 */
OMAP_OPP_DEF(true, 26000, 93),
/* OPP2 - OPP100 */
@@ -189,6 +189,27 @@ static struct omap_opp_def __initdata
omap36xx_dsp_rate_table[] = {
OMAP_OPP_DEF(0, 0, 0)
};
+/*
+ *XXX: !!! Ugly Alert !!!
+ * Need this info from hw_mods or equivalent.
+ */
+unsigned long get_l3_target_freq(struct omap_opp *opp)
+{
+ if (cpu_is_omap3630()) {
+ if (opp_to_freq(opp) >= 6)
+ return 2;
+ else
+ return 1;
+ } else {
+ if (opp_to_freq(opp) >= 5)
+ return 16600;
+ else
+ return 8300;
+ }
+
+ return 0;
+}
+
static inline void omap3_per_save_context(void)
{
omap_gpio_save_context();
@@ -1107,14 +1128,6 @@ void omap3_pm_off_mode_enable(int enable)
else
state = PWRDM_POWER_RET;
-#ifdef CONFIG_OMAP_PM_SRF
- resource_lock_opp(VDD1_OPP);
- resource_lock_opp(VDD2_OPP);
- if (resource_refresh())
- printk(KERN_ERR "Error: could not refresh resources\n");
- resource_unlock_opp(VDD1_OPP);
- resource_unlock_opp(VDD2_OPP);
-#endif
list_for_each_entry(pwrst, &pwrst_list, node) {
pwrst->next_state = state;
set_pwrdm_state(pwrst->pwrdm, state);
@@ -1347,30 +1360,33 @@ static void __init configure_vc(void)
void __init omap3_pm_init_opp_table(void)
{
- int i;
- struct omap_opp_def **omap3_opp_def_list;
- struct omap_opp_def *omap34xx_opp_def_list[] = {
+ int i, entries;
+ struct omap_opp **omap3_opp_def_list;
+ struct omap_opp *omap34xx_opp_def_list[] = {
omap34xx_mpu_rate_table,
omap34xx_l3_rate_table,
omap34xx_dsp_rate_table
};
- struct omap_opp_def *omap36xx_opp_def_list[] = {
+ struct omap_opp *omap36xx_opp_def_list[] = {
omap36xx_mpu_rate_table,
omap36xx_l3_rate_table,
omap36xx_dsp_rate_table
};
- struct omap_opp **omap3_rate_tables[] = {
- &mpu_opps,
- &dsp_opps,
- &l3_opps
- };
omap3_opp_def_
[PATCH 6/10] OPP layer and additional cleanups
Introduce a new types for
1. for identifying voltage rails.
2. generic voltage scaling routine and its registration function.
and the prototype for the registration function itself.
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index 65a6e04..d36cd1d 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -12,6 +12,9 @@
#define __ARCH_ARM_MACH_OMAP2_PM_H
#include
+#ifdef CONFIG_ARCH_OMAP3
+#include
+#endif
extern u32 enable_off_mode;
extern u32 sleep_while_idle;
@@ -23,6 +26,20 @@ extern void omap_sram_idle(void);
extern int omap3_can_sleep(void);
extern int set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
extern int omap3_idle_init(void);
+#ifdef CONFIG_ARCH_OMAP3
+extern unsigned long get_l3_target_freq(struct omap_opp *);
+enum volt_rail {
+ RAIL_NONE,
+ RAIL_VDD1,
+ RAIL_VDD2,
+};
+
+typedef int (*volt_scale_t) (enum volt_rail, struct omap_opp *,
+ struct omap_opp *);
+extern volt_scale_t voltage_scale;
+extern int pm_register_volt_scaling(volt_scale_t);
+#endif
+
struct cpuidle_params {
u8 valid;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/10] OPP layer and additional cleanups.
Remove unnecessary forward declaration of struct omap_opp. diff --git a/arch/arm/plat-omap/include/plat/io.h b/arch/arm/plat-omap/include/plat/io.h index 6c77b03..7e5319f 100644 --- a/arch/arm/plat-omap/include/plat/io.h +++ b/arch/arm/plat-omap/include/plat/io.h @@ -268,7 +268,6 @@ extern void omap_writew(u16 v, u32 pa); extern void omap_writel(u32 v, u32 pa); struct omap_sdrc_params; -struct omap_opp; extern void omap1_map_common_io(void); extern void omap1_init_common_hw(void); -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/10] OPP layer and additional cleanups
Remove struct omap_opp pointers. The book keeping of OPPs are now
encapsulated within the OPP layer.
diff --git a/arch/arm/plat-omap/common.c b/arch/arm/plat-omap/common.c
index 952d506..67a1850 100644
--- a/arch/arm/plat-omap/common.c
+++ b/arch/arm/plat-omap/common.c
@@ -52,12 +52,6 @@ int omap_board_config_size;
/* used by omap-smp.c and board-4430sdp.c */
void __iomem *gic_cpu_base_addr;
-#ifdef CONFIG_OMAP_PM_NONE
-struct omap_opp *mpu_opps;
-struct omap_opp *dsp_opps;
-struct omap_opp *l3_opps;
-#endif
-
static const void *get_config(u16 tag, size_t len, int skip, size_t *len_out)
{
struct omap_board_config_kernel *kinfo = NULL;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/10] OPP layer and additional cleanups.
Change in the resource arbitration APIs to use the OPP layer.
diff --git a/arch/arm/plat-omap/omap-pm-noop.c
b/arch/arm/plat-omap/omap-pm-noop.c
index f7437f7..f59c4aa 100644
--- a/arch/arm/plat-omap/omap-pm-noop.c
+++ b/arch/arm/plat-omap/omap-pm-noop.c
@@ -26,10 +26,6 @@
#include
-struct omap_opp *dsp_opps;
-struct omap_opp *mpu_opps;
-struct omap_opp *l3_opps;
-
/*
* Device-driver-originated constraints (via board-*.c files)
*/
@@ -158,42 +154,21 @@ const struct omap_opp *omap_pm_dsp_get_opp_table(void)
return NULL;
}
-void omap_pm_dsp_set_min_opp(u8 opp_id)
+void omap_pm_dsp_set_min_freq(unsigned long freq)
{
- if (opp_id == 0) {
+ if (!freq) {
WARN_ON(1);
return;
}
- pr_debug("OMAP PM: DSP requests minimum VDD1 OPP to be %d\n", opp_id);
+ pr_debug("OMAP PM: DSP requests minimum DSP freq to be %lu\n", freq);
- /*
-*
-* For l-o dev tree, our VDD1 clk is keyed on OPP ID, so we
-* can just test to see which is higher, the CPU's desired OPP
-* ID or the DSP's desired OPP ID, and use whichever is
-* highest.
-*
-* In CDP12.14+, the VDD1 OPP custom clock that controls the DSP
-* rate is keyed on MPU speed, not the OPP ID. So we need to
-* map the OPP ID to the MPU speed for use with clk_set_rate()
-* if it is higher than the current OPP clock rate.
-*
-*/
}
-u8 omap_pm_dsp_get_opp(void)
+unsigned long omap_pm_dsp_get_freq(void)
{
- pr_debug("OMAP PM: DSP requests current DSP OPP ID\n");
-
- /*
-* For l-o dev tree, call clk_get_rate() on VDD1 OPP clock
-*
-* CDP12.14+:
-* Call clk_get_rate() on the OPP custom clock, map that to an
-* OPP ID using the tables defined in board-*.c/chip-*.c files.
-*/
+ pr_debug("OMAP PM: DSP requests current DSP freq\n");
return 0;
}
diff --git a/arch/arm/plat-omap/omap-pm-srf.c b/arch/arm/plat-omap/omap-pm-srf.c
index f7bf353..0736d6a 100644
--- a/arch/arm/plat-omap/omap-pm-srf.c
+++ b/arch/arm/plat-omap/omap-pm-srf.c
@@ -25,10 +25,6 @@
#include
#include
-struct omap_opp *dsp_opps;
-struct omap_opp *mpu_opps;
-struct omap_opp *l3_opps;
-
#define LAT_RES_POSTAMBLE "_latency"
#define MAX_LATENCY_RES_NAME 30
@@ -78,16 +74,17 @@ void omap_pm_set_min_bus_tput(struct device *dev, u8
agent_id, unsigned long r)
WARN_ON(1);
return;
};
+#warning "Convert throughput to L3 frequency before invoking resource_request"
if (r == 0) {
pr_debug("OMAP PM: remove min bus tput constraint: "
"dev %s for agent_id %d\n", dev_name(dev), agent_id);
- resource_release("vdd2_opp", dev);
+ resource_release("l3_freq", dev);
} else {
pr_debug("OMAP PM: add min bus tput constraint: "
"dev %s for agent_id %d: rate %ld KiB\n",
dev_name(dev), agent_id, r);
- resource_request("vdd2_opp", dev, r);
+ resource_request("l3_freq", dev, r);
}
}
@@ -168,42 +165,27 @@ void omap_pm_set_max_sdma_lat(struct device *dev, long t)
static struct device dummy_dsp_dev;
-/*
- * DSP Bridge-specific constraints
- */
-const struct omap_opp *omap_pm_dsp_get_opp_table(void)
-{
- pr_debug("OMAP PM: DSP request for OPP table\n");
-
- /*
-* Return DSP frequency table here: The final item in the
-* array should have .rate = .opp_id = 0.
-*/
-
- return NULL;
-}
-
-void omap_pm_dsp_set_min_opp(u8 opp_id)
+void omap_pm_dsp_set_min_freq(unsigned long freq)
{
- if (opp_id == 0) {
+ if (!freq) {
WARN_ON(1);
return;
}
- pr_debug("OMAP PM: DSP requests minimum VDD1 OPP to be %d\n", opp_id);
+ pr_debug("OMAP PM: DSP requests minimum DSP freq to be %lu\n", freq);
/*
* For now pass a dummy_dev struct for SRF to identify the caller.
* Maybe its good to have DSP pass this as an argument
*/
- resource_request("vdd1_opp", &dummy_dsp_dev, opp_id);
+ resource_request("dsp_freq", &dummy_dsp_dev, freq);
return;
}
-u8 omap_pm_dsp_get_opp(void)
+unsigned long omap_pm_dsp_get_freq(void)
{
- pr_debug("OMAP PM: DSP requests current DSP OPP ID\n");
- return resource_get_level("vdd1_opp");
+ pr_debug("OMAP PM: DSP requests current DSP frequency\n");
+ return resource_get_level("dsp_freq");
return 0;
}
Change in the resource arbitration APIs to use the OPP layer.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/10] OPP layer and additional cleanups.
Moving to list based OPP layer and its new semantics.
diff --git a/arch/arm/plat-omap/include/plat/opp.h
b/arch/arm/plat-omap/include/plat/opp.h
index 9f91ad3..38ba00f 100644
--- a/arch/arm/plat-omap/include/plat/opp.h
+++ b/arch/arm/plat-omap/include/plat/opp.h
@@ -2,9 +2,9 @@
* OMAP OPP Interface
*
* Copyright (C) 2009 Texas Instruments Incorporated.
- * Nishanth Menon
- * Copyright (C) 2009 Deep Root Systems, LLC.
- * Kevin Hilman
+ * Romit Dasgupta
+ * Derived from the original series of patches by Nishanth Menon &
+ * Kevin Hilman.
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License version 2 as
@@ -13,223 +13,88 @@
#ifndef __ASM_ARM_OMAP_OPP_H
#define __ASM_ARM_OMAP_OPP_H
-extern struct omap_opp *mpu_opps;
-extern struct omap_opp *dsp_opps;
-extern struct omap_opp *l3_opps;
+#ifdef CONFIG_CPU_FREQ
+#include
+#endif
-struct omap_opp;
+#define OPP_ENABLED (1 << 0)
+#define OPP_DISABLED (1 << 1)
+#define OPP_H (1 << 2)
+#define OPP_L (1 << 3)
+#define OPP_EQ (1 << 4)
+#define OPP_ALL (OPP_ENABLED | OPP_DISABLED)
-/**
- * opp_get_voltage() - Gets the voltage corresponding to an opp
- * @opp: opp for which voltage has to be returned for
- *
- * Return voltage in micro volt corresponding to the opp, else
- * return 0
+/*
+ * XXX: Pending documentation.
*/
-unsigned long opp_get_voltage(const struct omap_opp *opp);
-/**
- * opp_get_freq() - Gets the frequency corresponding to an opp
- * @opp: opp for which frequency has to be returned for
- *
- * Return frequency in hertz corresponding to the opp, else
- * return 0
- */
-unsigned long opp_get_freq(const struct omap_opp *opp);
+struct omap_opp {
+ struct list_head opp_node;
+ unsigned int enabled:1;
+ unsigned long rate;
+ unsigned long uvolt;
+};
-/**
- * opp_get_opp_count() - Get number of opps enabled in the opp list
- * @num: returns the number of opps
- * @oppl: opp list
- *
- * This functions returns the number of opps if there are any OPPs enabled,
- * else returns corresponding error value.
- */
-int opp_get_opp_count(struct omap_opp *oppl);
+enum opp_t {
+ OPP_NONE = 0,
+ OPP_MPU,
+ OPP_L3,
+ OPP_DSP,
+};
-/**
- * opp_find_freq_exact() - search for an exact frequency
- * @oppl: OPP list
- * @freq: frequency to search for
- * @enabled: enabled/disabled OPP to search for
- *
- * searches for the match in the opp list and returns handle to the matching
- * opp if found, else returns ERR_PTR in case of error and should be handled
- * using IS_ERR.
- *
- * Note enabled is a modifier for the search. if enabled=true, then the match
is
- * for exact matching frequency and is enabled. if true, the match is for exact
- * frequency which is disabled.
- */
-struct omap_opp *opp_find_freq_exact(struct omap_opp *oppl,
-unsigned long freq, bool enabled);
+static inline long opp_to_volt(struct omap_opp *opp)
+{
+ return opp ? opp->uvolt : 0;
+}
-/* XXX This documentation needs fixing */
+static inline long opp_to_freq(struct omap_opp *opp)
+{
+ return opp ? opp->rate : 0;
+}
-/**
- * opp_find_freq_floor() - Search for an rounded freq
- * @oppl: Starting list
- * @freq: Start frequency
- *
- * Search for the lower *enabled* OPP from a starting freq
- * from a start opp list.
- *
- * Returns *opp and *freq is populated with the next match, else
- * returns NULL opp if found, else returns ERR_PTR in case of error.
- *
- * Example usages:
- * * find match/next lowest available frequency
- * freq = 35;
- * opp = opp_find_freq_floor(oppl, &freq)))
- * if (IS_ERR(opp))
- * pr_err ("unable to find a lower frequency\n");
- * else
- * pr_info("match freq = %ld\n", freq);
- *
- * * print all supported frequencies in descending order *
- * opp = oppl;
- * freq = ULONG_MAX;
- * while (!IS_ERR(opp = opp_find_freq_floor(opp, &freq)) {
- * pr_info("freq = %ld\n", freq);
- * freq--; * for next lower match *
- * }
- *
- * NOTE: if we set freq as ULONG_MAX and search low, we get the
- * highest enabled frequency
- */
-struct omap_opp *opp_find_freq_floor(struct omap_opp *oppl,
-unsigned long *freq);
+static inline unsigned int opp_is_valid(struct omap_opp *opp)
+{
+ return opp ? opp->enabled : 0;
+}
-/* XXX This documentation needs fixing */
+extern struct omap_opp *find_opp_by_freq(enum opp_t, unsigned long,
+ unsigned long);
-/**
- * opp_find_freq_ceil() - Search for an rounded freq
- * @oppl: Starting list
- * @freq: Start frequency
- *
- * Search for the higher *enabled* OPP from a starting freq
- * from a start opp list.
- *
- * Returns *opp and
[PATCH 1/10] OPP layer and additional cleanups.
Cleaner way to take care of the precision loss during integer division.
diff --git a/arch/arm/plat-omap/opp_twl_tps.c b/arch/arm/plat-omap/opp_twl_tps.c
index e0db39b..43dee2d 100644
--- a/arch/arm/plat-omap/opp_twl_tps.c
+++ b/arch/arm/plat-omap/opp_twl_tps.c
@@ -36,14 +36,7 @@ unsigned long omap_twl_vsel_to_uv(const u8 vsel)
*/
u8 omap_twl_uv_to_vsel(unsigned long uv)
{
- u8 vsel;
+ /* Takes care of precision loss due to integer division */
+ return (((uv + 99) / 100 - 6000) + 124) / 125;
- vsel = ((uv / 100) - 6000) / 125;
-
- /* round off to higher voltage */
- /* XXX Surely not the best way to handle this. */
- if (uv > omap_twl_vsel_to_uv(vsel))
- vsel++;
-
- return vsel;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/10] OPP layer and additional cleanups.
Hi,
The following set of patches apply on top of the Kevin's pm-wip-opp
branch. What I have tried to do in this set of patches are:
(Not in patch-set order)
* OPP layer internals have moved to list based implementation.
* The OPP layer APIs have been changed. The search APIs have been
reduced to one instead of opp_find_{exact|floor|ceil}.
* OPP book-keeping is now done inside OPP layer. We do not have to
maintain pointers to {mpu|dsp|l3}_opp, outside this layer.
* removed omap_opp_def as this is very similar to omap_opp.
* Cleaned up the SRF framework to use new OPP APIs.
* Removed VDD1,2 OPP resources and instead introduced voltage resources.
This results in leaner code.
* L3 frequency change now happens from cpufreq notifier mechanism.
* cpufreq driver now honors the CPUFREQ{H|L} flags.
* uv to vsel precision loss is handled cleanly.
Verified this on zoom2 with and without lock debugging.
Some output from cpufreq translation statistics.
#
cat /sys/devices/system/cpu/cpu0/cpufreq/stats/trans_table
From :
To
60555025 125000
60: 0 6804 4536 45364535
55: 4536 0 6804 45364535
50: 4537 4536 0 68044535
25: 4536 4536 4536 06802
125000: 6802 4535 4535 4535 0
diffstat output!
mach-omap2/pm.h | 17 +
mach-omap2/pm34xx.c | 79 --
mach-omap2/resource34xx.c| 542 ++-
mach-omap2/resource34xx.h| 62 ++--
mach-omap2/smartreflex.c | 285 +++---
mach-omap2/smartreflex.h | 16 -
plat-omap/common.c |6
plat-omap/cpu-omap.c | 73 +
plat-omap/include/plat/io.h |1
plat-omap/include/plat/opp.h | 265 +
plat-omap/omap-pm-noop.c | 35 --
plat-omap/omap-pm-srf.c | 38 ---
plat-omap/opp.c | 497 +--
plat-omap/opp_twl_tps.c | 11
14 files changed, 851 insertions(+), 1076 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SR2: VDD autocomp is not active
tarek attia wrote: > Thank you :) , > > I found the "omap2_clk_init_cpufreq_table" function in the kernel > image installed on my board ,and the hanging up is not permanent > ,sometimes occurs and some time doesn't . > > So what the reason may be for this case? ..also what is the usage of > "sr_vdd1_autocomp" ?? > > I know that I'm bothering you,,but thanks for your reply in advance :-) > Nothing to be bothered. Always glad to reply. sr_vdd1_autocomp enables Smart reflex voltage autocompensation for VDD1 voltage rail. You do seem to have the low level cpufreq driver in place. May be you can try with pm branch. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SR2: VDD autocomp is not active
tarek attia wrote: > Basically I'm working on beagleboard rev B5 . > I'm not using the pm branch of kevin ,,however I'm using source files > from the rowboat project (Android) ,just I enabled the cpufreq and > smartreflex in the kernel configuration while I'm building the linux > kernel of Android . > Do you have this function in your code? 'omap2_clk_init_cpufreq_table' If not please use pm branch. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SR2: VDD autocomp is not active
>>> Each time I enable the ondemand governor ,it works fine for a while >>> ,but each time the frequency is changed this messages appears :- >>> >>> SR1: VDD autocomp is not active >>> >>> then it works again ,but after small amount of time this messages >>> appears followed by hang up for the overall system >>> >>> SR2: VDD autocomp is not active >>> >>> >> try this: >> echo 1 > /sys/power/sr_vdd1_autocomp; echo 1 > /sys/power/sr_vdd2_autocomp What platform are you using? Also what is your git HEAD? I can smell 3630... ;-) please confirm. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: SR2: VDD autocomp is not active
tarek attia wrote: > Hi all, > > Each time I enable the ondemand governor ,it works fine for a while > ,but each time the frequency is changed this messages appears :- > > SR1: VDD autocomp is not active > > then it works again ,but after small amount of time this messages > appears followed by hang up for the overall system > > SR2: VDD autocomp is not active > > try this: echo 1 > /sys/power/sr_vdd1_autocomp; echo 1 > /sys/power/sr_vdd2_autocomp -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
Teerth, one more correction/improvement below:
> diff --git a/arch/arm/plat-omap/sram.c b/arch/arm/plat-omap/sram.c
> index d8d5094..ddbdaaf 100644
> --- a/arch/arm/plat-omap/sram.c
> +++ b/arch/arm/plat-omap/sram.c
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned long (*_omap3_sram_delay)(unsigned long);
> +unsigned int measure_sram_delay(unsigned int loop)
> +{
> + static struct omap_dm_timer *gpt;
> + unsigned long flags, diff = 0, gt_rate, mpurate;
> + unsigned int delay_sram, error_gain;
> +
> + omap_dm_timer_init();
> + gpt = omap_dm_timer_request_specific(10);
> + if (!gpt)
> + pr_err("Could not get the gptimer\n");
> + omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> +
> + gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> + omap_dm_timer_set_load_start(gpt, 0, 0);
> +
> + local_irq_save(flags);
> + diff = _omap3_sram_delay(loop);
> + local_irq_restore(flags);
> +
> + omap_dm_timer_stop(gpt);
> + omap_dm_timer_free(gpt);
> +
> + mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> +
> + /* calculate the sram delay */
> + delay_sram = (((mpurate / gt_rate) * diff) / 2);
Instead of 2 it should be (loop * 2).
> +
> + error_gain = mpurate / gt_rate;
> + delay_sram = delay_sram + error_gain;
> +
> + return delay_sram;
> +}
> +#endif
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
> +#ifdef CONFIG_ARCH_OMAP3
> +unsigned long (*_omap3_sram_delay)(unsigned long);
> +unsigned int measure_sram_delay(unsigned int loop)
> +{
> + static struct omap_dm_timer *gpt;
> + unsigned long flags, diff = 0, gt_rate, mpurate;
> + unsigned int delay_sram, error_gain;
> +
> + omap_dm_timer_init();
> + gpt = omap_dm_timer_request_specific(10);
> + if (!gpt)
> + pr_err("Could not get the gptimer\n");
> + omap_dm_timer_set_source(gpt, OMAP_TIMER_SRC_SYS_CLK);
> +
> + gt_rate = clk_get_rate(omap_dm_timer_get_fclk(gpt));
> + omap_dm_timer_set_load_start(gpt, 0, 0);
> +
> + local_irq_save(flags);
> + diff = _omap3_sram_delay(loop);
> + local_irq_restore(flags);
> +
> + omap_dm_timer_stop(gpt);
> + omap_dm_timer_free(gpt);
> +
> + mpurate = clk_get_rate(clk_get(NULL, "arm_fck"));
> +
> + /* calculate the sram delay */
> + delay_sram = mpurate/100) / (gt_rate/100)) * diff) / 2);
Can remove the 100 from the denominators as they cancel out.
> +
> + error_gain = ((mpurate/100) / (gt_rate/100));
Same as before.
> + delay_sram = delay_sram + error_gain;
> +
> + return delay_sram;
> +}
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/4] OMAP OPP: Add accessor function for getting OPP ID.
Kevin,
Some comments below:
Kevin Hilman wrote:
> Add new function opp_get_opp_id() for finding the OPP ID of a given
> OPP. This allows us to further hide OPP layer details.
>
> NOTE: OPP IDs are deprecated, and this function will eventually
> be removed after all users of OPP IDs are removed.
Let us not add a deprecated function. If we are fixing this let us fix it right
the first time.
>
> +u8 __deprecated opp_get_opp_id(struct omap_opp *opp)
> +{
> + return opp->opp_id;
> +}
> +
> int opp_get_opp_count(struct omap_opp *oppl)
> {
> u8 n = 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] io: remove redundant params in omap2_init_common_hw
> - omap2_init_common_hw(NULL, NULL, NULL, NULL, NULL); > + omap2_init_common_hw(NULL, NULL); A call to a function with NULL parameters suggests that the function is doing more than it should. I think omap2_init_common_hw and omap2_sdrc_init, _omap2_init_reprogram_sdrc, gpmc init can be separated from omap2_init_common_hw. If you are refactoring the code may be this should be taken care as well. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 5/9] omap3: pm: sr: replace get_opp with freq_to_opp
Kevin Hilman wrote: > Nishanth Menon writes: > > > SmartReflex is not quite working with this version which is in > pm-wip-opp. My (untested) theory below... Mostly you have not included SR in the build. OTOH I raised this issue earlier that the omap cpufreq driver is not honoring CPUFREQ_RELATION_L and CPUFREQ_RELATION_H flags. I do not understand why do we need to do a opp_find_freq_ceil here? The validate function of the frequency resources would have already identified if we can get this exact frequency or not. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Strange boot messages on mainline + omap3_defconfig
Gadiyar, Anand wrote: > I get some "mux: Unknown ball ..." messages booting > v2.6.33-rc1 from Linux' tree (built with the omap3_defconfig). > > This was on a 3630 SDP. > > Won't be looking at this for a while, so just reporting it. > > - Anand > > 0x588 > [0.00] mux: Unknown ball offset 0x58a > [0.00] mux: Unknown ball offset 0x58c > [0.00] mux: Unknown ball offset 0x58e > [0.00] mux: Unknown ball offset 0x590 > [0.00] mux: Unknown ball offset 0x578 > [0.00] mux: Unknown ball offset 0x57a > [0.00] mux: Unknown ball offset 0x57c > [0.00] mux: Unknown ball offset 0x57e > [0.00] mux: Unknown ball offset 0x580 > [0.00] mux: Unknown ball offset 0x582 > [0.00] mux: Unknown ball offset 0x584 > [0.00] mux: Unknown ball offset 0x586 > [0.00] mux: Unknown ball offset 0x570 > [0.00] mux: Unknown ball offset 0x572 > [0.00] mux: Unknown ball offset 0x40 > [0.00] mux: Unknown ball offset 0x0 > [0.00] mux: Unknown ball offset 0x2 > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-omap" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Try this in you .config CONFIG_OMAP_PACKAGE_CBB=y CONFIG_OMAP_PACKAGE_CUS=y CONFIG_OMAP_PACKAGE_CBP=y -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Strange boot messages on mainline + omap3_defconfig
> This was on a 3630 SDP. > > Won't be looking at this for a while, so just reporting it. > > - Anand > > 0x588 > [0.00] mux: Unknown ball offset 0x58a > [0.00] mux: Unknown ball offset 0x0 > [0.00] mux: Unknown ball offset 0x2 CONFIG_OMAP_PACKAGE_CBP is undefined for you IMHO. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/10 V5] omap3: pm: introduce support for 3630 OPPs
>>> [...]
>>>
>>>
To facilitate the ongoing discussions on OPP rework, and to have a
common base, this series is available as a branch in my linux-omap-pm
repo[1].
>
> Yes, I'm in the process cleaning that up.
>
> Once I get some of that cleanup done, I plan to rebase your OPP V5 and
> include it in the PM branch.
> I tried the latest HEAD on pm-wip-opp. Looks like the cpufreq tables are not
initialized because are not initializing {mpu|dsp|l3}_opps. Looks like the
comment on the latest commit is to use OPP APIs directly. Is there any patch
currently on that direction? Otherwise in the pm-wip-opp branch cpufreq is
broken.
Thanks,
-Romit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
Kevin Hilman wrote: > Kevin Hilman writes: > > [...] > >> PMC code is ARM generic and already largely exists in other places >> (oprofile for one.) So the use of the PMC will need to be generalized >> as well as be shown not to interfere with other users (as raised >> already by Jean Pihet.) > > Someone is already trying to generalize a PMC interface. You might want > to follow this thread: > > http://lists.infradead.org/pipermail/linux-arm-kernel/2009-December/005898.html > I tried to find the code in LO but I think it is not yet present. So with that in mind may I propose that the pmc functions we introduce in plat-omap will be present as __init code (so that we ensure no one uses it after the system finishes booting up) as long as the pmc infrastructure is not available for non-Oprofile uses? As I mentioned in an earlier thread, the pmc is used and stopped very early during the kernel boot and AFAICT there should not be any problem (probably we need to check its behavior on EMU/HS devices). Thanks, -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
Menon, Nishanth wrote: > Menon, Nishanth had written, on 12/11/2009 07:42 AM, the following: >> Dasgupta, Romit had written, on 12/11/2009 06:31 AM, the following: Also Richard indicated that there might be a few tricky things with perf Counters with specific devices - like EMU/HS/GP devices. It might need EMU Domain for the values to pass through and there might be a yet-not-measured increase In power which could impact usage numbers and may need additional Code to switch off the domain correctly while hitting OFF/RET.. >>> Yes someone with EMU/HS could run and let us know. OTOH there won't be any >>> increase in power as it is done only once during boot time after which the >>> perfcounters are stopped. >>> >>> By the way can you run this in 3630 and help us find what is the SRAM access >>> delay? I am sure it should be lesser since it has a process improvement >>> over 34xx. >>> >>> >> will try on SDP3630 among the boards that I have around. meanwhile, >> would like an explanation to my previous comments also esp on the black >> magic "6" ;) - thanks. >> > Just realized -> clock framework changes are not in yet, anyways, with > 3630SDP (without my OPP series and on pm branch): > Clocking rate (Crystal/Core/MPU): 26.0/400/600 MHz > SRAM delay: 54 > > Reprogramming SDRC clock to 4 Hz > Thanks Nishanth. As expected the SRAM delay cycles are less compared to 3430. However only 3 cycles!! ;-) -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
Jean Pihet wrote: > On Friday 11 December 2009 13:05:37 Reddy, Teerth wrote: >> Reposting the patch with proper format >> >> From: Teerth Reddy >> >> This patch sets the dpll3 clock stabilization delay during >> DVFS. The stabilization delay is calculated dynamically >> using the ARM performance counter. >> Currently 6us of SDRC stabilization value is used to get >> the correct delay. > That is a good thing to have! However the counters might already be in use, > cf. below > Yes, we had a concern about that and not sure how soon the perf counter starts up. We do this at very early boot code as a part of init_IRQ. much before profile_init. So I am not sure if your concern is valid. Thanks, -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
Reposting on behalf of Teerth.
This patch sets the dpll3 clock stabilization delay during
DVFS. The stabilization delay is calculated dynamically
using the ARM performance counter.
Currently 6us of SDRC stabilization value is used to get
the correct delay.
Signed-off-by: Romit Dasgupta
Signed-off-by: Teerth Reddy
---
mach-omap2/clock34xx.c| 32 +---
mach-omap2/sram34xx.S |7 ++
plat-omap/Makefile|1
plat-omap/include/plat/pmc.h |9
plat-omap/include/plat/sram.h |6 +
plat-omap/pmc.c | 47 ++
plat-omap/sram.c | 23
7 files changed, 108 insertions(+), 17 deletions(-)
diff --git a/arch/arm/mach-omap2/clock34xx.c b/arch/arm/mach-omap2/clock34xx.c
index da5bc1f..dd49938 100644
--- a/arch/arm/mach-omap2/clock34xx.c
+++ b/arch/arm/mach-omap2/clock34xx.c
@@ -37,6 +37,8 @@
#include
#include
+#include
+
#include
#include "clock.h"
#include "prm.h"
@@ -46,6 +48,8 @@
static const struct clkops clkops_noncore_dpll_ops;
+unsigned int delay_sram;
+
static void omap3430es2_clk_ssi_find_idlest(struct clk *clk,
void __iomem **idlest_reg,
u8 *idlest_bit);
@@ -333,14 +337,9 @@ static struct omap_clk omap34xx_clks[] = {
/* Scale factor for fixed-point arith in omap3_core_dpll_m2_set_rate() */
#define SDRC_MPURATE_SCALE 8
-/* 2^SDRC_MPURATE_BASE_SHIFT: MPU MHz that SDRC_MPURATE_LOOPS is defined for */
-#define SDRC_MPURATE_BASE_SHIFT9
-
-/*
- * SDRC_MPURATE_LOOPS: Number of MPU loops to execute at
- * 2^MPURATE_BASE_SHIFT MHz for SDRC to stabilize
- */
-#define SDRC_MPURATE_LOOPS 96
+/* SDRC_TIME_STABILIZE: Time for SDRC to stabilize in us */
+/* hardcoded currently */
+#defineSDRC_TIME_STABILIZE 6
/*
* DPLL5_FREQ_FOR_USBHOST: USBHOST and USBTLL are the only clocks
@@ -870,16 +869,10 @@ static int omap3_core_dpll_m2_set_rate(struct clk *clk,
unsigned long rate)
unlock_dll = 1;
}
- /*
-* XXX This only needs to be done when the CPU frequency changes
-*/
+ /* Calculate the number of MPU cycles to wait for SDRC to stabilize */
+
mpurate = arm_fck.rate / CYCLES_PER_MHZ;
- c = (mpurate << SDRC_MPURATE_SCALE) >> SDRC_MPURATE_BASE_SHIFT;
- c += 1; /* for safety */
- c *= SDRC_MPURATE_LOOPS;
- c >>= SDRC_MPURATE_SCALE;
- if (c == 0)
- c = 1;
+ c = ((SDRC_TIME_STABILIZE * mpurate) / (delay_sram * 2));
pr_debug("clock: changing CORE DPLL rate from %lu to %lu\n", clk->rate,
validrate);
@@ -1228,6 +1221,11 @@ int __init omap2_clk_init(void)
vclk = clk_get(NULL, "virt_prcm_set");
sclk = clk_get(NULL, "sys_ck");
#endif
+
+ /* Measure sram delay */
+ delay_sram = measure_sram_delay(1)/(1*2);
+ pr_debug("SRAM delay: %d\n", delay_sram);
+
return 0;
}
diff --git a/arch/arm/mach-omap2/sram34xx.S b/arch/arm/mach-omap2/sram34xx.S
index 82aa4a3..dc0ac72 100644
--- a/arch/arm/mach-omap2/sram34xx.S
+++ b/arch/arm/mach-omap2/sram34xx.S
@@ -298,3 +298,10 @@ core_m2_mask_val:
ENTRY(omap3_sram_configure_core_dpll_sz)
.word . - omap3_sram_configure_core_dpll
+ENTRY(__sram_wait_delay)
+ subsr0, r0, #1
+ bne __sram_wait_delay
+ bx lr
+
+ENTRY(__sram_wait_delay_sz)
+ .word . - __sram_wait_delay
diff --git a/arch/arm/plat-omap/Makefile b/arch/arm/plat-omap/Makefile
index 95f8413..aac43da 100644
--- a/arch/arm/plat-omap/Makefile
+++ b/arch/arm/plat-omap/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_ARCH_OMAP16XX) += ocpi.o
# omap_device support (OMAP2+ only at the moment)
obj-$(CONFIG_ARCH_OMAP2) += omap_device.o
obj-$(CONFIG_ARCH_OMAP3) += omap_device.o
+obj-$(CONFIG_ARCH_OMAP3) += pmc.o
obj-$(CONFIG_OMAP_MCBSP) += mcbsp.o
obj-$(CONFIG_OMAP_IOMMU) += iommu.o iovmm.o
diff --git a/arch/arm/plat-omap/include/plat/pmc.h
b/arch/arm/plat-omap/include/plat/pmc.h
new file mode 100644
index 000..6d73782
--- /dev/null
+++ b/arch/arm/plat-omap/include/plat/pmc.h
@@ -0,0 +1,9 @@
+#ifndef __PMC_H__
+#define __PMC_H__
+extern void start_perf_counter(void);
+extern void start_cycle_counter(void);
+extern void stop_cycle_counter(void);
+extern void stop_perf_counter(void);
+extern unsigned int cycle_count(void);
+extern unsigned int delay_sram;
+#endif
diff --git a/arch/arm/plat-omap/include/plat/sram.h
b/arch/arm/plat-omap/include/plat/sram.h
index 16a1b45..6019ed8 100644
--- a/arch/arm/plat-omap/include/plat/sram.h
+++ b/arch/arm/plat-omap/include/plat/sram.h
@@ -69,6 +69,12 @@ extern u32 omap3_sram_configure_core_dpll(
u32 sdrc_actim_ctrl_b_1, u32 sdrc_mr_1)
Re: [PATCH] OMAP3: PM: Dynamic calculation of SDRC clock stabilization delay
> > Also Richard indicated that there might be a few tricky things with perf > Counters with specific devices - like EMU/HS/GP devices. It might need EMU > Domain for the values to pass through and there might be a yet-not-measured > increase In power which could impact usage numbers and may need additional > Code to switch off the domain correctly while hitting OFF/RET.. > Yes someone with EMU/HS could run and let us know. OTOH there won't be any increase in power as it is done only once during boot time after which the perfcounters are stopped. By the way can you run this in 3630 and help us find what is the SRAM access delay? I am sure it should be lesser since it has a process improvement over 34xx. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] PM: Prevent direct cpufreq scaling during initialization
Sergey Lapin wrote: > On Thu, Dec 3, 2009 at 9:29 AM, Dasgupta, Romit wrote: >> >>>> Signed-off-by: Romit Dasgupta >>>> --- >>>> diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu- >>> omap.c >>>> index 449b6b6..f94df20 100644 >>>> --- a/arch/arm/plat-omap/cpu-omap.c >>>> +++ b/arch/arm/plat-omap/cpu-omap.c >>>> @@ -149,8 +149,6 @@ static int __init omap_cpu_init(struct >>> cpufreq_policy *policy) >>>>VERY_HI_RATE) / >>> 1000; >>>>} >>>> >>>> - clk_set_rate(mpu_clk, policy->cpuinfo.max_freq * 1000); >>>> - >>>>policy->min = policy->cpuinfo.min_freq; >>>>policy->max = policy->cpuinfo.max_freq; >>>>policy->cur = omap_getspeed(0); >>> This patch leads to hang with current PM branch, SRF and CPU IDLE enabled >>> on OMAP3525 rev C. >> Verified this to work on Zoom2 + SRF + CPUidle. Can you give some debug info >> if possible? >> > Well, that looks like false alarm - I tried clean tree without local > patches, and it seems to work. > > However, without this line removed, my system boots faster. Is it possible to > limit initial speed for unstable setups, but to boot as fast as possible? > > Thanks a lot, > S. omap_cpufreq_init is a lateinitcall. So quite some part of the kernel startup happens with whatever speed the bootloader passes control to kernel initialization code. Only after we invoke omap_cpufreq_init will the cpufreq framework try to call the offending code 'clk_set_rate'. Quite soon after this the cpufreq default governor (whatever it is for your system, performance, ondemand etc) is initialized. If you choose performance governor it will immediately scale the frequency to the highest available for the system. So the way to boot fast would be a) make your bootcode scale the voltage + freq to support the highest cpu freq. and/or b) make performance governor the default. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] PM: Prevent direct cpufreq scaling during initialization
It is seen that the OMAP specific cpufreq initialization code tries to scale the MPU frequency to the highest possible without taking care of the voltage level. On power on reset the power IC does not provide the necessary voltage for the highest available MPU frequency (that would satisfy all Si families). This potentially is an window of opportunity for things to go wrong. Signed-off-by: Romit Dasgupta --- diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c index 449b6b6..f94df20 100644 --- a/arch/arm/plat-omap/cpu-omap.c +++ b/arch/arm/plat-omap/cpu-omap.c @@ -149,8 +149,6 @@ static int __init omap_cpu_init(struct cpufreq_policy *policy) VERY_HI_RATE) / 1000; } - clk_set_rate(mpu_clk, policy->cpuinfo.max_freq * 1000); - policy->min = policy->cpuinfo.min_freq; policy->max = policy->cpuinfo.max_freq; policy->cur = omap_getspeed(0); -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3evm: touchscreen delays on pm branch
>> Selecting performance governors solved the issue. > > Nope. No cpufreq. I did notice that CPIO175 was not being set > properly; have made local changes. It is a definite improvement; > but there are still very noticeable delays. > If PER Power domain is entering RET then module wakeups wont work! If at C2 you are seeing this then it might explain. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3evm: touchscreen delays on pm branch
> > It is the same driver at SDP3430. I had earlier tried removing > cpuidle altogether and did not see this issue. I too believe that > issue is caused by clocks being going to (auto)idle. > > But then, Hemanth should be seeing the same behavior. Do you see PER powerdomain entering retention? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: omap3evm: touchscreen delays on pm branch
On Fri, 2009-11-27 at 14:11 +0530, Premi, Sanjeev wrote: > > -Original Message- > > From: Dasgupta, Romit > > Sent: Friday, November 27, 2009 2:10 PM > > To: Premi, Sanjeev > > Cc: linux-omap@vger.kernel.org > > Subject: Re: omap3evm: touchscreen delays on pm branch > > > > Premi, Sanjeev wrote: > > > Hi, > > > > > > I am finding the response of touchscreen on the omap3evm very slow. > > > > > > Here is my test: > > > On console, I run : watch -n2 "cat /proc/interrupts" > > > Then, I tap the touchscreen approximately once per second. However, > > > (usually) no interrupts are registered. As I increase the frequency > > > of 'taps' more and more interrupts are registered. But still not > > > matching exact taps. > > > > > > However, when I keep the cpu busy with "cat /dev/zero > /dev/null &" > > > each tap is recognized. > > > > > Do you see this even if we don't enable OFF? > > > Yes. Sleep_while_idle=0; enable_off_mode=0 > ~sanjeev Hopefully you have the same TSC driver. Nevertheless, can you please try this (just to see if clock domain idling is causing any problem or not): diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c index 1cfa5a6..79710a1 100644 --- a/arch/arm/mach-omap2/cpuidle34xx.c +++ b/arch/arm/mach-omap2/cpuidle34xx.c @@ -248,7 +248,8 @@ void omap_init_power_states(void) cpuidle_params_table[OMAP3_STATE_C2].threshold; omap3_power_states[OMAP3_STATE_C2].mpu_state = PWRDM_POWER_ON; omap3_power_states[OMAP3_STATE_C2].core_state = PWRDM_POWER_ON; - omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID; + omap3_power_states[OMAP3_STATE_C2].flags = CPUIDLE_FLAG_TIME_VALID | + CPUIDLE_FLAG_CHECK_BM; /* C3 . MPU CSWR + Core inactive */ omap3_power_states[OMAP3_STATE_C3].valid = -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: omap3evm: touchscreen delays on pm branch
Premi, Sanjeev wrote: > Hi, > > I am finding the response of touchscreen on the omap3evm very slow. > > Here is my test: > On console, I run : watch -n2 "cat /proc/interrupts" > Then, I tap the touchscreen approximately once per second. However, > (usually) no interrupts are registered. As I increase the frequency > of 'taps' more and more interrupts are registered. But still not > matching exact taps. > > However, when I keep the cpu busy with "cat /dev/zero > /dev/null &" > each tap is recognized. > Do you see this even if we don't enable OFF? -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2 v2] musb: Add context save and restore support
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -2167,21 +2167,163 @@ static int __devexit musb_remove(struct
> platform_device *pdev)
>
> #ifdef CONFIG_PM
>
> +static struct musb_context_registers musb_context;
> +
> +void musb_save_context(void __iomem *musb_base)
> +{
> + int i;
> +
> + musb_context.faddr = musb_readb(musb_base, MUSB_FADDR);
> + musb_context.power = musb_readb(musb_base, MUSB_POWER);
> + musb_context.intrtxe = musb_readw(musb_base, MUSB_INTRTXE);
> + musb_context.intrrxe = musb_readw(musb_base, MUSB_INTRRXE);
> + musb_context.intrusbe = musb_readb(musb_base, MUSB_INTRUSBE);
> + musb_context.frame = musb_readw(musb_base, MUSB_FRAME);
Not necessary for gadget
> + musb_context.index = musb_readb(musb_base, MUSB_INDEX);
> + musb_context.testmode = musb_readb(musb_base, MUSB_TESTMODE);
Not sure if it is necessary for gadget.
> + musb_context.devctl = musb_readb(musb_base, MUSB_DEVCTL);
> +
> + for (i = 0; i < MUSB_C_NUM_EPS; ++i) {
> + musb_writeb(musb_base, MUSB_INDEX, i);
> + musb_context.index_regs[i].txmaxp =
> + musb_readw(musb_base, 0x10 + MUSB_TXMAXP);
> + musb_context.index_regs[i].txcsr =
> + musb_readw(musb_base, 0x10 + MUSB_TXCSR);
> + musb_context.index_regs[i].rxmaxp =
> + musb_readw(musb_base, 0x10 + MUSB_RXMAXP);
> + musb_context.index_regs[i].rxcsr =
> + musb_readw(musb_base, 0x10 + MUSB_RXCSR);
> + musb_context.index_regs[i].rxcount =
> + musb_readw(musb_base, 0x10 + MUSB_RXCOUNT);
> + musb_context.index_regs[i].txtype =
> + musb_readb(musb_base, 0x10 + MUSB_TXTYPE);
> + musb_context.index_regs[i].txinterval =
> + musb_readb(musb_base, 0x10 + MUSB_TXINTERVAL);
> + musb_context.index_regs[i].rxtype =
> + musb_readb(musb_base, 0x10 + MUSB_RXTYPE);
> + musb_context.index_regs[i].rxinterval =
> + musb_readb(musb_base, 0x10 + MUSB_RXINTERVAL);
> +
> + musb_context.index_regs[i].txfifoadd =
> + musb_read_txfifoadd(musb_base);
> + musb_context.index_regs[i].rxfifoadd =
> + musb_read_rxfifoadd(musb_base);
> + musb_context.index_regs[i].txfifosz =
> + musb_read_txfifosz(musb_base);
> + musb_context.index_regs[i].rxfifosz =
> + musb_read_rxfifosz(musb_base);
If MUSB_CONFIGDATA_DYNFIFO is not set then saving FIFO address and sizes are
probably not necessary. Not sure if they are detrimental as well.
> +
> + musb_context.index_regs[i].txfunaddr =
> + musb_read_txfunaddr(musb_base, i);
> + musb_context.index_regs[i].txhubaddr =
> + musb_read_txhubaddr(musb_base, i);
> + musb_context.index_regs[i].txhubport =
> + musb_read_txhubport(musb_base, i);
> +
> + musb_context.index_regs[i].rxfunaddr =
> + musb_read_rxfunaddr(musb_base, i);
> + musb_context.index_regs[i].rxhubaddr =
> + musb_read_rxhubaddr(musb_base, i);
> + musb_context.index_regs[i].rxhubport =
> + musb_read_rxhubport(musb_base, i);
> + }
If we are in gadget mode, do we have to save this?
> +
> + musb_writeb(musb_base, MUSB_INDEX, musb_context.index);
> +
> + musb_platform_save_context(&musb_context);
> +}
Similar for restore.
> static int musb_suspend(struct device *dev)
> {
> struct platform_device *pdev = to_platform_device(dev);
> unsigned long flags;
> struct musb *musb = dev_to_musb(&pdev->dev);
> + u8 reg;
>
> if (!musb->clock)
> return 0;
>
> spin_lock_irqsave(&musb->lock, flags);
>
> + musb_save_context(musb->mregs);
> +
> if (is_peripheral_active(musb)) {
> - /* FIXME force disconnect unless we know USB will wake
> -* the system up quickly enough to respond ...
> + /* System is entering into suspend where gadget would not be
> +* able to respond to host and thus it will be in an unknown
> +* state for host.Re-enumemation of gadget is required after
> +* resume to make the gadget functional thus doing a force
> +* disconnect.
> */
> + reg = musb_readb(musb->mregs, MUSB_POWER);
> + reg &= ~MUSB_POWER_SOFTCONN;
> + musb_writeb(musb->mregs, MUSB_POWER, reg);
After this softdisconnect when we subsequently get reconnected followed by a
rese
Re: [PATCH 1/1]: Thaws refrigerated bdi flusher threads before invoking kthread_stop on them
Resubmitting after incorporationg the suggestions:
>
> Romit, please rework as suggested by Jens and resubmit.
Unfreezes the bdi flusher task when the said task needs to exit.
Steps to reproduce this.
1) Mount a file system from MMC/SD card.
2) Unmount the file system. This creates a flusher task.
3) Attempt suspend to RAM. System is unresponsive.
This is because the bdi flusher thread is already in the refrigerator and will
remain so until it is thawed. The MMC driver suspend routine call stack will
ultimately issue a 'kthread_stop' on the bdi flusher thread and will block
until the flusher thread is exited. Since the bdi flusher thread is in the
refrigerator it never cleans up until thawed.
Signed-off-by: Romit Dasgupta
---
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 5a37e20..5a9ab6f 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -603,11 +603,15 @@ static void bdi_wb_shutdown(struct backing_dev_info *bdi)
bdi_remove_from_list(bdi);
/*
+* Force unfreeze of bdi threads before stopping it, otherwise
+* it would never exit if it is stuck in the refrigerator.
* Finally, kill the kernel threads. We don't need to be RCU
* safe anymore, since the bdi is gone from visibility.
*/
- list_for_each_entry(wb, &bdi->wb_list, list)
+ list_for_each_entry(wb, &bdi->wb_list, list) {
+ wb->task->flags &= ~PF_FROZEN;
kthread_stop(wb->task);
+ }
}
void bdi_unregister(struct backing_dev_info *bdi)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1]: Thaws refrigerated bdi flusher threads before invoking kthread_stop on them
Hello Rafael,
As suggested I have added the relevant information in the
changelog. The patch is below:
> For completness, below is the information from the Romit's introductory
> message
> (Romit, I really think that should go into the chagelog):
Kicks out frozen bdi flusher task out of the refrigerator when the said task
needs to exit.
Steps to reproduce this.
1) Mount a file system from MMC/SD card.
2) Unmount the file system. This creates a flusher task.
3) Attempt suspend to RAM. System is unresponsive.
This is because the bdi flusher thread is already in the refrigerator and will
remain so until it is thawed. The MMC driver suspend routine ultimately will
issue a 'kthread_stop' on the bdi flusher thread and will block until the
flusher thread is exited. Since the bdi flusher thread is in the refrigerator
it never cleans up until thawed.
Signed-off-by: Romit Dasgupta
---
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 5a37e20..c757b05 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -606,8 +606,11 @@ static void bdi_wb_shutdown(struct backing_dev_info *bdi)
* Finally, kill the kernel threads. We don't need to be RCU
* safe anymore, since the bdi is gone from visibility.
*/
- list_for_each_entry(wb, &bdi->wb_list, list)
+ list_for_each_entry(wb, &bdi->wb_list, list) {
+ if (unlikely(frozen(wb->task)))
+ wb->task->flags &= ~PF_FROZEN;
kthread_stop(wb->task);
+ }
}
void bdi_unregister(struct backing_dev_info *bdi)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1]: Thaws refrigerated bdi flusher threads before invoking kthread_stop on them
Kicks out frozen bdi flusher task out of the refrigerator when the flusher task
needs to exit.
Signed-off-by: Romit Dasgupta
---
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index 5a37e20..c757b05 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -606,8 +606,11 @@ static void bdi_wb_shutdown(struct backing_dev_info *bdi)
* Finally, kill the kernel threads. We don't need to be RCU
* safe anymore, since the bdi is gone from visibility.
*/
- list_for_each_entry(wb, &bdi->wb_list, list)
+ list_for_each_entry(wb, &bdi->wb_list, list) {
+ if (unlikely(frozen(wb->task)))
+ wb->task->flags &= ~PF_FROZEN;
kthread_stop(wb->task);
+ }
}
void bdi_unregister(struct backing_dev_info *bdi)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/1]: Thaws refrigerated bdi flusher threads before invoking kthread_stop on them
Hi, Few days back I started facing problems during system suspend with MMC, SD card installed. I will restate how to reproduce the problem: 1) Mount a file system from MMC/SD card. 2) Unmount the file system. This creates a flusher task. 3) Attempt suspend to RAM. System is unresponsive. This is because the bdi flusher thread is already in the refrigerator and will remain so until it is thawed. The MMC driver suspend routine ultimately will issue a 'kthread_stop' on the bdi flusher thread and will block until the flusher thread is exited. Since the bdi flusher thread is in the refrigerator it never cleans up until thawed. Enabling khungtaskd gave the following dump: (the dump wraps beyond 80 cols). INFO: task sh:387 blocked for more than 120 seconds. "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. shD c027e86c 0 387 1 0x [] (schedule+0x2e0/0x36c) from [] (schedule_timeout+0x18/0x1ec) [] (schedule_timeout+0x18/0x1ec) from [] (wait_for_common+0xe0/0x198) [] (wait_for_common+0xe0/0x198) from [] (kthread_stop+0x44/0x78) [] (kthread_stop+0x44/0x78) from [] (bdi_unregister+0x64/0xa4) [] (bdi_unregister+0x64/0xa4) from [] (unlink_gendisk+0x20/0x3c) [] (unlink_gendisk+0x20/0x3c) from [] (del_gendisk+0x84/0xb4) [] (del_gendisk+0x84/0xb4) from [] (mmc_blk_remove+0x24/0x44) [] (mmc_blk_remove+0x24/0x44) from [] (mmc_bus_remove+0x18/0x20) [] (mmc_bus_remove+0x18/0x20) from [] (__device_release_driver+0x64/0xa4) [] (__device_release_driver+0x64/0xa4) from [] (device_release_driver+0x1c/0x28) [] (device_release_driver+0x1c/0x28) from [] (bus_remove_device+0x7c/0x90) [] (bus_remove_device+0x7c/0x90) from [] (device_del+0x110/0x160) [] (device_del+0x110/0x160) from [] (mmc_remove_card+0x50/0x64) [] (mmc_remove_card+0x50/0x64) from [] (mmc_sd_remove+0x24/0x30) [] (mmc_sd_remove+0x24/0x30) from [] (mmc_suspend_host+0x110/0x1a8) [] (mmc_suspend_host+0x110/0x1a8) from [] (omap_hsmmc_suspend+0x74/0x104) [] (omap_hsmmc_suspend+0x74/0x104) from [] (platform_pm_suspend+0x50/0x5c) [] (platform_pm_suspend+0x50/0x5c) from [] (pm_op+0x30/0x74) [] (pm_op+0x30/0x74) from [] (dpm_suspend_start+0x3b4/0x518) [] (dpm_suspend_start+0x3b4/0x518) from [] (suspend_devices_and_enter+0x3c/0x1c4) [] (suspend_devices_and_enter+0x3c/0x1c4) from [] (enter_state+0xe0/0x138) [] (enter_state+0xe0/0x138) from [] (state_store+0x94/0xbc) [] (state_store+0x94/0xbc) from [] (kobj_attr_store+0x18/0x1c) [] (kobj_attr_store+0x18/0x1c) from [] (sysfs_write_file+0x108/0x13c) [] (sysfs_write_file+0x108/0x13c) from [] (vfs_write+0xac/0x154) [] (vfs_write+0xac/0x154) from [] (sys_write+0x3c/0x68) [] (sys_write+0x3c/0x68) from [] (ret_fast_syscall+0x0/0x2c) Earlier I had sent a patch to thaw any refrigerated kernel thread when some active thread has invoked 'kthread_stop' on it. This was done with the assumption that all such kernel threads should invoke 'kthread_should_stop' after 'try_to_freeze' and exit if necessary. It looks there are some kernel threads which do not follow this. With that in mind I am sending a different patch to fix the above issue (in the next mail). Regards, -Romit -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
