cpu/arm: fixes necessary for toolchain upgrade by kaspar030 · Pull Request #13456 · RIOT-OS/RIOT

kaspar030 · 2020-02-24T13:21:14Z

Contribution description

The ARM toolchain upgrade (RIOT-OS/riotdocker#78) slipped through some new compiler warnings (which turn into errors now).

This PR tries to fix them.

Testing procedure

Issues/PRs references

Fixes #13434.

kaspar030 · 2020-02-24T13:22:01Z

Fixes these:

kaspar030 · 2020-02-24T13:22:29Z

I whish there was a "high priority" option in Murdock...

drivers/bmx280/include/bmx280_internals.h

kaspar030 · 2020-02-24T13:24:05Z

drivers/bmx280/include/bmx280_internals.h

 */
 #define CALIB_T_P_BASE                  (BMX280_DIG_T1_LSB_REG)
-#define CALIB_T_P_LEN                   (17U)
+#define CALIB_T_P_LEN                   (32U) /* might be larger than neeeded */


previous value was "17". The highest accessed index that popped up int the warning was "23". I chose 32 as the next highest power of two, on stack it would probably have chosen "24".

The 8-bit registers are named calib00...calib41 and are stored at memory addresses 0x88...0xA1 and 0xE1...0xE7.

so 26 should do

If I'm correct this should be 0x17 or 0d22. But we can change that in a follow-up with some more thorough testing. 32U should definitely be enough.

Reading the data sheets, it should be 26 24 (last byte is reserved) for both bmp280 and bme280

Since those are turned into register reads, I'd rather not read additional random registers without knowing if there might be some effect on-read.

Ok, let's merge as is then I open a PR for the correct value?

Why not use the right value right away?

Why not use the right value right away?

Because it will block all PRs for another 20 minutes :|

Ok, let's merge as is then I open a PR for the correct value?

Yes please!

kaspar030 · 2020-02-24T13:36:43Z

@RIOT-OS/ci @RIOT-OS/maintainers pls ACK. :)

benpicco · 2020-02-24T13:37:32Z

This will also fix #13434

kaspar030 · 2020-02-24T13:38:55Z

This will also fix #13434

There it is, didn't find the issue.

kaspar030 · 2020-02-24T13:50:14Z

There's still this:

/home/kaspar/src/riot/tests/unittests/tests-core/tests-core-atomic.c: In function 'test_atomic_inc_negative':
/home/kaspar/src/riot/tests/unittests/tests-core/tests-core-atomic.c:53:1: error: insn does not satisfy its constraints:
   53 | }                                                        
      | ^                                                        
(insn 145 144 146 4 (set (reg:SI 3 r3 [139])          
        (plus:SI (reg:SI 12 ip [orig:110 _1 ] [110])
            (const_int 1 [0x1]))) "/home/kaspar/src/riot/tests/unittests/tests-core/tests-core-atomic.c":50:9 811 {*thumb1_addsi3}
     (nil))                                                      
during RTL pass: cprop_hardreg                                   
/home/kaspar/src/riot/tests/unittests/tests-core/tests-core-atomic.c:53:1: internal compiler error: in extract_constrain_insn, at r
ecog.c:2211                                                      
Please submit a full bug report,        
with preprocessed source if appropriate.
See <https://bugs.archlinux.org/> for instructions.
make[2]: *** [/home/kaspar/src/riot/Makefile.base:110: /home/kaspar/src/riot/tests/unittests/bin/saml11-xpro/tests-core/tests-core-
atomic.o] Error 1

... when compiling the unittests for saml11-xpro...

I propose blacklisting until we have a fix.

benpicco · 2020-02-24T13:55:02Z

tests/unittests/Makefile

 include ../Makefile.tests_common

+# temporarily disable building, see #13456.
+BOARD_BLACKLIST := saml11-xpro


Suggested change

BOARD_BLACKLIST := saml11-xpro

BOARD_BLACKLIST := saml10-xpro saml11-xpro

benpicco · 2020-02-24T13:56:16Z

tests/unittests/Makefile

 include ../Makefile.tests_common

+# temporarily disable building, see #13456.
+BOARD_BLACKLIST := saml11-xpro


Or better yet

Suggested change

BOARD_BLACKLIST := saml11-xpro

FEATURES_BLACKLIST += cortex-m23

kaspar030 · 2020-02-24T13:57:37Z

cortex-m23

not exposed as arch feature :(

kaspar030 · 2020-02-24T14:01:41Z

/home/kaspar/src/riot/tests/unittests/tests-core/tests-core-atomic.c:53:1: internal compiler error: in extract_constrain_insn, at recog.c:2211

Sometimes I wonder, if RIOT's cute little compile test exposes something like this, what's gcc's or ARM's CI compiling?

kaspar030 · 2020-02-24T14:23:36Z

Thanks everyone!

benpicco · 2020-02-28T12:45:22Z

Looks like this also needs a backport, otherwise other packports are failing CI.

fjmolinas · 2020-02-28T13:25:51Z

Looks like this also needs a backport, otherwise other backports are failing CI.

@kaspar030 @benpicco are there other toolchain changes since the last release? I do not like the fact that whatever we are backporting will be tested with a different toolchain.

fjmolinas · 2020-03-03T10:51:01Z

Backport provided in #13536

pkg/openthread: add "-Wno-class-memaccess" to CXXFLAGS

0a4cc06

kaspar030 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: build system Area: Build system labels Feb 24, 2020

kaspar030 requested a review from jia200x as a code owner February 24, 2020 13:21

aabadie reviewed Feb 24, 2020

View reviewed changes

drivers/bmx280/include/bmx280_internals.h Outdated Show resolved Hide resolved

kaspar030 commented Feb 24, 2020

View reviewed changes

drivers/bmx280: increase CALIB_T_P_LEN

8a909b1

kaspar030 force-pushed the arm_gcc9_fixes branch from a4adf7e to 8a909b1 Compare February 24, 2020 13:25

kaspar030 added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Feb 24, 2020

kaspar030 mentioned this pull request Feb 24, 2020

sock_async: include sock_async_ctx.h before sock-types #13455

Merged

kaspar030 requested a review from miri64 as a code owner February 24, 2020 13:54

benpicco reviewed Feb 24, 2020

View reviewed changes

tests/unittests: temporarily disable for saml1[01]-xpro

13a6cfb

benpicco reviewed Feb 24, 2020

View reviewed changes

kaspar030 force-pushed the arm_gcc9_fixes branch from 9315972 to 13a6cfb Compare February 24, 2020 13:56

benpicco mentioned this pull request Feb 24, 2020

Add optional support for executable space protections #13387

Merged

benpicco approved these changes Feb 24, 2020

View reviewed changes

kaspar030 merged commit c11e8df into RIOT-OS:master Feb 24, 2020

kaspar030 deleted the arm_gcc9_fixes branch February 24, 2020 14:18

kaspar030 mentioned this pull request Feb 24, 2020

drivers/bmx280: provide correct CALIB_T_P_LEN #13458

Merged

dylad added a commit to dylad/RIOT that referenced this pull request Feb 25, 2020

tests/unittests: remove the temporary fix from RIOT-OS#13456

33d61a7

dylad added a commit to dylad/RIOT that referenced this pull request Feb 25, 2020

tests/unittests: remove the temporary fix from RIOT-OS#13456

9647542

dylad added a commit to dylad/RIOT that referenced this pull request Feb 26, 2020

tests/unittests: remove the temporary fix from RIOT-OS#13456

841140a

benpicco added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Feb 28, 2020

benpicco mentioned this pull request Feb 28, 2020

Revert "boards/nucleo-f767zi: add correct flash bank openocd config [backport 2020.01]" #13503

Closed

fjmolinas mentioned this pull request Mar 3, 2020

cpu/arm: fixes necessary for toolchain upgrade [backport 2020.01] #13536

Merged

	BOARD_BLACKLIST := saml11-xpro
	BOARD_BLACKLIST := saml10-xpro saml11-xpro

	BOARD_BLACKLIST := saml11-xpro
	FEATURES_BLACKLIST += cortex-m23

Conversation

kaspar030 commented Feb 24, 2020 • edited by aabadie Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

benpicco commented Feb 24, 2020

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

kaspar030 commented Feb 24, 2020

Uh oh!

benpicco commented Feb 28, 2020

Uh oh!

fjmolinas commented Feb 28, 2020

Uh oh!

fjmolinas commented Mar 3, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kaspar030 commented Feb 24, 2020 •

edited by aabadie

Loading