TI:OMAP: [PATCH 3/4] Support 720Mhz configuration for OMAP35xx

Khasim_Syed_Mohammed · January 8, 2010, 3:40pm

From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001

Nishanth_Menon · January 8, 2010, 7:52pm

From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 20:34:37 +0530
Subject: [PATCH] Support 720Mhz configuration for OMAP35xx

Adds a new API "twl4030_pmrecv_vsel_cfg" to select voltage and group
Adds support for 720Mhz in clock.c
Board file modified to use these new APIs and boot at 720Mhz

Could you split this into three patches please? easier to track
changes at a later date.

a) introducing generic voltage setting API for twl
b) introduce 720mhz
c) beagle support for C4 with 720Mhz.

Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 20 ++++++++++++++++++--
cpu/arm_cortexa8/omap3/clock.c | 21 +++++++++++++++++++++
drivers/power/twl4030.c | 24 +++++++++++++++---------
include/twl4030.h | 16 ++++++++++++++++
4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 0def5a6..7985ee9 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -122,9 +122,27 @@ int misc_init_r(void)
struct gpio *gpio5_base = (struct gpio *)OMAP34XX_GPIO5_BASE;
struct gpio *gpio6_base = (struct gpio *)OMAP34XX_GPIO6_BASE;

+ beagle_identify();
+

   twl4030\_power\_init\(\);
   twl4030\_led\_init\(\);
+ if (beagle_revision == REVISION_C4) {
+
+ /* Select TWL4030 VSEL to support 720Mhz */
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
+ VAUX2_VSEL_18,
+ TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
+ DEV_GRP_P1);
+
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VDD1_VSEL,
+ VDD1_VSEL_14,
+ TWL4030_PM_RECEIVER_VDD1_DEV_GRP,
+ DEV_GRP_P1);
+
+ prcm_config_720mhz();
+ }
+
/* Configure GPIOs to output */
writel(~(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1), &gpio6_base->oe);
writel(~(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
@@ -136,8 +154,6 @@ int misc_init_r(void)
writel(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
GPIO15 | GPIO14 | GPIO13 | GPIO12, &gpio5_base->setdataout);

- beagle_identify();
-
dieid_num_r();
   return 0;
diff --git a/cpu/arm_cortexa8/omap3/clock.c b/cpu/arm_cortexa8/omap3/clock.c
index 174c453..d67517a 100644
--- a/cpu/arm_cortexa8/omap3/clock.c
+++ b/cpu/arm_cortexa8/omap3/clock.c
@@ -402,3 +402,24 @@ void per_clocks_enable(void)
   sdelay\(1000\);
}
+
+/*
+ * Configure PRCM registers to get 720 Mhz
+ *
+ * NOTE: N value doesn't change, only M gets affected
+ */
+void prcm_config_720mhz(void)
+{
+ struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
+
+ /* Unlock MPU DPLL (slows things down, and needed later) */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOW_POWER_BYPASS);
+ wait_on_value(ST_MPU_CLK, 0, &prcm_base->idlest_pll_mpu, LDELAY);
+
+ /* Set M */
+ sr32(&prcm_base->clksel1_pll_mpu, 8, 11, 0x2D0);
+
+ /* lock mode */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOCK);
+ wait_on_value(ST_MPU_CLK, 1, &prcm_base->idlest_pll_mpu, LDELAY);

I know of dll lock infinite loops in some other system.. but that is a
different topic needing a different patch anyways.

+}
diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
index eb066cb..d68e515 100644
--- a/drivers/power/twl4030.c
+++ b/drivers/power/twl4030.c
@@ -59,16 +59,9 @@ void twl4030_power_reset_init(void)
}
}

-
/*
* Power Init
*/
-#define DEV_GRP_P1 0x20
-#define VAUX3_VSEL_28 0x03
-#define DEV_GRP_ALL 0xE0
-#define VPLL2_VSEL_18 0x05
-#define VDAC_VSEL_18 0x03
-
void twl4030_power_init(void)
{
unsigned char byte;
@@ -98,8 +91,6 @@ void twl4030_power_init(void)
TWL4030_PM_RECEIVER_VDAC_DEDICATED);
}

-#define VMMC1_VSEL_30 0x02
-
void twl4030_power_mmc_init(void)
{
unsigned char byte;
@@ -113,3 +104,18 @@ void twl4030_power_mmc_init(void)
twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, byte,
TWL4030_PM_RECEIVER_VMMC1_DEDICATED);
}
+
+/*
+ * Generic function to select Device Group and Voltage
+ */
+void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val,
+ u8 dev_grp, u8 dev_grp_sel)
+{
+ /* Select the Device Group */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp_sel,
+ dev_grp);
+
+ /* Select the Voltage */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_val,
+ vsel_reg);
+}

Assumption that i2c operations work 100% successfully! is'nt serial
bus subject to noise? and cant' i2c ops fail?

diff --git a/include/twl4030.h b/include/twl4030.h
index f260ecb..b96c96c 100644
--- a/include/twl4030.h
+++ b/include/twl4030.h
@@ -359,6 +359,22 @@
#define TWL4030_USB_PHY_DPLL_CLK (1 << 0)

/*
+ * Voltage Selection in PM Receiver Module
+ */
+#define VAUX2_VSEL_18 0x05
+#define VDD1_VSEL_14 0x40
+#define VAUX3_VSEL_28 0x03
+#define VPLL2_VSEL_18 0x05
+#define VDAC_VSEL_18 0x03
+#define VMMC1_VSEL_30 0x02
+
+/*
+ * Device Selection
+ */
+#define DEV_GRP_P1 0x20
+#define DEV_GRP_ALL 0xE0
+
+/*
* Convience functions to read and write from TWL4030
*
* chip_no is the i2c address, it must be one of the chip addresses
--
1.5.6.3

we should try review again after you have split the series up.
Regards,
Nishanth Menon

Khasim_Syed_Mohammed · January 9, 2010, 3:21am

From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 20:34:37 +0530
Subject: [PATCH] Support 720Mhz configuration for OMAP35xx

Adds a new API "twl4030_pmrecv_vsel_cfg" to select voltage and group
Adds support for 720Mhz in clock.c
Board file modified to use these new APIs and boot at 720Mhz

Could you split this into three patches please? easier to track
changes at a later date.

a) introducing generic voltage setting API for twl
b) introduce 720mhz
c) beagle support for C4 with 720Mhz.

Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 20 ++++++++++++++++++--
cpu/arm_cortexa8/omap3/clock.c | 21 +++++++++++++++++++++
drivers/power/twl4030.c | 24 +++++++++++++++---------
include/twl4030.h | 16 ++++++++++++++++
4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 0def5a6..7985ee9 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -122,9 +122,27 @@ int misc_init_r(void)
struct gpio *gpio5_base = (struct gpio *)OMAP34XX_GPIO5_BASE;
struct gpio *gpio6_base = (struct gpio *)OMAP34XX_GPIO6_BASE;

+ beagle_identify();
+
   twl4030\_power\_init\(\);
   twl4030\_led\_init\(\);
+ if (beagle_revision == REVISION_C4) {
+
+ /* Select TWL4030 VSEL to support 720Mhz */
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
+ VAUX2_VSEL_18,
+ TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
+ DEV_GRP_P1);
+
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VDD1_VSEL,
+ VDD1_VSEL_14,
+ TWL4030_PM_RECEIVER_VDD1_DEV_GRP,
+ DEV_GRP_P1);
+
+ prcm_config_720mhz();
+ }
+
/* Configure GPIOs to output */
writel(~(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1), &gpio6_base->oe);
writel(~(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
@@ -136,8 +154,6 @@ int misc_init_r(void)
writel(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
GPIO15 | GPIO14 | GPIO13 | GPIO12, &gpio5_base->setdataout);

- beagle_identify();
-
dieid_num_r();
   return 0;
diff --git a/cpu/arm_cortexa8/omap3/clock.c b/cpu/arm_cortexa8/omap3/clock.c
index 174c453..d67517a 100644
--- a/cpu/arm_cortexa8/omap3/clock.c
+++ b/cpu/arm_cortexa8/omap3/clock.c
@@ -402,3 +402,24 @@ void per_clocks_enable(void)
   sdelay\(1000\);
}
+
+/*
+ * Configure PRCM registers to get 720 Mhz
+ *
+ * NOTE: N value doesn't change, only M gets affected
+ */
+void prcm_config_720mhz(void)
+{
+ struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
+
+ /* Unlock MPU DPLL (slows things down, and needed later) */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOW_POWER_BYPASS);
+ wait_on_value(ST_MPU_CLK, 0, &prcm_base->idlest_pll_mpu, LDELAY);
+
+ /* Set M */
+ sr32(&prcm_base->clksel1_pll_mpu, 8, 11, 0x2D0);
+
+ /* lock mode */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOCK);
+ wait_on_value(ST_MPU_CLK, 1, &prcm_base->idlest_pll_mpu, LDELAY);
I know of dll lock infinite loops in some other system.. but that is a
different topic needing a different patch anyways.

Have you seen cpu/arm_cortexa8/omap3/syslib.c, wait_on_value does
handle such an instance, where your hardware is broken

We don't take care of failing systems, I would call them as hacks for
such devices.

+}
diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
index eb066cb..d68e515 100644
--- a/drivers/power/twl4030.c
+++ b/drivers/power/twl4030.c
@@ -59,16 +59,9 @@ void twl4030_power_reset_init(void)
}
}

-
/*
* Power Init
*/
-#define DEV_GRP_P1 0x20
-#define VAUX3_VSEL_28 0x03
-#define DEV_GRP_ALL 0xE0
-#define VPLL2_VSEL_18 0x05
-#define VDAC_VSEL_18 0x03
-
void twl4030_power_init(void)
{
unsigned char byte;
@@ -98,8 +91,6 @@ void twl4030_power_init(void)
TWL4030_PM_RECEIVER_VDAC_DEDICATED);
}

-#define VMMC1_VSEL_30 0x02
-
void twl4030_power_mmc_init(void)
{
unsigned char byte;
@@ -113,3 +104,18 @@ void twl4030_power_mmc_init(void)
twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, byte,
TWL4030_PM_RECEIVER_VMMC1_DEDICATED);
}
+
+/*
+ * Generic function to select Device Group and Voltage
+ */
+void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val,
+ u8 dev_grp, u8 dev_grp_sel)
+{
+ /* Select the Device Group */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp_sel,
+ dev_grp);
+
+ /* Select the Voltage */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_val,
+ vsel_reg);
+}

Assumption that i2c operations work 100% successfully! is'nt serial
bus subject to noise? and cant' i2c ops fail?

May be, such cases will be treated as system fail. Should be handled
separately for "broken platforms".

In beagleboard and EVMs atleast in last 4 revs we have never
encountered such problems.

diff --git a/include/twl4030.h b/include/twl4030.h
index f260ecb..b96c96c 100644
--- a/include/twl4030.h
+++ b/include/twl4030.h
@@ -359,6 +359,22 @@
#define TWL4030_USB_PHY_DPLL_CLK (1 << 0)

/*
+ * Voltage Selection in PM Receiver Module
+ */
+#define VAUX2_VSEL_18 0x05
+#define VDD1_VSEL_14 0x40
+#define VAUX3_VSEL_28 0x03
+#define VPLL2_VSEL_18 0x05
+#define VDAC_VSEL_18 0x03
+#define VMMC1_VSEL_30 0x02
+
+/*
+ * Device Selection
+ */
+#define DEV_GRP_P1 0x20
+#define DEV_GRP_ALL 0xE0
+
+/*
* Convience functions to read and write from TWL4030
*
* chip_no is the i2c address, it must be one of the chip addresses
--
1.5.6.3

we should try review again after you have split the series up.

I don't mind generating another patch series, but make sure you give
as much comments as possible with given patch set (this is fourth try
for the patch set), this level of discussion doesn't make sense for
the functionality that we are bringing in...

Regards,
Khasim

Nishanth_Menon · January 9, 2010, 2:57pm

Khasim Syed Mohammed said the following on 01/08/2010 09:21 PM:

From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 20:34:37 +0530
Subject: [PATCH] Support 720Mhz configuration for OMAP35xx

Adds a new API "twl4030_pmrecv_vsel_cfg" to select voltage and group
Adds support for 720Mhz in clock.c
Board file modified to use these new APIs and boot at 720Mhz


Could you split this into three patches please? easier to track
changes at a later date.

a) introducing generic voltage setting API for twl
b) introduce 720mhz
c) beagle support for C4 with 720Mhz.

Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 20 ++++++++++++++++++--
cpu/arm_cortexa8/omap3/clock.c | 21 +++++++++++++++++++++
drivers/power/twl4030.c | 24 +++++++++++++++---------
include/twl4030.h | 16 ++++++++++++++++
4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 0def5a6..7985ee9 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -122,9 +122,27 @@ int misc_init_r(void)
       struct gpio *gpio5_base = (struct gpio *)OMAP34XX_GPIO5_BASE;
       struct gpio *gpio6_base = (struct gpio *)OMAP34XX_GPIO6_BASE;

+ beagle_identify();
+
             twl4030_power_init();
       twl4030_led_init();

+ if (beagle_revision == REVISION_C4) {
+
+ /* Select TWL4030 VSEL to support 720Mhz */
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
+ VAUX2_VSEL_18,
+ TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
+ DEV_GRP_P1);
+
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VDD1_VSEL,
+ VDD1_VSEL_14,
+ TWL4030_PM_RECEIVER_VDD1_DEV_GRP,
+ DEV_GRP_P1);
+
+ prcm_config_720mhz();
+ }
+
       /* Configure GPIOs to output */
       writel(~(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1), &gpio6_base->oe);
       writel(~(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
@@ -136,8 +154,6 @@ int misc_init_r(void)
       writel(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
               GPIO15 | GPIO14 | GPIO13 | GPIO12, &gpio5_base->setdataout);

- beagle_identify();
-
       dieid_num_r();

       return 0;
diff --git a/cpu/arm_cortexa8/omap3/clock.c b/cpu/arm_cortexa8/omap3/clock.c
index 174c453..d67517a 100644
--- a/cpu/arm_cortexa8/omap3/clock.c
+++ b/cpu/arm_cortexa8/omap3/clock.c
@@ -402,3 +402,24 @@ void per_clocks_enable(void)

       sdelay(1000);
}
+
+/*
+ * Configure PRCM registers to get 720 Mhz
+ *
+ * NOTE: N value doesn't change, only M gets affected
+ */
+void prcm_config_720mhz(void)
+{
+ struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
+
+ /* Unlock MPU DPLL (slows things down, and needed later) */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOW_POWER_BYPASS);
+ wait_on_value(ST_MPU_CLK, 0, &prcm_base->idlest_pll_mpu, LDELAY);
+
+ /* Set M */
+ sr32(&prcm_base->clksel1_pll_mpu, 8, 11, 0x2D0);
+
+ /* lock mode */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOCK);
+ wait_on_value(ST_MPU_CLK, 1, &prcm_base->idlest_pll_mpu, LDELAY);


I know of dll lock infinite loops in some other system.. but that is a
different topic needing a different patch anyways.


Have you seen cpu/arm_cortexa8/omap3/syslib.c, wait_on_value does
handle such an instance, where your hardware is broken

I was wondering about the timeout LDELAY inwhich case wait_on_value returns 0?

We don't take care of failing systems, I would call them as hacks for
such devices.

this is a sad statement I call them error recovery unfortunately.

+}
diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
index eb066cb..d68e515 100644
--- a/drivers/power/twl4030.c
+++ b/drivers/power/twl4030.c
@@ -59,16 +59,9 @@ void twl4030_power_reset_init(void)
       }
}

-
/*
* Power Init
*/
-#define DEV_GRP_P1 0x20
-#define VAUX3_VSEL_28 0x03
-#define DEV_GRP_ALL 0xE0
-#define VPLL2_VSEL_18 0x05
-#define VDAC_VSEL_18 0x03
-
void twl4030_power_init(void)
{
       unsigned char byte;
@@ -98,8 +91,6 @@ void twl4030_power_init(void)
                            TWL4030_PM_RECEIVER_VDAC_DEDICATED);
}

-#define VMMC1_VSEL_30 0x02
-
void twl4030_power_mmc_init(void)
{
       unsigned char byte;
@@ -113,3 +104,18 @@ void twl4030_power_mmc_init(void)
       twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, byte,
                            TWL4030_PM_RECEIVER_VMMC1_DEDICATED);
}
+
+/*
+ * Generic function to select Device Group and Voltage
+ */
+void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val,
+ u8 dev_grp, u8 dev_grp_sel)
+{
+ /* Select the Device Group */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp_sel,
+ dev_grp);
+
+ /* Select the Voltage */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_val,
+ vsel_reg);
+}


Assumption that i2c operations work 100% successfully! is'nt serial
bus subject to noise? and cant' i2c ops fail?


May be, such cases will be treated as system fail. Should be handled
separately for "broken platforms".

In beagleboard and EVMs atleast in last 4 revs we have never
encountered such problems.

I mean never seen an i2c read/write failure? I have seen at least a couple unfortunately when one of the SDP3430's had some one solder a wrong pull up resistor and another where a pull up resistor was torn off by accident.

these are broken platforms ofcourse :). sigh, seeing that the rest of the file is messed up in this regards, I leave it for the community to further comment on this.

diff --git a/include/twl4030.h b/include/twl4030.h
index f260ecb..b96c96c 100644
--- a/include/twl4030.h
+++ b/include/twl4030.h
@@ -359,6 +359,22 @@
#define TWL4030_USB_PHY_DPLL_CLK (1 << 0)

/*
+ * Voltage Selection in PM Receiver Module
+ */
+#define VAUX2_VSEL_18 0x05
+#define VDD1_VSEL_14 0x40
+#define VAUX3_VSEL_28 0x03
+#define VPLL2_VSEL_18 0x05
+#define VDAC_VSEL_18 0x03
+#define VMMC1_VSEL_30 0x02
+
+/*
+ * Device Selection
+ */
+#define DEV_GRP_P1 0x20
+#define DEV_GRP_ALL 0xE0
+
+/*
* Convience functions to read and write from TWL4030
*
* chip_no is the i2c address, it must be one of the chip addresses
--
1.5.6.3


we should try review again after you have split the series up.

I don't mind generating another patch series, but make sure you give
as much comments as possible with given patch set (this is fourth try
for the patch set), this level of discussion doesn't make sense for

Hmm.. Please do not get frustrated, at least I am trying to make the code generic and flexible enough for usable in other platforms. your work is good, and we are going in the right direction, we will have a solution soon I hope.Could I request a [V4] inside your patch subject to keep the reviewer aware of this as well as in the --- diffstat section add link to previous discussions for us to refer back on?

Khasim_Syed_Mohammed · January 10, 2010, 3:02am

Khasim Syed Mohammed said the following on 01/08/2010 09:21 PM:
From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 20:34:37 +0530
Subject: [PATCH] Support 720Mhz configuration for OMAP35xx

Adds a new API "twl4030_pmrecv_vsel_cfg" to select voltage and group
Adds support for 720Mhz in clock.c
Board file modified to use these new APIs and boot at 720Mhz

Could you split this into three patches please? easier to track
changes at a later date.

a) introducing generic voltage setting API for twl
b) introduce 720mhz
c) beagle support for C4 with 720Mhz.
Signed-off-by: Syed Mohammed Khasim <khasim@ti.com>
---
board/ti/beagle/beagle.c | 20 ++++++++++++++++++--
cpu/arm_cortexa8/omap3/clock.c | 21 +++++++++++++++++++++
drivers/power/twl4030.c | 24 +++++++++++++++---------
include/twl4030.h | 16 ++++++++++++++++
4 files changed, 70 insertions(+), 11 deletions(-)

diff --git a/board/ti/beagle/beagle.c b/board/ti/beagle/beagle.c
index 0def5a6..7985ee9 100644
--- a/board/ti/beagle/beagle.c
+++ b/board/ti/beagle/beagle.c
@@ -122,9 +122,27 @@ int misc_init_r(void)
struct gpio *gpio5_base = (struct gpio *)OMAP34XX_GPIO5_BASE;
struct gpio *gpio6_base = (struct gpio *)OMAP34XX_GPIO6_BASE;

+ beagle_identify();
+
twl4030_power_init();
twl4030_led_init();

+ if (beagle_revision == REVISION_C4) {
+
+ /* Select TWL4030 VSEL to support 720Mhz */
+
twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VAUX2_DEDICATED,
+ VAUX2_VSEL_18,
+
TWL4030_PM_RECEIVER_VAUX2_DEV_GRP,
+ DEV_GRP_P1);
+
+ twl4030_pmrecv_vsel_cfg(TWL4030_PM_RECEIVER_VDD1_VSEL,
+ VDD1_VSEL_14,
+
TWL4030_PM_RECEIVER_VDD1_DEV_GRP,
+ DEV_GRP_P1);
+
+ prcm_config_720mhz();
+ }
+
/* Configure GPIOs to output */
writel(~(GPIO23 | GPIO10 | GPIO8 | GPIO2 | GPIO1),
&gpio6_base->oe);
writel(~(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
@@ -136,8 +154,6 @@ int misc_init_r(void)
writel(GPIO31 | GPIO30 | GPIO29 | GPIO28 | GPIO22 | GPIO21 |
GPIO15 | GPIO14 | GPIO13 | GPIO12,
&gpio5_base->setdataout);

- beagle_identify();
-
dieid_num_r();
  return 0;
diff --git a/cpu/arm_cortexa8/omap3/clock.c
b/cpu/arm_cortexa8/omap3/clock.c
index 174c453..d67517a 100644
--- a/cpu/arm_cortexa8/omap3/clock.c
+++ b/cpu/arm_cortexa8/omap3/clock.c
@@ -402,3 +402,24 @@ void per_clocks_enable(void)
  sdelay\(1000\);
}
+
+/*
+ * Configure PRCM registers to get 720 Mhz
+ *
+ * NOTE: N value doesn't change, only M gets affected
+ */
+void prcm_config_720mhz(void)
+{
+ struct prcm *prcm_base = (struct prcm *)PRCM_BASE;
+
+ /* Unlock MPU DPLL (slows things down, and needed later) */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOW_POWER_BYPASS);
+ wait_on_value(ST_MPU_CLK, 0, &prcm_base->idlest_pll_mpu,
LDELAY);
+
+ /* Set M */
+ sr32(&prcm_base->clksel1_pll_mpu, 8, 11, 0x2D0);
+
+ /* lock mode */
+ sr32(&prcm_base->clken_pll_mpu, 0, 3, PLL_LOCK);
+ wait_on_value(ST_MPU_CLK, 1, &prcm_base->idlest_pll_mpu,
LDELAY);
I know of dll lock infinite loops in some other system.. but that is a
different topic needing a different patch anyways.
Have you seen cpu/arm_cortexa8/omap3/syslib.c, wait_on_value does
handle such an instance, where your hardware is broken
I was wondering about the timeout LDELAY inwhich case wait_on_value returns
0?

We don't take care of failing systems, I would call them as hacks for
such devices.

this is a sad statement I call them error recovery unfortunately.

+}
diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
index eb066cb..d68e515 100644
--- a/drivers/power/twl4030.c
+++ b/drivers/power/twl4030.c
@@ -59,16 +59,9 @@ void twl4030_power_reset_init(void)
}
}

-
/*
* Power Init
*/
-#define DEV_GRP_P1 0x20
-#define VAUX3_VSEL_28 0x03
-#define DEV_GRP_ALL 0xE0
-#define VPLL2_VSEL_18 0x05
-#define VDAC_VSEL_18 0x03
-
void twl4030_power_init(void)
{
unsigned char byte;
@@ -98,8 +91,6 @@ void twl4030_power_init(void)
TWL4030_PM_RECEIVER_VDAC_DEDICATED);
}

-#define VMMC1_VSEL_30 0x02
-
void twl4030_power_mmc_init(void)
{
unsigned char byte;
@@ -113,3 +104,18 @@ void twl4030_power_mmc_init(void)
twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, byte,
TWL4030_PM_RECEIVER_VMMC1_DEDICATED);
}
+
+/*
+ * Generic function to select Device Group and Voltage
+ */
+void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val,
+ u8 dev_grp, u8 dev_grp_sel)
+{
+ /* Select the Device Group */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp_sel,
+ dev_grp);
+
+ /* Select the Voltage */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_val,
+ vsel_reg);
+}

Assumption that i2c operations work 100% successfully! is'nt serial
bus subject to noise? and cant' i2c ops fail?

May be, such cases will be treated as system fail. Should be handled
separately for "broken platforms".

In beagleboard and EVMs atleast in last 4 revs we have never
encountered such problems.

I mean never seen an i2c read/write failure? I have seen at least a couple
unfortunately when one of the SDP3430's had some one solder a wrong pull up
resistor and another where a pull up resistor was torn off by accident.

these are broken platforms ofcourse :).

Yeah,

sigh, seeing that the rest of the
file is messed up in this regards, I leave it for the community to further
comment on this.

diff --git a/include/twl4030.h b/include/twl4030.h
index f260ecb..b96c96c 100644
--- a/include/twl4030.h
+++ b/include/twl4030.h
@@ -359,6 +359,22 @@
#define TWL4030_USB_PHY_DPLL_CLK (1 << 0)

/*
+ * Voltage Selection in PM Receiver Module
+ */
+#define VAUX2_VSEL_18 0x05
+#define VDD1_VSEL_14 0x40
+#define VAUX3_VSEL_28 0x03
+#define VPLL2_VSEL_18 0x05
+#define VDAC_VSEL_18 0x03
+#define VMMC1_VSEL_30 0x02
+

Did you mean these lines ? When I apply the patch I don't see these
kind of lines, they are properly arranged in TABs. I have also checked
every patch with checkpatch.pl (from Linux). There are no such
alignment issues.

+/*
+ * Device Selection
+ */
+#define DEV_GRP_P1 0x20
+#define DEV_GRP_ALL 0xE0
+
+/*
* Convience functions to read and write from TWL4030
*
* chip_no is the i2c address, it must be one of the chip addresses
--
1.5.6.3

we should try review again after you have split the series up.

I don't mind generating another patch series, but make sure you give
as much comments as possible with given patch set (this is fourth try
for the patch set), this level of discussion doesn't make sense for

Hmm.. Please do not get frustrated, at least I am trying to make the code
generic and flexible enough for usable in other platforms.

oh, I am not frustrated. I can correct the changes any number of
times. They are not that big. My only concern was it was not worth as
the functionality getting into u-boot was very small.

your work is
good, and we are going in the right direction, we will have a solution soon
I hope.Could I request a [V4] inside your patch subject to keep the reviewer
aware of this as well as in the --- diffstat section add link to previous
discussions for us to refer back on?

Let me see how I can do that.

Nishanth_Menon · January 10, 2010, 3:44pm

Khasim Syed Mohammed said the following on 01/09/2010 09:02 PM:

Khasim Syed Mohammed said the following on 01/08/2010 09:21 PM:

From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001
From: Syed Mohammed Khasim <khasim@ti.com>
Date: Fri, 8 Jan 2010 20:34:37 +0530
Subject: [PATCH] Support 720Mhz configuration for OMAP35xx

[...]

diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
index eb066cb..d68e515 100644
--- a/drivers/power/twl4030.c
+++ b/drivers/power/twl4030.c
@@ -59,16 +59,9 @@ void twl4030_power_reset_init(void)
      }
}

-
/*
* Power Init
*/
-#define DEV_GRP_P1 0x20
-#define VAUX3_VSEL_28 0x03
-#define DEV_GRP_ALL 0xE0
-#define VPLL2_VSEL_18 0x05
-#define VDAC_VSEL_18 0x03
-
void twl4030_power_init(void)
{
      unsigned char byte;
@@ -98,8 +91,6 @@ void twl4030_power_init(void)
                           TWL4030_PM_RECEIVER_VDAC_DEDICATED);
}

-#define VMMC1_VSEL_30 0x02
-
void twl4030_power_mmc_init(void)
{
      unsigned char byte;
@@ -113,3 +104,18 @@ void twl4030_power_mmc_init(void)
      twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, byte,
                           TWL4030_PM_RECEIVER_VMMC1_DEDICATED);
}
+
+/*
+ * Generic function to select Device Group and Voltage
+ */
+void twl4030_pmrecv_vsel_cfg(u8 vsel_reg, u8 vsel_val,
+ u8 dev_grp, u8 dev_grp_sel)
+{
+ /* Select the Device Group */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, dev_grp_sel,
+ dev_grp);
+
+ /* Select the Voltage */
+ twl4030_i2c_write_u8(TWL4030_CHIP_PM_RECEIVER, vsel_val,
+ vsel_reg);
+}

Assumption that i2c operations work 100% successfully! is'nt serial
bus subject to noise? and cant' i2c ops fail?

May be, such cases will be treated as system fail. Should be handled
separately for "broken platforms".

In beagleboard and EVMs atleast in last 4 revs we have never
encountered such problems.

I mean never seen an i2c read/write failure? I have seen at least a couple
unfortunately when one of the SDP3430's had some one solder a wrong pull up
resistor and another where a pull up resistor was torn off by accident.

these are broken platforms ofcourse :).

Yeah,

sigh, seeing that the rest of the
file is messed up in this regards, I leave it for the community to further
comment on this.

[...]

+#define VAUX3_VSEL_28 0x03
+#define VPLL2_VSEL_18 0x05
+#define VDAC_VSEL_18 0x03
+#define VMMC1_VSEL_30 0x02
+

Did you mean these lines ? When I apply the patch I don't see these
kind of lines, they are properly arranged in TABs. I have also checked
every patch with checkpatch.pl (from Linux). There are no such
alignment issues.

no, I meant usage of twl4030_i2c_write_u8() without error check throughout the file.

Regards,
Nishanth Menon

Khasim_Syed_Mohammed · January 10, 2010, 5:21pm

Yeah, cleaning up the entire file might have to be a separate effort.
Nothing related to this patch.

I am wondering what one would do if I2C fails, an error message ???
Any way it means hardware is broken. Which will go through debugging
any way.

Regards,
Khasim

eballetbo · January 14, 2010, 11:42am

Hello all,

I've tested your patches with an IGEP v2 at 720MHz and an IGEP v2 at
600MHz and looks great.

I see in your beagleboard file you check for C4 revision and then set
up CPU to 720MHz. Is ok, but I think should be better create a more
generic function and read the PROD_ID register. If you see at

spruf98d.pdf (page 171)
1.5.2.1 720MHz Device Identification
      Read device ID to be able to identify if this device is 720MHz
capable or not. There is SKU ID that is blown in efuse and that it
readable from following the memory location.
      Register 0x4830A20C [ProdID.SKUID] [0:3]:
            • 0x0 for normal 600/430MHz device.
            • 0x8 for 720/520MHz device.

And this is a function proposal

--- snip ---
#define PRODUCT_ID_SKUID 0x4830A20C
#define CPU_35XX_PID_MASK 0x0000000F
#define CPU_35XX_600MHZ_DEV 0x0
#define CPU_35XX_720MHZ_DEV 0x8
static u32 get_prod_id(void)
{
  u32 p;
  /* get production ID */
  p = __raw_readl(PRODUCT_ID_SKUID);
  return (p & CPU_35XX_PID_MASK);
}
--- snip ---

Best regards,
Enric