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

Issue 688743 link

Starred by 1 user
Status: Verified
Owner:
█ sha...@chromium.org
Last visit > 30 days ago
Closed: Feb 2017
Cc:
aaboagye@chromium.org
drinkcat@chromium.org
█ rspangler@chromium.org
adurbin@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

ec: Reduce UART Tx volume

Project Member Reported by sha...@chromium.org, Feb 4 2017 
Host-side EC UART logging is a useful feature, but we typically need 4096 bytes in our Tx buffer in order to completely log power-on / sysjump until first log retrieval without wrapping around.

Goal: Reduce UART Tx volume so that everything can fit in 2048 bytes on a typical boot, so that we can cut our Tx buffer size in half.

We can achieve this by shortening certain strings that are super-verbose ("clearing MKBP common fifo", "(re)initializing buttons and interrupts", "Base Accel ODR: 10000 - roundup 1 from config 1 [AP 0]]", etc), not printing all of our sha256, and reducing the "%T" / current time print precision to 3 digits right of the decimal / msec (we can leave usec precision as a CONFIG option).

Comment 1 by sha...@chromium.org, Feb 4 2017

Components: OS>Firmware>EC
Labels: -Pri-3 OS-Chrome Pri-1

Comment 2 by rspangler@chromium.org, Feb 6 2017

Cc: aaboagye@chromium.org
This will also help with binary size, since those strings take up a non-trivial amount of space.

We can get more cryptic as long as we have a decoder ring.  At some point we'll be down to POST codes... :)

One way to do this would be something like:

CPRT("Clr-MKBP-F", "Clearing MKBO FIFO");

CPRT("BAccODR %d/%d/%d/%d", 
     "Base Accel ODR: %d - roundup %d from config %d [AP %d]",
     a, b, c, d)

If CONFIG_CONSOLE_VERBOSE, you get the second format string (and 6 digits of time).  If not, you get the first one.

We could also have a CPRINTV() macro which only prints if CONFIG_CONSOLE_VERBOSE, and compiles to nothing if it's brief.  We've periodically had things like that on a module-by-module basis.
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 11 2017 

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

commit eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Sat Feb 11 21:06:38 2017

console: Add non-verbose print config option

Shorten certain long prints and reduce the precision of timestamp prints
when CONFIG_CONSOLE_VERBOSE is undef'd.

BUG= chromium:688743 
BRANCH=gru
TEST=On kevin, cold reset the EC, boot to OS, and verify cros_ec.log
contains all data since sysjump and is < 2K bytes (~1500 bytes).

Change-Id: Ia9390867788d0ab3087f827b0296107b4e9d4bca
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/438932
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>

[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/accel_lis2dh.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/vboot_hash.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/accel_kionix.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/motion_sense.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/als_si114x.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/keyboard_mkbp.c
[add] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/sensor_common.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/accel_bma2x2.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/build.mk
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/include/config.h
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/button.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/accelgyro_lsm6ds0.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/gyro_l3gd20h.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/common/printf.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/include/motion_sense.h
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/accelgyro_bmi160.c
[modify] https://crrev.com/eb2e38ec56c5701a1d9bcc3618957b4d4dee50f6/driver/accelgyro_lsm6dsm.c
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 16 2017

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

commit caba209720d315759d1a5342cfbf06a6a9b24930
Author: Shawn Nematbakhsh <shawnn@chromium.org>
Date: Thu Feb 16 02:20:16 2017

console: Add non-verbose print config option

Shorten certain long prints and reduce the precision of timestamp prints
when CONFIG_CONSOLE_VERBOSE is undef'd.

BUG= chromium:688743 
BRANCH=gru
TEST=On kevin, cold reset the EC, boot to OS, and verify cros_ec.log
contains all data since sysjump and is < 2K bytes (~1500 bytes).

Change-Id: Ia9390867788d0ab3087f827b0296107b4e9d4bca
Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/438932
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Shawn N <shawnn@chromium.org>
Reviewed-by: Shawn N <shawnn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/442754

[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/vboot_hash.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/driver/accel_kionix.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/motion_sense.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/driver/als_si114x.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/keyboard_mkbp.c
[add] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/sensor_common.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/driver/accel_bma2x2.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/build.mk
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/include/config.h
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/button.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/driver/accelgyro_lsm6ds0.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/common/printf.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/include/motion_sense.h
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/driver/accelgyro_bmi160.c
[modify] https://crrev.com/caba209720d315759d1a5342cfbf06a6a9b24930/driver/gyro_l3gd20h.c

Comment 5 by sha...@chromium.org, Feb 27 2017

Status: Verified (was: Untriaged)
►

Sign in to add a comment