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

[TOPIC-GPIO] gpio: Update mcux igpio driver to use new gpio api #19247

Merged
merged 3 commits into from
Oct 15, 2019

Conversation

MaureenHelm
Copy link
Member

Updates the mcux igpio driver and all associated boards to use new
device tree compatible gpio configuration flags. Implements new port
get/set/clear/toggle and pin_interrupt_configure functions recently
added to the gpio api.

Note that the driver now handles configuring by port inefficiently,
making separate register writes for each pin in the port. However, port
configuration is deprecated in the new gpio api and will soon be
removed.

Tested with:

  • samples/basic/blinky
  • samples/basic/button
  • tests/drivers/gpio/gpio_api_1pin

On boards:

  • mimxrt1015_evk
  • mimxrt1020_evk
  • mimxrt1050_evk
  • mimxrt1060_evk
  • mimxrt1064_evk

Signed-off-by: Maureen Helm [email protected]

@galak galak changed the title gpio: Update mcux igpio driver to use new gpio api [TOPIC-GPIO] gpio: Update mcux igpio driver to use new gpio api Sep 18, 2019
Copy link
Collaborator

@agansari agansari left a comment

Choose a reason for hiding this comment

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

Code is OKAY.
Tested on i.mx 1050 rt, works as expected.

@pabigot
Copy link
Collaborator

pabigot commented Sep 19, 2019

Has anybody run the 2-pin test #19249 on this? Hack it to specify usable pins and do pinmux if necessary.

@MaureenHelm
Copy link
Member Author

Rebased to incorporate #19248. Still need to run the 2-pin test

drivers/gpio/gpio_mcux_igpio.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_mcux_igpio.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_mcux_igpio.c Show resolved Hide resolved
} else if (pin < 32) {
shift = 2 * (pin - 16);
base->ICR2 = (base->ICR2 & ~(3 << shift)) | (icr << shift);
} else {
Copy link
Member

Choose a reason for hiding this comment

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

There is an assert in the API that guarantees that pin number is smaller than the maximum port width. Since the maximum port width in case of mcux driver is 32 we could skip the else.

@galak
Copy link
Collaborator

galak commented Oct 2, 2019

Need to be rebased on updated topic-gpio branch.

@MaureenHelm
Copy link
Member Author

Rebased to pick up 2-pin test, which now passes. The problem I had earlier was fixed by locking interrupts in mcux_igpio_pin_interrupt_configure

@zephyrbot zephyrbot added the area: Tests Issues related to a particular existing or missing test label Oct 4, 2019
@@ -0,0 +1,13 @@
/*
* Copyright (c) 2019 Nordic Semiconductor ASA
Copy link
Collaborator

Choose a reason for hiding this comment

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

change copyright

Copy link
Member Author

Choose a reason for hiding this comment

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

I kept the Nordic copyright because I copied the file from another board

Comment on lines +42 to +56
#ifdef CONFIG_BOARD_MIMXRT1050_EVK
IOMUXC_SetPinMux(IOMUXC_GPIO_AD_B1_06_GPIO1_IO22, 0);
IOMUXC_SetPinMux(IOMUXC_GPIO_AD_B1_07_GPIO1_IO23, 0);

IOMUXC_SetPinConfig(IOMUXC_GPIO_AD_B1_06_GPIO1_IO22,
IOMUXC_SW_PAD_CTL_PAD_PKE_MASK |
IOMUXC_SW_PAD_CTL_PAD_HYS_MASK |
IOMUXC_SW_PAD_CTL_PAD_SPEED(2) |
IOMUXC_SW_PAD_CTL_PAD_DSE(6));

IOMUXC_SetPinConfig(IOMUXC_GPIO_AD_B1_07_GPIO1_IO23,
IOMUXC_SW_PAD_CTL_PAD_PKE_MASK |
IOMUXC_SW_PAD_CTL_PAD_SPEED(2) |
IOMUXC_SW_PAD_CTL_PAD_DSE(6));
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

i moved this to board's pinmux initialization

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes this is ugly here, but I also don't want to make it the default pinmux configuration for the board. I think it's more likely these pins will be used for UART.

@carlescufi
Copy link
Member

@MaureenHelm
The topic branch has been rebased. Could you please rebase this PR against it?

Updates the mcux igpio driver and all associated boards to use new
device tree compatible gpio configuration flags. Implements new port
get/set/clear/toggle and pin_interrupt_configure functions recently
added to the gpio api.

Assumes the gpio api layer handles translating logical flags to physical
flags.

Removes port configuration support since that feature is deprecated in
the new gpio api.

Tested with:
- samples/basic/blinky
- samples/basic/button
- tests/drivers/gpio/gpio_api_1pin

On boards:
- mimxrt1015_evk
- mimxrt1020_evk
- mimxrt1050_evk
- mimxrt1060_evk
- mimxrt1064_evk

Signed-off-by: Maureen Helm <[email protected]>
Fixes the user button label in the device tree to match the board's
silkscreen.

Signed-off-by: Maureen Helm <[email protected]>
Enables the 2-pin gpio test on the mimxrt1050_evk board by adding a dts
overlay and configuring pinmuxes on the arduino header.

Signed-off-by: Maureen Helm <[email protected]>
@MaureenHelm
Copy link
Member Author

The topic branch has been rebased. Could you please rebase this PR against it?

Done

@carlescufi carlescufi merged commit be4e399 into zephyrproject-rtos:topic-gpio Oct 15, 2019
@MaureenHelm MaureenHelm deleted the mcux-igpio branch October 15, 2019 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Boards area: GPIO area: Tests Issues related to a particular existing or missing test platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants