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

add support to store devicetree phandle+specifier as "foo_dt_spec" structures #23691

Open
pabigot opened this issue Mar 23, 2020 · 6 comments
Open
Labels
area: Devicetree Enhancement Changes/Updates/Additions to existing features

Comments

@pabigot
Copy link
Collaborator

pabigot commented Mar 23, 2020

#23245 improves the devicetree access solution significantly, but currently lacks a replacement for phandle-array values which are a series of phandle + specifier. Example is a GPIO device reference plus a pin and initial flags; or an ADC device and and an input channel. In the existing access solution a C initializer list is generated that captures the combined values, e.g. { "GPIO_0", 1, 0" }.

This gap complicates conversions of drivers that use the initializer list, e.g. the following:

-       .bus_name = DT_INST_0_MAXIM_DS3231_BUS_NAME,
+       .bus_name = DT_INST_BUS_LABEL(0),
        /* Driver does not currently use 32k GPIO. */
-#ifdef DT_INST_0_MAXIM_DS3231_ISW_GPIOS_CONTROLLER
-       .isw_gpios = DT_INST_0_MAXIM_DS3231_ISW_GPIOS,
+#if DT_INST_NODE_HAS_PROP(0, isw_gpios)
+       .isw_gpios = DT_INST_GPIO(0, isw_gpios),
 #endif
-       .addr = DT_INST_0_MAXIM_DS3231_BASE_ADDRESS,
+       .addr = DT_INST_REG_ADDR(0),

requires local addition of the following macro:

#define DT_INST_GPIO(inst, gpio_pha) {		\
	DT_INST_GPIO_LABEL(inst, gpio_pha),	\
	DT_INST_GPIO_PIN(inst, gpio_pha),	\
	DT_INST_GPIO_FLAGS(inst, gpio_pha),	\
}

to initialize the local definition:

struct gpios {
        const char *ctrl;
        gpio_pin_t pin;
        gpio_dt_flags_t flags;
};

Such a definition has not been standardized because technically different GPIO controllers could use different cells in their specifiers.

Where we have consistent cells for all drivers it would good to have a standard structure that can be used to store a phandle+specifier, and a way to initialize such a structure.

See discussion at/around: #23245 (comment)

@pabigot pabigot added the Enhancement Changes/Updates/Additions to existing features label Mar 23, 2020
@pabigot
Copy link
Collaborator Author

pabigot commented Mar 23, 2020

@mbolivar @galak let's discuss here rather than clutter up #23245

@pabigot pabigot mentioned this issue Mar 23, 2020
12 tasks
@mbolivar-nordic
Copy link
Contributor

@pabigot collecting a list of where we want interim "LPF" style macros as discussed in #23245:

  • GPIOs
  • ADC

Interrupts also? Anything else?

@pabigot
Copy link
Collaborator Author

pabigot commented Mar 23, 2020

git grep -e -cells dts/bindings/ | sed -r -e 's@^[^:]*:\s*@@' | sort | uniq probably shows the candidates. At minimum clock dma gpio interrupt io-channel (ADC) pinmux pwm may be candidates. An issue is that the number of cells and their names/roles is binding-specific.

@mbolivar-nordic mbolivar-nordic self-assigned this Sep 3, 2020
@mbolivar-nordic mbolivar-nordic changed the title add support to store devicetree phandle+specifier as an opaque value add support to store devicetree phandle+specifier as "foo_dt_spec" structures Sep 30, 2021
@mbolivar-nordic
Copy link
Contributor

I've renamed the issue now that we are using 'foo_dt_spec' (e.g. gpio_dt_spec, i2c_dt_spec, spi_dt_spec) for this type of thing.

@mbolivar-nordic
Copy link
Contributor

mbolivar-nordic commented May 25, 2022

A status update as we head into v3.1: we have foo_dt_spec structures for the following cases, which overlaps somewhat with the above list.

Still not done from the above list: dmas and clocks properties are usuallly phandle+specifiers in DT, but have no corresponding spec structures in their zephyr APIs.

I would argue that interrupts does not fit within the scope of this issue, as it is a plain array, not a phandle-array, in terms of its type in the bindings.

@zephyrbot
Copy link
Collaborator

Hi @mbolivar-nordic,

This issue, marked as an Enhancement, was opened a while ago and did not get any traction. Please confirm the issue is correctly assigned and re-assign it otherwise.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

@pabigot you are also encouraged to help moving this issue forward by providing additional information and confirming this request/issue is still relevant to you.

Thanks!

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

4 participants