Bring in the newest script. This is close enough to v5.8 that it will likely be final. Keep the U-Boot changes to $logFunctions

Signed-off-by: Simon Glass sjg@chromium.org ---

scripts/checkpatch.pl | 216 +++++++++++------------------------------- 1 file changed, 55 insertions(+), 161 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index c2641bc995e..cec09fbade8 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -61,9 +61,7 @@ my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; my $typedefsfile = ""; my $color = "auto"; -my $allow_c99_comments = 1; # Can be overridden by --ignore C99_COMMENT_TOLERANCE -# git output parsing needs US English output, so first set backtick child process LANGUAGE -my $git_command ='export LANGUAGE=en_US.UTF-8; git'; +my $allow_c99_comments = 1;

sub help { my ($exitcode) = @_; @@ -470,19 +468,8 @@ our $logFunctions = qr{(?x: seq_vprintf|seq_printf|seq_puts )};

-our $allocFunctions = qr{(?x: - (?:(?:devm_)? - (?:kv|k|v)[czm]alloc(?:_node|_array)? | - kstrdup(?:_const)? | - kmemdup(?:_nul)?) | - (?:\w+)?alloc_skb(?:ip_align)? | - # dev_alloc_skb/netdev_alloc_skb, et al - dma_alloc_coherent -)}; - our $signature_tags = qr{(?xi: Signed-off-by:| - Co-developed-by:| Acked-by:| Tested-by:| Reviewed-by:| @@ -588,27 +575,6 @@ foreach my $entry (@mode_permission_funcs) { } $mode_perms_search = "(?:${mode_perms_search})";

-our %deprecated_apis = ( - "synchronize_rcu_bh" => "synchronize_rcu", - "synchronize_rcu_bh_expedited" => "synchronize_rcu_expedited", - "call_rcu_bh" => "call_rcu", - "rcu_barrier_bh" => "rcu_barrier", - "synchronize_sched" => "synchronize_rcu", - "synchronize_sched_expedited" => "synchronize_rcu_expedited", - "call_rcu_sched" => "call_rcu", - "rcu_barrier_sched" => "rcu_barrier", - "get_state_synchronize_sched" => "get_state_synchronize_rcu", - "cond_synchronize_sched" => "cond_synchronize_rcu", -); - -#Create a search pattern for all these strings to speed up a loop below -our $deprecated_apis_search = ""; -foreach my $entry (keys %deprecated_apis) { - $deprecated_apis_search .= '|' if ($deprecated_apis_search ne ""); - $deprecated_apis_search .= $entry; -} -$deprecated_apis_search = "(?:${deprecated_apis_search})"; - our $mode_perms_world_writable = qr{ S_IWUGO | S_IWOTH | @@ -908,7 +874,7 @@ sub seed_camelcase_includes { $camelcase_seeded = 1;

if (-e ".git") { - my $git_last_include_commit = `${git_command} log --no-merges --pretty=format:"%h%n" -1 -- include`; + my $git_last_include_commit = `git log --no-merges --pretty=format:"%h%n" -1 -- include`; chomp $git_last_include_commit; $camelcase_cache = ".checkpatch-camelcase.git.$git_last_include_commit"; } else { @@ -936,7 +902,7 @@ sub seed_camelcase_includes { }

if (-e ".git") { - $files = `${git_command} ls-files "include/*.h"`; + $files = `git ls-files "include/*.h"`; @include_files = split('\n', $files); }

@@ -960,13 +926,13 @@ sub git_commit_info {

return ($id, $desc) if ((which("git") eq "") || !(-e ".git"));

- my $output = `${git_command} log --no-color --format='%H %s' -1 $commit 2>&1`; + my $output = `git log --no-color --format='%H %s' -1 $commit 2>&1`; $output =~ s/^\s*//gm; my @lines = split("\n", $output);

return ($id, $desc) if ($#lines < 0);

- if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous/) { + if ($lines[0] =~ /^error: short SHA1 $commit is ambiguous./) { # Maybe one day convert this block of bash into something that returns # all matching commit ids, but it's very slow... # @@ -1010,7 +976,7 @@ if ($git) { } else { $git_range = "-1 $commit_expr"; } - my $lines = `${git_command} log --no-color --no-merges --pretty=format:'%H %s' $git_range`; + my $lines = `git log --no-color --no-merges --pretty=format:'%H %s' $git_range`; foreach my $line (split(/\n/, $lines)) { $line =~ /^([0-9a-fA-F]{40,40}) (.*)$/; next if (!defined($1) || !defined($2)); @@ -1025,7 +991,6 @@ if ($git) { }

my $vname; -$allow_c99_comments = !defined $ignore_type{"C99_COMMENT_TOLERANCE"}; for my $filename (@ARGV) { my $FILE; if ($git) { @@ -2691,24 +2656,6 @@ sub process { } else { $signatures{$sig_nospace} = 1; } - -# Check Co-developed-by: immediately followed by Signed-off-by: with same name and email - if ($sign_off =~ /^co-developed-by:$/i) { - if ($email eq $author) { - WARN("BAD_SIGN_OFF", - "Co-developed-by: should not be used to attribute nominal patch author '$author'\n" . "$here\n" . $rawline); - } - if (!defined $lines[$linenr]) { - WARN("BAD_SIGN_OFF", - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline); - } elsif ($rawlines[$linenr] !~ /^\s*signed-off-by:\s*(.*)/i) { - WARN("BAD_SIGN_OFF", - "Co-developed-by: must be immediately followed by Signed-off-by:\n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); - } elsif ($1 ne $email) { - WARN("BAD_SIGN_OFF", - "Co-developed-by and Signed-off-by: name/email do not match \n" . "$here\n" . $rawline . "\n" .$rawlines[$linenr]); - } - } }

# Check email subject for common tools that don't need to be mentioned @@ -2729,10 +2676,8 @@ sub process { ($line =~ /^\s*(?:WARNING:|BUG:)/ || $line =~ /^\s*[\s*\d+.\d{6,6}\s*]/ || # timestamp - $line =~ /^\s*[<[0-9a-fA-F]{8,}>]/) || - $line =~ /^(?:\s+\w+:\s+[0-9a-fA-F]+){3,3}/ || - $line =~ /^\s*#\d+\s*[[0-9a-fA-F]+]\s*\w+ at [0-9a-fA-F]+/) { - # stack dump address styles + $line =~ /^\s*[<[0-9a-fA-F]{8,}>]/)) { + # stack dump address $commit_log_possible_stack_dump = 1; }

@@ -2904,17 +2849,6 @@ sub process { } }

-# check for invalid commit id - if ($in_commit_log && $line =~ /(^fixes:|\bcommit)\s+([0-9a-f]{6,40})\b/i) { - my $id; - my $description; - ($id, $description) = git_commit_info($2, undef, undef); - if (!defined($id)) { - WARN("UNKNOWN_COMMIT_ID", - "Unknown commit id '$2', maybe rebased or not pulled?\n" . $herecurr); - } - } - # ignore non-hunk lines and lines being removed next if (!$hunk_line || $line =~ /^-/);

@@ -3044,7 +2978,7 @@ sub process { my @compats = $rawline =~ /"([a-zA-Z0-9-,.+_]+)"/g;

my $dt_path = $root . "/Documentation/devicetree/bindings/"; - my $vp_file = $dt_path . "vendor-prefixes.yaml"; + my $vp_file = $dt_path . "vendor-prefixes.txt";

foreach my $compat (@compats) { my $compat2 = $compat; @@ -3059,7 +2993,7 @@ sub process {

next if $compat !~ /^([a-zA-Z0-9-]+),/; my $vendor = $1; - `grep -Eq "\"\^\Q$vendor\E,\.\*\":" $vp_file`; + `grep -Eq "^$vendor\b" $vp_file`; if ( $? >> 8 ) { WARN("UNDOCUMENTED_DT_STRING", "DT compatible string vendor "$vendor" appears un-documented -- check $vp_file\n" . $herecurr); @@ -3083,24 +3017,16 @@ sub process { $comment = '..'; }

-# check SPDX comment style for .[chsS] files - if ($realfile =~ /.[chsS]$/ && - $rawline =~ /SPDX-License-Identifier:/ && - $rawline !~ m@^+\s*\Q$comment\E\s*@) { - WARN("SPDX_LICENSE_TAG", - "Improper SPDX comment style for '$realfile', please use '$comment' instead\n" . $herecurr); - } - if ($comment !~ /^$/ && - $rawline !~ m@^+\Q$comment\E SPDX-License-Identifier: @) { - WARN("SPDX_LICENSE_TAG", - "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr); + $rawline !~ /^+\Q$comment\E SPDX-License-Identifier: /) { + WARN("SPDX_LICENSE_TAG", + "Missing or malformed SPDX-License-Identifier tag in line $checklicenseline\n" . $herecurr); } elsif ($rawline =~ /(SPDX-License-Identifier: .*)/) { - my $spdx_license = $1; - if (!is_SPDX_License_valid($spdx_license)) { - WARN("SPDX_LICENSE_TAG", - "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr); - } + my $spdx_license = $1; + if (!is_SPDX_License_valid($spdx_license)) { + WARN("SPDX_LICENSE_TAG", + "'$spdx_license' is not supported in LICENSES/...\n" . $herecurr); + } } } } @@ -3108,14 +3034,6 @@ sub process { # check we are in a valid source file if not then ignore this hunk next if ($realfile !~ /.(h|c|s|S|sh|dtsi|dts)$/);

-# check for using SPDX-License-Identifier on the wrong line number - if ($realline != $checklicenseline && - $rawline =~ /\bSPDX-License-Identifier:/ && - substr($line, @-, @+ - @-) eq "$;" x (@+ - @-)) { - WARN("SPDX_LICENSE_TAG", - "Misplaced SPDX-License-Identifier tag - use line $checklicenseline instead\n" . $herecurr); - } - # line length limit (with some exclusions) # # There are a few types of lines that may extend beyond $max_line_length: @@ -3953,23 +3871,14 @@ sub process { WARN("STATIC_CONST_CHAR_ARRAY", "static const char * array should probably be static const char * const\n" . $herecurr); - } - -# check for initialized const char arrays that should be static const - if ($line =~ /^+\s*const\s+(char|unsigned\s+char|_*u8|(?:[us]_)?int8_t)\s+\w+\s*[\s*(?:\w+\s*)?]\s*=\s*"/) { - if (WARN("STATIC_CONST_CHAR_ARRAY", - "const array should probably be static const\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/(^.\s*)const\b/${1}static const/; - } - } + }

# check for static char foo[] = "bar" declarations. if ($line =~ /\bstatic\s+char\s+(\w+)\s*[\s*]\s*=\s*"/) { WARN("STATIC_CONST_CHAR_ARRAY", "static char array declaration should probably be static const char\n" . $herecurr); - } + }

# check for const <foo> const where <foo> is not a pointer or array type if ($sline =~ /\bconst\s+($BasicType)\s+const\b/) { @@ -4677,7 +4586,7 @@ sub process {

# closing brace should have a space following it when it has anything # on the line - if ($line =~ /}(?!(?:,|;|)|}))\S/) { + if ($line =~ /}(?!(?:,|;|)))\S/) { if (ERROR("SPACING", "space required after that close brace '}'\n" . $herecurr) && $fix) { @@ -5027,6 +4936,17 @@ sub process { while ($line =~ m{($Constant|$Lval)}g) { my $var = $1;

+#gcc binary extension + if ($var =~ /^$Binary$/) { + if (WARN("GCC_BINARY_CONSTANT", + "Avoid gcc v4.3+ binary constant extension: <$var>\n" . $herecurr) && + $fix) { + my $hexval = sprintf("0x%x", oct($var)); + $fixed[$fixlinenr] =~ + s/\b$var\b/$hexval/; + } + } + #CamelCase if ($var !~ /^$Constant$/ && $var =~ /[A-Z][a-z]|[a-z][A-Z]/ && @@ -5208,7 +5128,7 @@ sub process { next if ($arg =~ /.../); next if ($arg =~ /^type$/i); my $tmp_stmt = $define_stmt; - $tmp_stmt =~ s/\b(sizeof|typeof|__typeof__|__builtin\w+|typecheck\s*(\s*$Type\s*,|#+)\s*(*\s*$arg\s*)*\b//g; + $tmp_stmt =~ s/\b(typeof|__typeof__|__builtin\w+|typecheck\s*(\s*$Type\s*,|#+)\s*(*\s*$arg\s*)*\b//g; $tmp_stmt =~ s/#+\s*$arg\b//g; $tmp_stmt =~ s/\b$arg\s*##//g; my $use_cnt = () = $tmp_stmt =~ /\b$arg\b/g; @@ -5607,8 +5527,7 @@ sub process { my ($s, $c) = ctx_statement_block($linenr - 3, $realcnt, 0); # print("line: <$line>\nprevline: <$prevline>\ns: <$s>\nc: <$c>\n\n\n");

- if ($s =~ /(?:^|\n)[ +]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:([^)]*)\s*)?\s*$allocFunctions\s*(/ && - $s !~ /\b__GFP_NOWARN\b/ ) { + if ($s =~ /(?:^|\n)[ +]\s*(?:$Type\s*)?\Q$testval\E\s*=\s*(?:([^)]*)\s*)?\s*(?:devm_)?(?:[kv][czm]alloc(?:_node|_array)?\b|kstrdup|kmemdup|(?:dev_)?alloc_skb)/) { WARN("OOM_MESSAGE", "Possible unnecessary 'out of memory' message\n" . $hereprev); } @@ -5729,7 +5648,7 @@ sub process { # ignore udelay's < 10, however if (! ($delay < 10) ) { CHK("USLEEP_RANGE", - "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.rst\n" . $herecurr); + "usleep_range is preferred over udelay; see Documentation/timers/timers-howto.txt\n" . $herecurr); } if ($delay > 2000) { WARN("LONG_UDELAY", @@ -5741,7 +5660,7 @@ sub process { if ($line =~ /\bmsleep\s*((\d+));/) { if ($1 < 20) { WARN("MSLEEP", - "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.rst\n" . $herecurr); + "msleep < 20ms can sleep for up to 20ms; see Documentation/timers/timers-howto.txt\n" . $herecurr); } }

@@ -5890,18 +5809,6 @@ sub process { "__aligned(size) is preferred over __attribute__((aligned(size)))\n" . $herecurr); }

-# Check for __attribute__ section, prefer __section - if ($realfile !~ m@\binclude/uapi/@ && - $line =~ /\b__attribute__\s*(\s*(.*_*section_*\s*(\s*("[^"]*")/) { - my $old = substr($rawline, $-[1], $+[1] - $-[1]); - my $new = substr($old, 1, -1); - if (WARN("PREFER_SECTION", - "__section($new) is preferred over __attribute__((section($old)))\n" . $herecurr) && - $fix) { - $fixed[$fixlinenr] =~ s/\b__attribute__\s*(\s*(\s*_*section_*\s*(\s*\Q$old\E\s*)\s*)\s*)/__section($new)/; - } - } - # Check for __attribute__ format(printf, prefer __printf if ($realfile !~ m@\binclude/uapi/@ && $line =~ /\b__attribute__\s*(\s*(\s*format\s*(\s*printf/) { @@ -6024,7 +5931,7 @@ sub process { while ($fmt =~ /(%[*\d.]*p(\w))/g) { $specifier = $1; $extension = $2; - if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOxt]/) { + if ($extension !~ /[SsBKRraEhMmIiUDdgVCbGNOx]/) { $bad_specifier = $specifier; last; } @@ -6144,11 +6051,11 @@ sub process { my $max = $7; if ($min eq $max) { WARN("USLEEP_RANGE", - "usleep_range should not use min == max args; see Documentation/timers/timers-howto.rst\n" . "$here\n$stat\n"); + "usleep_range should not use min == max args; see Documentation/timers/timers-howto.txt\n" . "$here\n$stat\n"); } elsif ($min =~ /^\d+$/ && $max =~ /^\d+$/ && $min > $max) { WARN("USLEEP_RANGE", - "usleep_range args reversed, use min then max; see Documentation/timers/timers-howto.rst\n" . "$here\n$stat\n"); + "usleep_range args reversed, use min then max; see Documentation/timers/timers-howto.txt\n" . "$here\n$stat\n"); } }

@@ -6271,8 +6178,8 @@ sub process { } }

-# check for pointless casting of alloc functions - if ($line =~ /*\s*)\s*$allocFunctions\b/) { +# check for pointless casting of kmalloc return + if ($line =~ /*\s*)\s*[kv][czm]alloc(_node){0,1}\b/) { WARN("UNNECESSARY_CASTS", "unnecessary cast may hide bugs, see http://c-faq.com/malloc/mallocnocast.html%5Cn" . $herecurr); } @@ -6280,7 +6187,7 @@ sub process { # alloc style # p = alloc(sizeof(struct foo), ...) should be p = alloc(sizeof(*p), ...) if ($perl_version_ok && - $line =~ /\b($Lval)\s*=\s*(?:$balanced_parens)?\s*((?:kv|k|v)[mz]alloc(?:_node)?)\s*(\s*(sizeof\s*(\s*struct\s+$Lval\s*))/) { + $line =~ /\b($Lval)\s*=\s*(?:$balanced_parens)?\s*([kv][mz]alloc(?:_node)?)\s*(\s*(sizeof\s*(\s*struct\s+$Lval\s*))/) { CHK("ALLOC_SIZEOF_STRUCT", "Prefer $3(sizeof(*$1)...) over $3($4...)\n" . $herecurr); } @@ -6443,6 +6350,19 @@ sub process { } }

+# check for bool bitfields + if ($sline =~ /^.\s+bool\s*$Ident\s*:\s*\d+\s*;/) { + WARN("BOOL_BITFIELD", + "Avoid using bool as bitfield. Prefer bool bitfields as unsigned int or u<8|16|32>\n" . $herecurr); + } + +# check for bool use in .h files + if ($realfile =~ /.h$/ && + $sline =~ /^.\s+bool\s*$Ident\s*(?::\s*d+\s*)?;/) { + CHK("BOOL_MEMBER", + "Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384%5Cn" . $herecurr); + } + # check for semaphores initialized locked if ($line =~ /^.\s*sema_init.+,\W?0\W?)/) { WARN("CONSIDER_COMPLETION", @@ -6461,20 +6381,6 @@ sub process { "please use device_initcall() or more appropriate function instead of __initcall() (see include/linux/init.h)\n" . $herecurr); }

-# check for spin_is_locked(), suggest lockdep instead - if ($line =~ /\bspin_is_locked(/) { - WARN("USE_LOCKDEP", - "Where possible, use lockdep_assert_held instead of assertions based on spin_is_locked\n" . $herecurr); - } - -# check for deprecated apis - if ($line =~ /\b($deprecated_apis_search)\b\s*(/) { - my $deprecated_api = $1; - my $new_api = $deprecated_apis{$deprecated_api}; - WARN("DEPRECATED_API", - "Deprecated use of '$deprecated_api', prefer '$new_api' instead\n" . $herecurr); - } - # check for various structs that are normally const (ops, kgdb, device_tree) # and avoid what seem like struct definitions 'struct foo {' if ($line !~ /\bconst\b/ && @@ -6509,12 +6415,6 @@ sub process { "Using $1 should generally have parentheses around the comparison\n" . $herecurr); }

-# nested likely/unlikely calls - if ($line =~ /\b(?:(?:un)?likely)\s*(\s*!?\s*(IS_ERR(?:_OR_NULL|_VALUE)?|WARN)/) { - WARN("LIKELY_MISUSE", - "nested (un)?likely() calls, $1 already uses unlikely() internally\n" . $herecurr); - } - # whine mightly about in_atomic if ($line =~ /\bin_atomic\s*(/) { if ($realfile =~ m@^drivers/@) { @@ -6674,12 +6574,6 @@ sub process { "unknown module license " . $extracted_string . "\n" . $herecurr); } } - -# check for sysctl duplicate constants - if ($line =~ /.extra[12]\s*=\s*&(zero|one|int_max)\b/) { - WARN("DUPLICATED_SYSCTL_CONST", - "duplicated sysctl range checking value '$1', consider using the shared one in include/linux/sysctl.h\n" . $herecurr); - } }

# If we have no input at all, then there is nothing to report on

Add an option to indicate that U-Boot-specific checks should be enabled. Add a function to house the code that will be added.

Signed-off-by: Simon Glass sjg@chromium.org ---

.checkpatch.conf | 3 +++ scripts/checkpatch.pl | 12 ++++++++++++ 2 files changed, 15 insertions(+)

diff --git a/.checkpatch.conf b/.checkpatch.conf index 95f19635d35..ed0c2150ba8 100644 --- a/.checkpatch.conf +++ b/.checkpatch.conf @@ -28,3 +28,6 @@

# A bit shorter of a description is OK with us. --min-conf-desc-length=2 + +# Extra checks for U-Boot +--u-boot diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index cec09fbade8..93863c8f086 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -60,6 +60,7 @@ my $codespell = 0; my $codespellfile = "/usr/share/codespell/dictionary.txt"; my $conststructsfile = "$D/const_structs.checkpatch"; my $typedefsfile = ""; +my $u_boot = 0; my $color = "auto"; my $allow_c99_comments = 1;

@@ -121,6 +122,7 @@ Options: --typedefsfile Read additional types from this file --color[=WHEN] Use colors 'always', 'never', or only when output is a terminal ('auto'). Default is 'auto'. + --u-boot Run additional checks for U-Boot -h, --help, --version display this help and exit

When FILE is - read standard input. @@ -225,6 +227,7 @@ GetOptions( 'codespell!' => $codespell, 'codespellfile=s' => $codespellfile, 'typedefsfile=s' => $typedefsfile, + 'u-boot' => $u_boot, 'color=s' => $color, 'no-color' => $color, #keep old behaviors of -nocolor 'nocolor' => $color, #keep old behaviors of -nocolor @@ -2235,6 +2238,11 @@ sub pos_last_openparen { return length(expand_tabs(substr($line, 0, $last_openparen))) + 1; }

+# Checks specific to U-Boot +sub u_boot_line { + my ($realfile, $line, $herecurr) = @_; +} + sub process { my $filename = shift;

@@ -3102,6 +3110,10 @@ sub process { "adding a line without newline at end of file\n" . $herecurr); }

+ if ($u_boot) { + u_boot_line($realfile, $line, $herecurr); + } + # check we are in a valid source file C or perl if not then ignore this hunk next if ($realfile !~ /.(h|c|pl|dtsi|dts)$/);

A common problem when submitting a new uclass is to forget to add sandbox tests. Add a warning for this.

Of course tests should always be added for new code, but this one seems to be missed by nearly every new contributor.

Signed-off-by: Simon Glass sjg@chromium.org ---

scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 93863c8f086..9dc42520e2d 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2241,6 +2241,12 @@ sub pos_last_openparen { # Checks specific to U-Boot sub u_boot_line { my ($realfile, $line, $herecurr) = @_; + + # ask for a test if a new uclass ID is added + if ($realfile =~ /uclass-id.h/ && $line =~ /^+/) { + WARN("NEW_UCLASS", + "Possible new uclass - make sure to add a sandbox driver, plus a test in test/dm/<name>.c\n" . $herecurr); + } }

sub process {

We want people to use the livetree API, so request it.

Signed-off-by: Simon Glass sjg@chromium.org ---

scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 9dc42520e2d..e3a2a289aea 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2247,6 +2247,12 @@ sub u_boot_line { WARN("NEW_UCLASS", "Possible new uclass - make sure to add a sandbox driver, plus a test in test/dm/<name>.c\n" . $herecurr); } + + # try to get people to use the livetree API + if ($line =~ /^+.*fdtdec_/) { + WARN("LIVETREE", + "Use the livetree API (dev_read_...)\n" . $herecurr); + } }

sub process {

This request is made with nearly every new command. Point to some docs on how to do it.

Signed-off-by: Simon Glass sjg@chromium.org ---

doc/README.commands | 50 +++++++++++++++++++++++++++++++++++++++++++ scripts/checkpatch.pl | 6 ++++++ 2 files changed, 56 insertions(+)

diff --git a/doc/README.commands b/doc/README.commands index 716ad227aa1..229f86d8fb2 100644 --- a/doc/README.commands +++ b/doc/README.commands @@ -134,3 +134,53 @@ by writing in u-boot.lds ($(srctree)/board/boardname/u-boot.lds) these .u_boot_list : { KEEP(*(SORT(.u_boot_list*))); } + +Writing tests +------------- + +All new commands should have tests. Tests for existing commands are very +welcome. + +It is fairly easy to write a test for a command. Enable it in sandbox, and +then add code that runs the command and checks the output. + +Here is an example: + +/* Test 'acpi items' command */ +static int dm_test_acpi_cmd_items(struct unit_test_state *uts) +{ + struct acpi_ctx ctx; + void *buf; + + buf = malloc(BUF_SIZE); + ut_assertnonnull(buf); + + ctx.current = buf; + ut_assertok(acpi_fill_ssdt(&ctx)); + console_record_reset(); + run_command("acpi items", 0); + ut_assert_nextline("dev 'acpi-test', type 1, size 2"); + ut_assert_nextline("dev 'acpi-test2', type 1, size 2"); + ut_assert_console_end(); + + ctx.current = buf; + ut_assertok(acpi_inject_dsdt(&ctx)); + console_record_reset(); + run_command("acpi items", 0); + ut_assert_nextline("dev 'acpi-test', type 2, size 2"); + ut_assert_nextline("dev 'acpi-test2', type 2, size 2"); + ut_assert_console_end(); + + console_record_reset(); + run_command("acpi items -d", 0); + ut_assert_nextline("dev 'acpi-test', type 2, size 2"); + ut_assert_nextlines_are_dump(2); + ut_assert_nextline("%s", ""); + ut_assert_nextline("dev 'acpi-test2', type 2, size 2"); + ut_assert_nextlines_are_dump(2); + ut_assert_nextline("%s", ""); + ut_assert_console_end(); + + return 0; +} +DM_TEST(dm_test_acpi_cmd_items, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT); diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index e3a2a289aea..22869992e90 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2253,6 +2253,12 @@ sub u_boot_line { WARN("LIVETREE", "Use the livetree API (dev_read_...)\n" . $herecurr); } + + # add tests for new commands + if ($line =~ /^+.*do_($Ident)(struct cmd_tbl.*/) { + WARN("CMD_TEST", + "Possible new command - make sure you add a test\n" . $herecurr); + } }

sub process {

There is a lot of use of #ifdefs in U-Boot. In an effort reduce this, suggest using the compile-time construct.

Signed-off-by: Simon Glass sjg@chromium.org ---

scripts/checkpatch.pl | 6 ++++++ 1 file changed, 6 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 22869992e90..33ec4e2bfd4 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2259,6 +2259,12 @@ sub u_boot_line { WARN("CMD_TEST", "Possible new command - make sure you add a test\n" . $herecurr); } + + # use if instead of #if + if ($line =~ /^+#if.*CONFIG.*/) { + WARN("PREFER_IF", + "Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible\n" . $herecurr); + } }

sub process {