Hi, On Fri, 23 Sept 2022 at 10:19, Tom Rini <tr...@konsulko.com> wrote: > > On Fri, Sep 23, 2022 at 06:07:03PM +0200, Pali Rohár wrote: > > On Friday 23 September 2022 11:57:30 Tom Rini wrote: > > > On Fri, Sep 23, 2022 at 05:45:07PM +0200, Pali Rohár wrote: > > > > On Thursday 22 September 2022 17:06:31 Heinrich Schuchardt wrote: > > > > > On 9/22/22 15:14, Pali Rohár wrote: > > > > > > On Thursday 22 September 2022 13:27:51 Simon Glass wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Wed, 21 Sept 2022 at 15:56, Tom Rini <tr...@konsulko.com> > > > > > > > wrote: > > > > > > > > > > > > > > > > On Wed, Sep 21, 2022 at 03:54:13PM +0200, Pali Rohár wrote: > > > > > > > > > On Wednesday 21 September 2022 09:49:24 Tom Rini wrote: > > > > > > > > > > On Mon, Sep 05, 2022 at 11:31:15AM +0200, Pali Rohár wrote: > > > > > > > > > > > > > > > > > > > > > On certain places it is required to flush output print > > > > > > > > > > > buffers to ensure > > > > > > > > > > > that text strings were sent to console or serial devices. > > > > > > > > > > > For example when > > > > > > > > > > > printing message that U-Boot is going to boot kernel or > > > > > > > > > > > when U-Boot is > > > > > > > > > > > going to change baudrate of terminal device. > > > > > > > > > > > > > > > > > > > > > > Some console devices, like UART, have putc/puts functions > > > > > > > > > > > which just put > > > > > > > > > > > characters into HW transmit queue and do not wait until > > > > > > > > > > > all data are > > > > > > > > > > > transmitted. Doing some sensitive operations (like > > > > > > > > > > > changing baudrate or > > > > > > > > > > > starting kernel which resets UART HW) cause that U-Boot > > > > > > > > > > > messages are lost. > > > > > > > > > > > > > > > > > > > > > > Therefore introduce a new flush() function, implement it > > > > > > > > > > > for all serial > > > > > > > > > > > devices via pending(false) callback and use this new > > > > > > > > > > > flush() function on > > > > > > > > > > > sensitive places after which output device may go into > > > > > > > > > > > reset state. > > > > > > > > > > > > > > > > > > > > > > This change fixes printing of U-Boot messages: > > > > > > > > > > > "## Starting application at ..." > > > > > > > > > > > "## Switch baudrate to ..." > > > > > > > > > > > > > > > > > > > > > > Changes in v3: > > > > > > > > > > > * add macro STDIO_DEV_ASSIGN_FLUSH() > > > > > > > > > > > * fix commit messages > > > > > > > > > > > * remove changes from serial.c > > > > > > > > > > > > > > > > > > > > > > Changes in v2: > > > > > > > > > > > * split one big patch into smaller 6 patches > > > > > > > > > > > * add config option to allow disabling this new function > > > > > > > > > > > > > > > > > > > > > > Pali Rohár (6): > > > > > > > > > > > sandbox: Add function os_flush() > > > > > > > > > > > console: Implement flush() function > > > > > > > > > > > serial: Implement flush callback > > > > > > > > > > > serial: Implement serial_flush() function for console > > > > > > > > > > > flush() fallback > > > > > > > > > > > serial: Call flush() before changing baudrate > > > > > > > > > > > boot: Call flush() before booting > > > > > > > > > > > > > > > > > > > > Including the change you suggested to 4/6 to fix that build > > > > > > > > > > issue, > > > > > > > > > > there's at least one more large issue that prevents CI from > > > > > > > > > > getting too > > > > > > > > > > far: > > > > > > > > > > https://source.denx.de/u-boot/u-boot/-/pipelines/13534 > > > > > > > > > > and they all have a failure similar to: > > > > > > > > > > https://source.denx.de/u-boot/u-boot/-/jobs/500794#L51 > > > > > > > > > > > > > > > > > > It looks like that some efi stuff overloads u-boot functions, > > > > > > > > > in this > > > > > > > > > case newly added flush() function. > > > > > > > > > > > > > > > > > > Any idea how to handle this issue? > > > > > > > > > > > > > > I think you should rename the EFI flush() function to something > > > > > > > like > > > > > > > efi_flush(). > > > > > > > > > > > > Ok! I renamed EFI flush() function to efi_flush(). > > > > > > > > > > > > I put all patches into github pull request which started CI test: > > > > > > https://github.com/u-boot/u-boot/pull/241 > > > > > > > > > > When merging, please, use > > > > > [PATCH 1/1] efi_selftest: prefix test functions with efi_st_ > > > > > https://lists.denx.de/pipermail/u-boot/2022-September/495334.html > > > > > > > > Hello! I updated https://github.com/u-boot/u-boot/pull/241 pull request > > > > for CI test. I added there above efi_selftest patch and after that CI > > > > test passed. > > > > > > > > So Tom, is this enough for you? Or do you need something more? > > > > > > Were you also going to rename flush as suggested? > > > > I have not done any renaming (yet). I pushed on CI test above efi_st fix > > and this patch series (with 4/6 build fix issue). Nothing more. > > > > About renaming, I'm not sure what if flush_console() is better name as > > this is generic API and it can do e.g. flushing LCD output. Just like > > generic puts() or getc(). I just do not know if generic name is better > > or worse... > > I'm not sure either, lets wait a little bit longer to see if anyone else > chimes in with a strong opinion?
I feel that flush() is probably the right name for now, since this is a generic function as the rest of those functions don't have prefixes or suffixes. Regards, Simon