According to util/kbc1126/README.md, for these ECs to work, the
address and size of their two firmware should be written to $s-0x100`
(`$s` means the image size, done with kbc1126_ec_insert), which means
that every existing section (especially those used to store code)
should not overlap this address, otherwise the bootblock will get
damaged when inserting firmwares of the EC.
In this commit, ecfw_ptr is a structure initialized at build time
according to CONFIG_KBC1126_FW1_OFFSET and CONFIG_KBC1126_FW2_OFFSET
(to do so, they should be redefined as hex), and linked to
CONFIG_ECFW_PTR_ADDR within bootblock, so kbc1126_ec_insert is not
needed at build time any more.
Test passed on Elitebook Folio 9470m.
Signed-off-by: Bill XIE <persmule@hardenedlinux.org>
Change-Id: I4f0de0c4d7283e630242fbe84a46e0547783c49e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51671
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Some boards with the chrome EC will need to support more than 4
temperature sensors, so modify the number of TSRs supported when
generating the ACPI code. Note that the EC memory map already has
support for up to 16 TSRs, so no change is required on the EC
side.
BUG=b:207585491
TEST=with previous patch and some test data in brya0 overridetree.cb,
dump the SSDT and verify that all of the existing Methods for TSR0-TSR3
are also added for TSR4, as well as all Notify, etc.
Signed-off-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Change-Id: Id002230bc872b0f818b0bf2b87987298189c973d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59633
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-by: Scott Chao <scott_chao@wistron.corp-partner.google.com>
Reviewed-by: Sumeet R Pawnikar <sumeet.r.pawnikar@intel.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Add a new config for boards with dGPUs to enable reporting fan duty and
temperature. The dGPU is not yet enabled on any boards, so it always
reports the temp as 0. However, the EC firmware does use the dGPU's fan
and so reports valid information for fan speed.
Change-Id: Iae1063ee6a082a77ed026178eb9471bbc2b2fadf
Signed-off-by: Jeremy Soller <jeremy@system76.com>
Signed-off-by: Tim Crawford <tcrawford@system76.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57881
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Given EC CON and associated USB port objects, custom_pld or pld_group
information is retrieved from port and added to ACPI table as _PLD field
for typec connector.
BUG=b:202446737
TEST=emerge-brya coreboot & SSDT dump in Brya test device
Signed-off-by: Won Chung <wonchung@google.com>
Change-Id: Ibc56ecd4e8954ffaace3acd9528a064b5fa2cf6f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59401
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Add EC support that supports different Q Events and EC memory.
Created from the ITE IT5570E and IT8987E datasheets, all using
data port 0x4e.
Tested with Ubuntu 20.04.3 and Windows 10 on:
* StarBook Mk V (TGL + IT5570E):
* ITE Firmware 1.00
* Merlin Firmware 1.00
* LabTop Mk IV (CML + IT8987E):
* ITE Firmware 1.04
* LabTop Mk III (KBL + IT8987E):
* ITE Firmware 3.12
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
Change-Id: I8023c26de23c874c84106fda96e64dcfa0c5ba32
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58343
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Andy Pont <andy.pont@sdcsystems.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Remove old code in favour of new format of firmware API.
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
Change-Id: Iaf8f37a08c232b8754e57f022782f21284fa07dd
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58344
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
google_chromeec_usb_pd_get_info() is used in ec.c only. Make it
static and drop from ec.h.
BUG=b:192947843
Signed-off-by: Derek Huang <derek.huang@intel.corp-partner.google.com>
Change-Id: I4b3df4223d5c26ea1c1a52b26f7d49fa4c947de8
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58060
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Add API to allow AP to send the command to EC to enter DP ALT mode
and API to wait for DP HPD event.
BUG=b:192947843
TEST=select ENABLE_TCSS_DISPLAY_DETECTION in Kconfig.name. Build
coreboot and update your system. Boot the system you will find below
message in the coreboot log with or without USB-C display connected:
'HPD ready after %lu ms' or 'HPD not ready after %ldms. Abort.'.
Signed-off-by: Derek Huang <derek.huang@intel.corp-partner.google.com>
Change-Id: Id11510c1ff58579ae2cddfe5a4d69646fd84f5c3
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57138
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
1. Update google_chromeec_pd_get_amode() to return bitmask.
2. Update google_chromeec_wait_for_displayport() to handle the
updated return value of google_chromeec_pd_get_amode().
3. Drop google_chromeec_pd_get_amode() from ec.h and make it static
because it's not used outside of ec.c.
BUG=b:192947843
Signed-off-by: Derek Huang <derek.huang@intel.corp-partner.google.com>
Change-Id: I6020c4305e30018d4c97d862c16e8d642c951765
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58058
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Add parameter `active_cable` to obtain the cable type
(active or passive) which is needed for USB-C configuration for
some SoCs (at least Intel TGL and ADL), change the function name to
google_chromeec_usb_pd_get_info() for better understanding.
BUG=b:192947843
Signed-off-by: Derek Huang <derek.huang@intel.corp-partner.google.com>
Change-Id: Ie91a3096d49d5dde75e60ab0f2f38152cef720f6
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58057
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
These issues were found and fixed by codespell, a useful tool for
finding spelling errors.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I5b8ecdfe75d99028fee820a2034466a8ad1c5e63
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58080
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This code is identical except for some renaming.
Change-Id: I93795a6087ce0daca27c0d5038a1febd6ca9c775
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56522
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Implement _BIX method to expose battery cycle count.
Requires an EC version with support for cycle count.
Change-Id: I5f7a1d275caff59960aaf9c39b9c707970350987
Signed-off-by: Ian Douglas Scott <idscott@system76.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57678
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jeremy Soller <jeremy@system76.com>
There was no code attached to this driver and hence one couldn't hook
it up to any device. Even if mentioned in the `devicetree.cb` it was
still dead code.
Change-Id: I12415ea9e0120b1d00524f8f39f9b2d02f46ba05
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/57431
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
- Chromebook have some platform need support MICMUTE and KBDILLUMTOGGLE.
- Sync ec_commands.h
This change syncs the coreboot version of google ec_commands.h with the
ec_commands.h from the google ec repository. This is a straight copy
except for the the copyright header.
BUG=b:194146863
BRANCH=none
TEST=check on evtest
type 4 (EV_MSC), code 4 (MSC_SCAN), value 9e
type 1 (EV_KEY), code 228 (KEY_KBDILLUMTOGGLE), 1
type 4 (EV_MSC), code 4 (MSC_SCAN), value 9b
type 1 (EV_KEY), code 248 (KEY_MICMUTE), value 1
Signed-off-by: Scott Chao <scott_chao@wistron.corp-partner.google.com>
Change-Id: Ie4fa3e627f448265f72279704d258b2d3fe8fc17
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56710
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Use bit-wise and logical operators instead of their equivalent methods.
Built roda/rw11 with `BUILD_TIMELESS=1` and coreboot.rom remains the
same.
Change-Id: I30807e14b2a9a8203a76d418f586423bcaec2a3a
Signed-off-by: Felix Singer <felix.singer@secunet.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56606
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Use `FOOO[1337]` instead of `Index(FOOO, 1337)`.
Built roda/rw11 with `BUILD_TIMELESS=1` and coreboot.rom remains the
same.
Change-Id: I4f5d5cb8ce8c3ae37dc44ca87bd67596af9feee8
Signed-off-by: Felix Singer <felix.singer@secunet.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56604
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The GPE offset of 16 is PCH specific.
Built roda/rw11 with `BUILD_TIMELESS=1` and coreboot.rom remains the
same.
Change-Id: I4ec38fc28d2436f84a090bb4ab38f20612cfd795
Signed-off-by: Nico Huber <nico.huber@secunet.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56080
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The patch adds an interface for configuring GPIOs inside the Kontron
CPLD/EC. This allows to statically define the mode for each GPIO pin
in devicetree.cb of the motherboard or carrier board. For example:
chip ec/kontron/kempld
device gpio 0 on
register "gpio[0]" = "KEMPLD_GPIO_INPUT"
register "gpio[4]" = "KEMPLD_GPIO_OUTPUT_LOW"
register "gpio[5]" = "KEMPLD_GPIO_OUTPUT_HIGH"
register "gpio[11]" = "KEMPLD_GPIO_DEFAULT"
end
end
In this case, <device gpio 0>, like all other devices, is not a real
device inside the EC. These definitions are used to understand the EC
resources and systematize configuration options, but if mark this as
<off>, the initialization step will be skipped in the driver code.
Use KEMPLD_GPIO_DEFAULT or skip it in devicetree.cb to not configure
the GPIO and keep the default mode after CPLD reset.
This work is based on code from the drivers/gpio/gpio-kempld.c linux
driver. Tested on Kontron mAL-10 COMe module [1].
[1] CB:54380 , Change-Id: I7d354aa32ac8c64f54b2bcbdb4f1b8915f55264e
Change-Id: Id767aa451fbf2ca1c0dccfc9aa2c024c6f37c1bb
Signed-off-by: Maxim Polyakov <max.senia.poliak@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47595
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
google_chromeec_get_event returns 0 for no event. Return
EC_HOST_EVENT_NONE=0 to improve readability.
BUG=b:184074997
TEST=Build and boot guybrush without error
Signed-off-by: Rob Barnes <robbarnes@google.com>
Change-Id: Ic08ed9ccdd7c0023d0fe8b641fcf60dca495a242
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55547
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
This change syncs the coreboot version of google ec_commands.h with the
ec_commands.h from the google ec repository. This is a straight copy
except for the the copyright header.
BUG=b:184074997
TEST=Build and boot guybrush
BRANCH=None
Change-Id: I095c3316d720328cb7b8dd1b72ffc108208b14bd
Signed-off-by: Rob Barnes <robbarnes@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55911
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Somehow, enabling the notification to the OS driver breaks the
functionality it was meant to enable. Until this can be resolved,
disable the driver notification, so that the key functions as intended.
Test: build/boot librem_bdw and librem_skl boards, verify trackpad
enable toggle via Fn+F1 works properly.
Change-Id: Ic7bdb3154a87c4202b5ee1fd333281ef78db1104
Signed-off-by: Matt DeVillier <matt.devillier@puri.sm>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55657
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
google_chromeec_get_event returns an event number and 0 when there's no event.
This function is usually called in a loop until there are no more events, so
it makes sense to return 0 (i.e. no event) when there's an error.
BUG=b:184074997
TEST=Boot guybrush, no ec errors
Change-Id: I6c0186e4637af9ae24f45cce3638f0913227d6a7
Signed-off-by: Rob Barnes <robbarnes@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55437
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Raul Rangel <rrangel@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
All functions in ec_skuid.c except google_chromeec_get_board_sku()
are for SMBIOS platforms. Move these functions to a new file to allow
non-SMBIOS platforms to use google_chromeec_get_board_sku() without
having to declare MAINBOARD_SMBIOS_MANUFACTURER.
BUG=none
TEST=emerge-cherry coreboot
BRANCH=none
Change-Id: I8916223f5f04afe4761be4ad3313e900efae90d4
Signed-off-by: Yu-Ping Wu <yupingso@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/55174
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Support for Star Labs labtop series EC
Signed-off-by: Sean Rhodes <sean@starlabs.systems>
Change-Id: I1967f7c4a7e3cab714f22844bf36749e0c9652b6
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52797
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Building google/sarien with a 64-bit compiler (x86_64-linux-gnu) fails
with the error below.
src/ec/google/wilco/mailbox.c: In function 'wilco_ec_transfer':
src/ec/google/wilco/mailbox.c:184:43: error: format '%lu' expects argument of type 'long unsigned int', but argument 4 has type 'size_t' {aka 'unsigned int'} [-Werror=format=]
184 | printk(BIOS_ERR, "%s: data too short (%lu bytes, expected %zu)",
| ~~^
| |
| long unsigned int
| %u
185 | __func__, rs.data_size - skip_size, msg->response_size);
| ~~~~~~~~~~~~~~~~~~~~~~~~
| |
| size_t {aka unsigned int}
`data_size` has type `uint16_t`, and `skip_size` has type `size_t`,
whose size differs in 32-bit (unsigned int) and 64-bit (unsigned long).
So use the length modifier `z` for a `size_t` argument.
Found-by: x86_64-linux-gnu-gcc-10 (Debian 10.2.1-6) 10.2.1 20210110
Change-Id: Ida27323daeed9b8ff487302d0f3d6fcce0bbb705
Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54786
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Duncan Laurie
This adds the required method to access temperature data from the
ChromeEC.
BUG=b:186166365
TEST=Boot guybrush to the OS and verify temperatures
$ tail /sys/devices/virtual/thermal/thermal_zone*/temp
==> /sys/devices/virtual/thermal/thermal_zone0/temp <==
31900
==> /sys/devices/virtual/thermal/thermal_zone1/temp <==
34900
==> /sys/devices/virtual/thermal/thermal_zone2/temp <==
31900
==> /sys/devices/virtual/thermal/thermal_zone3/temp <==
33900
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: I418b6691a7d00a4c2d89c9c1fe8f9416602be0f1
Reviewed-on: https://review.coreboot.org/c/coreboot/+/54133
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
coreboot needs to access EC RFWU entry in order to suspend and resume PD
and modes setting. This change adds ec_retimer_fw_update implementation
for retimer firmware upgrade.
BUG=b:186521258
TEST=Build image successfully.
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: Ib937d8bd72fc39487854773573b435bf2add672a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52713
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Along with upstream kernel for Retimer firmware update, coreboot changes
the ec_retimer_fw_update format. This change removes this API and will
add implementation later once the dependent definition is complete.
BUG=b:186521258
TEST=Build image successfully.
Signed-off-by: John Zhao <john.zhao@intel.com>
Change-Id: I2d074b84fb3cb87b443871104b72b6c316af5279
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52710
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
The CMOS option system does not support negative integers. Thus, retype
and rename the option API functions to reflect this.
Change-Id: Id3480e5cfc0ec90674def7ef0919e0b7ac5b19b3
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52672
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
This change is needed to update the option API to use unsigned integers.
The CMOS option system does not support negative numbers.
The volume field is only 8 bits long. Do not set the volume if it is out
of range. Also, use an out-of-range value as fallback to skip setting
the volume when it cannot be read using the option API, to preserve the
current behavior.
Change-Id: I7af68bb5c1ecd4489ab4b826b9a5e7999c77b1ff
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52675
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Rewrite google_chromeec_status_check to use stopwatch instead of a
delay in a while loop. In practice the while loop ends up taking
much longer than one second to timeout. Using stopwatch library will
accurately timeout after one second.
BUG=b:183524609
TEST=Build and run on guybrush
BRANCH=None
Change-Id: I363ff7453bcf81581884f92797629a6f96d42580
Signed-off-by: Rob Barnes <robbarnes@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51775
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
- remove unused Kconfig options
- change ACPI device name and HID
- remove ACPI for unused color keyboard backlight
- add support for RGB notification LED
- rename Wifi LED ACPI variable
- set some battery info defaults not populated by the EC
Change-Id: I72eca9deb83e5a6d919d6fcbd3b354fbf6e7a925
Signed-off-by: Matt DeVillier <matt.devillier@puri.sm>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52391
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Initial commit is a clone of ec/system76/ec with string changes;
Purism-specific functionality will be added in subsequent commits.
Change-Id: I8c51724e6dbfe1bc09496537f9e031643f95c755
Signed-off-by: Matt DeVillier <matt.devillier@puri.sm>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52390
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
When EC_GOOGLE_CHROMEEC_INCLUDE_SSFC_IN_FW_CONFIG is enabled and SSFC is
not set, all fw_config is invalidated. But for some platform this may
not be necessary, we can treat missing SSFC as zero and use other 32
bits of firmware config.
BUG=b:184809649
TEST=boot and check fw_config is not -1 even if ssfc is not set
BRANCH=zork
Signed-off-by: Kangheui Won <khwon@chromium.org>
Change-Id: I21c7b0d449a694d28ad7b3f14b035e3a5830030a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52205
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
Reviewed-by: Marco Chen <marcochen@google.com>
Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
Bill XIE