[U-Boot] [PATCH 67/72] serial: Unconditionally enable CONFIG_SERIAL_MULTI

[Stephen Warren]

On 10/09/2012 04:13 PM, Tom Rini wrote:
> On 10/09/12 15:09, Stephen Warren wrote:
>> On 10/09/2012 03:38 PM, Tom Rini wrote:
>>> On Tue, Oct 09, 2012 at 02:33:51PM -0600, Stephen Warren wrote:
>>>> On 09/29/2012 03:53 PM, Marek Vasut wrote:
>>>>> Enable CONFIG_SERIAL_MULTI for all builds of U-Boot. That 
>>>>> includes both SPL builds and non-SPL builds, everything. To 
>>>>> avoid poluting this patch with removal of ifdef-endif 
>>>>> constructions containing CONFIG_SERIAL_MULTI, the 
>>>>> CONFIG_SERIAL_MULTI is temporarily added into CPPFLAGS in 
>>>>> config.mk . This will be again removed in following patch.
>>>>
>>>> Marek,
>>>>
>>>> This patch (at least, the version of it checked into
>>>> u-boot/next) breaks U-Boot on Tegra.
>>>>
>>>> I believe the reason is because nothing ever calls 
>>>> serial_initialize() in the SPL on Tegra. If I edit 
>>>> arch/arm/cpu/arm720t/tegra20/spl.c function 
>>>> preloader_console_init() to call serial_initialize() right
>>>> before it calls serial_init(), then everything works fine.
>>>> Without this, drivers/serial/serial.c is never initialised (not
>>>> even BSS cleared!) and so it's left set to some bogus value,
>>>> and so get_current() doesn't replace it with
>>>> default_serial_console(), so an invalid pointer is accessed,
>>>> causing a hang or crash.
>>>>
>>>> I'm not sure quite what the correct solution is here. Is
>>>> Tegra's custom spl.c doing too much; stuff that should come
>>>> from the common spl.c is cut/paste here? Or, is adding the call
>>>> to serial_initialize() the correct fix? Or, is the root-cause
>>>> of the problem that BSS (serial_current) isn't being cleared to
>>>> 0?
>>>
>>> Tegra SPL needs to either be updated ala common/spl/ 
>>> (preloader_console_init() calls serial_init()) or switched over
>>> to it.
> 
>> Hmm, well it's already calling serial_init() just not 
>> serial_initialize(). Perhaps the issue is the following code
>> missing from Tegra's SPL:
> 
>> /* Clear the BSS. */ memset(__bss_start, 0, __bss_end__ -
>> __bss_start);
> 
>> Anyway, I'll let Allen take a look at it, since he's most familiar 
>> with Tegra SPL.
> 
> Yes, if SPL wasn't previously clearing the BSS, this would be a
> problem now.

It was at least at some point in time; Tegra's spl.c does:

>         /*
>          * We call relocate_code() with relocation target same as the
>          * CONFIG_SYS_SPL_TEXT_BASE. This will result in relocation getting
>          * skipped. Instead, only .bss initialization will happen. That's
>          * all we need
>          */
>         debug(">>board_init_f()\n");
>         relocate_code(CONFIG_SPL_STACK, &gdata, CONFIG_SPL_TEXT_BASE);

Perhaps that call doesn't clear BSS any more?