linux-kernel - Re: [PATCH 1/2] arm: dts: realview/versatile/ste: Update spi clock name

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] [day] [month] [year] [list]

Message-ID: <20220308063304.GA67419@9a2d8922b8f1>
Date:   Tue, 8 Mar 2022 12:03:04 +0530
From:   Kuldeep Singh <singh.kuldeep87k@...il.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        devicetree@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] arm: dts: realview/versatile/ste: Update spi clock
 name

On Mon, Mar 07, 2022 at 07:14:09PM -0600, Rob Herring wrote:
> On Mon, Mar 7, 2022 at 2:54 PM Kuldeep Singh <singh.kuldeep87k@...il.com> wrote:
> >
> > SPI clock for pl022 is "sspclk" and meanwhile few platforms such as
> > realview, versatile etc. specify "SSPCLK" as clock name. Even though
> > bindings checks don't differentiate between the two names still keep
> > same convention throughout i.e sspclk to align with other platforms.
> 
> I don't see the point in worrying about this. The binding already
> allows both. Why? Because I wrote the schema and checked it against
> all the in tree users. The resulting warnings are what should be fixed
> in the dts files IMO. What's not warning doesn't need to be fixed.

Hi Rob,

I personally share a different opinion though. Kindly bear with me for
my explanations.

As there were no checks on DTs before, so DTs have been defining names
which may not align with bindings. In case of pl022, doc says sspclk is
connected clock and whereas binding specify SSPCLK and sspclk both. Why
keep both versions in bindings? just because DTs have it. I don't think
it's justifiable enough and that's what prompted me to keep single
entry. Please let me know in case my understanding is incorrect.

I kept "sspclk" as it is aligned with "apb_pclk" and I have not seen
many DT defining names in caps.

Moreover, I ran out few checks and came up with some observations:
	1. Keep SSPCLK in binding, 'make dtbs_check' doesn't complain. An
	obvious behavior.
	2. Remove SSPCLK from binding, 'make dtbs_check' again doesn't
	complain.

For 2), though binding doesn't have SSPCLK, still DTs with clock-name
SSPCLK do not complain which further leads me to conclusion that casing
is not considered while comparing. So, why should we keep both and can't
simply discard other one.

This may not be a concern as per warnings point of view, but let's say
in future comparison checks get even more stricter, then this will roll
out as a warning and needs to be looked again. Why shall we leave out a
chance which may need to be revisited later?

Regards
Kuldeep