Re: HALs in Zephyr (was Re: STM32Cube SDK in Zephyr)


Maureen Helm
 

Hi Fabien,

I'm also interested in this topic. I posted several weeks ago on gerrit some
patches to add support for the STM32L4 family. The reviewers wanted me to
import the ST headers (they were not there at the time) to reuse the
peripheral's base register addresses, I do not fully agree with it but this is just a
few defines from ST headers that follow the same coding convention as Zephyr
so that's fine. Now the reviewers also want me to start using the structure that
defines the peripherals registers from the ST headers, but here that's where it
starts to bothering me: this code does *NOT* follow the Zephyr coding style at
all.
You're right, it doesn't follow the Zephyr coding style, which is why we are importing these headers into ext/ and not into arch/arm/. Their style is defined by the ARM Cortex Microcontroller Software Interface Standard (CMSIS), specifically the CMSIS-CORE component of the standard, and has been widely adopted by ARM Cortex-M SoC vendors.
http://www.keil.com/pack/doc/CMSIS/Core/html/index.html

By reusing existing CMSIS peripheral register definitions from the SoC vendors as-is, there is a lot less custom code we need to write, test, and maintain for Zephyr. By importing them into ext/, we're keeping non-conformant code imported from other sources separate from the rest of the tree; everything else is expected to follow the Zephyr coding style.

As for vendor driver SDKs that are built on top of CMSIS register definitions, we are still in a bit of a learning period. For NXP SoCs, I would like to leverage the Kinetis SDK (ksdk) drivers with shims as much as possible for the same reasons as the CMSIS peripheral register definitions - less custom code to write, test, and maintain. For ST SoCs, Erwan recently started a similar exercise with STM32Cube. We've only done this for a handful of drivers, though, so it's possible we will find issues or incompatibilities as we fan out. When that happens, we should try to address them upstream with the vendors whenever possible.


I feel that if I have something that works, that does not duplicate existing code,
and that follow the existing coding style, my code should be merged as-is. But
right now we are asking me the opposite:
to start using symbols that do not follow the coding style.

If that's what maintainers want, I will follow even if I don't like it, but at least I
would love to see an official guideline from the maintainers of what is the
preferred approach here.
I agree that we need to publish some guidelines. I'll work on writing an ARM SoC porting guide over the next few weeks, and hope you'll be willing to provide some feedback on it when I have a draft ready.


About STM32Cube CMSIS files:
Proposition for now is to move progressively towards the support of
STM32Cube on available STM32 based boards and that new boards include
STM32Cube CMSIS files from start.
Shouldn't you import all possible HALs _today_ in order to enforce
that? Otherwise it'll lead to unnecessary churn when people are doing
platform/SoC ports.
I disagree with preemptively importing all the HALs today as that would clutter the tree with a lot of unused code. We also don't know which SoCs people will want to port to. I'll make sure to include a section about importing HALs in the above-mentioned ARM SoC porting guide.

Join devel@lists.zephyrproject.org to automatically receive all group messages.