[PATCH rtems 00/14] #4981 Overwork flashdev

berndmoessner80 at gmail.com berndmoessner80 at gmail.com
Tue Jan 2 20:45:21 UTC 2024


From: Bernd Moessner <berndmoessner80 at gmail.com>

Dear all,

as outlined in https://devel.rtems.org/ticket/4981 I`d like to
overwork the flashdev API. The provided patch series should at
least serve as a fundament for the discussion.

It includes more, but h:

1) Bugfixes

1.1) missing c++ include guards

1.2) missing default cases

1.3) There are some false positive test cases. Ie. they fail,
are expected to fail, but fail for a different reason.
The test cases are not taking into account that fputs works
on buffered IO per default. The region mechanism is there to
protect reads / writes to an area outside the
currently active region. This part works fine, but due to
buffering newlib tries to read even more bytes than the user
requested - which in turn triggers the region mechanism and
causes the read to fail. In addtion to that, theres a bug in
the _update_and_return function due to which a wrong address
(the absolute address and not the relative address of the
region) is returned to newlib.

2) Refactoring / Style Changes:

2.1) Not sure if there are naming conventions for IOCTL defines
and functions, but I think it makes sense if an IOCTL is used
to "get" a value, that "get" is reflected by the macro and
function names.

2.2) Flashdev used the term "regions", but from my point of
view "partions" fits better

2.3) I'd like to call the minimum write block size simply
minimum write size. The point is that the term block is already
predefined for flash devices (Bytes => Pages => Sectors => Block
=> Die => Chip). We should should also make sure that the
writes are aligned to the min write size and are carried out
with this size. I've added alignment checks, but the user
must set the buffering size for newlib using setvbuf.

3) API Extension

3.1) The API is missing a function to get the erase size.
Partions / Regions need to be aligned to the erase size of
the flash memory.

3.2) The region / partition mechnism works differently now.
There are IOCTL the create, delete, activate, deactivate
partitions, and one to get idx of the currently active
partion. I think the internal implementation becomes
much simpler if we reduce the number of allowed partitions
from 32 to 16.

The patch set addresses all issues I have found. I have
updated and extended the test cases. I must admit that I am
a bit insecure wrt. to the RTEMS style guide - I`d be very
happy if one could have a close look at that. Aaron Nyholm has
added the flashdev API last year. The changes within this
patch set are too large to contribute without his agreement.



Bernd Moessner (14):
  FIX: Add missing C++ include guards
  Flashdev: Unify IOCTL macro names
  Flashdev: Align IOCTL function and macro names
  Flashdev: Align IOCTL and shell function names
  FIX: Add missing default case
  Flashdev: Make mutex name more generic
  Flashdev: Refactor write_block_size and add function to dergister
    flashdev
  Flashdev: Add IOCTL to get the erase size
  FIX: Regions must be aligned with erase size
  Flashdev: Refactoring, replace region with partition jargon and allow
    IOTCLs to return a value
  Flashdev: Update copyright notice
  Add IOCTL to get the active partition idx
  Flashdev: Allow flash geometry beeing set from test case
  FIX: 80 char per line limit issues

 cpukit/dev/flash/flashdev.c                   | 1110 ++++++++++-------
 cpukit/include/dev/flash/flashdev.h           |  309 +++--
 cpukit/libmisc/shell/main_flashdev.c          |  102 +-
 testsuites/libtests/flashdev01/init.c         |  318 ++++-
 .../libtests/flashdev01/test_flashdev.c       |  166 ++-
 .../libtests/flashdev01/test_flashdev.h       |   10 +-
 6 files changed, 1322 insertions(+), 693 deletions(-)

-- 
2.34.1



More information about the devel mailing list