Patch 423e9e0fc0: Documentation/lint: Use Super I/O instead of SuperIO
added the word SuperIO to the checkpatch spelling list.
There were unfortunately some issues with this.
1) This introduced a problem because the comparison is used in different
cases in different places. The misspelled word is compared ignoring
the case, but when looking for the correct word, it looks through the
list for the misspelling in all lowercase. When it couldn't find the
word "superio" in the list, the variable came back uninitialized.
2) The spellcheck feature isn't enabled in checkpatch unless the option
--strict is enabled, so this wasn't getting reported anyway.
3) SuperIO (or superio) will match the KCONFIG options such as
CONFIG_SUPERIO_NUVOTON_NCT5104D, and suggest "Super I/O" which doesn't
make any sense.
Signed-off-by: Martin Roth <gaumless@gmail.com>
Change-Id: I464305af539926ac8a45c9c0d59eeb2c78dea17a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/61434
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
This error prevented the last line of the Kconfig tree from being
printed or added to the output file. This is a significant problem if
you try to use the generated file as the kconfig source, because it
changes CONFIG_HAVE_RAMSTAGE from defaulting to yes to defaulting to
NO. This causes the build to stop working.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I3ec11f1ac59533a078fd3bd4d0dbee9df825a97a
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58992
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Instead of using two variables, one for the boolean value and one for
the path, use just one with the path. Since an empty string evalutes to
false, this simplification does not change behaviour.
Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
Change-Id: I2f1171789af6815094446f107f3c634332a3427e
Reviewed-on: https://review.coreboot.org/c/coreboot/+/58401
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This rule was creating trouble:
* A symbol may only be declared inside or outside a choice.
The linter treats every occurence of a `config` entry as a symbol
declaration, even when it's just setting a default or adding selects.
This is not easy to fix as the symbol objects are not created first
and then added to the $symbols array when we know what kind of decla-
ration we have, but are created incrementally inside this global
list.
Change-Id: I48a17f6403470251be6b6d44bb82a8bdcbefe9f6
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/56410
Reviewed-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Martin Roth <martinroth@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This patch adds a new test to checkpatch that identifies cases where a
line after a conditional statement is incorrectly intended (possibly
indicating the mistake of forgetting to add braces), like this:
if (a)
b;
c;
Unfortunately, it seems like checkpatch is partially unmaintained in
upstream Linux at the moment with maintainers either not responding at
all or not even willing to look at new patches [1]. Since detecting this
error class is important to coreboot, let's just carry this feature
locally for now.
[1] https://lkml.org/lkml/2021/4/15/1488
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I7bb90b56dfc7582271d2b82cb42a2c1df477054f
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51838
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Paul Menzel <paulepanter@mailbox.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Nico Huber <nico.h@gmx.de>
I wished there was a way to do this in smaller steps, but with
every line fixed an error somewhere else became visible. Here
is a (probably incomplete) list of the issues:
* Only one set of parentheses was supported. This is a hard
to solve problem without a real parser (one solution is to
use an recursive RE, see below).
* The precedence order was wrong. Might have been adapted just
to give a positive result for the arbitrary state of the tree.
* Numbered match variables (e.g. $1, $2, etc.) are not local.
Calling handle_expressions() recursively once with $1, then
with $2, resulted in using the final $2 after the first
recursive call (garbage, practically).
Also, symbol and expression parsing was mixed, making things
harder to follow.
To remedy the issues:
* Split handle_symbol() out. It is called with whitespace
stripped, to keep the uglier REs in handle_expressions().
* Match balanced parentheses and quotes when splitting
expressions. In this recursive RE
/(\((?:[^\(\)]++|(?-1))*\))/
the `(?-1)` references the outer-most group, thus the whole
expression itself. So it matches a pair of parentheses with
a mix of non-parentheses and the recursive rule itself inside.
This allows us to:
* Order the expression matches according to their precedence
rules. Now we can match `<expr> '||' <expr>` first as we should
and everything else falls into its place.
* Remove the bail-out that silenced the undefined behavior.
Change-Id: Ibc1be79adc07792f0721f0dc08b50422b6da88a9
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52067
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Gerrit is able to add reviewers based on entries in the `MAINTAINERS`
file. For inclusion and exclusion matches either paths or regular
expressions can be used. The syntax is described in the header of the
file.
When matching a path, there are two sensible possibilities:
- `path/to/file` matches a file.
- `path/to/dir/` matches a folder including its contents recursively.
- `path/to/dir/*` matches all files in that folder, without recursing
into its subfolders.
The trailing slash in the second example is essential. Without it, only
the directory entry itself matches when, for example, the folder gets
deleted, renamed or its permissions get modified. Reviewers in the list
won't get added to changes of any files or directories below that path.
Thus, add a linter script to ensure a path match on a directory always
ends with `/` or `/*` as shown above.
Change-Id: I9873184c0df4a0b4455f803828e2719887e545db
Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52210
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Some of the src/vendorcode/ directories are used to import a whole
codebase from somewhere else which uses a completely different coding
style. For those directories, excluding them from checkpatch makes
sense. However, other directories are simply implementing
vendor-specific extensions that were written by coreboot developers
specifically for coreboot in coreboot's coding style. Those directories
should be covered by checkpatch.
This patch narrows the existing blanket exception of src/vendorcode/ to
the amd, cavium, intel and mediatek directories (which actually include
large amounts of foreign source). The eltan, google and siemens
directories (which seem to contain code specifically written for
coreboot) will now be covered by checkpatch.
Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I1feaba37c469714217fff4d160e595849e0230b9
Reviewed-on: https://review.coreboot.org/c/coreboot/+/51827
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Werner Zeh <werner.zeh@siemens.com>
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This reverts commit 15e379aaf3.
It triggers on directories that only contain artifacts and no
checked in code. As this happens a lot when switching branches,
it makes it impossible to commit new code.
Change-Id: I38a86c8a5d5dc14ca5f6cba789bcb8c0fcaefb0b
Signed-off-by: Nico Huber <nico.h@gmx.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50354
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The codebase currently has only unix line endings, so add a lint tool
to check for windows line endings.
BUG=None
TEST=Verify that line endings are caught both inside and outside a git
repo.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: I6faf99a3184e4843640fb8965f8124de0bc52ce7
Reviewed-on: https://review.coreboot.org/c/coreboot/+/50851
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Instead of hardcoding paths to the executables, use the version in the
path. This allows the scripts to work on more systems, and allows the
binary version to be changed more easily if needed.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: Ifcc56aa21092cd3866eacb6a02d198110ec6051d
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48904
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
It would seem that `pres` is an abbreviation for `presence`. Personally,
over the last ~2.5 years, I have seen checkpatch complaints about `pres`
on several occasions, and all of them were abbreviations for `presence`.
Given the high false positive rate for this entry, comment it out.
Change-Id: I72f1811fb1f766e7de7c4957fd9ba844c0728029
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/49463
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Check that nobody misuses the Kconfigs SUBSYSTEM_*_ID. They are meant to
be used for overriding the devicetree subsystem ids locally but shall
not be added to a board's Kconfig. Instead, the devicetree option
`subsystemid` should be used.
Add a linter script for this that finds and warns about such misuse.
Also add a note in the Kconfigs' description.
TEST=CB:45513
Change-Id: I21c021c718154f1396f795a555af47a76d6efe03
Signed-off-by: Michael Niewöhner <foss@mniewoehner.de>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/45513
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
The lint-stable makefile target only watches for errors in the Kconfig
file, so has not protected additional "Naked" references to BOOL type
Kconfig symbols from entering the tree. Update it to an error so that
they can't continue coming into the codebase.
Signed-off-by: Martin Roth <martin@coreboot.org>
Change-Id: Icce2a9a627c4fbcaa220df18474cb8bfea8b2a8c
Reviewed-on: https://review.coreboot.org/c/coreboot/+/43826
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The majority of the codebase has been converted to use SPDX identifiers
now, so let's enforce those by default. The only exceptions are
src/include and src/lib, which are not being checked since many of the
files there do not have license headers at all. Files with custom
licenses that aren't covered by SPDX can be listed as exceptions at the
top of lint-000-license-headers.
Change-Id: Ie6642153793d5735c74c5950bc9e27ee7eecacbc
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41602
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Paul Menzel <paulepanter@users.sourceforge.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Convert the remaining files in src/drivers to use SPDX identifiers.
int15.h and default_brightness_levels.asl did not have license headers,
but they were both copied from other GPL2 files, so they should be under
the GPL2 as well.
ne2k.c and drm_dp_helper.h are licensed under custom BSD-like licenses
that do not have an SPDX equivalent, so they are added as exceptions
to the license header lint.
Change-Id: I87fb1c637b8d11b0463f7c19f70b847413e14aed
Signed-off-by: Jacob Garber <jgarber1@ualberta.ca>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41601
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
The Historical Permission Notice and Disclaimer (with and without
permission to sell) is a BSD-style license family that OSI and SPDX
consider deprecated - and yet, it's right here in our tree.
Change-Id: I61624b6e54e9aba6e2f54822c1f68967c416ad3d
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41221
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Stefan thinks they don't add value.
Command used:
sed -i -e '/file is part of /d' $(git grep "file is part of " |egrep ":( */\*.*\*/\$|#|;#|-- | *\* )" | cut -d: -f1 |grep -v crossgcc |grep -v gcov | grep -v /elf.h |grep -v nvramtool)
The exceptions are for:
- crossgcc (patch file)
- gcov (imported from gcc)
- elf.h (imported from GNU's libc)
- nvramtool (more complicated header)
The removed lines are:
- fmt.Fprintln(f, "/* This file is part of the coreboot project. */")
-# This file is part of a set of unofficial pre-commit hooks available
-/* This file is part of coreboot */
-# This file is part of msrtool.
-/* This file is part of msrtool. */
- * This file is part of ncurses, designed to be appended after curses.h.in
-/* This file is part of pgtblgen. */
- * This file is part of the coreboot project.
- /* This file is part of the coreboot project. */
-# This file is part of the coreboot project.
-# This file is part of the coreboot project.
-## This file is part of the coreboot project.
--- This file is part of the coreboot project.
-/* This file is part of the coreboot project */
-/* This file is part of the coreboot project. */
-;## This file is part of the coreboot project.
-# This file is part of the coreboot project. It originated in the
- * This file is part of the coreinfo project.
-## This file is part of the coreinfo project.
- * This file is part of the depthcharge project.
-/* This file is part of the depthcharge project. */
-/* This file is part of the ectool project. */
- * This file is part of the GNU C Library.
- * This file is part of the libpayload project.
-## This file is part of the libpayload project.
-/* This file is part of the Linux kernel. */
-## This file is part of the superiotool project.
-/* This file is part of the superiotool project */
-/* This file is part of uio_usbdebug */
Change-Id: I82d872b3b337388c93d5f5bf704e9ee9e53ab3a9
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41194
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This is all code coming from the outside, so let's keep these files
untouched as much as possible.
A couple of files is added to the list by name because their license,
while free, can't be properly modelled in SPDX:
- lzmadecode is (LGPL OR CPL) WITH special-exception
- stack.c and start16 are some weird (but free) US Gov't license grant
- two XGI related files have "BSD except for Linux, where it's GPL"
Change-Id: I42dec503b9c427a66792d3fec99ca8df1a360e47
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41193
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: HAOUAS Elyes <ehaouas@noos.fr>
If they were removed instead, it would be too easy to end up adding them
back again. They are kept in a comment so that they can be tracked.
Also, explain why these two entries have been commented out.
Change-Id: I8225944b5e3d1e022af169dda33e0344d4c3bccd
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/39618
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Held <felix-coreboot@felixheld.de>
