Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DT: accept standard syntax for phandle in chosen node #21623

Closed
b0661 opened this issue Dec 30, 2019 · 6 comments
Closed

DT: accept standard syntax for phandle in chosen node #21623

b0661 opened this issue Dec 30, 2019 · 6 comments
Assignees
Labels
area: Devicetree Enhancement Changes/Updates/Additions to existing features

Comments

@b0661
Copy link
Collaborator

b0661 commented Dec 30, 2019

Is your enhancement proposal related to a problem? Please describe.

I am using a modified edtlib to preprocess the DT for code generation. The modified variant can attach a binding to the chosen node. This is mostly for error detection but also to create some chosen properties for OOT usage.

The edtlib normal node handling, which is enforced by using the binding for chosen, expects standard syntax for a phandle.

	chosen {
		zephyr,console = <&uart1>;
	};

Zephyr edtlib/dtlib expects a special syntax in the chosen node (which IMO is not in line with the device tree specification):

	chosen {
		zephyr,console = &uart1;
	};

The Zephyr build process uses the unmodified edtlib and bails out while in kconfig:

dtlib.DTError: expected property 'zephyr,console' on /chosen in ..._build/zephyr/refboard.dts.pre.tmp to be assigned with either 'zephyr,console = &foo' or 'zephyr,console = "/path/to/node"', not 'zephyr,console = < &uart1 >;'

Describe the solution you'd like

Make the DT chosen node accept standard syntax for phandle.

Ideally treat the chosen node as a normal node with an (automagically) attached binding. Change the device tree to use standard syntax in chosen nodes. This allows all types of values in the chosen node; not only phandles as it is the case currently.

Additional context

Modifications to edtlib that allows the /chosen node to be handled as a normal node with a compatible 'chosen' binding:

In Node class:

def _init_binding(self):
    ...

    if "compatible" in self._node.props:
        self.compats = self._node.props["compatible"].to_strings()
    elif self._node.name == "chosen":
        # use default compatible for chosen node
        self.compats = ["chosen"]
    else:
        self.compats = []

    if self.compats:
        on_bus = self.on_bus

        for compat in self.compats:
            if (compat, on_bus) in self.edt._compat2binding:
                # Binding found
                self.matching_compat = compat
                self._binding, self.binding_path = \
                    self.edt._compat2binding[compat, on_bus]
                return
    else:
        # No 'compatible' property. See if the parent binding has a
        # 'child-binding:' key that gives the binding (or a legacy
        # 'sub-node:' key).
        binding_from_parent = self._binding_from_parent()
        if binding_from_parent:
            self._binding = binding_from_parent
            self.binding_path = self.parent.binding_path
            self.matching_compat = self.parent.matching_compat
            print(self.matching_compat, self.binding_path)
            return

    # No binding found
    self._binding = self.binding_path = self.matching_compat = None

In edtlib.py on global scope:

def _dt_compats(dt):
    # Returns a set() with all 'compatible' strings in the devicetree
    # represented by dt (a dtlib.DT instance)

    compats = [compat
                for node in dt.node_iter()
                    if "compatible" in node.props
                        for compat in node.props["compatible"].to_strings()]
    compats.append('chosen')
    return {*compats}

The 'chosen' binding:

description: Chosen properties

compatible: "chosen"

properties:
    stdout-path:
        type: string
        required: false
        description: >
          Device to be used for boot console output
          If the character ":" is present in the value, this terminates the path.
          The meaning of any characters following the ":" is device-specific, and
          must be specified in the relevant binding documentation.
          For UART devices, the preferred binding is a string in the form:
            path:<baud>{<parity>{<bits>{<flow>}}}
          where
            path    - path of uart device
            baud	- baud rate in decimal
            parity	- 'n' (none), 'o', (odd) or 'e' (even)
            bits	- number of data bits
            flow	- 'r' (rts)
          For example: 115200n8r

    zephyr,flash:
        type: phandle
        required: false
        description: TBD

    zephyr,sram:
        type: phandle
        required: false
        description: TBD

    zephyr,ccm:
        type: phandle
        required: false
        description: TBD

    zephyr,console:
        type: phandle
        required: false
        description: TBD

    zephyr,shell-uart:
        type: phandle
        required: false
        description: TBD

    zephyr,bt-uart:
        type: phandle
        required: false
        description: TBD

    zephyr,uart-pipe:
        type: phandle
        required: false
        description: TBD

    zephyr,bt-mon-uart:
        type: phandle
        required: false
        description: TBD

    zephyr,uart-mcumgr:
        type: phandle
        required: false
        description: TBD
@b0661 b0661 added the Enhancement Changes/Updates/Additions to existing features label Dec 30, 2019
@ulfalizer
Copy link
Collaborator

ulfalizer commented Dec 30, 2019

x = &foo is a path reference, and expands to x = "/path/to/node/with/label/foo", while x = <&foo> is a phandle reference, and expands to e.g. x = <5>.

There's no special handling for properties in chosen. It's just that chosen normally contains path references instead of phandle references.

Seems to be used with path references in Linux too. See e.g. arch/arm/boot/dts/am335x-guardian.dts:

chosen {
	stdout-path = &uart0;
	tick-timer = &timer2;
};

@ulfalizer
Copy link
Collaborator

ulfalizer commented Dec 30, 2019

There's no type: for path references at the moment though, like type: path. Maybe that could be added.

On the dtlib end, there's Property.to_path(), but the only thing in edtlib that uses it at the moment is the EDT.chosen_node() helper.

@ulfalizer
Copy link
Collaborator

Added support for a path type that might be useful: #21625

@b0661
Copy link
Collaborator Author

b0661 commented Dec 31, 2019

@ulfalizer, thank you for the fast response.

I applied #21625 to Zephyr and also use the changed edtlib.py for code generation. It works, thanks!

I now use the path type in the 'chosen' binding:

description: Chosen properties

compatible: "chosen"

properties:
    stdout-path:
        type: string
        required: false
        description: ...

    zephyr,flash:
        type: path
        required: false
        description: TBD

    ...

Out of curiosity I tested the following chosen node:

	chosen {
		stdout-path = "/soc/uart1:115200n8r";
		zephyr,console = &uart1;
	};

I works too. I assume it only works because the EDT.chosen_node() helper is never called for "stdout-path", which is okay for me because the non Zephyr properties in the chosen node are only used in code generation.

@ulfalizer
Copy link
Collaborator

Out of curiosity I tested the following chosen node:

	chosen {
		stdout-path = "/soc/uart1:115200n8r";
		zephyr,console = &uart1;
	};

I works too. I assume it only works because the EDT.chosen_node() helper is never called for "stdout-path", which is okay for me because the non Zephyr properties in the chosen node are only used in code generation.

Yup, the type is only checked if Property.to_path() in dtlib gets called (e.g. by EDT.chosen_node()).

ulfalizer added a commit to ulfalizer/zephyr that referenced this issue Jan 2, 2020
Add binding support for a 'path' property type, for properties that are
assigned node paths. Usually, paths are assigned with path references
like 'foo = &label' (common in /chosen), but plain strings are accepted
as well as long as they're valid paths.

The 'path' type is mostly for completeness at this point, but might be
useful for zephyrproject-rtos#21623.
The support is there already in dtlib.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
galak pushed a commit that referenced this issue Jan 8, 2020
Add binding support for a 'path' property type, for properties that are
assigned node paths. Usually, paths are assigned with path references
like 'foo = &label' (common in /chosen), but plain strings are accepted
as well as long as they're valid paths.

The 'path' type is mostly for completeness at this point, but might be
useful for #21623.
The support is there already in dtlib.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
@b0661
Copy link
Collaborator Author

b0661 commented Jan 9, 2020

#21625 works for me. Will go on with the idea to attach a default binding to the chosen node.

@b0661 b0661 closed this as completed Jan 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

3 participants