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

[Allen Martin]

On Tue, Oct 09, 2012 at 03:15:40PM -0700, Stephen Warren wrote:
> 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?

The clear_bss() in arm720t/start.S is surrounded by "#ifndef
CONFIG_SPL_BUILD".  It was probably removed long ago, I just never
noticed because nothing in SPL was using any globals.

Moving to common spl.c definately seems like the right thing to do to
avoid this type of thing.  I'll work on it, but it probably won't be
until next week, is that ok?

-Allen

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------