RE: [PATCH 5/5] [CPUFREQ] EXYNOS4210: Add support ASV feature

2011-11-04 Thread Kukjin Kim 
MyungJoo Ham wrote:
 
 Hello,
 
Hi,

I got the comments from Jaecheol Lee but his mail client has some problem so
I'm sending instead.

 On Wed, Nov 2, 2011 at 9:43 PM, Kukjin Kim kgene@samsung.com wrote:
 []
  +static void __init set_volt_table(void)
  +{
  +       unsigned int tmp, i, asv_group = 0;
  +
  +       tmp = __raw_readl(S5P_INFORM2);
 
 As I've mentioned in the ASV patch thread, do we really need to use an
 INFORM register simply to save the id of supported voltage ranges?
 
 Why aren't we using an extern variable here? For example, extern int
 asv_group_id; and define it at asv.h or somewhere else.
 
 At reboot, we are going to init ASV driver and will get the ASV value
 again; thus, we don't need to use such a preserving register anyway.
 At suspend/resume, the value in RAM does not disappear and the IPL
 does not care this value; thus, it is meaningless to use INFORM2 for
 this value.
 
ASV feature had been implemented in bootloader hence inform register was
used to return the result value to cpufreq driver. If there is no problem to
use the inform register why don't you keep this manner.

(snip)

Thanks.

Best regards,
Kgene.
--
Kukjin Kim kgene@samsung.com, Senior Engineer,
SW Solution Development Team, Samsung Electronics Co., Ltd.

--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 5/5] [CPUFREQ] EXYNOS4210: Add support ASV feature

2011-11-02 Thread Kukjin Kim 
From: Jaecheol Lee jc@samsung.com

This patch adds support ASV on Exynos4210. Exynos4 CPUFREQ
driver uses Adaptive Supply Voltage to configure voltage table.

Signed-off-by: Jaecheol Lee jc@samsung.com
---
 arch/arm/mach-exynos4/include/mach/cpufreq.h |7 +++
 drivers/cpufreq/exynos4210-cpufreq.c |   76
+-
 2 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/arch/arm/mach-exynos4/include/mach/cpufreq.h
b/arch/arm/mach-exynos4/include/mach/cpufreq.h
index 7e00931..16bb3e9 100644
--- a/arch/arm/mach-exynos4/include/mach/cpufreq.h
+++ b/arch/arm/mach-exynos4/include/mach/cpufreq.h
@@ -37,3 +37,10 @@ enum cpufreq_lock_ID {
 int exynos4_cpufreq_lock(unsigned int nId,
enum cpufreq_level_request cpufreq_level);
 void exynos4_cpufreq_lock_free(unsigned int nId);
+
+#define SUPPORT_1400MHZ(1  31)
+#define SUPPORT_1200MHZ(1  30)
+#define SUPPORT_1000MHZ(1  29)
+
+#define SUPPORT_FREQ_SHIFT 29
+#define SUPPORT_FREQ_MASK  7
diff --git a/drivers/cpufreq/exynos4210-cpufreq.c
b/drivers/cpufreq/exynos4210-cpufreq.c
index 30e1949..8dcd9b1 100644
--- a/drivers/cpufreq/exynos4210-cpufreq.c
+++ b/drivers/cpufreq/exynos4210-cpufreq.c
@@ -60,6 +60,8 @@ static struct cpufreq_frequency_table exynos4_freq_table[]
= {
{0, CPUFREQ_TABLE_END},
 };
 
+static unsigned int exynos4_volt_table[CPUFREQ_LEVEL_END];
+
 /* This defines are for cpufreq lock */
 #define CPUFREQ_MIN_LEVEL  (CPUFREQ_LEVEL_END - 1)
 unsigned int cpufreq_lock_id;
@@ -111,30 +113,6 @@ static unsigned int clkdiv_cpu1[CPUFREQ_LEVEL_END][2] =
{
{ 3, 0 },
 };
 
-struct cpufreq_voltage_table {
-   unsigned intindex;  /* any */
-   unsigned intarm_volt;   /* uV */
-};
-
-static struct cpufreq_voltage_table exynos4_volt_table[CPUFREQ_LEVEL_END] =
{
-   {
-   .index  = L0,
-   .arm_volt   = 135,
-   }, {
-   .index  = L1,
-   .arm_volt   = 130,
-   }, {
-   .index  = L2,
-   .arm_volt   = 120,
-   }, {
-   .index  = L3,
-   .arm_volt   = 110,
-   }, {
-   .index  = L4,
-   .arm_volt   = 105,
-   },
-};
-
 static unsigned int exynos4_apll_pms_table[CPUFREQ_LEVEL_END] = {
/* APLL FOUT L0: 1200MHz */
((150  16) | (3  8) | 1),
@@ -152,6 +130,26 @@ static unsigned int
exynos4_apll_pms_table[CPUFREQ_LEVEL_END] = {
((200  16) | (6  8) | 3),
 };
 
+/*
+ * ASV group voltage table
+ */
+static const unsigned int asv_voltage[CPUFREQ_LEVEL_END][8] = {
+   /*
+* SS, A1, A2, B1, B2, C1, C2, D
+* @1200 :
+* @1000 :
+* @800  :  ASV_VOLTAGE_TABLE
+* @500  :
+* @200  :
+*/
+   { 135, 135, 130, 1275000, 125, 1225000, 120,
1175000 },
+   { 130, 125, 120, 1175000, 115, 1125000, 110,
1075000 },
+   { 120, 115, 110, 1075000, 105, 1025000, 100,
975000 },
+   { 110, 105, 100, 975000, 975000, 95, 925000, 925000
},
+   { 105, 100, 975000, 95, 95, 925000, 925000, 925000
},
+
+};
+
 static int exynos4_verify_speed(struct cpufreq_policy *policy)
 {
return cpufreq_frequency_table_verify(policy, exynos4_freq_table);
@@ -312,7 +310,7 @@ static int exynos4_target(struct cpufreq_policy *policy,
}
 
/* get the voltage value */
-   arm_volt = exynos4_volt_table[index].arm_volt;
+   arm_volt = exynos4_volt_table[index];
 
cpufreq_notify_transition(freqs, CPUFREQ_PRECHANGE);
 
@@ -392,7 +390,7 @@ int exynos4_cpufreq_lock(unsigned int id,
cpufreq_notify_transition(freqs, CPUFREQ_PRECHANGE);
 
/* get the voltage value */
-   arm_volt = exynos4_volt_table[cpufreq_level].arm_volt;
+   arm_volt = exynos4_volt_table[cpufreq_level];
regulator_set_voltage(arm_regulator, arm_volt,
arm_volt);
 
@@ -518,11 +516,37 @@ static struct cpufreq_driver exynos4_driver = {
 #endif
 };
 
+static void __init set_volt_table(void)
+{
+   unsigned int tmp, i, asv_group = 0;
+
+   tmp = __raw_readl(S5P_INFORM2);
+
+   switch (tmp   (SUPPORT_FREQ_MASK  SUPPORT_FREQ_SHIFT)) {
+   case SUPPORT_1200MHZ:
+   asv_group = (tmp  0xF);
+   break;
+   case SUPPORT_1400MHZ:
+   case SUPPORT_1000MHZ:
+   default:
+   /* Not supported and assign typical ASV group */
+   asv_group = 2;
+   break;
+   }
+
+   printk(KERN_INFO DVFS: VDD_ARM Voltage table set with %d Group\n,
asv_group);
+
+   for (i = 0 ; i  CPUFREQ_LEVEL_END ; i++)
+   exynos4_volt_table[i] = asv_voltage[i][asv_group];
+}
+

Re: [PATCH 5/5] [CPUFREQ] EXYNOS4210: Add support ASV feature

2011-11-02 Thread MyungJoo Ham 
Hello,

On Wed, Nov 2, 2011 at 9:43 PM, Kukjin Kim kgene@samsung.com wrote:
[]
 +static void __init set_volt_table(void)
 +{
 +       unsigned int tmp, i, asv_group = 0;
 +
 +       tmp = __raw_readl(S5P_INFORM2);

As I've mentioned in the ASV patch thread, do we really need to use an
INFORM register simply to save the id of supported voltage ranges?

Why aren't we using an extern variable here? For example, extern int
asv_group_id; and define it at asv.h or somewhere else.

At reboot, we are going to init ASV driver and will get the ASV value
again; thus, we don't need to use such a preserving register anyway.
At suspend/resume, the value in RAM does not disappear and the IPL
does not care this value; thus, it is meaningless to use INFORM2 for
this value.


 +
 +       switch (tmp   (SUPPORT_FREQ_MASK  SUPPORT_FREQ_SHIFT)) {
 +       case SUPPORT_1200MHZ:
 +               asv_group = (tmp  0xF);
 +               break;
 +       case SUPPORT_1400MHZ:
 +       case SUPPORT_1000MHZ:
 +       default:
 +               /* Not supported and assign typical ASV group */
 +               asv_group = 2;
 +               break;
 +       }
 +
 +       printk(KERN_INFO DVFS: VDD_ARM Voltage table set with %d Group\n,
 asv_group);
 +
 +       for (i = 0 ; i  CPUFREQ_LEVEL_END ; i++)
 +               exynos4_volt_table[i] = asv_voltage[i][asv_group];
 +}
 +
  static int __init exynos4_cpufreq_init(void)
  {
        int i;
        unsigned int tmp;

 +       set_volt_table();
 +
        cpu_clk = clk_get(NULL, armclk);
        if (IS_ERR(cpu_clk))
                return PTR_ERR(cpu_clk);
 --
 1.7.1

 --
 To unsubscribe from this list: send the line unsubscribe linux-samsung-soc 
 in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html




-- 
MyungJoo Ham, Ph.D.
Mobile Software Platform Lab, DMC Business, Samsung Electronics
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html