SAMA5D3 Xplained: SPL broken after panic added to /lib/time.c:94

Manuel Luís Reis mluis.reis at gmail.com
Mon Apr 5 17:42:14 CEST 2021


Hi Eugen,

> Does the node have the property
>
> u-boot,dm-pre-reloc;
>
> Maybe try to add this and see if it changes anything ?

Thanks. Indeed it didn't have those properties. I have added them and also
had to remove the udelay call in ddr2_init for the fix to work.
The udelay call in ddr2_init was also failing (maybe too early for a timer
call!?), but it seems to be redundant as it sits between two nops,
and is working well as far as I can tell. If extra time is needed in that
place, maybe extra NOPs could be considered instead.

I am sending a patch for the fix. Could you review it please?

Let me know if you'd like me to change anything.

Thanks again,
--------------------------------------------------------------------------------------------------------


>From cff45e15a96facc702c852a9beff27907b2c92e5 Mon Sep 17 00:00:00 2001
From: Manuel Reis <mluis.reis at gmail.com>
Date: Mon, 5 Apr 2021 16:31:38 +0100
Subject: [PATCH] add u-boot properties to pit timer

timer node was not detected when udelay was used.
udelay call removed from ddr2_init as: it sits between two NOP;

Signed-off-by: Manuel Reis <mluis.reis at gmail.com>
---
 arch/arm/dts/sama5d3.dtsi   | 1 +
 arch/arm/mach-at91/mpddrc.c | 3 ---
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm/dts/sama5d3.dtsi b/arch/arm/dts/sama5d3.dtsi
index 6ed218eaad..3cd30d574d 100644
--- a/arch/arm/dts/sama5d3.dtsi
+++ b/arch/arm/dts/sama5d3.dtsi
@@ -1316,6 +1316,7 @@
                        };

                        pit: timer at fffffe30 {
+                               u-boot,dm-pre-reloc;
                                compatible = "atmel,at91sam9260-pit";
                                reg = <0xfffffe30 0xf>;
                                interrupts = <3 IRQ_TYPE_LEVEL_HIGH 5>;
diff --git a/arch/arm/mach-at91/mpddrc.c b/arch/arm/mach-at91/mpddrc.c
index 5422c05456..2e52595e30 100644
--- a/arch/arm/mach-at91/mpddrc.c
+++ b/arch/arm/mach-at91/mpddrc.c
@@ -66,9 +66,6 @@ int ddr2_init(const unsigned int base,
        /* Issue a NOP command */
        atmel_mpddr_op(mpddr, ATMEL_MPDDRC_MR_MODE_NOP_CMD, ram_address);

-       /* A 200 us is provided to precede any signal toggle */
-       udelay(200);
-
        /* Issue a NOP command */
        atmel_mpddr_op(mpddr, ATMEL_MPDDRC_MR_MODE_NOP_CMD, ram_address);

-- 
2.27.0







On Mon, 5 Apr 2021 at 14:58, <Eugen.Hristev at microchip.com> wrote:

> On 4/5/21 3:47 PM, Manuel Luís Reis wrote:
> > Hi Eugen,
> >
> >>   As Sean Anderson pointed out : the call to timer must have not worked
> at
> >> all before this change that now breaks things.
> >> Have you tried removing the call to the timer from this mem_init, to see
> >> if this makes the board boot correctly ?
> >>
> >> In mem_init I guess there are multiple calls to this timer function.
> >
> > Indeed I have. After my previous emails trying to find if there was
> > anything messing with the timer, I went the other way and dug further
> > into mem_init and ddr2_init. In ddr2_init there is a call to udelay,
> > udelay(200) in arch/arm/mach-at91/mpddrc.c line 70.
> >
> > I have commented this out in v2020.07 with no visible consequences. In
> > v2021.01, SPL did move a bit further, BUT when dealing with the sd
> > card, within the mmc driver, the errors continued. There are quite a
> > few udelay() calls within the mmc drivers, so I felt I shouldn't be
> > changing that.
> >
> > Instead, I went to check the udelay -> get_ticks -> dm_timer_init
> > function. And it seems the problem is exactly here. The function tries
> > to find a timer in the device tree, if I understand correctly, but it
> > doesn't find one, returning the -ENODEV  error, on the following
> > function: ret = uclass_first_device_err(UCLASS_TIMER, &dev);
> >
> > I checked the device tree and the pit timer is there. The UCLASS
> > driver looks good to me as per documentation. I have also tried adding
> > the "chosen" "tick-timer" parameter in the device tree pointing to the
> > pit timer so it would be recognized in the PLATDATA section of this
> > function, but still it wasn't recognized. (I might not have done this
> > correctly though it was compiled successfully)
> >
> > So there seems to be an issue of some kind of misconfiguration with
> > the declaration of the timer in the device tree that it doesn't get
> > read back.
>
>
> Does the node have the property
>
> u-boot,dm-pre-reloc;
>
> Maybe try to add this and see if it changes anything ?
>
> >
> > Any ideas?
> >
> > Thanks
> >
> >
> >
> >
> >
> >
> >
> >
> > On Mon, 5 Apr 2021 at 13:44, <Eugen.Hristev at microchip.com> wrote:
> >>
> >> On 4/4/21 7:25 PM, Manuel Reis wrote:
> >>> Hi again,
> >>>
> >>> I guess I misunderstood things a bit. Apologies for that.
> >>> Nevertheless, timer_init in board_init_f is pointing to the weak
> >>> timer_init in /lib/time.c, which is empty.
> >>>
> >>> Is there a way we can force start the pit timer?
> >>> Any pointer would be very helpful. Let me know if you'd like me to do
> >>> something in particular.
> >>>
> >>> Thanks
> >>>
> >>>
> >>>
> >>> On sex, 2 abr, 2021 at 18:15, Manuel Luís Reis <mluis.reis at gmail.com>
> >>> wrote:
> >>>> FYI:
> >>>>
> >>>> the output from serial port:
> >>>> <debug_uart>
> >>>> board_init_f spl_atmel.c 130 mem_init 182 ddr2_init mpddr.c 66udelay
> >>>> lib time.c 196__udelay lib time.c 177Could not initialize timer (err
> >>>> -11)
> >>>>
> >>>> udelay lib time.c 196__udelay lib time.c 177Could not initialize
> >>>> timer (err -11)
> >>>>
> >>>> udelay lib time.c 196__udelay lib time.c 177Could not initialize
> >>>> timer (err -11)
> >>>> ...
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On Fri, 2 Apr 2021 at 18:12, Manuel Luís Reis <mluis.reis at gmail.com>
> >>>> wrote:
> >>>>>
> >>>>>   > As it seems from the dump of dm_dump_all() the atmel_pit_timer is
> >>>>> not
> >>>>>   > probed. I did a bit of debug and the dm_timer_init() ->
> >>>>>   > uclass_first_device() -> uclass_find_first_device() found zero
> >>>>> timers
> >>>>>   > registered for UCLASS_TIMER. The driver is compiled.  Also
> >>>>> checked that
> >>>>>   > atmel_pit_timer probe function is not called at all. The question
> >>>>> should be
> >>>>>   > why it is not probed at all?
> >>>>>
> >>>>>   Hi,
> >>>>>
> >>>>>   So, I put objdump and puts to some good use and could backtrace the
> >>>>>   initial error to a udelay call in ddr2_init function on mpddr.c
> >>>>> file.
> >>>>>   This function is called from mem_init on
> >>>>>   sama5d3_xplained/sama5d3_xplained.c, which in turn is called from
> >>>>>   board_init_f on spl_atmel.c.
> >>
> >> Hi Manuel,
> >>
> >> As Sean Anderson pointed out : the call to timer must have not worked at
> >> all before this change that now breaks things.
> >> Have you tried removing the call to the timer from this mem_init, to see
> >> if this makes the board boot correctly ?
> >>
> >> In mem_init I guess there are multiple calls to this timer function.
> >>
> >>>>>   I couldn't, however, find which timer_init function is being called
> >>>>>   here. I still have a few to try, but disassembly gives me a pretty
> >>>>>   empty function:
> >>>>>
> >>>>>   00303690 <timer_init>:
> >>>>>     303690:       e3a00000        mov     r0, #0
> >>>>>     303694:       e12fff1e        bx      lr
> >>>>>
> >>>>>   This work didn't give me many answers, but I got a couple of newbie
> >>>>> questions:
> >>>>>
> >>>>>   Why is it calling board_init_f from spl_atmel.c and not spl_at91.c?
> >>>>>   Isn't the latter the appropriate architecture for this board?
> >>>>>   Do you know which timer_init function it is getting here?  Could
> >>>>> this
> >>>>>   be the reason timer isn't getting probed? >>>>>
> >>>>>   Cheers,
> >>>
> >>>
> >>
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-add-u-boot-properties-to-pit-timer.patch
Type: text/x-patch
Size: 1375 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210405/465a8d62/attachment.bin>


More information about the U-Boot mailing list