I'm satisfied with the API. I have a couple minor comments on the
implementation.
On Thu, Apr 20, 2023 at 1:23 AM <aaron.nyh...@unfoldedeffective.com> wrote:
>
> From: Aaron Nyholm <aaron.nyh...@southerninnovation.com>
>
> ---
> cpukit/dev/flash/flashdev.c | 849 ++++++++++++++++++++++++++++
> cpukit/include/dev/flash/flashdev.h | 458 +++++++++++++++
> spec/build/cpukit/librtemscpu.yml | 4 +
> 3 files changed, 1311 insertions(+)
> create mode 100644 cpukit/dev/flash/flashdev.c
> create mode 100644 cpukit/include/dev/flash/flashdev.h
>
> diff --git a/cpukit/dev/flash/flashdev.c b/cpukit/dev/flash/flashdev.c
> new file mode 100644
> index 0000000000..03343f015f
> --- /dev/null
> +++ b/cpukit/dev/flash/flashdev.c
> @@ -0,0 +1,849 @@
> +/*
> + * Copyright (C) 2023 Aaron Nyholm
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include "config.h"
> +#endif
> +
> +#include <dev/flash/flashdev.h>
> +
> +#include <rtems/imfs.h>
> +#include <rtems/score/assert.h>
> +
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +
> +#define RTEMS_FLASHDEV_REGION_ALLOC_FULL 0xFFFFFFFFUL
> +#define RTEMS_FLASHDEV_REGION_UNDEFINED 0xFFFFFFFFUL
> +#define RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH 32
> +
> +static int rtems_flashdev_do_init(
> + rtems_flashdev *flash,
> + void ( *destroy )( rtems_flashdev *flash )
> +);
> +
> +static int rtems_flashdev_ioctl_erase(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + void *arg
> +);
> +
> +static off_t rtems_flashdev_get_region_offset(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop
> +);
> +
> +static size_t rtems_flashdev_get_region_size(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop
> +);
> +
> +static int rtems_flashdev_ioctl_set_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + void *arg
> +);
> +
> +static int rtems_flashdev_ioctl_create_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + rtems_flashdev_region *region_in
> +);
> +
> +static int rtems_flashdev_ioctl_update_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + rtems_flashdev_region *region_in
> +);
> +
> +static int rtems_flashdev_ioctl_clear_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop
> +);
> +
> +static uint32_t rtems_flashdev_ioctl_jedec_id(
> + rtems_flashdev *flash
> +);
> +
> +static uint32_t rtems_flashdev_ioctl_flash_type(
> + rtems_flashdev *flash,
> + void *arg
> +);
> +
> +static int rtems_flashdev_ioctl_pageinfo_offset(
> + rtems_flashdev *flash,
> + void *arg
> +);
> +
> +static int rtems_flashdev_ioctl_pageinfo_index(
> + rtems_flashdev *flash,
> + void *arg
> +);
> +
> +static int rtems_flashdev_ioctl_page_count(
> + rtems_flashdev *flash,
> + void *arg
> +);
> +
> +static int rtems_flashdev_ioctl_write_block_size(
> + rtems_flashdev *flash,
> + void *arg
> +);
> +
> +static int rtems_flashdev_get_addr(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + size_t count,
> + off_t *addr
> +);
> +
> +static int rtems_flashdev_update_and_return(
> + rtems_libio_t *iop,
> + int status,
> + size_t count,
> + off_t new_offset
> +);
> +
> +static uint32_t rtems_flashdev_find_unalloced_region(
> + rtems_flashdev_region_table *region_table
> +);
> +
> +static uint32_t rtems_flashdev_set_region(
> + rtems_flashdev_region_table *region_table,
> + int index
> +);
> +
> +static uint32_t rtems_flashdev_unset_region(
> + rtems_flashdev_region_table *region_table,
> + int index
> +);
> +
> +static uint32_t rtems_flashdev_check_allocation(
> + rtems_flashdev_region_table *region_table,
> + int index
> +);
> +
> +static uint32_t rtems_flashdev_get_region_index(
> + rtems_libio_t *iop
> +)
> +{
> + return (uint32_t)iop->data0;
> +}
> +
> +static int rtems_flashdev_is_region_defined(
> + rtems_libio_t *iop
> +)
> +{
> + return (rtems_flashdev_get_region_index( iop ) !=
> RTEMS_FLASHDEV_REGION_UNDEFINED);
> +}
> +
> +static void rtems_flashdev_set_region_index(
> + rtems_libio_t *iop,
> + uint32_t index
> +)
> +{
> + iop->data0 = index;
> +}
> +
> +static int rtems_flashdev_check_offset_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + off_t offset
> +)
> +{
> + if ( ( rtems_flashdev_is_region_defined( iop ) ) &&
> + ( offset > rtems_flashdev_get_region_size( flash, iop ) ) ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> + return 0;
> +}
> +
> +static void rtems_flashdev_obtain( rtems_flashdev *flash )
> +{
> + rtems_recursive_mutex_lock( &flash->mutex );
> +}
> +
> +static void rtems_flashdev_release( rtems_flashdev *flash )
> +{
> + rtems_recursive_mutex_unlock( &flash->mutex );
> +}
> +
> +static ssize_t rtems_flashdev_read(
> + rtems_libio_t *iop,
> + void *buffer,
> + size_t count
> +)
> +{
> + rtems_flashdev *flash = IMFS_generic_get_context_by_iop( iop );
> + off_t addr;
> + int status;
> +
> + if ( flash->read == NULL ) {
> + return 0;
> + }
> +
> + /* Get flash address */
> + status = rtems_flashdev_get_addr( flash, iop, count, &addr );
> + if ( status == -1 ) {
maybe use a real error code. It is also more robust to check if < 0
for negative-coded error conditions.
> + return status;
> + }
> +
> + /* Read flash */
> + rtems_flashdev_obtain( flash );
> + status = ( *flash->read )( flash, addr, count, buffer );
> + rtems_flashdev_release( flash );
> +
> + /* Update offset and return */
> + return rtems_flashdev_update_and_return( iop, status, count, addr + count
> );
> +}
> +
> +static ssize_t rtems_flashdev_write(
> + rtems_libio_t *iop,
> + const void *buffer,
> + size_t count
> +)
> +{
> + rtems_flashdev *flash = IMFS_generic_get_context_by_iop( iop );
From this point on, this function is identical to the above read
function, using flash->write instead of flash->read. You could
refactor to a helper function.
> + off_t addr;
> + int status;
> +
> + if ( flash->write == NULL ) {
> + return 0;
> + }
> +
> + /* Get flash address */
> + status = rtems_flashdev_get_addr( flash, iop, count, &addr );
> + if ( status == -1 ) {
> + return status;
> + }
> +
> + /* Write to flash */
> + rtems_flashdev_obtain( flash );
> + status = ( *flash->write )( flash, addr, count, buffer );
> + rtems_flashdev_release( flash );
> +
> + /* Update offset and return */
> + return rtems_flashdev_update_and_return( iop, status, count, addr + count
> );
> +}
> +
> +static int rtems_flashdev_ioctl(
> + rtems_libio_t *iop,
> + ioctl_command_t command,
> + void *arg
> +)
> +{
> + rtems_flashdev *flash = IMFS_generic_get_context_by_iop( iop );
> + int err = 0;
> +
> + rtems_flashdev_obtain( flash );
> +
> + switch ( command ) {
> + case RTEMS_FLASHDEV_IOCTL_OBTAIN:
> + rtems_flashdev_obtain( flash );
> + err = 0;
> + break;
> + case RTEMS_FLASHDEV_IOCTL_RELEASE:
> + rtems_flashdev_release( flash );
> + err = 0;
> + break;
> + case RTEMS_FLASHDEV_IOCTL_JEDEC_ID:
> + *( (uint32_t *) arg ) = rtems_flashdev_ioctl_jedec_id( flash );
> + err = 0;
> + break;
> + case RTEMS_FLASHDEV_IOCTL_ERASE:
> + err = rtems_flashdev_ioctl_erase( flash, iop, arg );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_REGION_SET:
> + err = rtems_flashdev_ioctl_set_region( flash, iop, arg );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_REGION_UNSET:
> + err = rtems_flashdev_ioctl_clear_region( flash, iop );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_TYPE:
> + err = rtems_flashdev_ioctl_flash_type( flash, arg );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_PAGEINFO_BY_OFFSET:
> + err = rtems_flashdev_ioctl_pageinfo_offset( flash, arg );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_PAGEINFO_BY_INDEX:
> + err = rtems_flashdev_ioctl_pageinfo_index( flash, arg );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_PAGE_COUNT:
> + err = rtems_flashdev_ioctl_page_count( flash, arg );
> + break;
> + case RTEMS_FLASHDEV_IOCTL_WRITE_BLOCK_SIZE:
> + err = rtems_flashdev_ioctl_write_block_size( flash, arg );
> + break;
> + }
> +
> + rtems_flashdev_release( flash );
> + if ( err != 0 ) {
> + rtems_set_errno_and_return_minus_one( err );
> + } else {
> + return 0;
> + }
> +}
> +
> +static off_t rtems_flashdev_lseek(
> + rtems_libio_t *iop,
> + off_t offset,
> + int whence
> +)
> +{
> + off_t tmp_offset;
> + rtems_flashdev *flash = IMFS_generic_get_context_by_iop( iop );
> +
> + if ( offset < 0 ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> +
> + switch ( whence ) {
> + case SEEK_CUR:
> + tmp_offset = iop->offset + offset;
> + break;
> + case SEEK_SET:
> + tmp_offset = offset;
> + break;
> + case SEEK_END:
> + default:
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> +
> + if ( ( rtems_flashdev_is_region_defined(iop) ) &&
> + ( tmp_offset > rtems_flashdev_get_region_size( flash, iop ) ) ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> +
> + iop->offset = tmp_offset;
> + return iop->offset;
> +}
> +
> +static int rtems_flashdev_close( rtems_libio_t *iop )
> +{
> + rtems_flashdev *flash = IMFS_generic_get_context_by_iop( iop );
> + rtems_flashdev_ioctl_clear_region( flash, iop );
> + return rtems_filesystem_default_close( iop );
> +}
> +
> +static int rtems_flashdev_open(
> + rtems_libio_t *iop,
> + const char *path,
> + int oflag,
> + mode_t mode
> +)
> +{
> + int ret = rtems_filesystem_default_open( iop, path, oflag, mode );
> + rtems_flashdev_set_region_index(iop, RTEMS_FLASHDEV_REGION_UNDEFINED);
> + return ret;
> +}
> +
> +static const rtems_filesystem_file_handlers_r rtems_flashdev_handler = {
> + .open_h = rtems_flashdev_open,
> + .close_h = rtems_flashdev_close,
> + .read_h = rtems_flashdev_read,
> + .write_h = rtems_flashdev_write,
> + .ioctl_h = rtems_flashdev_ioctl,
> + .lseek_h = rtems_flashdev_lseek,
> + .fstat_h = IMFS_stat,
> + .ftruncate_h = rtems_filesystem_default_ftruncate,
> + .fsync_h = rtems_filesystem_default_fsync_or_fdatasync,
> + .fdatasync_h = rtems_filesystem_default_fsync_or_fdatasync,
> + .fcntl_h = rtems_filesystem_default_fcntl,
> + .kqfilter_h = rtems_filesystem_default_kqfilter,
> + .mmap_h = rtems_filesystem_default_mmap,
> + .poll_h = rtems_filesystem_default_poll,
> + .readv_h = rtems_filesystem_default_readv,
> + .writev_h = rtems_filesystem_default_writev };
> +
> +static void rtems_flashdev_node_destroy( IMFS_jnode_t *node )
> +{
> + rtems_flashdev *flash;
> +
> + flash = IMFS_generic_get_context_by_node( node );
> +
> + ( *flash->destroy )( flash );
> +
> + IMFS_node_destroy_default( node );
> +}
> +
> +static const IMFS_node_control
> + rtems_flashdev_node_control = IMFS_GENERIC_INITIALIZER(
> + &rtems_flashdev_handler,
> + IMFS_node_initialize_generic,
> + rtems_flashdev_node_destroy
> +);
Keep all the function declarations together at the top.
> +
> +int rtems_flashdev_register( rtems_flashdev *flash, const char *flash_path )
> +{
> + int rv;
> + rtems_flashdev_region_table *table = flash->region_table;
> + int alloc_array_len;
> +
> + rv = IMFS_make_generic_node( flash_path,
when breaking function calls, put the first argument on its own line.
we really need some good autoformatting :)
> + S_IFCHR | S_IRWXU | S_IRWXG | S_IRWXO,
only use two (extra) indent levels for line breaks.
> + &rtems_flashdev_node_control,
> + flash );
> + if ( rv != 0 ) {
> + ( *flash->destroy )( flash );
> + }
> +
> +
> + if (table != NULL) {
> + alloc_array_len =
> table->max_regions/RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH +
> + ((table->max_regions%RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH) != 0);
> + for
> + (
keep ( with for.
> + int i = 0;
decls at start of function, not inside for loops.
> + i < alloc_array_len;
> + i++
> + )
> + {
> + table->bit_allocator[i] = 0;
> + }
> + }
This for loop can be replaced with memset. not sure if we have a
bzero() equivalent available.
> +
> + return rv;
> +}
> +
> +static int rtems_flashdev_do_init(
> + rtems_flashdev *flash,
> + void ( *destroy )( rtems_flashdev *flash )
> +)
> +{
> + rtems_recursive_mutex_init( &flash->mutex, "RTEMS_FLASHDEV Flash" );
> + flash->destroy = destroy;
> + flash->read = NULL;
> + flash->write = NULL;
> + flash->erase = NULL;
> + flash->jedec_id = NULL;
> + flash->flash_type = NULL;
> + flash->page_info_by_offset = NULL;
> + flash->page_info_by_index = NULL;
> + flash->page_count = NULL;
> + flash->write_block_size = NULL;
> + flash->region_table = NULL;
> + return 0;
> +}
> +
> +void rtems_flashdev_destroy( rtems_flashdev *flash )
> +{
> + rtems_recursive_mutex_destroy( &flash->mutex );
> +}
> +
> +void rtems_flashdev_destroy_and_free( rtems_flashdev *flash )
> +{
> + if ( flash == NULL ) {
> + return;
> + }
> + rtems_recursive_mutex_destroy( &( flash->mutex ) );
> + free( flash );
> + flash = NULL;
> + return;
> +}
> +
> +int rtems_flashdev_init( rtems_flashdev *flash )
> +{
> + memset( flash, 0, sizeof( *flash ) );
> +
> + return rtems_flashdev_do_init( flash, rtems_flashdev_destroy );
> +}
> +
> +rtems_flashdev *rtems_flashdev_alloc_and_init( size_t size )
> +{
> + rtems_flashdev *flash = NULL;
> +
> + if ( size >= sizeof( *flash ) ) {
> + flash = calloc( 1, size );
> + if ( flash != NULL ) {
> + int rv;
> +
> + rv = rtems_flashdev_do_init( flash, rtems_flashdev_destroy_and_free );
> + if ( rv != 0 ) {
> + rtems_recursive_mutex_destroy( &flash->mutex );
> + free( flash );
> + return NULL;
> + }
> + }
> + }
> +
> + return flash;
> +}
> +
> +static int rtems_flashdev_get_addr(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + size_t count,
> + off_t *addr
> +)
> +{
> + off_t new_offset;
> +
> + /* Check address is in valid region */
> + new_offset = iop->offset + count;
> +
> + if (rtems_flashdev_check_offset_region(flash, iop, new_offset)) {
> + return -1;
> + }
> +
> + /* Get address for operation */
> + if ( !rtems_flashdev_is_region_defined( iop ) ) {
> + *addr = iop->offset;
> + } else {
> + *addr = ( iop->offset + rtems_flashdev_get_region_offset( flash, iop ) );
> + }
> + return 0;
> +}
> +
> +static int rtems_flashdev_update_and_return(
> + rtems_libio_t *iop,
> + int status,
> + size_t count,
> + off_t new_offset
> +)
> +{
> + /* Update offset and return */
> + if ( status == 0 ) {
> + iop->offset = new_offset;
> + return count;
> + } else {
> + rtems_set_errno_and_return_minus_one( status );
> + }
> +}
> +
> +static int rtems_flashdev_ioctl_erase(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + void *arg
> +)
> +{
> + rtems_flashdev_region *erase_args_1;
> + off_t check_offset;
> + int status;
> +
> + if ( flash->erase == NULL ) {
> + return 0;
> + }
> +
> + erase_args_1 = (rtems_flashdev_region *) arg;
> + /* Check erasing valid region */
> + check_offset = erase_args_1->offset + erase_args_1->size;
> + if ( rtems_flashdev_is_region_defined( iop ) && (
> + rtems_flashdev_check_offset_region(flash, iop, check_offset) ||
> + ( erase_args_1->offset <
> + rtems_flashdev_get_region_offset( flash, iop ) ) ) ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> +
> + /* Erase flash */
> + status = ( *flash->erase )( flash, erase_args_1->offset,
> erase_args_1->size );
> + return status;
> +}
> +
> +static int rtems_flashdev_ioctl_set_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + void *arg
> +)
> +{
> + rtems_flashdev_region *region_in;
> + rtems_flashdev_region_table *table = flash->region_table;
> + region_in = (rtems_flashdev_region *) arg;
> +
> + if (flash->region_table == NULL) {
> + rtems_set_errno_and_return_minus_one( ENOMEM );
> + }
> +
> + if ( !rtems_flashdev_is_region_defined( iop ) &&
> + rtems_flashdev_find_unalloced_region(table)
> + == RTEMS_FLASHDEV_REGION_ALLOC_FULL)
> + {
> + /* New region to allocate and all regions allocated */
> + rtems_set_errno_and_return_minus_one( ENOMEM );
> + } else if ( !rtems_flashdev_is_region_defined( iop ) &&
> + rtems_flashdev_find_unalloced_region(table) !=
> + RTEMS_FLASHDEV_REGION_ALLOC_FULL ) {
> + /* New region to allocate and space to allocate region */
> + return rtems_flashdev_ioctl_create_region( flash, iop, region_in );
> + } else {
> + /* Updating existing region */
> + return rtems_flashdev_ioctl_update_region( flash, iop, region_in );
> + }
This logic can be simplified.
if (!rtems_flashdev_is_region_defined( iop )) {
if (rtems_flashdev_find_unalloced_region(table) == ...) {
/* New region to allocate and all regions allocated */...}
else { /* New region to allocate and space to allocate region */...}
} else {
/* Updating existing region */
}
> +}
> +
> +static int rtems_flashdev_ioctl_create_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + rtems_flashdev_region *region_in
> +)
> +{
> + int i;
> + rtems_flashdev_region_table *table = flash->region_table;
> +
> + /* Find unallocated region slot */
> + i = rtems_flashdev_find_unalloced_region(flash->region_table);
unalloced should maybe be unallocated?
What happens if it returns RTEMS_FLASHDEV_REGION_ALLOC_FULL?
> +
> + /* Set region values */
> + table->regions[ i ].offset = region_in->offset;
> + table->regions[ i ].size = region_in->size;
> +
> + /* Set region as allocated and link iop */
> + rtems_flashdev_set_region(flash->region_table, i);
> + rtems_flashdev_set_region_index( iop, i );
> +
> + return 0;
> +}
> +
> +static int rtems_flashdev_ioctl_update_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop,
> + rtems_flashdev_region *region_in
> +)
> +{
> + uint32_t region_index = rtems_flashdev_get_region_index( iop );
> + rtems_flashdev_region_table *table = flash->region_table;
> +
> + /**
> + * If region index is larger then maximum region index or region
> + * index at given index is undefined return an error.
> + */
> + if (
> + ( region_index >= flash->region_table->max_regions ) ||
> + ( rtems_flashdev_check_allocation( table, region_index ) == 0)
> + )
> + {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> +
> + /* Set region values */
> + table->regions[ region_index ].offset = region_in->offset;
> + table->regions[ region_index ].size = region_in->size;
> +
> + return 0;
> +}
> +
> +static int rtems_flashdev_ioctl_clear_region(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop
> +)
> +{
> + uint32_t region_index = rtems_flashdev_get_region_index( iop );
> +
> + if (flash->region_table == NULL) {
> + rtems_set_errno_and_return_minus_one( ENOMEM );
> + }
> +
> + /* Check region to clear */
> + if ( region_index == RTEMS_FLASHDEV_REGION_UNDEFINED ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> +
> + /* Clear region */
> + rtems_flashdev_unset_region( flash->region_table, region_index );
> + rtems_flashdev_set_region_index( iop, RTEMS_FLASHDEV_REGION_UNDEFINED );
> + return 0;
> +}
> +
> +static off_t rtems_flashdev_get_region_offset(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop
> +)
> +{
> + rtems_flashdev_region_table *table = flash->region_table;
> + return table->regions[ rtems_flashdev_get_region_index( iop ) ].offset;
add a comment that says the region is already checked to be defined.
Maybe add an assert.
> +}
> +
> +static size_t rtems_flashdev_get_region_size(
> + rtems_flashdev *flash,
> + rtems_libio_t *iop
> +)
> +{
> + rtems_flashdev_region_table *table = flash->region_table;
> + return table->regions[ rtems_flashdev_get_region_index( iop ) ].size;
ditto
> +}
> +
> +static uint32_t rtems_flashdev_ioctl_jedec_id( rtems_flashdev *flash )
> +{
> + if ( flash->jedec_id == NULL ) {
> + return 0;
> + } else {
> + return ( *flash->jedec_id )( flash );
> + }
> +}
> +
> +static uint32_t rtems_flashdev_ioctl_flash_type(
> + rtems_flashdev *flash,
> + void *arg
> +)
> +{
> + rtems_flashdev_flash_type *type = (rtems_flashdev_flash_type*)arg;
> + if ( flash->flash_type == NULL ) {
> + return 0;
> + } else {
> + return ( *flash->flash_type )( flash, type );
> + }
> +}
> +
> +static int rtems_flashdev_ioctl_pageinfo_offset(
> + rtems_flashdev *flash,
> + void *arg
> +)
> +{
> + rtems_flashdev_ioctl_page_info *page_info;
> +
> + if ( arg == NULL ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> + if ( flash->page_info_by_offset == NULL ) {
> + return 0;
> + } else {
> + page_info = (rtems_flashdev_ioctl_page_info *) arg;
> + return ( *flash->page_info_by_offset )( flash,
> + page_info->location,
> + &page_info->page_info.offset,
> + &page_info->page_info.size );
> + }
> +}
> +
> +static int rtems_flashdev_ioctl_pageinfo_index( rtems_flashdev *flash,
> + void *arg )
> +{
> + rtems_flashdev_ioctl_page_info *page_info;
> +
> + if ( arg == NULL ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> + if ( flash->page_info_by_index == NULL ) {
> + return 0;
> + } else {
> + page_info = (rtems_flashdev_ioctl_page_info *) arg;
> + return ( *flash->page_info_by_index )( flash,
> + page_info->location,
> + &page_info->page_info.offset,
> + &page_info->page_info.size );
> + }
> +}
> +
> +static int rtems_flashdev_ioctl_page_count( rtems_flashdev *flash, void *arg
> )
> +{
> + if ( arg == NULL ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> + if ( flash->page_count == NULL ) {
> + return 0;
> + } else {
> + return ( *flash->page_count )( flash, ( (int *) arg ) );
> + }
> +}
> +
> +static int rtems_flashdev_ioctl_write_block_size(
> + rtems_flashdev *flash,
> + void *arg
> +)
> +{
> + if ( arg == NULL ) {
> + rtems_set_errno_and_return_minus_one( EINVAL );
> + }
> + if ( flash->write_block_size == NULL ) {
> + return 0;
> + } else {
> + return ( *flash->write_block_size )( flash, ( (size_t *) arg ) );
> + }
> +}
> +
> +static uint32_t rtems_flashdev_find_unalloced_region(
> + rtems_flashdev_region_table *region_table
> +)
> +{
> + int final_index =
> + ( region_table->max_regions / RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH );
> +
> + for ( int i = 0; i < final_index; i++ ) {
> + /* If region the next 32 regions are full check the next allocator */
> + if (region_table->bit_allocator[ i ] ==
> RTEMS_FLASHDEV_REGION_ALLOC_FULL) {
> + continue;
> + }
> +
> + for (int x = 0; x < RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH; x++) {
> + if ( ! ( ( ( region_table->bit_allocator[ i ] ) >> x ) & 1UL ) ) {
> + return RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH * i + x;
> + }
> + }
> + }
> +
> + for
> + (
> + int i = 0;
> + i < ( region_table->max_regions % RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH
> );
> + i++
> + )
> + {
> + if ( ! ( ( ( region_table->bit_allocator[ final_index ] ) >> i ) & 1UL )
> ) {
> + return RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH * final_index + i;
> + }
if you get to this point, I think it means 'i' of the previous loop is
final_index. I suspect these two loops could be meaningfully merged.
to iterate over the range of the RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH
+ max_regions % RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH, I see this
calculation of the number of 32-bit fields in the regions a few times,
it might make sense to provide a macro for it.
> + }
> +
> + return RTEMS_FLASHDEV_REGION_ALLOC_FULL;
> +
no space needed here.
> +}
> +
> +static uint32_t rtems_flashdev_set_region(
> + rtems_flashdev_region_table *region_table,
> + int index
> +)
> +{
> + int array_index = index / RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH;
> + int shift = index % RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH;
> +
> + region_table->bit_allocator[ array_index ] |= 1UL << shift;
> +
> + return index;
> +}
> +
> +static uint32_t rtems_flashdev_unset_region(
> + rtems_flashdev_region_table *region_table,
> + int index
> +)
> +{
> + int array_index = index / RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH;
> + int shift = index % RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH;
> +
> + region_table->bit_allocator[ array_index ] &= ~( 1UL << shift );
> +
> + return index;
> +}
> +
> +static uint32_t rtems_flashdev_check_allocation(
> + rtems_flashdev_region_table *region_table,
> + int index
> +)
> +{
> + int array_index = index / RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH;
> + int shift = index%RTEMS_FLASHDEV_REGION_BITALLOC_LENGTH;
> +
> + return ( ( region_table->bit_allocator[ array_index ] >> shift ) & 1UL );
> +}
> diff --git a/cpukit/include/dev/flash/flashdev.h
> b/cpukit/include/dev/flash/flashdev.h
> new file mode 100644
> index 0000000000..aee585d252
> --- /dev/null
> +++ b/cpukit/include/dev/flash/flashdev.h
> @@ -0,0 +1,458 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +
> +/**
> + * @file
> + *
> + * @ingroup Flash
> + *
> + * @brief Generic Flash API
> + */
> +
> +/*
> + * Copyright (C) 2023 Aaron Nyholm
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS
> IS"
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _DEV_FLASHDEV_H
> +#define _DEV_FLASHDEV_H
> +
> +#include <rtems/thread.h>
> +#include <sys/types.h>
> +
> +typedef struct rtems_flashdev rtems_flashdev;
> +
> +/**
> + * @defgroup Generic Flash API
> + *
> + * @ingroup RTEMSDeviceDrivers
> + *
> + * @brief Generic Flash API to wrap specific device drivers.
> + *
> + * @{
> + */
> +
> +/* IOCTL Calls */
> +
> +/**
> + * @brief Obtains the flash device.
> + *
> + * This command has no argument.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_OBTAIN 0
> +/**
> + * @brief Releases the flash device.
> + *
> + * This command has no argument.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_RELEASE 1
> +/**
> + * @brief Returns the JEDEC ID of the flash device.
> + *
> + * @param[out] jedec_id Pointer to uint32_t in which the JEDEC ID is
> + * returned in.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_JEDEC_ID 2
> +/**
> + * @brief Erases flash device.
> + *
> + * @param[in] erase_args Pointer to rtems_flashdev_region struct
> + * containing offset and size of erase to be performed.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_ERASE 3
> +/**
> + * @brief Set a region that limits read, write and erase calls to within it.
> + * Regions are file descriptor specific and limited to a single region per
> + * file descriptor and 32 regions total per flash device. Regions can be
> + * changed or updated by calling this IOCTL again.
> + *
> + * @param[in] region Pointer to rtems_flashdev_region struct containing
> + * base and length of defined region.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_REGION_SET 4
> +/**
> + * @brief Removes the set region on the file descriptor.
> + *
> + * This command has no argument.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_REGION_UNSET 5
> +/**
> + * @brief Returns the type of flash device (e.g. NOR or NAND).
> + *
> + * @param[out] flash_type Pointer to integer which is set to the flash
> + * type macro value.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_TYPE 6
> +
> +/**
> + * @brief Get the size and address of flash page at given offset
> + *
> + * The offset ignores the region limiting. To find page of region
> + * limited offset add the base of the region to the desired offset.
> + *
> + * @param[in,out] rtems_flashdev_ioctl_page_info arg Pointer to struct
> + * with offset and space for return values.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_PAGEINFO_BY_OFFSET 7
> +
> +/**
> + * @brief Get the size and address of nth flash page where n is index passed
> in.
> + *
> + * The index ignores the region limiting.
> + *
> + * @param[in,out] rtems_flashdev_ioctl_page_info arg Pointer to struct
> + * with index and space for return values.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_PAGEINFO_BY_INDEX 8
> +
> +/**
> + * @brief Get the number of pages in flash device.
> + *
> + * @param[out] count Integer containing the number of pages.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_PAGE_COUNT 9
> +
> +/**
> + * @brief Get the minimum write size supported by the driver.
> + *
> + * @param[out] count Integer containing the minimum write size.
> + */
> +#define RTEMS_FLASHDEV_IOCTL_WRITE_BLOCK_SIZE 10
> +
> +/**
> + * @brief The maximum number of region limited file descriptors
> + * allowed to be open at once.
> + */
> +#define RTEMS_FLASHDEV_MAX_REGIONS 32
> +
> +/**
> + * @brief Enum for flash type returned from IOCTL call.
> + */
> +typedef enum rtems_flashdev_flash_type {
> + /**
> + * @brief The flash device is NOR flash.
> + **/
> + RTEMS_FLASHDEV_NOR,
> + /**
> + * @brief The flash device is NAND flash.
> + */
> + RTEMS_FLASHDEV_NAND
> +} rtems_flashdev_flash_type;
> +
> +/**
> + * @brief General definition for on flash device.
> + */
> +typedef struct rtems_flashdev_region {
> + /**
> + * @brief Base of region.
> + */
> + off_t offset;
> + /**
> + * @brief Length of region.
> + */
> + size_t size;
> +} rtems_flashdev_region;
> +
> +/**
> + * @brief Struct holding region definitions
> + */
> +typedef struct rtems_flashdev_region_table {
> + /**
> + * @brief The maximum regions that can be defined at once.
> + */
> + int max_regions;
> +
> + /**
> + * @brief Pointer to array of rtems_flashdev_region of length
> + * max_regions
> + */
> + rtems_flashdev_region* regions;
> +
> + /**
> + * @brief Array of uint32_t acting as bit allocator
> + * for regions array.
> + */
> + uint32_t *bit_allocator;
> +} rtems_flashdev_region_table;
> +
> +/**
> + * @brief Page information returned from IOCTL calls.
> + */
> +typedef struct rtems_flashdev_ioctl_page_info {
> + /**
> + * @brief Offset or index to find page at.
> + */
> + off_t location;
> +
> + /**
> + * @brief Information returned about the page. Including the
> + * base offset and size of page.
> + */
> + rtems_flashdev_region page_info;
> +} rtems_flashdev_ioctl_page_info;
> +
> +/**
> + * @brief Flash device.
> + */
> +struct rtems_flashdev {
> + /**
> + * @brief Call to the device driver to read the flash device.
> + *
> + * @param[in] flash Pointer to flash device.
> + * @param[in] offset Address to read from.
> + * @param[in] count Number of bytes to read.
> + * @param[out] buffer Buffer for data to be read into.
> + *
> + * @retval 0 Successful operation.
> + * @retval 1 Failed operation.
> + */
> + int ( *read )(
> + rtems_flashdev *flash,
> + uintptr_t offset,
> + size_t count,
> + void *buffer
> + );
> +
> + /**
> + * @brief Call to the device driver to write to the flash device.
> + *
> + * @param[in] flash Pointer to flash device.
> + * @param[in] offset Address to write to.
> + * @param[in] count Number of bytes to read.
> + * @param[in] buffer Buffer for data to be written from.
> + *
> + * @retval 0 Successful operation.
> + * @retval 1 Failed operation.
> + */
> + int ( *write )(
> + rtems_flashdev *flash,
> + uintptr_t offset,
> + size_t count,
> + const void *buffer
> + );
> +
> + /**
> + * @brief Call to the device driver to erase the flash device.
> + *
> + * @param[in] flash Pointer to flash device.
> + * @param[in] offset Address to erase at.
> + * @param[in] count Number of bytes to erase.
> + *
> + * @retval 0 Successful operation.
> + * @retval 1 Failed operation.
> + */
> + int ( *erase )(
> + rtems_flashdev *flash,
> + uintptr_t offset,
> + size_t count
> + );
> +
> + /**
> + * @brief Call to the device driver to return the JEDEC ID.
> + *
> + * @param[in] flash The flash device.
> + *
> + * @retval JEDEC ID.
> + */
> + uint32_t ( *jedec_id )(
> + rtems_flashdev *flash
> + );
> +
> + /**
> + * @brief Call to the device driver to return the flash type.
> + *
> + * @param[in] flash The flash device.
> + * @param[out] type The type of flash device.
> + *
> + * @retval 0 Success
> + * @retbal Other Failure
> + */
> + int ( *flash_type )(
> + rtems_flashdev *flash,
> + rtems_flashdev_flash_type *type
> + );
> +
> + /**
> + * @brief Call to device driver to get size and offset of page at
> + * given offset.
> + *
> + * @param[in] flash The flash device
> + * @param[in] search_offset The offset of the page which info is to be
> + * returned.
> + * @param[out] page_offset The offset of the start of the page
> + * @param[out] page_size The size of the page
> + *
> + * @retval 0 Success.
> + * @retval non-zero Failed.
> + */
> + int ( *page_info_by_offset )(
> + rtems_flashdev *flash,
> + off_t search_offset,
> + off_t *page_offset,
> + size_t *page_size
> + );
> +
> + /**
> + * @brief Call to device driver to get size and offset of page at
> + * given index.
> + *
> + * @param[in] flash The flash device
> + * @param[in] search_index The index of the page which info is to be
> returned.
> + * @param[out] page_offset The offset of the start of the page
> + * @param[out] page_size The size of the page
> + *
> + * @retval 0 Success.
> + * @retval non-zero Failed.
> + */
> + int ( *page_info_by_index )(
> + rtems_flashdev *flashdev,
> + off_t search_index,
> + off_t *page_offset,
> + size_t *page_size
> + );
> +
> + /**
> + * @brief Call to device driver to return the number of pages on the flash
> + * device.
> + *
> + * @param[out] page_count The number of pages on the flash device.
> + *
> + * @retval 0 Success.
> + * @retval non-zero Failed.
> + */
> + int ( *page_count )(
> + rtems_flashdev *flashdev,
> + int *page_count
> + );
> +
> + /**
> + * @brief Call to device driver to return the minimum write size of the
> + * flash device.
> + *
> + * @param[out] write_block_size The minimum write size of the flash device.
> + *
> + * @retval 0 Success.
> + * @retval non-zero Failed.
> + */
> + int ( *write_block_size )(
> + rtems_flashdev *flashdev,
> + size_t *write_block_size
> + );
> +
> + /**
> + * @brief Destroys the flash device.
> + *
> + * @param[in] flash Pointer to flash device.
> + */
> + void ( *destroy )(
> + rtems_flashdev *flashdev
> + );
> +
> + /**
> + * @brief Pointer to device driver.
> + */
> + void *driver;
> +
> + /**
> + * @brief Mutex to protect the flash device access.
> + */
> + rtems_recursive_mutex mutex;
> +
> + /**
> + * @brief Region table defining size and memory for region allocations
> + */
> + rtems_flashdev_region_table *region_table;
> +};
> +
> +/**
> + * @brief Allocate and initialize the flash device.
> + *
> + * After a successful allocation and initialization of the flash device
> + * it must be destroyed via rtems_flashdev_destroy_and_free().
> + *
> + * @param[in] size The number of bytes to allocate.
> + *
> + * @retval NULL Failed to set up flash device.
> + * @retval non-NULL The new flash device.
> + */
> +rtems_flashdev *rtems_flashdev_alloc_and_init(
> + size_t size
> +);
> +
> +/**
> + * @brief Initialize the flash device.
> + *
> + * After a successful initialization of the flash device it must be
> + * destroyed via rtems_flashdev_destory().
> + *
> + * After initialization and before registration read, write, erase, jedec_id
> + * and flash_type functions need to be set in the flashdev.
> + *
> + * @param[in] flash The flash device to initialize.
> + *
> + * @retval 1 Failed to set up flash device.
> + * @retval 0 Successful set up of flash device.
> + */
> +int rtems_flashdev_init(
> + rtems_flashdev *flash
> +);
> +
> +/**
> + * @brief Register the flash device.
> + *
> + * This function always claims ownership of the flash device.
> + *
> + * After initialization and before registration read, write, erase, jedec_id
> + * and flash_type functions need to be set in the flashdev.
> + *
> + * @param[in] flash The flash device.
> + * @param[in] flash_path The path to the flash device file.
> + *
> + * @retval 0 Successful operation.
> + * @retval non-zero Failed operation.
> + */
> +int rtems_flashdev_register(
> + rtems_flashdev *flash,
> + const char *flash_path
> +);
> +
> +/**
> + * @brief Destroys the flash device.
> + *
> + * @param[in] flash The flash device.
> + */
> +void rtems_flashdev_destroy(
> + rtems_flashdev *flash
> +);
> +
> +/**
> + * @brief Destroys the flash device and frees its memory.
> + *
> + * @param[in] flash The flash device.
> + */
> +void rtems_flashdev_destroy_and_free(
> + rtems_flashdev *flash
> +);
> +
> +/** @} */
> +
> +#endif /* _DEV_FLASHDEV_H */
> diff --git a/spec/build/cpukit/librtemscpu.yml
> b/spec/build/cpukit/librtemscpu.yml
> index 2c969aa5f6..86c5c8ff2b 100644
> --- a/spec/build/cpukit/librtemscpu.yml
> +++ b/spec/build/cpukit/librtemscpu.yml
> @@ -52,6 +52,9 @@ install:
> - destination: ${BSP_INCLUDEDIR}/dev/spi
> source:
> - cpukit/include/dev/spi/spi.h
> +- destination: ${BSP_INCLUDEDIR}/dev/flash
> + source:
> + - cpukit/include/dev/flash/flashdev.h
> - destination: ${BSP_INCLUDEDIR}/dev/can
> source:
> - cpukit/include/dev/can/can.h
> @@ -522,6 +525,7 @@ links:
> - role: build-dependency
> uid: vckey
> source:
> +- cpukit/dev/flash/flashdev.c
> - cpukit/dev/i2c/eeprom.c
> - cpukit/dev/i2c/fpga-i2c-slave.c
> - cpukit/dev/i2c/gpio-nxp-pca9535.c
> --
> 2.25.1
>
> _______________________________________________
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
