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

[Core] Use CRC calculation subsystem for split_common #13418

Merged
merged 1 commit into from
Jul 2, 2021

Conversation

KarlK90
Copy link
Member

@KarlK90 KarlK90 commented Jul 2, 2021

Description

Now that the compilation bugs have been cleared out in #13253 switch the new transaction system implemented in #13353 to use the CRC functions added in #12641.

@XScorpion2 had issues with stripped crc8 functions in their build. So I compiled both an AVR build of CRKBD and an ARM build of zvecr's split blackpill with and without LTO enabled. The functions are present under all configurations. This is the disassembly of the ELF files for the crc8 functions:

AVR GCC 5.4.0

  1. make crkbd/rev1:default
  2. avr-gdb -batch -ex "disassemble/rs 'crc8.constprop.13'" crkbd_rev1_default.elf
ASM DUMP
Dump of assembler code for function crc8.constprop.13:
quantum/crc.c:
43      __attribute__((weak)) uint8_t crc8(const void *data, size_t data_len) {
   0x0000390e <+0>:     fc 01   movw    r30, r24
   0x00003910 <+2>:     ac 01   movw    r20, r24
   0x00003912 <+4>:     4c 5f   subi    r20, 0xFC       ; 252
   0x00003914 <+6>:     5f 4f   sbci    r21, 0xFF       ; 255

44          const uint8_t *d   = (const uint8_t *)data;
45          crc_t          crc = 0xff;
   0x00003916 <+8>:     8f ef   ldi     r24, 0xFF       ; 255

52                      crc = (crc_t)((crc << 1) ^ 0x31);
   0x00003918 <+10>:    91 e3   ldi     r25, 0x31       ; 49

49              crc ^= d[i];
   0x0000391a <+12>:    21 91   ld      r18, Z+
   0x0000391c <+14>:    82 27   eor     r24, r18
   0x0000391e <+16>:    28 e0   ldi     r18, 0x08       ; 8
   0x00003920 <+18>:    30 e0   ldi     r19, 0x00       ; 0

51                  if ((crc & 0x80) != 0)
   0x00003922 <+20>:    87 ff   sbrs    r24, 7
   0x00003924 <+22>:    03 c0   rjmp    .+6             ;  0x392c <crc8.constprop.13+30>

52                      crc = (crc_t)((crc << 1) ^ 0x31);
   0x00003926 <+24>:    88 0f   add     r24, r24
   0x00003928 <+26>:    89 27   eor     r24, r25
   0x0000392a <+28>:    01 c0   rjmp    .+2             ;  0x392e <crc8.constprop.13+32>

53                  else
54                      crc <<= 1;
   0x0000392c <+30>:    88 0f   add     r24, r24
   0x0000392e <+32>:    21 50   subi    r18, 0x01       ; 1
   0x00003930 <+34>:    31 09   sbc     r19, r1

50              for (j = 0; j < 8; j++) {
   0x00003932 <+36>:    b9 f7   brne    .-18            ;  0x3922 <crc8.constprop.13+20>

48          for (i = 0; i < data_len; i++) {
   0x00003934 <+38>:    e4 17   cp      r30, r20
   0x00003936 <+40>:    f5 07   cpc     r31, r21
   0x00003938 <+42>:    81 f7   brne    .-32            ;  0x391a <crc8.constprop.13+12>

55              }
56          }
57          return crc;
58      }
   0x0000393a <+44>:    08 95   ret
End of assembler dump.

ARM GCC 10.2.1

  1. make zvecr/split_blackpill:default
  2. gdb-multiarch -batch -ex "disassemble/rs crc8" zvecr_split_blackpill_default.elf
ASM DUMP
Dump of assembler code for function crc8:
quantum/crc.c:
43      __attribute__((weak)) uint8_t crc8(const void *data, size_t data_len) {
   0x0800398c <+0>:     42 1e   subs    r2, r0, #1
   0x0800398e <+2>:     c3 1c   adds    r3, r0, #3

44          const uint8_t *d   = (const uint8_t *)data;
45          crc_t          crc = 0xff;
   0x08003990 <+4>:     ff 20   movs    r0, #255        ; 0xff

49              crc ^= d[i];
   0x08003992 <+6>:     12 f8 01 1f     ldrb.w  r1, [r2, #1]!
   0x08003996 <+10>:    48 40   eors    r0, r1
   0x08003998 <+12>:    08 21   movs    r1, #8

51                  if ((crc & 0x80) != 0)
   0x0800399a <+14>:    10 f0 80 0f     tst.w   r0, #128        ; 0x80
   0x0800399e <+18>:    4f ea 40 00     mov.w   r0, r0, lsl #1

52                      crc = (crc_t)((crc << 1) ^ 0x31);
   0x080039a2 <+22>:    18 bf   it      ne
   0x080039a4 <+24>:    80 f0 31 00     eorne.w r0, r0, #49     ; 0x31

50              for (j = 0; j < 8; j++) {
   0x080039a8 <+28>:    01 39   subs    r1, #1

53                  else
54                      crc <<= 1;
   0x080039aa <+30>:    c0 b2   uxtb    r0, r0

50              for (j = 0; j < 8; j++) {
   0x080039ac <+32>:    f5 d1   bne.n   0x800399a <crc8+14>

48          for (i = 0; i < data_len; i++) {
   0x080039ae <+34>:    9a 42   cmp     r2, r3
   0x080039b0 <+36>:    ef d1   bne.n   0x8003992 <crc8+6>

55              }
56          }
57          return crc;
58      }
   0x080039b2 <+38>:    70 47   bx      lr
End of assembler dump.

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).

@github-actions github-actions bot added the core label Jul 2, 2021
@zvecr zvecr requested review from tzarc and a team July 2, 2021 20:16
@zvecr zvecr merged commit 04bc74d into qmk:develop Jul 2, 2021
@KarlK90 KarlK90 deleted the enable-crc-subsystem branch July 3, 2021 00:18
cadusk pushed a commit to cadusk/qmk_firmware that referenced this pull request Jul 3, 2021
* qmk/develop:
  Switch split_common to CRC subsystem (qmk#13418)
nhongooi pushed a commit to nhongooi/qmk_firmware that referenced this pull request Dec 5, 2021
BorisTestov pushed a commit to BorisTestov/qmk_firmware that referenced this pull request May 23, 2024
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.

4 participants