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

[Chris Johns]

On 20/4/2023 7:42 am, Gedare Bloom wrote:
> On Mon, Apr 17, 2023 at 9:55 PM Chris Johns <chrisj at rtems.org> wrote:
>>
>> On 18/4/2023 1:48 pm, Gedare Bloom wrote:
>>> On Sun, Apr 16, 2023 at 6:48 PM Aaron Nyholm
>>> <aaron.nyholm at unfoldedeffective.com> wrote:
>>>> As for the RTEMS_FLASHDEV_MAX_REGIONS being set at 32 I chose the value as it seamed a good balance between memory usage and function. I wanted to avoid using malloc as your previous feedback suggested. If configurable size is necessary what’s the best way to approach it without using malloc?
>>>>
>>> A configure-time option may be viable, e.g., spec/build/cpukit/opt*
>>> You should probably elevate that question to its own email thread to
>>> get other opinions.
>>
>> Is there an option that shows how to control a defined in an API header?
>>
> This has mostly been done in the past at a BSP level, so there are
> plenty of examples within BSP drivers and frameworks. Whether it is
> suitable to a cpukit driver framework is something I don't really
> know.
> 
> A quick example could be:
> BSP_CONSOLE_BAUD
> used in bsps/powerpc/shared/console
> define in spec/build/bsps/optconsolebaud.yml
> 
> I'm not saying this is for sure the right way to do this, but I think
> it is worth thinking through.

It is something Aaron and I discuss early on and it has just sat and not moved.
I think it is good to have it raised as part of the review.

> I think you would have to avoid using the define in the header itself
> to make it portable. The way it's defined in this code can be done
> with a RTEMS_ZERO_LENGTH_ARRAY. Then the driver that instantiates the
> structure can determine for itself the size of how many regions to
> support. Even that would be an improvement to the current hard-coded
> method. The size itself could be made a field of the rtems_flashdev
> that the driver would setup at init time. Whether it is made
> configurable in the spec/build approach, or determined by the driver
> itself, is less concerning to me than the fact it is currently not
> configurable at all and is assumed to be sufficient for all flash
> drivers to be 32.

We have just discussed this and he is looking at making the table a pointer and
adding a size for the table and the flash device instance can add a table that
suites the device. This means extra checks for the pointer being a NULL but not
much more (I hope). I think this change will make a better driver.

Chris