Skip to content

Conversation

@MaureenHelm
Copy link
Member

Updates the rv32m1 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.

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

Stops quietly reconfiguring pinmuxes to gpio mode. The pinmux must now
be configured explicitly in the board's pinmux.c or in the application.

Tested with:

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

On boards:

  • rv32m1_vega_ri5cy

Passes most, but not all of the 2-pin test.

@zephyrbot zephyrbot added area: Boards area: Tests Issues related to a particular existing or missing test labels Oct 7, 2019
Copy link
Member

Choose a reason for hiding this comment

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

This check is already implemented in gpio_rv32m1_pin_interrupt_configure. We could skip it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the same as the mcux driver. Can we postpone these changes to a future PR to keep mcux and rv32m1 in sync?

Copy link
Member

Choose a reason for hiding this comment

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

OK, the code is harmless.

Copy link
Member

Choose a reason for hiding this comment

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

This is handled by gpio_rv32m1_pin_interrupt_configure function.

Copy link
Member

Choose a reason for hiding this comment

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

We're mixing styles. To be consistent / Misra-C compliant we would need to write it as

if (((flags & GPIO_INT_ENABLE) != 0) && ((flags & GPIO_INPUT) == 0)) {

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I believe we should move this check to gpio_rv32m1_pin_interrupt_configure function (and read input/output configuration from the register).

Copy link
Member

Choose a reason for hiding this comment

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

Should be removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, it is still needed

Copy link
Member

Choose a reason for hiding this comment

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

The code on line 162 will clear IRQ configuration from PCR register but we don't configure interrupts here. If we merge #19553 the gpio_rv32m1_configure would still modify interrupt configuration.

Copy link
Member

Choose a reason for hiding this comment

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

This function is not used anymore by gpio_rv32m1_configure. We could move it directly above gpio_rv32m1_pin_interrupt_configure. It would make the code easier to read.

@carlescufi
Copy link
Member

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

@MaureenHelm
Copy link
Member Author

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

Done

@carlescufi
Copy link
Member

@pabigot and @mnkp from the API meeting: @MaureenHelm would like to merge this as-is and follow-up as described here. Could you approve?

Explicitly configures the rgb led pinmuxes as gpios. Currently the gpio
driver quietly changes the pinmux to gpio mode when configuring a gpio
pin, but this behavior is about to change.

Signed-off-by: Maureen Helm <[email protected]>
Updates the rv32m1 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.

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

Stops quietly reconfiguring pinmuxes to gpio mode. The pinmux must now
be configured explicitly in the board's pinmux.c or in the application.

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

On boards:
- rv32m1_vega_ri5cy

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

Signed-off-by: Maureen Helm <[email protected]>
@galak galak merged commit b8d4899 into zephyrproject-rtos:topic-gpio Oct 22, 2019
@MaureenHelm MaureenHelm deleted the rv32m1-gpio branch October 23, 2019 16:46
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