search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.93k stars 6.65k forks source link

ARM: Move CMSIS out of main tree #23373

Closed ioannisg closed 4 years ago

ioannisg commented 4 years ago

Introduction

This is a proposal to move the CMSIS out of the main Zephyr repository.

Problem description

CMSIS is one of the two "hal" layers that are still part of the main Zephyr tree, residing inside the /ext folder.

Proposed change

We would like to remove the ARM CMSIS "hal" layer from the /ext directory of the Zephyr main tree and instead, place it in a "hal" repository in the Zephyrproject organization, which, ideally, will mirror the ARM CMSIS repository.

Detailed RFC

The repository content could be fetched using the west tool, as we do for all other HAL layers.

Dependencies

Not applicable.

Alternatives

Keep having the CMSIS as part of the main zephyr repository.

ioannisg commented 4 years ago

FYI @galak @stephanosio @MaureenHelm @carlescufi @erwango

Is this something we should try to agree upon and do for v2.3 release?

carlescufi commented 4 years ago

FYI @galak @stephanosio @MaureenHelm @carlescufi @erwango

Is this something we should try to agree upon and do for v2.3 release?

There was a "requirement" a while back that one should be able to build for qemu without any additional modules/repos. This is the reason we keep CMSIS in the tree. Perhaps @nashif can comment since I recall him being the one that first introduced this concept (which kind of makes sense to me as well).

ioannisg commented 4 years ago

It is not documented, anywhere that we need to build QEMU without additional repositories (to the best of my knowledge, of course)

Anyway, this does not even hold, today, at least for qemu_cortex_m0 (Nordic) and qemu_cortex_m3 (TI) that, I believe, require vendor HALs.

carlescufi commented 4 years ago

Anyway, this does not even hold, today, at least for qemu_cortex_m0 (Nordic) and qemu_cortex_m3 (TI) that, I believe, require vendor HALs.

Not for qemu_cortex_m3. The UART driver is in-tree.

ioannisg commented 4 years ago

Ah, OK. Are you sure this Soc/Board does not need TI HALs at all?

In any case the argument holds for the M0 Qemu

stephanosio commented 4 years ago

Unless there is something wrong with my current understanding of how things work, HALs (including CMSIS) need not reside in the main repository.

The first thing that CI does is to west update and this means that everything (including the to-be-modular-ised CMSIS) will get pulled before the CI builds tests and samples.

carlescufi commented 4 years ago

Unless there is something wrong with my current understanding of how things work, HALs (including CMSIS) need not reside in the main repository.

The first thing that CI does is to west update and this means that everything (including the to-be-modular-ised CMSIS) will get pulled before the CI builds tests and samples.

Yes, but the idea here was that downstream distros of zephyr that do not necessarily include those HALs can still run the qemu basic kernel tests.

carlescufi commented 4 years ago

Ah, OK. Are you sure this Soc/Board does not need TI HALs at all?

Almost sure.

In any case the argument holds for the M0 Qemu

Right, but this is new.

stephanosio commented 4 years ago

Re: CMSIS

ioannisg commented 4 years ago

@carlescufi FYI there will be one more QEMU platform that will probably require external HAL (STM32) support; the Cortex-M4F: #23185.

carlescufi commented 4 years ago

@stephanosio @ioannisg I have no objections to going forward with this. We should create a new repo for CMSIS that contains all the other applicable components as well (DSP in particular is of interest to many). I would however like to hear from @MaureenHelm, @galak and @nashif before we go ahead.

carlescufi commented 4 years ago

@stephanosio note that for CMSIS-DSP we will need to settle this first:

https://github.com/zephyrproject-rtos/zephyr/issues/7516

A discussion is ongoing at the board level regarding this.

stephanosio commented 4 years ago

@stephanosio note that for CMSIS-DSP we will need to settle this first:

7516

A discussion is ongoing at the board level regarding this.

@carlescufi In #21600, I have already integrated CMSIS-DSP to the Zephyr build system (with full Kconfig support for customisation) in the source code form and linking binary blobs will not be necessary; I suppose #7516 won't be relevant in that case?

In terms of licensing, all CMSIS components (including the CMSIS-DSP) are licensed Apache 2.0.

p.s. of course, users will be able to simply disable the Kconfig option for this and use the binary blobs provided by the SoC vendor if they wish.

galak commented 4 years ago

I'd ask if we need the qemu_cortex_m3 platform at all. Is mps2_an385 sufficient?

stephanosio commented 4 years ago

I'd ask if we need the qemu_cortex_m3 platform at all. Is mps2_an385 sufficient?

@galak https://github.com/zephyrproject-rtos/zephyr/issues/22870

carlescufi commented 4 years ago

@carlescufi In #21600, I have already integrated CMSIS-DSP to the Zephyr build system (with full Kconfig support for customisation) in the source code form and linking binary blobs will not be necessary; I suppose #7516 won't be relevant in that case?

@stephanosio maybe I'm missing something, so CMSIS-DSP is offered both in source code and library form in the CMSIS repo? I thought it was only available as libraries.

carlescufi commented 4 years ago

I'd ask if we need the qemu_cortex_m3 platform at all. Is mps2_an385 sufficient?

Agreed with @stephanosio. Once Cortex-M4 is supported in qemu, m3 is not required at all.

stephanosio commented 4 years ago

@stephanosio maybe I'm missing something, so CMSIS-DSP is offered both in source code and library form in the CMSIS repo? I thought it was only available as libraries.

Yep, the upstream CMSIS repository contains both the source code and pre-built libs for the CMSIS-DSP.

The source code can be found here: https://github.com/ARM-software/CMSIS_5/tree/develop/CMSIS/DSP/Source

nashif commented 4 years ago

I'd ask if we need the qemu_cortex_m3 platform at all. Is mps2_an385 sufficient?

We tried to remove it when we enabled mps2_an385 but there was some resistance due to some drivers and network tests depending on it if I remember correctly. IMO we should consolidate this and cleanup many of the qemu duplicates we have now, also for x86.

nashif commented 4 years ago

if we are going to remove cmsis and put it in a module, we should also remove the altera hal at the same time and probably other items in ext and complete abolish "ext/"

ioannisg commented 4 years ago

For ARMv7-M things are pretty clear; we don't need qemu_cortex_m3, we have mps2_an385 for a Cortex-M3 and we can make it run without MPU. We want to have Cortex-M4F - that's what #23185 will do.

ioannisg commented 4 years ago

if we are going to remove cmsis and put it in a module, we should also remove the altera hal at the same time and probably other items in ext and complete abolish "ext/"

Any particular reason these things need to happen "at the same time"? CMSIS and altera-hal are totally unrelated. Agree with abolishing /ext if possible - just that I am more keen of doing things in steps

galak commented 4 years ago

We need to sync whatever the CMSIS HAL repo would be w/TFM's CMSIS usage as well.

carlescufi commented 4 years ago

@stephanosio @ioannisg I will move forward with this. I think it should be done in steps:

  1. Create the repo
  2. Populate it with CMSIS-Core
  3. Move CMSIS-Core from ext/ to the repo
  4. Populate the repo with CMSIS-DSP
  5. Add CMSIS-DSP support in Zephyr