[U-Boot] [PATCH] arm: socfpga: make debug uart work on socfpga_gen5

[Simon Goldschmidt]

On Mon, Jan 7, 2019 at 11:58 PM Marek Vasut <marex at denx.de> wrote:
>
> On 1/7/19 10:01 PM, Simon Goldschmidt wrote:
> > Am 07.01.2019 um 21:47 schrieb Marek Vasut:
> >> On 1/7/19 9:33 PM, Simon Goldschmidt wrote:
> >>> Am 07.01.2019 um 21:25 schrieb Marek Vasut:
> >>>> On 1/7/19 9:24 PM, Simon Goldschmidt wrote:
> >>>>> Am 07.01.2019 um 21:19 schrieb Marek Vasut:
> >>>>>> On 1/7/19 8:36 PM, Simon Goldschmidt wrote:
> >>>>>>> When debug UART is enabled on socfpga_gen5, the debug uart driver
> >>>>>>> hangs
> >>>>>>> in an endless loop because 'socfpga_bridges_reset' calls printf
> >>>>>>> before
> >>>>>>> the debug UART is initialized.
> >>>>>>>
> >>>>>>> After the generic fix for this in the UART driver did not work
> >>>>>>> due to
> >>>>>>> portability issues, let's just drop this printf statement when
> >>>>>>> called
> >>>>>>> from SPL with debug UART enabled.
> >>>>>>>
> >>>>>>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com>
> >>>>>>
> >>>>>> Can we have an un-portable fix which at least works on SoCFPGA ? :)
> >>>>>
> >>>>> This one worked on socfpga but broke rockchip:
> >>>>>
> >>>>> https://patchwork.ozlabs.org/patch/992553/
> >>>>>
> >>>>> However, the message below wasn't shown either with that patch
> >>>>> applied.
> >>>>> The code just runs too early to enable the UART.
> >>>>>
> >>>>> Do you want to keep the message (although I failed to see in which
> >>>>> situation it can be printed) or do you just dislike the #ifdef thing?
> >>>>
> >>>> I'd like to keep the error message if possible. Is it possible ?
> >>>
> >>> I have *never* seen this message yet. I have failed to produce a
> >>> situation where it is shown.
> >>
> >> I believe that.
> >>
> >>> This function ('socfpga_bridges_reset') is called 5 times throughout the
> >>> code, but only *one* single time with 'reset=0' as argument (only with
> >>> 0, the code in question is executed). And this is in SPL before
> >>> initializing the console and even before the debug UART can be
> >>> initialized.
> >>>
> >>> As I could see, the printf *is* executed on every boot (I saw the code
> >>> hanging when enabling debug UART). However, when not booting from FPGA,
> >>> it is just normal that the FPGA is not ready when running SPL. Why do
> >>> you want an error message here anyway?
> >>
> >> I was under the impression this is an error message, but it might not be
> >> so ? Maybe the wording is incorrect ?
> >
> > Now that I re-read it, "aborting" is incorrect, yes.
> >
> > So how should we proceed? This is an error message that can never be
> > shown (like the code is now) but breaks debug UART.
> >
> > I'd say we can drop it altogether. It might only be interesint if (in
> > the future) that code would get called from somewhere else (i.e. later,
> > after console initialization).
> >
> > Re-reading spl_gen5.c, there are some 'debug' calls before the debug
> > uart is initialized which probably would need to be removed as well, but
> > that's a different story...
>
> How come those don't hang the system then ?

I just haven't enabled debug output in spl_gen5.c, yet. I guess they would hang
the system when enabling them.

While it would be easy to remove these debug statements, to be future-proof
it would of course make sense to make the debug UART robust against this.

But given the problems with Rockchip ns16550, we would need a dedicated
debug UART for socfpga to solve this. And that would probably mean code
duplication.

Regards,
Simon