Re: [v2 10/10] iio: imu: add BNO055 I2C driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/9/21 10:21 AM, Joe Perches wrote:
(cc'ing Andi Kleen, who wrote this code a decade ago)

On Tue, 2021-11-09 at 07:47 -0800, Randy Dunlap wrote:
On 11/9/21 3:56 AM, Andrea Merello wrote:
Il giorno ven 29 ott 2021 alle ore 00:04 Randy Dunlap <rdunlap@xxxxxxxxxxxxx> ha scritto:
On 10/28/21 3:18 AM, Andrea Merello wrote:
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 <andrea.merello@xxxxxx>
---
    drivers/iio/imu/bno055/Kconfig      |  6 ++++
    drivers/iio/imu/bno055/Makefile     |  1 +
[]
diff --git a/drivers/iio/imu/bno055/Kconfig b/drivers/iio/imu/bno055/Kconfig
[]
@@ -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
[]
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@xxxxxxxxx/raw

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



Which now says:

WARNING: please write a paragraph that describes the config symbol fully
#16: FILE: drivers/iio/Kconfig:101:
+config BOSH_BNO055_I2C

(This is for a dummy entry that I made for testing, not from Andrea's
patch.)

Thanks, Joe.

Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Acked-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>

--
~Randy



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux