Giter Club home page Giter Club logo

lopper's People

Contributors

bentheredonethat avatar conallogriofaamd avatar dependabot[bot] avatar etoillon avatar habetsm-xilinx avatar hakonfam avatar its-izhar avatar kedareswararao avatar madhavamd avatar mbolivar-nordic avatar onkarharsh avatar orzelmichal avatar shubhraamd avatar sivadur avatar srilaxmig avatar tammyleino avatar zeddii avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

lopper's Issues

Missing `reg` semantics for child nodes of indirect buses

As of 4fc81d4, the spec seems to
leave the semantics here undefined:

/ {
	bus {
		compatible = "indirect-bus";
		clock {
			#clock-cells = <0>;
			compatible = "fixed-clock";
			clock-frequency = <32768>;
		};
	};
};

If we map in bus, do we always map in clock?

The current semantics for how nodes under an indirect-bus are included
or excluded rely on each node's reg property, which doesn't cover
this case.

Shared access via `access` property

As of 4fc81d4, the access property in the
"openamp,domain-v1" binding has the following in its specification:

   Description A list of devices the domain shall have exclusive access to,
               using bus firewalls or other similar technologies.

I can imagine scenarios where a precisely specified subset of the
execution domains in a system, each with a different CPU, needs access
to a resource, not just the default domain, a single domain, or a set
of domains that share a CPU.

Consider a scenario where an application processor needs to share a
peripheral at runtime with a helper core running an RTOS, with mutual
exclusion established at runtime via out-of-band means like an RPMsg
channel. This has 2 cpu clusters, each with their own domain, that
both need access to the peripheral. But absent other mutual exclusion,
in the system we want to prevent access from any other cluster, etc.

There doesn't seem to be a way to describe this in the current binding.

Am I missing something?

bug in parsing .dts files

Given certain values in the reg property of node, value not parsed properly.

sample input device-trees/system-device-tree.dts attached. output for the node

    channel0vdev0buffer: channel0vdev0buffer@3ed48000 {
            no-map;
            #address-cells = <2>;
            #size-cells = <2>;
            reg = <0x3ed48000 0x100000>;
            compatible = "openamp,xlnx,mem-carveout";
    };

should print the register value as 0x3ed48000 0x100000
but when parsed via

  • for k,v in sdt.tree.nodes.items():
  •    if "channel0" in k: #and "openamp,xlnx,mem-carveout" in v["compatible"]:
    
  •        print(v['reg'])
    

output is reg = ">ิ€","","","";

system-device-tree.dts.txt

Requiring `#ranges-address-cells` and `#ranges-size-cells`

As of 4fc81d4, neither the
#ranges-address-cells nor #ranges-size-cells properties in the
"cpus,cluster" binding are required in the specification.

It doesn't seem possible to do much with a CPU cluster that doesn't
have this information, though, because there's no way to specify its
address map without them.

Shouldn't these be required?

Purpose of secure-bus?

When I was last working on system devicetree, we were discussing how to represent address maps for Arm v8-M CPUs with TrustZone support. In this case, the same IP blocks often effectively have different addresses depending on the security state of the CPU at the time of access.

It looks like secure-bus might be meant as a solution to that. Is that so? If so, I think it's not needed at least for Zephyr and it actually complicates things for us. I think having as many address-map properties as needed, with identical semantics (and therefore no differences in the places where the number of address and size cells are set; see #129 for more on this) would be better.

In effect this is similar to the way the pinctrl-<N> properties work for pin control: any node with pin control support can have pinctrl-0, pinctrl-1, ... properties, as many as needed, and with identical semantics. Each property represents a different pin selection depending on the state of the IP block modeled by the node.

Similarly, as many address map properties as needed, each of which reflects some state of the cpus cluster, would seem to work well for us handling Arm v8-M in zephyr, at least for now. (This opens up the possibility of having an address-map or address-map-0 property, then address-map-1, address-map-2, ... as well, but that is probably a separate issue.)

Why do domains have a mandatory `id`?

As of 4fc81d4, the "openamp,domain-v1" binding says id is required.

Why?

Why do we need an id at all, given that we can already refer to a
domain explicitly by its phandle, etc.?

At least within Zephyr, I cannot think of a reason why we would use
this property, so I think it should at least be made optional.

Clarifications on non-indirect-bus reference nodes in `address-map`

I could use some general clarification on what happens if a device
node is directly mapped into an address-map property, but it's the
child of one or more nodes which are not visible to the
corresponding CPU cluster.

Abstractly, the idea that "the node is visible but its parents aren't"
doesn't quite make sense to me from the perspective of how to generate
an equivalent standard devicetree.

For example:

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

		/* Non-secure address map. */
		address-map = <0 &peripheral 0 0x1000>;

		/* ... */
	};

	bus {
		compatible = "indirect-bus";
		intermediate-node@... {
			peripheral: peripheral@... {
				reg = <... 0x1000>;
			};
		};
	}
};

It's not clear to me how to generate a standard devicetree for this
that both preserves the structure of the original system devicetree
and respects visibility within an address map.

Can we introduce some sensible restrictions on non-bus nodes in
address-map properties to avoid ambiguity here?

For example, one rule which would clear it up for me would be "such
nodes must be children of the root node". Then there's no problem with
intermediate nodes, and address translation through the root node's
#address-cells and #size-cells properties seems trivial, since the
peripheral is already directly underneath the root.

I was thinking perhaps we need some sort of rule along these lines:

  • you have to have explicit address translation via ranges
    properties as needed all the way from the root to the node you are
    mapping in via address-map

  • all the nodes along the path from root to the peripheral
    have to be mapped into the address-map too

But that seems to break other simple cases, like mapping in a SPI or
I2C slave:

/ {
	bus {
		compatible = "indirect-bus";
		spi@deadbeef {
			cs-gpios = <&gpio1 0 GPIO_ACTIVE_LOW>;
			sensor@0 { /* ... */ };
		};
	}
};

Here it seems clear that we want sensor@0 mapped into the CPU
cluster if we map in bus, so there must be some sort of transitive
inclusion of child nodes when you map in parent nodes, at least for
indirect-bus nodes.

How to add lable for existing node?

The format is: "path":"property":"replacement" to modify property. My original node is as below:

                cpu@0 {
                        device_type = "cpu";
                        compatible = "arm,armv8";
                        reg = <0x00 0x00>;
                        enable-method = "psci";
                };

And I want to change it to

                CPU0:cpu@0 {
                        device_type = "cpu";
                        compatible = "arm,armv8";
                        reg = <0x00 0x00>;
                        enable-method = "psci";
                };

How to add it?

When the existing device has some phandle and add a node with label, it will cause duplicated phandle issues. I used below operation to add a node.

      lop_2 {
           // add node
           compatible = "system-device-tree-v1,lop,add";
           node_src = "idle-states";
           node_dest = "/cpus/idle-states";
           idle-states {
               entry-method = "arm,psci";

               CPU_SLEEP_0: cpu-sleep-0 {
                   compatible = "arm,idle-state";
                   local-timer-stop;
                   arm,psci-suspend-param = <0x0010000>;
                   entry-latency-us = <40>;
                   exit-latency-us = <100>;
                   min-residency-us = <150>;
               };

               CLUSTER_SLEEP_0: cluster-sleep-0 {
                   compatible = "arm,idle-state";
                   local-timer-stop;
                   arm,psci-suspend-param = <0x1010000>;
                   entry-latency-us = <500>;
                   exit-latency-us = <1000>;
                   min-residency-us = <2500>;
               };
           };
       };

And I used dtc to covert the dtb file to dts file, it has below errors

$ dtc -I dtb fvp-base-revc-xen.dtb -o fvp-base-revc-xen.dts
fvp-base-revc-xen.dts: ERROR (explicit_phandles): /cpus/idle-states/cluster-sleep-0: duplicated phandle 0x2 (seen before at /bus@8000000/motherboard-bus@8000000/iofpga-bus@300000000/sysreg@10000)
fvp-base-revc-xen.dts: ERROR (explicit_phandles): /cpus/cpu0: duplicated phandle 0x3 (seen before at /refclk32khz)
fvp-base-revc-xen.dts: ERROR (explicit_phandles): /interrupt-controller@2f000000: duplicated phandle 0x1 (seen before at /cpus/idle-states/cpu-sleep-0)
ERROR: Input tree has errors, aborting (use -f to force output)

specification: #address-cells and #size-cells for domain memory and sram properties are unclear

memory is a sequence of start, size, flags tuples. #address-cells and
#size-cells express how many cells are used to specify start and size
respectively.

https://raw.githubusercontent.com/devicetree-org/lopper/0da7cbd70bc2c3417e5e909bdd90eea2a0a0c912/specification/source/chapter3-execution-domains.rst

Is that the #address-cells and #size-cells for the root node, or something else? It can't be those properties for the CPU cluster associated with the domain, because those values must be 1 and 0 respectively.

Should this actually say #ranges-address-cells instead of #address-cells, and #ranges-size-cells instead of #size-cells?

The same question applies to the sram domain property.

Updated `address-map` properties for Arm v8-M and similar

Problem statement

The existing address-map property in the CPU cluster binding does
not properly account for multiple possible cluster address maps
depending on runtime CPU state.

Example

The motivating example is an Arm v8-M CPU with TrustZone. SoCs with
these CPUs frequently have different secure- and non-secure addresses
for identical peripherals. That is, the CPU clusters in these SoCs
have address maps that vary based on the current execution level of
the CPU.

Proposal

  1. Extend the "cpus,cluster" binding to allow properties matching the
    pattern address-map-* to allow for multiple address maps.

    This draws inspiration from the pinctrl binding,
    which allows users to select multiple pin configurations, one per
    desired state, using pinctrl-<N> properties. (In the address map
    situation, we are using multiple properties to describe hardware
    rather than configure it, but the mechanism is similar.)

  2. Extend the hardware specific portions of the "openamp,domain-v1"
    binding to allow the domain to express which address map to use,
    either implicitly or explicitly.

Application to Arm v8-M

Here are a couple of options for how we could use this. Other ideas
welcome.

Option 1

With the following macros:

#define EXECUTION_LEVEL_SECURE (1U << 31)
#define EXECUTION_LEVEL_NONSECURE (0U << 31)

And the following example CPU cluster:

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

		/* Non-secure address map. */
		address-map =
			...
			<0x40000000 &peripherals 0x0 0x10000000>;
		/* Secure address map. */
		address-map-secure =
                        ...,
			<0x50000000 &peripherals 0x0 0x10000000>;

		cpu@0 {
			#address-cells = <1>;
			#size-cells = <1>;
			device_type = "cpu";
			compatible = "arm,cortex-m33f";
			reg = <0>;  /* ... */
		};
	};

We could define the following domains:

	domains {
		domain_cpu0 {
			compatible = "openamp,domain-v1";
			cpus = <&cpu0 0x1 EXECUTION_LEVEL_SECURE>;
                        /* ... */
                };

		domain_cpu0_ns {
			compatible = "openamp,domain-v1";
			cpus = <&cpu0 0x1 EXECUTION_LEVEL_NONSECURE>;
                        /* ... */
                };
        };

With the definition in the binding for "arm,cortex-m33f" CPUs that the
secure execution level should use the address-map-secure property,
and analogously for the non-secure execution level, we could allow the
generated DTS files for a given domain to automatically apply the
correct address map in this situation.

Option 2

We could be more explicit by changing the above as follows:

	cpu0: cpu0-cluster {
		/* Non-secure address map. */
		address-map-0 =
			...
			<0x40000000 &peripherals 0x0 0x10000000>;
		/* Secure address map. */
		address-map-1 =
                        ...,
			<0x50000000 &peripherals 0x0 0x10000000>;

                address-map-names = "non-secure", "secure";
	};

And then extend the domain binding so we could, for example, do
something like this:

	domains {
		domain_cpu0 {
			compatible = "openamp,domain-v1";
			cpus = <&cpu0 0x1 EXECUTION_LEVEL_SECURE>;
                        address-map-name = "secure";
                };
        };

with the same effect.

This is more redundant and introduces room for error, however.

A zero `cpu-mask` means "all of them"

As of 4fc81d4, the cpu-mask
property in the "openamp,domain-v1" binding says:

- *cpu-mask* is a bitfield indicating the subset of CPUs in the cluster which
  the domain runs on

I'd like to propose that we extend this binding to say that a value of
0 means that the domain runs on all of the CPUs in the cluster.

This seems like a common case and it is a useful shorthand which may
help users, especially since we could define a self-documenting macro
like DOMAIN_CPU_MASK_ALL for this.

`memory` and `sram` clarification for `#address-cells` and `#size-cells`

As of 4fc81d4, the memory
property in the "openamp,domain-v1" binding has the following in its
specification:

- *start* is the physical address of the start of the memory range. The
  number of cells used to represent the start address is determined by
  the *#address-cells* property.
- *size* is the size of the memory range, in bytes. The number of cells
  used to represent the size is determined by the *#size-cells*
  property.

There is a similar comment for sram.

In each case, the node that contains the #address-cells and
#size-cells properties is not specified. Which node is it?

Forward lopper options to assist

Hello,

I'm using lopper assist to create files. The output directory is set by calling the lopper and the filename is set by config.dts. In order to have consistent output directory I would like to forward the output argument from lopper to assist as it does for verbose option.
I currently implemented it on my local repo

Thank you

Nested indirect buses

Is nesting indirect buses allowed? It seems to introduce a lot of
complication if so, and my guess is that it should just be explicitly
forbidden in the specification, but I'd like to know.

For example:

/ {
	parent {
		compatible = "indirect-bus";
		child {
			compatible = "indirect-bus";
			peripheral@deadbeef { /* ... */ };
		};
	};
};

If this is allowed, what are the semantics for mapping in
peripheral@deadbeef above in the address-map property of a
"cpus,cluster" node?

  • If I map in parent, do I automatically get peripheral@deadbeef
    too even if I don't map child?

    • If so, via what address map translation through child?
    • If not, does that mean I can simply ignore any "indirect-bus"
      node, along with all of its children, that aren't mapped
      explicitly (assuming peripheral@deadbeef is not present in the
      address-map property)?
  • If I map in child but not parent, how can I generate an
    equivalent devicetree? Do I need to lift child to be under the
    root node or something like that?

specification: domain-specific access flags are undefined

This statement appears to be incorrect:

Access flags are domain specific and have default values defined in
the System Device Tree specification for each domain type. Different
domains types have different compatible strings, in addition to
"openamp,domain".

https://github.com/devicetree-org/lopper/blob/0da7cbd70bc2c3417e5e909bdd90eea2a0a0c912/specification/source/chapter3-execution-domains.rst

I don't see any place where these are defined.

It's also not clear what flags are being spoken about here:

  • device access flags in the access property of a domain?
  • memory access flags in the memory property?
  • sram access flags in sram property?

All three? None? Just some? Etc.

Lopper removes "reserved-memory" nodes because it's not referenced

Hello,
I'm not sure if there is a bug. But to me it seems lopper is stripping the reserved-memory node even if there are sub-nodes that are referenced. To me it feels like this commit 9005018 is introducing this problem. Before that the function resolve_all_refs() was correctly returning all the nodes and incremented their ref count by 1. After this commit the function returns just a very short list which to me looks like are the nodes from the domain file, containing only the following four entries:

/partition/linux_standalone
/partition
/
/cpus-cluster@0

So, all other nodes have a ref count of 0. This is not a problem for most of the nodes because if they are referenced their ref count will be incremented later. But the reserved-memory node is a special case, because only its sub-nodes are actually checked and their ref count incremented. But then in the process_domain() function also the reserved-memory node is checked and then directly removed because its ref count is 0. To fix this I added the following in the process_domain() function:

--- a/lopper/assists/openamp.py
+++ b/lopper/assists/openamp.py
@@ -295,6 +295,7 @@ def process_domain( tgt_node, sdt, options ):
                         # to a list of nodes that we'll use to check for referenced children later. Anything
                         # with no reference, will be removed.
                         anode.ref = 1
+                        node_parent.ref = 1
 
         # filter #1:
         #    - starting at /

Any idea on the matter?

Partial `reg` semantics for child nodes of indirect buses

This is similar to #170, except it's a situation where a
node has a reg property, but critical information is missing.

As of 4fc81d4, it's not clear what
happens if I map in a node via an address-map property which has one
of #address-cells or #size-cells set to zero in the parent node.
In this case, the node's register blocks are missing information that
we need to decide if the node is visible to the CPU cluster containing
the address-map property or not.

What do we do here?

  • always visible?
  • never visible?
  • visible if the base address is visible when #size-cells is 0?

I guess a zero #address-cells but a nonzero #size-cells is not
practical, but I also don't know that it's explicitly forbidden by
DTSpec.

Incorrect usage of unit addresses in various nodes

From the devicetree specification, section 2.2.2 Node Names:

The
unit-address must match the first
address specified in the reg property of the node. If the node has no
reg property, the @unit-address must be omitted and the
node-name alone differentiates the node from other nodes at the same
level in the tree.

From this we can also infer a derived requirement that any node with a unit address must have a parent node with #address-cells set, and the reg property must be consistent with it.

The system devicetree specification and associated examples violates this requirement in several places:

  1. system-device-tree.dts

https://github.com/devicetree-org/lopper/blob/da29be7393b1b3237c3a566b42a5dd5f81cf391c/specification/system-device-tree.dts

This has a root node with #address-cells = <2>, so the indirect-bus and cpus,cluster child nodes should either 1. not have unit addresses or 2. have reg properties with 2 cells for the address. This is also true of multiple simple-bus nodes which are children of the root node.

  1. specification.md

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

Same issue: various nodes have unit addresses but no reg property. Additionally, the context makes it unclear whether the parent node has #address-cells set.

Use lopper to extract partial device trees from a given device tree

A user should be able to provide lopper with a device node and a device tree (containing the node). Lopper should be able to generate a device tree which will contain the node and the sub-nodes (for eg clocks, power) used by the node.

As an illustration :-
python3 lopper.py -i <path_to_device_tree> -e node@addr -o <generated_device_tree>

Further on, it should be possible for the user to exclude certain properties in the generated device tree.
As an illustration :-
python3 lopper.py -i <path_to_device_tree> -e serial@ff000000 -r "power-domains;current-speed" -o <generated_device_tree>

So the new device tree will contain serial@ff000000 and all the nodes referenced by it (for eg clocks) except for the properties which are to be excluded.

How to use lopper to process multiple dtbs(with different compatibility)?

Hello dear Lopper community, I want to try out lopper recently and have come across a problem.

I have some dtb files that are used on various boards, those dtb have different compatible and nodes, looks like:

// board-1.dts
/dts-v1/;
/ {
	compatible = "type1";

	...

	serial {
		...
	};
	...

   };
// board-2.dts
/dts-v1/;
/ {
	compatible = "type2";

	...

	serial {
		...
	};
	...

   };

I want to add different lops for them.

lop1.dts for board-1.dts, modify board-1.dts and extract some nodes to output-1.dtb.

lop2.dts for board-2.dts, modify board-2.dts and extract some nodes to output-2.dtb.

I'm going to handle them through a script, and generate the corresponding output-x.dtb.

But if lop1.dts is used for board-2.dts, it may generate the wrong output1.dtb.

How can I do a "compatible" check in lop1.dts? If compatible is not type1, is it possible not to execute the lopper operations that are defined in lops1.dts?

Thanks a lot!

Support for OpenAMP YAML Overlays has regressed

Unable to use OpenAMP

I am using lopper-demo inputs and this is failing

bash script below:

LOPPER=lopper
OVERLAY=opper/demos/openamp/inputs/openamp-overlay-zynqmp.yaml
LOPS_DIR=${LOPPER}/lopper/lops
SDT=lopper/demos/openamp/inputs/system-dt/system-top.dts

python3 ${LOPPER}/lopper/main.py -f --enhanced --permissive
-i ${OVERLAY}
-i ${LOPS_DIR}/lop-load.dts
-i ${LOPS_DIR}/lop-xlate-yaml.dts
-i ${LOPS_DIR}/lop-openamp-versal.dts
-O .
${SDT}
openamp_output.dts

The domains YAML is not applied properly.
error output:
[WARNING]: assist <function openamp_parse at 0x7f302b1f9430> failed: '/domains'
<class 'KeyError'> init.py 1442

CC @zeddii

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

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.

Address mapping formulas

As of 4fc81d4, the specification
semantics for address-map properties is still not 100% clear to me
when I try to implement it.

I think it would be really good for the specification to include some
formulas or pseudo-code that says the exact way to translate a node's
register blocks to the CPU cluster address space, including a formula
for deciding whether a node is visible to the address map based on
register range overlaps.

This relates to the issue about whether we need a rule requiring
explicit address map translation via ranges properties for any nodes
we want to map in that aren't a child of the root.

YAML support cont'd

Hi Bruce,

The following are some open items for the YAML support:

  • sample YAML inputs with: domains, resource groups
  • documentation for how the YAML should describe "custom" properties and not be put into JSON

Thanks
Ben

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.