public inbox for linux-omap@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up
@ 2010-01-05  2:42 Paul Walmsley
  2010-01-05  2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05  2:42 UTC (permalink / raw)
  To: linux-omap

Hello,

This series fixes several compilation errors for OMAP2xxx chips when
CONFIG_CPU_FREQ is set.  These were reported and partially fixed
by Felipe Balbi <felipe.balbi@nokia.com>.

Considerable work is still needed to fully support dynamic frequency
changes on OMAP2xxx-series chips.  Readers interested in such a
project are encouraged to review the Maemo Diablo RX-34 and RX-44
kernel source at:

    http://repository.maemo.org/pool/diablo/free/k/kernel-source-diablo/

Boot-tested on N800 with n8x0_defconfig with CONFIG_CPU_FREQ; compile-tested
for 2430SDP.


- Paul

---

Felipe Balbi (1):
      OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled

Paul Walmsley (3):
      OMAP2xxx OPP: clean up comments in OPP data
      OMAP2 clock: dynamically allocate CPUFreq frequency table
      OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table


 arch/arm/mach-omap2/clock2xxx.c         |   60 ++++++++++++++++++++++++++-----
 arch/arm/mach-omap2/opp2420_data.c      |   38 ++++++++++----------
 arch/arm/mach-omap2/opp2430_data.c      |   30 ++++++++--------
 arch/arm/plat-omap/clock.c              |   10 +++++
 arch/arm/plat-omap/cpu-omap.c           |    1 +
 arch/arm/plat-omap/include/plat/clock.h |    2 +
 6 files changed, 99 insertions(+), 42 deletions(-)


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
  2010-01-05  2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
@ 2010-01-05  2:42 ` Paul Walmsley
  2010-01-05 10:51   ` Felipe Balbi
  2010-01-05  2:42 ` [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data Paul Walmsley
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05  2:42 UTC (permalink / raw)
  To: linux-omap; +Cc: Felipe Balbi

From: Felipe Balbi <felipe.balbi@nokia.com>

if we enable CPUFREQ we can't build omap2 for two reasons,
one of them is fixed by the patch below.

It's failing because the __must_be_array() check in
ARRAY_SIZE() is failing and printing the following message:

arch/arm/mach-omap2/clock2xxx.c:453: error: negative width in bit-field '<anonymous>'

Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
[paul@pwsan.com: commit message updated; changed rate variable name]
Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock2xxx.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
index d0e3fb7..2f3c3ba 100644
--- a/arch/arm/mach-omap2/clock2xxx.c
+++ b/arch/arm/mach-omap2/clock2xxx.c
@@ -455,12 +455,15 @@ static struct cpufreq_frequency_table freq_table[ARRAY_SIZE(rate_table)];
 void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 {
 	struct prcm_config *prcm;
+	long rate;
 	int i = 0;
 
+	sys_ck_rate = clk_get_rate(sclk);
+
 	for (prcm = rate_table; prcm->mpu_speed; prcm++) {
 		if (!(prcm->flags & cpu_mask))
 			continue;
-		if (prcm->xtal_speed != sys_ck.rate)
+		if (prcm->xtal_speed != sys_ck_rate)
 			continue;
 
 		/* don't put bypass rates in table */



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data
  2010-01-05  2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
  2010-01-05  2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
@ 2010-01-05  2:42 ` Paul Walmsley
  2010-01-05  2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
  2010-01-05  2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
  3 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05  2:42 UTC (permalink / raw)
  To: linux-omap; +Cc: Richard Woodruff

Revise some of the comments in the OMAP2xxx OPP data for clarity.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Richard Woodruff <r-woodruff2@ti.com>
---
 arch/arm/mach-omap2/opp2420_data.c |   38 +++++++++++++++++++-----------------
 arch/arm/mach-omap2/opp2430_data.c |   30 ++++++++++++++--------------
 2 files changed, 35 insertions(+), 33 deletions(-)

diff --git a/arch/arm/mach-omap2/opp2420_data.c b/arch/arm/mach-omap2/opp2420_data.c
index 126a939..e6dda69 100644
--- a/arch/arm/mach-omap2/opp2420_data.c
+++ b/arch/arm/mach-omap2/opp2420_data.c
@@ -9,45 +9,47 @@
  * The OMAP2 processor can be run at several discrete 'PRCM configurations'.
  * These configurations are characterized by voltage and speed for clocks.
  * The device is only validated for certain combinations. One way to express
- * these combinations is via the 'ratio's' which the clocks operate with
+ * these combinations is via the 'ratios' which the clocks operate with
  * respect to each other. These ratio sets are for a given voltage/DPLL
- * setting. All configurations can be described by a DPLL setting and a ratio
- * There are 3 ratio sets for the 2430 and X ratio sets for 2420.
- *
- * 2430 differs from 2420 in that there are no more phase synchronizers used.
- * They both have a slightly different clock domain setup. 2420(iva1,dsp) vs
- * 2430 (iva2.1, NOdsp, mdm)
+ * setting. All configurations can be described by a DPLL setting and a ratio.
  *
  * XXX Missing voltage data.
+ * XXX Missing 19.2MHz sys_clk rate sets (needed for N800/N810)
  *
  * THe format described in this file is deprecated.  Once a reasonable
  * OPP API exists, the data in this file should be converted to use it.
  *
  * This is technically part of the OMAP2xxx clock code.
+ *
+ * Considerable work is still needed to fully support dynamic frequency
+ * changes on OMAP2xxx-series chips.  Readers interested in such a
+ * project are encouraged to review the Maemo Diablo RX-34 and RX-44
+ * kernel source at:
+ *     http://repository.maemo.org/pool/diablo/free/k/kernel-source-diablo/
  */
 
 #include "opp2xxx.h"
 #include "sdrc.h"
 #include "clock.h"
 
-/*-------------------------------------------------------------------------
- * Key dividers which make up a PRCM set. Ratio's for a PRCM are mandated.
+/*
+ * Key dividers which make up a PRCM set. Ratios for a PRCM are mandated.
  * xtal_speed, dpll_speed, mpu_speed, CM_CLKSEL_MPU,
  * CM_CLKSEL_DSP, CM_CLKSEL_GFX, CM_CLKSEL1_CORE, CM_CLKSEL1_PLL,
  * CM_CLKSEL2_PLL, CM_CLKSEL_MDM
  *
- * Filling in table based on H4 boards and 2430-SDPs variants available.
- * There are quite a few more rates combinations which could be defined.
+ * Filling in table based on H4 boards available.  There are quite a
+ * few more rate combinations which could be defined.
  *
- * When multiple values are defined the start up will try and choose the
- * fastest one. If a 'fast' value is defined, then automatically, the /2
- * one should be included as it can be used.	Generally having more that
- * one fast set does not make sense, as static timings need to be changed
- * to change the set.	 The exception is the bypass setting which is
- * availble for low power bypass.
+ * When multiple values are defined the start up will try and choose
+ * the fastest one. If a 'fast' value is defined, then automatically,
+ * the /2 one should be included as it can be used.  Generally having
+ * more than one fast set does not make sense, as static timings need
+ * to be changed to change the set.  The exception is the bypass
+ * setting which is available for low power bypass.
  *
  * Note: This table needs to be sorted, fastest to slowest.
- *-------------------------------------------------------------------------*/
+ **/
 const struct prcm_config omap2420_rate_table[] = {
 	/* PRCM I - FAST */
 	{S12M, S660M, S330M, RI_CM_CLKSEL_MPU_VAL,		/* 330MHz ARM */
diff --git a/arch/arm/mach-omap2/opp2430_data.c b/arch/arm/mach-omap2/opp2430_data.c
index edb8167..1b9596a 100644
--- a/arch/arm/mach-omap2/opp2430_data.c
+++ b/arch/arm/mach-omap2/opp2430_data.c
@@ -1,5 +1,5 @@
 /*
- * opp2420_data.c - old-style "OPP" table for OMAP2420
+ * opp2430_data.c - old-style "OPP" table for OMAP2430
  *
  * Copyright (C) 2005-2009 Texas Instruments, Inc.
  * Copyright (C) 2004-2009 Nokia Corporation
@@ -9,16 +9,16 @@
  * The OMAP2 processor can be run at several discrete 'PRCM configurations'.
  * These configurations are characterized by voltage and speed for clocks.
  * The device is only validated for certain combinations. One way to express
- * these combinations is via the 'ratio's' which the clocks operate with
+ * these combinations is via the 'ratios' which the clocks operate with
  * respect to each other. These ratio sets are for a given voltage/DPLL
- * setting. All configurations can be described by a DPLL setting and a ratio
- * There are 3 ratio sets for the 2430 and X ratio sets for 2420.
+ * setting. All configurations can be described by a DPLL setting and a ratio.
  *
  * 2430 differs from 2420 in that there are no more phase synchronizers used.
  * They both have a slightly different clock domain setup. 2420(iva1,dsp) vs
  * 2430 (iva2.1, NOdsp, mdm)
  *
  * XXX Missing voltage data.
+ * XXX Missing 19.2MHz sys_clk rate sets.
  *
  * THe format described in this file is deprecated.  Once a reasonable
  * OPP API exists, the data in this file should be converted to use it.
@@ -30,24 +30,24 @@
 #include "sdrc.h"
 #include "clock.h"
 
-/*-------------------------------------------------------------------------
- * Key dividers which make up a PRCM set. Ratio's for a PRCM are mandated.
+/*
+ * Key dividers which make up a PRCM set. Ratios for a PRCM are mandated.
  * xtal_speed, dpll_speed, mpu_speed, CM_CLKSEL_MPU,
  * CM_CLKSEL_DSP, CM_CLKSEL_GFX, CM_CLKSEL1_CORE, CM_CLKSEL1_PLL,
  * CM_CLKSEL2_PLL, CM_CLKSEL_MDM
  *
- * Filling in table based on H4 boards and 2430-SDPs variants available.
- * There are quite a few more rates combinations which could be defined.
+ * Filling in table based on 2430-SDPs variants available.  There are
+ * quite a few more rate combinations which could be defined.
  *
- * When multiple values are defined the start up will try and choose the
- * fastest one. If a 'fast' value is defined, then automatically, the /2
- * one should be included as it can be used.	Generally having more that
- * one fast set does not make sense, as static timings need to be changed
- * to change the set.	 The exception is the bypass setting which is
- * availble for low power bypass.
+ * When multiple values are defined the start up will try and choose
+ * the fastest one. If a 'fast' value is defined, then automatically,
+ * the /2 one should be included as it can be used.  Generally having
+ * more than one fast set does not make sense, as static timings need
+ * to be changed to change the set.  The exception is the bypass
+ * setting which is available for low power bypass.
  *
  * Note: This table needs to be sorted, fastest to slowest.
- *-------------------------------------------------------------------------*/
+ */
 const struct prcm_config omap2430_rate_table[] = {
 	/* PRCM #4 - ratio2 (ES2.1) - FAST */
 	{S13M, S798M, S399M, R2_CM_CLKSEL_MPU_VAL,		/* 399MHz ARM */



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
  2010-01-05  2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
  2010-01-05  2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
  2010-01-05  2:42 ` [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data Paul Walmsley
@ 2010-01-05  2:42 ` Paul Walmsley
  2010-01-05 10:42   ` Felipe Balbi
  2010-01-05  2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
  3 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05  2:42 UTC (permalink / raw)
  To: linux-omap; +Cc: Felipe Balbi

Dynamically allocate the CPUFreq frequency table on OMAP2xxx chips.
This fixes some compilation problems, since the kernel may not know
what chip it is running on until boot-time.  This also reduces the size
of the CPUFreq frequency table.

Problem reported by Felipe Balbi <felipe.balbi@nokia.com>.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Felipe Balbi <felipe.balbi@nokia.com>
---
 arch/arm/mach-omap2/clock2xxx.c |   50 ++++++++++++++++++++++++++++++++-------
 1 files changed, 41 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
index 2f3c3ba..5c21afe 100644
--- a/arch/arm/mach-omap2/clock2xxx.c
+++ b/arch/arm/mach-omap2/clock2xxx.c
@@ -449,14 +449,16 @@ int omap2_select_table_rate(struct clk *clk, unsigned long rate)
 #ifdef CONFIG_CPU_FREQ
 /*
  * Walk PRCM rate table and fillout cpufreq freq_table
+ * XXX This should be replaced by an OPP layer in the near future
  */
-static struct cpufreq_frequency_table freq_table[ARRAY_SIZE(rate_table)];
+static struct cpufreq_frequency_table *freq_table;
 
 void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 {
-	struct prcm_config *prcm;
-	long rate;
+	const struct prcm_config *prcm;
+	long sys_ck_rate;
 	int i = 0;
+	int tbl_sz = 0;
 
 	sys_ck_rate = clk_get_rate(sclk);
 
@@ -470,21 +472,51 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 		if (prcm->dpll_speed == prcm->xtal_speed)
 			continue;
 
+		tbl_sz++;
+	}
+
+	/*
+	 * XXX Ensure that we're doing what CPUFreq expects for this error
+	 * case and the following one
+	 */
+	if (tbl_sz == 0) {
+		pr_warning("%s: no matching entries in rate_table\n",
+			   __func__);
+		goto ocict_exit;
+	}
+
+	/* Include the CPUFREQ_TABLE_END terminator entry */
+	tbl_sz++;
+
+	freq_table = kzalloc(sizeof(struct cpufreq_frequency_table) * tbl_sz,
+			     GFP_KERNEL);
+	if (!freq_table) {
+		pr_err("%s: could not kzalloc frequency table\n", __func__);
+		goto ocict_exit;
+	}
+
+	for (prcm = rate_table; prcm->mpu_speed; prcm++) {
+		if (!(prcm->flags & cpu_mask))
+			continue;
+		if (prcm->xtal_speed != sys_ck_rate)
+			continue;
+
+		/* don't put bypass rates in table */
+		if (prcm->dpll_speed == prcm->xtal_speed)
+			continue;
+
 		freq_table[i].index = i;
 		freq_table[i].frequency = prcm->mpu_speed / 1000;
 		i++;
 	}
 
-	if (i == 0) {
-		printk(KERN_WARNING "%s: failed to initialize frequency "
-		       "table\n", __func__);
-		return;
-	}
-
 	freq_table[i].index = i;
 	freq_table[i].frequency = CPUFREQ_TABLE_END;
 
 	*table = &freq_table[0];
+
+ocict_exit:
+	return;
 }
 #endif
 



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
  2010-01-05  2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
                   ` (2 preceding siblings ...)
  2010-01-05  2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
@ 2010-01-05  2:42 ` Paul Walmsley
  2010-01-05 10:46   ` Felipe Balbi
  3 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05  2:42 UTC (permalink / raw)
  To: linux-omap

OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
so this patch ensures that it is freed if CPUFreq terminates.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
---
 arch/arm/mach-omap2/clock2xxx.c         |    7 +++++++
 arch/arm/plat-omap/clock.c              |   10 ++++++++++
 arch/arm/plat-omap/cpu-omap.c           |    1 +
 arch/arm/plat-omap/include/plat/clock.h |    2 ++
 4 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
index 5c21afe..ec57927 100644
--- a/arch/arm/mach-omap2/clock2xxx.c
+++ b/arch/arm/mach-omap2/clock2xxx.c
@@ -518,6 +518,12 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 ocict_exit:
 	return;
 }
+
+void omap2_clk_exit_cpufreq_table(struct cpufreq_frequency_table **table)
+{
+	kfree(freq_table);
+}
+
 #endif
 
 struct clk_functions omap2_clk_functions = {
@@ -529,6 +535,7 @@ struct clk_functions omap2_clk_functions = {
 	.clk_disable_unused	= omap2_clk_disable_unused,
 #ifdef	CONFIG_CPU_FREQ
 	.clk_init_cpufreq_table	= omap2_clk_init_cpufreq_table,
+	.clk_exit_cpufreq_table	= omap2_clk_exit_cpufreq_table,
 #endif
 };
 
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index a64d84c..d9f8c84 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -329,6 +329,16 @@ void clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
 		arch_clock->clk_init_cpufreq_table(table);
 	spin_unlock_irqrestore(&clockfw_lock, flags);
 }
+
+void clk_exit_cpufreq_table(struct cpufreq_frequency_table **table)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&clockfw_lock, flags);
+	if (arch_clock->clk_exit_cpufreq_table)
+		arch_clock->clk_exit_cpufreq_table(table);
+	spin_unlock_irqrestore(&clockfw_lock, flags);
+}
 #endif
 
 /*-------------------------------------------------------------------------*/
diff --git a/arch/arm/plat-omap/cpu-omap.c b/arch/arm/plat-omap/cpu-omap.c
index f8ddbdd..6d3d333 100644
--- a/arch/arm/plat-omap/cpu-omap.c
+++ b/arch/arm/plat-omap/cpu-omap.c
@@ -134,6 +134,7 @@ static int __init omap_cpu_init(struct cpufreq_policy *policy)
 
 static int omap_cpu_exit(struct cpufreq_policy *policy)
 {
+	clk_exit_cpufreq_table(&freq_table);
 	clk_put(mpu_clk);
 	return 0;
 }
diff --git a/arch/arm/plat-omap/include/plat/clock.h b/arch/arm/plat-omap/include/plat/clock.h
index 309b6d1..94fe2a0 100644
--- a/arch/arm/plat-omap/include/plat/clock.h
+++ b/arch/arm/plat-omap/include/plat/clock.h
@@ -119,6 +119,7 @@ struct clk_functions {
 	void		(*clk_disable_unused)(struct clk *clk);
 #ifdef CONFIG_CPU_FREQ
 	void		(*clk_init_cpufreq_table)(struct cpufreq_frequency_table **);
+	void		(*clk_exit_cpufreq_table)(struct cpufreq_frequency_table **);
 #endif
 };
 
@@ -135,6 +136,7 @@ extern unsigned long followparent_recalc(struct clk *clk);
 extern void clk_enable_init_clocks(void);
 #ifdef CONFIG_CPU_FREQ
 extern void clk_init_cpufreq_table(struct cpufreq_frequency_table **table);
+extern void clk_exit_cpufreq_table(struct cpufreq_frequency_table **table);
 #endif
 
 extern const struct clkops clkops_null;



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
  2010-01-05  2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
@ 2010-01-05 10:42   ` Felipe Balbi
  2010-01-05 16:22     ` Paul Walmsley
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2010-01-05 10:42 UTC (permalink / raw)
  To: ext Paul Walmsley
  Cc: linux-omap@vger.kernel.org, Balbi Felipe (Nokia-D/Helsinki)

Hi,

On Tue, Jan 05, 2010 at 03:42:07AM +0100, ext Paul Walmsley wrote:
>@@ -470,21 +472,51 @@ void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
> 		if (prcm->dpll_speed == prcm->xtal_speed)
> 			continue;
>
>+		tbl_sz++;
>+	}
>+
>+	/*
>+	 * XXX Ensure that we're doing what CPUFreq expects for this error
>+	 * case and the following one
>+	 */
>+	if (tbl_sz == 0) {
>+		pr_warning("%s: no matching entries in rate_table\n",
>+			   __func__);
>+		goto ocict_exit;

you only use this ocict_exit here, so how about only return instead of a 
goto to a return ?

-- 
balbi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
  2010-01-05  2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
@ 2010-01-05 10:46   ` Felipe Balbi
  2010-01-05 15:58     ` Paul Walmsley
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2010-01-05 10:46 UTC (permalink / raw)
  To: ext Paul Walmsley; +Cc: linux-omap@vger.kernel.org

On Tue, Jan 05, 2010 at 03:42:08AM +0100, ext Paul Walmsley wrote:
>OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
>so this patch ensures that it is freed if CPUFreq terminates.

wasn't this introduced by the previous patch ?

-- 
balbi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
  2010-01-05  2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
@ 2010-01-05 10:51   ` Felipe Balbi
  2010-01-05 16:03     ` Paul Walmsley
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2010-01-05 10:51 UTC (permalink / raw)
  To: ext Paul Walmsley
  Cc: linux-omap@vger.kernel.org, Balbi Felipe (Nokia-D/Helsinki)

Hi,

On Tue, Jan 05, 2010 at 03:42:05AM +0100, ext Paul Walmsley wrote:
>From: Felipe Balbi <felipe.balbi@nokia.com>
>
>if we enable CPUFREQ we can't build omap2 for two reasons,
>one of them is fixed by the patch below.
>
>It's failing because the __must_be_array() check in
>ARRAY_SIZE() is failing and printing the following message:
>
>arch/arm/mach-omap2/clock2xxx.c:453: error: negative width in bit-field '<anonymous>'
>
>Signed-off-by: Felipe Balbi <felipe.balbi@nokia.com>
>[paul@pwsan.com: commit message updated; changed rate variable name]

and you forgot to change the variable definition...

>Signed-off-by: Paul Walmsley <paul@pwsan.com>
>---
> arch/arm/mach-omap2/clock2xxx.c |    5 ++++-
> 1 files changed, 4 insertions(+), 1 deletions(-)
>
>diff --git a/arch/arm/mach-omap2/clock2xxx.c b/arch/arm/mach-omap2/clock2xxx.c
>index d0e3fb7..2f3c3ba 100644
>--- a/arch/arm/mach-omap2/clock2xxx.c
>+++ b/arch/arm/mach-omap2/clock2xxx.c
>@@ -455,12 +455,15 @@ static struct cpufreq_frequency_table freq_table[ARRAY_SIZE(rate_table)];
> void omap2_clk_init_cpufreq_table(struct cpufreq_frequency_table **table)
> {
> 	struct prcm_config *prcm;
>+	long rate;
> 	int i = 0;
>
>+	sys_ck_rate = clk_get_rate(sclk);

The variable defined is called rate but you use sys_ck_rate. 
I used rate, because that's what other functions are using and 
I wanted to keep the code consistent. Anyways, as is now this patch 
won't compile again :-p

-- 
balbi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
  2010-01-05 10:46   ` Felipe Balbi
@ 2010-01-05 15:58     ` Paul Walmsley
  2010-01-19 19:26       ` Paul Walmsley
  0 siblings, 1 reply; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 15:58 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org

On Tue, 5 Jan 2010, Felipe Balbi wrote:

> On Tue, Jan 05, 2010 at 03:42:08AM +0100, ext Paul Walmsley wrote:
> > OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
> > so this patch ensures that it is freed if CPUFreq terminates.
> 
> wasn't this introduced by the previous patch ?

The dynamically-allocated array?  Yes.  Your point being?


- Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled
  2010-01-05 10:51   ` Felipe Balbi
@ 2010-01-05 16:03     ` Paul Walmsley
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 16:03 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org

On Tue, 5 Jan 2010, Felipe Balbi wrote:

> and you forgot to change the variable definition...

Thanks, looks like I sent an older version of the series.  I will resend.

> The variable defined is called rate but you use sys_ck_rate. I used rate,
> because that's what other functions are using and I wanted to keep the code
> consistent. 

Did you examine the other functions omap2_round_to_table_rate() and 
omap2_select_table_rate()?

What advantage is there to keeping this variable named "rate", which is 
less specific given that this function also deals with DPLL rates, MPU 
rates, and crystal rates?


- Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table
  2010-01-05 10:42   ` Felipe Balbi
@ 2010-01-05 16:22     ` Paul Walmsley
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-05 16:22 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org

On Tue, 5 Jan 2010, Felipe Balbi wrote:

> On Tue, Jan 05, 2010 at 03:42:07AM +0100, ext Paul Walmsley wrote:
> > @@ -470,21 +472,51 @@ void omap2_clk_init_cpufreq_table(struct
> > cpufreq_frequency_table **table)
> > 		if (prcm->dpll_speed == prcm->xtal_speed)
> > 			continue;
> > 
> > +		tbl_sz++;
> > +	}
> > +
> > +	/*
> > +	 * XXX Ensure that we're doing what CPUFreq expects for this error
> > +	 * case and the following one
> > +	 */
> > +	if (tbl_sz == 0) {
> > +		pr_warning("%s: no matching entries in rate_table\n",
> > +			   __func__);
> > +		goto ocict_exit;
> 
> you only use this ocict_exit here, so how about only return instead of a goto
> to a return ?

It's used twice.  But your latter point is a good one.  I will change 
that.


- Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the CPUFreq frequency table
  2010-01-05 15:58     ` Paul Walmsley
@ 2010-01-19 19:26       ` Paul Walmsley
  0 siblings, 0 replies; 12+ messages in thread
From: Paul Walmsley @ 2010-01-19 19:26 UTC (permalink / raw)
  To: Felipe Balbi; +Cc: linux-omap@vger.kernel.org

Hi Felipe,

On Tue, 5 Jan 2010, Paul Walmsley wrote:

> On Tue, 5 Jan 2010, Felipe Balbi wrote:
> > On Tue, Jan 05, 2010 at 03:42:08AM +0100, ext Paul Walmsley wrote:
> > > OMAP2xxx uses a dynamically-allocated cpufreq_frequency_table array,
> > > so this patch ensures that it is freed if CPUFreq terminates.
> > 
> > wasn't this introduced by the previous patch ?
> 
> The dynamically-allocated array?  Yes.  Your point being?

By the way, I am sorry for this - it was too brusque for how I 
believe collaborative development should be.  Next time I will be more 
clear about the question...

regards,

- Paul

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2010-01-19 19:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-01-05  2:42 [PATCH 0/4] OMAP2xxx clock/CPUFreq: fix compilation errors; clean up Paul Walmsley
2010-01-05  2:42 ` [PATCH 1/4] OMAP2: clock: clk2xxx.c doesn't compile if CPUFREQ is enabled Paul Walmsley
2010-01-05 10:51   ` Felipe Balbi
2010-01-05 16:03     ` Paul Walmsley
2010-01-05  2:42 ` [PATCH 2/4] OMAP2xxx OPP: clean up comments in OPP data Paul Walmsley
2010-01-05  2:42 ` [PATCH 3/4] OMAP2 clock: dynamically allocate CPUFreq frequency table Paul Walmsley
2010-01-05 10:42   ` Felipe Balbi
2010-01-05 16:22     ` Paul Walmsley
2010-01-05  2:42 ` [PATCH 4/4] OMAP clock/CPUFreq: avoid leaking the " Paul Walmsley
2010-01-05 10:46   ` Felipe Balbi
2010-01-05 15:58     ` Paul Walmsley
2010-01-19 19:26       ` Paul Walmsley

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox