Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hw/mcu/nordic: Add common flash and QSPI flash HAL #3270

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

m-gorecki
Copy link
Contributor

No description provided.

@m-gorecki m-gorecki force-pushed the common_flash branch 2 times, most recently from 75ae1f3 to e58a064 Compare July 18, 2024 10:05
@m-gorecki m-gorecki force-pushed the common_flash branch 2 times, most recently from 068357a to 9da67d4 Compare July 22, 2024 07:55
@m-gorecki m-gorecki marked this pull request as ready for review July 22, 2024 08:17
Comment on lines 22 to 25
#include "nrf.h"
#include "nrf_hal.h"
#include <hal/hal_flash_int.h>
#include "nrfx_nvmc.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixing <> with "" looks inconsistent

Comment on lines 35 to 42
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with shorter function names second line is misaligned.

Same for hal_qspi.c

.spi_mode = MYNEWT_VAL(QSPI_SPI_MODE),
.sck_freq = MYNEWT_VAL(QSPI_SCK_FREQ),
},
.xip_offset = MYNEWT_VAL(QSPI_XIP_OFFSET),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QSPI_XIP_OFFSET is not defined for NRF52 based devices

@@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment lost after name was changed

@@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment lost after name was changed

@@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment lost after name was changed

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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strange alignment

Comment on lines +28 to +30
#define NRF_FLASH_SECTOR_SZ 1024
#else
#define NRF_FLASH_SECTOR_SZ 4096
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tabs instead of spaces

Comment on lines +23 to +25
#include "nrf.h"
#include "nrf_hal.h"
#include "nrfx_nvmc.h"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using < > pair would be more consistent

};

static int
nrf_flash_read(const struct hal_flash *dev, uint32_t address, void *dst,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second line alignment mismatch

}

static int
nrf_flash_write(const struct hal_flash *dev, uint32_t address,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second line alignment mismatch

}

static int
nrf_flash_sector_info(const struct hal_flash *dev, int idx,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

second line alignment mismatch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants