[U-Boot] [beagleboard] TI:OMAP: [PATCH 3/4] Support 720Mhz configuration for OMAP35xx
Nishanth Menon
menon.nishanth at gmail.com
Sat Jan 9 15:57:59 CET 2010
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 at gmail.com> wrote:
>
>> On Fri, Jan 8, 2010 at 9:40 AM, Khasim Syed Mohammed
>> <khasim at beagleboard.org> wrote:
>>
>>> From bba669562fa208d12f4c7cd8188446e8576cd6ee Mon Sep 17 00:00:00 2001
>>> From: Syed Mohammed Khasim <khasim at 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 at 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
>
>
More information about the U-Boot
mailing list