linux-kernel - Re: [PATCH 01/18] MIPS: DTS: jz4780: remove cpu clock-names as reported by dtbscheck

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <B8D9DAF0-435B-4701-84E4-580FAA9CE74F@goldelico.com>
Date:   Sat, 9 Apr 2022 15:02:05 +0200
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
Cc:     Rob Herring <robh+dt@...nel.org>,
        Paul Cercueil <paul@...pouillou.net>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        linux-mips@...r.kernel.org, letux-kernel@...nphoenux.org
Subject: Re: [PATCH 01/18] MIPS: DTS: jz4780: remove cpu clock-names as
 reported by dtbscheck

Hi Krzysztof,

> Am 09.04.2022 um 13:07 schrieb Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>:
> 
> On 08/04/2022 20:37, H. Nikolaus Schaller wrote:
>> arch/mips/boot/dts/ingenic/ci20.dtb: cpu@0: clock-names does not match any of the regexes: pinctrl-[0-9]+
>> 	From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
>> arch/mips/boot/dts/ingenic/ci20.dtb: cpu@1: clock-names does not match any of the regexes: pinctrl-[0-9]+
>> 	From schema: Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
> 
> No need to put the same warning twice (cpu index really does not
> matter). One warning is enough.

Well, how can you as a reviewer see immediately that dtbscheck
warns for both instances and that they are both really fixed by the
complete patch?

Assume there were only the first one mentioned and the second one
forgotten to fix...

If it is not included here (completely), you have to run dtbscheck 
yourself to see that there are two instances involved. Or deduce it
from looking into the source file.

In both cases much more work for you or other reviewers.

This is why I have kept this redundancy. One commit hunk for every
warning line.

And it may only look redundant in the commit message, not in code 
where it would really hurt to have duplication.

> 
> Fixing warnings is good, but what if the property should be there and
> the bindings are not correct? If you know the answer, please add it to
> the commit msg.

It has of course been tested on real hardware.
There was only one case where the bindings seem to be wrong (patch 13/18).

And usually I simply assume such generic bindings are tested on many
other platforms and therefore the reference. What are bindings and
dtbscheck good for if we doubt them as DTS developers?

> This applies to all your patches. Blind fixing of DTS warnings my
> produce incorrect results. :(

Every patch of this series has been tested on real hardware to have
no negative side-effects.

> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
> 
> 
> Best regards,
> Krzysztof

BR and thanks,
Nikolaus