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

Update GPIO API to support flags used by Linux DT bindings #16648

Merged
merged 12 commits into from
Sep 18, 2019

Conversation

mnkp
Copy link
Member

@mnkp mnkp commented Jun 5, 2019

This is a first PR in series which updates GPIO API, drivers and samples to implement changes discussed in #15611, #10339. It is based on work done in #12651, #11880.

The primary focus of this PR is to introduce standard flags used by Linux DT bindings. The specific changes implemented:

  • Added GPIO_ACTIVE_LOW, GPIO_ACTIVE_HIGH to indicate pin active state.
  • Added GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE to configure single ended pin
    driving mode.
  • Added GPIO_PULL_UP, GPIO_PULL_DOWN flags.

Support for GPIO_PERSISTENT, GPIO_TRANSITORY flags (bit 3) will be added later.

Remaining changes:

  • Added flags to support gpio and pinctrl bindings defined in form of properties.

    • GPIO_INPUT, GPIO_OUTPUT to configure pin as input or output.
    • Added GPIO_OUTPUT_LOW, GPIO_OUTPUT_HIGH flags to initialize output in low or high state.
  • All GPIO flags which should not be used in DTS were removed from DT bindings gpio.h file. Support for DTS properties of GPIO node such as: input, output-low, output-high, line-name, etc. will be added in the future.

  • GPIO_INT_* flags were reworked to support configuration of pin interrupts on logical or physical levels.

  • Following flags were deprecated: GPIO_DIR_, GPIO_DS_DISCONNECT_, GPIO_PUD_, GPIO_INT_ACTIVE_, GPIO_INT_DOUBLE_EDGE, GPIO_POL_*.

  • Added gpio_pin_set, gpio_pin_get functions which work with pin logical levels, taking into account GPIO_ACTIVE_LOW flag

gpio_sam, gpio_gecko drivers along with .dts and other infrastructure files were updated to reflect above changes. Also 'button' and 'blinky' samples were updated to use the new GPIO flags.

This PR is not yet complete. To be able to merge it we need to update all existing GPIO drivers, samples and tests to use the new flags.

Any functional changes not mentioned here will be addressed in subsequent PRs. Specifically this PR doesn't modify flags related to pin drive strength, debouncing neither attempts to introduce functions operating on ports.

@mnkp mnkp added area: GPIO area: API Changes to public APIs DNM This PR should not be merged (Do Not Merge) labels Jun 5, 2019
@zephyrbot
Copy link
Collaborator

zephyrbot commented Jun 5, 2019

Found the following issues, please fix and resubmit:

checkpatch issues

-:2169: WARNING:LINE_SPACING: Missing a blank line after declarations
#2169: FILE: drivers/gpio/gpio_sam.c:43:
+	struct gpio_sam_runtime *const dev_data = DEV_DATA(dev);
+	Pio *const pio = cfg->regs;

-:2169: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2169: FILE: drivers/gpio/gpio_sam.c:43:
+	Pio *const pio = cfg->regs;
 	    ^

-:2318: WARNING:LINE_SPACING: Missing a blank line after declarations
#2318: FILE: drivers/gpio/gpio_sam.c:203:
+	const struct gpio_sam_config *const cfg = DEV_CFG(dev);
+	Pio *const pio = cfg->regs;

-:2318: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2318: FILE: drivers/gpio/gpio_sam.c:203:
+	Pio *const pio = cfg->regs;
 	    ^

-:2329: WARNING:LINE_SPACING: Missing a blank line after declarations
#2329: FILE: drivers/gpio/gpio_sam.c:214:
+	const struct gpio_sam_config *const cfg = DEV_CFG(dev);
+	Pio *const pio = cfg->regs;

-:2329: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2329: FILE: drivers/gpio/gpio_sam.c:214:
+	Pio *const pio = cfg->regs;
 	    ^

-:2339: WARNING:LINE_SPACING: Missing a blank line after declarations
#2339: FILE: drivers/gpio/gpio_sam.c:224:
+	const struct gpio_sam_config *const cfg = DEV_CFG(dev);
+	Pio *const pio = cfg->regs;

-:2339: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2339: FILE: drivers/gpio/gpio_sam.c:224:
+	Pio *const pio = cfg->regs;
 	    ^

-:2350: WARNING:LINE_SPACING: Missing a blank line after declarations
#2350: FILE: drivers/gpio/gpio_sam.c:235:
+	const struct gpio_sam_config *const cfg = DEV_CFG(dev);
+	Pio *const pio = cfg->regs;

-:2350: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2350: FILE: drivers/gpio/gpio_sam.c:235:
+	Pio *const pio = cfg->regs;
 	    ^

-:2361: WARNING:LINE_SPACING: Missing a blank line after declarations
#2361: FILE: drivers/gpio/gpio_sam.c:246:
+	const struct gpio_sam_config *const cfg = DEV_CFG(dev);
+	Pio *const pio = cfg->regs;

-:2361: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2361: FILE: drivers/gpio/gpio_sam.c:246:
+	Pio *const pio = cfg->regs;
 	    ^

-:2374: WARNING:LINE_SPACING: Missing a blank line after declarations
#2374: FILE: drivers/gpio/gpio_sam.c:259:
+	struct gpio_sam_runtime *const dev_data = DEV_DATA(dev);
+	Pio *const pio = cfg->regs;

-:2374: ERROR:SPACING: need consistent spacing around '*' (ctx:WxV)
#2374: FILE: drivers/gpio/gpio_sam.c:259:
+	Pio *const pio = cfg->regs;
 	    ^

-:2667: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2667: FILE: include/drivers/gpio.h:251:
+#define GPIO_DIR_IN             __DEPRECATED_MACRO GPIO_INPUT

-:2674: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2674: FILE: include/drivers/gpio.h:258:
+#define GPIO_DIR_OUT            __DEPRECATED_MACRO GPIO_OUTPUT

-:2680: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2680: FILE: include/drivers/gpio.h:264:
+#define GPIO_DS_DISCONNECT_LOW  __DEPRECATED_MACRO GPIO_OPEN_SOURCE

-:2686: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2686: FILE: include/drivers/gpio.h:270:
+#define GPIO_DS_DISCONNECT_HIGH __DEPRECATED_MACRO GPIO_OPEN_DRAIN

-:2697: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2697: FILE: include/drivers/gpio.h:281:
+#define GPIO_PUD_NORMAL		__DEPRECATED_MACRO 0

-:2704: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2704: FILE: include/drivers/gpio.h:288:
+#define GPIO_PUD_PULL_UP	__DEPRECATED_MACRO GPIO_PULL_UP

-:2711: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2711: FILE: include/drivers/gpio.h:295:
+#define GPIO_PUD_PULL_DOWN	__DEPRECATED_MACRO GPIO_PULL_DOWN

-:2717: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2717: FILE: include/drivers/gpio.h:301:
+#define GPIO_INT                __DEPRECATED_MACRO GPIO_INT_ENABLE

-:2732: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2732: FILE: include/drivers/gpio.h:316:
+#define GPIO_INT_ACTIVE_LOW     __DEPRECATED_MACRO GPIO_INT_LOW_0

-:2747: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2747: FILE: include/drivers/gpio.h:331:
+#define GPIO_INT_ACTIVE_HIGH    __DEPRECATED_MACRO GPIO_INT_HIGH_1

-:2753: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2753: FILE: include/drivers/gpio.h:337:
+#define GPIO_INT_DOUBLE_EDGE    __DEPRECATED_MACRO GPIO_INT_EDGE_BOTH

-:2764: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2764: FILE: include/drivers/gpio.h:348:
+#define GPIO_POL_NORMAL		__DEPRECATED_MACRO GPIO_ACTIVE_HIGH

-:2770: WARNING:COMPLEX_MACRO: Macros with complex values should be enclosed in parentheses
#2770: FILE: include/drivers/gpio.h:354:
+#define GPIO_POL_INV		__DEPRECATED_MACRO GPIO_ACTIVE_LOW

-:4125: WARNING:LONG_LINE_STRING: line over 80 characters
#4125: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_config.c:210:
+		TC_PRINT("Board configuration does not allow to run the test\n");

-:4736: WARNING:MISORDERED_TYPE: type 'int unsigned' should be specified in [[un]signed] [short|int|long|long long] order
#4736: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_pin_interrupt.c:128:
+				   int unsigned int_flags)

-:4850: WARNING:LONG_LINE: line over 80 characters
#4850: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_pin_interrupt.c:242:
+	test_gpio_pin_interrupt_edge(GPIO_ACTIVE_HIGH, GPIO_INT_EDGE_TO_INACTIVE);

-:4852: WARNING:LONG_LINE: line over 80 characters
#4852: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_pin_interrupt.c:244:
+	test_gpio_pin_interrupt_edge(GPIO_ACTIVE_LOW, GPIO_INT_EDGE_TO_INACTIVE);

-:4880: WARNING:LONG_LINE: line over 80 characters
#4880: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_pin_interrupt.c:272:
+	test_gpio_pin_interrupt_level(GPIO_ACTIVE_HIGH, GPIO_INT_LEVEL_INACTIVE);

-:5076: WARNING:LONG_LINE: line over 80 characters
#5076: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:187:
+		port_set_masked_raw_and_verify(port, 1 << TEST_PIN, test_vector[i], i);

-:5124: WARNING:LONG_LINE: line over 80 characters
#5124: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:235:
+		port_set_masked_and_verify(port, 1 << TEST_PIN, test_vector[i], i);

-:5175: WARNING:LONG_LINE: line over 80 characters
#5175: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:286:
+		port_set_masked_and_verify(port, 1 << TEST_PIN, test_vector[i], i);

-:5186: WARNING:LONG_LINE: line over 80 characters
#5186: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:297:
+		port_set_masked_raw_and_verify(port, 1 << TEST_PIN, test_vector[i], i);

-:5240: WARNING:LONG_LINE: line over 80 characters
#5240: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:351:
+		port_set_masked_and_verify(port, 1 << TEST_PIN, test_vector[i], i);

-:5243: WARNING:LONG_LINE: line over 80 characters
#5243: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:354:
+		val_raw_expected |= (test_vector[i] & (1 << TEST_PIN)) ^ (1 << TEST_PIN);

-:5253: WARNING:LONG_LINE: line over 80 characters
#5253: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:364:
+		port_set_masked_raw_and_verify(port, 1 << TEST_PIN, test_vector[i], i);

-:5258: WARNING:LONG_LINE: line over 80 characters
#5258: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:369:
+		val_expected |= (test_vector[i] & (1 << TEST_PIN)) ^ (1 << TEST_PIN);

-:5382: WARNING:LONG_LINE: line over 80 characters
#5382: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:493:
+		port_set_clr_bits_raw(port, test_vector[i][0], test_vector[i][1], i);

-:5422: WARNING:LONG_LINE: line over 80 characters
#5422: FILE: tests/drivers/gpio/gpio_api_1pin/src/test_port.c:533:
+		port_set_clr_bits(port, test_vector[i][0], test_vector[i][1], i);

- total: 7 errors, 35 warnings, 5063 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

Your patch has style problems, please review.

NOTE: Ignored message types: AVOID_EXTERNS BRACES CONFIG_EXPERIMENTAL CONST_STRUCT DATE_TIME FILE_PATH_CHANGES MINMAX NETWORKING_BLOCK_COMMENT_STYLE PRINTK_WITHOUT_KERN_LEVEL SPLIT_STRING VOLATILE

NOTE: If any of the errors are false positives, please report
      them to the maintainers.



Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

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

This review is for the API changes only. I didn't have time time to look at the samples or driver changes.

include/dt-bindings/gpio/gpio.h Show resolved Hide resolved
include/dt-bindings/gpio/gpio.h Outdated Show resolved Hide resolved
include/dt-bindings/gpio/gpio.h Outdated Show resolved Hide resolved
drivers/gpio/gpio_handlers.c Outdated Show resolved Hide resolved
drivers/gpio/gpio_handlers.c Outdated Show resolved Hide resolved
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

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

Thank you for the updates! I've had some time to look at the use in drivers and applications and have some questions.

Keep up the great work!

@@ -91,18 +93,31 @@ static int gpio_gecko_configure(struct device *dev,
return -ENOTSUP;
}

if ((flags & GPIO_DIR_MASK) == GPIO_DIR_IN) {
if ((flags & GPIO_PUD_MASK) == GPIO_PUD_PULL_UP) {
if (flags & GPIO_OUTPUT) {
Copy link
Contributor

Choose a reason for hiding this comment

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

When I was looking at the flags, I wondered what the code would do when both GPIO_OUTPUT and GPIO_INPUT were set.

This driver respects output before input, but I don't see it documented that OUTPUT always has higher priority if both are set. Is that your intent?

I think it should be documented if so. Though my preference would be slightly for rejecting this as an invalid combination -- perhaps a common helper function, gpio_flags_are_valid(u32_t flags), could be added for this purpose?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There are several devices where it's perfectly valid and very useful to have a particular GPIO configured for input and output simultaneously. The API should allow that, on those devices.

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough. What are the intended semantics if the user requests both but the driver only supports one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Dunno; -ENOTSUP would seem to make sense, though -EINVAL is used here for a combination that would be acceptable on other platforms (GPIO_INT with GPIO_OUTPUTshould be fine if GPIO_INPUT is also set).

In this case the comments suggest that bidirectional is handled in GPIO_OUTPUT case, though I don't see how that works and it's not how I would expect it to be expressed for Nordic or SX1509B. In those input and output are simply independent functions.

The API documentation needs to be reviewed and updated to describe exactly what is and is not allowed, and what behavior the different combinations have to provide in order to meet the API's promises. GPIO_DEBOUNCE is one I noticed that still has about the same chance for misinterpretation as GPIO_POL_INV did in the current API.

Copy link
Contributor

Choose a reason for hiding this comment

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

-ENOTSUP sounds like a good plan to me for unsupported (but valid) combinations of flags.

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 bit of code was written specifically for the gecko GPIO module. In case any of the output modes are enabled the input will also be implicitly enabled. This is actually a fairly common approach among SoC vendors but surely not the only one. Application code that wants to read from the input must pass GPIO_INPUT flag to the configure function. In case of the gecko platform if it passes GPIO_OUTPUT only the input direction will still work. A portable application code shouldn't rely on this 'feature'. The gecko GPIO driver doesn't perform extra checks, e.g. the implementation of gpio_pin_get function doesn't check if the input direction was explicitly configured as this would increase function's code size, make it slower.

As for the GPIO_OUTPUT, GPIO_INPUT flag usage. If an application wants to write to the pin it should pass GPIO_OUTPUT flag to the configure function, if it intends to read from the pin it should pass GPIO_INPUT flag. If it intends to write and read it should pass both flags. If no flag is passed the pin will be disabled.

In general, if the function is not able to provide features requested by the configuration flags it should return -ENOTSUP.

I agree though we should better document error code / assert usage.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree though we should better document error code / assert usage.

That would be great if you don't mind. I really like your proposal of adding asserts into gpio.h itself in addition to docs. 👍

drivers/gpio/gpio_gecko.c Show resolved Hide resolved
drivers/gpio/gpio_sam.c Show resolved Hide resolved
Copy link
Contributor

@mbolivar mbolivar left a comment

Choose a reason for hiding this comment

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

requesting changes while semantic specification debate is ongoing

Copy link
Collaborator

@b0661 b0661 left a comment

Choose a reason for hiding this comment

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

I think to have a single function to configure GPIO interrupts and GPIO I/O characteristics is a misconception. There is a reason why Linux only provides GPIO DT bindings for the latter.

In my use case I have a SPI chip select handled by hardware (pin muxed to SPI) where I still want to get interrupts on an active chip select. If I would configure the interrupt by gpio_pin_configure() the pin would be muxed to the GPIO function.

Interrupt configuration should be done in the gpio_callback struct. (E.g. in STM32 interrupt generation and GPIO are even two distinct blocks.)

@pabigot
Copy link
Collaborator

pabigot commented Jun 9, 2019

Interrupt configuration should be done in the gpio_callback struct. (E.g. in STM32 interrupt generation and GPIO are even two distinct blocks.)

Without deep analysis this makes a lot of sense. Some drivers would have to make sure they preserve non-interrupt-related configuration as interrupts are enabled and disabled, but that shouldn't be too painful.

Depending on the model used to describe interrupt vs gpio configurations I can imagine that for some hardware the GPIO might have to be changed during the periods when interrupts are enabled (e.g. enable GPIO_INPUT to receive the signals), but should be restored when they're disabled (e.g. disable GPIO_INPUT for power savings). This could introduce a need for the driver to maintain pin-specific state internally rather than rely on what can be read back from peripheral registers. I don't have a problem with that in principle, and in fact it would make it trivial to add API to read the current GPIO configuration, something I've sometimes wanted to do.

Copy link
Member

@anangl anangl left a comment

Choose a reason for hiding this comment

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

LGTM.

My little doubt is only regarding flags with value of 0, like GPIO_INT_LEVELS_PHYSICAL or GPIO_INT_LEVEL.
Without checking the actual value of such flag, perhaps when modifying some code, one could try to use something like if (flags & GPIO_INT_LEVELS_PHYSICAL), which obviously won't work as expected. We need to support the flags of value 0 defined in DT bindings, but maybe we could get rid of these two (not add GPIO_INT_LEVELS_PHYSICAL and deprecate GPIO_INT_LEVEL)? Are they actually useful?

And I support the @b0661's proposal of moving the interrupt configuration flags to the callback structure. This would also allow creating generic parts of code that could enable/disable/reconfigure interrupts without having to know the actual pin configuration.

@mnkp
Copy link
Member Author

mnkp commented Jul 1, 2019

My little doubt is only regarding flags with value of 0, like GPIO_INT_LEVELS_PHYSICAL or GPIO_INT_LEVEL.
Without checking the actual value of such flag, perhaps when modifying some code, one could try to use something like if (flags & GPIO_INT_LEVELS_PHYSICAL), which obviously won't work as expected.

That's indeed a limitation of this approach. When writing a device driver one needs to always check the definition of the flag not to use a 'dummy' one. They shouldn't be used by the device drivers, only by the application level code. And in this case they make the code easier to read. The application level code will never want to perform any operation on these flags so there is no danger that the flag will be misused. It can be misused by the author of the device driver but this is entirely under our control, happens only once when writing the driver and should be caught when testing the code. I removed GPIO_INT_LEVELS_PHYSICAL flag but would prefer to leave the other flags as is.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Sep 17, 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.

static inline int gpio_pin_set(struct device *port, unsigned int pin, int value)
{
struct gpio_driver_data *const data =
(struct gpio_driver_data *const)port->driver_data;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You're making an immutable pointer to mutable data, which isn't very useful and looks weird because other pointers aren't treated that way. The fixup I provided made clear that the code should not be changing invert:

-       struct gpio_driver_data *const data = port->driver_data;
+       const struct gpio_driver_data *data =
+               (const struct gpio_driver_data *)port->driver_data;

3 occurrences if you want to change it.

mnkp and others added 6 commits September 18, 2019 05:30
This commit makes following changes to GPIO dt-bindings flags:
- Added GPIO_ACTIVE_LOW, GPIO_ACTIVE_HIGH to indicate pin active state.
- Added GPIO_OPEN_DRAIN, GPIO_OPEN_SOURCE to configure single ended pin
  driving mode.
- Added GPIO_PULL_UP, GPIO_PULL_DOWN flags.
- GPIO_INPUT, GPIO_OUTPUT to configure pin as input or output.
- Added GPIO_OUTPUT_LOW, GPIO_OUTPUT_HIGH flags to initialize output
  in low or high state.
- reworked GPIO_INT_* flags to configure pin interrupts.
- following flags were deprecated: GPIO_DIR_*, GPIO_DS_DISCONNECT_*,
  GPIO_PUD_*, GPIO_INT_ACTIVE_*, GPIO_INT_DOUBLE_EDGE, GPIO_POL_*.

To be aligned with Linux DTS standard any GPIO flags that should not be
used in DTS files are moved from include/dt-bindings/gpio/gpio.h file to
include/drivers/gpio.h with an exception of several old flags which
removal would cause DTS compilation errors. Those remaining old flags
will be removed from include/dt-bindings/gpio/gpio.h at a later stage.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
This commit adds following functions which work with pin logical levels
(take into account GPIO_ACTIVE_LOW flag):
- gpio_port_get, gpio_port_set_masked, gpio_port_set_bits,
  gpio_port_clear_bits, gpio_port_set_clr_bits
- gpio_pin_get, gpio_pin_set
Functions which work with pin physical levels:
- gpio_port_get_raw, gpio_port_set_masked_raw, gpio_port_set_bits_raw,
  gpio_port_clear_bits_raw, gpio_port_set_clr_bits_raw
- gpio_pin_get_raw, gpio_pin_set_raw
As well as functions:
- gpio_port_toggle_bits, gpio_pin_toggle_bits

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
This commit moves interrupt configuration for a single pin from
gpio_pin_configure to gpio_pin_interrupt_configure function.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Both pin sets and values encoding pin values are ultimately represented
by 32-bit unsigned integers. Provide typedefs that make the role of a
parameter explicit.

Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
gpio_api_1pin testcase verifies following new GPIO API functionality:
- pin active level flags
- input/output configuration flags
- pin drive flags
- gpio_port_*, gpio_pin_* functions
- pin interrupts

The test is using only 1 GPIO pin (defined in DTS as LED0) and relies on
the ability of the driver to configure pin simultanously as in/out.
Drivers that do not allow to configure pins in in/out mode should still
pass the test, however, most of the testcases will be skipped.

The test does not require any modifications to board hardware.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Update driver code and board files to use new GPIO configuration flags
such as GPIO_ACTIVE_LOW. Also add implementation of new port_* driver
API as well as gpio_pin_interrupt_configure function.

Tested on efr32_slwstk6061a board.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Update driver code and board files to use new GPIO configuration flags
such as GPIO_ACTIVE_LOW. Also add implementation of new port_* driver
API as well as gpio_pin_interrupt_configure function.

Tested on sam_e70_xplained board.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
mnkp and others added 5 commits September 18, 2019 06:24
Update driver code and board files to use new GPIO configuration flags
such as GPIO_ACTIVE_LOW. Also add implementation of new port_* driver
API as well as gpio_pin_interrupt_configure function.

Tested on nrf52840_pca10056 board.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Signed-off-by: Peter Bigot <peter.bigot@nordicsemi.no>
Update driver code and board files to use new GPIO configuration flags
such as GPIO_ACTIVE_LOW. Also add implementation of new port_* driver
API as well as gpio_pin_interrupt_configure function.

Tested on frdm_k64f board.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Update gpio_pin_configure() to take into account GPIO flags defined by
the devicetree.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Update gpio_pin_configure() to take into account GPIO flags defined by
the devicetree. Use gpio_pin_get/gpio_pin_set to verify reading/writing
of logical pin values. Use gpio_pin_interrupt_configure() to configure
interrupts.

Signed-off-by: Piotr Mienkowski <piotr.mienkowski@gmail.com>
Temp patch to see if this at least passes a full shippable build
or not.

Signed-off-by: Kumar Gala <kumar.gala@linaro.org>
@galak
Copy link
Collaborator

galak commented Sep 18, 2019

Pulled in dc2d4c0 and merged with the NRF commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Boards area: GPIO area: Samples Samples area: Tests Issues related to a particular existing or missing test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants