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

scripts: extract_dts_includes: fails on multiple includes in yaml bindings file #7067

Closed
b0661 opened this issue Apr 13, 2018 · 5 comments · Fixed by #7548
Closed

scripts: extract_dts_includes: fails on multiple includes in yaml bindings file #7067

b0661 opened this issue Apr 13, 2018 · 5 comments · Fixed by #7548
Assignees
Labels
area: Devicetree bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug

Comments

@b0661
Copy link
Collaborator

b0661 commented Apr 13, 2018

Inheriting from multiple bindings fails.

inherits:
    !include [binding1.yaml, binding2.yaml]

Following the above the 'node_type' node property does not account for multiple base types.

Loader.include for yaml.SequenceNode and yaml_traverse_inherited seems to be broken.

@MaureenHelm MaureenHelm added bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug area: Devicetree labels Apr 13, 2018
@galak
Copy link
Collaborator

galak commented Apr 18, 2018

@erwango can you look into this issue.

@galak galak closed this as completed Apr 18, 2018
@galak galak reopened this Apr 18, 2018
@erwango
Copy link
Member

erwango commented Apr 18, 2018

@b0661 : In order to solve this is a coherent way, do you have any example of use for multiple bindings inclusion?

@b0661 b0661 closed this as completed Apr 18, 2018
@b0661
Copy link
Collaborator Author

b0661 commented Apr 18, 2018

I tried to model STM32 RCC by combining a clock-provider.yaml and clock-consumer.yaml binding. The node type of the RCC would be both in this case. My suggestion is to make the 'node_type' node property a list. I have a working example available. Sorry no pull request yet.

diff --git a/scripts/dts/extract_dts_includes.py b/scripts/dts/extract_dts_includes.py
index 12a3b6ab6..d966ce231 100755
--- a/scripts/dts/extract_dts_includes.py
+++ b/scripts/dts/extract_dts_includes.py
@@ -54,7 +54,7 @@ class Loader(yaml.Loader):
         elif isinstance(node, yaml.SequenceNode):
             result = []
             for filename in self.construct_sequence(node):
-                result += self.extractFile(filename)
+                result.append(self.extractFile(filename))
             return result
 
         elif isinstance(node, yaml.MappingNode):
@@ -450,15 +450,24 @@ def yaml_traverse_inherited(node):
     :return: node
     """
 
+    if 'node_type' not in node.keys():
+        node['node_type'] = []
     if 'inherits' in node.keys():
-        if 'id' in node['inherits'].keys():
-            node['inherits']['node_type'] = node['inherits']['id']
-            node['inherits'].pop('id')
-        if 'inherits' in node['inherits'].keys():
-            node['inherits'] = yaml_traverse_inherited(node['inherits'])
-        dict_merge(node['inherits'], node)
-        node = node['inherits']
+        if isinstance(node['inherits'], list):
+            inherits_list  = node['inherits']
+        else:
+            inherits_list  = [node['inherits'],]
         node.pop('inherits')
+        for inherits in inherits_list:
+            if 'id' in inherits.keys():
+                node['node_type'].append(inherits['id'])
+                inherits.pop('id')
+            if 'inherits' in inherits.keys():
+                inherits = yaml_traverse_inherited(inherits)
+                if 'node_type' in inherits.keys():
+                    node['node_type'].extend(inherits['node_type'])
+            dict_merge(inherits, node)
+            node = inherits
     return node

Clock consumer

# -- Assigned clock parents and rates --
# Some platforms may require initial configuration of default parent clocks
# and clock frequencies. Such a configuration can be specified in a device tree
# node through assigned-clocks, assigned-clock-parents and assigned-clock-rates
# properties.
---
title: Clock Consumer Base Structure
id: clock-consumer
version: 0.1

description: >
    This binding gives the base structures for all clock consumers.

properties:

    clocks:
      type: array
      category: optional
      description: >
        List of phandle and clock specifier pairs, one pair for each clock
        input to the device. Note - if the clock provider specifies '0' for
        clock-cells, then only the phandle portion of the pair will appear.
      generation: define

    clock-names:
      type: array
      category: optional
      description: >
        List of clock input name strings sorted in the same order as the clocks property.
      generation: define

    clock-ranges:
      type: empty
      category: optional
      description: >
        Empty property indicating that child nodes can inherit named clocks from
        this node. Useful for bus nodes to provide a clock to their children.
      generation: define

    assigned-clocks:
      type: array
      category: optional
      description: >
        List of phandle and clock specifier pairs, one pair for each assigned
        clock input. Note - if the clock provider specifies '0' for
        clock-cells, then only the phandle portion of the pair will appear.
      generation: define

    assigned-clock-parents:
      type: array
      category: optional
      description: >
        List of parent clocks in the form of a phandle and clock
        specifier pair. The list shall correspond to the clocks listed in the
        assigned-clocks directive.
      generation: define

    assigned-clock-rates:
      type: array
      category: optional
      description: >
        List of frequencies in Hz. The list shall correspond to the clocks
        listed in the assigned-clocks directive.
      generation: define
...

Clock provider

---
title: Clock Provider Base Structure
id: clock-provider
version: 0.1

description: >
    This binding gives the base structures for all clock providers.

properties:

    "#clock-cells":
      type: int
      category: optional
      description: >
        If this device will also be providing controllable clocks, the
        clock_cells property needs to be specified.

    clock-output-names:
      type: array
      category: optional
      description: >
        A list of strings of clock output signal names indexed by the first
        cell in the clock specifier.
      generation: define

    clock-indices:
      type: array
      category: optional
      description: >
        The identifying number for the clocks in the node. If it is not linear
        from zero, then this allows the mapping of identifiers into the
        clock-output-names array.
      generation: define
...

@b0661 b0661 reopened this Apr 18, 2018
@erwango
Copy link
Member

erwango commented May 3, 2018

@b0661 : Is that ok if we set this ticket as 'enhancement' instead of 'bug'? (Medium priority 'bug' issues are blocking for upcoming release)
This is blocking further advanced development but not breaking anything currently.

@b0661
Copy link
Collaborator Author

b0661 commented May 3, 2018

@erwango

Is that ok if we set this ticket as 'enhancement' instead of 'bug'?

Yes, shure it is ok to set it to 'enhancement'.

@erwango erwango added Enhancement Changes/Updates/Additions to existing features and removed bug The issue is a bug, or the PR is fixing a bug labels May 4, 2018
@nashif nashif added area: Debugging bug The issue is a bug, or the PR is fixing a bug and removed Enhancement Changes/Updates/Additions to existing features area: Debugging labels May 18, 2018
b0661 added a commit to b0661/zephyr that referenced this issue May 22, 2018
Correctly process multiple include files given to the
!include command of the YAML loader.

The fix only targets the sequential definition of include files.

Fixes zephyrproject-rtos#7067

Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
galak pushed a commit that referenced this issue May 25, 2018
Correctly process multiple include files given to the
!include command of the YAML loader.

The fix only targets the sequential definition of include files.

Fixes #7067

Signed-off-by: Bobby Noelte <b0661n0e17e@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree bug The issue is a bug, or the PR is fixing a bug priority: medium Medium impact/importance bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants