[U-Boot] [beagleboard] TI:OMAP: [PATCH 3/4] Support 720Mhz configuration for OMAP35xx

Khasim Syed Mohammed khasim at beagleboard.org
Sun Jan 10 04:02:24 CET 2010


On Sat, Jan 9, 2010 at 8:27 PM, Nishanth Menon <menon.nishanth at gmail.com> wrote:
> 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 :).
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.

>> the functionality that we are bringing in...
>>
>> Regards,
>> Khasim
>>
>>
>
>


More information about the U-Boot mailing list