btashton commented on a change in pull request #2735: URL: https://github.com/apache/incubator-nuttx/pull/2735#discussion_r563327349
########## File path: arch/arm/src/nrf52/Make.defs ########## @@ -163,3 +163,89 @@ endif ifeq ($(CONFIG_PM),y) CHIP_CSRCS += nrf52_pminitialize.c endif + +ifeq ($(CONFIG_NRF52_SOFTDEVICE_CONTROLLER),y) + +NRFXLIB_UNPACK := sdk-nrfxlib +NRFXLIB_VER := 1.4.2 +NRFXLIB_REF := v$(NRFXLIB_VER) +NRFXLIB_TGZ := $(NRFXLIB_REF).tar.gz +NRFXLIB_URL := https://github.com/nrfconnect/sdk-nrfxlib/archive + +NRFX_UNPACK := nrfx +NRFX_VER := 2.4.0 +NRFX_REF := v$(NRFX_VER) +NRFX_TGZ := $(NRFX_REF).tar.gz +NRFX_URL := https://github.com/NordicSemiconductor/nrfx/archive + +CMSIS_UNPACK := CMSIS_5 +CMSIS_VER := 5.7.0 +CMSIS_REF := $(CMSIS_VER) +CMSIS_TGZ := $(CMSIS_REF).tar.gz +CMSIS_URL := https://github.com/ARM-software/CMSIS_5/archive + +$(NRFXLIB_TGZ): + $(Q) echo "Downloading: NRFXLIB" + $(Q) curl -L $(NRFXLIB_URL)/$(NRFXLIB_TGZ) -o chip/$(NRFXLIB_TGZ) + +$(NRFX_TGZ): + $(Q) echo "Downloading: NRFX" + $(Q) curl -L $(NRFX_URL)/$(NRFX_TGZ) -o chip/$(NRFX_TGZ) + +$(CMSIS_TGZ): + $(Q) echo "Downloading: CMSIS" + $(Q) curl -L $(CMSIS_URL)/$(CMSIS_TGZ) -o chip/$(CMSIS_TGZ) + +chip/$(NRFXLIB_UNPACK): $(NRFXLIB_TGZ) + $(Q) echo "Unpacking: NRXFLIB" + $(Q) cd chip && tar zxf $(NRFXLIB_TGZ) + $(Q) mv chip/$(NRFXLIB_UNPACK)-$(NRFXLIB_VER)* chip/$(NRFXLIB_UNPACK) + $(Q) touch chip/$(NRFXLIB_UNPACK) + +chip/$(NRFX_UNPACK): $(NRFX_TGZ) + $(Q) echo "Unpacking: NRXF" + $(Q) cd chip && tar zxf $(NRFX_TGZ) + $(Q) mv chip/$(NRFX_UNPACK)-$(NRFX_VER)* chip/$(NRFX_UNPACK) + $(Q) touch chip/$(NRFX_UNPACK) + +chip/$(CMSIS_UNPACK): $(CMSIS_TGZ) Review comment: I'm super uncomfortable with this, this is pulling in an entire new HAL for ARM. I don't think we actually need to do this. We should be able to just stub any needed interfaces for the libraries. ########## File path: arch/arm/src/nrf52/Make.defs ########## @@ -163,3 +163,89 @@ endif ifeq ($(CONFIG_PM),y) CHIP_CSRCS += nrf52_pminitialize.c endif + +ifeq ($(CONFIG_NRF52_SOFTDEVICE_CONTROLLER),y) + +NRFXLIB_UNPACK := sdk-nrfxlib +NRFXLIB_VER := 1.4.2 +NRFXLIB_REF := v$(NRFXLIB_VER) +NRFXLIB_TGZ := $(NRFXLIB_REF).tar.gz +NRFXLIB_URL := https://github.com/nrfconnect/sdk-nrfxlib/archive + +NRFX_UNPACK := nrfx +NRFX_VER := 2.4.0 +NRFX_REF := v$(NRFX_VER) +NRFX_TGZ := $(NRFX_REF).tar.gz +NRFX_URL := https://github.com/NordicSemiconductor/nrfx/archive + +CMSIS_UNPACK := CMSIS_5 +CMSIS_VER := 5.7.0 +CMSIS_REF := $(CMSIS_VER) +CMSIS_TGZ := $(CMSIS_REF).tar.gz +CMSIS_URL := https://github.com/ARM-software/CMSIS_5/archive + +$(NRFXLIB_TGZ): + $(Q) echo "Downloading: NRFXLIB" + $(Q) curl -L $(NRFXLIB_URL)/$(NRFXLIB_TGZ) -o chip/$(NRFXLIB_TGZ) + +$(NRFX_TGZ): Review comment: What do we actually need out of here that we cannot supply ourselves. This is pulling in the whole nrf HAL ########## File path: arch/arm/src/nrf52/Make.defs ########## @@ -163,3 +163,89 @@ endif ifeq ($(CONFIG_PM),y) CHIP_CSRCS += nrf52_pminitialize.c endif + +ifeq ($(CONFIG_NRF52_SOFTDEVICE_CONTROLLER),y) + +NRFXLIB_UNPACK := sdk-nrfxlib +NRFXLIB_VER := 1.4.2 +NRFXLIB_REF := v$(NRFXLIB_VER) +NRFXLIB_TGZ := $(NRFXLIB_REF).tar.gz +NRFXLIB_URL := https://github.com/nrfconnect/sdk-nrfxlib/archive + +NRFX_UNPACK := nrfx +NRFX_VER := 2.4.0 +NRFX_REF := v$(NRFX_VER) +NRFX_TGZ := $(NRFX_REF).tar.gz +NRFX_URL := https://github.com/NordicSemiconductor/nrfx/archive + +CMSIS_UNPACK := CMSIS_5 +CMSIS_VER := 5.7.0 +CMSIS_REF := $(CMSIS_VER) +CMSIS_TGZ := $(CMSIS_REF).tar.gz +CMSIS_URL := https://github.com/ARM-software/CMSIS_5/archive + +$(NRFXLIB_TGZ): Review comment: This library seems like the only one we actually need and we should be able to only pull the lib files out. ########## File path: arch/arm/src/nrf52/Make.defs ########## @@ -163,3 +163,89 @@ endif ifeq ($(CONFIG_PM),y) CHIP_CSRCS += nrf52_pminitialize.c endif + +ifeq ($(CONFIG_NRF52_SOFTDEVICE_CONTROLLER),y) + +NRFXLIB_UNPACK := sdk-nrfxlib +NRFXLIB_VER := 1.4.2 +NRFXLIB_REF := v$(NRFXLIB_VER) +NRFXLIB_TGZ := $(NRFXLIB_REF).tar.gz +NRFXLIB_URL := https://github.com/nrfconnect/sdk-nrfxlib/archive + +NRFX_UNPACK := nrfx +NRFX_VER := 2.4.0 +NRFX_REF := v$(NRFX_VER) +NRFX_TGZ := $(NRFX_REF).tar.gz +NRFX_URL := https://github.com/NordicSemiconductor/nrfx/archive + +CMSIS_UNPACK := CMSIS_5 +CMSIS_VER := 5.7.0 +CMSIS_REF := $(CMSIS_VER) +CMSIS_TGZ := $(CMSIS_REF).tar.gz +CMSIS_URL := https://github.com/ARM-software/CMSIS_5/archive + +$(NRFXLIB_TGZ): + $(Q) echo "Downloading: NRFXLIB" + $(Q) curl -L $(NRFXLIB_URL)/$(NRFXLIB_TGZ) -o chip/$(NRFXLIB_TGZ) + +$(NRFX_TGZ): + $(Q) echo "Downloading: NRFX" + $(Q) curl -L $(NRFX_URL)/$(NRFX_TGZ) -o chip/$(NRFX_TGZ) + +$(CMSIS_TGZ): + $(Q) echo "Downloading: CMSIS" + $(Q) curl -L $(CMSIS_URL)/$(CMSIS_TGZ) -o chip/$(CMSIS_TGZ) + +chip/$(NRFXLIB_UNPACK): $(NRFXLIB_TGZ) + $(Q) echo "Unpacking: NRXFLIB" + $(Q) cd chip && tar zxf $(NRFXLIB_TGZ) + $(Q) mv chip/$(NRFXLIB_UNPACK)-$(NRFXLIB_VER)* chip/$(NRFXLIB_UNPACK) + $(Q) touch chip/$(NRFXLIB_UNPACK) + +chip/$(NRFX_UNPACK): $(NRFX_TGZ) + $(Q) echo "Unpacking: NRXF" + $(Q) cd chip && tar zxf $(NRFX_TGZ) + $(Q) mv chip/$(NRFX_UNPACK)-$(NRFX_VER)* chip/$(NRFX_UNPACK) + $(Q) touch chip/$(NRFX_UNPACK) + +chip/$(CMSIS_UNPACK): $(CMSIS_TGZ) Review comment: But why not just provide our header and only use the lib files. Then we avoid having to pull in the other two libraries? ########## File path: arch/arm/src/nrf52/Make.defs ########## @@ -163,3 +163,89 @@ endif ifeq ($(CONFIG_PM),y) CHIP_CSRCS += nrf52_pminitialize.c endif + +ifeq ($(CONFIG_NRF52_SOFTDEVICE_CONTROLLER),y) + +NRFXLIB_UNPACK := sdk-nrfxlib +NRFXLIB_VER := 1.4.2 +NRFXLIB_REF := v$(NRFXLIB_VER) +NRFXLIB_TGZ := $(NRFXLIB_REF).tar.gz +NRFXLIB_URL := https://github.com/nrfconnect/sdk-nrfxlib/archive + +NRFX_UNPACK := nrfx +NRFX_VER := 2.4.0 +NRFX_REF := v$(NRFX_VER) +NRFX_TGZ := $(NRFX_REF).tar.gz +NRFX_URL := https://github.com/NordicSemiconductor/nrfx/archive + +CMSIS_UNPACK := CMSIS_5 +CMSIS_VER := 5.7.0 +CMSIS_REF := $(CMSIS_VER) +CMSIS_TGZ := $(CMSIS_REF).tar.gz +CMSIS_URL := https://github.com/ARM-software/CMSIS_5/archive + +$(NRFXLIB_TGZ): + $(Q) echo "Downloading: NRFXLIB" + $(Q) curl -L $(NRFXLIB_URL)/$(NRFXLIB_TGZ) -o chip/$(NRFXLIB_TGZ) + +$(NRFX_TGZ): + $(Q) echo "Downloading: NRFX" + $(Q) curl -L $(NRFX_URL)/$(NRFX_TGZ) -o chip/$(NRFX_TGZ) + +$(CMSIS_TGZ): + $(Q) echo "Downloading: CMSIS" + $(Q) curl -L $(CMSIS_URL)/$(CMSIS_TGZ) -o chip/$(CMSIS_TGZ) + +chip/$(NRFXLIB_UNPACK): $(NRFXLIB_TGZ) + $(Q) echo "Unpacking: NRXFLIB" + $(Q) cd chip && tar zxf $(NRFXLIB_TGZ) + $(Q) mv chip/$(NRFXLIB_UNPACK)-$(NRFXLIB_VER)* chip/$(NRFXLIB_UNPACK) + $(Q) touch chip/$(NRFXLIB_UNPACK) + +chip/$(NRFX_UNPACK): $(NRFX_TGZ) + $(Q) echo "Unpacking: NRXF" + $(Q) cd chip && tar zxf $(NRFX_TGZ) + $(Q) mv chip/$(NRFX_UNPACK)-$(NRFX_VER)* chip/$(NRFX_UNPACK) + $(Q) touch chip/$(NRFX_UNPACK) + +chip/$(CMSIS_UNPACK): $(CMSIS_TGZ) Review comment: I'm only talking about the interfaces that we need for this that should be a quite small surface. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org