linux-kernel - Re: [PATCH v4 3/3] arm64: dts: ti: Add support for Siemens IOT2050 boards

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: <20210311140056.fzvke3rrg6c2uuoa@uncouth>
Date:   Thu, 11 Mar 2021 08:00:56 -0600
From:   Nishanth Menon <nm@...com>
To:     Jan Kiszka <jan.kiszka@...mens.com>
CC:     Tero Kristo <kristo@...nel.org>, Rob Herring <robh+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        Le Jin <le.jin@...mens.com>,
        Bao Cheng Su <baocheng.su@...mens.com>,
        Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: Add support for Siemens IOT2050
 boards

On 14:44-20210311, Jan Kiszka wrote:
> On 11.03.21 14:17, Nishanth Menon wrote:
> > On 10:37-20210310, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@...mens.com>
> >> +	spidev@0 {
> >> +		compatible = "rohm,dh2228fv";
> >> +		spi-max-frequency = <20000000>;
> >> +		reg = <0>;
> > 
> > Jan,
> > 
> > As part of my final sanity checks, I noticed that we missed this: is a checkpatch warning
> > 
> > WARNING: DT compatible string "rohm,dh2228fv" appears un-documented -- check ./Documentation/devicetree/bindings/
> > #629: FILE: arch/arm64/boot/dts/ti/k3-am65-iot2050-common.dtsi:581:
> > 		compatible = "rohm,dh2228fv";
> > 
> > I cannot pick up nodes that are'nt documented as yaml in
> > 	Documentation/devicetree
> > 
> > I know this is irritating to find such nodes that already have previous
> > users and the person coming last gets to deal with "new rules".. but
> > sorry for catching this so late.
> > 
> > Here are the options that come to mind:
> > 
> > option 1) - drop the node and resubmit.
> > 
> > option 2) - get the documentation into linux master tree and then submit
> > the patches.
> > 
> 
> As you said, I'm not setting a precedence here:
> 
> arch/arm/boot/dts/imx28-cfa10049.dts:                   compatible = "rohm,dh2228fv";
> arch/arm/boot/dts/rv1108-elgin-r1.dts:          compatible = "rohm,dh2228fv";
> arch/arm/boot/dts/socfpga_cyclone5_socdk.dts:           compatible = "rohm,dh2228fv";
> drivers/spi/spidev.c:   { .compatible = "rohm,dh2228fv" },
> 
> Was just just never documented as binding? Or why is no one allowed to 
> use this anymore? What is to be used instead for spidev?

See [1] compare the compatibles against
Documentation/devicetree/bindings -> I think you should describe what
your hardware really is though.

Unfortunately devicetree migration has been far from being smooth.. it
was like chewing an elephant - linux community had to attack it in
pieces..

Yes - it was unfortunately one of those cases where the driver support
was introduced long back and no binding was introduced at that time (it
was'nt mandatory then).. then we added a mandatory requirement that it
be documented in txt.. over years realized things are'nt great with
unstructured txt description of binding, now moving on converting
existing txt files to yaml and schemas to static check the dts...
evolution over the years, I guess.

I am on a fight internally as well to have all our legacy txt files
converted over to yaml.. and am having to put up a stance - see [2]

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/spi/spidev.c#n678
[2] https://lore.kernel.org/linux-arm-kernel/20210311134908.jsh2lywtwzvlyvbc@finally/T/#u

> 
> > 
> > I think we should just drop the node and resubmit - since this is a more
> > intrusive change and I don't have your platform handy, I am going to
> > suggest you make a call :(
> 
> This breaks userspace here, and we would need to carry that node on top.
> 

Uggh... that sucks.. but I think that would be lower tradeoff to make
than me (as it stands now) having to drop the patch series.

> BTW, I already brought up the topic internally to get you some boards 
> for testing.

Thanks.. While it might help me personally to get some on my internal
farm, it might be good to get them on kernelci as well on the longer
run.

> 
> I've done that and addressed all that I could (former patch 4). We 
> import those from k3, and I don't feel confident how to resolve them.
> See also v1 of this patch.

Yeah - i noticed that upstream dt-schema has gotten even more stricter
even though the dts has remained the same.. I need to spend time in
digging at it.

At this point the only big kicker is the checkpatch stuff which I cant
let through - if i do that arnd will probably kick everything from my
PR out :( - which I cant do.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D