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

[stm32] Add facilities for type-erased gpio manipulation #543

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

chris-durand
Copy link
Member

@chris-durand chris-durand commented Jan 26, 2021

This adds the following:

  1. A type-erased wrapper for Gpios:
GpioAccessor pin = GpioAccessor::template fromGpio<GpioA1>();
pin.toggle();

This is implemented on STM32 register level. It stores the GPIOPort instance pointer and a 16-bit pre-computed bit value.
Thus, this is quite efficient.

  1. An extended version of SoftwareGpioPort providing single pin access with runtime indexing:
using Port = RandomAccessPort<GpioB1, GpioC3, GpioD6>;
Port::setOutput();

// Enable output with index 2 => GpioB1
Port::set(2);

Port port;
GpioAccessor pin = port[0];
pin.set(); // Enable D6

// toggle all pins
for(auto pin : port)
    pin.toggle();

The pin wrapper is a by-product of implementing the port class. My use-case is setting and reading Gpios from a sequence with a runtime index received from a communication link. I couldn't get reasonable code generation with a language-level solution on top of GpioXX classes. I presume this could be useful for others as well.

TODO:

  • Test in hardware
  • Add hardware unit test

@salkinium
Copy link
Member

Very cool! This should still be very fast.

i'm quite unhappy with our lack of dynamic peripheral support and lots of code is unnecessarily duplicated by our literal code gen, even though the Peripheral Instances are often of the exact same type or compatible (sub-)layout (like some of the timers). We should remove the lbuild code gen for a more traditional C++ approach, since I don't see a performance boost for our current approach at all. Although we will still need lbuild for generating the interrupts for UART etc.

Ideally I'd like to keep the complicated things like baudrate and connect stuff in compile time, which calls into a platform-specific inline, but non-generated API. That way you can still bit-bang individual IOs or entire ports really fast, but also use runtime indexing transparently.

The main issue holding us back is AVRs, since their registers are not friendly to "algorithmic compression" with their random bit stuffing into registers of different devices. But I would suggest we just ignore that and move on with Cortex-M.

Then on top of that we can have a (minimal) virtual interface that is used by non-platform specific drivers. Together with fibers that would allow to transparently use IO expanders in a non-blocking way (not possible right now). For some of the really fast IO expanders over SPI you could probably even bit-bang I2C (!) fairly transparently.

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

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

Merge?
(Even without unit test for now?)

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

Yes!

@chris-durand
Copy link
Member Author

We can consider merging, if you are fine with having no support for GpioInverted with this. It could be done at either some space or time overhead. I am very busy right now, but intend to add the test in the future.

@chris-durand
Copy link
Member Author

I guess I would run it on a Nucleo tomorrow night for testing again. That could be turned into a tiny unit test. Let's see then, ok?

@salkinium
Copy link
Member

salkinium commented May 11, 2021

Note that inspired by this code I updated the GPIO wrapper class used in my ELVA project, which was implemented using virtual functions, to now instead use a runtime computation and it shaved off about a dozen kB of Flash.

I need to test this more thoroughly and perhaps check how AVRs fare with this API style, however, for STM32/Cortex-M this is still pretty efficient:

ASM listings

set()

08005246 <_ZN4elva11VirtualGpio3setEb>:
_ZN4elva11VirtualGpio3setEb():
./elva/src/elva/gpio/virtual_gpio.hpp:103
		port()->BSRR = mask() << ((isInverted xor value) ? 0 : 16);
 8005246:	79c2      	ldrb	r2, [r0, #7]
_ZNK4elva11VirtualGpio4maskEv():
./elva/src/elva/gpio/virtual_gpio.hpp:164
	inline uint32_t mask() const { return pinMask; }
 8005248:	8883      	ldrh	r3, [r0, #4]
_ZN4elva11VirtualGpio3setEb():
./elva/src/elva/gpio/virtual_gpio.hpp:103
		port()->BSRR = mask() << ((isInverted xor value) ? 0 : 16);
 800524a:	428a      	cmp	r2, r1
 800524c:	bf0c      	ite	eq
 800524e:	2110      	moveq	r1, #16
 8005250:	2100      	movne	r1, #0
_ZNK4elva11VirtualGpio4portEv():
./elva/src/elva/gpio/virtual_gpio.hpp:163
	inline GPIO_TypeDef* port() const { return reinterpret_cast<GPIO_TypeDef*>(portAddress); }
 8005252:	6802      	ldr	r2, [r0, #0]
_ZN4elva11VirtualGpio3setEb():
./elva/src/elva/gpio/virtual_gpio.hpp:103
		port()->BSRR = mask() << ((isInverted xor value) ? 0 : 16);
 8005254:	408b      	lsls	r3, r1
 8005256:	6193      	str	r3, [r2, #24]
./elva/src/elva/gpio/virtual_gpio.hpp:104
	}
 8005258:	4770      	bx	lr

read():

08005232 <_ZNK4elva11VirtualGpio4readEv>:
_ZNK4elva11VirtualGpio4portEv():
./elva/src/elva/gpio/virtual_gpio.hpp:163
	inline GPIO_TypeDef* port() const { return reinterpret_cast<GPIO_TypeDef*>(portAddress); }
 8005232:	6803      	ldr	r3, [r0, #0]
_ZNK4elva11VirtualGpio4readEv():
./elva/src/elva/gpio/virtual_gpio.hpp:64
		const bool value = port()->IDR & mask();
 8005234:	691a      	ldr	r2, [r3, #16]
_ZNK4elva11VirtualGpio4maskEv():
./elva/src/elva/gpio/virtual_gpio.hpp:164
	inline uint32_t mask() const { return pinMask; }
 8005236:	8883      	ldrh	r3, [r0, #4]
_ZNK4elva11VirtualGpio4readEv():
./elva/src/elva/gpio/virtual_gpio.hpp:65
		return isInverted xor value;
 8005238:	79c0      	ldrb	r0, [r0, #7]
./elva/src/elva/gpio/virtual_gpio.hpp:64
		const bool value = port()->IDR & mask();
 800523a:	4013      	ands	r3, r2
./elva/src/elva/gpio/virtual_gpio.hpp:66
	}
 800523c:	2b00      	cmp	r3, #0
 800523e:	bf18      	it	ne
 8005240:	f080 0001 	eorne.w	r0, r0, #1
 8005244:	4770      	bx	lr

setOutput() was inlined:

_ZN4elva11VirtualGpio9setOutputEv():
./elva/src/elva/gpio/virtual_gpio.hpp:73
		port()->MODER = (port()->MODER & ~mask2()) | mask2(i(Mode::Output));
 80054d8:	6813      	ldr	r3, [r2, #0]
_ZNK4elva11VirtualGpio5mask2Em():
./elva/src/elva/gpio/virtual_gpio.hpp:165
	inline uint32_t mask2(uint32_t value=3ul) const { return value << position2; }
 80054da:	2001      	movs	r0, #1
_ZN4elva11VirtualGpio9setOutputEv():
./elva/src/elva/gpio/virtual_gpio.hpp:73
		port()->MODER = (port()->MODER & ~mask2()) | mask2(i(Mode::Output));
 80054dc:	ea23 0305 	bic.w	r3, r3, r5
_ZNK4elva11VirtualGpio5mask2Em():
./elva/src/elva/gpio/virtual_gpio.hpp:165
	inline uint32_t mask2(uint32_t value=3ul) const { return value << position2; }
 80054e0:	fa00 f101 	lsl.w	r1, r0, r1
_ZN4elva11VirtualGpio9setOutputEv():
./elva/src/elva/gpio/virtual_gpio.hpp:73
		port()->MODER = (port()->MODER & ~mask2()) | mask2(i(Mode::Output));
 80054e4:	430b      	orrs	r3, r1
 80054e6:	6013      	str	r3, [r2, #0]
_ZN4elva11GpioControl15configureOutputEhhN4modm8platform4Gpio10OutputTypeENS3_9InputTypeEib():
./elva/src/elva/gpio/gpio_control.cpp:101

configure() also inlined:

_ZNK4elva11VirtualGpio4portEv():
./elva/src/elva/gpio/virtual_gpio.hpp:163
	inline GPIO_TypeDef* port() const { return reinterpret_cast<GPIO_TypeDef*>(portAddress); }
 80054ae:	6822      	ldr	r2, [r4, #0]
_ZNK4elva11VirtualGpio5mask2Em():
./elva/src/elva/gpio/virtual_gpio.hpp:165
	inline uint32_t mask2(uint32_t value=3ul) const { return value << position2; }
 80054b0:	79a1      	ldrb	r1, [r4, #6]
_ZN4elva11VirtualGpio9configureEN4modm8platform4Gpio10OutputTypeENS3_11OutputSpeedE():
./elva/src/elva/gpio/virtual_gpio.hpp:33
		port()->OSPEEDR = (port()->OSPEEDR & ~mask2()) | mask2(i(speed));
 80054b2:	6890      	ldr	r0, [r2, #8]
_ZNK4elva11VirtualGpio5mask2Em():
./elva/src/elva/gpio/virtual_gpio.hpp:165
	inline uint32_t mask2(uint32_t value=3ul) const { return value << position2; }
 80054b4:	2303      	movs	r3, #3
 80054b6:	fa03 f501 	lsl.w	r5, r3, r1
 80054ba:	2302      	movs	r3, #2
 80054bc:	408b      	lsls	r3, r1
_ZN4elva11VirtualGpio9configureEN4modm8platform4Gpio10OutputTypeENS3_11OutputSpeedE():
./elva/src/elva/gpio/virtual_gpio.hpp:33
		port()->OSPEEDR = (port()->OSPEEDR & ~mask2()) | mask2(i(speed));
 80054be:	ea20 0005 	bic.w	r0, r0, r5
 80054c2:	4318      	orrs	r0, r3
 80054c4:	6090      	str	r0, [r2, #8]
_ZNK4elva11VirtualGpio4maskEv():
./elva/src/elva/gpio/virtual_gpio.hpp:164
	inline uint32_t mask() const { return pinMask; }
 80054c6:	88a3      	ldrh	r3, [r4, #4]
_ZN4elva11VirtualGpio9configureEN4modm8platform4Gpio10OutputTypeENS3_11OutputSpeedE():
./elva/src/elva/gpio/virtual_gpio.hpp:34
		port()->OTYPER  = (port()->OTYPER  & ~mask())  | (i(type) ? mask() : 0);
 80054c8:	6850      	ldr	r0, [r2, #4]
 80054ca:	2e00      	cmp	r6, #0
 80054cc:	ea20 0003 	bic.w	r0, r0, r3
 80054d0:	bf08      	it	eq
 80054d2:	2300      	moveq	r3, #0
 80054d4:	4318      	orrs	r0, r3
 80054d6:	6050      	str	r0, [r2, #4]

@chris-durand chris-durand force-pushed the feature/stm32_type_erased_gpio branch from c53a6dd to ae6002e Compare May 11, 2021 20:22
@salkinium
Copy link
Member

I added a more complete Runtime Gpio type, but I broke something in the RandomAccessPort. I also need to add a virtual base class for Gpio to implement UnusedGpio and GpioExpander pins (but the interface has too many functions for that).

@salkinium salkinium force-pushed the feature/stm32_type_erased_gpio branch from bf44fe8 to ae6002e Compare May 15, 2021 08:02
@salkinium
Copy link
Member

I've moved my changes into #631 to explore this in a more structured way. Are you ok with waiting until the runtime GPIO interface is more mature? Your code is pretty independent of modm, so it's probably easy to just include in the project directly.

@chris-durand
Copy link
Member Author

I am fine with not merging yet. Didn't have any time to work on this last week, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants