Khasim Syed Mohammed said the following on 01/08/2010 09:21 PM:
On Sat, Jan 9, 2010 at 1:22 AM, Nishanth Menon menon.nishanth@gmail.com wrote:
On Fri, Jan 8, 2010 at 9:40 AM, Khasim Syed Mohammed khasim@beagleboard.org wrote:
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?
the functionality that we are bringing in...
Regards, Khasim