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

Some GPIO manipulations in matrix.c change to atomic. #10491

Merged
merged 8 commits into from
Oct 26, 2020

Conversation

mtei
Copy link
Contributor

@mtei mtei commented Sep 30, 2020

Description

Some GPIO manipulations in matrix.c are safer to do atomically, so I changed them that way.

I think this is a solution to #5953 that has less scope of influence than #9575.

EDIT:

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@mtei mtei added the core label Sep 30, 2020
@mtei mtei requested review from fauxpark, zvecr, a team and drashna September 30, 2020 12:29
@drashna drashna requested a review from tzarc October 4, 2020 22:26
@mtei mtei changed the base branch from master to develop October 5, 2020 12:54
@mtei
Copy link
Contributor Author

mtei commented Oct 20, 2020

Implemented the macro ATOMIC_BLOCK_FORCEON, which has the same function as ATOMIC_BLOCK(ATOMIC_FORCEON) of avr-gcc, for ChibiOS.

For the time being, the implementation of ATOMIC_BLOCK_FORCEON is in quantum/quantum_atomic_extend.h. Please let me know where it is appropriate to put it.

Copy link
Member

@tzarc tzarc left a comment

Choose a reason for hiding this comment

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

Been testing it a few days on the Djinn, haven't seen any issues with daily use.

@tzarc tzarc requested a review from a team October 22, 2020 19:23
@mtei mtei marked this pull request as ready for review October 22, 2020 23:37
@mtei
Copy link
Contributor Author

mtei commented Oct 23, 2020

I added the ATOMIC_BLOCK_FORCEON macro to quantum/quantum.h that can be used as follows.

void some_function() {
     // some process
     ATOMIC_BLOCK_FORCEON {
        // Atomic Processing
     }
     // some process
}

Example:

void setPinOutput_writeLow_non_atomic(pin_t pin) {
    setPinOutput(pin);
    writePinLow(pin);
}

void setPinOutput_writeLow(pin_t pin) {
    ATOMIC_BLOCK_FORCEON {
        setPinOutput(pin);
        writePinLow(pin);
    }
}
AVR result (helix/rev2/sc) (click)
Disassembly of section .text.setPinOutput_writeLow_non_atomic:

00000000 <setPinOutput_writeLow_non_atomic>:
   0:	e8 2f       	mov	r30, r24
   2:	e2 95       	swap	r30
   4:	ef 70       	andi	r30, 0x0F	; 15
   6:	f0 e0       	ldi	r31, 0x00	; 0
   8:	31 a1       	ldd	r19, Z+33	; 0x21
   a:	28 2f       	mov	r18, r24
   c:	2f 70       	andi	r18, 0x0F	; 15
   e:	81 e0       	ldi	r24, 0x01	; 1
  10:	90 e0       	ldi	r25, 0x00	; 0
  12:	00 c0       	rjmp	.+0      	; 0x14 <setPinOutput_writeLow_non_atomic+0x14>
			12: R_AVR_13_PCREL	.text.setPinOutput_writeLow_non_atomic+0x16
  14:	88 0f       	add	r24, r24
  16:	2a 95       	dec	r18
  18:	02 f4       	brpl	.+0      	; 0x1a <setPinOutput_writeLow_non_atomic+0x1a>
			18: R_AVR_7_PCREL	.text.setPinOutput_writeLow_non_atomic+0x14
  1a:	38 2b       	or	r19, r24
  1c:	31 a3       	std	Z+33, r19	; 0x21
  1e:	92 a1       	ldd	r25, Z+34	; 0x22
  20:	80 95       	com	r24
  22:	89 23       	and	r24, r25
  24:	82 a3       	std	Z+34, r24	; 0x22
  26:	08 95       	ret

Disassembly of section .text.setPinOutput_writeLow:

00000000 <setPinOutput_writeLow>:
   0:	f8 94       	cli	 		;  /** interrupt disable **/
   2:	e8 2f       	mov	r30, r24	;  /** Same code as above **/
   4:	e2 95       	swap	r30
   6:	ef 70       	andi	r30, 0x0F	; 15
   8:	f0 e0       	ldi	r31, 0x00	; 0
   a:	31 a1       	ldd	r19, Z+33	; 0x21
   c:	28 2f       	mov	r18, r24
   e:	2f 70       	andi	r18, 0x0F	; 15
  10:	81 e0       	ldi	r24, 0x01	; 1
  12:	90 e0       	ldi	r25, 0x00	; 0
  14:	00 c0       	rjmp	.+0      	; 0x16 <setPinOutput_writeLow+0x16>
			14: R_AVR_13_PCREL	.text.setPinOutput_writeLow+0x18
  16:	88 0f       	add	r24, r24
  18:	2a 95       	dec	r18
  1a:	02 f4       	brpl	.+0      	; 0x1c <setPinOutput_writeLow+0x1c>
			1a: R_AVR_7_PCREL	.text.setPinOutput_writeLow+0x16
  1c:	38 2b       	or	r19, r24
  1e:	31 a3       	std	Z+33, r19	; 0x21
  20:	92 a1       	ldd	r25, Z+34	; 0x22
  22:	80 95       	com	r24
  24:	89 23       	and	r24, r25
  26:	82 a3       	std	Z+34, r24	; 0x22
  28:	78 94       	sei	 		;  /** interrupt enable **/
  2a:	08 95       	ret

ARM result (plank/rev6) (click)
00000000 <setPinOutput_writeLow_non_atomic>:
   0:	b538      	push	{r3, r4, r5, lr}
   2:	2201      	movs	r2, #1
   4:	f020 050f 	bic.w	r5, r0, #15
   8:	f000 000f 	and.w	r0, r0, #15
   c:	fa02 f400 	lsl.w	r4, r2, r0
  10:	4621      	mov	r1, r4
  12:	4628      	mov	r0, r5
  14:	b2a4      	uxth	r4, r4
  16:	f7ff fffe 	bl	0 <_pal_lld_setgroupmode>
			16: R_ARM_THM_CALL	_pal_lld_setgroupmode
  1a:	836c      	strh	r4, [r5, #26]
  1c:	bd38      	pop	{r3, r4, r5, pc}

Disassembly of section .text.setPinOutput_writeLow:

00000000 <setPinOutput_writeLow>:
   0:	b538      	push	{r3, r4, r5, lr}
   2:	2320      	movs	r3, #32
   4:	f383 8811 	msr	BASEPRI, r3	;  /** interrupt disable **/
   8:	2201      	movs	r2, #1		;  /** Same code as above **/
   a:	f020 050f 	bic.w	r5, r0, #15
   e:	f000 000f 	and.w	r0, r0, #15
  12:	fa02 f400 	lsl.w	r4, r2, r0
  16:	4621      	mov	r1, r4
  18:	4628      	mov	r0, r5
  1a:	b2a4      	uxth	r4, r4
  1c:	f7ff fffe 	bl	0 <_pal_lld_setgroupmode>
			1c: R_ARM_THM_CALL	_pal_lld_setgroupmode
  20:	836c      	strh	r4, [r5, #26]
  22:	2300      	movs	r3, #0
  24:	f383 8811 	msr	BASEPRI, r3	;  /** interrupt enable **/
  28:	bd38      	pop	{r3, r4, r5, pc}

(see https://github.com/mtei/qmk_firmware/blob/matrix_c_atomic/docs/internals_gpio_control.md#atomic-operation)

@mtei mtei merged commit 8337fcc into qmk:develop Oct 26, 2020
skullydazed pushed a commit that referenced this pull request Oct 28, 2020
* Changed the processing of select_xxx()/unselect_xxx() in quantum/matrix.c to be atomic.

* Changed the processing of select_xxx()/unselect_xxx() in quantum/split_common/matrix.c to be atomic.

* update matrix.c

* add ATOMIC_BLOCK_FORCEON macro to quantum/quantum_atomic_extend.h

* quantum_atomic_extend.h's contents move into quantum.h

* update ATOMIC_BLOCK_xxx for unknown platform

* ATOMIC_BLOCK macro support PROTOCOL_ARM_ATSAM

* Add Atomic Operation section in docs/internals_gpio_control.md
noroadsleft pushed a commit that referenced this pull request Oct 30, 2020
* Changed the processing of select_xxx()/unselect_xxx() in quantum/matrix.c to be atomic.

* Changed the processing of select_xxx()/unselect_xxx() in quantum/split_common/matrix.c to be atomic.

* update matrix.c

* add ATOMIC_BLOCK_FORCEON macro to quantum/quantum_atomic_extend.h

* quantum_atomic_extend.h's contents move into quantum.h

* update ATOMIC_BLOCK_xxx for unknown platform

* ATOMIC_BLOCK macro support PROTOCOL_ARM_ATSAM

* Add Atomic Operation section in docs/internals_gpio_control.md
noroadsleft added a commit that referenced this pull request Nov 28, 2020
* Branch point for 2020 November 28 Breaking Change                                                

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (#10183)                                           

* Add support for soft serial to ATmega32U2 (#10204)                                               

* Change MIDI velocity implementation to allow direct control of velocity value (#9940)            

* Add ability to build a subset of all keyboards based on platform.                                

* Actually use eeprom_driver_init().                                                               

* Make bootloader_jump weak for ChibiOS. (#10417)                                                  

* Joystick 16-bit support (#10439)                                                                 

* Per-encoder resolutions (#10259)                                                                 

* Share button state from mousekey to pointing_device (#10179)                                     

* Add hotfix for chibios keyboards not wake (#10088)                                               

* Add advanced/efficient RGB Matrix Indicators (#8564)                                             

* Naming change.                                                                                   

* Support for STM32 GPIOF,G,H,I,J,K (#10206)                                                       

* Add milc as a dependency and remove the installed milc (#10563)                                  

* ChibiOS upgrade: early init conversions (#10214)                                                 

* ChibiOS upgrade: configuration file migrator (#9952)                                             

* Haptic and solenoid cleanup (#9700)                                                              

* XD75 cleanup (#10524)                                                                            

* OLED display update interval support (#10388)                                                    

* Add definition based on currently-selected serial driver. (#10716)                               

* New feature: Retro Tapping per key (#10622)                                                      

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (#10638)             

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (#10530)

* Rescale both ChibiOS and AVR backlighting.                                                       

* Reduce Helix keyboard build variation (#8669)                                                    

* Minor change to behavior allowing display updates to continue between task ticks (#10750)        

* Some GPIO manipulations in matrix.c change to atomic. (#10491)                                   

* qmk cformat (#10767)                                                                             

* [Keyboard] Update the Speedo firmware for v3.0 (#10657)                                          

* Maartenwut/Maarten namechange to evyd13/Evy (#10274)                                             

* [quantum] combine repeated lines of code (#10837)                                                

* Add step sequencer feature (#9703)                                                               

* aeboards/ext65 refactor (#10820)                                                                 

* Refactor xelus/dawn60 for Rev2 later (#10584)                                                    

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (#10824)                                 

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (#10549)                                    

* update chibios os usb for the otg driver (#8893)                                                 

* Remove HD44780 References, Part 4 (#10735)                                                       

* [Keyboard] Add Valor FRL TKL (+refactor) (#10512)                                                

* Fix cursor position bug in oled_write_raw functions (#10800)                                     

* Fixup version.h writing when using SKIP_VERSION=yes (#10972)                                     

* Allow for certain code in the codebase assuming length of string. (#10974)                       

* Add AT90USB support for serial.c (#10706)                                                        

* Auto shift: support repeats and early registration (#9826)                                       

* Rename ledmatrix.h to match .c file (#7949)                                                      

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (#10231)                                        

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (#10840)                                        

* Merge point for 2020 Nov 28 Breaking Change
@mtei mtei deleted the matrix_c_atomic branch December 21, 2020 20:46
xgnxs pushed a commit to xgnxs/qmk_firmware that referenced this pull request Jan 9, 2021
* Branch point for 2020 November 28 Breaking Change                                                

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183)                                           

* Add support for soft serial to ATmega32U2 (qmk#10204)                                               

* Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940)            

* Add ability to build a subset of all keyboards based on platform.                                

* Actually use eeprom_driver_init().                                                               

* Make bootloader_jump weak for ChibiOS. (qmk#10417)                                                  

* Joystick 16-bit support (qmk#10439)                                                                 

* Per-encoder resolutions (qmk#10259)                                                                 

* Share button state from mousekey to pointing_device (qmk#10179)                                     

* Add hotfix for chibios keyboards not wake (qmk#10088)                                               

* Add advanced/efficient RGB Matrix Indicators (qmk#8564)                                             

* Naming change.                                                                                   

* Support for STM32 GPIOF,G,H,I,J,K (qmk#10206)                                                       

* Add milc as a dependency and remove the installed milc (qmk#10563)                                  

* ChibiOS upgrade: early init conversions (qmk#10214)                                                 

* ChibiOS upgrade: configuration file migrator (qmk#9952)                                             

* Haptic and solenoid cleanup (qmk#9700)                                                              

* XD75 cleanup (qmk#10524)                                                                            

* OLED display update interval support (qmk#10388)                                                    

* Add definition based on currently-selected serial driver. (qmk#10716)                               

* New feature: Retro Tapping per key (qmk#10622)                                                      

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638)             

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530)

* Rescale both ChibiOS and AVR backlighting.                                                       

* Reduce Helix keyboard build variation (qmk#8669)                                                    

* Minor change to behavior allowing display updates to continue between task ticks (qmk#10750)        

* Some GPIO manipulations in matrix.c change to atomic. (qmk#10491)                                   

* qmk cformat (qmk#10767)                                                                             

* [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657)                                          

* Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274)                                             

* [quantum] combine repeated lines of code (qmk#10837)                                                

* Add step sequencer feature (qmk#9703)                                                               

* aeboards/ext65 refactor (qmk#10820)                                                                 

* Refactor xelus/dawn60 for Rev2 later (qmk#10584)                                                    

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824)                                 

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549)                                    

* update chibios os usb for the otg driver (qmk#8893)                                                 

* Remove HD44780 References, Part 4 (qmk#10735)                                                       

* [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512)                                                

* Fix cursor position bug in oled_write_raw functions (qmk#10800)                                     

* Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972)                                     

* Allow for certain code in the codebase assuming length of string. (qmk#10974)                       

* Add AT90USB support for serial.c (qmk#10706)                                                        

* Auto shift: support repeats and early registration (qmk#9826)                                       

* Rename ledmatrix.h to match .c file (qmk#7949)                                                      

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231)                                        

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840)                                        

* Merge point for 2020 Nov 28 Breaking Change
drashna pushed a commit to zsa/qmk_firmware that referenced this pull request Jan 13, 2021
* Branch point for 2020 November 28 Breaking Change

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183)

* Add support for soft serial to ATmega32U2 (qmk#10204)

* Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940)

* Add ability to build a subset of all keyboards based on platform.

* Actually use eeprom_driver_init().

* Make bootloader_jump weak for ChibiOS. (qmk#10417)

* Joystick 16-bit support (qmk#10439)

* Per-encoder resolutions (qmk#10259)

* Share button state from mousekey to pointing_device (qmk#10179)

* Add hotfix for chibios keyboards not wake (qmk#10088)

* Add advanced/efficient RGB Matrix Indicators (qmk#8564)

* Naming change.

* Support for STM32 GPIOF,G,H,I,J,K (qmk#10206)

* Add milc as a dependency and remove the installed milc (qmk#10563)

* ChibiOS upgrade: early init conversions (qmk#10214)

* ChibiOS upgrade: configuration file migrator (qmk#9952)

* Haptic and solenoid cleanup (qmk#9700)

* XD75 cleanup (qmk#10524)

* OLED display update interval support (qmk#10388)

* Add definition based on currently-selected serial driver. (qmk#10716)

* New feature: Retro Tapping per key (qmk#10622)

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638)

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530)

* Rescale both ChibiOS and AVR backlighting.

* Reduce Helix keyboard build variation (qmk#8669)

* Minor change to behavior allowing display updates to continue between task ticks (qmk#10750)

* Some GPIO manipulations in matrix.c change to atomic. (qmk#10491)

* qmk cformat (qmk#10767)

* [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657)

* Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274)

* [quantum] combine repeated lines of code (qmk#10837)

* Add step sequencer feature (qmk#9703)

* aeboards/ext65 refactor (qmk#10820)

* Refactor xelus/dawn60 for Rev2 later (qmk#10584)

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824)

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549)

* update chibios os usb for the otg driver (qmk#8893)

* Remove HD44780 References, Part 4 (qmk#10735)

* [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512)

* Fix cursor position bug in oled_write_raw functions (qmk#10800)

* Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972)

* Allow for certain code in the codebase assuming length of string. (qmk#10974)

* Add AT90USB support for serial.c (qmk#10706)

* Auto shift: support repeats and early registration (qmk#9826)

* Rename ledmatrix.h to match .c file (qmk#7949)

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231)

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840)

* Merge point for 2020 Nov 28 Breaking Change
@sigprof sigprof mentioned this pull request Apr 3, 2021
13 tasks
@sigprof sigprof mentioned this pull request Apr 18, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Branch point for 2020 November 28 Breaking Change                                                

* Remove matrix_col_t to allow MATRIX_ROWS > 32 (qmk#10183)                                           

* Add support for soft serial to ATmega32U2 (qmk#10204)                                               

* Change MIDI velocity implementation to allow direct control of velocity value (qmk#9940)            

* Add ability to build a subset of all keyboards based on platform.                                

* Actually use eeprom_driver_init().                                                               

* Make bootloader_jump weak for ChibiOS. (qmk#10417)                                                  

* Joystick 16-bit support (qmk#10439)                                                                 

* Per-encoder resolutions (qmk#10259)                                                                 

* Share button state from mousekey to pointing_device (qmk#10179)                                     

* Add hotfix for chibios keyboards not wake (qmk#10088)                                               

* Add advanced/efficient RGB Matrix Indicators (qmk#8564)                                             

* Naming change.                                                                                   

* Support for STM32 GPIOF,G,H,I,J,K (qmk#10206)                                                       

* Add milc as a dependency and remove the installed milc (qmk#10563)                                  

* ChibiOS upgrade: early init conversions (qmk#10214)                                                 

* ChibiOS upgrade: configuration file migrator (qmk#9952)                                             

* Haptic and solenoid cleanup (qmk#9700)                                                              

* XD75 cleanup (qmk#10524)                                                                            

* OLED display update interval support (qmk#10388)                                                    

* Add definition based on currently-selected serial driver. (qmk#10716)                               

* New feature: Retro Tapping per key (qmk#10622)                                                      

* Allow for modification of output RGB values when using rgblight/rgb_matrix. (qmk#10638)             

* Add housekeeping task callbacks so that keyboards/keymaps are capable of executing code for each main loop iteration. (qmk#10530)

* Rescale both ChibiOS and AVR backlighting.                                                       

* Reduce Helix keyboard build variation (qmk#8669)                                                    

* Minor change to behavior allowing display updates to continue between task ticks (qmk#10750)        

* Some GPIO manipulations in matrix.c change to atomic. (qmk#10491)                                   

* qmk cformat (qmk#10767)                                                                             

* [Keyboard] Update the Speedo firmware for v3.0 (qmk#10657)                                          

* Maartenwut/Maarten namechange to evyd13/Evy (qmk#10274)                                             

* [quantum] combine repeated lines of code (qmk#10837)                                                

* Add step sequencer feature (qmk#9703)                                                               

* aeboards/ext65 refactor (qmk#10820)                                                                 

* Refactor xelus/dawn60 for Rev2 later (qmk#10584)                                                    

* add DEBUG_MATRIX_SCAN_RATE_ENABLE to common_features.mk (qmk#10824)                                 

* [Core] Added `add_oneshot_mods` & `del_oneshot_mods` (qmk#10549)                                    

* update chibios os usb for the otg driver (qmk#8893)                                                 

* Remove HD44780 References, Part 4 (qmk#10735)                                                       

* [Keyboard] Add Valor FRL TKL (+refactor) (qmk#10512)                                                

* Fix cursor position bug in oled_write_raw functions (qmk#10800)                                     

* Fixup version.h writing when using SKIP_VERSION=yes (qmk#10972)                                     

* Allow for certain code in the codebase assuming length of string. (qmk#10974)                       

* Add AT90USB support for serial.c (qmk#10706)                                                        

* Auto shift: support repeats and early registration (qmk#9826)                                       

* Rename ledmatrix.h to match .c file (qmk#7949)                                                      

* Split RGB_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10231)                                        

* Split LED_MATRIX_ENABLE into _ENABLE and _DRIVER (qmk#10840)                                        

* Merge point for 2020 Nov 28 Breaking Change
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
* Changed the processing of select_xxx()/unselect_xxx() in quantum/matrix.c to be atomic.

* Changed the processing of select_xxx()/unselect_xxx() in quantum/split_common/matrix.c to be atomic.

* update matrix.c

* add ATOMIC_BLOCK_FORCEON macro to quantum/quantum_atomic_extend.h

* quantum_atomic_extend.h's contents move into quantum.h

* update ATOMIC_BLOCK_xxx for unknown platform

* ATOMIC_BLOCK macro support PROTOCOL_ARM_ATSAM

* Add Atomic Operation section in docs/internals_gpio_control.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants