Skip to content

Conversation

@anangl
Copy link
Member

@anangl anangl commented Nov 19, 2019

Pairs of gpio_pin_configure/gpio_pin_write are replaced with calls to
gpio_pin_configure that configure a given pin as output with the proper
initial state.

@anangl
Copy link
Member Author

anangl commented Nov 19, 2019

@alextsam Could you verify that the changes for the actinius_icarus board are correct? I don't have such board at hand to check it myself. @takumiando Could you check the changes for the degu_evk board? I don't have this one either.

@takumiando
Copy link
Contributor

@anangl Thank you for your fix! I checked and approved your changes.

@alextsam
Copy link
Contributor

@anangl the changes are good, thank you! However, should I update also the device tree to use the new defines? eg currently we have GPIO_INT_ACTIVE_LOW which I guess should be replaced with GPIO_ACTIVE_LOW etc

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.

should I update also the device tree to use the new defines? eg currently we have GPIO_INT_ACTIVE_LOW which I guess should be replaced with GPIO_ACTIVE_LOW etc

Thanks @alextsam for pointing it out. I overlooked this during the review. It's best if we do it in this PR. I'm marking it as changes requested.

@anangl
Copy link
Member Author

anangl commented Nov 22, 2019

should I update also the device tree to use the new defines? eg currently we have GPIO_INT_ACTIVE_LOW which I guess should be replaced with GPIO_ACTIVE_LOW etc

Thanks @alextsam for pointing it out. I overlooked this during the review. It's best if we do it in this PR. I'm marking it as changes requested.

Yes, my bad, I focused only on changes in code and forgot about dts files. Thanks @alextsam!
I'll update dts files for both boards in a moment.

In initialization code, pairs of gpio_pin_configure/gpio_pin_write are
replaced with calls to gpio_pin_configure that configure a given pin as
output with the proper initial state.

dts file for the board is also updated with the new GPIO flags.

Signed-off-by: Andrzej Głąbek <[email protected]>
@anangl anangl force-pushed the gpio/convert_nrf_boards branch from dab28f7 to 9804c62 Compare November 22, 2019 10:18
@anangl anangl requested a review from mnkp November 22, 2019 10:19
@alextsam
Copy link
Contributor

alextsam commented Nov 23, 2019

Thank you very much for the changes! One small thing that I just noticed is line 102:

		irq-gpios = <&gpio0 28 0>, <&gpio0 29 0>;

the 0 did not matter there before, but now with the logical read this should be fixed to

		irq-gpios = <&gpio0 28 GPIO_ACTIVE_HIGH>, <&gpio0 29 GPIO_ACTIVE_HIGH>;

Should I push to anangl:gpio/convert_nrf_boards so then you just squash?
@mnkp @anangl

@anangl
Copy link
Member Author

anangl commented Nov 25, 2019

@alextsam

the 0 did not matter there before, but now with the logical read this should be fixed to

		irq-gpios = <&gpio0 28 GPIO_ACTIVE_HIGH>, <&gpio0 29 GPIO_ACTIVE_HIGH>;

There is no need to change it. GPIO_ACTIVE_HIGH has value of 0, and it is the default setting. In most such cases (not all, I know) 0 is used, not GPIO_ACTIVE_HIGH. Thus, I think it will be better to keep it as it is. See #16648 (comment).

In initialization code, pairs of gpio_pin_configure/gpio_pin_write are
replaced with calls to gpio_pin_configure that configure a given pin as
output with the proper initial state.

dts file for the board is also updated with the new GPIO flags.

Signed-off-by: Andrzej Głąbek <[email protected]>
@anangl anangl force-pushed the gpio/convert_nrf_boards branch from 9804c62 to 3e273c1 Compare November 25, 2019 07:47
@galak galak merged commit b6b6c86 into zephyrproject-rtos:topic-gpio Dec 11, 2019
@anangl anangl deleted the gpio/convert_nrf_boards branch December 12, 2019 08:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants