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