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


Amit Kucheria
 

On Tue, Nov 15, 2016 at 2:18 AM, Maureen Helm <maureen.helm(a)nxp.com> wrote:
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'm skeptical about how this'll work because all the vendor SDK
definitions will leak into drivers (I'm currently refactoring my SPI
driver for nRF to work with hal files in ext/, because the build
system includes them by default, though I have no real use for them).
Since there isn't a standard way for vendors to do SDKs, every driver
will end up looking different within the same subsystem. When there
are no easy patterns, there will be less code reuse in the subsystem.

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.

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.
That would be appreciated if that is the official position. Then we
won't have SoC and platform ports starting with clean, native code
that then needs to be converted over to use the HAL after submission.
It is a waste of everyone's time.

Cheers,
Amit

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