Do not require registers to be passed to GPIO configuration functions

[Piroro-hs]

ref: #37

[Rahix]

For anyone curious like me, the generated assembly looks like this (building the toggle.rs example while having into_push_pull_output() annotated with #[inline(never)]):

Old Version (231c959)

08000d8a <stm32f3xx_hal::gpio::Pin<Gpio,Index,Mode>::into_push_pull_output::h647ca225cf9e2a43>:
 8000d8a:	f241 0000 	movw	r0, #4096	; 0x1000
 8000d8e:	2201      	movs	r2, #1
 8000d90:	f6c4 0000 	movt	r0, #18432	; 0x4800
 8000d94:	6801      	ldr	r1, [r0, #0]
 8000d96:	f362 619b 	bfi	r1, r2, #26, #2
 8000d9a:	6001      	str	r1, [r0, #0]
 8000d9c:	6841      	ldr	r1, [r0, #4]
 8000d9e:	f421 5100 	bic.w	r1, r1, #8192	; 0x2000
 8000da2:	6041      	str	r1, [r0, #4]
 8000da4:	4770      	bx	lr

New Version (6aba91b)

08000d8a <stm32f3xx_hal::gpio::Pin<Gpio,Index,Mode>::into_push_pull_output::h5da17e05f75fc27c>:
 8000d8a:	b5d0      	push	{r4, r6, r7, lr}
 8000d8c:	af02      	add	r7, sp, #8
 8000d8e:	f241 0400 	movw	r4, #4096	; 0x1000
 8000d92:	f44f 5100 	mov.w	r1, #8192	; 0x2000
 8000d96:	f6c4 0400 	movt	r4, #18432	; 0x4800
 8000d9a:	1d20      	adds	r0, r4, #4
 8000d9c:	2200      	movs	r2, #0
 8000d9e:	f000 f809 	bl	8000db4 <volatile_atomic_bic_or>
 8000da2:	4620      	mov	r0, r4
 8000da4:	f04f 6140 	mov.w	r1, #201326592	; 0xc000000
 8000da8:	f04f 6280 	mov.w	r2, #67108864	; 0x4000000
 8000dac:	e8bd 40d0 	ldmia.w	sp!, {r4, r6, r7, lr}
 8000db0:	f000 b800 	b.w	8000db4 <volatile_atomic_bic_or>

08000db4 <volatile_atomic_bic_or>:
 8000db4:	e850 3f00 	ldrex	r3, [r0]
 8000db8:	438b      	bics	r3, r1
 8000dba:	4313      	orrs	r3, r2
 8000dbc:	e840 3300 	strex	r3, r3, [r0]
 8000dc0:	2b00      	cmp	r3, #0
 8000dc2:	d1f7      	bne.n	8000db4 <volatile_atomic_bic_or>
 8000dc4:	4770      	bx	lr

With a critical-section

Just for fun I also added a critical section to the function (the version from the master branch) to see how this would look in comparison (not a full implementation of the concept):

08000d8a <stm32f3xx_hal::gpio::Pin<Gpio,Index,Mode>::into_push_pull_output::h647ca225cf9e2a43>:
 8000d8a:	b5d0      	push	{r4, r6, r7, lr}
 8000d8c:	af02      	add	r7, sp, #8
 8000d8e:	f000 f825 	bl	8000ddc <__primask_r>
 8000d92:	4604      	mov	r4, r0
 8000d94:	f000 f818 	bl	8000dc8 <__cpsid>
 8000d98:	f241 0000 	movw	r0, #4096	; 0x1000
 8000d9c:	2201      	movs	r2, #1
 8000d9e:	f6c4 0000 	movt	r0, #18432	; 0x4800
 8000da2:	6801      	ldr	r1, [r0, #0]
 8000da4:	f362 619b 	bfi	r1, r2, #26, #2
 8000da8:	6001      	str	r1, [r0, #0]
 8000daa:	6841      	ldr	r1, [r0, #4]
 8000dac:	f421 5100 	bic.w	r1, r1, #8192	; 0x2000
 8000db0:	6041      	str	r1, [r0, #4]
 8000db2:	07e0      	lsls	r0, r4, #31
 8000db4:	bf18      	it	ne
 8000db6:	bdd0      	popne	{r4, r6, r7, pc}
 8000db8:	e8bd 40d0 	ldmia.w	sp!, {r4, r6, r7, lr}
 8000dbc:	f000 b806 	b.w	8000dcc <__cpsie>

08000dc8 <__cpsid>:
 8000dc8:	b672      	cpsid	i
 8000dca:	4770      	bx	lr

08000dcc <__cpsie>:
 8000dcc:	b662      	cpsie	i
 8000dce:	4770      	bx	lr

08000ddc <__primask_r>:
 8000ddc:	f3ef 8010 	mrs	r0, PRIMASK
 8000de0:	4770      	bx	lr

The source diff

diff --git a/src/gpio.rs b/src/gpio.rs
index 0cfdac95e670..d15c5b9eeeaa 100644
--- a/src/gpio.rs
+++ b/src/gpio.rs
@@ -395,14 +395,17 @@ where
     }
 
     /// Configures the pin to operate as a push-pull output pin
+    #[inline(never)]
     pub fn into_push_pull_output(
         self,
         moder: &mut Gpio::MODER,
         otyper: &mut Gpio::OTYPER,
     ) -> Pin<Gpio, Index, Output<PushPull>> {
-        moder.output(self.index.index());
-        otyper.push_pull(self.index.index());
-        self.into_mode()
+        cortex_m::interrupt::free(|_cs| {
+            moder.output(self.index.index());
+            otyper.push_pull(self.index.index());
+            self.into_mode()
+        })
     }
 
     /// Configures the pin to operate as an open-drain output pin

---

I hope this provides some insight into how the different approaches compare. It would be interesting to calculate the WCET as well (for the code where that is possible).

[Sh3Rm4n]

They were a bit of chatting around how to test this, before applying these ergonomic improvements overall in the embedded-hal meeting ~ a month ago: https://matrix.to/#/!BHcierreUuwCMxVqOf:matrix.org/$J63pKvA9WeSSGtQZQkqFKH-xzk0gXfjEJIIDEAvjRmU?via=matrix.org&via=privacytools.io&via=mozilla.org

I have not given it much thought since then, but maybe someone has some ideas.

I want to push #212 a bit forward, to have a better reference point. Maybe I'll find some time this week to add a few more (hopefully all relevant gpio tests).

[Sh3Rm4n]

#212 is now merged and v0.7.0 is released. This would introduce a major, but really wanted change. I would be more confident, that we write some tests for this change, so that we can be more confident, that this is really working (and also atomic and concurrency safe)

[Piroro-hs]

Updated to prepare asm! stabilization in 1.59.

generated assembly

08001178 <stm32f3xx_hal::gpio::Pin<Gpio,Index,Mode>::into_push_pull_output::h8ad8b21bb542ca03>:
 8001178: 41 f2 04 00   movw    r0, #4100
 800117c: c4 f6 00 00   movt    r0, #18432
 8001180: 50 e8 00 1f   ldrex   r1, [r0]
 8001184: 21 f4 00 51   bic     r1, r1, #8192
 8001188: 40 e8 00 11   strex   r1, r1, [r0]
 800118c: 00 29         cmp     r1, #0
 800118e: f7 d1         bne     0x8001180 <stm32f3xx_hal::gpio::Pin<Gpio,Index,Mode>::into_push_pull_output::h8ad8b21bb542ca03+0x8> @ imm = #-18
 8001190: 41 f2 00 00   movw    r0, #4096
 8001194: 01 21         movs    r1, #1
 8001196: c4 f6 00 00   movt    r0, #18432
 800119a: 50 e8 00 2f   ldrex   r2, [r0]
 800119e: 61 f3 9b 62   bfi     r2, r1, #26, #2
 80011a2: 40 e8 00 22   strex   r2, r2, [r0]
 80011a6: 00 2a         cmp     r2, #0
 80011a8: f7 d1         bne     0x800119a <stm32f3xx_hal::gpio::Pin<Gpio,Index,Mode>::into_push_pull_output::h8ad8b21bb542ca03+0x22> @ imm = #-18
 80011aa: 70 47         bx      lr

[Sh3Rm4n]

I haven't looked into this PR in a while, but it seems this is a huge improvement on ergonomics. Also, I have not followed up on any benchmark proposal, we should do before merging this, but in any case, let's push this forward, when 1.59 is released!

[Sh3Rm4n]

Great work. I like that the patch has more removed lines than added lines 😁

Have you tested it with the current test set (reminds me, that I could do it myself as running the code now is easier than ever)!

And do you have an idea how to stress test it? Or did you do it yourself already? :)

[Sh3Rm4n]

Hey, this is conflicting with #316 with will probably will introduce much work on this PR again.

I just wanted to ask if you have time to adjust to these changes. I'm willing to pick this up if I find time myself if that's okay for you :)