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.

Sign up for GitHub

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

Add dts yaml support for same sensor on i2c or spi #11854

Merged
merged 8 commits into from
Dec 7, 2018
Merged

Add dts yaml support for same sensor on i2c or spi #11854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
de4cffe
scripts: extract_dts_includes: Handle YAML inheritance when we load
galak Dec 4, 2018
2524232
scripts: extract_dts_includes: remove passing yaml param to functions
galak Dec 4, 2018
7a55bb2
scripts: extract_dts_includes: Add accessor function to access binding
galak Dec 4, 2018
ad40a6c
scripts: extract_dts_includes: Pass node address to get_binding
galak Dec 5, 2018
126a64f
scripts: extract_dts_includes: Split out yaml_list info
galak Dec 5, 2018
b780774
scripts: extract_dts_includes: Support same sensor on different bus
galak Dec 5, 2018
4d08522
scripts: extract_dts_includes: Generate device bus info define
galak Dec 7, 2018
867c33e
sensor: lsm6dsl: Update DTS support
galak Dec 4, 2018
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: 1 addition & 1 deletion boards/arm/96b_argonkey/96b_argonkey.dts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@

/* ST Microelectronics LSM6DSL accel/gyro sensor */
lsm6dsl@1 {
compatible = "st,lsm6dsl-spi";
compatible = "st,lsm6dsl";
reg = <1>;
spi-max-frequency = <1000000>;
Copy link
Member

Choose a reason for hiding this comment

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

IIUC, this whole change allows to have these bus related properties documented by the binding depending on the bus. Is that compatible with device tree usage in eg Linux?

I'm still not convinced how this would work at driver level. To me we should be able to build matching sensor driver using "compatible".
When "compatible" carried SPI/I2C inforamtion, we could have used #idef DT_COMPAT_ST_LSM6DL_SPI / DT_COMPAT_ST_LSM6DL_I2C to know if driver should be compiled with SPI or I2C fields.
Now this information is no more present we'll need to have a new piece of info to know how to build the driver with fields matching the bus.

Copy link
Collaborator

@avisconti avisconti Dec 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Actually in both cases the driver (e.g. LSM6DSL in this case) needs an extra configuration
parameter, say LSM6DSL_BUS_TYPE, to select spi/i2c bus case. The changes introduced
by @galak are somehow a cleaner solution, but what we would need instead is a way to
extract the bus type from the dts w/o the need for such configuration parameter.

Copy link
Member

@erwango erwango Dec 5, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

but then it should be available as a compilation input, not as a configuration input.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The bus type can be inferred by looking at the parent node, and the tooling generate the appropriate flags. The BME280 supports SPI and I2C, and the Linux binding documentation doesn't represent which is being used: it just provides two drivers, each of which are compatible with bosch,bme280.

The inference would be simplified by marking all I2C bus nodes as compatible with zephyr,i2c and similarly for other generic Zephyr drivers. This is something I'd like done regardless.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IIUC, this whole change allows to have these bus related properties documented by the binding depending on the bus. Is that compatible with device tree usage in eg Linux?

Yes, if you look at how various sensor drivers in linux you'll see they use the same compatible regardless of what bus the device is on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The inference would be simplified by marking all I2C bus nodes as compatible with zephyr,i2c and similarly for other generic Zephyr drivers. This is something I'd like done regardless.

I don't see us ever doing this, as we should be aiming for the device tree's to have less zephyr specific details not more. The intent would be to know that a bus is I2C or SPI via the binding YAML info.

irq-gpios = <&gpiob 1 0>;
Expand Down
12 changes: 6 additions & 6 deletions boards/arm/96b_argonkey/dts_fixup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@
#define DT_VL53L0X_I2C_ADDR DT_ST_STM32_I2C_V1_40005800_ST_VL53L0X_29_BASE_ADDRESS
#define DT_VL53L0X_I2C_MASTER_DEV_NAME DT_ST_STM32_I2C_V1_40005800_ST_VL53L0X_29_BUS_NAME

#define DT_LSM6DSL_DEV_NAME DT_ST_STM32_SPI_40003800_ST_LSM6DSL_SPI_1_LABEL
#define DT_LSM6DSL_SPI_SELECT_SLAVE DT_ST_STM32_SPI_40003800_ST_LSM6DSL_SPI_1_BASE_ADDRESS
#define DT_LSM6DSL_SPI_MASTER_DEV_NAME DT_ST_STM32_SPI_40003800_ST_LSM6DSL_SPI_1_BUS_NAME
#define DT_LSM6DSL_SPI_BUS_FREQ DT_ST_STM32_SPI_40003800_ST_LSM6DSL_SPI_1_SPI_MAX_FREQUENCY
#define DT_LSM6DSL_GPIO_DEV_NAME DT_ST_STM32_SPI_40003800_ST_LSM6DSL_SPI_1_IRQ_GPIOS_CONTROLLER
#define DT_LSM6DSL_GPIO_PIN_NUM DT_ST_STM32_SPI_40003800_ST_LSM6DSL_SPI_1_IRQ_GPIOS_PIN
#define DT_LSM6DSL_DEV_NAME DT_ST_STM32_SPI_40003800_ST_LSM6DSL_1_LABEL
#define DT_LSM6DSL_SPI_SELECT_SLAVE DT_ST_STM32_SPI_40003800_ST_LSM6DSL_1_BASE_ADDRESS
#define DT_LSM6DSL_SPI_MASTER_DEV_NAME DT_ST_STM32_SPI_40003800_ST_LSM6DSL_1_BUS_NAME
#define DT_LSM6DSL_SPI_BUS_FREQ DT_ST_STM32_SPI_40003800_ST_LSM6DSL_1_SPI_MAX_FREQUENCY
#define DT_LSM6DSL_GPIO_DEV_NAME DT_ST_STM32_SPI_40003800_ST_LSM6DSL_1_IRQ_GPIOS_CONTROLLER
#define DT_LSM6DSL_GPIO_PIN_NUM DT_ST_STM32_SPI_40003800_ST_LSM6DSL_1_IRQ_GPIOS_PIN

#define CONFIG_LP3943_DEV_NAME DT_ST_STM32_I2C_V1_40005C00_TI_LP3943_60_LABEL
#define CONFIG_LP3943_I2C_ADDRESS DT_ST_STM32_I2C_V1_40005C00_TI_LP3943_60_BASE_ADDRESS
Expand Down
0 dts/bindings/sensor/st,lsm6dsl.yaml → dts/bindings/sensor/st,lsm6dsl-i2c.yaml
View file
Open in desktop
File renamed without changes.
2 changes: 1 addition & 1 deletion dts/bindings/sensor/st,lsm6dsl-spi.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ inherits:

properties:
compatible:
constraint: "st,lsm6dsl-spi"
constraint: "st,lsm6dsl"

irq-gpios:
type: compound
Expand Down
17 changes: 6 additions & 11 deletions scripts/dts/extract/clocks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@ class DTClocks(DTDirective):
def __init__(self):
pass

def _extract_consumer(self, node_address, yaml, clocks, def_label):
def _extract_consumer(self, node_address, clocks, def_label):

clock_consumer = reduced[node_address]
clock_consumer_compat = get_compat(node_address)
clock_consumer_bindings = yaml[clock_consumer_compat]
clock_consumer_bindings = get_binding(node_address)
clock_consumer_label = 'DT_' + get_node_label(node_address)

clock_index = 0
Expand All @@ -42,8 +41,8 @@ def _extract_consumer(self, node_address, yaml, clocks, def_label):
str(clock_provider)))
clock_provider_node_address = phandles[cell]
clock_provider = reduced[clock_provider_node_address]
clock_provider_compat = get_compat(clock_provider_node_address)
clock_provider_bindings = yaml[clock_provider_compat]
clock_provider_bindings = get_binding(
clock_provider_node_address)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Off topic but this sort of thing strengthens my belief that a strict 80-character limit on line length is decreasing code clarity more than increasing it. Especially when EOL escapes used to meet the requirement aren't consistent (see next line).

Copy link
Member

Choose a reason for hiding this comment

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

The 80 char limit is not enforced. checkpatch will emit a warning, but CI will not fail. Contributors are free to exceed the limit when it makes sense to do so.

clock_provider_label = get_node_label( \
clock_provider_node_address)
nr_clock_cells = int(clock_provider['props'].get(
Expand Down Expand Up @@ -86,7 +85,6 @@ def _extract_consumer(self, node_address, yaml, clocks, def_label):
if clock_cells_string == clock_cell_name:
add_prop_aliases(
node_address,
yaml,
lambda alias:
self.get_label_string([
alias,
Expand All @@ -97,7 +95,6 @@ def _extract_consumer(self, node_address, yaml, clocks, def_label):
else:
add_prop_aliases(
node_address,
yaml,
lambda alias:
self.get_label_string([
alias,
Expand Down Expand Up @@ -138,7 +135,6 @@ def _extract_consumer(self, node_address, yaml, clocks, def_label):
if node_address in aliases:
add_prop_aliases(
node_address,
yaml,
lambda alias:
self.get_label_string([
alias,
Expand All @@ -156,11 +152,10 @@ def _extract_consumer(self, node_address, yaml, clocks, def_label):
# @brief Extract clocks related directives
#
# @param node_address Address of node owning the clockxxx definition.
# @param yaml YAML definition for the owning node.
# @param prop clockxxx property name
# @param def_label Define label string of node owning the directive.
#
def extract(self, node_address, yaml, prop, def_label):
def extract(self, node_address, prop, def_label):

properties = reduced[node_address]['props'][prop]

Expand All @@ -172,7 +167,7 @@ def extract(self, node_address, yaml, prop, def_label):

if prop == 'clocks':
# indicator for clock consumers
self._extract_consumer(node_address, yaml, prop_list, def_label)
self._extract_consumer(node_address, prop_list, def_label)
else:
raise Exception(
"DTClocks.extract called with unexpected directive ({})."
Expand Down
15 changes: 13 additions & 2 deletions scripts/dts/extract/compatible.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ def __init__(self):
#
# @param node_address Address of node owning the
# compatible definition.
# @param yaml YAML definition for the owning node.
# @param prop compatible property name
# @param def_label Define label string of node owning the
# compatible definition.
#
def extract(self, node_address, yaml, prop, def_label):
def extract(self, node_address, prop, def_label):

# compatible definition
binding = get_binding(node_address)
compatible = reduced[node_address]['props'][prop]
if not isinstance(compatible, list):
compatible = [compatible, ]
Expand All @@ -43,6 +43,17 @@ def extract(self, node_address, yaml, prop, def_label):
compat_defs: "1",
}
insert_defs(node_address, load_defs, {})
# Generate #define for BUS a "sensor" might be on
# for example:
# #define DT_ST_LPS22HB_PRESS_BUS_SPI 1
if 'parent' in binding:
bus = binding['parent']['bus']
compat_defs = 'DT_' + compat_label + '_BUS_' + bus.upper()
load_defs = {
compat_defs: "1",
}
insert_defs(node_address, load_defs, {})


##
# @brief Management information for compatible.
Expand Down
4 changes: 1 addition & 3 deletions scripts/dts/extract/default.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ def __init__(self):
# @brief Extract directives in a default way
#
# @param node_address Address of node owning the clockxxx definition.
# @param yaml YAML definition for the owning node.
# @param prop property name
# @param prop type (string, boolean, etc)
# @param def_label Define label string of node owning the directive.
#
def extract(self, node_address, yaml, prop, prop_type, def_label):
def extract(self, node_address, prop, prop_type, def_label):
prop_def = {}
prop_alias = {}

Expand Down Expand Up @@ -58,7 +57,6 @@ def extract(self, node_address, yaml, prop, prop_type, def_label):
if node_address in aliases:
add_prop_aliases(
node_address,
yaml,
lambda alias:
convert_string_to_label(alias) + '_' + prop_name,
label,
Expand Down
3 changes: 1 addition & 2 deletions scripts/dts/extract/directive.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ def __init__():
# @brief Extract directive information.
#
# @param node_address Address of node issueuing the directive.
# @param yaml YAML definition for the node.
# @param prop Directive property name
# @param def_label Define label string of node owning the directive.
#
def extract(self, node_address, yaml, prop, def_label):
def extract(self, node_address, prop, def_label):
pass
15 changes: 7 additions & 8 deletions scripts/dts/extract/flash.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ def extract_partition(self, node_address):

insert_defs(node_address, prop_def, prop_alias)

def _extract_flash(self, node_address, yaml, prop, def_label):
def _extract_flash(self, node_address, prop, def_label):
load_defs = {}

if node_address == 'dummy-flash':
Expand All @@ -78,14 +78,14 @@ def _extract_flash(self, node_address, yaml, prop, def_label):
flash_props = ["label", "write-block-size", "erase-block-size"]
for prop in flash_props:
if prop in self._flash_node['props']:
default.extract(node_address, None, prop, None, def_label)
default.extract(node_address, prop, None, def_label)
insert_defs(node_address, load_defs, {})

#for address in reduced:
# if address.startswith(node_address) and 'partition@' in address:
# self._extract_partition(address, yaml, 'partition', None, def_label)
# self._extract_partition(address, 'partition', None, def_label)

def _extract_code_partition(self, node_address, yaml, prop, def_label):
def _extract_code_partition(self, node_address, prop, def_label):
load_defs = {}

if node_address == 'dummy-flash':
Expand Down Expand Up @@ -116,19 +116,18 @@ def _extract_code_partition(self, node_address, yaml, prop, def_label):
#
# @param node_address Address of node owning the
# flash definition.
# @param yaml YAML definition for the owning node.
# @param prop compatible property name
# @param def_label Define label string of node owning the
# compatible definition.
#
def extract(self, node_address, yaml, prop, def_label):
def extract(self, node_address, prop, def_label):

if prop == 'zephyr,flash':
# indicator for flash
self._extract_flash(node_address, yaml, prop, def_label)
self._extract_flash(node_address, prop, def_label)
elif prop == 'zephyr,code-partition':
# indicator for code_partition
self._extract_code_partition(node_address, yaml, prop, def_label)
self._extract_code_partition(node_address, prop, def_label)
else:
raise Exception(
"DTFlash.extract called with unexpected directive ({})."
Expand Down
26 changes: 25 additions & 1 deletion scripts/dts/extract/globals.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
reduced = {}
defs = {}
structs = {}
bindings = {}
bus_bindings = {}
bindings_compat = []
old_alias_names = False

regs_config = {
Expand Down Expand Up @@ -274,7 +277,7 @@ def enable_old_alias_names(enable):
global old_alias_names
old_alias_names = enable

def add_prop_aliases(node_address, yaml,
def add_prop_aliases(node_address,
alias_label_function, prop_label, prop_aliases):
node_compat = get_compat(node_address)
new_alias_prefix = 'DT_' + convert_string_to_label(node_compat)
Expand All @@ -287,3 +290,24 @@ def add_prop_aliases(node_address, yaml,
prop_aliases[new_alias_label] = prop_label
if (old_alias_names and old_alias_label != prop_label):
prop_aliases[old_alias_label] = prop_label

def get_binding(node_address):
compat = get_compat(node_address)
parent_addr = get_parent_address(node_address)
parent_compat = get_compat(parent_addr)

if parent_compat in get_binding_compats():
parent_binding = bindings[parent_compat]

if 'child' in parent_binding:
bus = parent_binding['child']['bus']
binding = bus_bindings[bus][compat]
else:
binding = bindings[compat]
else:
binding = bindings[compat]

return binding

def get_binding_compats():
return bindings_compat
6 changes: 2 additions & 4 deletions scripts/dts/extract/interrupts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,12 @@ def _find_parent_irq_node(self, node_address):
#
# @param node_address Address of node owning the
# interrupts definition.
# @param yaml YAML definition for the owning node.
# @param prop compatible property name
# @param names (unused)
# @param def_label Define label string of node owning the
# compatible definition.
#
def extract(self, node_address, yaml, prop, names, def_label):
def extract(self, node_address, prop, names, def_label):

node = reduced[node_address]

Expand Down Expand Up @@ -67,7 +66,7 @@ def extract(self, node_address, yaml, prop, names, def_label):
except:
name = []

cell_yaml = yaml[get_compat(irq_parent)]
cell_yaml = get_binding(irq_parent)
l_cell_prefix = ['IRQ']

for i in range(reduced[irq_parent]['props']['#interrupt-cells']):
Expand All @@ -84,7 +83,6 @@ def extract(self, node_address, yaml, prop, names, def_label):
if node_address in aliases:
add_prop_aliases(
node_address,
yaml,
lambda alias:
'_'.join([convert_string_to_label(alias)] +
l_cell_prefix + name + l_cell_name),
Expand Down
5 changes: 2 additions & 3 deletions scripts/dts/extract/pinctrl.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,11 @@ def __init__(self):
# @brief Extract pinctrl information.
#
# @param node_address Address of node owning the pinctrl definition.
# @param yaml YAML definition for the owning node.
# @param prop pinctrl-x key
# @param def_label Define label string of client node owning the pinctrl
# definition.
#
def extract(self, node_address, yaml, prop, def_label):
def extract(self, node_address, prop, def_label):

pinconf = reduced[node_address]['props'][prop]

Expand All @@ -40,7 +39,7 @@ def extract(self, node_address, yaml, prop, def_label):
for p in prop_list:
pin_node_address = phandles[p]
pin_subnode = '/'.join(pin_node_address.split('/')[-1:])
cell_yaml = yaml[get_compat(pin_node_address)]
cell_yaml = get_binding(pin_node_address)
cell_prefix = 'PINMUX'
post_fix = []

Expand Down
5 changes: 1 addition & 4 deletions scripts/dts/extract/reg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@ def __init__(self):
#
# @param node_address Address of node owning the
# reg definition.
# @param yaml YAML definition for the owning node.
# @param names (unused)
# @param def_label Define label string of node owning the
# compatible definition.
#
def extract(self, node_address, yaml, names, def_label, div):
def extract(self, node_address, names, def_label, div):

node = reduced[node_address]
node_compat = get_compat(node_address)
Expand Down Expand Up @@ -90,15 +89,13 @@ def extract(self, node_address, yaml, names, def_label, div):
if node_address in aliases:
add_prop_aliases(
node_address,
yaml,
lambda alias:
'_'.join([convert_string_to_label(alias)] +
l_addr + l_idx),
l_addr_fqn,
prop_alias)
add_prop_aliases(
node_address,
yaml,
lambda alias:
'_'.join([convert_string_to_label(alias)] +
l_size + l_idx),
Expand Down
Loading