Attention is currently required from: Damien Zammit, Angel Pons, Arthur Heymans.
Hello build bot (Jenkins), Damien Zammit, Angel Pons, Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/50353
to look at the new patch set (#3).
Change subject: nb/intel/x4x/raminit: Fix Clock Enable on DDR3
......................................................................
nb/intel/x4x/raminit: Fix Clock Enable on DDR3
On Asus P5QC this allows raminit to progress further (there are still
other issues).
Add a comment why the code differs from the reference code (or what
the blob on Intel boards does).
Change-Id: Ie1eb77ed2d2b563fb3b92ca33b900f7d1dbc85fe
Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz>
---
M src/northbridge/intel/x4x/raminit_ddr23.c
1 file changed, 14 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/50353/3
--
To view, visit https://review.coreboot.org/c/coreboot/+/50353
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ie1eb77ed2d2b563fb3b92ca33b900f7d1dbc85fe
Gerrit-Change-Number: 50353
Gerrit-PatchSet: 3
Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Damien Zammit
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Damien Zammit
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-MessageType: newpatchset
Attention is currently required from: HAOUAS Elyes.
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50286 )
Change subject: Documentation: Add chain-include list of headers
......................................................................
Patch Set 5:
(1 comment)
File Documentation/chain-include.md:
https://review.coreboot.org/c/coreboot/+/50286/comment/fabf690b_47f992ff
PS3, Line 33: * arch/io.h
> Files that call both pci_configX() and inb/outb() need to keep the direct <arch/io.h> include. […]
Done
--
To view, visit https://review.coreboot.org/c/coreboot/+/50286
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I474d0d4bd660b62c24508bc3eba67154e820d8a4
Gerrit-Change-Number: 50286
Gerrit-PatchSet: 5
Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Attention: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Comment-Date: Sat, 06 Feb 2021 09:21:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-MessageType: comment
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/50247 )
Change subject: Documentation: Codify some guidelines for headers and chain-including
......................................................................
Documentation: Codify some guidelines for headers and chain-including
There has been some repeated discussion about how header includes should
be formatted, specifically on the topic of chain-including. The coding
style currently doesn't say anything about the topic but clearly people
have some basic assumptions. This patch tries to codify some common
ground rules that are supposed to reflect the existing practice.
Signed-off-by: Julius Werner <jwerner(a)chromium.org>
Change-Id: Ibbcde306a814f52b3a41b58c7a33bdd99b0187e0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50247
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
Reviewed-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Reviewed-by: HAOUAS Elyes <ehaouas(a)noos.fr>
Reviewed-by: Angel Pons <th3fanbus(a)gmail.com>
---
M Documentation/coding_style.md
1 file changed, 39 insertions(+), 10 deletions(-)
Approvals:
build bot (Jenkins): Verified
Kyösti Mälkki: Looks good to me, approved
HAOUAS Elyes: Looks good to me, approved
Angel Pons: Looks good to me, but someone else must approve
diff --git a/Documentation/coding_style.md b/Documentation/coding_style.md
index ac0de4e..ca45096 100644
--- a/Documentation/coding_style.md
+++ b/Documentation/coding_style.md
@@ -834,22 +834,51 @@
out-of-range result. Typical examples would be functions that return
pointers; they use NULL or the ERR_PTR mechanism to report failure.
-Don't re-invent the kernel macros
-----------------------------------
+Headers and includes
+---------------
-The header file include/linux/kernel.h contains a number of macros that
-you should use, rather than explicitly coding some variant of them
-yourself. For example, if you need to calculate the length of an array,
-take advantage of the macro
+Headers should always be included at the top of the file, preferrably in
+alphabetical order. Includes should always use the `#include <file.h>`
+notation, except for rare cases where a file in the same directory that
+is not part of a normal include path gets included (e.g. local headers
+in mainboard directories), which should use `#include "file.h"`. Headers
+that can be included from both assembly files and .c files should keep
+all C code wrapped in `#ifndef __ASSEMBLER__` blocks, including includes
+to other headers that don't follow that provision.
+
+Files should generally include every header they need a definition from
+directly (and not include any unnecessary extra headers). Excepted from
+this are certain headers that intentionally chain-include other headers
+which logically belong to them and are just factored out into a separate
+location for implementation or organizatory reasons. This could be
+because part of the definitions is generic and part SoC-specific (e.g.
+`<gpio.h>` chain-including `<soc/gpio.h>`), architecture-specific (e.g.
+`<device/mmio.h>` chain-including `<arch/mmio.h>`), separated out into
+commonlib[/bsd] for sharing/license reasons (e.g. `<cbfs.h>`
+chain-including `<commonlib/bsd/cbfs_serialized.h>`) or just split out
+to make organizing subunits of a larger header easier. This can also
+happen when certain definitions need to be in a specific header for
+legacy POSIX reasons but we would like to logically group them together
+(e.g. `uintptr_t` is in `<stdint.h>` and `size_t` in `<stddef.h>`, but
+it's nicer to be able to just include `<types.h>` and get all the common
+type and helper function stuff we need everywhere).
+
+The headers `<kconfig.h>`, `<rules.h>` and `<commonlib/bsd/compiler.h>`
+are always automatically included in all compilation units by the build
+system and should not be included manually.
+
+Don't re-invent common macros
+-----------------------------
+
+The header file `src/commonlib/bsd/include/commonlib/bsd/helpers.h`
+contains a number of macros that you should use, rather than explicitly
+coding some variant of them yourself. For example, if you need to
+calculate the length of an array, take advantage of the macro
```c
#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
```
-There are also min() and max() macros that do strict type checking if
-you need them. Feel free to peruse that header file to see what else is
-already defined that you shouldn't reproduce in your code.
-
Editor modelines and other cruft
--------------------------------
--
To view, visit https://review.coreboot.org/c/coreboot/+/50247
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibbcde306a814f52b3a41b58c7a33bdd99b0187e0
Gerrit-Change-Number: 50247
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: HAOUAS Elyes <ehaouas(a)noos.fr>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth(a)google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-MessageType: merged