We'll process the init sequence as part of the polling loop. This
should have several advantages:
* It eases error handling, i.e. we can return to an earlier state.
* We don't have to stall initialization when a keyboard takes a
little longer.
* Generally, these keyboards can be hot-plugged (albeit not by
design).
Change-Id: I9cf5cf31eb420b3994bec20e56a72d37f3d2996e
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47086
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Draining the keyboard's buffer is only possible when the keyboard
port is enabled. We should also disable input scanning before, as
the buffer could be filled again with new keystrokes otherwise.
Change-Id: Ibac9c0d04880ff4a3efda5ac53da2f9731f6602c
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47085
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Even if we are careful, it's still possible that we read spurious
data from the keyboard, e.g. keystrokes. Namely, when we send the
reset/disable command, there is a race before the command is pro-
cessed.
So we should always process data from the keyboard in a loop. We
break it, when an ACK (0xfa) or a NAK (0xfe) is received, and warn
on unexpected data unless it might be due to the mentioned race.
This also gives us the opportunity to use command-specific timeouts
which we take from Linux: 1s for the keyboard self-test (as there
are keyboards that perform the test before acking the command) and
200ms for all other commands.
Change-Id: I60a2643a8ff4b9231c63bf970c8749c97c7d8926
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47083
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Only one EEPROM is used to store the board settings, and its I2C address
is constant. Thus, there's no need to pass its address as a parameter.
In addition, reduce the scope of the `I2C_ADDR_EEPROM` definition, since
using it outside of eeprom.c would bypass the API's abstraction layer.
Change-Id: I958304e6ed6df05af923139d44ff4fd1de204738
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46565
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Drop chipset register definitions in mainboard code in favor of existing
definitions in a header. These definitions are not mainboard-specific.
Tested with BUILD_TIMELESS=1, Prodrive Hermes remains identical.
Change-Id: I29d6f35ec27bff43cf52ae697e905b6a7b48a8d1
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48805
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Enable Runtime D3 for the volteer variants that have GPIO power control
of the NVMe device attached to PCIe Root Port 9.
Enable the GPIO for power control for variants that do not already have
it configured to allow the power to be disabled in D3 state.
BUG=b:169356808
TEST=tested on voema
Signed-off-by: David Wu <david_wu@quanta.corp-partner.google.com>
Change-Id: I28ef074225c533e1a97b6ec4a1a5dd1dcc198168
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48848
Reviewed-by: Paul Fagerburg <pfagerburg@chromium.org>
Reviewed-by: Tim Wawrzynczak <twawrzynczak@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Pointers to structs can be very useful, especially when they point to an
array element. In this case, changing one pointer allows the function to
be rewritten more concisely, since most redundancy can be eliminated.
Tested on Asus P8Z77-V LX2, still boots. No functional difference.
Change-Id: I7f0c37ea49db640f197162f371165a6f8e9c1b9c
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48612
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Ensure that IOSAV is finished before continuing. This might solve some
random failures on the I/O and roundtrip latency training algorithm.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: Ic08a40346b6c60e372bada10f9c4ee42eb974f9f
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48403
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Most ofte, `iosav_run_once` precedes a `wait_for_iosav` call. Add a
helper function to reduce clutter. The cases where `iosav_run_once`
isn't followed by `wait_for_iosav` will be handled in a follow-up.
Tested on Asus P8Z77-V LX2, still boots.
Change-Id: Ic76f53c2db41512287f41b696a0c4df42a5e0f12
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48402
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Now that the purpose of each training algorithm is clear, replace the
last instances of the original names in comments and print statements
with the current, correct names. Also, print which channel has failed
command training, for completeness and consistency with other errors.
Change-Id: I9cc5c4b04499297825ca004c6bd1648a68449d2c
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48601
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Use absolute values for the Rx and Tx bus timings instead of values
relative to the CA (Command/Address) bus timing. This makes the
calculations more accurate, less complex and less error-prone.
Tested on Asus P8H61-M PRO, still boots. Training results do not seem to
be affected by this patch, and the margins roughly have the same shape.
Change-Id: I28ff1bdaadf1fcbca6a5e5ccdd456de683206410
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/47771
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This file was being written to the root src directory. It is the only
file being written to src during a normal build, while all others are
being written to $(obj). I added a new variable to allow specifying the
xcompile path. This allows generating a single file if building multiple
boards. I also moved the default location into $(obj) so we don't
pollute the src directory by default.
I also cleaned up the generation of xcompile by removing the unnecessary
eval and NOCOMPILE check.
I also left .xcompile in distclean so it cleans up stale files.
Since .xcompile is written into $(obj), `make clean` will now remove it.
The tegra Makefiles are outside of the normal build process, so I just
updated those Makefiles to point to the default xcompile location of a
normal build. The what-jenkins-does target had to be updated to support
these special targets. We generate an xcompile specifically for these
targets and pass it into the Makefile. Ideally we should get these
targets added to the main build.
BUG=b:112267918
TEST=ran `emerge-grunt coreboot` and `make what-jenkins-does`
Signed-off-by: Raul E Rangel <rrangel@chromium.org>
Change-Id: Ia83f234447b977efa824751c9674154b77d606b0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/28101
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Some background first: The original XT keyboards used what we call
scancode set #1 today. The PC/AT keyboards introduced scancode set #2,
but for compatibility, its controller translated scancodes back to
set #1 by default. Newer keyboards (maybe all we have to deal with)
also support switching the scancode set.
This means the translation option in the controller and the scancode
set selection in the keyboard have to match. In libpayload, we only
support set #1 scancodes. So we either need the controller's trans-
lation on and set #2 selected in the keyboard, or the controller's
translation off and set #1 selected in the keyboard.
Valid configurations:
* SET #1 + XLATE off
* SET #2 + XLATE on
Both with and without the PC_KEYBOARD_AT_TRANSLATED option, we were
only configuring one of the two settings, leaving room for invalid
configurations. With this change, we try to select scancode set #2
first, which seems to be the most supported one, and configure the
controller's translation accordingly. We try to fall back to set #1
on failure.
We also keep translation disabled during configuration steps to
ensure that the controller doesn't accidentally translate confi-
guration data.
On the coreboot side, we leave the controller's translation at its
default setting, unless DRIVERS_PS2_KEYBOARD is enabled. The latter
enables the translation unconditionally. For QEMU this means that
the option effectively toggles the translation, as QEMU's controller
has it disabled by default. This probably made a lot of earlier
testing inconsistent.
Fixes: commit a95a6bf646 (libpayload/drivers/i8402/kbd: Fix qemu)
The reset introduced there effectively reverted the scancode
selection made before (because 2 is the default). It's unclear
if later changes to the code were only necessary to work
around it.
Change-Id: Iad85af516a7b9f9c0269ff9652ed15ee81700057
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/46724
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Current implementation uses CPUID 0Bh function that returns the number
of logical cores of requested level. The problem with this approach is
that this value doesn't change when HyperThreading is disabled (it's in
the Intel docs), so it breaks generate_cpu_entries() because `numcpus`
ends up being zero due to integer division truncation.
- Use MSR 0x35 instead, which returns the correct number of logical
processors with and without HT.
- Use cpu_read_topology() to gather the required information
Tested on Prodrive Hermes, the ACPI code is now generated even with
HyperThreading disabled.
Change-Id: Id9b985a07cd3f99a823622f766c80ff240ac1188
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48691
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
According to doc# IHD-OS-BXT-Vol 2b-05.17 the cycle delay is in the bit
range 8:4 of register PP_CONTROL. The current code writes the value to
bits 4:0, though. Correct that by shifting the value left by 4 bits.
Change-Id: If407932c847da39b19e307368c9e52ba1c93bccd
Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48759
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
The CHROMEOS option was never used with ibexpeak, code was copy-pasted
and forked from bd82x6x. Since a custom ibexpeak/nvs.h was already made,
an accompanying globalnvs.asl is added here too without chromeos_acpi_t.
Change-Id: I16406516b51c13d49593bc8a3e1e5b868eea6f24
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48766
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
