[RFC] Peripherals cleanup in nRF52xxx BSPs

[Andrzej Kaczmarek <an...@codecoup.pl>]

Hi all,

It was discussed before [1] that MCU peripherals should be defined in
hw/mcu package instead of hw/bsp. Seems like everyone agreed on this, but
as far as I see everything is still unfortunately mixed between both
packages. I experienced this when trying to create new BSP for some eval
board I
 use... there's a lot of duplicated code created by c&p, settings are
defined in different packages which is confusing and makes BSPs
inconsistent in terms of which peripherals can be used (even though they
share the same MCU). So I created this RFC instead...


What changed:
* A
ll syscfg settings
 related to MCU are *defined* in hw/mcu package and then their *values* are
overridden in BSP.
 Previously we had (some) peripherals defined in hw/mcu package and their
configuration settings (like pins) defined in hw/bsp so basically each BSP
could have own names for these settings (they did not due to c&p but many
BSPs are lacking settings for peripherals). Package-level restrictions are
added to make sure enabled peripherals are configured properly (i.e. have
pins assigned).

* hw/mcu package defines syscfg settings which BSP shall override to
specify which MCU it uses. This is used at the moment to enforce
package-level restrictions. I know there are already similar symbols in
BSPs, but the problem is again that they are defined in each hw/bsp
packages instead of being defined in one place and then have values
overridden.

* Common code to create and init devices for peripherals is added to hw/mcu
package and hal_bsp_init() just calls this function to ensure each BSP
provides the same devices, as configured
.
* Soft PWM is left as a part of BSP
.
* Bit-banger UART is left as a part of BSP. Previously nRF52832 BSPs
created uart1 device as bit-banger UART but this does not seem right to me
since this is not a regular MCU UART, so updated BSPs create bit-banger
UART as uartbb0 device instead (uart1 is reserved for UART1 on nRF52840).

The most obvious problem here is that existing BSPs do not build with
refactored hw/mcu package and probably we should allow users to transition
smoothly to new package. For this reason I created an exact copy of
pre-refactor package and called it nrf52xxx-compat so updating BSP to 1.5
will just require changing dependency. After 1.5 release we remove compat
package and force all users to switch to new nrf52xxx on 1.6. I know this
just duplicates existing code, but I do not have any better idea here and I
would prefer to avoid creating some nasty glue layers which will eventually
transform one mess into another.

For now I updated nrf52dk and nrf52840pdk packages and if these changes are
accepted I can update all BSPs in core.

Here's PR: https://github.com/apache/mynewt-core/pull/1313.
Comments?

[1]
http://mail-archives.apache.org/mod_mbox/mynewt-dev/201802.mbox/%3CC7D6A5CB-3D7F-4541-BB7B-035EB2DC98D6%40runtime.io%3E

Best,
Andrzej

Re: [RFC] Peripherals cleanup in nRF52xxx BSPs

[Andrzej Kaczmarek <an...@codecoup.pl>]

Hi,

Since there were no objections here and PR was reviewed and approved, it
has been merged. This means if anyone has any out-of-tree BSP based on
nR52xxx, you will need to either temporarily switch it to use
"hw/mcu/nordic/nrf52xxx-compat" package or make necessary adjustments in
BSP.
I will update all BSPs in mynewt-core in separate PR(s), for now they use
compat package.

Best,
Andrzej


On Fri, Aug 3, 2018 at 1:02 PM Andrzej Kaczmarek <
andrzej.kaczmarek@codecoup.pl> wrote:

> Hi all,
>
> It was discussed before [1] that MCU peripherals should be defined in
> hw/mcu package instead of hw/bsp. Seems like everyone agreed on this, but
> as far as I see everything is still unfortunately mixed between both
> packages. I experienced this when trying to create new BSP for some eval
> board I
>  use... there's a lot of duplicated code created by c&p, settings are
> defined in different packages which is confusing and makes BSPs
> inconsistent in terms of which peripherals can be used (even though they
> share the same MCU). So I created this RFC instead...
>
>
> What changed:
> * A
> ll syscfg settings
>  related to MCU are *defined* in hw/mcu package and then their *values*
> are overridden in BSP.
>  Previously we had (some) peripherals defined in hw/mcu package and their
> configuration settings (like pins) defined in hw/bsp so basically each BSP
> could have own names for these settings (they did not due to c&p but many
> BSPs are lacking settings for peripherals). Package-level restrictions are
> added to make sure enabled peripherals are configured properly (i.e. have
> pins assigned).
>
> * hw/mcu package defines syscfg settings which BSP shall override to
> specify which MCU it uses. This is used at the moment to enforce
> package-level restrictions. I know there are already similar symbols in
> BSPs, but the problem is again that they are defined in each hw/bsp
> packages instead of being defined in one place and then have values
> overridden.
>
> * Common code to create and init devices for peripherals is added to
> hw/mcu package and hal_bsp_init() just calls this function to ensure each
> BSP provides the same devices, as configured
> .
> * Soft PWM is left as a part of BSP
> .
> * Bit-banger UART is left as a part of BSP. Previously nRF52832 BSPs
> created uart1 device as bit-banger UART but this does not seem right to me
> since this is not a regular MCU UART, so updated BSPs create bit-banger
> UART as uartbb0 device instead (uart1 is reserved for UART1 on nRF52840).
>
> The most obvious problem here is that existing BSPs do not build with
> refactored hw/mcu package and probably we should allow users to transition
> smoothly to new package. For this reason I created an exact copy of
> pre-refactor package and called it nrf52xxx-compat so updating BSP to 1.5
> will just require changing dependency. After 1.5 release we remove compat
> package and force all users to switch to new nrf52xxx on 1.6. I know this
> just duplicates existing code, but I do not have any better idea here and I
> would prefer to avoid creating some nasty glue layers which will eventually
> transform one mess into another.
>
> For now I updated nrf52dk and nrf52840pdk packages and if these changes
> are accepted I can update all BSPs in core.
>
> Here's PR: https://github.com/apache/mynewt-core/pull/1313.
> Comments?
>
> [1]
> http://mail-archives.apache.org/mod_mbox/mynewt-dev/201802.mbox/%3CC7D6A5CB-3D7F-4541-BB7B-035EB2DC98D6%40runtime.io%3E
>
> Best,
> Andrzej
>
>