[PATCH] checkpatch: add DT compatible string documentation check

[PATCH] checkpatch: add DT compatible string documentation check

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

This adds a simple check that any compatible strings in DeviceTree dts
files are present in Documentation/devicetree/bindings. This should be a
temporary check until we have more sophisticated binding schema checking.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
Cc: Grant Likely <grant.likely@linaro.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Joe Perches <joe@perches.com>
---
 scripts/checkpatch.pl | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9c98100..3fc3517 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2034,6 +2034,21 @@ sub process {
 			     "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) if ($replacement->{$flag});
 		}
 
+# check for compatible documentation
+		if ($realfile =~ /\.dts/ && $rawline =~ /\+\s*compatible\s*=/) {
+			my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,]+)\"/g;
+
+			foreach my $compat (@compats) {
+				my $compat2 = $compat;
+				$compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/;
+				`egrep -rq "$compat|$compat2" "Documentation/devicetree/bindings/"`;
+				if ( $? >> 8 ) {
+					WARN("UNDOCUMENTED_DT_BINDING",
+					     "DT compatible string \"$compat\" appears un-documented -- check Documentation/devicetree/bindings/\n" . $herecurr);
+				}
+			}
+		}
+
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
 
-- 
1.8.3.2

Re: [PATCH] checkpatch: add DT compatible string documentation check

[Joe Perches]

On Tue, 2013-11-26 at 10:01 -0600, Rob Herring wrote:
> From: Rob Herring <rob.herring@calxeda.com>

Hi Rob.

> This adds a simple check that any compatible strings in DeviceTree dts
> files are present in Documentation/devicetree/bindings. This should be a
> temporary check until we have more sophisticated binding schema checking.
[]
> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
[]
> @@ -2034,6 +2034,21 @@ sub process {
>  			     "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) if ($replacement->{$flag});
>  		}
>  
> +# check for compatible documentation
> +		if ($realfile =~ /\.dts/ && $rawline =~ /\+\s*compatible\s*=/) {
> +			my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,]+)\"/g;

I believe there needs to be a \+\. in the brackets
for lines like:

arch/c6x/boot/dts/tms320c6678.dtsi: compatible = "ti,c64x+timer64";
and
arch/powerpc/boot/dts/mpc8313erdb.dts: compatible = "fsl,sec2.2", "fsl,sec2.1",

> +
> +			foreach my $compat (@compats) {
> +				my $compat2 = $compat;
> +				$compat2 =~ s/\,[a-z]*\-/\,<\.\*>\-/;
> +				`egrep -rq "$compat|$compat2" "Documentation/devicetree/bindings/"`;

Perhaps the egrep should be grep -E

> +				if ( $? >> 8 ) {
> +					WARN("UNDOCUMENTED_DT_BINDING",
> +					     "DT compatible string \"$compat\" appears un-documented -- check Documentation/devicetree/bindings/\n" . $herecurr);
> +				}
> +			}
> +		}
> +
>  # check we are in a valid source file if not then ignore this hunk
>  		next if ($realfile !~ /\.(h|c|s|S|pl|sh)$/);
>  

Re: [PATCH] checkpatch: add DT compatible string documentation check

[Rob Herring]

On Tue, Nov 26, 2013 at 10:43 AM, Joe Perches <joe@perches.com> wrote:
> On Tue, 2013-11-26 at 10:01 -0600, Rob Herring wrote:
>> From: Rob Herring <rob.herring@calxeda.com>
>
> Hi Rob.
>
>> This adds a simple check that any compatible strings in DeviceTree dts
>> files are present in Documentation/devicetree/bindings. This should be a
>> temporary check until we have more sophisticated binding schema checking.
> []
>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> []
>> @@ -2034,6 +2034,21 @@ sub process {
>>                            "Use of $flag is deprecated, please use \`$replacement->{$flag} instead.\n" . $herecurr) if ($replacement->{$flag});
>>               }
>>
>> +# check for compatible documentation
>> +             if ($realfile =~ /\.dts/ && $rawline =~ /\+\s*compatible\s*=/) {
>> +                     my @compats = $rawline =~ /\"([a-zA-Z0-9\-\,]+)\"/g;
>
> I believe there needs to be a \+\. in the brackets
> for lines like:
>
> arch/c6x/boot/dts/tms320c6678.dtsi: compatible = "ti,c64x+timer64";

I think we don't really want '+' (or '_') in compatible strings. That
would really a check against the documentation versus whether the dts
matches the documentation. So I probably should add a separate check
on use of documented, but deprecated compatible strings. Ideally, that
check would be on the binding doc, but since they are unstructured
currently, we'd have to do it on dts files.

> and
> arch/powerpc/boot/dts/mpc8313erdb.dts: compatible = "fsl,sec2.2", "fsl,sec2.1",

Yes, indeed.

Rob

Re: [PATCH] checkpatch: add DT compatible string documentation check

[Joe Perches]

On Tue, 2013-11-26 at 11:11 -0600, Rob Herring wrote:
> I think we don't really want '+' (or '_') in compatible strings.

What about lines like:
arch/powerpc/boot/dts/kmeter1.dts:  compatible = "ucc_geth";

Re: [PATCH] checkpatch: add DT compatible string documentation check

[Rob Herring]

On 11/26/2013 11:29 AM, Joe Perches wrote:
> On Tue, 2013-11-26 at 11:11 -0600, Rob Herring wrote:
>> I think we don't really want '+' (or '_') in compatible strings.
> 
> What about lines like:
> arch/powerpc/boot/dts/kmeter1.dts:  compatible = "ucc_geth";

I didn't say we don't have them, but for new bindings we want to flag
things like this. It is fine if new dts files use those existing
bindings, but no one should be creating new bindings with the deprecated
style. The problem is that I can't really automate this check on the
binding documentation until we move to schema based bindings (which
won't be soon). So I would like to warn if dts files use deprecated
styles. I don't expect this to be a frequent occurrence.

Rob