[PATCH v3 1/3] cpukit/flash: Add API for Flash devices

Gedare Bloom gedare at rtems.org
Wed Apr 26 19:03:52 UTC 2023


 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.nyholm at unfoldedeffective.com> wrote:
>
> From: Aaron Nyholm <aaron.nyholm at 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 at rtems.org
> http://lists.rtems.org/mailman/listinfo/devel


More information about the devel mailing list