linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: frowand.list@gmail.com (Frank Rowand)
To: linux-arm-kernel@lists.infradead.org
Subject: [patch 2/7] dt: dtb version: document chosen/dtb-info node binding
Date: Thu, 19 Mar 2015 10:02:51 -0700	[thread overview]
Message-ID: <550B013B.1060002@gmail.com> (raw)
In-Reply-To: <20150319134910.GF18473@leverpostej>

On 3/19/2015 6:49 AM, Mark Rutland wrote:
> On Thu, Mar 19, 2015 at 03:33:22AM +0000, Frank Rowand wrote:
>> From: Frank Rowand <frank.rowand@sonymobile.com>
>>
>> Add /chosen/dtb-node binding.
> 
> Why? It doesn't matter what the cover says, the commit message should
> have a rationale.
> 
> Who needs this information, and when do they need it?
> 
> Why is the existing information insufficient?

Will update and add better information.

> 
>> Signed-off-by: Frank Rowand <frank.rowand@sonymobile.com>
>> ---
>>  Documentation/devicetree/bindings/chosen.txt |   37 +++++++++++
>>
>> Index: b/Documentation/devicetree/bindings/chosen.txt
>> ===================================================================
>> --- a/Documentation/devicetree/bindings/chosen.txt
>> +++ b/Documentation/devicetree/bindings/chosen.txt
>> @@ -46,6 +46,43 @@ on PowerPC "stdout" if "stdout-path" is
>>  should only use the "stdout-path" property.
>>  
>>  
>> +dtb-info node
>> +----------------
>> +
>> +Information that describes where the device tree blob (DTB) came from and the
>> +environment it was created in.
>> +
>> +This node is normally created by including arch/arm/boot/dts/skeleton.dtsi,
>> +which includes include/dt-bindings/version.dtsi.
>> +
>> +Properties:
>> +
>> +version
>> +	The version of the DTB.  This is analagous to the linux kernel version.
>> +
>> +	This is a format free field intended for human consumption.  User space
>> +	programs should not have any expections about this property.
> 
> I doubt that this would stay the case for long were this property added.
> 
>> +	The DTB number in this property is incremented each time a make that
>> +	creates one or more DTBs is invoked.  If the make creates multiple
>> +	DTBs then this number is only incremented once.
>> +
>> +	The DTB number is stored in file .version_dtb.
> 
> This is irrelevant to the binding, and as you mention above, you can
> make no expectations about this property.

OK, I'll remove the reference to .version_dtb.

> 
> I'm not sure I see the point in adding a property which is not
> well-defined and not guarnateed to be in any way stable.

This binding is kind of an odd ball to me.  It is clearly _not_ describing
hardware, which is really the central point of the dtb.  But the chosen
node is allowed to cover policy things like the boot command line.

So I would encourage a slightly unusual mind set when reviewing this
binding (not that I really know what that mind set is!).

The answer to the specific question of why add a "not well-defined"
binding is the same answer as to why have a kernel version string.
There was a proposal to remove the kernel version string and that
was quickly dropped.

I should have written more about why this is useful.

This binding provides the provenance of the DTB.  What source (and
version of the source) and version of the compiler (dtc) was used
to make the DTB.

If I want to debug DTB related issues, read the source that was used
to create the DTB, or slightly modify the DTB - where is the source
and what is the version of it in the source control system.

When building and installing a DTB, did the version that I wanted to
install on the target actually get on the target.

> 
>> +
>> +version-linux
>> +	The version of the linux kernel most recently built in the source
>> +	control system that contains the source used to build the DTB.
>> +
>> +	The linux kernel version number is not incremented for a make that
>> +	creates a DTB.
> 
> ...so if I build a DTB outside of a linux source tree I don't get to
> describe that?

I will make the name more generic.

> 
>> +dtb-path
>> +	The build directory relative path of the DTB.
>> +
>> +dts-path
>> +	The absolute path of the .dts file compiled to create the DTB.
> 
> Why do you need the DTB path?

Paranoia.  Even if not probable, one could build different DTBs from
the same .dts.

> 
> Why do these differ w.r.t. absolute/relative?

Those are the forms of the path that are present in the build
system.  If there is a good reason to change one of them to the
other form, I could add the extra complexity to massage the path.

> 
> Why would you _ever_ need an absolute path!?

The absolute path tells you which source repository contained the source.

> 
> Mark.
> 

  reply	other threads:[~2015-03-19 17:02 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-19  3:29 [patch 0/7] dt: dtb version: add version info to dtb Frank Rowand
2015-03-19  3:31 ` [patch 1/7] dt: dtb version: consolidate documentation of chosen node bindings Frank Rowand
2015-03-19 13:40   ` Mark Rutland
2015-03-19  3:33 ` [patch 2/7] dt: dtb version: document chosen/dtb-info node binding Frank Rowand
2015-03-19 13:23   ` Rob Herring
2015-03-19 16:42     ` Frank Rowand
2015-03-19 18:41     ` Russell King - ARM Linux
2015-03-19 18:53       ` Mark Rutland
2015-03-19 19:01       ` Frank Rowand
2015-03-19 19:32         ` Russell King - ARM Linux
2015-03-19 20:44           ` Frank Rowand
2015-03-20 14:42             ` Mark Rutland
2015-03-19 13:49   ` Mark Rutland
2015-03-19 17:02     ` Frank Rowand [this message]
2015-03-19 17:23       ` Geert Uytterhoeven
2015-03-19 19:12       ` Mark Rutland
2015-03-19 21:27         ` Frank Rowand
2015-03-20 15:25           ` Mark Rutland
2015-03-19 17:37   ` Frank Rowand
2015-03-19  3:34 ` [patch 3/7] dt: dtb version: arm dts Makefile Frank Rowand
2015-03-19  3:36 ` [patch 4/7] dt: dtb version: kernel Makefile Frank Rowand
2015-03-19  3:38 ` [patch 5/7] dt: dtb version: kbuild scripts Frank Rowand
2015-03-19  3:39 ` [patch 6/7] dt: dtb version: dtsi files Frank Rowand
2015-03-19 18:46   ` Sascha Hauer
2015-03-19  3:41 ` [patch 7/7] dt: dtb version: report dtb info Frank Rowand
2015-03-19  8:12 ` [patch 0/7] dt: dtb version: add version info to dtb Gregory CLEMENT
2015-03-19 17:05   ` Frank Rowand
2015-03-20 13:46 ` Rob Herring
2015-03-20 19:14   ` Uwe Kleine-König

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=550B013B.1060002@gmail.com \
    --to=frowand.list@gmail.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --subject='Re: [patch 2/7] dt: dtb version: document chosen/dtb-info node binding' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).