[U-Boot] BeagleBoard B7 (OMAP3530) fails to boot with the current U-Boot master branch

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Sun, 5 Mar 2017 09:27:44 -0500
Tom Rini <trini@konsulko.com> wrote:

> On Sun, Mar 05, 2017 at 02:51:54PM +0200, Siarhei Siamashka wrote:
> > Hello All,
> > 
> > I have just unexpectedly found an old Beagleboard B7 in my closet and
> > tried to compile the current U-Boot master branch for it. Appears that
> > it is broken since almost 2 years ago:
> > 
> >    https://lists.denx.de/pipermail/u-boot/2015-July/220332.html
> >    https://lists.denx.de/pipermail/u-boot/2016-May/253777.html
> > 
> > And after trying to narrow it down, looks like the failure happens in
> > the prcm_init() function from 'arch/arm/mach-omap2/omap3/clock.c' when
> > it is compiled as Thumb2. At least the SPL can successfully boot after
> > the following change (GCC6 is needed for this pragma):  
> 
> Good job sorting this out.  I think it might be best to just disable
> Thumb on beagleboard.

Well, apparently this "disabling Thumb" plan is not moving anywhere
in practice. The current situation is less than perfect and downstream
(Robert Nelson) is already disabling Thumb since a while ago together
with CONFIG_SPL_EXT_SUPPORT. And the mainline U-Boot keeps providing
releases, which are unusable for OMAP3530 out-of-the box.

Regarding Thumb2 support in general. Yes, the old r1p3 revision of
Cortex A8 in OMAP3530 is affected by multiple Thumb errata. But I
checked the errata list and seems like we don't have to worry
about 657417 (A 32bit branch instruction that spans two 4K regions
can result in an incorrect instruction fetch or processor deadlock)
after all because the bug affects processors with 32KiB of L1
I-Cache, while OMAP3530 only has 16 KiB. The other Thumb errata
might also have no real effect on U-Boot, because some of them
involve the use of MMU and virtual addresses. In the end, we are
applying them for the sake of the Linux kernel and userland software.

Yes, I still don't like how errata workarounds are handled by U-Boot.
This stuff looks way too complicated and frameworkish. I mean that we
get the v7_arch_cp15_set_l2aux_ctrl() and v7_arch_cp15_set_acr()
functions implemented in the C code, and they also do some elaborate
runtime detection of the SoC variant. These functions are compiled as
Thumb code, so we get quite a bit of Thumb interworking activity
happening even before we have a chance to apply some relevant
errata workarounds. But no matter how ugly this stuff is, it does
not seem to cause real problems in practice.

So I suspect that the prcm_init() function fails not because of Thumb
as such, but because recompiling it as Thumb probably changes the
timings of the initialization in a subtle way. Also I don't quite
like the code constructs like this:

        /* M (CORE_DPLL_MULT): CM_CLKSEL1_PLL[16:26] */
        clrsetbits_le32(&prcm_base->clksel1_pll,
	                0x07FF0000, ptr->m << 16);

        /* N (CORE_DPLL_DIV): CM_CLKSEL1_PLL[8:14] */
        clrsetbits_le32(&prcm_base->clksel1_pll,
                        0x00007F00, ptr->n << 8);

Wouldn't it be more reasonable to change both M and N values as a
single atomic operation? Maybe it's a good idea to check the the
OMAP3530 errata list to see if there are any known quirks in the
DPLL setup sequence. But I would prefer to leave the further
investigation up to the TI people and local OMAP experts.

Either way, as a quick and dirty workaround, we currently can enable
ARM mode compilation for just a singe clock.c file instead of the
whole SPL. And this source file can be even split into parts if
we want better granularity.

I have sent some OMAP3530 patches to the list:
    https://lists.denx.de/pipermail/u-boot/2017-March/283074.html

-- 
Best regards,
Siarhei Siamashka