This adds an oak libpayload config, that should fit all oak-based
devices such as elm.
Change-Id: Iabb71404ff84029a5976371a353e8c92e781ca1f
Signed-off-by: Paul Kocialkowski <contact@paulk.fr>
Reviewed-on: https://review.coreboot.org/18447
Tested-by: build bot (Jenkins)
Reviewed-by: Julius Werner <jwerner@chromium.org>
These parameters are probably the result of copying from the Thinkpad
X60 code.
Change-Id: I29763b38618d4b306c37424c5c4b57dfcf69424b
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/18290
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Set the proper memory configuration for the MinnowBoard 3. The current
values are copied from intel/leafhill. Set the proper values for
MinnowBoard 3.
Change-Id: Ie37842f5ce2cabaa892f42ee945c91fe3ace527a
Signed-off-by: Brenton Dong <brenton.m.dong@intel.com>
Reviewed-on: https://review.coreboot.org/18374
Tested-by: build bot (Jenkins)
Reviewed-by: Mario Scheithauer <mario.scheithauer@siemens.com>
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
This commit adds the initial scaffolding for the MinnowBoard 3
with Apollo Lake silicon.
This mainboard is based on Intel's Leafhill CRB with Apollo Lake
silicon. In a first step, it concerns only a copy of intel/leafhill
directory with name changes. Special adaptations for MinnowBoard 3
mainboard will follow in separate commits.
Change-Id: I7563fe37c89511c7035c5bffc9b034b379cfcaf4
Signed-off-by: Brenton Dong <brenton.m.dong@intel.com>
Reviewed-on: https://review.coreboot.org/18298
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth@google.com>
This file reportedly didn't compile on SUSE Linux with gcc 4.3.4:
[...]
> HOSTCC cbfstool/fsp_relocate.o
> In file included from coreboot/src/commonlib/fsp_relocate.c:18:
> coreboot/src/commonlib/include/commonlib/fsp.h:26: error:
> expected '=', ',', ';', 'asm' or '__attribute__' before
> 'fsp_component_relocate'
[...]
According to POSIX-2008[1], sys/types.h defines ssize_t, so include it.
This should not break coreboot code (as opposed to utils code), as we
have a sys/types.h in src/include.
[1]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_types.h.html
Change-Id: Id3694dc76c41d800ba09183e4b039b0719ac3d93
Signed-off-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Reviewed-on: https://review.coreboot.org/18417
Tested-by: build bot (Jenkins)
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Martin Roth <martinroth@google.com>
This is more consistent with newer Intel targets.
Change-Id: I52ee8d3f0c330a03bd6c18eed08e578dd6ae284b
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Reviewed-on: https://review.coreboot.org/18371
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
There was a 'typo' where the subsystem id was set instead of the codec
vendor id. This caused the lynxpoint HDA codecs init to fail to find
the proper codecid verbs so codecs were never initialized. That caused
the headphones jack to not work.
Change-Id: I975031643fc42937ecaea2300639b90632543f67
Signed-off-by: Youness Alaoui <youness.alaoui@puri.sm>
Reviewed-on: https://review.coreboot.org/18411
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
Tested-by: build bot (Jenkins)
The M.2 SSD is on the SATA port 3, which also required the DTLE setting
to be set.
This fixes issues with the M.2 SSD not being detected/stable.
Change-Id: Id39d9ec395a2d9d32be4c079678d0708f08b3935
Signed-off-by: Youness Alaoui <youness.alaoui@puri.sm>
Reviewed-on: https://review.coreboot.org/18409
Tested-by: build bot (Jenkins)
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
The Broadwell SATA controller supports IOBP registers on ports 0 and 1 but
Browell supports up to 4 ports, so we need to support setting IOBP for
ports 2 and 3 as well.
The magic numbers (IOBP SECRT88 and DTLE) for ports 2 and 3 were only
guessed by looking at ports 0 and 1 and extrapolating from there.
Port 3 has been tested (DTLE setting on Librem 13) and confirmed to work
so we can assume that port 2 and 3 magic numbers are valid, but having
someone confirm them (through non-public documents?) would be great.
Change-Id: I59911cfa677749ceea9a544a99b444722392e72d
Signed-off-by: Youness Alaoui <youness.alaoui@puri.sm>
Reviewed-on: https://review.coreboot.org/18408
Tested-by: build bot (Jenkins)
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
In order to allow GPIOs to be set/clear according to their polarity,
provide helper functions that check for polarity and call set/clear
SoC functions for generating ACPI code.
BUG=None
BRANCH=None
TEST=Verified that the ACPI code generated remains the same as before
for reef.
Change-Id: Ie8bdb9dc18e61a4a658f1447d6f1db0b166d9c12
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/18427
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Tested-by: build bot (Jenkins)
This is done to avoid any conflicts with same IRQ enums defined by other
drivers.
BUG=None
BRANCH=None
TEST=Compiles successfully
Change-Id: I539831d853286ca45f6c36c3812a6fa9602df24c
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/18444
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Change-Id: I538c28fb1bc412947ef9df947fa3f6a3312aeb4b
Signed-off-by: Patrick Georgi <pgeorgi@chromium.org>
Reviewed-on: https://review.coreboot.org/18322
Tested-by: build bot (Jenkins)
Reviewed-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Martin Roth <martinroth@google.com>
Adding spi_xfer_two_vectors as .xfer_vector for ipq40xx spi_ctrlr.
Commit c2973d196d ("UPSTREAM: spi: Get rid of SPI_ATOMIC_SEQUENCING")
has added a new driver method xfer_vector to support combined write-read
operation within a single CS cycle. The method is wrapped in the
spi_xfer_vector() API. When spi_ctrlr structure does not have
xfer_vector method, API calls write and read operations sequentially.
However the QCA40xx SPI driver has "forced" CS activation-inactivation
in xfer method, so individual operation will break CS after write
operation, making combined write-read cycle broken.
Adding xfer_vector method to spi_ctrlr is a simple fix to prevent this.
BUG=None
BRANCH=none
TEST=built and run on Gale
Change-Id: I2258e563d0793bcacd626f78b8e96b3649a8e4a4
Signed-off-by: Patrick Georgi <pgeorgi@chromium.org>
Original-Commit-Id: 88a8824951cef4fe293dfa6e3a1a837ae07b6156
Original-Change-Id: I031e85ce5b847353cb1084f6f68b2af8c6f702e1
Original-Signed-off-by: Yuji Sasaki <sasakiy@google.com>
Original-Reviewed-on: https://chromium-review.googlesource.com/433439
Original-Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Original-Reviewed-by: Kan Yan <kyan@google.com>
Reviewed-on: https://review.coreboot.org/18297
Tested-by: build bot (Jenkins)
Reviewed-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Reviewed-by: Martin Roth <martinroth@google.com>
Adds the necessary plumbing for acpi_device_path() to find the LPC
bridge on the AMD Family14 northbridge with an SB800 southbridge.
This is necessary for TPM support since the acpi path to the LPC bridge
(_SB.PCI0.ISAB) doesn't match the built-in default in tpm.c
(_SB.PCI0.LPCB).
Change-Id: I1ba5865d3531d8a4f41399802d58aacdf95fc604
Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>
Reviewed-on: https://review.coreboot.org/18402
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Tested-by: build bot (Jenkins)
With recent change (a4b11e5c90: soc/intel/skylake: Perform CPU MP Init
before FSP-S Init) to perform CPU MP init before FSP-S init, suspend
resume is currently broken for all skylake/kabylake boards. All the
skylake/kabylake boards store external stage cache in TSEG, which is
relocated post MP-init. Thus, if FSP loading and initialization is
done after MP-init, then ramstage is not able to:
1. Save FSP component in external stage cache during normal boot, and
2. Load FSP component from external stage cache during resume
In order to fix this, ensure that FSP loading happens separately from
FSP initialization. Add fsp_load callback for pre_mp_init which ensures
that the required FSP component is loaded/saved from/to external stage
cache.
BUG=chrome-os-partner:63114
BRANCH=None
TEST=Verified that 100 cycles of suspend/resume worked fine on poppy.
Change-Id: I5b4deaf936a05b9bccf2f30b949674e2ba993488
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/18414
Tested-by: build bot (Jenkins)
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Add a function to allow FSP component loading separately from silicon
initialization. This enables SoCs that might not have stage cache
available during silicon initialization to load/save components from/to
stage cache before it is relocated or destroyed.
BUG=chrome-os-partner:63114
BRANCH=None
TEST=Compiles successfully.
Change-Id: Iae77e20568418c29df9f69bd54aa571e153740c9
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/18413
Tested-by: build bot (Jenkins)
Reviewed-by: Subrata Banik <subrata.banik@intel.com>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
poppy board uses Maxim 98927 speaker codec and Realtek RT5663
for headset. Select the apropriate NHLT blobs to be packaged in CBFS.
Also, generate the required ACPI NHLT table for codec and the supported
topology in poppy.
BUG=chrome-os-partner:62051
BRANCH=None
TEST=With the required driver support in kernel verify that
the Audio plays on on-board speakers and headset, recording
works from on-board mics and headset mics.
Change-Id: I98c65038b35fe99a661807de0766e6eac2c80eed
Signed-off-by: M Naveen <naveen.m@intel.com>
Signed-off-by: Rizwan Qureshi <rizwan.qureshi@intel.com>
Signed-off-by: Naresh G Solanki <naresh.solanki@intel.com>
Reviewed-on: https://review.coreboot.org/18214
Tested-by: build bot (Jenkins)
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Wacom I2C driver does the same thing as I2C HID driver, other than
defining macros for Wacom HID. Instead of maintaining two separate
drivers providing the same functionality, update all wacom devices to
use generic I2C HID driver.
BUG=None
BRANCH=None
TEST=Verified that ACPI nodes for wacom devices are unchanged.
Change-Id: Ibb3226d1f3934f5c3c5d98b939756775d11b792c
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/18401
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
Enable Fast-Plus speed for the touchscreen device so it can
be used at 1MHz instead of 400KHz.
BUG=chrome-os-partner:61277
TEST=manual testing on Eve P1, needs backported kernel patches
to actually make use of any I2C speed other than 400KHz
Change-Id: I3f44ff4a02a02a7b05e69ad54d4c6d60e5878393
Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: https://review.coreboot.org/18397
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins)
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Add the audio devices to Eve mainboard:
- Describe Maxim 98927 speaker amps and RT5663 headphone codec
in ACPI so they can be enumerated by the OS.
- Supply NHLT binaries for MAX98927, RT5663, and DMIC_4CH.
BUG=chrome-os-partner:61009
TEST=manual testing on Eve P1 with updated kernel to ensure that
both speakers and headset are functional. DMIC support is
is still being worked on and is not yet functional.
Change-Id: I5243e35d159a0ed15c6004e94ba5a50b28cff0a9
Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: https://review.coreboot.org/18398
Reviewed-by: Furquan Shaikh <furquan@google.com>
Tested-by: build bot (Jenkins)
Created using autoport plus some manual work and copying from G505S to
account for the non-H8 EC.
This model uses the same ENE KB9012 EC as the G505S.
Tested:
- Mainboard variant with 8GB Elpida DDR3
- SeaBIOS payload
- Booting into Linux 4.9.6 with Debian/unstable installed on the
internal HDD/SDD slot
- Native raminit
- Both native VGA init and option rom VGA init
- Basic TPM functionality (auto-detection and RNG)
- Battery status readout
- Basic ACPI functions (power button event; power-off; reboot)
- thinkpad-acpi hotkey functions
- thinkpad-acpi LED control (red thinkpad LED)
- Suspend to RAM and resume works
- Mini displayport output works
Known issues:
- Patches needed for EC battery support
https://review.coreboot.org/#/c/18348/https://review.coreboot.org/#/c/18349/
- No thermal zone since temperature sensing is not H8-compatible
and needs to be reverse engineered.
Not tested:
- msata/wwan (probably works)
Signed-off-by: Tobias Diedrich <ranma+coreboot@tdiedrich.de>
Change-Id: I52bc4515277e5c18afbb14a80a9ac788049f485c
Reviewed-on: https://review.coreboot.org/18351
Tested-by: build bot (Jenkins)
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Since we are not using gpio regulators on reef anymore, remove the
selection from Kconfig as well.
BUG=None
BRANCH=None
TEST=Compiles successfully.
Change-Id: Iae7d88dec3ac476d65b292f97a6ba3add71ce07a
Signed-off-by: Furquan Shaikh <furquan@chromium.org>
Reviewed-on: https://review.coreboot.org/18399
Tested-by: build bot (Jenkins)
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
This mainboard uses two i210 Ethernet controller. Therfore we enable the
usage of the i210 driver and have to provide a function to search for a
valid MAC address for all i210 devices by using Siemens hwilib.
Change-Id: I36246cdef987fcece15a297ebb2f41561fca1f69
Signed-off-by: Mario Scheithauer <mario.scheithauer@siemens.com>
Reviewed-on: https://review.coreboot.org/18380
Tested-by: build bot (Jenkins)
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Not selecting the Kconfig option `GENERATE_SMBIOS_TABLES` the build
fails with the error below.
```
CC ramstage/ec/lenovo/h8/h8.o
src/ec/lenovo/h8/h8.c:201:2: error: unknown field 'get_smbios_strings' specified in initializer
.get_smbios_strings = h8_smbios_strings,
^
src/ec/lenovo/h8/h8.c:201:2: error: initialization from incompatible pointer type [-Werror]
src/ec/lenovo/h8/h8.c:201:2: error: (near initialization for 'h8_dev_ops.read_resources') [-Werror]
cc1: all warnings being treated as errors
```
So add the appropriate preprocessor guards to fix the build error.
Change-Id: I3baed452d422539a805c628a8c4a6a8c2a809317
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: https://review.coreboot.org/17770
Tested-by: build bot (Jenkins)
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Using x86 RDRAND instruction, two functions are supplied to
generate a 32bit or 64bit number.
One potential usage is the sealing key generation for SGX.
BUG=chrome-os-partner:62438
BRANCH=NONE
TEST=Tested on Eve to generate a 64bit random number.
Change-Id: I50cbeda4de17ccf2fc5efc1fe04f6b1a31ec268c
Signed-off-by: Robbie Zhang <robbie.zhang@intel.com>
Reviewed-on: https://review.coreboot.org/18362
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Currently the USB OC pins definition only being defined up to OC3.
For PCH-H, OC4 and OC5 are needed, so add both into OC pin enum.
Changes is being verified and booted to Yocto with Saddle Brook.
Change-Id: Idaed6fa7dcddb9c688966e8bc59f656aec2b26eb
Signed-off-by: Teo Boon Tiong <boon.tiong.teo@intel.com>
Reviewed-on: https://review.coreboot.org/18364
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Setting both bits 27 and 7 of PCH register PMSYNC_CFG (PMSYNC
Configuration; offset 0x33c8) causes pre-OS display init to fail
on HSW-U/Lynxpoint and BDW-U ChromeOS devices when the VBIOS/GOP
driver is run after the register is set. A re-examination of
Intel's reference code reveals that bit 7 should be set for the
LP PCH, and bit 27 for non-LP, but not both simultaneously.
The previous workaround was to disable the entire power optimizer
section via a Kconfig option, which isn't ideal.
Test: unset bit 27 of PMSYNC_CFG and boot google/lulu,
observe functional pre-OS video output
Change-Id: I446e169d23dd446710a1648f0a9b9599568b80aa
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/18385
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth@google.com>
Reviewed-by: Duncan Laurie <dlaurie@chromium.org>
We've been able to narrow down the problem to a single register/
single bit, so revert this commit and address the problem in a
follow-on commit.
This reverts commit 0f2025da0f.
Change-Id: I780f9ea2976dd223aaa3e060aef6e1af8012c346
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/18384
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth@google.com>
It explains the prerequisites to run the script, some
background on how to setup the computer running the script,
and the board it gathers the information from.
That information is too long to fit inside the script's
help.
Change-Id: Iecba7310ff1583149c02728e955716775bcbbdc4
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@no-log.org>
Reviewed-on: https://review.coreboot.org/6660
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth@google.com>
Move code common code from each variant's mainboard.asl into
common ACPI code for all variants (like google/auron). This also
adds the _PRW method for the LID0 device for falco and peppy, which
omitted the function when they were originally upstreamed.
See Chromium commit c8b41f7, falco: Add _PRW for LID0 ACPI Device
Change-Id: I7f5129340249a986f5996af37c01ccbde8d374e8
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/18368
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martinroth@google.com>
Add a test in case we have a DIMM2 not populated but DIMM3 is.
Change-Id: I14f82afe03884740570838e7b2771233356c518d
Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr>
Reviewed-on: https://review.coreboot.org/18386
Tested-by: build bot (Jenkins)
Reviewed-by: Nico Huber <nico.h@gmx.de>
Coverity is detecting 'sp' as a variable which has not been initialized.
This is obviously not correct, so this patch *TRIES* to mark it as false
I'm not positive that this will work because the annotation needs to go
on the line above the error, but this error is inside of a # define.
Does the whole #define count as one line? Can it go on the line
above the #define in the .h file? Does it have to precede every line
where the #define is used? The documentation doesn't make this clear.
Should suppress coverity issues: 1368525 & 1368527
uninit_use: Using uninitialized value sp.
Change-Id: Ibae5e206c4ff47991ea8a11b6b59972b24b71796
Signed-off-by: Martin Roth <martinroth@google.com>
Reviewed-on: https://review.coreboot.org/18247
Tested-by: build bot (Jenkins)
Reviewed-by: Jonathan Neuschäfer <j.neuschaefer@gmx.net>
Apply the measured rise and fall times for I2C bus 1 on Eve
so it can be tuned properly for 400KHz operation.
BUG=chrome-os-partner:63020
TEST=verify I2C1 bus speed with a scope
Change-Id: I32b5aa460ea35aadca7f3d52324a64880764919f
Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: https://review.coreboot.org/18396
Tested-by: build bot (Jenkins)
Reviewed-by: Furquan Shaikh <furquan@google.com>
Currently UART0 GPIOs are being put into native mode during FSP-S
stage, so have ramstage re-configure them back to regular GPIO mode.
GPP_C8 does not seem to be functioning properly when routed to the
APIC, possibly due to the UART0 being enabled even though it is unused,
which is required because UART0 is PCI 1e.0 and so must be present for
other 1e.x functions to be enumerated. Instead, use this pin as a GPIO
interrupt so it will be routed through the GPIO controller at IRQ 14.
GPP_C9 was inverted and was only working because the pin was being
re-configured in FSP-S.
Also export the reset gpio as a device property so it can be used by
the kernel driver, which will stop it from complaining at boot.
BUG=chrome-os-partner:61233
TEST=verify that the interrupt and device is functional in the OS
Change-Id: Iaf9efbf50a13a981c6a9bbd507475777837e9c12
Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: https://review.coreboot.org/18395
Tested-by: build bot (Jenkins)
Reviewed-by: Furquan Shaikh <furquan@google.com>
There is an enable_s0ix config option in the devicetree that should
be used to disable it when not set:
- do not export C8/C9/C10 C-states in _CST
- do not enable SLP_S0 in FSP
BUG=chrome-os-partner:58666
TEST=test on eve board to ensure that OS only sees 3 ACPI C-states
instead of 6 and that it no longer attempts to enter C10
Change-Id: I90e4dc776d1d17d0b700cda63c8476786cd2e4ff
Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: https://review.coreboot.org/18394
Tested-by: build bot (Jenkins)
Reviewed-by: Furquan Shaikh <furquan@google.com>
Add support for more ACPI features in the generic SPI ACPI
driver so it can be flexible enough to support more devices,
or devices in different configurations.
- add a wake pin
- add support for using IRQ GPIO instead of PIRQ
- add power resource support with enable and reset gpios
BUG=chrome-os-partner:61233
TEST=ensure existing SSDT generation is unchanged,
and test that new features generate expected code
Change-Id: Ibe37cc87e488004baa2c08a369f73c86e6cd6dce
Signed-off-by: Duncan Laurie <dlaurie@chromium.org>
Reviewed-on: https://review.coreboot.org/18393
Tested-by: build bot (Jenkins)
Reviewed-by: Furquan Shaikh <furquan@google.com>