Add basic CMSIS core headers #384

Merged
kilograham merged 1 commit from cmsis into develop 2021-05-11 10:47:14 -05:00
kilograham commented 2021-05-05 15:26:44 -05:00 (Migrated from github.com)

Adds a new cmsis_core INTERFACE library, which provides the basic CMSIS macros/definitions, and RP2040.h and system_RP2040.h
By default the isr_ methods in the vector table are renamed to their CMSIS versions, but this can be overridden by defining PICO_CMSIS_RENAME_EXCEPTIONS=0

fixes #21

Adds a new cmsis_core INTERFACE library, which provides the basic CMSIS macros/definitions, and RP2040.h and system_RP2040.h By default the isr_ methods in the vector table are renamed to their CMSIS versions, but this can be overridden by defining PICO_CMSIS_RENAME_EXCEPTIONS=0 fixes #21
kilograham commented 2021-05-05 15:27:25 -05:00 (Migrated from github.com)

This depends on #382 for the exception/irq handler renaming

This depends on #382 for the exception/irq handler renaming
lurch (Migrated from github.com) reviewed 2021-05-11 11:24:49 -05:00
@ -0,0 +2,4 @@
# at least if the vendor/device is RaspberryPi/RP2040...
## PICO_CMAKE_CONFIG: PICO_CMSIS_PATH, directory to locate CMSIS installation, default="included stub CORE only impl", group=build
#if (DEFINED PICO_CMSIS_PATH)
lurch (Migrated from github.com) commented 2021-05-11 11:24:49 -05:00

It doesn't really matter, because this is all commented-out anyway, but should the PICO_CMSIS_PATH variables you've used here actually be PICO_CMSIS_CORE_PATH so that it matches up with the non-commented-out code below?

It doesn't really matter, because this is all commented-out anyway, but should the `PICO_CMSIS_PATH` variables you've used here actually be `PICO_CMSIS_CORE_PATH` so that it matches up with the non-commented-out code below?
lurch (Migrated from github.com) reviewed 2021-05-11 11:26:56 -05:00
@ -0,0 +7,4 @@
#ifndef _CMSIS_RENAME_EXCEPTIONS_H
#define _CMSIS_RENAME_EXCEPTIONS_H
// PICO_CONFIG: PICO_CMSIS_RENAME_EXCEPTIONS, Whether to rename SDK exceptions such as isr_nmi to their CMSIS equivalent i.e. NMI_Handler, type=bool, default=0, group=cmsis_core
lurch (Migrated from github.com) commented 2021-05-11 11:26:55 -05:00

This line says default=0, but line 14 seems to set the default to 1? 😕

This line says `default=0`, but line 14 seems to set the default to 1? :confused:
lurch (Migrated from github.com) reviewed 2021-05-11 11:31:07 -05:00
@ -0,0 +7,4 @@
/*
* Copyright (c) 2009-2018 Arm Limited. All rights reserved.
*
* SPDX-License-Identifier: Apache-2.0
lurch (Migrated from github.com) commented 2021-05-11 11:31:07 -05:00

If we're including Apache-licensed code into the SDK, do we need to mention something about this in our README.md ? 🤷 🤷

If we're including Apache-licensed code into the SDK, do we need to mention something about this in our README.md ? :shrug: :shrug:
lurch (Migrated from github.com) reviewed 2021-05-11 11:33:58 -05:00
@ -0,0 +1,275 @@
/******************************************************************************
* @file mpu_armv7.h
* @brief CMSIS MPU API for Armv7-M MPU
lurch (Migrated from github.com) commented 2021-05-11 11:33:58 -05:00

We probably don't need an Armv7-M header? 😉 Or does the CMSIS stuff fail to compile without it?

We probably don't need an Armv7-M header? :wink: Or does the CMSIS stuff fail to compile without it?
lurch (Migrated from github.com) reviewed 2021-05-11 11:37:13 -05:00
@ -0,0 +23,4 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*
* SPDX-License-Identifier: BSD-3-Clause
lurch (Migrated from github.com) commented 2021-05-11 11:37:13 -05:00

This file has both:

SPDX-License-Identifier: Apache-2.0
SPDX-License-Identifier: BSD-3-Clause

Does that mean it's dual-licensed, and the user can pick which licence they use it under? If that is the case, perhaps it's worth adding a quick comment explaining that?

This file has both: ``` SPDX-License-Identifier: Apache-2.0 SPDX-License-Identifier: BSD-3-Clause ``` Does that mean it's dual-licensed, and the user can pick which licence they use it under? If that is the case, perhaps it's worth adding a quick comment explaining that?
lurch commented 2021-05-11 11:41:29 -05:00 (Migrated from github.com)

FYI I didn't review this previously, because it was marked as a Draft PR.

FYI I didn't review this previously, because it was marked as a Draft PR.
kilograham (Migrated from github.com) reviewed 2021-05-11 13:34:39 -05:00
@ -0,0 +7,4 @@
#ifndef _CMSIS_RENAME_EXCEPTIONS_H
#define _CMSIS_RENAME_EXCEPTIONS_H
// PICO_CONFIG: PICO_CMSIS_RENAME_EXCEPTIONS, Whether to rename SDK exceptions such as isr_nmi to their CMSIS equivalent i.e. NMI_Handler, type=bool, default=0, group=cmsis_core
kilograham (Migrated from github.com) commented 2021-05-11 13:34:39 -05:00

please open a PR

please open a PR
kilograham (Migrated from github.com) reviewed 2021-05-11 13:35:21 -05:00
@ -0,0 +1,275 @@
/******************************************************************************
* @file mpu_armv7.h
* @brief CMSIS MPU API for Armv7-M MPU
kilograham (Migrated from github.com) commented 2021-05-11 13:35:21 -05:00

it is a badly named file

it is a badly named file
kilograham commented 2021-05-11 13:36:25 -05:00 (Migrated from github.com)

FYI I didn't review this previously, because it was marked as a Draft PR.

ah, makes sense!

> FYI I didn't review this previously, because it was marked as a Draft PR. ah, makes sense!
lurch (Migrated from github.com) reviewed 2021-05-11 14:57:29 -05:00
@ -0,0 +7,4 @@
#ifndef _CMSIS_RENAME_EXCEPTIONS_H
#define _CMSIS_RENAME_EXCEPTIONS_H
// PICO_CONFIG: PICO_CMSIS_RENAME_EXCEPTIONS, Whether to rename SDK exceptions such as isr_nmi to their CMSIS equivalent i.e. NMI_Handler, type=bool, default=0, group=cmsis_core
lurch (Migrated from github.com) commented 2021-05-11 14:57:28 -05:00

What's the intended behaviour - change the PICO_CONFIG default to 1, or change the actual #define to 0 ?

What's the intended behaviour - change the PICO_CONFIG default to 1, or change the actual #define to 0 ?
kilograham (Migrated from github.com) reviewed 2021-05-11 18:24:19 -05:00
@ -0,0 +7,4 @@
#ifndef _CMSIS_RENAME_EXCEPTIONS_H
#define _CMSIS_RENAME_EXCEPTIONS_H
// PICO_CONFIG: PICO_CMSIS_RENAME_EXCEPTIONS, Whether to rename SDK exceptions such as isr_nmi to their CMSIS equivalent i.e. NMI_Handler, type=bool, default=0, group=cmsis_core
kilograham (Migrated from github.com) commented 2021-05-11 18:24:18 -05:00

yeah the #define is correct i.e. 1

yeah the #define is correct i.e. 1
lurch commented 2021-05-11 18:38:56 -05:00 (Migrated from github.com)

ping @ghollingworth for queries about the mixed-licensing.

ping @ghollingworth for queries about the mixed-licensing.
liamfraser (Migrated from github.com) reviewed 2021-05-18 10:57:26 -05:00
liamfraser (Migrated from github.com) left a comment

LGTM

LGTM
ghollingworth commented 2021-06-07 01:04:44 -05:00 (Migrated from github.com)

Graham,

We should add the CMSIS license to this LICENSE file and just list the path of files that are covered by it.

Gordon

Graham, We should add the CMSIS license to this LICENSE file and just list the path of files that are covered by it. Gordon
kilograham commented 2021-06-07 09:22:25 -05:00 (Migrated from github.com)

@ghollingworth you are a day late and a dollar short. this is already merged/released. Also

We should add the CMSIS license to this LICENSE file and just list the path of files that are covered by it.

What is this license? list the files where?

might be simplest to submit a PR.

@ghollingworth you are a day late and a dollar short. this is already merged/released. Also > We should add the CMSIS license to this LICENSE file and just list the path of files that are covered by it. What is _this_ license? list the files where? might be simplest to submit a PR.
lurch commented 2021-06-07 09:29:26 -05:00 (Migrated from github.com)

I suspect Gordon might have been thinking something along the lines of https://github.com/raspberrypi/firmware/blob/master/README.md ? 🤷

EDIT: Or https://github.com/raspberrypi/rpi-eeprom/blob/master/LICENSE ? 🤷

I suspect Gordon might have been thinking something along the lines of https://github.com/raspberrypi/firmware/blob/master/README.md ? :shrug: EDIT: Or https://github.com/raspberrypi/rpi-eeprom/blob/master/LICENSE ? :shrug:
ghollingworth commented 2021-06-08 01:11:35 -05:00 (Migrated from github.com)

No I was thinking along the lines of the debian copyright format:

https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/

This allows something like the following:

Files: *
Copyright: 1975-2010 Ulla Upstream
License: GPL-2+

Files: debian/*
Copyright: 2010 Daniela Debianizer
License: GPL-2+

Files: debian/patches/fancy-feature
Copyright: 2010 Daniela Debianizer
License: GPL-3+

Files: */*.1
Copyright: 2010 Manuela Manpager
License: GPL-2+
No I was thinking along the lines of the debian copyright format: https://www.debian.org/doc/packaging-manuals/copyright-format/1.0/ This allows something like the following: ``` Files: * Copyright: 1975-2010 Ulla Upstream License: GPL-2+ Files: debian/* Copyright: 2010 Daniela Debianizer License: GPL-2+ Files: debian/patches/fancy-feature Copyright: 2010 Daniela Debianizer License: GPL-3+ Files: */*.1 Copyright: 2010 Manuela Manpager License: GPL-2+ ```
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-raspberrypi/pico-sdk!384
No description provided.