SAMA5D3 Xplained: SPL broken after panic added to /lib/time.c:94
Eugen.Hristev at microchip.com
Eugen.Hristev at microchip.com
Mon Apr 5 18:00:38 CEST 2021
On 4/5/21 6:42 PM, Manuel Luís Reis wrote:
> 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?
Okay, I am glad that you somehow found a solution.
I am not sure why the ddr2_init call to udelay fails. Maybe we should
initialize something (timer) before calling ddr2_init ?
Or timers are initialized just after ddr2 , in which case we need to
revert the order ?
To make a proper review, please use 'git send-email' command to send
your patch , not attach it to the email here.
If you modify the device tree, and some other code, you need to have two
separate patches.
each patch has to adhere to the subject rule : subsystem: component:
change ...
Check other commits in the area you are committing to see how they look like
>
> 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 <mailto: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
> <mailto: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
> <mailto: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
> <mailto: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 <mailto: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 <mailto: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,
> >>>
> >>>
> >>
>
More information about the U-Boot
mailing list