summaryrefslogtreecommitdiff
path: root/src/soc/amd
AgeCommit message (Collapse)Author
2020-08-24soc/amd/common: Move interrupt and wake status clearJosie Nordrum
Move interrupt status and wake status clearing to after GPIO config so that configuration does not incorrectly set interrupt or wake status. i.e. when PULL_UP is configured on a pad, it incorrectly sets in the interrupt status bit. Thus, the interrupt status bit must be cleared after initial pad configuration is complete. BUG=b:164892883, b:165342107 TEST=None BRANCH=None Signed-off-by: Josie Nordrum <josienordrum@google.com> Change-Id: If4a5db4bfa6a2ee9827f38e9595f487a4dcfac2c Reviewed-on: https://review.coreboot.org/c/coreboot/+/44640 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-24soc/amd/picasso: Add console & timestamp buffers to psp_verstageMartin Roth
Create areas for console & timestamp data in psp_verstage and pass it to the x86 to save for use later. BUG=b:159220781 TEST=Build & Boot trembyle Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I41c8d7a1565e761187e941d7d6021805a9744d06 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42830 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-08-24soc/amd/picasso: Store ddr_frequency in MT/sRob Barnes
This field eventually gets interpreted as MT/s by SMBIOS instead of MHz. Translate from Mhz to MT/s by multiplying by 2. BUG=b:154654737 TEST=dmidecode -t 17 matches expected speed Change-Id: I51b58cb0380f2a2bf000347395ac918ac0717060 Signed-off-by: Rob Barnes <robbarnes@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44540 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-23soc/amd/picasso/romstage: Set HDA disable UPD if controller disabledFelix Held
FSP has recently added support for a UPD switch to disable the non-GPU HD Audio controller. This change adds the coreboot side of the feature. To avoid having two HD Audio enable options, the value of the hd_audio_enable UPD is determined by the enable state of the non-GPU HD Audio controller in the platform devicetree. BUG=b:158535201,b:162302028 BRANCH=zork TEST=With the corresponding FSP change applied the non-GPU HD Audio device is hidden when switched off in devicetree and remains present and functional when switched on in devicetree. Change-Id: Ib2965e0742f4148e42a44ddad8ee05f0c4c7237e Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44680 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Matt Papageorge <matthewpapa07@gmail.com> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-21soc/amd/picasso: If using VBOOT, skip the APOB_NV region for ROMartin Roth
When booting from the RO region of a VBOOT enabled ROM, there shouldn't be a reliance on anything outside of the RO section. This includes the APOB_NV region (similar to the MRC cache region). By skipping the region when setting up the BIOS Directory table, the PSP won't try to use the region when booting. The APOB_NV region is still used for the VBOOT RW sections. BUG=b:158363448 TEST=Build RO with no APOB_NV region. Dump the BDT and verify that it's not in RO, but is in RW_A & RW_B. Boot into recovery. Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: I13c35ba8a2331492744d2acf257db15e4a53102a Reviewed-on: https://review.coreboot.org/c/coreboot/+/44046 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-21soc/amd/common: add rudimentary ATIF supportAaron Durbin
The Linux kenerl driver for AMD gpu currently has a floor value of 12 for brightness settings (AMDGPU_DM_DEFAULT_MIN_BACKLIGHT). AMD indicates they did this because they were concerned with certain panels flickering at lower backlight values. However, for unaffected panels it's desirable to be able to have the panel "turn off" at the lowest backlight setting. The only way to do that is to provide ATIF bindings that indicate backlight range. Option SOC_AMD_COMMON_BLOCK_GRAPHICS_ATIF is added to provide a full range for the backlight setting. If needed, this path can be built upon for fuller support, but for the time being this is the only thing necessary to make the backlight be full range. BUG=b:163583825 Change-Id: If76801a8daf6a5e56ba7d118956f3ebce74e567a Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44642 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-20soc/amd/acpi: Move ACPI IVRS generation to corebootJason Glenesk
Add code for IVRS generation to coreboot. Publish coreboot generated structure rather than IVRS generated by FSP binary. Reference Doc: 48882_IOMMU_3.05_PUB.pdf BUG=b:155307433 TEST=Boot trembyle to shell and extract and compare IVRS tables and make sure they cover the same devices. Change-Id: I693f4399766c71c3ad53539634c65ba59afd0fe1 Signed-off-by: Jason Glenesk <jason.glenesk@amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43804 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-08-19soc/amd/picasso: log and print GPIO wake eventsAaron Durbin
Capture the GPIO subsystem wake state and add events to the eventlog. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I7f10bf4599ea7928cc87b6b10ac11a7c30e58406 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44535 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-19soc/amd/common: add gpio subsystem event reportingAaron Durbin
In order to log gpio events for wake purposes the state of the gpio subsystem should be snapshotted. Add the ability to capture state of gpio subystem as well as saving up to 16 gpios that indicate their wake status. Likewise, provide the eventlog additions based on state. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I49fca56c87543aa8aad0eb7da5c5cb570c4349d5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44534 Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-19soc/amd/picasso: Use cbfs to locate the AMD firmwareMartin Roth
Switch from locating the AMD firmware in the RW_A & RW_B regions with their hardcoded locations to using CBFS to find them. They still need to be at the hardcoded locations so that we can set the location inside the binary, but instead of just setting the pointer directly to them, we now search for them with cbfs. BUG=b:154441227 TEST=Boot & verify that binaries are located in both RW-A & RW-B Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I27b0593e0db7a9e6ba9b0633ac93b4d93954f002 Reviewed-on: https://review.coreboot.org/c/coreboot/+/42831 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Eric Peers <epeers@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-19soc/amd/picasso: Remove now unused #defineMartin Roth
This #define wasn't removed when the tests were removed, so get rid of it now. BUG=None TEST=Build Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: Ie0005b6ee97037bf3dfb80f0c2408d8bd9ee9633 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44537 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-08-18soc/amd/picasso: fix GPE snapshot stateAaron Durbin
In CB:44488 the cbmem addition was re-filling the object when it should be memcpy()ing from static object. Correct that oversight. The side effect from the previous implementation would be if FSP-M modified the GPE state. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I158a89ae28431896fa9b5789292000fcbf0b066d Reviewed-on: https://review.coreboot.org/c/coreboot/+/44533 Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-18elog: rename ELOG_WAKE_SOURCE_GPIO to ELOG_WAKE_SOURCE_GPEAaron Durbin
The wake source macro for GPE events was using 'GPIO'. However, current usage is really all GPEs. Therefore, provide clarity in the naming in order to allow for proper GPIO wake events that are separate from the ACPI GPE block. BUG=b:159947207 Change-Id: I27d0ab439c58b1658ed39158eddb1213c24d328f Signed-off-by: Aaron Durbin <adurbin@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44527 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-18src: Remove unused 'include <delay.h>'Elyes HAOUAS
Change-Id: I6afea5c102299e570378a1656d3dcd329a373399 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44093 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-18src: Remove unuse '<timestamp.h>Elyes HAOUAS
Change-Id: I4fa03c4576bb0256b73f1d36ca840e120b750a74 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44099 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2020-08-18src: Remove unneded whitespace before tabElyes HAOUAS
Also remove unneded tab in 'picasso/Makefile.c' file. Change-Id: Id25b2d308645c449c205b3a946f89b6b6de62a47 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44441 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Georgi <pgeorgi@google.com>
2020-08-17soc/amd/common: add GPE event logsAaron Durbin
GPE events were not be recorded in the eventlog. Add those to the eventlog when the status register indicates those events. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: Ifb3167fd24f2171b2baf1a65eb81a318eb3e7a86 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44489 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/picasso: snapshot chipset state early in boot sequenceAaron Durbin
Previously the chipset state was snapshotted very late in the boot (ramstage). Instead start gathering the state early in romstage prior to calling any FSP routines so there's a clean snapshot. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: Id41686e6cdf5bebc9633b514b4121b0447f9be2d Reviewed-on: https://review.coreboot.org/c/coreboot/+/44488 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/stoneyridge: remove unused soc_power_reg objectAaron Durbin
Now that no one is consuming this object, remove its definition. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: Ib5aeec1733b6c9fa49569e30c4c369f70af0939c Reviewed-on: https://review.coreboot.org/c/coreboot/+/44487 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/picasso: remove unused soc_power_reg objectAaron Durbin
Now that no one is consuming this object, remove its definition. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I60e4a9bfdf2752923f46a35aaab7034f9fa9b309 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44486 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/common: removed unused functionsAaron Durbin
Now that all users of the functions manipulating global state and using soc-specific objects are removed remove those functions. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I18c4c8b0c7852dde8cf0b6b3f11e43e15c3ce155 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44485 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/picasso: use new ACPI helper functions from commonAaron Durbin
Transition the current call sequence to using the newly added common ACPI helper functions. Semantically, the expectations are that this sequence is the equivalent of previous acpi_clear_pm1_status(). However, in subsequent patches picasso will be snapshotting state way sooner than ramstage. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I34e2ba7c5cd123b98c39291537e74175ec043e85 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44484 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/stoneyridge: use new ACPI helper functions from commonAaron Durbin
Transition the current call sequence to using the newly added common ACPI helper functions. Semantically, the expectations are that this sequence is the equivalent of previous acpi_clear_pm1_status(). BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: Id3ae19013c68d2c97b084046f600596ecc462374 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44483 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/common: add acpi_fill_gnvs()Aaron Durbin
In order to reduce code duplication provide an acpi_fill_gnvs() helper function. Intent is to move stoneyridge and picasso over to using this common implementation instead of duplicating it. BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: I21c6e2c24eaf42f31ae57c05df7f633d7dc266d9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44482 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-17soc/amd/common: add single function ACPI PM1 GPE helpersAaron Durbin
The existing code in common/block/acpi is mixing multiple operations: saving things to cbmem in common code but then soc code uses that information, reliant upon soc-specific struct soc_power_reg object, and only saving/snapshotting ACPI registers very deep in ramstage. To unwind the above provide some functions that are more targeted: - Add struct acpi_pm_gpe_state object - Add acpi_fill_pm_gpe_state() - Add acpi_pm_gpe_add_events_print_events() - Add acpi_clear_pm_gpe_status() BUG=b:159947207 Signed-off-by: Aaron Durbin <adurbin@chromium.org> Change-Id: Ia7afed2861343802b3c78728784f7cfaf6f53f62 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44481 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-15soc/amd/picasso: use FADT devicetree configuration optionsMartin Roth
Two of the items in the FADT ACPI table frequently are partially board- specific, so let's make it easy to update them via devicetree settings. - fadt_boot_arch 0="legacy free" which while reasonable, probably isn't what will be wanted by most mainboards, so this should generally get updated in the specific devicetree. - In fadt_flags all chipset-specific flags get set while the mainboard has to set all other flags that it needs to have set. This patch changes the default for fadt_boot_arch. Change-Id: I6e8d0c60cadfdd24b6926703b252abbc56d436de Signed-off-by: Martin Roth <martinroth@chromium.org> Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43418 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-08-14soc/amd/picasso/acpi: Set missing RTC offsetsRaul E Rangel
The RTC Date Alarm and RTC AltCentury fields are supported on picasso. These get consumed by the linux kernel: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/third_party/kernel/v5.4/drivers/rtc/rtc-cmos.c;l=1243 BUG=b:160277722 TEST=Boot kernel and make sure suspend stress test works. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ie83d7e0a06107a6de095f3e4c521d91e90920c0b Reviewed-on: https://review.coreboot.org/c/coreboot/+/44448 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-13drivers/intel/fsp2_0: don't select FSP_USES_CB_STACK on FSP 2.0 platformFelix Held
soc/amd/picasso selected FSP_USES_CB_STACK even though it is FSP 2.0 based, so it doesn't reuse coreboot's stack, but sets up its own stack. In contrast to all other FSP 2.0 based platforms, this stack isn't in the CAR region, since AMD Picasso doesn't support CAR and the DRAM is already available when the x86 cores are released from reset. Selecting FSP_USES_CB_STACK ended up doing the right thing, but is semantically wrong. Instead of wrongly selecting FSP_USES_CB_STACK in soc/amd/picasso we take the corresponding code path if ENV_CACHE_AS_RAM is false which is only the case for non-CAR platforms. BUG=b:155501050 TEST=Timeless build results in an identical binary for amd/mandolin, asrock/h110m-dvs and intel/coffeelake_rvp11 which cover all 3 cases here. Change-Id: Icd0ff8e17a535e2c247793b64f4b0565887183d8 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44406 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2020-08-12soc/amd/common/espi_util: rename espi_check_statusFelix Held
espi_poll_status describes better what the function actually does, since it polls the status register instead of just doing a single read to check. Change-Id: I0feeef5504bd911e1fb0a00d4f4c546df3548db2 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44354 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-11amd/picasso/acpi: Add power resources for UART0Kangheui Won
Follow-up for a31a769 - "amd/picasso/acpi: Add power resources for I2C and UART". Now PSP properly handles UART0 D3, we can shutdown UART0. BUG=b:158772504 TEST=suspend_stress_test for 50 cycles, * echo 1 > /sys/module/acpi/parameters/aml_debug_output * dmesg | grep FUR to check on&off for FUR0 [ 2413.647500] ACPI Debug: "AOAC.FUR0._OFF" [ 2413.736265] ACPI Debug: "AOAC.FUR0._ON" Change-Id: I25457e18b69d28a83e42c2fe02b45a3979ad58cd Signed-off-by: Kangheui Won <khwon@chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44266 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-08-11soc/amd/picasso: Correct processor ACPI scopeJason Glenesk
Change namespace from _PR to _SB. Cq-Depend: chrome-internal:3208104 BUG=b:153242529 TEST=Boot a trembyle with change applied and dump SSDTs to ensure processors are in _SB scope. Change-Id: I534f02dc50756759da945cf64d5b3623b0ec9db1 Signed-off-by: Jason Glenesk <jason.glenesk@amd.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44325 Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-11soc/amd/common/espi_util: espi_send_command: improve error messageFelix Held
It's only an error if bits other than ESPI_STATUS_DNCMD_COMPLETE are set in the status register. If ESPI_STATUS_DNCMD_COMPLETE isn't set, the command failed, so we expect that one to be set. Change-Id: I6f1fb5a59b1ecadd6724a07212626f21fb90e7e7 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44352 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-11soc/amd/common/espi_util: espi_std_io_decode: fix edge case bugFelix Held
When address and data register for the SIO control register access is passed as one I/O region with a size of 2, the corresponding special decode enable register should be used instead of a generic one to save the rather limited generic ones for other decode ranges. Change-Id: Ie54ff6afa2bd2156f7b3a3cf83091f1f932b6993 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44351 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-11soc/amd/common/espi_util: simplify espi_std_io_decode functionFelix Held
We can just return at all places where the ret variable was written before its value gets returned at the end of the function. Change-Id: Id87f41c0d9e3397879ac3d15b13179cca1a1263f Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44350 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-11soc/amd/common/espi_util: make decode enable parameter uint32_tFelix Held
Since this is a bit mask applied to the raw value of a 32 bit register, this should be a 32 bit unsigned type. Change-Id: I9d9930963d8c827a84dc1f67e2f2fa8f95ab40f2 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44349 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-11soc/amd/common/espi_util: make reg parameter unsignedFelix Held
Th register number passed to the low level read/write functions should never be negative. Change-Id: I5d7e117b3badab900d030be8e69ded026d659f8a Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44348 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-08-11soc/amd/stoneyridge/acpi: clean up global NVSFelix Held
Some fields in GNVS seem to be copied over from Apollolake to Stoneyridge. This patch removes the unused fields. Change-Id: I135c4a4547668fe67e74d0ea9ae3a03c3687375f Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44184 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-08-07soc/amd/picasso/acpi: remove AOAC device enables from global NVSFelix Held
These values in GNVS are written, but never read/used. aoac.asl contains proper ACPI power management functions for the AOAC devices that directly access the state from the device's registers instead of relying on cached values in GNVS, so the corresponding GNVS entries can be dropped. BUG=b:161165393 TEST=Mandolin still boots and dmesg shows no new ACPI errors. Change-Id: Iee78df215308bd9b656228be787fac121d10ca99 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44245 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-08-05mb/google/zork: keep the c-state IO base address alignmentChris Wang
Align the C-state MSR value of BSP with AGESA. BUG=b:162705221 BRANCH=none TEST=Check the MSR value is correct and BSP can enter CC6 with AVT tool Signed-off-by: Chris Wang <chris.wang@amd.corp-partner.google.com> Change-Id: Ib98d34af518439d338326446c20601867ad31690 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44135 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-08-04soc/amd/picasso/acpi: clean up global NVSFelix Held
Some fields in GNVS seem to be copied over from Apollolake to Stoneyridge to Picasso. This patch removes the unused fields. BUG=b:161165393 TEST=Mandolin still boots and dmesg shows no new ACPI errors. Change-Id: I8c6b580543089bf0180a7caeb9e6a47dc4ed4a1d Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/44154 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-08-03soc/amd/picasso: set is_rv to 1 for RV familyAkshu Agrawal
RV has difference in clk framework. In RV we get a 48Mhz fixed clk, while in ST we had 25Mhz, 48mhz clocks and a Mux to select between them. To differentiate set the fmw property to 1 for boards using RV family of SoC. Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com> BUG=b:158906189 TEST=rt5682 driver get the correct clk and tested audio playback Change-Id: I685ded1607c2c7edc5e48f0bada258ebde192bb8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44009 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-08-03Change all assert(0) to BUG()Julius Werner
I would like to make assertions evaluate at compile time where possible, but sometimes people used a literal assert(0) to force an assertion in a certain code path. We already have BUG() for that so let's just replace those instances with that. Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I674e5f8ec7f5fe8b92b1c7c95d9f9202d422ce32 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44047 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-30mb/amd,google/mandolin,zork: Set EFS SPI platform configMatt Papageorge
Set platform defaults for SPI settings in Kconfig for EFS. BUG=b:158755102 TEST=Build and boot test on Tremblye and Morphius. Verify values in output image in a hex editor. Measure 1st x86 timestamp, perf improves by over a second. Change-Id: I765dada14700f4800263d2d3844af07fad0e5b71 Signed-off-by: Matt Papageorge <matthewpapa07@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43303 Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-30amd/common/block/spi: Add EFS SPI configurations to KconfigMatt Papageorge
The Embedded Firmware Structure should contain SPI speed, mode and Micron support for the PSP to program. Add Kconfig options to specify these values to use for future platform changes. BUG=b:158755102 TEST=Test menuconfig and platform build for Trembyle and Mandolin. Signed-off-by: Matt Papageorge <matt.papageorge@amd.corp-partner.google.com> Change-Id: I78558fa3fa27c70820f0f3d636544127adab6f8b Reviewed-on: https://review.coreboot.org/c/coreboot/+/42567 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Martin Roth <martinroth@google.com> Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-30soc/amd/picasso: Split ops for internal and external PCIe GPP bridgesFurquan Shaikh
This change splits the device operations for internal and external PCIe GPP bridges so that the external bridges use `pciexp_scan_bridge()` instead of `pci_scan_bridge()`. `pciexp_scan_bridge()` is required for external GPP bridges to enable ASPM on downstream devices if supported. BUG=b:162352484 TEST=Verified on Trembyle: $ lspci -s 1:00.0 -vvv | grep ASPM LnkCap: Port #0, Speed 2.5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s <4us, L1 <64u ClockPM+ Surprise- LLActRep- BwNot- ASPMOptComp+ LnkCtl: ASPM L1 Enabled; RCB 64 bytes Disabled- CommClk L1SubCap: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1+ L1_PM_Substates+ L1SubCtl1: PCI-PM_L1.2+ PCI-PM_L1.1+ ASPM_L1.2+ ASPM_L1.1 Signed-off-by: Furquan Shaikh <furquan@google.com> Change-Id: Ice2aa3e4758adccf7b0b89d4222fc65a40761153 Reviewed-on: https://review.coreboot.org/c/coreboot/+/44016 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Eric Peers <epeers@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-07-29util/apcb: Strip SPD manufacturer informationRob Barnes
Strip manufacturer information from SPDs before injecting into APCB. This allows more flexibility around changing DRAM modules in the future. BUG=b:162098961 TEST=Boot, dump memory info Signed-off-by: Rob Barnes <robbarnes@google.com> Change-Id: I1bbc81a858f381f62dbd38bb57b3df0e6707d647 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43832 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-28soc/amd/picasso: Add controls for SMT and downcoringMarshall Dawson
BUG=b:159198385 TEST=confirm both using Mandolin Signed-off-by: Marshall Dawson <marshalldawson3rd@gmail.com> Change-Id: I91654817608ab62e4104959b8876333911b90175 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43299 Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-28src/soc/amd: Add include <types.h>Elyes HAOUAS
BIT(x) needs <types.h>. Change-Id: Icaeda969cae52d9c62d976db4ead0e734efa838c Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43706 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-28soc/amd/picasso: Enable VBNV_BACKUP_TO_FLASH for psp_verstageMartin Roth
Enable the Kconfig flag VBOOT_VBNV_CMOS_BACKUP_TO_FLASH for psp_verstage to save the vbnv data to the SPI rom. BUG=b:161366241 TEST=Boot Morphius, Read rom from SPI and extract the RW_NVRAM region. See that it's getting updated. Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: I0d4b92fa321a8409468b8d8fc40be0d4b57b664b Reviewed-on: https://review.coreboot.org/c/coreboot/+/43487 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-28soc/amd/picasso: Init SPI in psp_verstageMartin Roth
SPI needs to be initialized to save VBNV (Vboot Non-Volatile memory) to flash. BUG=b:159811539 TEST=Build & boot. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: Iebf3ed3f5d6be0dda717d91d5b2fbcf2a1cc43cc Reviewed-on: https://review.coreboot.org/c/coreboot/+/43308 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-07-28soc/amd/picasso/Makefile.inc: force an error if PSPBTLDR_FILE is not setRonald G Minnich
Currently, if PSPBTLDR_FILE is empty, the md5sum will hang forever on stdin, leading to the appearance of a hung script. This is confusing. There's no option to md5sum to say "you must use this file", so instead, use dd with if to ensure we at least get an error if the file is not found. Not optimal, but better than what we have now. Change-Id: Ia13035bc592bdf2a515dfd2e052ae9135e218612 Signed-off-by: Ronald G Minnich <rminnich@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43677 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jeremy Soller <jeremy@system76.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-27soc/amd: Use spi_writeX & spi_readX for all spi accessesMartin Roth
BUG=b:161366241 TEST=Build & boot Trembyle Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: Ied7789e9315c75174df9a686c831c5a969ce3bfe Reviewed-on: https://review.coreboot.org/c/coreboot/+/43773 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-07-27soc/amd/common: Move spi access functions into their own fileMartin Roth
Because there was a lot of discussion about the size increase, I also looked at the impact of calling the get_spi_bar() function vs reading spi_base directly and just not worring about whether or not spi_base was already set. Using the spi_base variable directly is 77 bytes bytes for all 6 functions. it's roughly double the size to call the function at 153 bytes. This was almost entirely due to setting up a call stack. If we add an assert into each function to make sure that the spi_base variable is set, it doubles from the size of the function call to 333 bytes. For my money, the function call is the best bet, because it not only protects us from using spi_base before it's set, it also gets the value for us (at least on x86, on the PSP, it still just dies.) BUG=b:161366241 TEST: Build Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I0b0d005426ef90f09bf090789acb9d6383f17bd2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43772 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2020-07-27soc/amd/picasso: Set __USER_SPACE__ for psp_verstageMartin Roth
Mark that psp_verstage is running in userspace so that it won't run the code in dcache_clean_all() and hang the system. BUG=b:161554141 TEST=Run board through a bunch of recovery cycles. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I936dcec18a2be9ec8636ce77bb0954f4fc58153e Reviewed-on: https://review.coreboot.org/c/coreboot/+/43785 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-27soc/amd/picasso: make USB over-current pin mapping configurableFelix Held
Neither the family 17h model 10-1Fh PPR nor the internal FSP source seems to have the mapping of the USB OC pins to the four bit values, so this is based on the information from the family 15h model 70-7Fh BKDG which also corresponds to what I'd have expected here. BUG=b:162010077 Change-Id: I581ef1d730e9d729d9849d7e73ef1c1b67b2c4cf Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43833 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-26soc/amd/common/block/psp/psp_smm.c: Add missing <string.h>Elyes HAOUAS
'memset' needs <string.h>. Change-Id: Idc1d72e92c97cd5139ae7439aadb575ef011129a Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42342 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-26soc/amd/common: Refactor and consolidate code for spi baseMartin Roth
Previously, the spi base address code was using a number of different functions in a way that didn't work for use on the PSP. This patch consolidates all of that to a single saved value that gets the LPC SPI base address by default on X86, and allows the PSP to set it to a different value. BUG=b:159811539 TEST=Build with following patch to set the SPI speed in psp_verstage. Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: I50d9de269bcb88fbf510056a6216e22a050cae6b Reviewed-on: https://review.coreboot.org/c/coreboot/+/43307 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-26soc/amd/picasso: Update postcode valueMartin Roth
I accidentally had the same value for two different postcode entries. Fix that. BUG=None TEST=Watch postcodes in psp_verstage Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: Id0bf18efc7e79278a21683c11a1084d2a7d97e6a Reviewed-on: https://review.coreboot.org/c/coreboot/+/43774 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-26Kconfig: Remove unnecessary choice namesMartin Roth
The only reason to use a named choice statement is if you plan on having the choice statement in multiple places. Since none of these are used in multiple places, we can get rid of the names. Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: Ie5f84e9dc38050234976bd193ac5fbf649e564f4 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43765 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-26smp/spinlock: Do not define barrier() globallyKyösti Mälkki
It's not stricly related to spinlocks. If defined, a better location should be found and the name collisions with other barrier() defined in nb/intel solved. Change-Id: Iae187b5bcc249c2a4bc7bee80d37e34c13d9e63d Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43810 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-26arch/x86: Move cpu_relax()Kyösti Mälkki
It's not related to spinlocks and the actual implementation was also guarded by CONFIG(SMP). With a single call-site in x86-specific code, empty stubs for other arch are currently not necessary. Also drop an unused included on a nearby line. Change-Id: I00439e9c1d10c943ab5e404f5d687d316768fa16 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43808 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-26src: Remove unused 'include <cbmem.h>'Elyes HAOUAS
Change-Id: Ib41341b42904dc3050a97b70966dde7e46057d6b Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43362 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com>
2020-07-26amd/picasso: rework USB2 PHY tune parameter handlingFelix Held
BUG=b:161923068 Change-Id: I67f23c0602e345fbd806e661a4462cf07f93ef64 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43783 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Furquan Shaikh <furquan@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-25soc/amd/picasso: don't apply unconfigured USB2 PHY tune parametersFelix Held
Since FSP pre-populates the UPD struct with the non-zero default values, coreboot shouldn't set them to zero in the case that they aren't configured in the board's devicetree. Since all parameters being zero is a valid case, this patch adds another devicetree option that applying the devicetree settings for the USB2 PHY tuning depends on being set. BUG=b:161923068 Change-Id: I66e5811ce64298b0644d2881420634a8ce1379d7 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43781 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-24soc/amd/picasso: mark usb2_phy_tune struct as packedFelix Held
Since the binary layout of this struct matters, it should be marked as packed. Since all struct elements are uint8_t, this shouldn't result in a different layout though. BUG=b:161923068 Change-Id: I6a390c3a3f35eaf8a72928b4cef0e9f405770619 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43780 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-24soc/amd/picasso/sb: Gate FCH AL2AHB clocksMatt Papageorge
Gate the A-Link to AHB Bridge clocks to save power. These are internal clocks and are unneeded for Raven/Picasso. This was previously performed within the AGESA FSP but this change relocates it into coreboot. BUG=b:154144239 TEST=Check AL2AHB clock gate bits at the end of POST before and after change with HDT. Change-Id: Ifcbc144a8769f8ea440cdd560bab146bf5058cf9 Signed-off-by: Matt Papageorge <matthewpapa07@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/42829 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-24soc/amd/picasso/fsp_params: add missing newline between functionsFelix Held
Change-Id: If5d798a410f092e0ce99c16c84809b6b2e30cc2e Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43779 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-24soc/amd/picasso/fsp_params: add asserts for descriptor countFelix Held
With the updated FSP UPD headers there are enough DXIO descriptor slots in the UPD, so we can now add asserts to make sure that the mainboard doesn't pass more DXIO/DDI descriptors than the UPD has slots for. This is part of the DXIO/DDI descriptor handling cleanup. BUG=b:158695393 Change-Id: Ia220d5a9d4ff11707b795b04662ff7eead4e2888 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43435 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-23soc/amd/common/gpio: Fix definition of GPIO_INT_ENABLE_STATUS_DELIVERYFurquan Shaikh
This change fixes the definition of `GPIO_INT_ENABLE_STATUS_DELIVERY` to use `GPIO_INT_ENABLE_DELIVERY` instead of `GPIO_INT_ENABLE_STATUS_DELIVERY`. Signed-off-by: Furquan Shaikh <furquan@google.com> Change-Id: I64d912200779875cf121cec4476fd39de74c0223 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43695 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-07-23amd/picasso: rename PCIe descriptor to DXIO descriptorFelix Held
Most of the DXIO descriptors are used to configure PCIe engines and lanes, but on Picasso system some of the DXIO lanes can also be configured as SATA or XGBE ports. Change-Id: I28da1b21cf0de1813d87a6873b8d4ef3c1e0e9dd Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43675 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-20src: Make HAVE_CF9_RESET set the FADT reset registerAngel Pons
All supported x86 chips select HAVE_CF9_RESET, and also use 0xcf9 as reset register in FADT. How unsurprising. We might as well use that information to automatically fill in the FADT accordingly. So, do it. To avoid having x86-specific code under arch-agnostic `acpi/`, create a new optional `arch_fill_fadt` function, and override it for x86 systems. Tested on Asus P8Z77-V LX2 with Linux 5.7.6 and Windows 10 at the end of the patch train, both operating systems are able to boot successfully. Change-Id: Ib436b04aafd66c3ddfa205b870c1e95afb3e846d Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43389 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Rudolph <siro@das-labor.org> Reviewed-by: Frans Hendriks <fhendriks@eltan.com>
2020-07-20src: Drop useless cache flush settings in FADTAngel Pons
They are ignored if the ACPI_FADT_WBINVD flag is set, which is required on current ACPI versions and only maintained for ACPI 1.0 compatibility. Tested on Asus P8Z77-V LX2 with Linux 5.7.6 and Windows 10 at the end of the patch train, both operating systems are able to boot successfully. Change-Id: Ief1219542ba71d18153b64180e0ff60bd1e7687b Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43390 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
2020-07-20soc/amd/stoneyridge: Select HAVE_CF9_RESETAngel Pons
Looks like some preparation is needed before reset. However, Picasso also needs some special handling and still selects this option without selecting HAVE_CF9_RESET_PREPARE. So, just add HAVE_CF9_RESET for now. Change-Id: I0c6da9a43a28dbee916fd6bda9ae380ebd619edf Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43388 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-20src: Never overwrite `fadt->flags`Angel Pons
Instead, just flip the desired bits using bitwise operations. As this is initially zero, the resulting value is the same. This allows flags to be set from anywhere regardless of execution order. Tested on Asus P8Z77-V LX2 with Linux 5.7.6 and Windows 10 at the end of the patch train, both operating systems are able to boot successfully. Change-Id: Icfd580a20524936cd0adac574331b09fb2aea925 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43387 Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-20amd/{hudson,stoney,picasso}: Drop PM2 settings from FADTAngel Pons
The PM2_CNT register block is no longer needed, as explained in some comments. While they may have been copy-pasted around a lot, they are at least true for Hudson, and it makes sense to assume that they are true for newer chipsets as well. As per the ACPI specification, version 6.3, section 4.8.1.3 (PM2 Control Register): This register block is optional, if not supported its block pointer and length contain a value of zero. Since the FADT struct defaults to zero in coreboot, we don't need to do anything to indicate PM2_CNT is not supported. So, drop unneeded values. Change-Id: Iabc7985c84aabe40ad98fdc9fc6ccbbab0a516c1 Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43381 Reviewed-by: Nico Huber <nico.h@gmx.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-20src: Drop useless PM1b settings from FADTAngel Pons
None of the currently-supported chips has PM1b_EVT nor PM1b_CNT event register blocks. According to the ACPI specification, version 6.3, sections 4.8.1.1 and 4.8.1.2 (PM1 Event/Control Registers): If the PM1b_EVT_BLK is not supported, its pointer contains a value of zero in the FADT. If the PM1b_CNT_BLK is not supported, its pointer contains a value of zero in the FADT. Since the FADT struct defaults to zero in coreboot, we don't need to do anything with PM1b for now. So, drop unneeded writes to PM1b fields. Tested on Asus P8Z77-V LX2 with Linux 5.7.6 and Windows 10 at the end of the patch train, both operating systems are able to boot successfully. Change-Id: Iff788b2ff17ba190a8dd9b0b540f1ef059a1a0ea Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43380 Reviewed-by: Patrick Rudolph <siro@das-labor.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-20src: Drop useless GPE1 settings from FADTAngel Pons
None of the currently-supported chips has a GPE1 block. The ACPI spec, version 6.3, section 4.8.1.6 (General-Purpose Event Registers) says: If a generic register block is not supported then its respective block pointer and block length values in the FADT table contain zeros. Since the FADT struct defaults to zero in coreboot, we don't need to do anything with GPE1 for now. So, drop the unneeded writes to GPE1 fields. Tested on Asus P8Z77-V LX2 with Linux 5.7.6 and Windows 10 at the end of the patch train, both operating systems are able to boot successfully. Change-Id: Iefc4bbc6e16fac12e0a9324d5a50b20aad59a6cd Signed-off-by: Angel Pons <th3fanbus@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43379 Reviewed-by: Patrick Rudolph <siro@das-labor.org> Reviewed-by: Michael Niewöhner Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-17soc/amd/picasso: Drop the addition of I2S machine device from ACP driverFurquan Shaikh
I2S machine device has its own driver now. So, this change drops the support for adding I2S machine device ACPI node from ACP driver. BUG=b:157708581 Change-Id: I9069d92ae991e05fddcc7d45a2fd21e98c3b0de8 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43544 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-07-17soc/amd/picasso: Add .scan_bus operation for ACP deviceFurquan Shaikh
This change adds `.scan_bus` device operation for ACP device to allow mainboards to add devices under it. BUG=b:157708581 Change-Id: I088bf81d7c7c5f59ade1d2f0dd24e5fc2b1ff876 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43542 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-17soc/amd/common/gpio: Add macro for PAD_NCFurquan Shaikh
This change adds a macro `PAD_NC` for configuring no-connect pads. This configures the pad as input with pull-down. Change-Id: I47c41c88ccfebe2c5dd9a24f85a120af9c8f56b5 Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43521 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-07-17soc/amd/picasso: Drop _INI and OSFL methodsFurquan Shaikh
This change drops _INI and OSFL methods under \_SB since they are not doing anything useful. _INI only calls OSFL and OSFL initializes OSVR if not already initialized and returns OSVR value. However, OSVR is not used anywhere and hence both these functions can be dropped. BUG=b:153879530 Signed-off-by: Furquan Shaikh <furquan@google.com> Change-Id: I4f3e1c93a855006cc115087fded20bfb76c1133e Reviewed-on: https://review.coreboot.org/c/coreboot/+/43515 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-17soc/amd/picasso: Move PMOD global variable to globalnvs.aslFurquan Shaikh
Global variable `PMOD` that stores the interrupt mode used by OS is required by all mainboards. This change moves the variable definition to globalnvs.asl under picasso. Additionally, ACPI spec says that BIOS should assume interrupt mode as PIC until _PIC() method is called by OS. Thus, this change also updates the default value of PMOD as 0 i.e. PIC mode. BUG=b:153879530 Signed-off-by: Furquan Shaikh <furquan@google.com> Change-Id: I731c03d965882281a7a23f55894451210ba72274 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43514 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-17soc/amd/picasso: Drop empty method CIRQFurquan Shaikh
This change drops empty method CIRQ() from pci_int.asl. BUG=b:153879530 Signed-off-by: Furquan Shaikh <furquan@google.com> Change-Id: Ib342dcbc52cfacbd73a8a50ee087d97562d94c97 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43513 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-17soc/amd/picasso: Use read-modify-write for ACP_I2S_PIN_CONFIGFurquan Shaikh
This change uses read-modify-write to update ACP_I2S_PIN_CONFIG instead of a write operation since the other bits in the register are reserved. Change-Id: Ic64e1907858ec293c5f759e627d19c00d748a30e Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43503 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-17soc/amd/picasso: Configure ACP_PME_EN and ACP_I2S_WAKE_ENAkshu Agrawal
This change adds support for configuring ACP_PME_EN and ACP_I2S_WAKE_EN using the mainboard setting for `acp_pme_enable` and `acp_i2s_wake_enable` in the devicetree. This is required to get I2S_Wake event on headset jack plug/unplug when using CODEC_GPI pad. BUG=b:146317284,b:161328042 Signed-off-by: Akshu Agrawal <akshu.agrawal@amd.com> Change-Id: I522d7497940f499fbc3181d866f2b44e979bba7a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/coreboot/+/1969104 Reviewed-by: Raul E Rangel <rrangel@chromium.org> Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43495 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-16soc/amd/picasso: remove unused fadt_pm_profile devicetree settingFelix Held
commit 56da63c3dc3f50cfac541c779b608e1bae9e635c removed overriding that field in the FADT. Change-Id: I0c8ff9ab125129dc856949c47a3a0c14e4109c73 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43417 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-16soc/amd/common: Don't get eSPI address from PCI if not on x86Martin Roth
Exclude lpc_get_spibase() on the PSP. This also simplifies the espi_get_bar() function. BUG=b:159811539 TEST=Build & boot trembyle; Verify address in PSP & x86 Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: I5927dd40610860b54bb35a7e5b03ddb731597745 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43468 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-16soc/amd/picasso: Get rid of VERSTAGE_SIZEMartin Roth
Currently, the code and data in psp_verstage is ~59K. Adding the code to save vbnv to the SPI rom increases that to 66K. Getting rid of VERSTAGE_SIZE allows verstage to grow as it needs to. BUG=b:161366241 TEST=Build & Boot Morphius with VBOOT_VBNV_CMOS_BACKUP_TO_FLASH enabled Signed-off-by: Martin Roth <martinroth@chromium.org> Change-Id: Ic6853b70073f9e781fc10402a2a47c9c8e0d49d3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43486 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-16soc/amd/stoneyridge: Remove unused SPI #definesMartin Roth
These #defines are not used, and conflict with #defines in amdblocks/spi.h BUG=None TEST=Build stoney platforms Signed-off-by: Martin Roth <martin@coreboot.org> Change-Id: I29b77a6b21a4deda6f28f5b057988cf3921540e2 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43485 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Rob Barnes <robbarnes@google.com> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2020-07-16soc/amd/picasso/acpi,mb/{zork,mandolin}: Stop clearing PciExpWakeStatusRaul E Rangel
The kernel already clears this: https://source.chromium.org/chromiumos/chromiumos/codesearch/+/master:src/third_party/kernel/v5.4/drivers/acpi/acpica/hwregs.c;l=390 No reason to have the firmware do it as well. BUG=b:153001807, b:154756391 TEST=Build Trembyle, boot, suspend, and resume and didn't see any ACPI errors. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ia5c79fb95dc885eaef8abc4257b6ba18c1ef1b66 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43428 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-16soc/amd/picasso,mb/{zork,mandolin}: Remove invalid UPWS variableRaul E Rangel
PMx0EE is not defined in the Picasso PPR. BUG=b:153001807, b:154756391 TEST=None Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I98caf0cd2d0bdcf19de2b945dcf74f5cf7354769 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43424 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-16soc/amd/picasso: Drop mainboard_romstage_entry_s3Furquan Shaikh
mainboard_romstage_entry_s3() was dropped from zork (CB:43476). This function call in picasso does not do anything and hence is being dropped. BUG=b:154351731 Signed-off-by: Furquan Shaikh <furquan@google.com> Change-Id: I10e15422d7eef5af9c19737c32e433718b6479d6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43477 Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-15soc/amd/picasso/acpi: Delete unused and invalid OperationRegionsRaul E Rangel
0xc50, 0xc52, 0xc6f don't exist on Picasso. The PCI config space registers define SATA and OHCI which are at the wrong bus locations. I just remove the whole section since it's not used. We never access the PCIe Error region, or the PM2 region either. BUG=b:153001807, b:154756391 TEST=Build Trembyle Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I98aee09770f1df9f553c94580c1ee00c06a9cec1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43427 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-15soc/amd/picasso/acpi: Remove old AOAC register definitionsRaul E Rangel
We no longer need this code. It's been added differently in CB:42473. BUG=b:153001807, b:154756391 TEST=Build Trembyle Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I6fe1e465f137ba6afbf9f0dbce501b5fc845e210 Reviewed-on: https://review.coreboot.org/c/coreboot/+/43426 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-15soc/amd/picasso/acpi: Remove invalid and unnecessary devicesRaul E Rangel
These devices are not referenced by anything else. BUG=b:153001807, b:154756391 TEST=None Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I6ea3c326247dce095b5ac1706dbc37f8b215a21e Reviewed-on: https://review.coreboot.org/c/coreboot/+/43425 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Furquan Shaikh <furquan@google.com>
2020-07-15amd/picasso: rework DXIO and DDI UPD handlingFelix Held
Turning the DXIO and DDI descriptor fields in the FSP_S_CONFIG struct into arrays allows to properly iterate over the fields. BUG=b:158695393 TEST=Mandolin still boots. Change-Id: I85debe4d52399e933768b89b665ff10c9f7779f8 Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43434 Reviewed-by: Aaron Durbin <adurbin@chromium.org> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-14soc/amd/common/block/include/amdblocks/acpi.h: Add missing <types.h>Elyes HAOUAS
BIT(x) needs <types.h> Change-Id: I5dc0d45567ae9879a7e12f2ccc48929d2abc9456 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43371 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
2020-07-14src: Remove unused 'include <cpu/x86/msr.h>'Elyes HAOUAS
Found using: diff <(git grep -l '#include <cpu/x86/msr.h>' -- src/) <(git grep -l 'IA32_EFER\|EFER_\|TSC_MSR\|IA32_\|FEATURE_CONTROL_LOCK_BIT\|FEATURE_ENABLE_VMX\|SMRR_ENABLE\|CPUID_\|SGX_GLOBAL_ENABLE\|PLATFORM_INFO_SET_TDP\|SMBASE_RO_MSR\|MCG_CTL_P\|MCA_BANKS_MASK\|FAST_STRINGS_ENABLE_BIT\|SPEED_STEP_ENABLE_BIT\|ENERGY_POLICY_\|SMRR_PHYSMASK_\|MCA_STATUS_\|VMX_BASIC_HI_DUAL_MONITOR\|MC0_ADDR\|MC0_MISC\|MC0_CTL_MASK\|msr_struct\|msrinit_struct\|soc_msr_read\|soc_msr_write\|rdmsr\|wrmsr\|mca_valid\|mca_over\|mca_uc\|mca_en\|mca_miscv\|mca_addrv\|mca_pcc\|mca_idv\|mca_cecc\|mca_uecc\|mca_defd\|mca_poison\|mca_sublink\|mca_err_code\|mca_err_extcode\|MCA_ERRCODE_\|MCA_BANK_\|MCA_ERRTYPE_\|mca_err_type\|msr_set_bit\|msr_t\|msrinit_t' -- src/) |grep '<' Change-Id: I45a41e77e5269969280e9f95cfc0effe7f117a40 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41969 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-14src: Remove unused 'include <stdint.h>Elyes HAOUAS
Found using: diff <(git grep -l '#include <stdint.h>' -- src/) <(git grep -l 'int8_t\|int16_t\|int32_t\|int64_t\|intptr_t\|intmax_t\|s8\|u8\|s16\|u16\|s32\|u32\|s64\|u64\|INT8_MIN\|INT8_MAX\|INT16_MIN\|INT16_MAX\|INT32_MIN\|INT32_MAX\|INT64_MIN\|INT64_MAX\|INTMAX_MIN\|INTMAX_MAX' -- src/) |grep -v vendorcode |grep '<' Change-Id: I5e14bf4887c7d2644a64f4d58c6d8763eb74d2ed Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/41827 Reviewed-by: Patrick Georgi <pgeorgi@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2020-07-13soc/amd/picasso: supply SMBIOS type 17Rob Barnes
Extract DRAM info from AMD_FSP_DMI_HOB and store it as mem_info in cbmem with id CBMEM_ID_MEMINFO. Subsquently extract mem_info objects from cbmem to build SMBIOS type 17 tables. BUG=b:148277751,b:160947978 TEST=dmidecode -t 17 BRANCH=none Change-Id: Iacedbb017d19516674070f89ba0aa217f55383e3 Signed-off-by: Rob Barnes <robbarnes@google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/43351 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>