[U-Boot] [PATCH] arm, am33xx: move s_init to a common place

[Lokesh Vutla <lokeshvutla@ti.com>]

Hi Heiko,
On Monday 24 June 2013 09:46 PM, Heiko Schocher wrote:
> Hello Lokesh,
>
> Am 24.06.2013 06:01, schrieb Lokesh Vutla:
>> Hi Heiko,
>> On Thursday 20 June 2013 09:22 AM, Heiko Schocher wrote:
>>> Hello Tom,
>>>
>>> Am 14.06.2013 16:58, schrieb Tom Rini:
>>>> On Fri, Jun 14, 2013 at 07:59:26AM +0200, Heiko Schocher wrote:
>>>>> Hello Tom,
>>>>>
>>>>> Am 13.06.2013 17:53, schrieb Tom Rini:
>>>>>> On Thu, Jun 13, 2013 at 05:53:17AM +0200, Heiko Schocher wrote:
>>>>>>
>>>>>>> move s_init from every board code to a common place.
>>>>>>>
>>>>>>> Signed-off-by: Heiko Schocher <hs@denx.de>
>>>>>>> Cc: Tom Rini <trini@ti.com>
>>>>>>> Cc: Matt Porter <mporter@ti.com>
>>>>>>> Cc: Lars Poeschel <poeschel@lemonage.de>
>>>>>>> Cc: Tom Rini <trini@ti.com>
>>>>>>> Cc: Enric Balletbo i Serra <eballetbo@iseebcn.com>
>>>>>>>
>>>>>>> ---
>>>>>>> This patch is based on the following patches:
>>>>>>>
>>>>>>> - [U-Boot,v2] arm, am33xx: move rtc32k_enable() to common place
>>>>>>>     http://patchwork.ozlabs.org/patch/248908/
>>>>>>>
>>>>>>> - [U-Boot] arm, am33xx: move uart soft reset code to common place
>>>>>>>     http://patchwork.ozlabs.org/patch/248508/
>>>>>>
>>>>>> These two apply best to u-boot-ti, and with them this patch doesn't
>>>>>> apply cleanly.  Please sort that out.
>>>>>
>>>>> I based my patches on u-boot ... I look at this ..
>>>>>
>>>>>> The following adds moving ti814x_evm into the mix and I've sent Matt
>>>>>> some binaries to give a whirl to test on the board:
>>>>>>
>>>>> [...]
>>>>>>    /*
>>>>>>     * Basic board specific setup.  Pinmux has been handled already.
>>>>>>
>>>>>> Please fold into v2
>>>>>>
>>>>>> Signed-off-by: Tom Rini <trini@ti.com>
>>>>>
>>>>> Ok, thanks!
>>>>
>>>> There's a minor bug in what I posted, however.  ti814x needs timer_init
>>>> called _before_ pll_init() as setting the sata clocks (which are shared
>>>> with other periphrals that we do enable right now) needs udelay(50) to
>>>> settle as we go along.  That also needs to be commented in the code as I
>>>> had to think about it for a bit to recall exactly what was going on.
>>>
>>> Do you have an update here for me?
>> We can have a timer_init for am33xx boards also. It doesn't harm.
>> So keep timer_init in your common s_init
>
> Ok, fine.
>
>>>>> BTW:
>>>>> I just realized that I have on one of the three boards a problem,
>>>>> that in spl code calling the rtc32k_enable() crashes ... which
>>>>> votes against moving this to a common place ... I haveno real idea
>>>>> why ... did you heard from such a behaviour? Is there some am335x
>>>>> soc, which differs from the others?
>> On which board it is giving a problem?
>
> Not in mainline yet, posting soon ...
>
>> Did you make sure clocks for rtc are enabled?
>
> Yes.
I posted a clean up series for am33xx today
"[PATCH 0/4] ARM: AM33xx: Cleanup clocks and hwinit"
Can you please try with this series on your board.
Check the SPL log for any clock failure. If no error, then
ideally registers should be accessible. (Please make sure
register offsets that you are using are correct).

If not there is something really bad.

Thanks and regards,
Lokesh
>
> I have 3 boards with an am335x, two works with the
> rtc32k_enable() call without problems ... the third board
> hang when accessing rtc registers ... no idea why ...
> Code on all three boards is at this point identical, all
> use 24MHz ...
>
>> I am making a cleanup series for am33xx boards. If you don't mind can I
>> take this
>> patch as part of my series.
>
> I am fine with that ... but what do we do with my
> probem with rtc23k_enable?
>
>
>> Thanks and regards,
>> Lokesh
>>>>
>>>> You aren't using a different clock crystal rate than the reference
>>>> platforms, are you?  I know that's a problem that needs solving still.
>>>
>>> I am prospecting, whats going on here ... but have no real idea,
>>> why it is not possible to write this registers ... if writing this
>>> registers, cpu hang ...
>>>
>>> But I want to have a common function here ... maybe it is OK to make
>>> the rtc32k_enable() call configurable through a define?
>>>
>>> Saying "CONFIG_SPL_AM33XX_DO_NOT_ENABLE_RTC32K"
>>>
>>> and document in the u-boot README this define, and why it is
>>> necessary?
>
> Would this be acceptable?
>
> bye,
> Heiko
>