netdev - Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

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]

Date:	Tue, 9 Aug 2011 14:13:01 -0500
From:	Scott Wood <scottwood@...escale.com>
To:	Robin Holt <holt@....com>
CC:	Marc Kleine-Budde <mkl@...gutronix.de>,
	Wolfgang Grandegger <wg@...ndegger.com>,
	U Bhaskar-B22300 <B22300@...escale.com>,
	<netdev@...r.kernel.org>, <socketcan-core@...ts.berlios.de>,
	PPC list <linuxppc-dev@...ts.ozlabs.org>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>
Subject: Re: [PATCH 5/5] [powerpc] Fix up fsl-flexcan device tree binding.

On 08/09/2011 01:45 PM, Robin Holt wrote:
> On Tue, Aug 09, 2011 at 01:17:47PM -0500, Scott Wood wrote:
>> On 08/09/2011 09:43 AM, Robin Holt wrote:
>>> In working with the socketcan developers, we have come to the conclusion
>>> the fsl-flexcan device tree bindings need to be cleaned up. 
>>> The driver does not depend upon any properties other than the required properties
>>> so we are removing the file.
>>
>> That is not the criterion for whether something should be expresed in
>> the device tree.  It's a description of the hardware, not a Linux driver
>> configuration file.  If there are integration parameters that can not be
>> inferred from "this is FSL flexcan v1.0", they should be expressed in
>> the node.
> 
> There are no properties other than the required properties.  The others
> were wrongly introduced and are not needed by the driver.

Not needed by this driver, or will never be needed by any reasonable
driver (or is not a good description of the hardware)?

The device tree is not an internal Linux implementation detail.  It is
shared by other OSes, firmwares, hypervisors, etc.  Bindings should be
created with care, and kept stable unless there's a good reason to break
compatibility.

devicetree-discuss@...ts.ozlabs.org should be CCed on device tree
discussions.

> When we
> removed the other properties and the wrong documentation of the mscan
> oscillator source in the fsl-flexcan.txt file, we were left with an
> Example: section and a one-line statement "The only properties supported
> are the required properties."  That seemed like the fsl-flexcan.txt
> file was then pointless.

There is the compatible string, and you could mention that there is a
single reg resource and a single interrupt.

>> Removing the binding altogether seems extreme as well -- we should have
>> bindings for all devices, even if there are no special properties.
> 
> Ok.  I can do that too.  Who is the definitive source for that answer?

For policy questions on device tree bindings?  Grant Likely is the
maintainer for device tree stuff.

A lot of the simpler bindings have been left undocumented so far, IMHO
it should be a goal to document them all.  There are some existing ones
that are documented despite not having special properties, e.g.
Documentation/devicetree/bindings/serio/altera_ps2.txt,
Documentation/devicetree/bindings/arm/sirf.txt,
Documentation/devicetree/bindings/powerpc/nintendo/wii.txt, etc.

> I assume we are talking about the fsl-flexcan.txt file when we say
> binding.  Is that correct?

Yes, although devicetree.org is another possibility.

>>> Additionally, the p1010*dts files are not
>>> following the standard for node naming in that they have a trailing -v1.0.
>>
>> What "standard for node naming"?  There's nothing wrong with putting a
> 
> For the answer to that, you will need to ask Wolfgang Grandegger.  I was
> working from his feedback.  Looking at the plethora of other node names,
> the vast majority do not have any -v#.#, and the ones that do also tend
> to have multiple versions. Based upon that, I suspect he is correct,
> but I do not know where the documentation is or if it even exists.

There's a lot of crap in old bindings, plus it's not appropriate for all
circumstances (specifying bindings should be done a little more
carefully than "what do most other bindings do?").  It's something we've
been doing lately for blocks that have a version number, but it's not
dynamically readable.

Looking in the FlexCAN chapter of the p1010 manual, I don't see any
reference to a block version, and I do see references to "previous
FlexCAN versions".  So I suggest "fsl,p1010-flexcan".

-Scott

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html