public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
From: Nishanth Menon <nm@ti.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Cc: Tero Kristo <kristo@kernel.org>, Rob Herring <robh+dt@kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
	Le Jin <le.jin@siemens.com>,
	Bao Cheng Su <baocheng.su@siemens.com>,
	Vignesh Raghavendra <vigneshr@ti.com>
Subject: Re: [PATCH v4 3/3] arm64: dts: ti: Add support for Siemens IOT2050 boards
Date: Thu, 11 Mar 2021 08:00:56 -0600	[thread overview]
Message-ID: <20210311140056.fzvke3rrg6c2uuoa@uncouth> (raw)
In-Reply-To: <8d076ff1-cdac-941f-e796-a2e6dba792ab@siemens.com>

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@siemens.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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-03-11 14:02 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-10  9:37 [PATCH v4 0/3] arm64: Add TI AM65x-based IOT2050 boards Jan Kiszka
2021-03-10  9:37 ` [PATCH v4 1/3] dt-bindings: Add Siemens vendor prefix Jan Kiszka
2021-03-10  9:37 ` [PATCH v4 2/3] dt-bindings: arm: ti: Add bindings for Siemens IOT2050 boards Jan Kiszka
2021-03-10  9:37 ` [PATCH v4 3/3] arm64: dts: ti: Add support " Jan Kiszka
2021-03-11 13:17   ` Nishanth Menon
2021-03-11 13:44     ` Jan Kiszka
2021-03-11 14:00       ` Nishanth Menon [this message]
2021-03-11 14:14         ` Jan Kiszka
2021-03-11 14:21           ` Nishanth Menon
2021-03-11 14:36             ` Jan Kiszka
2021-03-11 17:56               ` Nishanth Menon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210311140056.fzvke3rrg6c2uuoa@uncouth \
    --to=nm@ti.com \
    --cc=baocheng.su@siemens.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jan.kiszka@siemens.com \
    --cc=kristo@kernel.org \
    --cc=le.jin@siemens.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox