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

drivers: gpio: move non-standard dts flags to be soc specific #39767

Merged

Conversation

henrikbrixandersen
Copy link
Member

@henrikbrixandersen henrikbrixandersen commented Oct 27, 2021

Reserve the upper 8 bits of gpio_dt_flags_t for SoC specific flags and move the non-standard, hardware-specific GPIO devicetree flags (IO voltage level, drive strength, debounce filter) from the generic dt-bindings/gpio/gpio.h header to SoC specific dt-bindings headers.

Some of the SoC specific dt-bindings flags take up more bits than necessary in order to retain backwards compatibility with the deprecated GPIO flags. The width of these fields can be reduced/optimized once the deprecated flags are removed.

Remove hardcoded use of GPIO_INT_DEBOUNCE in GPIO client drivers. This flag can now be set in the devicetree for boards/SoCs with debounce filter support. The SoC specific debounce flags have had the _INT part of their name removed since these flag must be passed to gpio_pin_configure(), not gpio_pin_interrupt_configure().

Signed-off-by: Henrik Brix Andersen hebad@vestas.com

@github-actions github-actions bot added area: API area: Devicetree Binding labels Oct 27, 2021
@zephyrbot zephyrbot requested a review from mnkp Oct 28, 2021
keith-zephyr
keith-zephyr previously requested changes Nov 2, 2021
Copy link
Contributor

@keith-zephyr keith-zephyr left a comment

If the dt-bindings are going to be extended, I think all the GPIO flags should be moved into the dt-bindings instead of just the drive strength flags.

include/dt-bindings/gpio/gpio.h Outdated Show resolved Hide resolved
@keith-zephyr keith-zephyr requested a review from brockus-zephyr Nov 2, 2021
@carlescufi
Copy link
Member

@carlescufi carlescufi commented Nov 2, 2021

API meeting:

include/dt-bindings/gpio/gpio.h Outdated Show resolved Hide resolved
@carlescufi
Copy link
Member

@carlescufi carlescufi commented Nov 9, 2021

@henrikbrixandersen henrikbrixandersen marked this pull request as draft Nov 30, 2021
@keith-zephyr
Copy link
Contributor

@keith-zephyr keith-zephyr commented Jan 18, 2022

I'd like to see this change get approved. What's the current status of this PR?

@henrikbrixandersen henrikbrixandersen force-pushed the gpio_ds_flags_dts branch 3 times, most recently from a6e0693 to 2756a23 Compare Jan 19, 2022
@henrikbrixandersen henrikbrixandersen marked this pull request as ready for review Jan 19, 2022
@henrikbrixandersen
Copy link
Member Author

@henrikbrixandersen henrikbrixandersen commented Jan 19, 2022

I'd like to see this change get approved. What's the current status of this PR?

@keith-zephyr Thanks for reminding me. I have updated this PR to extend the size of gpio_dt_flags_t from 8 to 16 bits and moved the flags accordingly.

@henrikbrixandersen henrikbrixandersen requested a review from galak Jan 19, 2022
@henrikbrixandersen henrikbrixandersen added the dev-review label Jan 20, 2022
@henrikbrixandersen henrikbrixandersen removed the dev-review label Jan 20, 2022
@keith-zephyr keith-zephyr dismissed their stale review Jan 21, 2022

Removing my blocking vote.

@carlescufi carlescufi requested a review from gmarull Jan 24, 2022
@carlescufi
Copy link
Member

@carlescufi carlescufi commented Jan 24, 2022

@mnkp @gmarull @mbolivar-nordic please take a look

gmarull
gmarull previously approved these changes Feb 17, 2022
Copy link
Collaborator

@gmarull gmarull left a comment

LGTM. In some cases, like nRF, we can now offer more meaningful options for the users. Will look at it after this gets in.

mbolivar-nordic
mbolivar-nordic previously approved these changes Feb 17, 2022
Copy link
Member

@mbolivar-nordic mbolivar-nordic left a comment

We need an index of device drivers in the documentation that includes information on driver-specific features more than ever with changes like this going in...

Copy link
Member

@mnkp mnkp left a comment

Thanks @henrikbrixandersen, great work! I have some minor but blocking comments. Also, I think we should squash "drivers: gpio: allow specifying GPIO drive strength/debounce flags in dts" commit. I.e. implement all the changes in a single commit.

include/drivers/gpio.h Show resolved Hide resolved
@henrikbrixandersen
Copy link
Member Author

@henrikbrixandersen henrikbrixandersen commented Feb 22, 2022

Thanks @henrikbrixandersen, great work! I have some minor but blocking comments. Also, I think we should squash "drivers: gpio: allow specifying GPIO drive strength/debounce flags in dts" commit. I.e. implement all the changes in a single commit.

@mnkp Thank you for reviewing. I have squashed the two commits into one. Please see my question on how you'd like to handle deprecation of the old macros.

@henrikbrixandersen henrikbrixandersen requested a review from mnkp Feb 22, 2022
brockus-zephyr
brockus-zephyr previously approved these changes Feb 22, 2022
@henrikbrixandersen
Copy link
Member Author

@henrikbrixandersen henrikbrixandersen commented Feb 28, 2022

@mnkp Ping?

@@ -140,7 +141,7 @@ static int gpio_atcgpio100_config(const struct device *port,
key = k_spin_lock(&data->lock);

/* Set de-bounce */
if (flags & GPIO_INT_DEBOUNCE) {
if (flags & ATCGPIO100_GPIO_INT_DEBOUNCE) {
Copy link
Member

@anangl anangl Mar 8, 2022

Choose a reason for hiding this comment

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

I think we should get rid of this misleading INT_ component from the names of debounce flags. Unlike all other GPIO_INT_* flags, the debounce flag is supposed to be used in calls to gpio_pin_configure(), not gpio_pin_interrupt_configure().

Copy link
Member Author

@henrikbrixandersen henrikbrixandersen Mar 9, 2022

Choose a reason for hiding this comment

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

@anangl Thank you for reviewing. I have removed the _INT part of the name for the SoC specific debounce flags.

gmarull
gmarull previously approved these changes Mar 9, 2022
Reserve the upper 8 bits of gpio_dt_flags_t for SoC specific flags and
move the non-standard, hardware-specific GPIO devicetree flags (IO
voltage level, drive strength, debounce filter) from the generic
dt-bindings/gpio/gpio.h header to SoC specific dt-bindings headers.

Some of the SoC specific dt-bindings flags take up more bits than
necessary in order to retain backwards compatibility with the deprecated
GPIO flags. The width of these fields can be reduced/optimized once the
deprecated flags are removed.

Remove hardcoded use of GPIO_INT_DEBOUNCE in GPIO client drivers. This
flag can now be set in the devicetree for boards/SoCs with debounce
filter support. The SoC specific debounce flags have had the _INT part
of their name removed since these flag must be passed to
gpio_pin_configure(), not gpio_pin_interrupt_configure().

Signed-off-by: Henrik Brix Andersen <hebad@vestas.com>
gmarull
gmarull approved these changes Mar 9, 2022
anangl
anangl approved these changes Mar 9, 2022
include/dt-bindings/gpio/gpio.h Show resolved Hide resolved
mnkp
mnkp approved these changes Mar 9, 2022
@nashif nashif merged commit d4023b3 into zephyrproject-rtos:main Mar 10, 2022
26 checks passed
@henrikbrixandersen henrikbrixandersen deleted the gpio_ds_flags_dts branch Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment