Skip to content

Conversation

@vanti
Copy link
Contributor

@vanti vanti commented Oct 4, 2019

Updates the gpio drivers 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.

Tested with:

samples/basic/blinky
samples/basic/button
tests/drivers/gpio/gpio_api_1pin
tests/drivers/gpio/gpio_basic_api (overlays added for the tested boards)

On board:

cc3220sf_launchxl
cc3235sf_launchxl
cc1352r1_launchxl

@vanti vanti requested review from bwitherspoon and galak October 4, 2019 18:32
@zephyrbot zephyrbot added area: Boards area: Tests Issues related to a particular existing or missing test labels Oct 4, 2019
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks really weird to truncate the mask to 8 bits, but not truncate the value. Please truncate both or neither.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the truncation would occur regardless, but I agree I should be consistent with the type-casting.

Copy link
Contributor

Choose a reason for hiding this comment

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

From the prototype of GPIOPinWrite it appears that only 8 pins are supported (ucPins and ucVal are both unsigned char. Shouldn't there be a check on wiether the pin parameter is valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, good point. I will add a check.

@zephyrbot
Copy link

zephyrbot commented Oct 7, 2019

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.

@vanti
Copy link
Contributor Author

vanti commented Oct 8, 2019

Updated based on comments from @pabigot

@vanti
Copy link
Contributor Author

vanti commented Oct 8, 2019

Thanks @mnkp. Updated based on your comments.

@vanti
Copy link
Contributor Author

vanti commented Oct 9, 2019

Reworked flag comparison regarding GPIO_DISCONNECTED.

@carlescufi
Copy link
Member

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

@vanti
Copy link
Contributor Author

vanti commented Oct 11, 2019

Rebased and removed usage of GPIO_DIR_MASK and GPIO_DISCONNECTED, which are replaced with (GPIO_INPUT | GPIO_OUTPUT) and 0 respectively.

Copy link
Member

@mnkp mnkp left a comment

Choose a reason for hiding this comment

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

LGTM, however there is one blocking comment.

Maybe we should squash "drivers: gpio: cc32xx: add assertions to check number of pins" into "TOPIC-GPIO] gpio: Update cc32xx gpio driver to use new gpio api" since the change is part of the new API?

@vanti
Copy link
Contributor Author

vanti commented Oct 11, 2019

LGTM, however there is one blocking comment.

Maybe we should squash "drivers: gpio: cc32xx: add assertions to check number of pins" into "TOPIC-GPIO] gpio: Update cc32xx gpio driver to use new gpio api" since the change is part of the new API?

@mnkp I don't think it should be squashed. The assertions were not just added to conform to the new API; they were added across the board for existing functions as well (e.g. read, write, enable_callback). It is a bisectable change that enhances the robustness of the driver code in general.

vanti added 4 commits October 11, 2019 11:51
Updates the cc32xx gpio 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.

Tested with:

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

On boards:

cc3220sf_launchxl
cc3235sf_launchxl

Signed-off-by: Vincent Wan <[email protected]>
Updates the cc13x2/cc26x2 gpio 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.

Tested with:

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

On board:

cc1352r1_launchxl

Signed-off-by: Vincent Wan <[email protected]>
Adding overlays so that users can run this test on the following
boards:

- cc3220sf_launchxl
- cc3235sf_launchxl
- cc1352r1_launchxl

Instructions on pins to connect are included in the overlay files.

Signed-off-by: Vincent Wan <[email protected]>
On the TI CC32xx, the GPIO peripheral supports only 8 pins per port.
We should add assertions where appropriate to verify this.

Signed-off-by: Vincent Wan <[email protected]>
@carlescufi carlescufi merged commit d2a76e8 into zephyrproject-rtos:topic-gpio Oct 15, 2019
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants