[PATCH rtems6 - v1 00/16] Overwork flashdev - V1

[Bernd Moessner]

On 04.01.2024 20:40, Kinsey Moore wrote:
> On Thu, Jan 4, 2024 at 12:34 PM <berndmoessner80 at gmail.com> wrote:
>
>     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
>
>
> Thanks for all the work that went into this!
>
> I see that there are 2 versions of this patch set on the list. It 
> seems that one doesn't have a version identifier and one has a v1 
> identifier. Either would be fine, but I wouldn't recommend using both 
> as it can get confusing as to which is the latest version of the patch 
> set.
>
> Hopefully, I've chosen the correct set to review (16 patches). 
> Unfortunately, it was made harder to distinguish because the mail 
> server was down for a few days and they both got delivered within a 
> few tens of minutes of each other.
>
> Kinsey

Thanks a lot for going through all the commits and commenting on them!

Yes, you've chosen the correct patch set. I`ll work in your comments and 
resubmit the patch set after we've clarified if it is "min_write_size", 
"min_write_block_size" or something entirely new ;). Therefore, the 
patch set will become smaller as two patches fix bugs I've introduced. 
Do I resubmit it using "[PATCH rtems6 - v1 00/16] Overwork flashdev - 
V1" or as V2?

Bernd

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20240104/0f9a89c6/attachment-0001.htm>