Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Simon Glass <sjg@chromium.org>
To:     LKML <linux-kernel@vger.kernel.org>
Cc:     Tom Rini <trini@konsulko.com>, Joe Perches <joe@perches.com>,
        Simon Glass <sjg@chromium.org>,
        Andy Whitcroft <apw@canonical.com>,
        Dwaipayan Ray <dwaipayanray1@gmail.com>,
        Lukas Bulwahn <lukas.bulwahn@gmail.com>
Subject: [PATCH] checkpatch: Add checks for U-Boot
Date:   Fri, 28 Jan 2022 16:18:23 -0700
Message-Id: <20220128161802.1.Iea256111132262fc2b3e58f14a62d410b92f4831@changeid>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

U-Boot has built up a small number of checks specific to that code
base. Although it is very similar to Linux, it has some other
requirements specific to its driver model, etc.

It is convenient to upstream these to keep the versions in sync.

Add a new function to handle the U-Boot checks and an option to enable
them.

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

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

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index b01c36a15d9dd0..c3090d2e653013 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -65,6 +65,7 @@ my $codespell = 0;
 my $codespellfile = "/usr/share/codespell/dictionary.txt";
 my $user_codespellfile = "";
 my $conststructsfile = "$D/const_structs.checkpatch";
+my $u_boot = 0;
 my $docsfile = "$D/../Documentation/dev-tools/checkpatch.rst";
 my $typedefsfile;
 my $color = "auto";
@@ -136,6 +137,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
   --kconfig-prefix=WORD      use WORD as a prefix for Kconfig symbols (default
                              ${CONFIG_})
   -h, --help, --version      display this help and exit
@@ -320,6 +322,7 @@ GetOptions(
 	'codespell!'	=> \$codespell,
 	'codespellfile=s'	=> \$user_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
@@ -2572,6 +2575,111 @@ sub get_raw_comment {
 	return $comment;
 }
 
+# Checks specific to U-Boot
+# Args:
+#   line: Patch line to check
+#   auto: Auto variable name, e.g. "per_child_auto"
+#   suffix: Suffix to expect on member, e.g. "_priv"
+#   warning: Warning name, e.g. "PRIV_AUTO"
+sub u_boot_struct_name {
+	my ($line, $auto, $suffix, $warning, $herecurr) = @_;
+
+	# Use _priv as a suffix for the device-private data struct
+	if ($line =~ /^\+\s*\.${auto}\s*=\s*sizeof\(struct\((\w+)\).*/) {
+		my $struct_name = $1;
+		if ($struct_name !~ /^\w+${suffix}/) {
+			WARN($warning,
+				 "struct \'$struct_name\' should have a ${suffix} suffix\n"
+				 . $herecurr);
+		}
+	}
+}
+
+sub u_boot_line {
+	my ($realfile, $line, $rawline, $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);
+	}
+
+	# try to get people to use the livetree API
+	if ($line =~ /^\+.*fdtdec_/) {
+		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);
+	}
+
+	# use if instead of #if
+	if ($realfile =~ /\.c$/ && $line =~ /^\+#if.*CONFIG.*/) {
+		WARN("PREFER_IF",
+		     "Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where possible\n" . $herecurr);
+	}
+
+	# prefer strl(cpy|cat) over strn(cpy|cat)
+	if ($line =~ /\bstrn(cpy|cat)\s*\(/) {
+		WARN("STRL",
+		     "strl$1 is preferred over strn$1 because it always produces a nul-terminated string\n" . $herecurr);
+	}
+
+	# use defconfig to manage CONFIG_CMD options
+	if ($line =~ /\+\s*#\s*(define|undef)\s+(CONFIG_CMD\w*)\b/) {
+		ERROR("DEFINE_CONFIG_CMD",
+		      "All commands are managed by Kconfig\n" . $herecurr);
+	}
+
+	# Don't put common.h and dm.h in header files
+	if ($realfile =~ /\.h$/ && $rawline =~ /^\+#include\s*<(common|dm)\.h>*/) {
+		ERROR("BARRED_INCLUDE_IN_HDR",
+		      "Avoid including common.h and dm.h in header files\n" . $herecurr);
+	}
+
+	# Do not disable fdt / initrd relocation
+	if ($rawline =~ /^\+.*(fdt|initrd)_high=0xffffffff/) {
+		ERROR("DISABLE_FDT_OR_INITRD_RELOC",
+		     "fdt or initrd relocation disabled at boot time\n" . $herecurr);
+	}
+
+	# make sure 'skip_board_fixup' is not
+	if ($rawline =~ /.*skip_board_fixup.*/) {
+		ERROR("SKIP_BOARD_FIXUP",
+		     "Avoid setting skip_board_fixup env variable\n" . $herecurr);
+	}
+
+	# Do not use CONFIG_ prefix in CONFIG_IS_ENABLED() calls
+	if ($line =~ /^\+.*CONFIG_IS_ENABLED\(CONFIG_\w*\).*/) {
+		ERROR("CONFIG_IS_ENABLED_CONFIG",
+		      "CONFIG_IS_ENABLED() takes values without the CONFIG_ prefix\n" . $herecurr);
+	}
+
+	# Use _priv as a suffix for the device-private data struct
+	if ($line =~ /^\+\s*\.priv_auto\s*=\s*sizeof\(struct\((\w+)\).*/) {
+		my $struct_name = $1;
+		if ($struct_name !~ /^\w+_priv/) {
+			WARN("PRIV_AUTO", "struct \'$struct_name\' should have a _priv suffix");
+		}
+	}
+
+	# Check struct names for the 'auto' members of struct driver
+	u_boot_struct_name($line, "priv_auto", "_priv", "PRIV_AUTO", $herecurr);
+	u_boot_struct_name($line, "plat_auto", "_plat", "PLAT_AUTO", $herecurr);
+	u_boot_struct_name($line, "per_child_auto", "_priv", "CHILD_PRIV_AUTO", $herecurr);
+	u_boot_struct_name($line, "per_child_plat_auto", "_plat",
+		"CHILD_PLAT_AUTO", $herecurr);
+
+	# Now the ones for struct uclass, skipping those in common with above
+	u_boot_struct_name($line, "per_device_auto", "_priv",
+		"DEVICE_PRIV_AUTO", $herecurr);
+	u_boot_struct_name($line, "per_device_plat_auto", "_plat",
+		"DEVICE_PLAT_AUTO", $herecurr);
+}
+
 sub exclude_global_initialisers {
 	my ($realfile) = @_;
 
@@ -3753,6 +3861,10 @@ sub process {
 			     "Avoid using '.L' prefixed local symbol names for denoting a range of code via 'SYM_*_START/END' annotations; see Documentation/asm-annotations.rst\n" . $herecurr);
 		}
 
+		if ($u_boot) {
+			u_boot_line($realfile, $line, $rawline, $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)$/);
 
-- 
2.35.0.rc2.247.g8bbb082509-goog