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

drivers: flash: Add one byte read write emulation to qspi nor nrf #305

Merged
merged 7 commits into from
May 29, 2020
2 changes: 2 additions & 0 deletions cmake/app/boilerplate.cmake
Original file line number Diff line number Diff line change
@@ -89,6 +89,8 @@ if((NOT DEFINED ZEPHYR_BASE) AND (DEFINED ENV_ZEPHYR_BASE))
set(ZEPHYR_BASE ${ENV_ZEPHYR_BASE} CACHE PATH "Zephyr base")
endif()

find_package(ZephyrBuildConfiguration NAMES ZephyrBuild PATHS ${ZEPHYR_BASE}/../* QUIET NO_DEFAULT_PATH NO_POLICY_SCOPE)

# Note any later project() resets PROJECT_SOURCE_DIR
file(TO_CMAKE_PATH "${ZEPHYR_BASE}" PROJECT_SOURCE_DIR)

12 changes: 12 additions & 0 deletions doc/guides/env_vars.rst
Original file line number Diff line number Diff line change
@@ -106,6 +106,18 @@ To get this value back into your current terminal environment, **you must run**
The value will be lost if you close the window, etc.; run ``zephyr-env.cmd``
again to get it back.

Option 4: Using Zephyr Build Configuration CMake package
--------------------------------------------------------

Choose this option if you want to make those variable settings shared among all
users of your project.

Using a :ref:`cmake_build_config_package` allows you to commit the shared
settings into the repository, so that all users can share them.

It also removes the need for running ``source zephyr-env.sh`` or
``zephyr-env.cmd`` when opening a new terminal.

.. _zephyr-env:

Zephyr Environment Scripts
67 changes: 67 additions & 0 deletions doc/guides/zephyr_cmake_package.rst
Original file line number Diff line number Diff line change
@@ -382,6 +382,72 @@ Such a CMakeLists.txt could look as:

project(my_first_app)

.. _cmake_build_config_package:

Zephyr Build Configuration CMake package
****************************************

The Zephyr Build Configuration CMake package provides a possibility for a Zephyr based project to
control Zephyr build settings in a generic way.

It is similar to the use of ``.zephyrrc`` but with the possibility to automatically allow all users
to share the build configuration through the project repository.
But it also allows more advanced use cases than a ``.zephyrrc``-file, such as loading of additional
CMake boilerplate code.

The Zephyr Build Configuration CMake package will be loaded in the Zephyr boilerplate code after
initial properties and ``ZEPHYR_BASE`` has been defined, but before CMake code execution.
This allows the Zephyr Build Configuration CMake package to setup or extend properties such as:
``DTS_ROOT``, ``BOARD_ROOT``, ``TOOLCHAIN_ROOT`` / other toolchain setup, fixed overlays, and any
other property that can be controlled. It also allows inclusion of additional boilerplate code.

To provide a Zephyr Build Configuration CMake package, create ``ZephyrBuildConfig.cmake`` and place
it in a Zephyr workspace top-level folder as:

.. code-block:: none

<projects>/zephyr-workspace
├── zephyr
├── ...
└── zephyr application (can be named anything)
└── share/zephyrbuild-package/cmake/ZephyrBuildConfig.cmake

The Zephyr Build Configuration CMake package will not search in any CMake default search paths, and
thus cannot be installed in the CMake package registry. There will be no version checking on the
Zephyr Build Configuration package.

.. note:: ``share/zephyrbuild-package/cmake/ZephyrBuildConfig.cmake`` follows the same folder
structure as the Zephyr CMake package.

It is possible to place ``ZephyrBuildConfig.cmake`` directly in a
``<zephyr application>/cmake`` folder or another folder, as long as that folder is
honoring the `CMake package search`_ algorithm.

A sample ``ZephyrBuildConfig.cmake`` can be seen below.

.. code-block:: cmake

# ZephyrBuildConfig.cmake sample code

# To ensure final path is absolute and does not contain ../.. in variable.
get_filename_component(APPLICATION_PROJECT_DIR
${CMAKE_CURRENT_LIST_DIR}/../../..
ABSOLUTE
)

# Add this project to list of board roots
list(APPEND BOARD_ROOT ${APPLICATION_PROJECT_DIR})

# Default to GNU Arm Embedded toolchain if no toolchain is set
if(NOT ENV{ZEPHYR_TOOLCHAIN_VARIANT})
set(ZEPHYR_TOOLCHAIN_VARIANT gnuarmemb)
find_program(GNU_ARM_GCC arm-none-eabi-gcc)
if(NOT ${GNU_ARM_GCC} STREQUAL GNU_ARM_GCC-NOTFOUND)
# The toolchain root is located above the path to the compiler.
get_filename_component(GNUARMEMB_TOOLCHAIN_PATH ${GNU_ARM_GCC}/../.. ABSOLUTE)
endif()
endif()

Zephyr CMake package source code
********************************

@@ -428,3 +494,4 @@ The following is an overview of those files
.. _CMake package: https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html
.. _CMake user package registry: https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#user-package-registry
.. _CMake package version: https://cmake.org/cmake/help/latest/command/find_package.html#version-selection
.. _CMake package search: https://cmake.org/cmake/help/latest/command/find_package.html#search-procedure
12 changes: 12 additions & 0 deletions drivers/flash/Kconfig.nordic_qspi_nor
Original file line number Diff line number Diff line change
@@ -29,4 +29,16 @@ config NORDIC_QSPI_NOR_FLASH_LAYOUT_PAGE_SIZE
that API. By default the page size corresponds to the block
size (65536). Other option include the sector size (4096).

config NORDIC_QSPI_NOR_FLASH_ALLOW_STACK_USAGE_FOR_DATA_IN_FLASH
bool "Use stack for writing data stored in flash"
help
When this option is enabled writing data from FLASH is possible.
However the driver will use stack to copy the data from flash onto
the stack then start the DMA transfer. If you get an -EINVALID error
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding. If you have the time, it would be better to have this documentation in the header file for qspi_nor_write, since I think that's where the user will look if they get the error, but it's not a must.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okei I'll try to move the information there.

this could be due to the data being in flash. Either copy the data
locally or enable this option. This uses a 4 byte buffer to copy the
data which has a performance penalty for large chunks of data. If the
data is less than a word the function will already copy the data into
ram.

endif # NORDIC_QSPI_NOR
108 changes: 85 additions & 23 deletions drivers/flash/nrf_qspi_nor.c
Original file line number Diff line number Diff line change
@@ -248,8 +248,6 @@ static inline void qspi_wait_for_completion(struct device *dev,

if (res == NRFX_SUCCESS) {
k_sem_take(&dev_data->sync, K_FOREVER);
} else {
qspi_unlock(dev);
}
}

@@ -273,7 +271,6 @@ static void qspi_handler(nrfx_qspi_evt_t event, void *p_context)

if (event == NRFX_QSPI_EVENT_DONE) {
qspi_complete(dev);
qspi_unlock(dev);
}
}

@@ -319,14 +316,14 @@ static int qspi_erase(struct device *dev, u32_t addr, u32_t size)
return -EINVAL;
}

int rv = -EIO;
int rv = 0;
const struct qspi_nor_config *params = dev->config_info;

while (size) {
qspi_lock(dev);
while ((rv == 0) && (size > 0)) {
nrfx_err_t res = !NRFX_SUCCESS;
u32_t adj = 0;

qspi_lock(dev);
if (size == params->size) {
/* chip erase */
res = nrfx_qspi_chip_erase();
@@ -353,11 +350,13 @@ static int qspi_erase(struct device *dev, u32_t addr, u32_t size)
size -= adj;
} else {
LOG_ERR("erase error at 0x%lx size %zu", (long)addr, size);
return rv;
rv = qspi_get_zephyr_ret_code(res);
}
}

return 0;
qspi_unlock(dev);

return rv;
}

/**
@@ -491,16 +490,22 @@ static inline int qspi_nor_read_id(struct device *dev,
static int qspi_nor_read(struct device *dev, off_t addr, void *dest,
size_t size)
{
if (!dest) {
return -EINVAL;
}
void *dptr = dest;
sigvartmh marked this conversation as resolved.
Show resolved Hide resolved
size_t dlen = size;

u8_t __aligned(4) buf[4];

/* read size must be non-zero multiple of 4 bytes */
if (((size % 4U) != 0) || (size == 0)) {
if (!dest || (size == 0)) {
return -EINVAL;
}
/* address must be 4-byte aligned */
if ((addr % 4U) != 0) {

/* Since the QSPI driver requires data to be at least 4 bytes we need
* to use a 4 byte buffer for reads smaller than 4 bytes.
*/
if ((size > 0) && (size < 4U)) {
dest = buf;
de-nordic marked this conversation as resolved.
Show resolved Hide resolved
size = sizeof(buf);
} else if ((size % 4U) != 0) {
return -EINVAL;
}

@@ -523,25 +528,45 @@ static int qspi_nor_read(struct device *dev, off_t addr, void *dest,

qspi_wait_for_completion(dev, res);

return qspi_get_zephyr_ret_code(res);
qspi_unlock(dev);

int rc = qspi_get_zephyr_ret_code(res);

if ((rc == 0) && (dest != dptr)) {
memcpy(dptr, dest, dlen);
}

return rc;
}

static int qspi_nor_write(struct device *dev, off_t addr, const void *src,
size_t size)
{
if (!src) {
return -EINVAL;
}
u8_t __aligned(4) buf[4];

const void *sptr = src;
size_t dlen = size;

/* write size must be non-zero multiple of 4 bytes */
if (((size % 4U) != 0) || (size == 0)) {
if (!src || (size == 0)) {
return -EINVAL;
}

/* address must be 4-byte aligned */
if ((addr % 4U) != 0) {
return -EINVAL;
}

/* Since the QSPI driver requires data to be at least 4 bytes we need
* to use a 4 byte buffer for writes smaller than 4 bytes.
*/
if (size < 4U) {
sigvartmh marked this conversation as resolved.
Show resolved Hide resolved
src = buf;
size = sizeof(buf);
} else if ((size % 4U) != 0) {
return -EINVAL;
}


struct qspi_nor_data *const driver_data = dev->driver_data;
const struct qspi_nor_config *params = dev->config_info;

@@ -560,11 +585,48 @@ static int qspi_nor_write(struct device *dev, off_t addr, const void *src,
return -EINVAL;
}


nrfx_err_t res = NRFX_SUCCESS;

qspi_lock(dev);

nrfx_err_t res = nrfx_qspi_write(src, size, addr);
if (sptr != src) {
/* read out the whole word so that unchanged data can be
* written back
*/
res = nrfx_qspi_read(buf, size, addr);
qspi_wait_for_completion(dev, res);
if (res == NRFX_SUCCESS) {
memcpy(buf, sptr, dlen);
}
}

qspi_wait_for_completion(dev, res);
/* if the data smaller than a whole word the function already copies
* the data in ram
*/
if (res != NRFX_SUCCESS) {
/* pre-read failed */
} else if (IS_ENABLED(CONFIG_NORDIC_QSPI_NOR_FLASH_ALLOW_STACK_USAGE_FOR_DATA_IN_FLASH) &&
((u32_t)sptr < CONFIG_SRAM_BASE_ADDRESS) && !(size < 4U)) {
size_t bytes_written = 0;

while ((res == NRFX_SUCCESS)
&& (bytes_written < dlen)) {
memcpy(buf, ((u8_t *)sptr + bytes_written),
sizeof(buf));
res = nrfx_qspi_write(buf, sizeof(buf),
addr + bytes_written);
qspi_wait_for_completion(dev, res);
if (res == NRFX_SUCCESS) {
bytes_written += sizeof(buf);
}
}
} else {
res = nrfx_qspi_write(src, size, addr);
sigvartmh marked this conversation as resolved.
Show resolved Hide resolved
qspi_wait_for_completion(dev, res);
}

qspi_unlock(dev);

sigvartmh marked this conversation as resolved.
Show resolved Hide resolved
return qspi_get_zephyr_ret_code(res);
}