(cc'ing Andi Kleen, who wrote this code a decade ago)

On Tue, 2021-11-09 at 07:47 -0800, Randy Dunlap wrote:

quoted

On 11/9/21 3:56 AM, Andrea Merello wrote:

quoted

Il giorno ven 29 ott 2021 alle ore 00:04 Randy Dunlap [off-list ref] ha scritto:

quoted

On 10/28/21 3:18 AM, Andrea Merello wrote:

quoted

This path adds an I2C driver for communicating to a BNO055 IMU via I2C bus
and it enables the BNO055 core driver to work in this scenario.

Signed-off-by: Andrea Merello <redacted>
---
    drivers/iio/imu/bno055/Kconfig      |  6 ++++
    drivers/iio/imu/bno055/Makefile     |  1 +

[]

quoted

quoted

quoted

quoted

diff --git a/drivers/iio/imu/bno055/Kconfig b/drivers/iio/imu/bno055/Kconfig

[]

quoted

quoted

quoted

quoted

@@ -7,3 +7,9 @@ config BOSH_BNO055_SERIAL
        tristate "Bosh BNO055 attached via serial bus"
        depends on SERIAL_DEV_BUS
        select BOSH_BNO055_IIO
+
+config BOSH_BNO055_I2C
+     tristate "Bosh BNO055 attached via I2C bus"
+     depends on I2C
+     select REGMAP_I2C
+     select BOSH_BNO055_IIO

[]

quoted

quoted

quoted

The config entries that have user prompt strings should also
have help text.  scripts/checkpatch.pl should have told you
about that...

I'll add it, thanks. But FYI checkpatch doesn't complain about that here.

Hm, thanks for adding it and telling me about that.

checkpatch.pl does have some code for checking that but I confirmed
that it does not catch this simple case.

Joe, can you identify why checkpatch does not detect missing Kconfig
help text is this simple case?

Original patch here: https://lore.kernel.org/all/20211028101840.24632-11-andrea.merello@gmail.com/raw (local)

checkpatch is counting the diff header lines that follow the config entry.
Maybe this is clearer (better?) code:
---
  scripts/checkpatch.pl | 28 +++++++++++++++-------------
  1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 1784921c645da..b3ce8e04d7df7 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl

@@ -3483,20 +3483,22 @@ sub process {
  			my $cnt = $realcnt;
  			my $ln = $linenr + 1;
  			my $f;
-			my $is_start = 0;
-			my $is_end = 0;
+			my $needs_help = 0;
+			my $has_help = 0;
  			for (; $cnt > 0 && defined $lines[$ln - 1]; $ln++) {
  				$f = $lines[$ln - 1];
-				$cnt-- if ($lines[$ln - 1] !~ /^-/);
-				$is_end = $lines[$ln - 1] =~ /^\+/;
+				$cnt-- if ($f !~ /^-/);
  
  				next if ($f =~ /^-/);
-				last if (!$file && $f =~ /^\@\@/);
+				last if (!$file && $f =~ /^(?:\@\@|diff )/);
  
-				if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
-					$is_start = 1;
-				} elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) {
-					$length = -1;
+				if ($f =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) {
+					$needs_help = 1;
+					next;
+				} elsif ($f =~ /^\+\s*help\s*$/) {
+					$length = 0;
+					$has_help = 1;
+					next;
  				}
  
  				$f =~ s/^.//;

@@ -3510,16 +3512,16 @@ sub process {
  				# common words in help texts
  				if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice|
  						  if|endif|menu|endmenu|source)\b/x) {
-					$is_end = 1;
  					last;
  				}
-				$length++;
+				$length++ if ($has_help);
  			}
-			if ($is_start && $is_end && $length < $min_conf_desc_length) {
+			if ($needs_help &&
+			    (!$has_help ||
+			     ($has_help && $length < $min_conf_desc_length))) {
  				WARN("CONFIG_DESCRIPTION",
  				     "please write a paragraph that describes the config symbol fully\n" . $herecurr);
  			}
-			#print "is_start<$is_start> is_end<$is_end> length<$length>\n";
  		}
  
  # check MAINTAINERS entries