summaryrefslogtreecommitdiff
path: root/src/soc/amd
AgeCommit message (Collapse)Author
2022-03-08timestamps: Rename timestamps to make names more consistentJakub Czapiga
This patch aims to make timestamps more consistent in naming, to follow one pattern. Until now there were many naming patterns: - TS_START_*/TS_END_* - TS_BEFORE_*/TS_AFTER_* - TS_*_START/TS_*_END This change also aims to indicate, that these timestamps can be used to create time-ranges, e.g. from TS_BOOTBLOCK_START to TS_BOOTBLOCK_END. Signed-off-by: Jakub Czapiga <jacz@semihalf.com> Change-Id: I533e32392224d9b67c37e6a67987b09bf1cf51c6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62019 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org> Reviewed-by: Yu-Ping Wu <yupingso@google.com> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-07src: Make PCI ID define names shorterFelix Singer
Shorten define names containing PCI_{DEVICE,VENDOR}_ID_ with PCI_{DID,VID}_ using the commands below, which also take care of some spacing issues. An additional clean up of pci_ids.h is done in CB:61531. Used commands: * find -type f -exec sed -i 's/PCI_\([DV]\)\(EVICE\|ENDOR\)_ID_\([_0-9A-Za-z]\{2\}\([_0-9A-Za-z]\{8\}\)*[_0-9A-Za-z]\{0,5\}\)\t/PCI_\1ID_\3\t\t/g' * find -type f -exec sed -i 's/PCI_\([DV]\)\(EVICE\|ENDOR\)_ID_\([_0-9A-Za-z]*\)/PCI_\1ID_\3/g' Change-Id: If9027700f53b6d0d3964c26a41a1f9b8f62be178 Signed-off-by: Felix Singer <felixsinger@posteo.net> Reviewed-on: https://review.coreboot.org/c/coreboot/+/39331 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
2022-03-03soc/amd/*/northbridge,root_complex: add comment about PCI BARsFelix Held
Add a comment to point out that the read_resources functions aren't missing a pci_dev_read_resources call that would add the resources for the BARs of the PC device. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ie480832e0d7954135d2171dda986e477ef7b6c09 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62547 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-03soc/amd/*/northbridge,root_complex: simplify GNB IOAPIC resource indexFelix Held
In the northbridge's and root complex' read_resources function, the GNB IOAPIC resource used MMIO base address of the GNB IOAPIC as index which might be misleading. Instead use idx++ as a unique index for this resource. TEST=Resource allocator doesn't complain and no related warnings or errors in dmesg. The update_constraints console output changes like expected: Before: PCI: 00:00.0 fec01000 base fec01000 limit fec01fff mem (fixed) After: PCI: 00:00.0 0d base fec01000 limit fec01fff mem (fixed) Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I8061364879d772469882fc060f92676de6f600a9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62546 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-03soc/amd/*/northbridge,root_complex: simplify mmconf_resource indexFelix Held
In the northbridge's and root complex' read_resources function, the mmconf resource used the number of the MMIO_CONF_BASE MSR as index which might be misleading. Instead use idx++ as a unique index for this resource. TEST=Resource allocator doesn't complain and no related warnings or errors in dmesg. The update_constraints console output changes like expected: Before: PCI: 00:00.0 c0010058 base f8000000 limit fbffffff mem (fixed) After: PCI: 00:00.0 06 base f8000000 limit fbffffff mem (fixed) Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Id66c6153fad86bed36db7bd2455075f4a0850750 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62545 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-03soc/amd/stoneyridge/acpi: rename cpu.asl to pnot.aslFelix Held
After the patch that moved the generation of the PPKG object to Stoneyridge's acpi.c, only the PNOT object remained in its cpu.asl, so rename it to pnot.asl. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I0deb2d75cae98b8fcd31297d7fac5f27525efe65 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62540 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-03soc/amd/stoneyridge/acpi: generate PPKG object in generate_cpu_entriesFelix Held
Generate the PPKG object in the generate_cpu_entries function instead of generating the PCNT object that is the used in the PPKG method in cpu.asl to provide the PPKG object. This both simplifies the code and aligns Stoneyridge with the other AMD SoCs. This will also make the code behave correctly in a case where the number of CPU cores/threads isn't a power of two. TEST=None, but equivalent change on Picasso was verified to not break anything on Mandolin. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ib42d718102151a72a5fe812e83eb2eb4f9e7b611 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62539 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-03soc/amd/picasso/acpi: rename cpu.asl to pnot.aslFelix Held
After the patch that moved the generation of the PPKG object to Picasso's acpi.c, only the PNOT object remained in its cpu.asl, so rename it to pnot.asl. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ic77dacb146aa823fc99f779f465fff28b2aead68 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62538 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-03soc/amd/picasso/acpi: generate PPKG object in generate_cpu_entries callFelix Held
Generate the PPKG object in the generate_cpu_entries function instead of generating the PCNT object that is the used in the PPKG method in cpu.asl to provide the PPKG object. This both simplifies the code and aligns Picasso with Cezanne and Sabrina. This will also make the code behave correctly in a case where the number of CPU cores/threads isn't a power of two. TEST=Mandolin still boots successfully to Linux desktop and dmesg doesn't show any any possibly related problems. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ifb84435345c6d8c5d11a8b42e5538cfb86432780 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62517 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-03-02soc/amd/common/vboot: Verify location of CBMEMC transfer bufferRaul E Rangel
Since we want to read the non-x86 CBMEMC from SMM we need to be stricter on where we read from. This change forces the verstage binary and x86 code to agree on the CBMEMC transfer buffer location and size. BUG=b:221231786 TEST=Boot guybrush and verify verstage transfer buffer still ends up in cbmem Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ida7d50bef46f280be0db1e1f185b46abb0ae5c8f Reviewed-on: https://review.coreboot.org/c/coreboot/+/62501 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-03-02soc/amd/common/vboot: Remove parameter to replay_transfer_buffer_cbmemcRaul E Rangel
We don't need to force the caller to look up and cast the transfer region. We can do it in the function. BUG=b:221231786 TEST=Build guybrush Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ib46a673ef5a43deb56a6d522152085036a47ab66 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62401 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-03-02soc/amd/common/vboot: Split transfer buffer methods into separate fileRaul E Rangel
I want to reuse the transfer buffer methods in SMM, so I need to add them into their own file. I renamed `setup_cbmem_console` to `replay_transfer_buffer_cbmemc` so it has a more descriptive name. I also fixed the comment on `verify_psp_transfer_buf`. BUG=b:221231786 TEST=Boot guybrush to OS Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I4f3a8b414b91f601c3a9c3dc7af8f388286fe4da Reviewed-on: https://review.coreboot.org/c/coreboot/+/62348 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-03-02soc/amd/common/psp_verstage: Save transfer buffer during S0i3 resumeRaul E Rangel
We need to save the transfer buffer so we can transfer the cbmem console and timestamps into x86 DRAM. BUG=b:221231786 TEST=Boot guybrush and verify S0i3 resume works. Also dumped the transfer buffer from the OS and verified the console contents got transferred. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I1d3b34c90e0e18609b0c6a0cdedab35aeefbd84b Reviewed-on: https://review.coreboot.org/c/coreboot/+/62347 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-03-01soc/amd/common/fsp/fsp_validate.c: print warning instead of errorJulian Schroeder
If an AMD FSP binary has no valid image revision information, print a warning instead of an error. Change-Id: Ie9c5a387b81205fe93382778090260e41e261776 Signed-off-by: Julian Schroeder <julianmarcusschroeder@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62349 Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-28soc/amd/sabrina: Add XHCI configurationJon Murphy
Add xhci 2 controller support for additional USB port/ Dummy setting BUG=b:214413631 TEST=builds Signed-off-by: Jon Murphy <jpmurphy@google.com> Change-Id: I5c8885bf46ddbfc85b31585a4da7f746c1a6bcd5 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62350 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-26soc/amd/common/psp_verstage: Add missing post codes on S0i3 resumeRaul E Rangel
We print these out in the normal flow, so lets add them for S0i3 resume as well. BUG=b:221231786 TEST=Perform suspend/resume cycle on guybrush and verify we get the new POST codes. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ia7d607453d58084868cfa50770fd0f370b2ea2bd Reviewed-on: https://review.coreboot.org/c/coreboot/+/62346 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Rob Barnes <robbarnes@google.com> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-26soc/amd/{common/psp_verstage,soc/picasso}: Remove workbuf shrinkingRaul E Rangel
This feature was never used. Let's remove it to keep things simple. BUG=221231786 TEST=Boot test guybrush and morphius and verify transfer buffer is correctly passed. Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I93a284db919f82763dcd31cec76af4b773eb3f80 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62345 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Rob Barnes <robbarnes@google.com>
2022-02-25arch/x86: consolidate HPET base address definitionsFelix Held
Both the HPET_BASE_ADDRESS define from arch/x86/include/arch/hpet.h and the HPET_ADDRESS Kconfig option define the base address of the HPET MMIO region which is 0xfed00000 on all chipsets and SoCs in the coreboot tree. Since these two different constants are used in different places that however might end up used in the same coreboot build, drop the Kconfig option and use the definition from arch/x86 instead. Since it's no longer needed to check for a mismatch of those two constants, the corresponding checks are dropped too. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ia797bb8ac150ae75807cb3bd1f9db5b25dfca35e Reviewed-on: https://review.coreboot.org/c/coreboot/+/62307 Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Lance Zhao Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-25arch/x86: factor out and commonize HPET_BASE_ADDRESS definitionFelix Held
All x86 chipsets and SoCs have the HPET MMIO base address at 0xfed00000, so define this once in arch/x86 and include this wherever needed. The old AMD AGESA code in vendorcode that has its own definition is left unchanged, but sb/amd/cimx/sb800/cfg.c is changed to use the new common definition. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ifc624051cc6c0f125fa154e826cfbeaf41b4de83 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62304 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Angel Pons <th3fanbus@gmail.com> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
2022-02-24treewide: Write minor version at acpi_create_fadt() functionElyes Haouas
When "fadt->FADT_MinorVersion" is not explicitly set to the right value, gcc sets it up to "0". So set it correctly for treewide. Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Change-Id: Ic9a8e097f78622cd78ba432e3b1141b142485b9a Reviewed-on: https://review.coreboot.org/c/coreboot/+/62221 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Lance Zhao
2022-02-24soc/amd/*/include/soc/iomap.h: rework HPET base address checkFelix Held
The AMD SoCs had a check to make sure that HPET_ADDRESS_OVERRIDE isn't set so that the HPET_ADDRESS Kconfig option will have the right default value. Instead check if the HPET_ADDRESS Kconfig value matches the HPET_BASE_ADDRESS define in the SoC code which is the case if HPET_ADDRESS_OVERRIDE isn't selected. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Icf1832eb36c031e93ba24f342e9a8a7bf13faecc Reviewed-on: https://review.coreboot.org/c/coreboot/+/62275 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-22soc/amd/sabrina/i2c: remove TODOFelix Held
The SoC-specific I2C code and header file have been verified some time ago, but it seems that I forgot to remove the corresponding TODOs. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ifd162bda10e5993bc32db3a77588491397e3c19e Reviewed-on: https://review.coreboot.org/c/coreboot/+/62223 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-22soc/amd/common/block/lpc/espi_util: use __fallthroughFelix Held
Using __fallthrough instead of a comment about the fall-through being intentional should make clang stop complaining about intended fall- through statements. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I940529be02e20c72f6e97b2cfa10f0dd8f7020b6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62216 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
2022-02-21soc/amd/sabrina/fw.cfg: Change the instance of PMUI/D to 2Zheng Bao
Change-Id: Ie9dbed7d6dd1e5f0c97d4a6cedea3d6bd7b000a2 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62068 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Nikolai Vyssotski <nikolai.vyssotski@amd.corp-partner.google.com>
2022-02-21soc/amd/*/fw.cfg: Remove the misleading name for PMUI and PMUDZheng Bao
Add the information of substance and instance in the string for PMUI and PMUD. It is amdfwtool's job to extract the number from the string. Change-Id: I43235fefcbff5f730efaf0a8e70b906e62cee42e Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62066 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-21[acpi]{include,soc/amd,southbridge/amd}: Clarify ARM_boot_arch in commentsElyes Haouas
Change-Id: I8b209da90b5a591f62e760961c64c4c63e6ef65b Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62040 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-21src/acpi: Add macro for FADT Minor Version and use itElyes Haouas
Change-Id: I6a0e9b33c6a1045a3a4a6717487525b82d41e558 Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62036 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Lance Zhao
2022-02-18drivers/fsp/fsp2_0: Rework FSP Notify Phase API configsSubrata Banik
This patch renames all FSP Notify Phase API configs to primarily remove "SKIP_" prefix. 1. SKIP_FSP_NOTIFY_PHASE_AFTER_PCI_ENUM -> USE_FSP_NOTIFY_PHASE_POST_PCI_ENUM 2. SKIP_FSP_NOTIFY_PHASE_READY_TO_BOOT -> USE_FSP_NOTIFY_PHASE_READY_TO_BOOT 3. SKIP_FSP_NOTIFY_PHASE_END_OF_FIRMWARE -> USE_FSP_NOTIFY_PHASE_END_OF_FIRMWARE The idea here is to let SoC selects all required FSP configs to execute FSP Notify Phase APIs unless SoC deselects those configs to run native coreboot implementation as part of the `.final` ops. For now all SoC that uses FSP APIs have selected all required configs to let FSP to execute Notify Phase APIs. Note: coreboot native implementation to skip FSP notify phase API (post pci enumeration) is still WIP. Additionally, fixed SoC configs inclusion order alphabetically.  BUG=b:211954778 TEST=Able to build and boot brya. Signed-off-by: Subrata Banik <subratabanik@google.com> Change-Id: Ib95368872acfa3c49dad4eb7d0d73fca04b4a1fb Reviewed-on: https://review.coreboot.org/c/coreboot/+/61792 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Lean Sheng Tan <sheng.tan@9elements.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-18soc/amd/common/block/psp/Makefile: add fmap_config.h dependencyFelix Held
Compiling efs_fmap_check.c depends on fmap_config.h already being generated, so add this dependency. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I85e0900574f928d1594f8d1831ba58f959b75d27 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62132 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-18soc/amd/common/block/apob/apob_cache: use APOB cache size from FMAPFelix Held
Also add the Makefile dependency on the fmap_config.h file to make sure that this file already exists when it's included. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I540ea2c14fd187845efd3c0c8c1e4b8f82c8cac3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62130 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-17soc/amd/common/block/i2c: Add support for shared TPM_I2C controllerJan Dabros
There are platforms equipped with AMD SoC where I2C3 controller connected to TPM device is shared between X86 and PSP. In order to handle this, PSP acts as an I2C-arbitrator, where x86 (kernel) sends acquire and release requests to be accepted by PSP. An example of implementation within Linux kernel is available [1]. There is a need to introduce new ACPI_ID ("AMDI0019") so that dedicated driver on OS side can bind to it and handle this special setup. Since PSP takes care of I2C controller power management, we need to remove PowerResource object from DSDT. BUG=b:204508404 BRANCH=guybrush [1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=78d5e9e299e31bc2deaaa94a45bf8ea024f27e8c Signed-off-by: Jan Dabros <jsd@semihalf.com> Change-Id: Iccfc09d8c580d7ab2acb69d26b9c293cf625fb34 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61863 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-17src/soc: Remove space before tabElyes Haouas
Spaces before tabs are not allowed. Change-Id: I0d2c55c2e0108e59facd92b2e2c0f6c418ef6db0 Signed-off-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62055 Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-17amd/common/block/gpio/gpio: don't use -1 as bitmask in gpio_or32Felix Held
The and-mask passed to the gpio_update32 call needs all 32 bits to be set to ones. When building as 32 bit binary the -1UL will result in the needed bit mask, but for a 64 bit build the constant would have 64 bits set to ones which then gets truncated to 32 bits causing a compiler error. Use 0xffffffff as bit mask instead which behaves correctly in both cases and also clarifies what this is doing. TEST=Timeless build for Chausie results in identical image. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I0b6a50bd914fdbb7a78885efb6c610715e2d26c1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62053 Reviewed-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Aamir Bohra <aamirbohra@gmail.com> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-17amd/common/block/spi/fch_spi_ctrl: use uintptr_t for addressesFelix Held
This fixes a build failure when trying to build the code in 64 bit mode. TEST=Timeless build for Chausie results in identical image. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: If8fe7b626d9d72c0b8ed07ced93e46f795e36848 Reviewed-on: https://review.coreboot.org/c/coreboot/+/62052 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aamir Bohra <aamirbohra@gmail.com> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Elyes Haouas <ehaouas@noos.fr> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-16soc/amd/cezanne/psp_verstage/uart: Fix off by 1 errorRaul E Rangel
We only allow index = {0, 1}. Fix the check. BUG=b:215599230 TEST=Build guybrush BRANCH=guybrush Found-by: Coverity CID 1469611 Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I59615ab39faeded43b3803b4450c84ab8a8b81ba Reviewed-on: https://review.coreboot.org/c/coreboot/+/61988 Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-16soc/amd/common/block/psp: add PSP commandJason Glenesk
Add PSP command to send SPL fuse command if PSP indicates SPL fusing is required. Also add Kconfig option to enable sending message. BUG=b:180701885 TEST=On a platform that supports SPL fusing. Build an image with an SPL table indicating fusing is required, confirm that PSP indicates fusing required and coreboot sends the appropriate command. A message indicating PSP requested fusing will appear in the log: "PSP: Fuse SPL requested" Signed-off-by: Jason Glenesk <jason.glenesk@amd.corp-partner.google.com> Change-Id: If0575356a7c6172e2e0f2eaf9d1a6706468fe92d Reviewed-on: https://review.coreboot.org/c/coreboot/+/61462 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: ritul guru <ritul.bits@gmail.com>
2022-02-15soc/amd/sabrina: Select ACP gen2Fred Reitberger
Select ACP gen2 for Sabrina Change-Id: I107ebd390732b597629a3236d0e7d1f5e2c51379 Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61833 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-15soc/amd/common/acp: add acp_gen2Fred Reitberger
The gen2 ACP register definitions and locations are different from previous models. Specific code is refactored into acp_gen1 and acp_gen2. Update ACP register locations and definitions for gen2. Change-Id: If665b93cddf22435512f1276fcfee2f497dc6ef5 Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61832 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-13soc/amd/common/include/ioapic: make IOAPIC IDs not depend on MAX_CPUSFelix Held
Since the APIC bus isn't used since a long time and the IOAPIC and LAPIC talk to each other via the system bus, there is no longer the requirement that the IOAPIC IDs mustn't overlap with the LAPIC IDs that start at 0 and end at CONFIG_MAX_CPUS - 1. The current Intel code uses 2 as the IOAPIC ID while most of their CPUs have more than 2 logical cores resulting in the IOAPIC having the same ID as one of the LAPICs. All chipsets in soc/amd use the defines for FCH_IOAPIC_ID and GNB_IOAPIC_ID for initializing the IOAPIC register, writing both MADT and IVRS ACPI tables and there's no MPTable support for those SoCs that might also rely on those IDs being consistent. This patch changes the definitions for FCH_IOAPIC_ID and GNB_IOAPIC_ID from CONFIG_MAX_CPUS and CONFIG_MAX_CPUS + 1 to 0 and 1. This also makes sure that the IOAPIC IDs still fit in 4 bits despite Cezanne having a CONFIG_MAX_CPUS of 16 resulting in the IOAPIC IDs being larger than 4 bits with the old code. While the Cezanne FCH IOAPIC supports 8 bits of IOAPIC IDs, this is non-standard. TEST=AMD Mandolin and Google Liara still work. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Suggested-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Change-Id: Id3a356480bb8407e0347cb5cef691fde7edc8deb Reviewed-on: https://review.coreboot.org/c/coreboot/+/55430 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
2022-02-12soc/amd/cezanne/psp_verstage/uart: Fix off by 1 errorRaul E Rangel
FCH_UART_ID_MAX == 2, and there are 2 UARTS, so we don't need the -1. BUG=b:215599230 TEST=Build guybrush Found-by: Coverity CID 1469611 Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I5f0171ed2d3da7f86ba3cfd0457f60d2d5722625 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61869 Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-12soc/amd/cezanne: Allow to specify SPL table path in KconfigZheng Bao
BUG=b:216096562 Change-Id: I4a5ee335ea8808b595dc65ebafd15baedfbdd06e Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61837 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-12soc/amd/common: Scan bridge devices behind SoCs GPU ControllerKevin Chiu
Scan devices behind SoCs GPU controller to expose more buses. BUG=b:204401306 BRANCH=guybrush TEST=emerge-guybrush coreboot Change-Id: Ib78e6570f101c71efaf9cc1843defcb05301cd30 Signed-off-by: Kevin Chiu <kevin.chiu.17802@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61860 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-11soc/amd/common/acp: introduce acp_gen1Fred Reitberger
Refactor existing acp code into acp_gen1 variant as preparation for gen2 variant in sabrina. Change-Id: Id9248584237196b5404b79d3a8552cb90fe4491e Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61831 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-11soc/amd/common/fsp: check fsp image revisionJulian Schroeder
Check if FSP binary and coreboot FSP structures (fspmupd.h) match sufficiently. A change in minor number denotes less critical changes or additions to the FSP API that still allow for the boot process to proceed. A change of the AMD image revision major number will halt boot. The Fspmupd.h header now defines IMAGE_REVISION_ macros for AMD Picasso, Cezanne and Sabrina APUs. BUG=b:184650244 TEST=build, boot and check fsp image revision info. Example: FSP major = 1 FSP minor = 0 FSP revision = 5 FSP build = 0 Signed-off-by: Julian Schroeder <julianmarcusschroeder@gmail.com> Change-Id: I0fbf9413b0cf3e6093ee9c61ff692ff78ebefebc Reviewed-on: https://review.coreboot.org/c/coreboot/+/61281 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-11soc/amd/cezanne,picasso,sabrina: Fix incorrect values of CBFS amdfw position ↵Robert Zieba
makefile variables Currently apu/amdfw_a-position and apu/amdfw_b-position currently depend on CEZANNE_FW_A_POSITION and CEZANNE_FW_B_POSITION. This causes error messages from awk as these variables are sourced from fmap_config.h and these variables are expanded before fmap_config.h is built. However these variables should not be set to CEZANNE_FW_*_POSITION. These files end up in the FW_MAIN_* fmap regions. These regions are placed at the proper locations through the chromeos.fmd file. The apu/amdfw_*-position variables are the positions within these regions where the files end up. These variables should be set to 0x40 to coincide with the beginning of the FW_MAIN_* regions, accounting for the size of struct cbfs_file + filename + metadata, aligned to 64 bytes. Currently they end up in the correct locations only because fmap_config.h does not exist when the apu/amdfw_*-position variables are expanded. This change explicity sets the value of these variables to 0x40, removing the errors from awk and ensuring that these files end up in the correct location in the resulting image. These changes are also applied to the Picasso and Sabrina makefiles as well. BUG=b:198322933 TEST=Verified that the apu/amdfw_* files end up in the correct locations as reported by cbfstool during the build, did timeless builds and confirmed that coreboot.rom images were identical, tested AP firmware on guybrush and zork devices Signed-off-by: Robert Zieba <robertzieba@google.com> Change-Id: If1c2b61c5be0bcab52e19349dacbcc391e8aa909 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61349 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de> Reviewed-by: Rob Barnes <robbarnes@google.com>
2022-02-11soc/amd/common/block/lpc/espi_util: add decode range register helpersFelix Held
Introduce and use functions to translate eSPI IO/MMIO decode range IDs into the corresponding register bits and the IO/MMIO range and size register IDs into register offsets. This is a preparation to support the additional eSPI decode ranges on Sabrina where not all enable bits and base/size registers for one type of decode ranges are consecutive. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Id91fe32447a06b049e33dfdacc8edfa2ebb2df39 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61778 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-11soc/amd/common/block/include/espi: rename IO/MMIO base/size registersFelix Held
This aligns the register names more with the PPR. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I4e7dc8dfc0fa5e86b9d4425f2496be86e039b686 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61777 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-09soc/amd/common/block/psp: introduce AMD_SOC_SEPARATE_EFS_SECTIONFred Reitberger
On systems that use the first 128kByte of the SPI flash for the EC firmware, it is not possible to place the EFS/amdfw part at the lowest location in flash where the on-chip PSP firmware will look for the EFS, since this is at an offset of 128kByte into the flash which is where the cbfs master header resides when the main CBFS is placed right after the EC firmware. This patch introduces the AMD_SOC_SEPARATE_EFS_SECTION option that allows putting the EFS in a separate FMAP section that can be located right after the EC firmware FMAP section. The EFS FMAP partition is checked to ensure it begins at the expected location. Change-Id: I5ed0f76c9c9c9c180ee5f1b96f88689d0979bb5e Signed-off-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61558 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-02-08soc/amd/sabrina/Kconfig: remove SOC_AMD_COMMON_BLOCK_PCI_MMCONF TODOFelix Held
Sabrina uses the same MMIO_CONF_BASE MSR as the previous AMD CPUs to configure the PCI MMCONF base address. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I7e3064bab5ca1e277b04f9aae98f9adabce75399 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61681 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-08soc/amd/cezanne: Disable CONSOLE_CBMEM_PRINT_PRE_BOOTBLOCK_CONTENTSRaul E Rangel
Now that PSP verstage can directly write to the UART, we no longer need to manually dump the cbmem contents. Ideally if we can get picasso to add support for mapping the UART, or if we implement bit banging we can delete this functionality completely. BUG=b:215599230 TEST=Boot guybrush and verify verstage logs aren't printed twice Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Id70b24625c3b2f3d6fe470cf227a0083f5b974f9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61611 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-08soc/amd/common/psp_verstage: Add UART support to PSP consoleRaul E Rangel
This will allow PSP verstage to write logs to the serial console. We are no longer dependent on using a serial enabled PSP boot loader. Ideally we would delete this psp printk and use the standard printk. Since picasso doesn't currently support mapping the UART though, I'll keep it for now. BUG=b:215599230 TEST=Boot guybrush and verify PSP logs are output on serial console Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ibd77cc754fae5baccebe7adc5ae0790c79236d26 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61610 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-08soc/amd/sabrina/psp_verstage: Implement get_uart_baseRaul E Rangel
The Sabrina PSP doesn't support mapping the UART, so add a dummy function to return NULL. BUG=b:215599230 TEST=None Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Idad8e4874e78bb96730feecb5a7b17334d12217c Reviewed-on: https://review.coreboot.org/c/coreboot/+/61609 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-08soc/amd/picasso/psp_verstage: Implement get_uart_baseRaul E Rangel
The Picasso PSP doesn't support mapping the UART, so add a dummy function to return NULL. BUG=b:215599230 TEST=Build and boot morphius Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ie1f033ff86ebb0f755a9a0b6ff293aa3c8bbbeb6 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61608 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-08soc/amd/cezanne/psp_verstage: Implement get_uart_baseRaul E Rangel
This will allow directly using the UART console. On PSP releases that don't support mapping the UART, we will just return NULL which is perfectly acceptable. BUG=b:215599230 TEST=Boot guybrush and verify verstage can print to the console Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Ic8d7f0fe00794a715756f92e3fb32c6b512cb8aa Reviewed-on: https://review.coreboot.org/c/coreboot/+/61607 Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-07treewide: Remove "ERROR: "/"WARN: " prefixes from log messagesJulius Werner
Now that the console system itself will clearly differentiate loglevels, it is no longer necessary to explicitly add "ERROR: " in front of every BIOS_ERR message to help it stand out more (and allow automated tooling to grep for it). Removing all these extra .rodata characters should save us a nice little amount of binary size. This patch was created by running find src/ -type f -exec perl -0777 -pi -e 's/printk\(\s*BIOS_ERR,\s*"ERROR: /printk\(BIOS_ERR, "/gi' '{}' ';' and doing some cursory review/cleanup on the result. Then doing the same thing for BIOS_WARN with 's/printk\(\s*BIOS_WARNING,\s*"WARN(ING)?: /printk\(BIOS_WARNING, "/gi' Signed-off-by: Julius Werner <jwerner@chromium.org> Change-Id: I3d0573acb23d2df53db6813cb1a5fc31b5357db8 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61309 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr> Reviewed-by: Lance Zhao Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
2022-02-07soc/amd/cezanne: Add the fw SPL to fw.cfgZheng Bao
SPL: Security Patch Level The data in SPL is used for FW anti-rollback, preventing rollback of platform level firmware to older version that are deemed vulnerable from a security point of view. BUG=b:216096562 Change-Id: I0aa456b8b4eec506fbb319293f0903b293325cb0 Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61425 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-07soc/amd/sabrina/Kconfig: remove TODO from SOC_AMD_COMMON_BLOCK_UARTFelix Held
Sabrina is compatible with the common AMD UART block and also with the DRIVERS_UART_8250MEM_32 driver it selects. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I432414c1d501ffbd1047b378996e06d281a9fb6f Reviewed-on: https://review.coreboot.org/c/coreboot/+/61641 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
2022-02-05soc/amd/sabrina/Kconfig: remove TODO from SOC_AMD_COMMON_BLOCK_IOMMUFelix Held
Sabrina is compatible with the common AMD SOC_AMD_COMMON_BLOCK_IOMMU code. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I4c2e8553fde9467ca1b5e9085e36c33d138b7156 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61633 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-05nb,soc/amd/*/iommu: fix comment about IOMMU MMIO resourceFelix Held
This comment was added with the AMD family 15h Trinity IOMMU support in commit 88ebbeb7e2a914330c869147bacb190b4270532f and looks like a copy of the comment about the subtractive decode ranges in the LPC device. The IOMMU doesn't have any subtractively decoded I/O or MMIO ranges and this is also not what the code does. This resource is the MMIO region to configure the IOMMU instead, so fix the comment in all copies of the IOMMU support code. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I2e1e3a46b839b9e58b836932c1bc9b41b1b1dc02 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61631 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-05soc/amd/sabrina/Kconfig: remove TODO from SOC_AMD_COMMON_BLOCK_ACPIMMIOFelix Held
Sabrina is compatible with the common AMD ACPIMMIO function block mapping and access functions. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I890375654a9cb1156e481c5586007ac81ab84120 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61626 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-05soc/amd/common/include/acpimmio: drop 16 and 32 bit PM2 access functionsFelix Held
The PM2 ACPIMMIO region should only be accessed with 8 bit accesses. Using 16 or 32 bit read accesses will return the data from the first byte for all 2 or 4 bytes and 16 or 32 bit write accesses will result in only the first byte being written which is both unexpected behavior. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I5ace50d3b81b5bf3ea3b10aa02f25c58a6ea99b9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61625 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-05soc/amd/common/block/acpimmio/print_reset_status: extend bit name tableFelix Held
Bit 23 in the PM_RST_STATUS register is called LtReset on Stoneyridge and ShutdownMsg on Picasso/Cezanne/Sabrina. Bit 30 is reserved on Stoneyridge and defined as SdpParityErr on the newer SoCs. Bit 31 is only defined for Sabrina. Since the default value of undefined bits is 0 it isn't a problem to have descriptions for reserved reset status bits on some SoCs. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I0782116d327fcad3817a10eb237ac6c8294846b3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61624 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-05cpu/x86/lapic: Move LAPIC configuration to MP initKyösti Mälkki
Implementation for setup_lapic() did two things -- call enable_lapic() and virtual_wire_mode_init(). In PARALLEL_MP case enable_lapic() was redundant as it was already executed prior to initialize_cpu() call. For the !PARALLEL_MP case enable_lapic() is added to AP CPUs. Change-Id: I5caf94315776a499e9cf8f007251b61f51292dc5 Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/58387 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>
2022-02-04soc/amd/sabrina/include/amd_pci_int_defs.h: remove PIRQ_SATAFelix Held
Sabrina has no SATA controller, so remove the corresponding PIRQ mapping. This was verified with PPR #57243 Rev 1.53. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I98ffa3675c361e8a74c50ebfc37e79ae63dacc85 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61601 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-04soc/amd/sabrina/Kconfig: remove SOC_AMD_COMMON_BLOCK_DATA_FABRIC TODOFelix Held
The common AMD data fabric register access code is valid for Sabrina. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I97fb2c6006c09297584845a83342e75058d35713 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61600 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-04soc/amd/sabrina/Kconfig: remove TODO from SOC_AMD_COMMON_BLOCK_SMUFelix Held
The common AMD SMU code and the common AMD SMN access code that gets selected by the common SMU code are valid for Sabrina. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ic220dbb2f73b89554ac7e7b7e6dc7525ae8e9faa Reviewed-on: https://review.coreboot.org/c/coreboot/+/61599 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-04soc/amd/sabrina/Kconfig: remove TODO from SOC_AMD_COMMON_BLOCK_AOACFelix Held
The common AMD FCH AOAC bit definitions and helper functions are correct for Sabrina. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ie791cca0dc760e53e0f5c69c63ac78270ba6ad4f Reviewed-on: https://review.coreboot.org/c/coreboot/+/61598 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-04soc/amd/sabrina/Kconfig: remove TODO from SOC_AMD_COMMON_BLOCK_I2CFelix Held
Sabrina uses an identical I2C controller as Picasso and Cezanne. Also both the type and version read-only register of the I2C controller contain identical values. The dma_cr, dma_tdlr, dma_rdlr and clr_restart_det registers that are defined in the dw_i2c_regs struct in the common Designware I2C code aren't defined in the PPRs of Picasso, Cezanne and Sabrina, but since common DW I2C code doesn't access those, this is no problem. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I90732aa98518010686f73f80bee229b13e9bc89c Reviewed-on: https://review.coreboot.org/c/coreboot/+/61592 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-04soc/amd/common/block/i2c/i23c_pad_ctr: add & use I23C pad configurationFelix Held
I2C bus 0..2 on Sabrina uses a different pad type which supports 1.1V and 1.8V levels, but doesn't support 3.3V I2C levels. Compared to the existing I2C pad control registers the bit definitions are different, so add a separate function to configure those pads which however still has the same function signature and is compatible with same data structs used for the devicetree settings. PPR #57243 Rev 1.50 was used as a reference. TEST=None Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ie210c3437f2608d1e9fb99dcb151fc4190721375 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61570 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-03soc/amd/*/i2c: factor out common I2C pad configurationFelix Held
The I2C pad control registers of Picasso and Cezanne are identical and the one of Sabrina is a superset of it, so factor out the functionality. To avoid having devicetree settings that contain raw register bits, the i2c_pad_control struct is introduced and used. The old Picasso code for this had the RX level hard-coded for 3.3V I2C interfaces, so keep it this way in this patch but add a TODO for future improvements. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I1d70329644b68be3c4a1602f748e09db20cf6de1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61568 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-03soc/amd/*/i2c: introduce and use MISC_I2C_PAD_CTRL(bus) macroFelix Held
Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I9d098a55a5c6f6e022c3896750c752e2759e101b Reviewed-on: https://review.coreboot.org/c/coreboot/+/61567 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org>
2022-02-03soc/amd/*/i2c: drop unused mainboard_i2c_overrideFelix Held
No mainboard in the current tree implements mainboard_i2c_override. In a follow-up commit the i2c_pad_control struct is introduced to be able to make more parameters controllable by devicetree settings in the future. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I8f9ed5d50d26e4623dc5888cc8af090fdd00fc03 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61566 Reviewed-by: Raul Rangel <rrangel@chromium.org> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-02soc/amd/cezanne: Rename PSP_POSTCODES_ON_ESPI to PSP_INIT_ESPIRaul E Rangel
This flag only controls eSPI init in the PSP Stage 2 Boot Loader. It doesn't control if port 80s are written. This flag also doesn't currently control LPC init. The PSP is currently hard coded to remove any LPC init. BUG=b:215425753 TEST=build guybrush Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: Idf3f0dcc216df2fd15b016f9458a208b7e15c720 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61534 Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Reviewed-by: Rob Barnes <robbarnes@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-02soc/amd/cezanne,vc/cezanne: Implement svc_write_postcodeRaul E Rangel
This will allow verstage to write post codes. BUG=b:215425753 TEST=Boot guybrush and verify PSP post codes are printed 22-01-31 15:12:03.214 (S3->S0) 22-01-31 15:12:03.214 03 04 0f 0e f0 f1 f2 01 10 a0 a2 <--new Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I6ceee8fcb094f462de99c07aef8e96425d9c3270 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61522 Reviewed-by: Kangheui Won <khwon@chromium.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com> Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
2022-02-01psp_verstage: report developer mode to PSPKangheui Won
Add platform_report_mode function which report current developer mode status to the PSP. L1 widevine app in the PSP will use this information to select key box. BUG=b:211058864 TEST=build and boot guybrush TEST=build picasso chrome os boards Signed-off-by: Kangheui Won <khwon@chromium.org> Change-Id: I04b5fcfa338b485b36f1b946203f32823385c0b1 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61369 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-01psp_verstage: add new svc for cezanneKangheui Won
Add svc_set_platform_bootmode svc to cezanne. PSP will use this information to select proper widevine keybox. BUG=b:211058864 TEST=build guybrush Signed-off-by: Kangheui Won <khwon@chromium.org> Change-Id: I6bcc9e49a2b73d486cfecd7b240bf989cad94630 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61368 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Karthik Ramasubramanian <kramasub@google.com>
2022-02-01drivers/i2c/designware/dw_i2c: return enum cb_err from dw_i2c_initFelix Held
Using enum cb_err as return type instead of int improves the readability of the code. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I55e6d93ca141b687871ceaa763bbbbe966c4b4a3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61511 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Raul Rangel <rrangel@chromium.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
2022-01-31soc/amd/sabrina: Add PRE_X86_CBMEM_CONSOLE_SIZEFelix Held
Commit 86302a806c5cc9b575424305e761753710417692 (soc/amd/{common, cezanne,picasso}: Add PRE_X86_CBMEM_CONSOLE_SIZE) added this Kconfig option before the initial commit that added soc/amd/sabrina as copy of soc/amd/cezanne landed in the tree, so port the change forward to Sabrina. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I2e8df5e7b7f1ac0af772e8c565f616a68b28e29e Reviewed-on: https://review.coreboot.org/c/coreboot/+/61358 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina/chipset.cb: update USB portsFelix Held
The corresponding mainboard design guide was used as a reference here. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ie61af7dab35b560d2eec1ea62058f3a4dad5cb0f Reviewed-on: https://review.coreboot.org/c/coreboot/+/61094 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina/include/smu: update mailbox SMN addressesFelix Held
The SMU message response register was moved compared to Cezanne. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ie384de52b1efb1d52f9018315a4b72916a4c9cee Reviewed-on: https://review.coreboot.org/c/coreboot/+/61095 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina/include/southbridge: add new I2C_PAD_CTRL bitsFelix Held
Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Iac1b7308851c34bd1556c02af6b270e9346073e4 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61098 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina: drop PM_ESPI_CS_USE_DATA2 define and eSPI util codeFelix Held
The Sabrina SoC doesn't have the PM_ESPI_CS_USE_DATA2 bit defined in the PM_SPI_PAD_PU_PD register. It also doesn't have a physical LPC interface any more, so there are no LPC pins that can be reconfigured as eSPI interface. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I02bc8d007901c71942475fe707637c5da7227230 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61097 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina: drop CPPC codeFelix Held
The CPPC feature isn't available on the Sabrina SoC, so drop the corresponding code. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I71a1b0717571729ebca3600ac433e621cafc4e61 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61096 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina: update PCI devices in devicetree.cbFelix Held
Also update mb/amd/chausie accordingly. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Idb4dcffa48c3dbdcffb66f1398b99ee96562efb9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61093 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina: update pci_devs.hFelix Held
Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ic0226afd9e7fffd6bf196f06ee6c34b6b9c92f30 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61092 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina/fsp_m_params: drop sata_enable UPD writeFelix Held
There are no SATA controllers on the Sabrina SoC. The UPD field will be removed later as a part of the initial UPD header update. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Iedefd9f150e5bcb78173288e5fc9f1bbd6b498cd Reviewed-on: https://review.coreboot.org/c/coreboot/+/61091 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
2022-01-27soc/amd/sabrina/include: update smi.hFelix Held
Some of the names have slightly changed in the PPR, but I kept the current names for consistency across all AMD SoCs in coreboot. Revision 1.50 of the PPR #57243 was used as a reference. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I6bda656015858a57e221b8d7819f944c21564a39 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61090 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina/include/data_fabric: update IOMS0_FABRIC_IDFelix Held
The data fabric ID table in PPR #57243 Rev 1.50 has a different IOMS0 fabric ID than Cezanne. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I32890b5c03219f6ebf8180929d71ef726d382483 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61089 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
2022-01-27soc/amd/sabrina: drop graphics.cFelix Held
Since we don't need to support PCI ID remapping for finding the correct VBIOS binary for the integrated GPU, graphics.c can be dropped for now. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ifd5b678f472b3b5888353efd057203eb641be874 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61088 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina/cpu: update CPUIDFelix Held
Sabrina is family 17h model A0h. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I01e02e3491fb90941c767058986da876bdf7ca1d Reviewed-on: https://review.coreboot.org/c/coreboot/+/61087 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-27soc/amd/sabrina: add additional UART controllersFelix Held
Compared to Cezanne there are 3 more UART controllers. Revision 1.50 of the PPR #57243 was used as a reference. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I628b1a7a0930f3409acdcabda2b864d42bf6bd23 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61086 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com>
2022-01-27soc/amd/sabrina/gpio: update GPIO definitions and gpio_event_tableFelix Held
The GPIO and GPIO MUX mapping as well as some GPIO to GEVENT mappings have changed compared to Cezanne. Sabrina also doesn't have a remote GPIO bank. Revision 1.50 of PPR #57243 was used as a reference. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Iabb85a3d24c881055e94400d08d01505df44a07a Reviewed-on: https://review.coreboot.org/c/coreboot/+/61084 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com> Reviewed-by: Fred Reitberger <reitbergerfred@gmail.com>
2022-01-27soc/amd/sabrina/include/amd_pci_int_defs: add additional UARTsFelix Held
Compared to Cezanne there are 3 more UARTs controllers. The PCI interrupt index table in the new SoC's PPR #57243 Rev 1.50 doesn't contain a PIRQ mapping for UART4. The reference code has a mapping for this and it uses PIRQ mapping index 0x77 for UART4 and not for I2C5. Since the I2C5 controller isn't owned by the x86 side and I didn't see any mapping of the I2C5 controller into the x86 MMIO space, this seems very plausible. Also add the corresponding fields to the ACPI code. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I44780f5bc20966e6cc9867fca609d67f2893163d Reviewed-on: https://review.coreboot.org/c/coreboot/+/61083 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-26src: Add missing 'void' in function definitionElyes HAOUAS
Change-Id: I7fa1f9402b177a036f08bf99c98a6191c35fa0b5 Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> Reviewed-on: https://review.coreboot.org/c/coreboot/+/61371 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Jason Glenesk <jason.glenesk@gmail.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-01-26soc/amd/cezanne,picasso,sabrina: factor out get_threads_per_coreFelix Held
This code is common to at least all Zen-based APUs (Picasso, Cezanne, Sabrina) and is also useful outside of the SoC-specific dynamic ACPI table generation code. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Ie96d4429fb6ed9223efed9b3c754e04052d7ca7c Reviewed-on: https://review.coreboot.org/c/coreboot/+/61357 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Rob Barnes <robbarnes@google.com> Reviewed-by: Chris Wang <chris.wang@amd.corp-partner.google.com> Reviewed-by: Eric Peers <epeers@google.com>
2022-01-26soc/amd/common: Don't reserve VERSTAGE region when using PSP verstageRaul E Rangel
The VERSTAGE region is only needed when running verstage in the x86. This change reduces the early ram size by 512 KiB when using PSP verstage. BUG=none TEST=Boot guybrush to OS Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I45ce421397807dbb1eb48aedd05209b91e89aa4f Reviewed-on: https://review.coreboot.org/c/coreboot/+/61190 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-01-25soc/amd/cezanne: FSP: Add UPD entry for eDP tuningZheng Bao
The FSP gets these values from the UPD and sets the internal values. The document about eDP tuning is attached in issue tracker of this ticket, at the issue tracker b/203061533#comment6. BUG=b:203061533 Cq-Depend: chrome-internal:4303901 Change-Id: I9b85faac4f2fa1fb2c14bb85b615346d4379baac Signed-off-by: Zheng Bao <fishbaozi@gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/59918 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Patrick Huang <patrick.huang@amd.corp-partner.google.com> Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
2022-01-25soc/amd/sabrina/include/aoac_defs: add additional UARTsFelix Held
Compared to Cezanne there are 3 more UARTs controllers. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: Id98767197a21cb1a61f54fc9b256b10a9506c791 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61082 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-25soc/amd/sabrina/include/iomap: update MMIO device mappingsFelix Held
Compared to Cezanne there are 3 more UARTs with DMA controllers. Signed-off-by: Felix Held <felix-coreboot@felixheld.de> Change-Id: I3a3d255bb4976a55623f3a161e791e80f1d01c69 Reviewed-on: https://review.coreboot.org/c/coreboot/+/61081 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Marshall Dawson <marshalldawson3rd@gmail.com>
2022-01-25soc/amd/cezanne: Increase PRERAM_CBMEM_CONSOLE_SIZE to 0x2000Raul E Rangel
Let's increase this to avoid losing any logs. BUG=b:213828947 TEST=Boot guybrush and no longer see *** Pre-CBMEM romstage console overflowed, log truncated! Signed-off-by: Raul E Rangel <rrangel@chromium.org> Change-Id: I3258145e352af3a75893c7cc96f36eb238c99abb Reviewed-on: https://review.coreboot.org/c/coreboot/+/61100 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Kangheui Won <khwon@chromium.org>