New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 687228 link

Starred by 1 user

Issue metadata

Status: Archived
Owner: ----
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

EC: Flush console output before printing "console ready string"

Project Member Reported by aaboagye@chromium.org, Jan 31 2017

Issue description

The console task prints a string on init, followed by the prompt, when it's ready to receive input:

"Console is enabled; type HELP for help.\n"

I think that we should perform a cflush() prior to printing that string to prevent it being broken up by other tasks' prints (possibly just afterwards as well).

Recently, the firmware_ECBootTime test was changed to search for that string, but I fear it will fail if the "console ready" string gets broken up.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Mar 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/e78307e2424d182814775a24b6fea78278475a91

commit e78307e2424d182814775a24b6fea78278475a91
Author: philipchen <philipchen@google.com>
Date: Tue Mar 21 02:30:15 2017

console: ensure "Console is enabled" string is intact

BUG= chromium:687228 
BRANCH=none
TEST=boot 10 times on kevin, and
see the complete string "Console is enabled..."

Change-Id: I9bb7358eb0a3d8172b5584329b9837cf62def635
Reviewed-on: https://chromium-review.googlesource.com/457421
Commit-Ready: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>

[modify] https://crrev.com/e78307e2424d182814775a24b6fea78278475a91/common/console.c

Status: Fixed (was: Available)
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 21 2017

Labels: merge-merged-firmware-gru-8785.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/5a2ae6a22382efece37c619aedb48da7ac1b26e8

commit 5a2ae6a22382efece37c619aedb48da7ac1b26e8
Author: philipchen <philipchen@google.com>
Date: Tue Mar 21 05:55:50 2017

console: ensure "Console is enabled" string is intact

BUG= chromium:687228 
BRANCH=none
TEST=boot 10 times on kevin, and
see the complete string "Console is enabled..."

Change-Id: I9bb7358eb0a3d8172b5584329b9837cf62def635
Reviewed-on: https://chromium-review.googlesource.com/457421
Commit-Ready: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
(cherry picked from commit e78307e2424d182814775a24b6fea78278475a91)
Reviewed-on: https://chromium-review.googlesource.com/457152
Reviewed-by: Philip Chen <philipchen@chromium.org>
Commit-Queue: Philip Chen <philipchen@chromium.org>

[modify] https://crrev.com/5a2ae6a22382efece37c619aedb48da7ac1b26e8/common/console.c

Comment 4 by dchan@google.com, May 30 2017

Labels: VerifyIn-60
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 30 2017

Labels: merge-merged-firmware-cr50-9308.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/5cd472ea45c750d4156fe20ed7ce6ea98709dfa3

commit 5cd472ea45c750d4156fe20ed7ce6ea98709dfa3
Author: philipchen <philipchen@google.com>
Date: Fri Jun 30 16:59:22 2017

console: ensure "Console is enabled" string is intact

BUG= chromium:687228 
BRANCH=none
TEST=boot 10 times on kevin, and
see the complete string "Console is enabled..."

Change-Id: I9bb7358eb0a3d8172b5584329b9837cf62def635
Reviewed-on: https://chromium-review.googlesource.com/457421
Commit-Ready: Philip Chen <philipchen@chromium.org>
Tested-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Aseda Aboagye <aaboagye@chromium.org>
(cherry picked from commit e78307e2424d182814775a24b6fea78278475a91)
Reviewed-on: https://chromium-review.googlesource.com/556183
Reviewed-by: Philip Chen <philipchen@chromium.org>
Reviewed-by: Vadim Bendebury <vbendeb@chromium.org>
Tested-by: Vadim Bendebury <vbendeb@chromium.org>
Commit-Queue: Vadim Bendebury <vbendeb@chromium.org>

[modify] https://crrev.com/5cd472ea45c750d4156fe20ed7ce6ea98709dfa3/common/console.c

Comment 6 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 7 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)
Project Member

Comment 8 by bugdroid1@chromium.org, May 31 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/3dff02fa73108a5d0d68fc9bc0c11a1b32cae478

commit 3dff02fa73108a5d0d68fc9bc0c11a1b32cae478
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Thu May 31 10:50:20 2018

console: Do not flush the console in console_init

Currently, console_init calls cflush() twice, once before
"Console is enabled" string is printed, once afterwards.

The reason is that firmware_ECBootTime looks for that string,
and it may get corrupted/interleaved with others if the EC
is busy during initialization.

The problem here is that the CONSOLE task may have higher
priority than other tasks (for good reasons), but, on boot,
there are other more critical tasks that need to run (e.g.
RW image verification), rather than busy-looping waiting for
the console to be flushed.

By fixing firmware_ECBootTime to not look for the string anymore,
we do not need those 2 console flush.

BRANCH=poppy
BUG=b:35647963
BUG= chromium:687228 
CQ-DEPEND=CL:1075832
TEST=Flash staff, see that RW verification starts at 0.001037
     instead of 0.028087 (=> 27 ms faster).
TEST=test_that -b $BOARD $IP firmware_ECBootTime

Change-Id: I794e48eb69cc647c4595fd80265adee4a434d566
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1073180
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>

[modify] https://crrev.com/3dff02fa73108a5d0d68fc9bc0c11a1b32cae478/common/console.c

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 1 2018

Labels: merge-merged-firmware-poppy-10431.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/ec/+/7d71b3fba9e365c129054417bf02334b4c867d4f

commit 7d71b3fba9e365c129054417bf02334b4c867d4f
Author: Nicolas Boichat <drinkcat@chromium.org>
Date: Fri Jun 01 06:22:08 2018

console: Do not flush the console in console_init

Currently, console_init calls cflush() twice, once before
"Console is enabled" string is printed, once afterwards.

The reason is that firmware_ECBootTime looks for that string,
and it may get corrupted/interleaved with others if the EC
is busy during initialization.

The problem here is that the CONSOLE task may have higher
priority than other tasks (for good reasons), but, on boot,
there are other more critical tasks that need to run (e.g.
RW image verification), rather than busy-looping waiting for
the console to be flushed.

By fixing firmware_ECBootTime to not look for the string anymore,
we do not need those 2 console flush.

BRANCH=poppy
BUG=b:35647963
BUG= chromium:687228 
CQ-DEPEND=CL:1075832
TEST=Flash staff, see that RW verification starts at 0.001037
     instead of 0.028087 (=> 27 ms faster).
TEST=test_that -b $BOARD $IP firmware_ECBootTime

Change-Id: I794e48eb69cc647c4595fd80265adee4a434d566
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1073180
Reviewed-by: Randall Spangler <rspangler@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1081767
Reviewed-by: Daisuke Nojiri <dnojiri@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@chromium.org>

[modify] https://crrev.com/7d71b3fba9e365c129054417bf02334b4c867d4f/common/console.c

Sign in to add a comment