zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.49k stars 6.42k forks source link

GPIO: need generic API for optimized pin read/write operations #11917

Open pabigot opened 5 years ago

pabigot commented 5 years ago

I think we need macro-based APIs to avoid function call overhead to set and clear bits, which allow SoC-specific code to provide small and fast assembly implementations. This would be needed for time-sensitive bit banging protocol implementations.

Originally posted by @mbolivar in https://github.com/zephyrproject-rtos/zephyr/issues/2288#issuecomment-433343055

pabigot commented 5 years ago

I'm seeing dozens to nearly a hundred clock cycles for a plain GPIO set or clear on several platforms when using gpio_pin_write(): nrf51, nrf52, k64f, and l476rg. All of these should be able to be toggle an output with a couple instructions.

pfalcon commented 5 years ago

I think we need macro-based APIs to avoid function call overhead to set and clear bits, which allow SoC-specific code to provide small and fast assembly implementations.

Need for what? Setting/clearing bits in Zephyr works as follows:

  1. A system call is issued, a relatively expensive operation (CPU exception, saving of all/many registers).
  2. MMU context is replaced.
  3. A driver is called via indirect call.
  4. A driver for example performs pin operation via a GPIO extender connected over I2C.

It's unclear how to reconcile this view of Zephyr with a fact that specific SoCs has a way to toggle a limited subset of pins (builtin into SoC) using a couple of instructions.

pabigot commented 5 years ago

@pfalcon That's a fair point; I'm doing this in main() and probably shouldn't expect maximum speed in all cases. I've been comparing against direct use of the Nordic memory-mapped GPIO peripheral structures in the same code, which work and are much faster, so it's at least theoretically possible on some architectures in some contexts.

I suspect @mbolivar's original use case of bit-banged protocols implemented in a driver, might be different. Or not. I'm not clear yet on where all the privilege levels get transitioned and how.

pfalcon commented 5 years ago

@pabigot, so my point is that we need to keep that "wannabe a real protected OS" shift-face of Zephyr in mind. A usecase of "no kernel/userspace split" aka "kernel mode only" definitely exists, but I don't see organized stackholders for that mode unfortunately, whereas a need to support userspace mode and syscalls rampages thru Zephyr. And maintaining 2 sets of APIs requires an organized effort, otherwise we just spread too thin.

I suspect @mbolivar's original use case of bit-banged protocols implemented in a driver, might be different. Or not. I'm not clear yet on where all the privilege levels get transitioned and how.

Yes, that's what I'm asking abot too. If the idea is that those are just internal helper macros to allow to implement kernel-level bitbanging drivers, that's fine, just need to define that clearly. But I don't see bitbanging drivers to be implemented too much in Zephyr (again, maybe the interest is just ramping up).

carlescufi commented 5 years ago

This was briefly discussed with @xnlcasad and @galak at ELCE this year. I agree with @pabigot with the fundamental value in the proposition.

carlescufi commented 5 years ago

@pfalcon

Setting/clearing bits in Zephyr works as follows:

Userspace and kernel separation, aka CONFIG_USERSPACE is optional in Zephyr. So I see 2 ways forward with this:

a) Propose a new set of macro or codegen based-APIs that work only when userspace is disabled b) Allow GPIO access directly from the application in the context of a 'userspace driver'

Paging @andrewboie as the userspace maintainer

andrewboie commented 5 years ago

I don't see organized stackholders for that mode unfortunately, whereas a need to support userspace mode and syscalls rampages thru Zephyr

I have no idea what you are talking about. We surely support supervisor-only use-cases as a first class citizen in Zephyr. Rampages? Really?

Need for what? Setting/clearing bits in Zephyr works as follows:

A system call is issued, a relatively expensive operation (CPU exception, saving of all/many registers).
MMU context is replaced.
A driver is called via indirect call.
A driver for example performs pin operation via a GPIO extender connected over I2C.

The first two steps are skipped if either CONFIG_USERSPACE is disabled, or the caller isn't running in user mode. There's no overhead to this feature if you don't use it.

Propose a new set of macro or codegen based-APIs that work only when userspace is disabled

I'm 100% ok with this option.

pfalcon commented 5 years ago

Rampages? Really?

Yep, we need to adjust many APIs for userspace constraints like no callbacks, excessive argument introspection, checks, and copying (just recently I had to remember why Linux developers don't like ol' good ioctl(), and I have no idea what concerns will be raised for implementation of it as a syscall in Zephyr), and non-obvious API implications like described in https://github.com/zephyrproject-rtos/zephyr/issues/10911. Skipping long boring talk like that, "rampages" is a good word to describe it, YMMV ;-).

I'm 100% ok with this option.

Well, that's good to know! But how to organize it all and support so different configurations, e.g. how to even document them well? I'm glad to know there're people who want to work on that. And we aren't getting qemu_x86 back into nice simple cozy CONFIG_USERSPACE=n world, are we?

andrewboie commented 5 years ago

Yep, we need to adjust many APIs

We didn't change the driver subsystem APIs when we added system calls. We exposed a lot of the APIs to userspace, but they remain the same as they were before.

no callbacks

You think it's a good idea to allow user mode to register callbacks that run in interrupt context? You can still register them in supervisor mode.

excessive argument introspection, checks, and copying (just recently I had to remember why Linux developers don't like ol' good ioctl(), and I have no idea what concerns will be raised for implementation of it as a syscall in Zephyr)

Excessive? You are aware this is a security feature, right? What checks would you like to remove?

Well, that's good to know! But how to organize it all and support so different configurations, e.g. how to even document them well?

Don't expose them as system calls...this ticket becomes completely orthogonal to userspace.

and non-obvious API implications like described in #10911.

Non-obvious to people who can't be bothered to read the documentation I suppose.

And we aren't getting qemu_x86 back into nice simple cozy CONFIG_USERSPACE=n world, are we?

Huh? Are you still under the impression that enabling this config forces all threads to run in user mode or something?

mbolivar commented 5 years ago

Hi there. I want fast GPIO bit-banging APIs behind a standard Zephyr interface in particular to implement efficient LED strip protocols. Some chips, such as the WS2812 (https://wp.josh.com/2014/05/13/ws2812-neopixels-are-not-so-finicky-once-you-get-to-know-them/), have one-wire protocols with tight timing requirements.

I currently implement those in a generic way using SPI, but it's a hack (which has required additional hacks, such as bdde886ed59e993b6839780e57f6ed961d1613aa). There's a bit-banging driver (drivers/led_strip/ws2812_sw.c) also, but it's nRF-only.

I would like to be able to implement a driver in Zephyr using something similar to what the FastLED (http://fastled.io/) library implements in C++ in an Arduino context: code which compiles down to a few assembly instructions to hit bit set / reset registers directly and efficiently, which can be called in a loop with interrupts disabled to blit a line of lights.

So no, this has nothing to do with userspace.

pfalcon commented 5 years ago

Yep, we need to adjust many APIs

We didn't change the driver subsystem APIs when we added system calls. We exposed a lot of the APIs to userspace, but they remain the same as they were before.

Well, so I don't think it's a good approach in general. But otherwise, what I meant is probably "adjust design of APIs", first of all applied to new APIs. And per above, I don't even understand fully what design adjustment repertoire is needed in general, still learning (why I bring up such discussions).

You think it's a good idea to allow user mode to register callbacks that run in interrupt context?

Absolutely not. Actually, I rely on that as a litmus tests whether an API is general enough or, well, "kernel mode only". And actually, seeing that you're kinda flexible on these matters, I start to worry that something might be cooking, and one of these days you may submit PR "support callbacks into usermode". That would crumble down whatever criteria I've learned so far re: kernel vs userspace APIs. Because otherwise, everything is implementable. Linux somehow implements signal handlers ;-). And to clarify again: I really think that using "normal IPC" for "native Zephyr API" is better than trying to implement callbacks (which we may need one day anyway for deeper aspects of POSIX subsys).

Excessive? You are aware this is a security feature, right? What checks would you like to remove?

None. I need to implement quite a lot of such checks for syscalls which I in turn yet need to implement. That quite complicates my developer's life ;-). Sorry for whining again ;-).

Huh? Are you still under the impression that enabling this config forces all threads to run in user mode or something?

Nope, only that with a config like that, absolutely nobody dealing with Zephyr can escape the need to keep usermode in mind when designing APIs. If we find maintaining dichotomies of kernel-only vs userspace-capable APIs a suitable approach, fine. Worth to have been discussed IMHO.

andrewboie commented 5 years ago

And per above, I don't even understand fully what design adjustment repertoire is needed in general, still learning (why I bring up such discussions).

Since you're still learning, is antagonistic rhetoric like "rampage" really necessary when by your own admission you don't fully understand this feature or the design decisions that went into it?

And actually, seeing that you're kinda flexible on these matters, I start to worry that something might be cooking, and one of these days you may submit PR "support callbacks into usermode". That would crumble down whatever criteria I've learned so far re: kernel vs userspace APIs. Because otherwise, everything is implementable. Linux somehow implements signal handlers

Yes, we'd have to implement something like signals. Nothing cooking on this front at the moment. Mostly because the workaround (installing callbacks at app startup before dropping to user mode) doesn't give anyone any heartburn.

Nope, only that with a config like that, absolutely nobody dealing with Zephyr can escape the need to keep usermode in mind when designing APIs.

People should be free to ignore user mode when designing new APIs for Zephyr is what you are saying? I mean, they can, but such APIs may not be availble to user mode. I'm struggling to understand what the problem is here. Other than you don't like user mode because it isn't needed for your particular use-cases and would rather it simply wasn't there at all.

btw, if you actually look at boards/x86/qemu_x86_defconfig user mode isn't turned on there. It is however turned on when CONFIG_TEST is enabled for all of our test cases.

pfalcon commented 5 years ago

So no, this has nothing to do with userspace.

Sounds good, but expect questions like this: https://github.com/zephyrproject-rtos/zephyr/issues/10726 , multiplied and amplified as Zephyr community grows (there's a similar discussion on user list now too, so it's fair to say that UART APIs has got their dose of confusion already, from which we can learn already or not yet).

pfalcon commented 5 years ago

People should be free to ignore user mode when designing new APIs for Zephyr is what you are saying?

No, I'm saying the opposite - ideally, there should be a single API which works in both modes. Designing kernel-only API should be an exception with very well understood usecases. Otherwise we get maintenance, documentation, and community confusion issues.

mbolivar commented 5 years ago

Sounds good, but expect questions like this:

10726 , multiplied and amplified as Zephyr community grows (there's a similar discussion on user list now too, so it's fair to say that UART APIs has got their dose of confusion already, from which we can learn already or not yet).

I hope @andrewboie will correct me if I am wrong, but I wouldn't expect the GPIO-specific APIs I commented about to be available to userspace -- can't have userspace touching device registers directly IIUC.

My dream is that in-kernel drivers which use GPIOs can do so more efficiently while still using generic APIs if they need to. I believe I've given an example where this is needed.

Are you opposed to the idea in general, or have some other concern(s)?

pfalcon commented 5 years ago

I hope @andrewboie will correct me if I am wrong, but I wouldn't expect the GPIO-specific APIs I commented about to be available to userspace -- can't have userspace touching device registers directly IIUC.

I'm not sure if I explain myself not clear enough, especially given that @andrewboie criticizes me for using too strong words like "rampage" (though I don't see anything "antagonistic" in it, but ok).

So, let me be clear - you wouldn't expect these GPIO APIs to be available to the userspace, but users will. So, please be prepared to document them in such a way as to make it very clear where they work and where they don't.

pfalcon commented 5 years ago

btw, if you actually look at boards/x86/qemu_x86_defconfig user mode isn't turned on there

@andrewboie, disclaimer: I'm not trying to be antagonistic. But you just made my hair turn gray, on the suspicion that I hallucinated things here https://github.com/zephyrproject-rtos/zephyr/issues/10972#issuecomment-436738737 , and well, large part of (gone offtopic definitely) content here.

How, how a commit like 1c5642a40248c1e57db672576c6bf66f2b08353c could have been done? Let me be short and polite - please turn it back, or it never should have been turned on in the first place. But now we need to have it on, if we want all the stuff being discussed here to be paid attention by the people, or we'll have wildest regressions re: userspace functionality.

andrewboie commented 5 years ago

But now we need to have it on, if we want all the stuff being discussed here to be paid attention by the people, or

CONFIG_TEST turns it on. This gets enabled by all of Zephyr's unit tests. There isn't a problem here. This includes the socket tests.

The x86 emulator isn't the right place to enable it. For one thing, we have to test on ARM and ARC too, which also implement user mode. And we need to test on real hardware as well.

we'll have wildest regressions re: userspace functionality.

No, we won't. Have you looked at tests/Kconfig?

andrewboie commented 5 years ago

I hope @andrewboie will correct me if I am wrong, but I wouldn't expect the GPIO-specific APIs I commented about to be available to userspace -- can't have userspace touching device registers directly IIUC.

I'm not sure if I explain myself not clear enough, especially given that @andrewboie criticizes me for using too strong words like "rampage" (though I don't see anything "antagonistic" in it, but ok).

"wannabe" was another cute one

tbursztyka commented 5 years ago

a) Propose a new set of macro or codegen based-APIs that work only when userspace is disabled

I agree with @carlescufi proposal. To precise a bit: I guess you mean that the macros are unusable in userspace and not when CONFIG_USERSPACE is set? Either way, kernel space will be able to use these new macros right?

lpereira commented 5 years ago

Just some food for thought: the first few versions of the Arduino programming environment provided functions such as digitalWrite(int pin, bool value) and digitalRead(int pin). The pin parameter in these functions had to be looked up in runtime, to match the pin number in an Arduino board to the actual bit inside one of the PORTx registers on an AVR. This runtime lookup of course had a cost, and in many cases, was completely useless, because the Arduino model (and, thus, the microcontroller being used) is a compile-time setting.

What they did in the end was providing a macro-based version of digitalWrite() and digitalRead(), without changing the API at all, that, if called with a compile-time-constant pin (by testing with the GCC extension __builtin_constant_p() or something like that), would perform the "lookup" in compile time, generating a simple PORTn |= 1<<some_constant line; if pin wasn't considered a compile-time constant, the original function with runtime pin-to-port-and-bit-position was called.

xnlcasad commented 5 years ago

I think that a 'one size fits all' solution may prove a challenging goal for any software project. Getting all the nice abstractions with zero runtime cost while still catering for userspace, no-userspace, argument marshalling, and a plethora of hardware variations... not an easy task IMO.

For the record, after the conversation with @carlescufi @erwango and @galak at ELCE I scrambled the code that Zephyr generates when toggling a gpio on a nrf52 SoC with CONFIG_USERSPACE=n

The call to gpio_pin_write() assembles to some 38 instructions. Compare this to the 3 instructions required to do the job.

The question for me is: Is it worth to go thru more macro/code generation magic to solve it ? It seems it depends on the final use case.

Just because I've been recently looking at cpp templates and as I see that @pfalcon was involved in a cpp peripheral template driver, I dare to mention here that IMO cpp shines in its capabality to use compile time abstractions that neatly solve the use case at hand. Here is an equivalent example to the above gpio_pin_write(). Using templates to create static types which encode all of the hardware constants in the type itself is a very powerful technique.

pabigot commented 5 years ago

@xnlcasad +many.

A lot of Zephyr's APIs appear to be designed around least common denominator, making them suitable for toy applications but not real-world products. The sensor API is one. I'm still formulating my thoughts on this, though, so don't want to start a digression.

Also C++: Prior to discovering Zephyr I did a complete C++-17 framework for Nordic devices (NRF51 and NRF52) using nothing but the CMSIS headers. Templates, CRTP, constexpr functions: all this allows me to write generic code that at runtime maps down to a couple instructions that operate directly on the peripheral registers.

Currently Zephyr forbids use of C++ outside of applications, even though there is no technical reason I can see why it can't be used in drivers as long as it avoids certain features (exception, dynamic memory allocation, etc.). The more time I spend with things like the GPIO and ADC infrastructures the more strongly I'm tempted to port over what I did for that project.

carlescufi commented 5 years ago

@pabigot

A lot of Zephyr's APIs appear to be designed around greatest common denominator, making them suitable for toy applications but not real-world products.

I agree with the premise in general. Some APIs have been updated to try and cover the advanced functionality of the different underlying hardware variations, but there are many that still require work. Our weekly API meetings help, but we need more contributions in order to move at a swifter pace.

The sensor API is one. I'm still formulating my thoughts on this, though, so don't want to start a digression.

I've copied you on the relevant issue there.

Currently Zephyr forbids use of C++ outside of applications, even though there is no technical reason I can see why it can't be used in drivers as long as it avoids certain features (exception, dynamic memory allocation, etc.). The more time I spend with things like the GPIO and ADC infrastructures the more strongly I'm tempted to port over what I did for that project

The issue I see there is forcing C++ onto users. Depending on the toolchain and application this might be a burden, and this is also one of the reasons that the APIs need careful consideration. But this is the second time this suggestion has been posted now, so I think it deserves more attention.

pfalcon commented 5 years ago

The call to gpio_pin_write() assembles to some 38 instructions. Compare this to the 3 instructions required to do the job.

And that's too much! With bitbanding as supported by many (but not all) ARMs, that would be 1 instruction to set a pin to an exact value.

Just because I've been recently looking at cpp templates and as I see that @pfalcon was involved in a cpp peripheral template driver

Thanks for bringing it up ;-). Nowadays, I guess one would argue for need to go the Rust way: https://github.com/rust-embedded/awesome-embedded-rust .

All in all, I jumped on this thread because I'm an efficiency geek and very, very love this stuff. Tell me that something costs less cycles and less bytes - and I'm ready to vote it up no matter what it is. So, it was an act of self-policing and "raising awareness" that there are hidden costs with adding adhoc APIs, like maintenance, documentation, community confusion.

And it's rather ironic that while arguing for possible confusion, I showed that I'm completely confused myself about another Zephyr feature - "userspace". So, it's not just one-off GPIO case. Surely, we need those macros. But how all those features, gathered together, make Zephyr a coherent, easy to use system is an open question.

mbolivar commented 5 years ago

What they did in the end was providing a macro-based version of digitalWrite() and digitalRead(), without changing the API at all, that, if called with a compile-time-constant pin (by testing with the GCC extension __builtin_constant_p() or something like that), would perform the "lookup" in compile time

@lpereira I think an approach like that is probably workable in my use case, as long as the pin I'm bit-banging is a compile-time constant provided in a DTS overlay or so (which the driver implementation could ensure).

endian-benjamin commented 5 years ago

Userspace/kernelspace expectancy confusion seems like a complete non-issue - just expose the "unsafe" API in a clearly named header. gpio_THAT_BRIXES_UR_BOARDZ.h perhaps

mbolivar commented 5 years ago

Moving a comment over from https://github.com/zephyrproject-rtos/zephyr/issues/15611....

it would also be useful for other device drivers that need fast access to GPIOs built into the SoC to have a generic API for accessing the output registers for these devices.

@mnkp @pabigot to follow up on our brief discussion today, I thought I would look at all the existing GPIO drivers and see what types of registers they use. From there, I think we can see more clearly what this would look like in practice.

Here are my results (plain text formatted). Comments/corrections welcome; I did this by inspection:

Register type glossary
======================

bit set/clr
    distinct set and clear registers.

    set register: write 1 to a bit to set pin to active, writes of 0
    are ignored.

    clear register: write 1 to a bit to set pin to inactive, writes of 0
    are ignored.

input
    one-pin-per-bit readable data register; this may be the same as
    the output register

other
    pin access register is not any of the other types in this section

output
    one-pin-per-bit writeable data register; this may be the same as
    the input register

pin
    write some value to the register to set pin to active, another to
    inactive; i.e. writes to a register affect one pin only, and are
    not one-pin-per-bit

N/A
    reads or writes are not supported or not done with MMIO registers

Zephyr GPIO driver register types
=================================

Columns:

- No.: number
- Driver: driver name in zephyr/drivers/gpio
- "BSC": has bit set/clr register ("X" = yes, " " = no)
- O: has output register
- I: has input register
- Other: other register types and footnotes

No. Driver        BSC O I    Other
--- --------------------  --- - -    -----
 0. gpio_altera_nios2.c   X   X X    [1]
 1. gpio_atmel_sam3.c     X     X
 2. gpio_cc13xx_cc26xx.c  X     X    pin [2]
 3. gpio_cc2650.c         X     X
 4. gpio_cc32xx.c             X X
 5. gpio_cmsdk_ahb.c          X X
 6. gpio_dw.c             X     X
 7. gpio_esp32.c          X     X
 8. gpio_gecko.c          X   X X    [4]
 9. gpio_ht16k33.c                   special-purpose I2C led driver
10. gpio_imx.c                X X
11. gpio_intel_apl.c                 pin input and output
12. gpio_mchp_xec.c           X X
13. gpio_mcux.c           X   X X    [5]
14. gpio_mcux_igpio.c         X X
15. gpio_mcux_lpc.c             X    pin/other output [6]
16. gpio_mmio32.c             X X
17. gpio_nrfx.c           X   X      [7], for input see [8]
18. gpio_pcal9535a.c                 N/A [9]
19. gpio_qmsi.c           X     X
20. gpio_qmsi_ss.c           other? [10]
21. gpio_rv32m1.c         X   X X    [11]
22. gpio_sam0.c           X   X X    [12]
23. gpio_sam.c            X     X
24. gpio_sch.c            X     X    in some configurations only [13]
25. gpio_sifive.c             X      other input [8]
26. gpio_stellaris.c          X X
27. gpio_stm32.c          X   X X    [14]
28. gpio_sx1509b.c                   N/A [9]

[1] driver uses bit set/clr; but output register exists per Intel UG 01085
    27.5.2.1. ("data Register")

[2] driver uses bit set/clr; in
    zephyr/ext/hal/ti/simplelink/source/ti/devices/cc13x2_cc26x2/driverlib/gpio.h,
    GPIO_WriteDio() shows pin control register exists

[3] this is at least an input register; unclear to me if it's a data
    register

[4] driver uses bit set/clr, but output exists per 32.5.4 in
    https://www.silabs.com/documents/public/reference-manuals/EFM32WG-RM.pdf

[5] driver uses bit set/clr, but PDOR also referenced and seems to be an
    output register for compatible devices, see e.g. K64P144M120SF5RM
    55.2.1 Port Data Output Register (GPIOx_PDOR)

[6] "other": driver uses contiguous byte-sized per-pin registers.
    However, UM10914 11.5.2 GPIO port word pin registers describes
    individual pin control registers

[7] driver uses bit set/clr, nrf_gpio.h shows nrf_gpio_port_out_write()

[8] values from multiple registers are combined depending on pin's
    directionality; could be simplified if we assume input mode

[9] I2C GPIO expander

[10] from ARC-related comments and
     modules/hal/qmsi/drivers/gpio/qm_ss_gpio.c, it looks like this
     may be non-MMIO registers

[11] driver uses bit set/clr, but PDOR exists as in gpio_mcux.c
     (looks like same/similar IP)

[12] driver uses bit set/clr, but
     zephyr/ext/hal/atmel/asf/sam0/include/samd20/component/port.h
     declares PORT_OUTSET_Type as R/W

[13] there's a "legacy" I/O port access type in some configurations

[14] driver uses bit set/clr, but every ST part I've ever seen has an
     ODR (output data register) per-port which can also be used

From this, I think it's clear that if we provide "write register" accessors for both bit set/clr registers and output registers, as well as one "read register" accessor, we can cover almost every interesting case.

The write register APIs could look something like this:

/**
 * @brief Get a GPIO port's output register.
 *
 * Bits in the output register correspond to individual pins. Setting
 * a bit makes the pin active, and clearing it makes it inactive.
 *
 * @param dev GPIO device to get a reference to the output register for
 * @param output_reg on success, *output_reg contains the output register's address
 *
 * @return 0 on success, -ENOTSUP if not supported by this device.
 */
int gpio_get_output_register(struct device *dev, uintptr_t *output_reg);

/**
 * @brief Get a GPIO port's bit set register address.
 *
 * Bits in the bit clear register correspond to individual
 * pins. Writing 1 to a bit a bit makes the pin active; writes of zero
 * are ignored.
 *
 * @param dev GPIO device to get a reference to the bit set register for
 * @param bit_set_reg on success, *bit_set_reg contains the bit set register's address
 *
 * @return 0 on success, -ENOTSUP if not supported by this device / pin.
 */
int gpio_get_bit_set_register(struct device *dev, uintptr_t *bit_set_reg);

/**
 * @brief Get a GPIO port's bit clear register address.
 *
 * Bits in the bit clear register correspond to individual
 * pins. Writing 1 to a bit a bit makes the pin active; writes of zero
 * are ignored.
 *
 * @param dev GPIO device to get a reference to the bit clear register for
 * @param bit_clr_reg on success, *bit_clr_reg contains the bit clear register's address
 *
 * @return 0 on success, -ENOTSUP if not supported by this device / pin.
 */
int gpio_get_bit_clear_register(struct device *dev, uintptr_t *bit_clr_reg);

On the output side, driver authors would need to try both gpio_get_output_register() and gpio_get_bit_set_register() / gpio_get_bit_clear_register() as appropriate for the use case. For quick hacks or applications that know which one their SoC supports, you could just call the right one.

On the input side, a single gpio_get_input_register() seems to cover all the drivers where something like this is possible.

sslupsky commented 4 years ago

Regarding the gpio, has any thought been given to configuration of the gpio? That is, you can typically configure an output as digital common drain, p or n open drain, analog or high impedance. Inputs can be configured as digital input or analog input. Inputs and outputs can have additional attributes such pull up and pull down.

mbolivar commented 4 years ago

Regarding the gpio, has any thought been given to configuration of the gpio?

This issue is specifically about optimized read/write, so it's not the right place to ask this question. Please refer to https://github.com/zephyrproject-rtos/zephyr/issues/15611 instead.

You can also see the new gpio.h API in the topic-gpio branch of the zephyr repository for full details.

sslupsky commented 4 years ago

I will have a look at that issue, thank you.

However, I disagree that this issue is not related. In fact, it Is desirable to have optimized changes of state from the various output or input capabilities of a GPIO. That is, the GPIO states embody more than “low” and “high” and this API should reflect that.

mbolivar commented 4 years ago

That is, the GPIO states embody more than “low” and “high” and this API should reflect that.

The API operates on logical levels, low and high. You are talking about pin configuration; it's not to be discussed here.

pabigot commented 4 years ago

API 2020-07-07: Moved from triage to on-hold. Open for 18+ months, agreed useful, nobody has intent to work it.

nzmichaelh commented 4 years ago

+1 to an optimised read/write/toggle interface. The Itsybitsy M4 Express has a DotStar LED on an SPI-like interface but it's not routed to a physical SPI port. I'd like to do a generic software only SPI driver but it needs write and toggle operations.

lbdroid commented 3 years ago

+1 to an optimised read/write/toggle interface. The Itsybitsy M4 Express has a DotStar LED on an SPI-like interface but it's not routed to a physical SPI port. I'd like to do a generic software only SPI driver but it needs write and toggle operations.

Its connected to PB02/PB03, which can be connected to SERCOM5 pads 0 and 1. How is that not a physical SPI port?