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

devicetree: Add helper macros for GPIO element/array initialization #23795

Closed
wants to merge 1 commit into from

Conversation

galak
Copy link
Collaborator

@galak galak commented Mar 25, 2020

Pull helper macros out of tests/lib/devicetree/src/main.c and make them
generally available for drivers or applications to utilize.

Signed-off-by: Kumar Gala kumar.gala@linaro.org

Pull helper macros out of tests/lib/devicetree/src/main.c and make them
generally available for drivers or applications to utilize.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
@galak
Copy link
Collaborator Author

galak commented Mar 25, 2020

This is need for use in drivers/lora/sx1276.c

@zephyrbot zephyrbot added area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test labels Mar 25, 2020
* @return a struct initializer for the give index "idx" into the
* "phandle-array" gpio_pha
*/
#define DT_GPIO_ELEM(idx, node_id, gpio_pha) \
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about the name DT_GPIO_LPF_AT_IDX?

The other phandle array macros also take arguments in (node_id, gpio_pha, idx) order.

Copy link
Collaborator

Choose a reason for hiding this comment

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

DT_GPIO_CELLS_AT_IDX would be more consistent with the underlying devicetree terminology and would be highlight the similarity with other phandle-valued aggregates. I really find _LPF_ to be obscure. (Pins, Flags, what is L?)

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 think L is label

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, not obvious especially if label goes away. I'd rather find a name that covers all phandle+specifiers so we can use it consistently for DT_PWM_, DT_IOCHANNEL_, .... CELLS ignores the phandle, so maybe DT_GPIO_PHCELLS_? Ideally when the phandle reference changes from a label to an identifier we wouldn't have to change the name, just the definition of the macro and of the structure into which it should be stored.

But really this PR is jumping to a partial solution for #23691 without addressing the other part: eliminating the custom struct sx1276_dio. We do not need a global macro creating precedent and adding new names to convert lora/sx1276.c; that file can use a local define as I did in #23245 (review)

* type "phandle-array"
* @return an array initializer for the given "phandle-array" gpio_pha
*/
#define DT_GPIO_LISTIFY(node_id, gpio_pha) \
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be better to let the user pass the macro here as an argument instead of assuming DT_GPIO_ELEM? Keeps the API generic in cases where it's not a label/pin/flags always.

Copy link
Collaborator

@pabigot pabigot left a comment

Choose a reason for hiding this comment

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

I would strongly prefer a solution that solves #23691 generally (including the data structure), rather than just the macro and just for one driver API. Only two in-tree files use the DT_FOO_GPIOS initializer macro: lora/sx1276 and samples/boards/nrf/battery/src/battery.c. Both can be accommodated by in-file local definitions of a similar macro.

The battery sample also needs one for DT_FOO_IO_CHANNELS.

@galak
Copy link
Collaborator Author

galak commented Mar 26, 2020

Going to close this, and just implement a driver specific macro for lora/sx1276

@galak galak closed this Mar 26, 2020
@galak galak deleted the dt-gpio branch March 26, 2020 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants