#ranges-address-cells and #ranges-size-cells are undefined

[mbolivar-nordic]

These properties are not defined in specification.md:

https://github.com/devicetree-org/lopper/blob/da29be7393b1b3237c3a566b42a5dd5f81cf391c/specification/specification.md

They appear to be related to the address-map property, but they are not defined in the specification and their purpose is unclear. They only appear in the system-device-tree.dts document, whose relation to the specification is not clear, because specification.md never refers to system-device-tree.dts.

It is also unclear why the secure-address-map property should have a different location where the number of address and size cells is set than the regular address-map property, which uses #address-cells and #size-cells as far as I can tell from examples. But again, this is not defined in the specification.

[zeddii]

@sstabellini this is for you (but i can't assign it, as you aren't in the project yet).

[sstabellini]

Let's put aside secure-address-map given that it is about to go away as per issue #130.

As you can see from this patch:
https://lists.openampproject.org/archives/list/system-dt@lists.openampproject.org/thread/QEQAZUXYKT7P37RBSD4LBR5KRNWRUCVF/

#ranges-address-cells represents the number of cells to use for the first address in address-map.
#ranges-size-cells represents the number of cells to use for the size (last parameter) in address-map.

I realize that the detailed description of the binding as per the patch above didn't make it into specification.md. If you are happy to add that to the pull request, we'll merge it.

[mbolivar-nordic]

Thanks for the reference!

As you can see from this patch: https://lists.openampproject.org/archives/list/system-dt@lists.openampproject.org/thread/QEQAZUXYKT7P37RBSD4LBR5KRNWRUCVF/

I am unaware of the repository this patch applies to, though -- could you please explain what this is? My concern here is that there seem to be two places where system devicetree related content is being defined. IMO it would be preferable for core concepts like address map to be defined in the specification, and I'm worried about bitrot and content skew if there are multiple places where definitions are going.

[sstabellini]

Sorry for the confusion: that patch was never applied anywhere and predates the System Device Tree specification in the Lopper repository. Today there is only one place for the System Device Tree specification and it is under the Lopper repository.

However I linked the patch because it has a good write-up to define address-map together with #ranges-address-cells and #ranges-size-cells which is missing in specification.md. We need to add that content to specification.md in the Lopper repository.

[mbolivar-nordic]

Understood and thanks for explaining. I will send a patch.

[mbolivar-nordic]

@sstabellini as I mentioned in #131, here are some follow-up questions before I can send a patch.

The format of the value of the address-map property is an arbitrary number
of quartets of (node-address, ref-node, root-node-address, length).
Each quartet specified describes a contiguous address range.

• the node-address is a physical address within this node address space
  (the node in which the address-map property appears). The number of cells
  to represent the address is determined by the #ranges-address-cells
  property of this node.

This is a clear definition of the role of #ranges-address-cells, thanks.

• ref-node is a phandle that points to the node describing the resource
  whose address is being mapped into this node address space.

This is reasonably clear, but I believe it also suffers from the
problem described in #128: indirect-bus nodes themselves don't
(shouldn't) have addresses. The child nodes of indirect-bus nodes have
addresses, but the indirect-bus nodes themselves do not.

So I think the language should rather be something like:

  * *ref-node* is a phandle that points to the bus node whose resources
    (child nodes of the bus node) are being mapped into this node's address
    space.

OK with you?

• root-node-address is a physical address within the root node address
  space. The number of cells to represent the parent address can be
  determined from the #address-cells property of root node.

This "feels" wrong to me and I would need some clarification before I
could write a patch describing this.

Shouldn't it be taken from the #address-cells property of the
indirect bus node instead? There could be different numbers of address
cells in different indirect buses, I guess, so it seems like we should
be allowing for different number of cells for each entry in the
address-map property to handle this.

Could you please clarify why the root node's #address-cells is the
right choice?

• length specifies the size of the range in this node address space.
  The number of cells to represent the size can be determined by the
  #ranges-size-cells of this node.

I've been muddling over how to handle this and some things are not
quite clear to me.

Here is a base system devicetree where I'm purposely leaving
address-map blank so we can consider a few different examples:

/ {
	#address-cells = <1>;

	cpus-cluster {
		compatible = "cpus,cluster";
		#ranges-address-cells = <1>;
		#ranges-size-cells = <1>;

		address-map = /* ... will change per example ... */
	};

	bus: foo-bus {
		compatible = "indirect-bus";
		#address-cells = <1>;
		#size-cells = <1>;

		foo: node@0 {
			reg = <0 0x1000>;
		};

		bar: node@1000 {
			reg = <0x1000 0x1000>;
		};
	};
};

Here are a few example address-map properties that I'd like to
understand the meaning of more clearly.

Example 1: address-map = <0 &bus 0 0x2000>;

The intent here seems clear:

• foo is visible to /cpus-cluster and has base address 0
• bar is visible to /cpus-cluster and has base address 0x1000

Right?

Example 2: address-map = <0 &bus 0 0x1000>;

I'm not exactly sure what the shorter value of *length* should mean
in this case, but my guess is:

• foo is visible to /cpus-cluster and has base address 0
• bar is not visible to /cpus-cluster

Is that right?

Example 3: address-map = <0 &bus 0 0x1500>;

The meaning of this is not clear to me. Is bar visible or not? Is
this a buggy system devicetree that should result in an error
condition?

[sstabellini]

@sstabellini as I mentioned in #131, here are some follow-up questions before I can send a patch.

The format of the value of the address-map property is an arbitrary number
of quartets of (node-address, ref-node, root-node-address, length).
Each quartet specified describes a contiguous address range.

• the node-address is a physical address within this node address space
  (the node in which the address-map property appears). The number of cells
  to represent the address is determined by the #ranges-address-cells
  property of this node.

This is a clear definition of the role of #ranges-address-cells, thanks.

• ref-node is a phandle that points to the node describing the resource
  whose address is being mapped into this node address space.

This is reasonably clear, but I believe it also suffers from the problem described in #128: indirect-bus nodes themselves don't (shouldn't) have addresses. The child nodes of indirect-bus nodes have addresses, but the indirect-bus nodes themselves do not.

So I think the language should rather be something like:

  * *ref-node* is a phandle that points to the bus node whose resources
    (child nodes of the bus node) are being mapped into this node's address
    space.

OK with you?

It doesn't have to be a bus node. In our example we have:

	address-map = <0xf1000000 &amba 0xf1000000 0xeb00000>,
	              <0xf9000000 &amba_rpu 0xf9000000 0x10000>,
	              <0x0 &memory 0x0 0x80000000>,
	              <0x0 &tcm 0xFFE90000 0x10000>;

Where amba and amba_rpu are bus nodes, but that is just for convenience. Also memory nodes, sram nodes are acceptable, like memory and tcm. But also device nodes like can@ff070000 are also acceptable. We need to use a language that is generic enough to cover all the possible cases.

• root-node-address is a physical address within the root node address
  space. The number of cells to represent the parent address can be
  determined from the #address-cells property of root node.

This "feels" wrong to me and I would need some clarification before I could write a patch describing this.

Shouldn't it be taken from the #address-cells property of the indirect bus node instead? There could be different numbers of address cells in different indirect buses, I guess, so it seems like we should be allowing for different number of cells for each entry in the address-map property to handle this.

Could you please clarify why the root node's #address-cells is the right choice?

In practice the description I used is the closest to what you would like to see as possible given the way device tree works. Device tree always coverts addresses from parent to children or from children to parent. We cannot take an address child of bus elsewhere out of context. There could be parent busses that perform other address translations.

The best we can do is to use the parent address space, to which the indirect-bus node is translating addresses to.

• length specifies the size of the range in this node address space.
  The number of cells to represent the size can be determined by the
  #ranges-size-cells of this node.

I've been muddling over how to handle this and some things are not quite clear to me.

Here is a base system devicetree where I'm purposely leaving address-map blank so we can consider a few different examples:

/ {
	#address-cells = <1>;

	cpus-cluster {
		compatible = "cpus,cluster";
		#ranges-address-cells = <1>;
		#ranges-size-cells = <1>;

		address-map = /* ... will change per example ... */
	};

	bus: foo-bus {
		compatible = "indirect-bus";
		#address-cells = <1>;
		#size-cells = <1>;

		foo: node@0 {
			reg = <0 0x1000>;
		};

		bar: node@1000 {
			reg = <0x1000 0x1000>;
		};
	};
};

Here are a few example address-map properties that I'd like to understand the meaning of more clearly.

Example 1: address-map = <0 &bus 0 0x2000>;

The intent here seems clear:

* `foo` is visible to `/cpus-cluster` and has base address `0`

* `bar` is visible to `/cpus-cluster` and has base address `0x1000`

Right?

Correct

Example 2: address-map = <0 &bus 0 0x1000>;

I'm not exactly sure what the shorter value of *length* should mean in this case, but my guess is:

* `foo` is visible to `/cpus-cluster` and has base address `0`

* `bar` is _not_ visible to `/cpus-cluster`

Is that right?

Correct

Example 3: address-map = <0 &bus 0 0x1500>;

The meaning of this is not clear to me. Is bar visible or not? Is this a buggy system devicetree that should result in an error condition?

The first 0x500 bytes of bar are visible, not the rest. Is it valid? Depends on your configuration. I expect this is not valid in most cases, but it is always possible to find a corner case where this is valid, so we cannot rule it out from the specification. Think of bar as an mmio-sram region which you only want to give access to the first half to the cpu-cluster.

[mbolivar-nordic]

It doesn't have to be a bus node. In our example we have:

Ack, thanks. That is a useful example in system-device-tree.dts; I see that now. The specification text only has examples which reference bus nodes within address-map properties, which is why I think I missed it until now. I'll make sure the improved example lands in the spec patch as well.

The best we can do is to use the parent address space, to which the indirect-bus node is translating addresses to.

OK, follow-up on that then. The language you linked refers to a "physical address within the root node address
space", but then what if an indirect-bus node appears several levels deep in the tree, with intermediate ranges properties and different #address-cells changes between the root node and the parent of the indirect-bus?

The first 0x500 bytes of bar are visible, not the rest.

Interesting, so even though it's specified as a single register block, it's "cut off" halfway.

What I mean is, it would have seemed more intuitive to me to "cut" separate register blocks like this:

		bar: node@1000 {
			reg = <0x1000 0x500>, <0x1500 0x500>;
		};

Then only the first register block remains visible through the address-map and that seems sensible. Splitting a register block halfway through is a surprise to me, so that is a useful clarification, thanks.

Is it valid? Depends on your configuration.

OK, that's clear now. Maybe it's implicit in the way lopper works, but that wasn't obvious to me from the sample lops file I got from @zeddii that I've been basing my work on.

OK, I think other than the question about if you really mean the root node's #address-cells, it's all clear to me now.

[sstabellini]

The best we can do is to use the parent address space, to which the indirect-bus node is translating addresses to.

OK, follow-up on that then. The language you linked refers to a "physical address within the root node address space", but then what if an indirect-bus node appears several levels deep in the tree, with intermediate ranges properties and different #address-cells changes between the root node and the parent of the indirect-bus?

The address gets translated again and again until it reaches the top. The address at the top is the one we are supposed to use in address-map. We can't really use intermediary addresses.

The first 0x500 bytes of bar are visible, not the rest.

Interesting, so even though it's specified as a single register block, it's "cut off" halfway.

What I mean is, it would have seemed more intuitive to me to "cut" separate register blocks like this:

		bar: node@1000 {
			reg = <0x1000 0x500>, <0x1500 0x500>;
		};

Then only the first register block remains visible through the address-map and that seems sensible. Splitting a register block halfway through is a surprise to me, so that is a useful clarification, thanks.

Is it valid? Depends on your configuration.

OK, that's clear now. Maybe it's implicit in the way lopper works, but that wasn't obvious to me from the sample lops file I got from @zeddii that I've been basing my work on.

OK, I think other than the question about if you really mean the root node's #address-cells, it's all clear to me now.

Yeah. Longer explanation: I mean the #address-cells of the parent but given that cpu-clusters are always top-level nodes, then the #address-cells of the parent is really the #address-cells of the root node.

[mbolivar-nordic]

I'll include the fix for this issue as part of a general rework to make the system DT spec look like the DT spec in terms of how it defines bindings and the properties in them, so it might take a week or so to put it all together.