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

dts: bindings: base.yaml: Add hw instance id #23335

Closed

Conversation

nandojve
Copy link
Member

@nandojve nandojve commented Mar 8, 2020

Durant the process to convert Atmel drivers to use DT_INST_ defines was detected the necessity of a hardware instance identificator. Some drivers need configure port pins, clocks, DMA etc. This will
track the hardware instance on the DT_INST_x_ independent of the x value.

This will keep simple the conversion this add a new non mandatory property called . The driver that need these feature need add required=true on the dts/binding/driver and complement any
device tree with proper value.

Example of use:

foo0: foo@1000 {
	compatible = "man,foo-drv";
	id = <0>;
	status="disabled";
};
foo1: foo@1001 {
	compatible = "man,foo-drv";
	id = <1>;
};
barn: bar@2001 {
	compatible = "man,bar-drv";
	id = <10>;
};
barm: bar@3001 {
	compatible = "man,bar-drv";
	id = <20>;
};

Will generate:

DT_INST_0_MAN_FOO_DRV_ID=1
DT_INST_0_MAN_BAR_DRV_ID=10
DT_INST_1_MAN_BAR_DRV_ID=20

It is intent to be used as below to propagate the hardware id on all driver defines.

#if DT_INST_0_MAN_FOO_DRV
DRV_FOO_DEVICE_INIT(0, DT_INST_0_MAN_FOO_DRV_ID);
#endif

A good driver example that need this definition:

static const struct drv_foo_dev_cfg drv##n##_sam_config = {	\
	.regs = (Drv *)DT_INST_##n##_MAN_DRV_FOO_BASE_ADDRESS,	\
	.pin_rx = PIN_FOO##m##_RXD,				\
	.pin_tx = PIN_FOO##m##_TXD,				\
}

If m isn't defined the driver will invariably configure the wrong pins and communication won't happen. This can't be derived by node name for many reasons. For instance. the above DT snip defined barn and barm that have a name without number and an unusual value. It represents two instances of bar that shouldn't assume values other than defined on the id property.

see #23107

Signed-off-by: Gerson Fernando Budke nandojve@gmail.com

ulfalizer and others added 30 commits February 21, 2020 12:43
It's better to allow per-instance EDT configuration than to set a global
variable on the edtlib module. Enable/disable the warning for reg/unit
address mismatches via a flag to EDT.__init__(), instead of via a global
variable. That makes it consistent too.

Another option would be to pass the 'dtc' flags to EDT.__init__(), but
it makes the interface a bit ugly. Maybe if it needs to emulate lots of
other flags later.

Clarify that edtlib itself isn't meant to have any state in the comment
at the top of the module.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
Use DT_INST define as this is the preferred set of defines to utilize
for drivers.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines and remove Kconfig per instance
enablement in favor of DT_INST_ define existing.  Also, remove the
aliases that had been used for this driver in nxp_rt.dtsi.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  Replace dts_fixup.h use
for DT_RTC_0_NAME with DT_INST_0_NXP_KINETIS_RTC_LABEL to be
consistent.  Also, remove the aliases that had been used for this
driver in various nxp_k*.dtsi.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to fully use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to fully use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to fully use DT_INST_ defines.  Currently IRQs are not
generated from DTS on esp32 so we create #defines for what they should
look like.  Convert the IRQ defines to match DT_INST style so if/when
we have that info in DTS it will look the same.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  There was just one case for
CS_GPIOS that wasn't using DT_INST defines already.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
The ILITEK ILI9340 should have been in the dts and added as #defines in
dts_fixup.h.  Fix this by adding a display node in the dts.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
spi_dev_cs_gpio() takes a Node and returns the chip select GPIO for it.
Having that information available directly from Node is neater, so turn
it into a Node.spi_cs_gpio property instead.

That gets rid of the only public global function in edtlib, which might
make the API design clearer too.

Tested with the sensortile_box board, which uses SPI chip select.

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>
Add additional attributes to each edtlib.Node we process, before
calling into the write_foo() routines.

This includes the identifier returned by node_ident(), which is used
as the primary identifier for the node, as well as lists for instance
and aliases nodes, and a catchall list that contains all the other
identifiers in addition to the primary one.

Use this information in a new for_each_ident(node, ident) def that
will be put to use in the various write_foo() routines.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Group the macros together by namespace rather than putting all the
BASE_ADDRESS macros together and all the SIZE macros together. E.g.,
all the DT_INST_<x> namespace macros for each node now appear
consecutively.

Add a comment making it clear that this output comes from "regs",
since "BASE_ADDRESS" and "SIZE" are not property names.

Other than the order in which they appear and comments, the output
before and after this patch should be exactly the same.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Use augmented nodes to print macros grouped by namespace.

Other than the order in which they appear and comments, the output
before and after this patch should be exactly the same.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Mirror the change already done to write_regs().

Other than the order in which they appear and comments, the output
before and after this patch should be exactly the same.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is similar to the work already done for regs.

Other than the order in which they appear and comments, the output
before and after this patch should be exactly the same.

We're intentionally leaving some of the helpers in module scope here
to use some of the subroutines elsewhere later on when reworking
write_spi_dev().

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This uses augmented nodes in the same way as done previously.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This too is an attempt to reimplement the previous behavior exactly,
modulo the order in which things are defined.

This is the last function which is calling into the previous
implementation's out_node and node_*_alias() functions, so these can
be removed now.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
mbolivar-nordic and others added 14 commits February 21, 2020 12:43
Write the same results explicitly in its only remaining caller.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Pad node identifiers to 60 characters. This results in better
alignment in practice than the current value of 40, which is a bit
low.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This corresponds to the attribute by the same name in dtlib.Node.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Add a function takes a 'label' and returns "y" if we find an
"enabled" node that has a 'nodelabel' of 'label' in the EDT
otherwise we return "n"

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
The max-value should just be an int and not an array.  Change the type
to 'int' in the binding and fixup the driver to match.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Add girq and girq-bit to encode per device information.  This allows the
driver to get any device unique info from device tree.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

As part of this change we utilize the device tree for GIRQ info and
rename timer3 to 2 since we are doing this by instance number.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.  The driver mostly used DT_INST_ defines but
a few IRQ priority defines needed conversion.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Add girq and girq-bit to encode per device information.  This allows the
driver to get any device unique info from device tree.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
Convert driver to use DT_INST_ defines.  The preferred defines for
drivers are DT_INST_.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
@nandojve nandojve requested a review from galak as a code owner March 8, 2020 21:53
Durant the process to convert Atmel drivers to use DT_INST_ defines
was detected the necessity of a hardware instance identificator.
Some drivers need configure port pins, clocks, DMA etc. This will
track the hw instance on the DT_INST_x_ independent of the 'x' value.

This will keep simple the conversion this add a new non mandatory
property called <id>. The driver that need these feature need add
'required=true' on the dts/binding/driver  and complement any
device tree with proper value.

Examples:

foo0: foo@1000 {
	compatible = "man,foo-drv";
	id = <0>;
	status="disabled";
};
foo1: foo@1001 {
	compatible = "man,foo-drv";
	id = <1>;
};
barn: bar@2001 {
	compatible = "man,bar-drv";
	id = <10>;
};
barm: bar@3001 {
	compatible = "man,bar-drv";
	id = <20>;
};

Will generate:

DT_INST_0_MAN_FOO_DRV_ID=1
DT_INST_0_MAN_BAR_DRV_ID=10
DT_INST_1_MAN_BAR_DRV_ID=20

It is intent to be used as below to propagate the hardware id on all
driver defines.

DRV_FOO_DEVICE_INIT(0, DT_INST_0_MAN_FOO_DRV_ID);

A good driver example that need this definition:

static const struct drv_foo_dev_cfg drv##n##_sam_config = {	\
	.regs = (Drv *)DT_INST_##n##_MAN_DRV_FOO_BASE_ADDRESS,	\
	.pin_rx = PIN_FOO##m##_RXD,				\
	.pin_tx = PIN_FOO##m##_TXD,				\
}

If 'm' isn't defined the driver will invariably configure the wrong
pins and communication won't happen. This can't be derived by node
name for many reasons. For instance. the above DT snip defined 'barn'
and 'barm' that have a name without number and an unusual value. It
represents two instances of bar that shouldn't assume values other
than defined on the id property.

see zephyrproject-rtos#23107

Signed-off-by: Gerson Fernando Budke <nandojve@gmail.com>
@galak
Copy link
Collaborator

galak commented Mar 10, 2020

The solution for this should be utilizing NODELABEL defines, We are working on getting PR #23245 in a state that we should base all future conversion work on.

Copy link
Collaborator

@galak galak left a comment

Choose a reason for hiding this comment

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

Utilize NODELABEL as we will generate it in PR #23245

@nandojve
Copy link
Member Author

topic-devicetree already merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants