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

[WIP] scripts: extract_dts_includes.py: enhance #6761

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cmake/dts.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
# See ~/zephyr/doc/dts
set(GENERATED_DTS_BOARD_H ${PROJECT_BINARY_DIR}/include/generated/generated_dts_board.h)
set(GENERATED_DTS_BOARD_CONF ${PROJECT_BINARY_DIR}/include/generated/generated_dts_board.conf)
set(GENERATED_DTS_BOARD_EDTS ${PROJECT_BINARY_DIR}/include/generated/generated_dts_board.json)
set_ifndef(DTS_SOURCE ${BOARD_ROOT}/boards/${ARCH}/${BOARD_FAMILY}/${BOARD}.dts)
set_ifndef(DTS_COMMON_OVERLAYS ${ZEPHYR_BASE}/dts/common/common.dts)

Expand Down Expand Up @@ -108,6 +109,7 @@ if(CONFIG_HAS_DTS)
${DTS_FIXUPS}
--keyvalue ${GENERATED_DTS_BOARD_CONF}
--include ${GENERATED_DTS_BOARD_H}
--edts ${GENERATED_DTS_BOARD_EDTS}
)

# Run extract_dts_includes.py to create a .conf and a header file that can be
Expand Down
193 changes: 193 additions & 0 deletions scripts/dts/edtsdatabase.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,193 @@
#
# Copyright (c) 2018 Bobby Noelte
#
# SPDX-License-Identifier: Apache-2.0
#

from collections.abc import Mapping
import json

##
# @brief Extended DTS database
#
# Database schema:
#
# _edts dict(
# 'devices': dict(device-id : device-struct),
# 'compatibles': dict(compatible : sorted list(device-id)),
# 'device-types': dict(device-type : sorted list(device-id)),
# ...
# )
#
# device-struct: dict(
# 'device-id' : device-id,
# 'device-type' : list(device-type) or device-type,
# 'compatible' : list(compatible) or compatible,
# 'label' : label,
# property-name : property-value ...
# )
#
# Database types:
#
# device-id: <compatible>_<base address in hex>_<offset in hex>,
# compatible: any of ['st,stm32-spi-fifo', ...] - 'compatibe' from <binding>.yaml
# device-type: any of ['GPIO', 'SPI', 'CAN', ...] - 'id' from <binding>.yaml
# label: any of ['UART_0', 'SPI_11', ...] - label directive from DTS
#
class EDTSDatabase(Mapping):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, it would be nice to split between producer and consumer functions.

Copy link
Collaborator Author

@b0661 b0661 Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For clarity, it would be nice to split between producer and consumer functions.

I tried to by the function names:

  • Producer
    • insert_device_property
    • save
  • Consumer
    • device_property
    • compatible_devices
    • compatible_devices_id
    • load

I´m open to better method names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So a clear split in the file would be good. get_... for consumer could be nice as well
What about sub classes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about sub classes?

OK will go for mixin classes like codegen uses to separate the different functions.


def __init__(self, *args, **kw):
self._edts = dict(*args, **kw)
# setup basic database schema
for edts_key in ('devices', 'compatibles', 'device-types'):
if not edts_key in self._edts:
self._edts[edts_key] = dict()

def __getitem__(self, key):
return self._edts[key]

def __iter__(self):
return iter(self._edts)

def __len__(self):
return len(self._edts)

def convert_string_to_label(self, s):
# Transmute ,-@/ to _
s = s.replace("-", "_")
s = s.replace(",", "_")
s = s.replace("@", "_")
s = s.replace("/", "_")
# Uppercase the string
s = s.upper()
return s

def device_id(self, compatible, base_address, offset):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is 'offset'?

I think the function should take node_address and compatible into account.
node_address should look like: /soc/i2c@40005800/hts221@5f
From this is should be possible to build device_id: <compatible>_40005800_5F

And while I don't have any example yet, I think there could be more address level than that, so it should be taken into account upfront: <compatible>_<addr_1>_<addr_2>_<...>_<addr_n>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took offset only as an assumption from the string you provided as @galak ´s request. You may also call it index, ... Maybe @galak can explain what he would like device id to be build from and look like.

I don´t think it is important how device id will be generated as long as it is unique. My first proposal was to just use the 'label' property as this is unique for all activated devices.

In my view device_id is just a unique reference to a device struct. It should not be used for other purposes (like generating define labels from it).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let @galak provide more information here

# sanitize
base_address = int(base_address)
offset = int(offset)
compatibe = self.convert_string_to_label(compatible.strip())
return "{}_{:08X}_{:02X}".format(compatible, base_address, offset)

def device_id_by_label(self, label):
for device in self._edts['devices']:
if label == device['label']:
return device['device-id']

def _update_device_compatible(self, device_id, compatible):
if compatible not in self._edts['compatibles']:
self._edts['compatibles'][compatible] = list()
if device_id not in self._edts['compatibles'][compatible]:
self._edts['compatibles'][compatible].append(device_id)
self._edts['compatibles'][compatible].sort()

def _update_device_type(self, device_id, device_type):
if device_type not in self._edts['device-types']:
self._edts['device-types'][device_type] = list()
if device_id not in self._edts['device-types'][device_type]:
self._edts['device-types'][device_type].append(device_id)
self._edts['device-types'][device_type].sort()

##
# @brief Insert property value for the device of the given device id.
#
# @param device_id
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using device_id will impose to generate device id each time before calling insert_device_property.
It would be easier to set node_address + compat* in the arguments and then compute device_id inside the function

*: compatible could be computed from node_address, but requires to know 'reduced'

Copy link
Collaborator Author

@b0661 b0661 Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using device_id will impose to generate device id each time before calling insert_device_property.

  • On filling the EDTS database the device_id should be be generated at the beginning of extract_ and then re-used. reduced for 'compatible' is available here anyway.
def extract_<whatever>(node_address, ...)
    node = ....
    device_id = edts.device_id(node[props][...], ...)

There may also be a helper function in extract_dts_includes:

def extract_device_id(node_address):
    # extract necessary values from reduced
   return etds.device_id(compatible, base_address, offset)
  • On using the ETDS database the device id will be queried. Reduced is not necessary (and not available).
for device_id in edts.compatible_devices_id(compatible):
    # do something, eg
    label = edts.device_property(device_id, 'label')

It would be easier to set node_address + compat* in the arguments and then compute device_id inside the function

This can not be part of EDTSDatabase as this requires reduced to be available which breaks data encapsulation. There can well be a function in extract_dts_includes that provides this, e.g.

def extract_edts_device_property(node_address, property_path, property_value):
    # extract necessary values from reduced
    device_id = etds.device_id(compatible, base_address, offset)
    etds.insert_device_property(device_id, property_path, property_value)

# @param property_path Path of the property to access
# (e.g. 'reg/0', 'interrupts/prio', 'label', ...)
# @param property_value value
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way it works today is that we enter a top level property and a dict associated.
For instance:
'deflabel: interrupts'
("new_structs: [{'data': [33, 0], 'labels': ['irq', 'priority']}, {'data': "
"[34, 0], 'labels': ['irq', 'priority']}]")
''
Would be easier to stick to that in a first iteration

Copy link
Collaborator Author

@b0661 b0661 Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would become:

device_id = edts.device_id(compatible, ....) # or other helper function - TBD
edts.device_insert_property(device_id, 'interrupts', [{'data': [33, 0], 'labels': ['irq', 'priority']}, {'data': "
"[34, 0], 'labels': ['irq', 'priority']}])

#
def insert_device_property(self, device_id, property_path, property_value):
# special properties
if property_path == 'compatible':
self._update_device_compatible(device_id, property_value)
elif property_path == 'device-type':
self._update_device_type(device_id, property_value)

# normal property management
if device_id not in self._edts['devices']:
self._edts['devices'][device_id] = dict()
self._edts['devices'][device_id]['device-id'] = device_id
if property_path == 'device-id':
# already set
return
keys = property_path.strip("'").split('/')
property_access_point = self._edts['devices'][device_id]
for i in range(0, len(keys)):
if i < len(keys) - 1:
# there are remaining keys
if keys[i] not in property_access_point:
property_access_point[keys[i]] = dict()
property_access_point = property_access_point[key[i]]
else:
# we have to set the property value
if keys[i] in property_access_point:
# There is already a value set
current_value = property_access_point[keys[i]]
if not isinstance(current_value, list):
current_value = [current_value, ]
if isinstance(property_value, list):
property_value = current_value.extend(property_value)
else:
property_value = current_value.append(property_value)
property_access_point[keys[i]] = property_value

##
#
# @return list of devices that are compatible
def compatible_devices(self, compatible):
devices = list()
for device_id in self._edts['compatibles'][compatible]:
devices.append(self._edts['devices'][device_id])
return devices

##
#
# @return list of device ids of activated devices that are compatible
def compatible_devices_id(self, compatible):
return self._edts['compatibles'].get(compatible, [])

##
# @brief Get device tree property value for the device of the given device id.
#
#
# @param device_id
# @param property_path Path of the property to access
# (e.g. 'reg/0', 'interrupts/prio', 'device_id', ...)
# @return property value
#
def device_property(self, device_id, property_path, default="<unset>"):
property_value = self._edts['devices'][device_id]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be updated vs the latest version.
(And will likely require a more efficient (and iterative) parsing function)

for key in property_path.strip("'").split('/'):
if isinstance(property_value, list):
# TODO take into account prop with more than 1 elements of cell_size > 1
if isinstance(property_value[0], dict):
property_value = property_value[0]
try:
property_value = property_value[str(key)]
except TypeError:
property_value = property_value[int(key)]
except KeyError:
# we should have a dict
if isinstance(property_value, dict):
# look for key in labels
for x in range(0, len(property_value['labels'])):
if property_value['labels'][x] == key:
property_value = property_value['data'][x]
break
else:
return "Dict was expected here"
if property_value is None:
if default == "<unset>":
default = \
"Device tree property {} not available in {}[{}]".format(property_path, compatible, device_index)
return default
return property_value

def load(self, file_path):
with open(file_path, "r") as load_file:
self._edts = json.load(load_file)

def save(self, file_path):
with open(file_path, "w") as save_file:
json.dump(self._edts, save_file, indent = 4)

3 changes: 3 additions & 0 deletions scripts/dts/extract/globals.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,16 @@
# SPDX-License-Identifier: Apache-2.0
#

import sys
from collections import defaultdict
import edtsdatabase

# globals
phandles = {}
aliases = defaultdict(list)
chosen = {}
reduced = {}
edts = edtsdatabase.EDTSDatabase()

regs_config = {
'zephyr,flash' : 'CONFIG_FLASH',
Expand Down
3 changes: 3 additions & 0 deletions scripts/dts/extract_dts_includes.py
Original file line number Diff line number Diff line change
Expand Up @@ -740,6 +740,8 @@ def parse_arguments():
parser.add_argument("-d", "--dts", nargs=1, required=True, help="DTS file")
parser.add_argument("-y", "--yaml", nargs=1, required=True,
help="YAML file")
parser.add_argument("-e", "--edts", nargs=1, required=True,
help="Generate EDTS database file for the build system")
parser.add_argument("-f", "--fixup", nargs='+',
help="Fixup file(s), we allow multiple")
parser.add_argument("-i", "--include", nargs=1, required=True,
Expand Down Expand Up @@ -768,6 +770,7 @@ def main():
generate_keyvalue_file(defs, args.keyvalue[0])

generate_include_file(defs, args.include[0], args.fixup)
edts.save(args.edts[0])


if __name__ == '__main__':
Expand Down