Messages in this thread | | | Subject | Re: [v2 10/10] iio: imu: add BNO055 I2C driver | From | Randy Dunlap <> | Date | Tue, 9 Nov 2021 11:11:42 -0800 |
| |
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@infradead.org> 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@iit.it> >>>>> --- >>>>> 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@gmail.com/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@infradead.org> Acked-by: Randy Dunlap <rdunlap@infradead.org>
-- ~Randy
| |