erebion/coreboot
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

drivers/spi/flashconsole.c: Fix flashconsole

Browse source 
Commit 1f2408f573 ("console: Fix flushing for slow consoles") fixed a
typo related to some refactoring of the CBMEM fast code path. However,
this also seems to have indirectly broken the SPI flash console, causing
only the console header messages at the beginning of each stage to be
stored.

This is caused by multiple calls to flashconsole_tx_flush() without a
call to flashconsole_tx_byte() in between them. Data is accumulated in a
buffer during calls to flashconsole_tx_byte(), which is then written to
the flash during a flush. If no tx calls occur between flushes, the
second call will try to write data of length 0, which seems to cause
rdev_writeat() to return -1. This causes an early return, since the
return value of rdev_writeat() must match the data length in order for
the rest of the flashconsole_tx_flush to run. The flush function
contains a busy flag to prevent recursive calls to itself, and the early
return prevents it from being reset. Thus, the busy flag remains set for
the remainder of the stage, blocking all future flushes.

The multiple flushes occur because vprintk flushes console drivers after
the string has been sent, but flashconsole_tx_byte() also calls a flush
whenever a newline is encountered. Because of this, flushes are disabled
for the remainder of each stage after the first printk call containing a
newline is stored to the flash console. Although this newline check
could be omitted, flashconsole_tx_byte() also invokes a flush when its
data buffer is full, which shouldn't be avoided.

Prior to the mentioned commit, the incorrect logic happened to prevent
the flush in vprintk, preventing the double flush issue from occurring.
The mentioned commit inverted the logic, allowing the double flush to
occur.

Address this by returning early if len = 0. While we're here,
consolidate the early returns into a single check and change the busy
flag to a bool instead of an int.

TEST=Console messages are not missing in the flashconsole. Tested on the
Lenovo ThinkCentre M900 SFF.

Change-Id: Ic6c2418f04a687610df020df117f7be90b1724b9
Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/91428
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-by: Julius Werner <jwerner@chromium.org>
This commit is contained in:
Nicholas Chin 2026-02-24 23:29:17 -07:00 • committed by Matt DeVillier
commit ce5c915344
1 changed files with 8 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

16
src/drivers/spi/flashconsole.c
View file

@ -87,18 +87,20 @@ void flashconsole_tx_flush(void)
{
size_t len = line_offset;
size_t region_size;
static int busy;
/* Prevent any recursive loops in case the spi flash driver
* calls printk (in case of transaction timeout or
* any other error while writing) */
if (busy)
static bool busy;
/* In addition to being an obvious runtime optimization, checking for
* len = 0 also prevents false disabling of the driver, as rdev_writeat
* seems to return -1 if len is zero even though this should be a
* recoverable condition. */
if (busy || !rdev_ptr || len == 0)
return;
if (!rdev_ptr)
return;
busy = 1;
busy = true;
region_size = region_device_sz(rdev_ptr);
if (offset + len >= region_size)
len = region_size - offset;
@ -113,5 +115,5 @@ void flashconsole_tx_flush(void)
offset += len;
line_offset = 0;
busy = 0;
busy = false;
}