The stack used on the ASRock E350M1 is significantly less than
what we currently set (64k per core). In fact, we use about half
of the default stack size (4k) on core 0 and even less on non
BSP cores [1]:
$ grep stack coreboot_without_patch_but_monotonic_timer.log
CPU1: stack_base 002a0000, stack_end 002afff8
CPU1: stack: 002a0000 - 002b0000, lowest used address 002afda8, stack used: 600 bytes
CPU0: stack: 002b0000 - 002c0000, lowest used address 002bf75c, stack used: 2212 bytes
Removing the Kconfig variable STACK_SIZE to use the default results
in the following numbers of stack usage.
$ grep stack coreboot_with_patch.log
CPU1: stack_base 00287000, stack_end 00287ff8
CPU1: stack: 00287000 - 00288000, lowest used address 00287da8, stack used: 600 bytes
CPU0: stack: 00288000 - 00289000, lowest used address 0028875c, stack used: 2212 bytes
[1] http://review.coreboot.org/#/c/3154/
(comment May 2 10:21 AM)
Change-Id: Ibdb2102c86094fce3787e3b5a162ca8423de205c
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Tested-by: Paul Menzel <paulepanter@users.sourceforge.net>
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/3209
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
Due to
$ more src/southbridge/amd/cimx/sb800/Makefile.inc
[…]
romstage-y += cfg.c
romstage-y += early.c
romstage-y += smbus.c
ramstage-y += cfg.c
ramstage-y += late.c
[…]
`src/southbridge/amd/cimx/sb800/` is passed with the switch `-I` to
the compiler, where it is also going to find the header file
`sb_cimx.h`. Therefore use `#include <sb_cimx>` everywhere, which is
what some AMD SB800 based boards already do.
The only effect is, that the compiler will not needlessly look into
directories which do not contain the header file [1].
The following command was used for the replacement.
$ git grep -l sb_cimx.h src/mainboard/ | xargs sed -i 's/#include "sb_cimx.h"/#include <sb_cimx.h>/'
[1] http://gcc.gnu.org/onlinedocs/cpp/Search-Path.html
Change-Id: I96ab34bac1524e6c38c85dfe9d99cb6ef55e6d7c
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/3118
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
This is the same split as was done on the Persimmon.
Change-Id: I25bd63f23417b7926232f07eaaa7917170af9d60
Signed-off-by: Mike Loptien <mike.loptien@se-eng.com>
Reviewed-on: http://review.coreboot.org/3050
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
Now that the ASRock E350M1 builds without any warnings, remove the
config option `WARNINGS_ARE_ERRORS` set to no by default from
the file `Kconfig` so warnings are treated as errors to prevent
code from being added in the future introducing warnings.
Change-Id: Idfecfb1434158969334a4b37972b5fc6fd76e72a
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/3014
Tested-by: build bot (Jenkins)
Reviewed-by: Marc Jones <marc.jones@se-eng.com>
When building the ASRock E350M1, the following warnings are shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/buildOpts.romstage.o
In file included from src/mainboard/asrock/e350m1/buildOpts.c:294:0:
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2071:6: warning: "DDR1333_FREQUENCY" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2071:40: warning: "DDR1866_FREQUENCY" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2089:5: warning: "TIMING_MODE_AUTO" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2089:31: warning: "TIMING_MODE_SPECIFIC" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2113:5: warning: "QUADRANK_UNBUFFERED" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2113:33: warning: "QUADRANK_UNBUFFERED" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2127:5: warning: "POWER_DOWN_BY_CHIP_SELECT" is not defined [-Wundef]
src/vendorcode/amd/agesa/f14/Include/PlatformInstall.h:2127:28: warning: "POWER_DOWN_BY_CHIP_SELECT" is not defined [-Wundef]
[…]
Adding the corresponding defines as done for AMD Persimmon in
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Reviewed-on: http://review.coreboot.org/137
addresses the warnings.
Change-Id: Id311b2dacdba5f2e6b4d834e43db0310213a35f9
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2962
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
When building the ASRock E350M1, the following warning is shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/mptable.ramstage.o
src/mainboard/asrock/e350m1/mptable.c:64:12: warning: unused variable 'dev' [-Wunused-variable]
[…]
Removing the variable `dev` addresses the warning.
The same change was done in the following commit for the
AMD Persimmon board.
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Reviewed-on: http://review.coreboot.org/137
Change-Id: I83f4630cb6ab1e4c95d04b4e8423850ed1858e45
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2965
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
When building the ASRock E350M1, the following warning is shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/mptable.ramstage.o
src/mainboard/asrock/e350m1/mptable.c: In function 'smp_write_config_table':
src/mainboard/asrock/e350m1/mptable.c:58:3: warning: implicit declaration of function 'get_bus_conf' [-Wimplicit-function-declaration]
[…]
Including the header file `cpu/amd/amdfam14.h` declaring the
function addresses this warning.
The same change was done in the following commit for the
AMD Persimmon board.
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Reviewed-on: http://review.coreboot.org/137
Change-Id: I7912571fa57f6512b10fc9b5845427fcb6eb50c0
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2966
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
When building the ASRock E350M1, the following warning is shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/mainboard.ramstage.o
src/mainboard/asrock/e350m1/mainboard.c: In function 'mainboard_enable':
src/mainboard/asrock/e350m1/mainboard.c:63:2: warning: implicit declaration of function 'pm_iowrite' [-Wimplicit-function-declaration]
[…]
This warning was introduced by moving the initialization of the
ASF registers using `pm_iowrite` to `mainboard.c` in
commit db6c5bfd8b
Author: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Date: Thu Mar 21 22:21:28 2013 +0100
Asrock E350M1: Use SPD read code from F14 wrapper
Reviewed-on: http://review.coreboot.org/2875
and is fixed by including `southbridge/amd/cimx/cimx_util.h`
declaring `pm_iowrite`.
Note, that the other AMD SB800 based boards seem to use the
header file `southbridge/amd/sb800/sb800.h`, so no warning is shown
for those. But since the CIMx SB800 code is used, the routines
from the CIMx directory are more appropriate to declare these functions.
So delete the commented out include line for this header too.
Change-Id: I179aad5157c5a91294339a3e7b6c4c1715c6f099
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2957
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
When building the ASRock E350M1, the following warning is shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/irq_tables.ramstage.o
src/mainboard/asrock/e350m1/irq_tables.c: In function 'write_pirq_routing_table':
src/mainboard/asrock/e350m1/irq_tables.c:64:2: warning: implicit declaration of function 'get_bus_conf' [-Wimplicit-function-declaration]
[…]
Including the header file `cpu/amd/amdfam14.h` declaring the
function addresses this warning.
The same change was done in the following commit for the
AMD Persimmon board.
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Reviewed-on: http://review.coreboot.org/137
Change-Id: I40b5735feb7116961ca0c4d6940ec55cdf42d3c6
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2956
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
When building the ASRock E350M1, the following warning is shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/get_bus_conf.ramstage.o
src/mainboard/asrock/e350m1/get_bus_conf.c: In function 'get_bus_conf':
src/mainboard/asrock/e350m1/get_bus_conf.c:82:3: warning: implicit declaration of function 'agesawrapper_amdinitlate' [-Wimplicit-function-declaration]
[…]
Including the header file `agesawrapper.h` declaring the function
`agesawrapper_amdinitlate` fixes this warning.
All AMD Family 14 based boards already include that header file. For
example for the board AMD Persimmon the following patch fixed this
warning.
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Reviewed-on: http://review.coreboot.org/137
Change-Id: I695420b7071e07cb7d4667b2479b9a26ea13723d
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2955
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
When building the ASRock E350M1, the following warning is shown.
$ make # on Jenkins (build server)
[…]
CC mainboard/asrock/e350m1/PlatformGnbPcie.romstage.o
CC mainboard/asrock/e350m1/agesawrapper.romstage.o
CC mainboard/asrock/e350m1/buildOpts.romstage.o
src/mainboard/asrock/e350m1/PlatformGnbPcie.c: In function 'OemCustomizeInitEarly':
src/mainboard/asrock/e350m1/PlatformGnbPcie.c:131:5: warning: 'return' with a value, in function returning void [enabled by default]
[…]
The function signature is (the return type might not be part of this though [1]),
VOID
OemCustomizeInitEarly (
IN OUT AMD_EARLY_PARAMS *InitEarly
)
so do not return anything.
All other AMD Family 14 boards already have the correct code. For example
following commit fixed this for AMD Persimmon.
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Reviewed-on: http://review.coreboot.org/137
[1] http://cboard.cprogramming.com/cplusplus-programming/117286-what-exactly-function-signature.html
Change-Id: Ie60246bd9bb8452efd096e6838d8610f6364a6aa
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2954
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
Changes:
- Get rid of the E350M1 mainboard specific code and use the
platform generic function wrapper that was added in change
http://review.coreboot.org/#/c/2497/
AMD f14: Add SPD read functions to wrapper code
- Move DIMM addresses into devicetree.cb
- Add the ASF init that used to be in the SPD read code into
mainboard_enable()
Notes:
- The DIMM reads only happen in romstage, so the function is not
available in ramstage. Point the read-SPD callback to a generic
function in ramstage.
Change-Id: I08c2aebc62facc14f94400ee1ad188901ba73f19
Signed-off-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Reviewed-on: http://review.coreboot.org/2875
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins)
Here's the great news: From now on you don't have to worry about
hitting the right io.h include anymore. Just forget about romcc_io.h
and use io.h instead. This cleanup has a number of advantages, like
you don't have to guard device/ includes for SMM and pre RAM
anymore. This allows to get rid of a number of ifdefs and will
generally make the code more readable and understandable.
Potentially in the future some of the code in the io.h __PRE_RAM__
path should move to device.h or other device/ includes instead,
but that's another incremental change.
Change-Id: I356f06110e2e355e9a5b4b08c132591f36fec7d9
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Reviewed-on: http://review.coreboot.org/2872
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
Adding the 'WordBusNumber' macro to the PCI0
CRES ResourceTemplate in the Persimmon DSDT.
This sets up the bus number for the PCI0 device
and the secondary bus number in the CRS method.
This change came in response to a 'dmesg' error
which states:
'[FIRMWARE BUG]: ACPI: no secondary bus range in _CRS'
By adding the 'WordBusNumber' macro, ACPI can set
up a valid range for the PCIe downstream busses,
thereby relieving the Linux kernel from "guessing"
the valid range based off _BBN or assuming [0-0xFF].
The Linux kernel code that checks this bus range is
in `drivers/acpi/pci_root.c`. PCI busses can have
up to 256 secondary busses connected to them via
a PCI-PCI bridge. However, these busses do not
have to be sequentially numbered, so leaving out a
section of the range (eg. allowing [0-0x7F]) will
unnecessarily restrict the downstream busses.
This is the same change as made to Persimmon with
change-id I44f22:
http://review.coreboot.org/#/c/2592/
Change-Id: I5184df8deb7b5d2e15404d689c16c00493eb01aa
Signed-off-by: Mike Loptien <mike.loptien@se-eng.com>
Reviewed-on: http://review.coreboot.org/2736
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
I am removing the _INI method from the AZHD device because
it does not seem to do anything and causes errors in the
FWTS[1] (Firmware Test Suite) test 'method'. The INI
method performs device specific initialization and is
run when OSPM loads a description table. It must only
access OperationRegions that have been indicated as
available by the _REG (Region) method. We do not have a
_REG method and during my testing, I added a REG method
but it did not seem to make a difference in the PCI
register space. The bit fields defined as NSDI (Disable
No Snoop), NSDO (Disable No Snoop Override), and NSEN
(Enable No Snoop Request) do not ever get written from
their default values. And writing to these bit fields
does not seem to be necessary because I did not notice
any change in audio functionality.
In an effort to clean up as many FWTS errors as possible,
I propose removing this method altogether. I have seen no
change in operation (audio works with and without this
method) and there does not seem to be any change in lspci
or dmesg.
FWTS information can be found here:
[1]: https://wiki.ubuntu.com/Kernel/Reference/fwts
This is the same change as made to Persimmon in
Change-ID If8d86f:
http://review.coreboot.org/#/c/2726/
Change-Id: Iae70c3d0af1cdaca31b206ad6daba4d38ee6b780
Signed-off-by: Mike Loptien <mike.loptien@se-eng.com>
Reviewed-on: http://review.coreboot.org/2742
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
The _OSC method is used to tell the OS what capabilities
it can take control over from the firmware. This method
is described in chapter 6.2.9 of the ACPI spec v3.0.
The method takes 4 inputs (UUID, Rev ID, Input Count,
and Capabilities Buffer) and returns a Capabilites
Buffer the same size as the input Buffer. This Buffer
is generally 3 Dwords long consisting of an Errors
Dword, a Supported Capabilities Dword, and a Control
Dword. The OS will request control of certain
capabilities and the firmware must grant or deny control
of those features. We do not want to have control over
anything so let the OS control as much as it can.
The _OSC method is required for PCIe devices and dmesg
checks for its existence and issues an error if it is
not found.
This is the same change made to Persimmon with Change-ID
I149428:
http://review.coreboot.org/#/c/2684/
Change-Id: I2701d915338294bdade2ad334b22a51db980892e
Signed-off-by: Mike Loptien <mike.loptien@se-eng.com>
Reviewed-on: http://review.coreboot.org/2739
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Currently for Advansus A785E-I, ASRock E350M1 and ASUS M5A88-V
despite what is chosen in Kconfig »Chipset« menu item,
$ more .config
[…]
# CONFIG_ENABLE_IDE_COMBINED_MODE is not set
CONFIG_IDE_COMBINED_MODE=0x1
# CONFIG_SB800_SATA_IDE is not set
CONFIG_SB800_SATA_AHCI=y
# CONFIG_SB800_SATA_RAID is not set
CONFIG_SB800_SATA_MODE=0x2
[…]
the SATA controller is put into IDE mode.
$ lspci -nn | grep SATA
00:11.0 SATA controller [0106]: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 SATA Controller [IDE mode] [1002:4390] (rev 40)
Commit »sb800: Add sata ahci/raid mode kconfig option«
(d4a0e7d0) [1] added the options above to configure the mode
using Kconfig and some SB800 boards were adapted already. For
example commit »persimmon: sb800 sata mode configure update«
(1386fa74) [2] did so for AMD Persimmon.
Doing the same by assigning the Kconfig variable to the value in
`platform_cfg.h` integrates this with the three remaining boards
listed above.
The patch is successfully tested with the ASRock E350M1.
$ lspci -nn | grep SATA
00:11.0 SATA controller [0106]: Advanced Micro Devices [AMD] nee ATI SB7x0/SB8x0/SB9x0 SATA Controller [AHCI mode] [1002:4391] (rev 40)
[1] http://review.coreboot.org/225
[2] http://review.coreboot.org/227
Change-Id: I227257e2c8f04f18c27ff00fe62d42e372de67e4
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2610
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
Quoting Jens Rottmann [1]:
Nevertheless I still think this whole function is bogus for the E350M1. The
function assumes GPIO21 is wired to reset APU PCIe lane 0+1 (PCIe x8, port 4+5
as Coreboot/AGESA calls it), GPIO25 resets lane 2 (PCIe x4) and GPIO02 lane 3.
But the E350M1 has PCIe x16 i.e. probably APU lanes 0-3 bundled, completely
different layout. They could have chosen GPIO21 to force resets, or 25 - or
maybe 50 like on the Persimmon or any other they fancied or - and this is the
most probable - none at all. Having BiosGnbPcieSlotReset() toggle some GPIOs
without knowing what they do on the E350M1 (if anything at all) is nonsense.
In my opinion this whole function should just "return AGESA_UNSUPPORTED" and
good riddance.
[1] http://review.coreboot.org/#/c/2445/
Change-Id: Iac66da41182e838c7e6925250cc3982adbb3e4ec
Reported-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2489
Tested-by: build bot (Jenkins)
Reviewed-by: Marc Jones <marc.jones@se-eng.com>
Reviewed-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Since the merg of the ASRock E350M1 port (a649a96e) the compiler
warns about the following [1].
mainboard.c:35, GNU Compiler 4 (gcc), Priorität: Normal
no previous prototype for 'set_pcie_reset' [-Wmissing-prototypes]
mainboard.c:43, GNU Compiler 4 (gcc), Priorität: Normal
no previous prototype for 'set_pcie_dereset' [-Wmissing-prototypes]
Adding the function prototypes to the beginning of the file as
done in commit »Persimmon updates for AMD F14 rev C0« (d7a696d0)
addresses the warning.
[1] http://qa.coreboot.org/job/coreboot-gerrit/4975/warnings13Result/package.-139448264/file.-1544928473/
[2] http://review.coreboot.org/137
Change-Id: Iad2e62ec37c3a2f749a264974b61ac7c226e9b83
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2590
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
In the file `COPYING` in the coreboot repository and upstream [1]
just one space is used.
The following command was used to convert all files.
$ git grep -l 'MA 02' | xargs sed -i 's/MA 02/MA 02/'
[1] http://www.gnu.org/licenses/gpl-2.0.txt
Change-Id: Ic956dab2820a9e2ccb7841cab66966ba168f305f
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2490
Tested-by: build bot (Jenkins)
Reviewed-by: Anton Kochkov <anton.kochkov@gmail.com>
Commit 8487229b (Persimmon doesn't have HDMI so the GNB HD Audio should be
disabled.) turned off the device in AGESA. Now remove it from
devicetree.cb, too. This prevents the following boot message:
PCI: Left over static devices:
PCI: 00:01.1
PCI: Check your devicetree.cb.
Also clarify the line's comment a bit for the Fam14 boards which still
retain this device (to counter the loss of information ;-).
Change-Id: Ib671ed2e0d04bdef2869e8d70208d6e55cdea3fd
Signed-off-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Reviewed-on: http://review.coreboot.org/2537
Tested-by: build bot (Jenkins)
Reviewed-by: Peter Stuge <peter@stuge.se>
This patch reduces unnecessary differences between AMD Inagua, Persimmon,
Union Station, South Station and Asrock E350M1. It's only cosmetical, but
makes them a little bit easier to compare.
This is the remainder of the original http://review.coreboot.org/2464,
parts of which somehow got lost in a flurry of refactoring and splitting
patches.
Change-Id: I034228be9edaaa4122506763d7bb4158f8e0ec53
Signed-off-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Reviewed-on: http://review.coreboot.org/2529
Tested-by: build bot (Jenkins)
Reviewed-by: Peter Stuge <peter@stuge.se>
Currently on for example on AMD Persimmon and ASRock E350M1 Linux
complains, that the PBLK length is invalid [1].
ACPI: Invalid PBLK length [0]
Consequently, frequency scaling might not work correctly, though for
these two boards it seems to work according to PowerTOP.
Indeed, according to the ACPI specification [2], setting PBlockLength
to 0 is only allowed if there is no PBlockAddress. Otherwise it has to
be set to 6.
18.5.93 Processor (Declare Processor)
[…]
PBlockAddress provides the system I/O address for the processors
register block. Each processor can supply a different such
address. PBlockLength is the length of the processor register
block, in bytes and is either 0 (for no P_BLK) or 6. With one
exception, all processors are required to have the same
PBlockLength. The exception is that the boot processor can have
a non-zero PBlockLength when all other processors have a zero
PBlockLength. It is valid for every processor to have a
PBlockLength of 0.
And that is exactly what Linux is checking in
`drivers/acpi/processor_driver.c` [3].
static int acpi_processor_get_info(struct acpi_device *device)
{
[…]
/*
* On some boxes several processors use the same processor bus id.
* But they are located in different scope. For example:
* \_SB.SCK0.CPU0
* \_SB.SCK1.CPU0
* Rename the processor device bus id. And the new bus id will be
* generated as the following format:
* CPU+CPU ID.
*/
sprintf(acpi_device_bid(device), "CPU%X", pr->id);
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Processor [%d:%d]\n", pr->id,
pr->acpi_id));
if (!object.processor.pblk_address)
ACPI_DEBUG_PRINT((ACPI_DB_INFO, "No PBLK (NULL address)\n"));
else if (object.processor.pblk_length != 6)
printk(KERN_ERR PREFIX "Invalid PBLK length [%d]\n",
object.processor.pblk_length);
else {
pr->throttling.address = object.processor.pblk_address;
pr->throttling.duty_offset = acpi_gbl_FADT.duty_offset;
pr->throttling.duty_width = acpi_gbl_FADT.duty_width;
pr->pblk = object.processor.pblk_address;
/*
* We don't care about error returns - we just try to mark
* these reserved so that nobody else is confused into thinking
* that this region might be unused..
*
* (In particular, allocating the IO range for Cardbus)
*/
request_region(pr->throttling.address, 6, "ACPI CPU throttle");
}
[…]
}
This issue has proliferated to all AMD based boards so fix it for
all of them by setting P_BLK length to 6.
The DSDT of for example AMD Parmer and AMD Thatcher also set it
to 6 everywhere so this solution is taken instead of setting the
P_BLK system I/O base to 0 for all but the first processor which
is how it is done for earlier AMD based boards.
As note having to set this manually should not be needed and
this should be autogenerated as done for most of the Intel boards
and the AMD K8 based boards (`src/cpu/amd/model_fxx/powernow_acpi.c`).
[1] http://www.coreboot.org/pipermail/coreboot/2013-January/073636.html
[2] http://acpi.info/DOWNLOADS/ACPIspec40a.pdf
[3] http://git.kernel.org/?p=linux/kernel/git/torvalds/linux.git;a=blob;f=drivers/acpi/processor_driver.c;h=e83311bf1ebdaaaea1adbf2de1351cca907d3465;hb=5da1f88b8b727dc3a66c52d4513e871be6d43d19#l351
Tested-by: Paul Menzel <paulepanter@users.sourceforge.net>
• ASRock E350M1:
Tested-by: Paul Menzel <paulepanter@users.sourceforge.net>
• AMD Persimmon:
Tested-by: Martin Roth <martin.roth@se-eng.com>
Change-Id: Ie79fe4812532d124cc81747c75a4f3d88d00531c
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2189
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
The DSDT header contains the fields OEMID and OEM Table ID. See
for example ACPI specification 4.0a [1]
5.2.11.1 Differentiated System Description Table (DSDT)
on page 135. There Table 5-16 contains the descriptions.
Field Byte Length Byte Offset Description
===================================================
OEMID 6 10 OEM ID
OEM Table ID 8 16 The manufacture model ID.
Currently in coreboot there is no common method what to put in
these fields.
Mostly Intel based boards populate it with "CORE " ore "COREv4"
and AMD based boards populate it with the board vendor and
model number, abbreviated appropriately to fit into these fields.
On most boards the proprietary vendor BIOS seems to leave these
fields – displayed with `sudo dmidecode` under System Information –
blank
To Be Filled By O.E.M.
and fill out the Base Board Information with the board vendor and
model name.
In [2] Jens Rottmann argues that the this is really just the table
ID used for naming it and that »99% of the DSDT code is not board
specific«.
Both approaches seem to have their advantages, but using the
second one, developers often seem to forget to update them (for
example AMD Thather).
The current situation is at least not optimal. and therefore at
least unify the string in the OEM Table ID. If unifying the
OEM ID is also a good idea this should be done too.
If later on it should be decided that the board vendor and model
should be used again, this should be somehow derived from
Kconfig.
The following command was used for the change [3].
$ git grep -l '\/\* TABLE ID \*\/' | xargs sed -i '/TABLE ID/s/"\([^"]*\)"/"COREBOOT"/'
This patch is split out from [2].
[1] http://www.acpi.info/spec40a.htm
[2] http://review.coreboot.org/#/c/2464/
[3] http://stackoverflow.com/questions/5207838/sed-regex-matching-text-between-to-double-quotes-when-a-certain-text-appears-i
Change-Id: Iec98c615ce37f928abc1b500eff5aa865d772cb2
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2472
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
To reduce the differences between these file name the enabling
device function in the directory `src/mainboard` uniformly
`mainboard_enable` [1].
Thanks to the awesome help of gnomon and BlastHardcheese in the
IRC channel #sed on <irc.freenode.net>. gnomon came up with the
following command to do the actual work.
$ cd src/mainboard
$ for f in */*/mainboard.c ; \
> do src="$(awk '/\.enable_dev = /{v=$NF; sub(/,$/,"",v); print v}' "$f")" ; \
> [[ -z $src ]] && continue ; \
> printf '%s\n' "g/${src}/s/${src}\([,(]\)/mainboard_enable\1/p" w | ed -s "$f" ; \
> done
`src/mainboard/digitallogic/msm586seg/mainboard.c` and
`src/mainboard/technologic/ts5300/mainboard.c` had to be adapted
manually as no comma was used separating the struct members.
And with the following statement, gnomon is even more likable!
My pleasure entirely. Good luck with coreboot; I'm a big fan of the project.
[1] http://www.coreboot.org/pipermail/coreboot/2013-February/074548.html
Change-Id: Ife9cd0c2d9cc1ed14afc6d40063450553f06a6c6
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2493
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Reduce unnecessary differences between AMD based boards only
using the file `platform_cfg.h` for configuration making them
a little bit easier to compare.
Inagua & co. mention the board name in several places which are really not
that board specific. Sometimes people even forget to change it:
Union Station’s platform_cfg.h starts with "#ifndef _PERSIMMON_CFG_H_".
Funny. Change that to "_PLATFORM_CFG_H_" everywhere.
The following command was used.
$ find . -name platform_cfg.h | xargs sed -i '/_CFG_H_/s/_.*_/_PLATFORM_CFG_H_/'
More boards seem to use that kind of naming (`git grep _CFG_H_`)
but it is not certain that this will not break anything as for
example the board AMD Dinar also has header files for
configuration stuff for the north- and southbridge.
$ git grep _CFG_H_
[…]
src/mainboard/amd/dinar/platform_cfg.h:#ifndef _PLATFORM_CFG_H_
src/mainboard/amd/dinar/platform_cfg.h:#define _PLATFORM_CFG_H_
src/mainboard/amd/dinar/platform_cfg.h:#endif //_PLATFORM_CFG_H_
src/mainboard/amd/dinar/rd890_cfg.h:#ifndef _RD890_CFG_H_
src/mainboard/amd/dinar/rd890_cfg.h:#define _RD890_CFG_H_
src/mainboard/amd/dinar/rd890_cfg.h:#endif //_RD890_CFG_H_
src/mainboard/amd/dinar/sb700_cfg.h:#ifndef _SB700_CFG_H_
src/mainboard/amd/dinar/sb700_cfg.h:#define _SB700_CFG_H_
src/mainboard/amd/dinar/sb700_cfg.h:#endif //_SB700_CFG_H
[…]
Change-Id: Ida15fa6a7adfc770240ac30e795946000dae3f16
Signed-off-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2464
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
There were just whitespace differences and three boards did not
contain
printk(BIOS_DEBUG, "alib\n");
dump_mem(ssdt, ((void *)alib) + alib->length);
which is enclosed `#if DUMP_ACPI_TABLES == 1` to dump the ACPI
tables.
Basically the whitespace in the license header in Inagua’s file
was fixed and then the file copied over to the other directories.
Change-Id: I23f73acad427b5ec14cf51651af67240871f7488
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2470
Tested-by: build bot (Jenkins)
Reviewed-by: Alvaro G. <andor@pierdelacabeza.com>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
For AMD Inagua, the following two commits
commit 01f7ab9335
Author: Kerry Sheh <shekairui@gmail.com>
Date: Thu Jan 19 13:18:36 2012 +0800
Inagua: Synchronize AMD/inagua mainboard.
Reviewed-on: http://review.coreboot.org/542
and
commit d91c9b7e3c
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 10:59:55 2011 -0600
AMD Inagua platform updates
Reviewed-on: http://review.coreboot.org/136
replaced the constant `iobase` is set to by the define `SMBUS0_BASE_ADDRESS` from `OEM.h`.
Do the same for AMD Persimmon, South Station, Union station and ASRock E350M1.
Change-Id: If095cd9d9b28b118b4072c7c9d345bf620b774c9
Signed-off-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2453
Tested-by: build bot (Jenkins)
Looking at AssertSlotReset, the comments and all other case's it's
obvious this is a simple copy & paste error where someone just forgot
to change one occurrance of the GPIO nr. Also the AMD Inagua
schematics show that GPIO02 is what they really meant.
Also forward the fix to boards copied from Inagua (AMD South
Station, Union Station, Asrock E350M1).
Change-Id: I6b9a3d473245fa27604b2f148a730290277a88ed
Signed-off-by: Jens Rottmann <JRottmann@LiPPERTembedded.de>
Reviewed-on: http://review.coreboot.org/2445
Tested-by: build bot (Jenkins)
Reviewed-by: Peter Stuge <peter@stuge.se>
The name lapic_cluster is a bit misleading, since the construct is not local
APIC specific by concept. As implementations and hardware change, be more
generic about our naming. This will allow us to support non-x86 systems without
adding new keywords.
Change-Id: Icd7f5fcf6f54d242eabb5e14ee151eec8d6cceb1
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Reviewed-on: http://review.coreboot.org/2377
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
The name pci_domain was a bit misleading, since the construct is only
PCI specific in a particular (northbridge/cpu) implementation, but not
by concept. As implementations and hardware change, be more generic
about our naming. This will allow us to support non-PCI systems without
adding new keywords.
Change-Id: Ide885a1d5e15d37560c79b936a39252150560e85
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Reviewed-on: http://review.coreboot.org/2376
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
The following command was used to correct the grammatical mistake.
$ git grep -l 'This is the stub function will call' | xargs sed -i s,This is the stub function will call,This stub function will call, '{}'
sed: -e Ausdruck #1, Zeichen 6: Nicht beendeter `s'-Befehl
As this file seems to have been copied around a lot, it originally
seems to have come with the following commit for AMD Persimmon and
AMD Inagua.
commit 69da1b676c
Author: Frank Vibrans <frank.vibrans@amd.com>
Date: Mon Feb 14 19:04:45 2011 +0000
Add IBASE DB-FT1 and AMD Inagua motherboards. Patch 8 of 8.
Change-Id: I2e6630a5172738b01e6def7062284f167e5508b1
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2268
Tested-by: build bot (Jenkins)
Reviewed-by: Marc Jones <marcj303@gmail.com>
[…]
CC romstage.inc
src/mainboard/asrock/e350m1/romstage.c: In function 'cache_as_ram_main':
src/mainboard/asrock/e350m1/romstage.c:48:5: warning: unused variable 'reg8' [-Wunused-variable]
This change was already done for AMD Persimmon in the following
commit.
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
Change-Id: I8f1ae1a609b87b197583934f0556f66b64e6994d
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2230
Tested-by: build bot (Jenkins)
Reviewed-by: Peter Stuge <peter@stuge.se>
commit 585a400697
Author: zbao <fishbaozi@gmail.com>
Date: Thu Apr 12 11:27:26 2012 +0800
Leverage the Pstate table created by AGESA.
… introduced unneeded whitespace in front of a comma.
Revert that part of the above commit. In the file for AMD Dinar
tabs and spaces are mixed, but leave that alone for the beginning.
Change-Id: I279cd0cb0be8c79258034733773f2ae1c2207cce
Signed-off-by: Paul Menzel <paulepanter@users.sourceforge.net>
Reviewed-on: http://review.coreboot.org/2187
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin.roth@se-eng.com>
Size of BiosCallouts[] struct can be calculated as:
CallOutCount = sizeof (BiosCallouts) / sizeof (BiosCallouts [0]);
There is no longer need for REQUIRED_CALLOUTS define.
Originally that change was done for AMD Persimmon in
commit d7a696d0f2
Author: efdesign98 <efdesign98@gmail.com>
Date: Thu Sep 15 15:24:26 2011 -0600
Persimmon updates for AMD F14 rev C0
without deleting the define. This was ported to some of the other
boards and for some the define was not removed.
The AMD Inagua, Parmer and Thatcher boards were already adapted but
the define was left in. So just remove it for those.
Tested on Supermicro H8QGI.
Change-Id: Ia09795579a1170fa20ab94a30feb1af6821153d2
Signed-off-by: Aladyshev Konstantin <aladyshev@nicevt.ru>
Reviewed-on: http://review.coreboot.org/2049
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins)
Reviewed-by: Martin Roth <martin@se-eng.com>
Reviewed-by: Marc Jones <marcj303@gmail.com>
This gets rid of the somewhat unstructured placement of AMD's
sysinfo structure in CAR.
We used to carve out some CAR space using a Kconfig variable,
and then put sysinfo there manually (by "virtue" of pointer magic).
Now it's a variable with the CAR_GLOBAL qualifier, and build
system magic.
For this, the following steps were done (but must happen together
since the intermediates won't build):
- Add new CAR_GLOBAL sysinfo_car
- point all sysinfo pointers to sysinfo_car instead of GLOBAL_VAR
- remove DCACHE_RAM_GLOBAL_VAR_SIZE
- from CAR setup (no need to reserve the space)
- commented out code (that was commented out for years)
- only copy sizeof(sysinfo) into RAM after ram init, where
before it copied the whole GLOBAL_VAR area.
- from Kconfig
Change-Id: I3cbcccd883ca6751326c8e32afde2eb0c91229ed
Signed-off-by: Patrick Georgi <patrick@georgi-clan.de>
Reviewed-on: http://review.coreboot.org/1887
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Reviewed-by: Marc Jones <marcj303@gmail.com>
- Optionally override FSB clock detection in generic
LAPIC code with constant value.
- Override on AMD Model fxx, 10xxx, agesa CPUs with 200MHz
- compile LAPIC code for romstage, too
- Remove #include ".../apic_timer.c" in AMD based mainboards
- Remove custom udelay implementation from intel northbridges' romstages
Future work:
- remove the compile time special case
(requires some cpuid based switching)
- drop northbridge udelay implementations (i945, i5000) if
not required anymore (eg. can SMM use the LAPIC timer?)
Change-Id: I25bacaa2163f5e96ab7f3eaf1994ab6899eff054
Signed-off-by: Patrick Georgi <patrick.georgi@secunet.com>
Reviewed-on: http://review.coreboot.org/1618
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Tested-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
This patch is the beginning of a Kconfig cleanup series
- drop CONFIG_HAVE_BUS_CONFIG and add get_bus_conf.c if it
exists in the mainboard directory
- drop duplicate ACPI_SSDTX_NUM from mainboard Kconfig
if it only defines the defaul value of 0
- Add mptable.c, fadt.c, reset.c and ssdtX.asl when they
exist, not based on some Kconfig magic
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Change-Id: Ia14a7116dad6a724af7e531920fee9a51fd0b200
Reviewed-on: http://review.coreboot.org/1832
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Compose the name from Kconfig strings instead.
As the field is for debug print use only, a minor change in the output
should do no harm. The strings no longer include word "Mainboard".
Change-Id: Ifd24f408271eb5a5d1a08a317512ef00cb537ee2
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1635
Tested-by: build bot (Jenkins)
Reviewed-by: Patrick Georgi <patrick@georgi-clan.de>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
The includes removed here were previously required for
struct lb_memory and lb_add_memory_range().
Change-Id: Ie6c0d4ef55c2225aa709cf3fbad30ff1080e3610
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1391
Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Tested-by: build bot (Jenkins)
These existed to provide a hook to add reserved memory regions
in the coreboot memory table. Reserved memory are now
added as resources.
Change-Id: I9f83df33845cfa6973b018a51cf9444dbf0f8667
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1414
Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Tested-by: build bot (Jenkins)
Reviewed-by: Peter Stuge <peter@stuge.se>
UMA region can be determined at any time after the amount
of RAM is known and before the uma_resource() call.
Change-Id: I2a0bf2d3cad55ee70e889c88846f962b7faa0c7e
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1379
Reviewed-by: Zheng Bao <zheng.bao@amd.com>
Tested-by: build bot (Jenkins)
Reviewed-by: Anton Kochkov <anton.kochkov@gmail.com>
This reverts commit 78efc4c36c.
The broadcast patch was reverted, so this commit should also
be reverted. The reason for reverting the broadcast patch:
It turned out that sending IPIs via broadcast doesn't work on
Sandybridge. We tried to come up with a solution, but didn't
found any so far. So revert the code for now until we have
a working solution.
Change-Id: I05c27dec55fa681f455215be56dcbc5f22808193
Signed-off-by: Sven Schnelle <svens@stackframe.org>
Reviewed-on: http://review.coreboot.org/1380
Tested-by: build bot (Jenkins)
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
mainboard_config never worked right, at least not since we've had sconfig.
Hence, drop mainboard/<vendor>/<device>/chip.h and fix up the mainboards that
tried to use it anyways.
Change-Id: I7cd403ea188d8a9fd4c1ad15479fa88e02ab8e83
Signed-off-by: Stefan Reinauer <reinauer@google.com>
Reviewed-on: http://review.coreboot.org/1359
Tested-by: build bot (Jenkins)
Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Ronald G. Minnich <rminnich@gmail.com>
Use of uma_resource() in AMD northbridge code created a memory
resource marked as reserved. Such resources are removed
from system memory in write_coreboot_table().
Change-Id: Ib5e49e851d6622d8ece9d6d612e245b3962b9167
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1233
Tested-by: build bot (Jenkins)
Reviewed-by: Rudolf Marek <r.marek@assembler.cz>
These boards had identical UMA code:
amd/dbm690t
amd/pistachio
technexion/tim5690
technexion/tim8690
The ones below had whitespace or debug level change
compared to the one above:
kontron/kt690
siemens/sitemp_g1p1
These boards use AMDFAM10 guidelines in code:
asrock/939a785gmh
amd/mahogany
Change-Id: Id7c3f48035727f5847f2d7c3a6e87a3d15582003
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1210
Tested-by: build bot (Jenkins)
Reviewed-by: Patrick Georgi <patrick@georgi-clan.de>
Following boards had identical code:
amd/inagua
amd/persimmon
The following had only whitespace or debug level changes
compared to ones above.
amd/union_station
amd/south_station
asrock/e350m1
Change-Id: I11ee46e06e1dd510cba551166189ebcaa144464b
Signed-off-by: Kyösti Mälkki <kyosti.malkki@gmail.com>
Reviewed-on: http://review.coreboot.org/1208
Tested-by: build bot (Jenkins)
Reviewed-by: Patrick Georgi <patrick@georgi-clan.de>
