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

tests: drivers: gpio: gpio_basic_api: correct dts binding #29689

Closed
cfriedt opened this issue Nov 1, 2020 · 7 comments
Closed

tests: drivers: gpio: gpio_basic_api: correct dts binding #29689

cfriedt opened this issue Nov 1, 2020 · 7 comments
Assignees
Labels
area: GPIO Enhancement Changes/Updates/Additions to existing features

Comments

@cfriedt
Copy link
Member

cfriedt commented Nov 1, 2020

Is your enhancement proposal related to a problem? Please describe.
As @sjg20 pointed out in #26484, compatible strings should use '-' instead of '_'.

Describe the solution you'd like
The existing "test,gpio_basic_api" binding should be changed to "test,gpio-basic-api". All of the boards / relevant files in tests/drivers/gpio/gpio_basic_api should be corrected to reflect that change.

Describe alternatives you've considered

Additional context

@cfriedt cfriedt added the Enhancement Changes/Updates/Additions to existing features label Nov 1, 2020
@pabigot pabigot self-assigned this Nov 1, 2020
@pabigot
Copy link
Collaborator

pabigot commented Nov 2, 2020

While it's true that the vast majority of compatible strings use - as separators the devicetree specification does not mandate or even mention this, and there is existing practice in Zephyr and in Linux for using _ as a separator, particularly but not exclusively when there is no vendor, prefix.

I don't object to making this change if we agree and document that's a standard we should follow, and add checks to the tooling to warn when people violate it. @sjg20 @mbolivar @galak

@pabigot
Copy link
Collaborator

pabigot commented Nov 2, 2020

BTW: I believe the reason I went with test,gpio_basic_api originally is because this is the binding used for the gpio_basic_api test (not the gpio-basic-api test). I'm still a little bit inclined to allowing that as an exception, especially since it's essentially a private binding.

@galak
Copy link
Collaborator

galak commented Nov 2, 2020

I don't object to making this change if we agree and document that's a standard we should follow, and add checks to the tooling to warn when people violate it. @sjg20 @mbolivar @galak

While it seems general convention to use - instead of _ there isn't anything in the DTS specs around conventions for compatible beyond vendor, prefix usage. So if someone wants to propose that as a change to the DTS spec than I'm all for adding a warning in the tooling if it is accepted (https://github.com/devicetree-org/devicetree-specification).

@sjg20
Copy link
Collaborator

sjg20 commented Nov 4, 2020

Certainly this is a pretty strong convention. The DT spec examples all use hyphen and the the /aliases node requires it, but oddly it has capital letters for the alias labels.

This convention is a bit confusing in Zephyr because hyphen gets converted to underscore to become a valid C identifier

The underscore is reserved for phandle names.

@sjg20
Copy link
Collaborator

sjg20 commented Nov 4, 2020

@cfriedt I'm happy to have a crack at a DT spec update if you don't want to do it.

@cfriedt
Copy link
Member Author

cfriedt commented Nov 4, 2020

@cfriedt I'm happy to have a crack at a DT spec update if you don't want to do it.

I think it's unrelated to the other PR for the most part, so please go ahead.

@sjg20
Copy link
Collaborator

sjg20 commented Apr 1, 2021

Applied here:
devicetree-org/devicetree-specification#42

@sjg20 sjg20 closed this as completed Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: GPIO Enhancement Changes/Updates/Additions to existing features
Projects
None yet
Development

No branches or pull requests

4 participants