[U-Boot] AM335x configurations - console output at early stages lost

Hi Simon,

On Mon, May 4, 2015 at 7:45 PM, Simon Glass <sjg@chromium.org> wrote:

> Hi Vasili,
>
> On 4 May 2015 at 10:21, Vasili Galka <vvv444@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Thu, Apr 30, 2015 at 3:38 AM, Simon Glass <sjg@chromium.org> wrote:
> >>
> >> Hi Vasili,
> >>
> >> On 29 April 2015 at 10:57, Vasili Galka <vvv444@gmail.com> wrote:
> >> > Hi Tom,
> >> >
> >> > I’m working on rebasing an old U-Boot branch for our company’s AM335x
> >> > based board upon the current U-Boot release (v2015.04). Our branch was
> >> > initially based on the v2013.10 release (the ti/am335x/ board was
> >> > taken as reference).
> >> >
> >> > After making all the code compile, I discovered there was a change in
> >> > the U-Boot initialization sequence, introduced by commit
> >> > a6b541b09022acb6f7c2754100ae26bd44eed1d9, that prevents our code from
> >> > working:
> >> >
> >> > Prior to the above mentioned commit, the console serial port was
> >> > initialized at a very early stage in the armv7/am335x/board.c:s_init()
> >> > function (calling preloader_console_init()). Roughly saying, it was
> >> > the first thing done by the SPL code. This made quite much sense, as
> >> > it enabled all the code afterwards to output status/debugging info to
> >> > the console.
> >> > Moving the preloader_console_init() call to spl_board_init() caused it
> >> > to be run at a very "late" stage (as part of the board_init_r()
> >> > function). AFAIU, now all the code that runs till then has no ability
> >> > for any console output.
> >> > For example, in our particular case, the DDR configuration code (in
> >> > sdram_init()) reads the DDR type from an I2C EEPROM. Obviously, if
> >> > something goes wrong in this process, we would like to report it to
> >> > the console.
> >> >
> >> > I understand there was a good reason for the mentioned commit (the
> >> > GD), however, as you see, we have also lost some important
> >> > functionality with it. I would appreciate your advice on how to solve
> >> > it.
> >>
> >> It would certainly be very useful and it would be great if we could
> >> printf() from as soon as we have a stack (and perhaps printch() even
> >> earlier). I think it can be done, but I have not put in the time to
> >> figure it out.
> >>
> >> You could take a look at CONFIG_DEBUG_UART which at least allows
> >> primitive UART access before gd is set up. I wonder if we could do
> >> something like change printf() /purchatr() to use this when gd is
> >> NULL?
> >>
> >> Regards,
> >> Simon
> >
> >
> > Thanks for the suggestion! I'm attempting to make it work.
> > Was the "Debug UART" code (added in
> 21d004368fc8a4da07147c58dfe9a4e16d4ab761) ever tested on AM335x? It does
> not seem to work for me, I'm currently debugging in attempt to understand
> the cause.
>
> From memory I think I did test it locally but did not send patches to
> enable it.
>
> You will need to enable DEBUG_UART_NS16550, and supply suitable values
> for DEBUG_UART_BASE and DEBUG_UART_CLOCK.
>
> But the debug UART code (in ns16550.c) is really simple so it should
> not be possible to debug it.
>
> I think it would be good to have the DEBUG_UART_... options in the
> board's defconfig file, even though CONFIG_DEBUG_UART itself is
> disabled. Then people can enable it easily when they want to use the
> debug UART.
>
> Regards,
> Simon
>

Yes, of course, please see below the changes I applied. I expected to see a
character written on to the UART, but I see nothing. So I attempt to
compare the new serial code to the old one (from 2013) that definitely
works.

Best,
Vasili

---
 /*
  * Default to using SPI for environment, etc.

Hi Simon,

On Mon, May 4, 2015 at 7:52 PM, Vasili Galka <vvv444@gmail.com> wrote:

> Hi Simon,
>
>
> On Mon, May 4, 2015 at 7:45 PM, Simon Glass <sjg@chromium.org> wrote:
>
>> Hi Vasili,
>>
>> On 4 May 2015 at 10:21, Vasili Galka <vvv444@gmail.com> wrote:
>> >
>> > Hi Simon,
>> >
>> > On Thu, Apr 30, 2015 at 3:38 AM, Simon Glass <sjg@chromium.org> wrote:
>> >>
>> >> Hi Vasili,
>> >>
>> >> On 29 April 2015 at 10:57, Vasili Galka <vvv444@gmail.com> wrote:
>> >> > Hi Tom,
>> >> >
>> >> > I’m working on rebasing an old U-Boot branch for our company’s AM335x
>> >> > based board upon the current U-Boot release (v2015.04). Our branch
>> was
>> >> > initially based on the v2013.10 release (the ti/am335x/ board was
>> >> > taken as reference).
>> >> >
>> >> > After making all the code compile, I discovered there was a change in
>> >> > the U-Boot initialization sequence, introduced by commit
>> >> > a6b541b09022acb6f7c2754100ae26bd44eed1d9, that prevents our code from
>> >> > working:
>> >> >
>> >> > Prior to the above mentioned commit, the console serial port was
>> >> > initialized at a very early stage in the
>> armv7/am335x/board.c:s_init()
>> >> > function (calling preloader_console_init()). Roughly saying, it was
>> >> > the first thing done by the SPL code. This made quite much sense, as
>> >> > it enabled all the code afterwards to output status/debugging info to
>> >> > the console.
>> >> > Moving the preloader_console_init() call to spl_board_init() caused
>> it
>> >> > to be run at a very "late" stage (as part of the board_init_r()
>> >> > function). AFAIU, now all the code that runs till then has no ability
>> >> > for any console output.
>> >> > For example, in our particular case, the DDR configuration code (in
>> >> > sdram_init()) reads the DDR type from an I2C EEPROM. Obviously, if
>> >> > something goes wrong in this process, we would like to report it to
>> >> > the console.
>> >> >
>> >> > I understand there was a good reason for the mentioned commit (the
>> >> > GD), however, as you see, we have also lost some important
>> >> > functionality with it. I would appreciate your advice on how to solve
>> >> > it.
>> >>
>> >> It would certainly be very useful and it would be great if we could
>> >> printf() from as soon as we have a stack (and perhaps printch() even
>> >> earlier). I think it can be done, but I have not put in the time to
>> >> figure it out.
>> >>
>> >> You could take a look at CONFIG_DEBUG_UART which at least allows
>> >> primitive UART access before gd is set up. I wonder if we could do
>> >> something like change printf() /purchatr() to use this when gd is
>> >> NULL?
>> >>
>> >> Regards,
>> >> Simon
>> >
>> >
>> > Thanks for the suggestion! I'm attempting to make it work.
>> > Was the "Debug UART" code (added in
>> 21d004368fc8a4da07147c58dfe9a4e16d4ab761) ever tested on AM335x? It does
>> not seem to work for me, I'm currently debugging in attempt to understand
>> the cause.
>>
>> From memory I think I did test it locally but did not send patches to
>> enable it.
>>
>> You will need to enable DEBUG_UART_NS16550, and supply suitable values
>> for DEBUG_UART_BASE and DEBUG_UART_CLOCK.
>>
>> But the debug UART code (in ns16550.c) is really simple so it should
>> not be possible to debug it.
>>
>> I think it would be good to have the DEBUG_UART_... options in the
>> board's defconfig file, even though CONFIG_DEBUG_UART itself is
>> disabled. Then people can enable it easily when they want to use the
>> debug UART.
>>
>> Regards,
>> Simon
>>
>
> Yes, of course, please see below the changes I applied. I expected to see
> a character written on to the UART, but I see nothing. So I attempt to
> compare the new serial code to the old one (from 2013) that definitely
> works.
>
> Best,
> Vasili
>
> ...
>

I've altered the debug_uart_init() to mimic the behaviour of NS16550_init()
more closely and it fixed the problem. I'll send a clean patch shortly.

Many thanks!

Best regards,
Vasili

On 4 May 2015 at 11:17, Vasili Galka <vvv444@gmail.com> wrote:
> Hi Simon,
>
> On Mon, May 4, 2015 at 7:52 PM, Vasili Galka <vvv444@gmail.com> wrote:
>>
>> Hi Simon,
>>
>>
>> On Mon, May 4, 2015 at 7:45 PM, Simon Glass <sjg@chromium.org> wrote:
>>>
>>> Hi Vasili,
>>>
>>> On 4 May 2015 at 10:21, Vasili Galka <vvv444@gmail.com> wrote:
>>> >
>>> > Hi Simon,
>>> >
>>> > On Thu, Apr 30, 2015 at 3:38 AM, Simon Glass <sjg@chromium.org> wrote:
>>> >>
>>> >> Hi Vasili,
>>> >>
>>> >> On 29 April 2015 at 10:57, Vasili Galka <vvv444@gmail.com> wrote:
>>> >> > Hi Tom,
>>> >> >
>>> >> > I’m working on rebasing an old U-Boot branch for our company’s
>>> >> > AM335x
>>> >> > based board upon the current U-Boot release (v2015.04). Our branch
>>> >> > was
>>> >> > initially based on the v2013.10 release (the ti/am335x/ board was
>>> >> > taken as reference).
>>> >> >
>>> >> > After making all the code compile, I discovered there was a change
>>> >> > in
>>> >> > the U-Boot initialization sequence, introduced by commit
>>> >> > a6b541b09022acb6f7c2754100ae26bd44eed1d9, that prevents our code
>>> >> > from
>>> >> > working:
>>> >> >
>>> >> > Prior to the above mentioned commit, the console serial port was
>>> >> > initialized at a very early stage in the
>>> >> > armv7/am335x/board.c:s_init()
>>> >> > function (calling preloader_console_init()). Roughly saying, it was
>>> >> > the first thing done by the SPL code. This made quite much sense, as
>>> >> > it enabled all the code afterwards to output status/debugging info
>>> >> > to
>>> >> > the console.
>>> >> > Moving the preloader_console_init() call to spl_board_init() caused
>>> >> > it
>>> >> > to be run at a very "late" stage (as part of the board_init_r()
>>> >> > function). AFAIU, now all the code that runs till then has no
>>> >> > ability
>>> >> > for any console output.
>>> >> > For example, in our particular case, the DDR configuration code (in
>>> >> > sdram_init()) reads the DDR type from an I2C EEPROM. Obviously, if
>>> >> > something goes wrong in this process, we would like to report it to
>>> >> > the console.
>>> >> >
>>> >> > I understand there was a good reason for the mentioned commit (the
>>> >> > GD), however, as you see, we have also lost some important
>>> >> > functionality with it. I would appreciate your advice on how to
>>> >> > solve
>>> >> > it.
>>> >>
>>> >> It would certainly be very useful and it would be great if we could
>>> >> printf() from as soon as we have a stack (and perhaps printch() even
>>> >> earlier). I think it can be done, but I have not put in the time to
>>> >> figure it out.
>>> >>
>>> >> You could take a look at CONFIG_DEBUG_UART which at least allows
>>> >> primitive UART access before gd is set up. I wonder if we could do
>>> >> something like change printf() /purchatr() to use this when gd is
>>> >> NULL?
>>> >>
>>> >> Regards,
>>> >> Simon
>>> >
>>> >
>>> > Thanks for the suggestion! I'm attempting to make it work.
>>> > Was the "Debug UART" code (added in
>>> > 21d004368fc8a4da07147c58dfe9a4e16d4ab761) ever tested on AM335x? It does not
>>> > seem to work for me, I'm currently debugging in attempt to understand the
>>> > cause.
>>>
>>> From memory I think I did test it locally but did not send patches to
>>> enable it.
>>>
>>> You will need to enable DEBUG_UART_NS16550, and supply suitable values
>>> for DEBUG_UART_BASE and DEBUG_UART_CLOCK.
>>>
>>> But the debug UART code (in ns16550.c) is really simple so it should
>>> not be possible to debug it.
>>>
>>> I think it would be good to have the DEBUG_UART_... options in the
>>> board's defconfig file, even though CONFIG_DEBUG_UART itself is
>>> disabled. Then people can enable it easily when they want to use the
>>> debug UART.
>>>
>>> Regards,
>>> Simon
>>
>>
>> Yes, of course, please see below the changes I applied. I expected to see
>> a character written on to the UART, but I see nothing. So I attempt to
>> compare the new serial code to the old one (from 2013) that definitely
>> works.
>>
>> Best,
>> Vasili
>>
>> ...
>
>
> I've altered the debug_uart_init() to mimic the behaviour of NS16550_init()
> more closely and it fixed the problem. I'll send a clean patch shortly.

Great! The code is a bit of a mess, hoping we can simplify it once
everything moves to driver model.

- Simon