kasjer commented on code in PR #3270:
URL: https://github.com/apache/mynewt-core/pull/3270#discussion_r1713593420
##########
hw/mcu/nordic/nrf_common/src/hal_qspi.c:
##########
@@ -248,43 +241,51 @@ nrf91k_qspi_sector_info(const struct hal_flash *dev, int
idx,
}
static int
-nrf91k_qspi_init(const struct hal_flash *dev)
+nrf_qspi_init(const struct hal_flash *dev)
{
- const nrf_qspi_prot_conf_t config0 = {
- .readoc = MYNEWT_VAL(QSPI_READOC),
- .writeoc = MYNEWT_VAL(QSPI_WRITEOC),
- .addrmode = MYNEWT_VAL(QSPI_ADDRMODE),
- .dpmconfig = MYNEWT_VAL(QSPI_DPMCONFIG)
- };
- const nrf_qspi_phy_conf_t config1 = {
- .sck_delay = MYNEWT_VAL(QSPI_SCK_DELAY),
- .dpmen = 0,
- .spi_mode = MYNEWT_VAL(QSPI_SPI_MODE),
- .sck_freq = MYNEWT_VAL(QSPI_SCK_FREQ),
- };
- /*
- * Configure pins
- */
- NRF_QSPI->PSEL.CSN = MYNEWT_VAL(QSPI_PIN_CS);
- NRF_QSPI->PSEL.SCK = MYNEWT_VAL(QSPI_PIN_SCK);
- NRF_QSPI->PSEL.IO0 = MYNEWT_VAL(QSPI_PIN_DIO0);
- NRF_QSPI->PSEL.IO1 = MYNEWT_VAL(QSPI_PIN_DIO1);
- /*
- * Setup only known fields of IFCONFIG0. Other bits may be set by erratas
code.
- */
- nrf_qspi_ifconfig0_set(NRF_QSPI, &config0);
- nrf_qspi_ifconfig1_set(NRF_QSPI, &config1);
-
- NRF_QSPI->XIPOFFSET = 0x12000000;
-
- NRF_QSPI->ENABLE = 1;
- NRF_QSPI->TASKS_ACTIVATE = 1;
- while (NRF_QSPI->EVENTS_READY == 0);
-
+ int rc;
+
+ nrfx_qspi_config_t config = {
+ .pins = {
+ .csn_pin = MYNEWT_VAL(QSPI_PIN_CS),
+ .sck_pin = MYNEWT_VAL(QSPI_PIN_SCK),
+ .io0_pin = MYNEWT_VAL(QSPI_PIN_DIO0),
+ .io1_pin = MYNEWT_VAL(QSPI_PIN_DIO1),
#if (MYNEWT_VAL(QSPI_READOC) > 2) || (MYNEWT_VAL(QSPI_WRITEOC) > 1)
- NRF_QSPI->PSEL.IO2 = MYNEWT_VAL(QSPI_PIN_DIO2);
- NRF_QSPI->PSEL.IO3 = MYNEWT_VAL(QSPI_PIN_DIO3);
+ .io2_pin = MYNEWT_VAL(QSPI_PIN_DIO2),
+ .io3_pin = MYNEWT_VAL(QSPI_PIN_DIO3),
+#else
+ .io2_pin = NRF_QSPI_PIN_NOT_CONNECTED,
+ .io3_pin = NRF_QSPI_PIN_NOT_CONNECTED,
#endif
+ },
+ .prot_if = {
+ .readoc = MYNEWT_VAL(QSPI_READOC),
+ .writeoc = MYNEWT_VAL(QSPI_WRITEOC),
+ .addrmode = MYNEWT_VAL(QSPI_ADDRMODE),
+ .dpmconfig = MYNEWT_VAL(QSPI_DPMCONFIG)
+ },
+ .phy_if = {
+ .sck_delay = MYNEWT_VAL(QSPI_SCK_DELAY),
+ .dpmen = 0,
+ .spi_mode = MYNEWT_VAL(QSPI_SPI_MODE),
+ .sck_freq = MYNEWT_VAL(QSPI_SCK_FREQ),
+ },
+ .xip_offset = MYNEWT_VAL(QSPI_XIP_OFFSET),
Review Comment:
`QSPI_XIP_OFFSET` is not defined for NRF52 based devices
##########
hw/mcu/nordic/nrf_common/src/hal_qspi.c:
##########
@@ -97,16 +94,13 @@ const struct hal_flash nrf91k_qspi_dev = {
};
static int
-nrf91k_qspi_read(const struct hal_flash *dev, uint32_t address,
+nrf_qspi_read(const struct hal_flash *dev, uint32_t address,
Review Comment:
alignment lost after name was changed
##########
hw/mcu/nordic/nrf_common/src/hal_qspi.c:
##########
@@ -219,26 +180,58 @@ nrf91k_qspi_write(const struct hal_flash *dev, uint32_t
address,
}
static int
-nrf91k_qspi_erase_sector(const struct hal_flash *dev,
+nrf_qspi_erase_sector(const struct hal_flash *dev,
uint32_t sector_address)
{
int8_t erases;
erases = MYNEWT_VAL(QSPI_FLASH_SECTOR_SIZE) / 4096;
while (erases-- > 0) {
- while ((NRF_QSPI->STATUS & QSPI_STATUS_READY_Msk) == 0);
- NRF_QSPI->EVENTS_READY = 0;
- NRF_QSPI->ERASE.PTR = sector_address;
- NRF_QSPI->ERASE.LEN = NRF_QSPI_ERASE_LEN_4KB;
- NRF_QSPI->TASKS_ERASESTART = 1;
- while (NRF_QSPI->EVENTS_READY == 0);
+ nrfx_qspi_erase(NRF_QSPI_ERASE_LEN_4KB, sector_address);
sector_address += 4096;
}
+
return 0;
}
static int
-nrf91k_qspi_sector_info(const struct hal_flash *dev, int idx,
+nrf_qspi_erase(const struct hal_flash *dev, uint32_t address,
Review Comment:
strange alignment
##########
hw/mcu/nordic/nrf_common/src/hal_flash.c:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <assert.h>
+#include <hal/hal_flash_int.h>
+#include "nrf.h"
+#include "nrf_hal.h"
+#include "nrfx_nvmc.h"
+
+#ifdef NRF51
+#define NRF_FLASH_SECTOR_SZ 1024
+#else
+#define NRF_FLASH_SECTOR_SZ 4096
+#endif
+
+#define NRF_FLASH_SECTOR_CNT (NRF_MEMORY_FLASH_SIZE / NRF_FLASH_SECTOR_SZ)
+
+static int nrf_flash_read(const struct hal_flash *dev, uint32_t address,
+ void *dst, uint32_t num_bytes);
+static int nrf_flash_write(const struct hal_flash *dev, uint32_t address,
+ const void *src, uint32_t num_bytes);
+static int nrf_flash_erase_sector(const struct hal_flash *dev,
+ uint32_t sector_address);
+static int nrf_flash_sector_info(const struct hal_flash *dev, int idx,
+ uint32_t *address, uint32_t *sz);
+static int nrf_flash_init(const struct hal_flash *dev);
+
+static const struct hal_flash_funcs nrf_flash_funcs = {
+ .hff_read = nrf_flash_read,
+ .hff_write = nrf_flash_write,
+ .hff_erase_sector = nrf_flash_erase_sector,
+ .hff_sector_info = nrf_flash_sector_info,
+ .hff_init = nrf_flash_init
+};
+
+const struct hal_flash nrf_flash_dev = {
+ .hf_itf = &nrf_flash_funcs,
+ .hf_base_addr = NRF_MEMORY_FLASH_BASE,
+ .hf_size = NRF_MEMORY_FLASH_SIZE,
+ .hf_sector_cnt = NRF_FLASH_SECTOR_CNT,
+ .hf_align = MYNEWT_VAL(MCU_FLASH_MIN_WRITE_SIZE),
+ .hf_erased_val = 0xff,
+};
+
+static int
+nrf_flash_read(const struct hal_flash *dev, uint32_t address, void *dst,
+ uint32_t num_bytes)
+{
+ memcpy(dst, (void *)address, num_bytes);
+ return 0;
+}
+
+static int
+nrf_flash_write(const struct hal_flash *dev, uint32_t address,
Review Comment:
second line alignment mismatch
##########
hw/mcu/nordic/nrf_common/src/hal_flash.c:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <assert.h>
+#include <hal/hal_flash_int.h>
+#include "nrf.h"
+#include "nrf_hal.h"
+#include "nrfx_nvmc.h"
Review Comment:
using `<` `>` pair would be more consistent
##########
hw/mcu/nordic/nrf_common/src/hal_qspi.c:
##########
@@ -219,26 +180,58 @@ nrf91k_qspi_write(const struct hal_flash *dev, uint32_t
address,
}
static int
-nrf91k_qspi_erase_sector(const struct hal_flash *dev,
+nrf_qspi_erase_sector(const struct hal_flash *dev,
Review Comment:
alignment lost after name was changed
##########
hw/mcu/nordic/nrf_common/src/hal_flash.c:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <assert.h>
+#include <hal/hal_flash_int.h>
+#include "nrf.h"
+#include "nrf_hal.h"
+#include "nrfx_nvmc.h"
+
+#ifdef NRF51
+#define NRF_FLASH_SECTOR_SZ 1024
+#else
+#define NRF_FLASH_SECTOR_SZ 4096
+#endif
+
+#define NRF_FLASH_SECTOR_CNT (NRF_MEMORY_FLASH_SIZE / NRF_FLASH_SECTOR_SZ)
+
+static int nrf_flash_read(const struct hal_flash *dev, uint32_t address,
+ void *dst, uint32_t num_bytes);
+static int nrf_flash_write(const struct hal_flash *dev, uint32_t address,
+ const void *src, uint32_t num_bytes);
+static int nrf_flash_erase_sector(const struct hal_flash *dev,
+ uint32_t sector_address);
+static int nrf_flash_sector_info(const struct hal_flash *dev, int idx,
+ uint32_t *address, uint32_t *sz);
+static int nrf_flash_init(const struct hal_flash *dev);
+
+static const struct hal_flash_funcs nrf_flash_funcs = {
+ .hff_read = nrf_flash_read,
+ .hff_write = nrf_flash_write,
+ .hff_erase_sector = nrf_flash_erase_sector,
+ .hff_sector_info = nrf_flash_sector_info,
+ .hff_init = nrf_flash_init
+};
+
+const struct hal_flash nrf_flash_dev = {
+ .hf_itf = &nrf_flash_funcs,
+ .hf_base_addr = NRF_MEMORY_FLASH_BASE,
+ .hf_size = NRF_MEMORY_FLASH_SIZE,
+ .hf_sector_cnt = NRF_FLASH_SECTOR_CNT,
+ .hf_align = MYNEWT_VAL(MCU_FLASH_MIN_WRITE_SIZE),
+ .hf_erased_val = 0xff,
+};
+
+static int
+nrf_flash_read(const struct hal_flash *dev, uint32_t address, void *dst,
Review Comment:
second line alignment mismatch
##########
hw/mcu/nordic/nrf_common/src/hal_qspi.c:
##########
@@ -149,20 +135,14 @@ nrf91k_qspi_read(const struct hal_flash *dev, uint32_t
address,
}
static int
-nrf91k_qspi_write(const struct hal_flash *dev, uint32_t address,
+nrf_qspi_write(const struct hal_flash *dev, uint32_t address,
Review Comment:
alignment lost after name was changed
##########
hw/mcu/nordic/nrf_common/src/hal_flash.c:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <assert.h>
+#include <hal/hal_flash_int.h>
+#include "nrf.h"
+#include "nrf_hal.h"
+#include "nrfx_nvmc.h"
+
+#ifdef NRF51
+#define NRF_FLASH_SECTOR_SZ 1024
+#else
+#define NRF_FLASH_SECTOR_SZ 4096
Review Comment:
tabs instead of spaces
##########
hw/mcu/nordic/nrf_common/src/hal_flash.c:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include <string.h>
+#include <assert.h>
+#include <hal/hal_flash_int.h>
+#include "nrf.h"
+#include "nrf_hal.h"
+#include "nrfx_nvmc.h"
+
+#ifdef NRF51
+#define NRF_FLASH_SECTOR_SZ 1024
+#else
+#define NRF_FLASH_SECTOR_SZ 4096
+#endif
+
+#define NRF_FLASH_SECTOR_CNT (NRF_MEMORY_FLASH_SIZE / NRF_FLASH_SECTOR_SZ)
+
+static int nrf_flash_read(const struct hal_flash *dev, uint32_t address,
+ void *dst, uint32_t num_bytes);
+static int nrf_flash_write(const struct hal_flash *dev, uint32_t address,
+ const void *src, uint32_t num_bytes);
+static int nrf_flash_erase_sector(const struct hal_flash *dev,
+ uint32_t sector_address);
+static int nrf_flash_sector_info(const struct hal_flash *dev, int idx,
+ uint32_t *address, uint32_t *sz);
+static int nrf_flash_init(const struct hal_flash *dev);
+
+static const struct hal_flash_funcs nrf_flash_funcs = {
+ .hff_read = nrf_flash_read,
+ .hff_write = nrf_flash_write,
+ .hff_erase_sector = nrf_flash_erase_sector,
+ .hff_sector_info = nrf_flash_sector_info,
+ .hff_init = nrf_flash_init
+};
+
+const struct hal_flash nrf_flash_dev = {
+ .hf_itf = &nrf_flash_funcs,
+ .hf_base_addr = NRF_MEMORY_FLASH_BASE,
+ .hf_size = NRF_MEMORY_FLASH_SIZE,
+ .hf_sector_cnt = NRF_FLASH_SECTOR_CNT,
+ .hf_align = MYNEWT_VAL(MCU_FLASH_MIN_WRITE_SIZE),
+ .hf_erased_val = 0xff,
+};
+
+static int
+nrf_flash_read(const struct hal_flash *dev, uint32_t address, void *dst,
+ uint32_t num_bytes)
+{
+ memcpy(dst, (void *)address, num_bytes);
+ return 0;
+}
+
+static int
+nrf_flash_write(const struct hal_flash *dev, uint32_t address,
+ const void *src, uint32_t num_bytes)
+{
+ int sr;
+
+ __HAL_DISABLE_INTERRUPTS(sr);
+ nrfx_nvmc_bytes_write(address, src, num_bytes);
+ __HAL_ENABLE_INTERRUPTS(sr);
+
+ return 0;
+}
+
+static int
+nrf_flash_erase_sector(const struct hal_flash *dev, uint32_t sector_address)
+{
+ int sr;
+ int rc;
+
+ __HAL_DISABLE_INTERRUPTS(sr);
+ rc = nrfx_nvmc_page_erase(sector_address);
+ __HAL_ENABLE_INTERRUPTS(sr);
+
+ if (rc != NRFX_SUCCESS) {
+ return -1;
+ } else {
+ return 0;
+ }
+}
+
+static int
+nrf_flash_sector_info(const struct hal_flash *dev, int idx,
Review Comment:
second line alignment mismatch
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
