Added preliminary GPIO Subsystem. #25
Conversation
|
Thanks! GPIOs are a basic feature we've needed bindings for. I'll give this a more thorough look soon, but my initial impression is we might want to separate the safe bindings from the more ergonomic pin group API. In the zephyr crate, we could just provide fairly direct, but safe access to the syscalls, and then maybe have the rest in another crate. e.g. if gpio_pin_t can be greater than 32 depending on the gpio driver (I assume it can but I've never used Zephyr's GPIO interface), we should probably allow the user to supply a number directly as they would with the C API rather than constrain it to an enum at this level. Then if the enum-based wrapper is a better interface, that could be a layer above implemented in terms of the low-level safe bindings. |
|
Thanks for taking a first look .
The zephyr API uses a uint32 for the port pins, with each bit being a pin. Therefore it is constrained at 32 pins maximum. In general, my intend for this API was to allow for the same functionality and flexibility as the zephyr API (not withstanding something I missed). That said, the pin group functionality does restrict you somewhat, in order to prevent obvious mistakes like calling set on en input pin etc. Separating a basic API and the pin group should be easy enough though. My suggestion would be implementing the port based API on a lower level and wrapping that inside the pin group API. I would advise against moving away from the enums though, unless we encounter a situation where it clearly is constraining. It makes the possible inputs for the functions a whole lot clearer. |
| P31, | ||
| } | ||
| impl Pin { | ||
| //FIXME better solution for this abomination |
There was a problem hiding this comment.
I use this crate for this.
| } else { | ||
| let driver_data: *const raw::gpio_driver_data = | ||
| self.0.data as *const raw::gpio_driver_data; | ||
| Ok(unsafe { (*driver_data).invert }) |
There was a problem hiding this comment.
I get a compile error on Zephyr 2.3 because the device private data field is named differently.
763 | self.0.data as *const raw::gpio_driver_data;
| ^^^^ unknown field
|
= note: available fields are: `name`, `config_info`, `driver_api`, `driver_data`
I assume the inversion is to account for active low pins. We should use gpio_port_get instead of the _raw versions so we're not dependent on internal data structures. I see that probably wasn't possible because those functions are inline and not syscalls, so they don't get bindings.
We can create our own thunks for functions that need to be real symbols to supplement the ones automatically generate for syscalls. If you look at the generated file here, you can see how those are defined to emit a real function for each context.
./build/zephyr/include/generated/syscall_thunks.c
#if defined(__ZEPHYR_SUPERVISOR__)
int z_kernelctx_gpio_port_get_raw(struct device * port, gpio_port_value_t * value)
#elif defined(__ZEPHYR_USER__)
int z_userctx_gpio_port_get_raw(struct device * port, gpio_port_value_t * value)
#else
int z_anyctx_gpio_port_get_raw(struct device * port, gpio_port_value_t * value)
#endif
{
return gpio_port_get_raw(port, value);
}
We could manually create something like the above for the missing gpio functions, maybe call it gpio_thunks.c and include it from syscall-thunk-{any,user,kernel}.c
| const PINS_0_TO_7 = 0x000000FF; | ||
| const PINS_8_TO_15 = 0x0000FF00; | ||
| const PINS_16_TO_23 = 0x00FF0000; | ||
| const PINS_24_TO_31 = 0xFF000000; |
There was a problem hiding this comment.
Do you expect clients will use these group definitions? Maybe we should keep the initial API minimal.
| Err(io::Error::from_raw_os_error(ENOTSUP)) | ||
| } else { | ||
| unsafe { | ||
| let api = self.0 .0.api as *const raw::gpio_driver_api; |
There was a problem hiding this comment.
Similarly api is not defined on 2.3.
|
Is this PR still worked on? Or is GPIO support currently not planned? |
|
@konradmoesch I don't personally have a need for GPIOs, so I haven't put in the effort to finish this up. |
I have created a subsystem around the Zephyr GPIO driver.
It includes support for interrupt callbacks, individual pin manipulation and masked pin manipulation.
The API centers around PinGroups as I call them, This is simply a mask of pins with a common usage, and can be one pin. To allow for flexibility, the API allows merging them back if needed.
Right now these PinGroups are not fixed to a specific port however. A limitation puts some burden on the user when combining different pin groups and is something I would like to fix in the future when I see a good way to implement that using the type system.
Any comments or feedback is welcome.