RFC: API Change: Flash: New API to access flash information

[de-nordic]

Introduction

Introducing API change that allows to define "virtual" flash devices over real flash devices.

Currently some of flash API calls, like the flash_api_pages_layout, often propagate pointers to structures directly defined at driver level; this makes it hard to setup "virtual" device that could lie about the information returned by such API call as it may require run-time allocation of resources as the "real-device" can never be assumed, which is not possible. Aim of this RFC is to discuss changes to API that would prevent direct access to layout and this way make it easier to generate "virtual" devices. Such virtual devices would allow defining devices that could serve as replacement for flash map API or direct encryption devices, with no need to special API layer for them, as basically any function or utility could use flash API directly, without knowing whether it is real device or "virtual".

Problem description

The flash_api_pages_layout , when enabled with CONFIG_FLASH_PAGE_LAYOUT, gives direct access to array that defines layout of flash, the user code can iterate over for gathering information; additionally there are flash_get_page_info_by_offs, and flash_get_page_info_by_idx that utilize this access. With such direct access virtual device is not able to provide own definition of layout.

Proposed change

Proposed change is to replace the optional flash_api_pages_layout, with set of following API calls: flash_get_page_info, flash_get_page_count, flash_get_size that would allow to quick obtain the same information but does not give direct access to information from driver. The missing functionality of flash_get_page_info_by_offs and flash_get_page_info_by_idx can be easily added with use of the above function, even in form of utility library for flash access

Proposed change (Detailed)

Proposed addition to flash API calls:

int flash_get_page_info(const struct device *dev, off_t offset, struct flash_page_info *fpi);
ssize_t flash_get_page_count(const struct device *dev);
ssize_t flash_get_size(const struct device *dev);

note that the last two calls would be very quick as they will be given literal value during compilation. Additional additions to struct flash_parameters would be max_page_size, which is maximum page size on the device (or any in case of uniform devices), and flags which will hold flags; currently there will be only one flag defined FDF_NON_UNIFORM_LAYOUT that means that the device has pages of different size.

With such definitions, in contrary to flash_api_pages_layout, any "virtual" driver that would be able to query the information from underlying driver, process it if needed, and return to caller.

Dependencies

Anything that uses flash_api_pages_layout will not be able to use "virtual" devices. All flash drivers will require some patching.

Concerns and Unresolved Questions

Initialization order impact, power management (regarding the "virtual devices", device readiness).

Alternatives

There is no such attempt to flatten the API layers, where flash access is considered.

Related PRs

Proposed flash API change: https://github.com/zephyrproject-rtos/zephyr/pull/36484 Demo of flash partition device (formerly flash area device) that is possible due to change: https://github.com/zephyrproject-rtos/zephyr/pull/36487

[de-nordic]

FYI @nvlsianpu , @jfischer-no , @mbolivar-nordic , @galak , @nandojve .

[Laczen]

Do we really need virtual flash devices ? It seems to be like we would better look at the linux solution for memory technology devices (mtd) and adapt zephyr to the approach of linux. This would result in the flash api being deprecated completely and replaced with an mtd api.

Probably it is also not needed to create a virtual device as a real device, only the "master" devices needs to be a device. Other things like partitions can be (const) structs that are using this master device. A simple approach providing all the required functionality is provided in https://github.com/zephyrproject-rtos/zephyr/pull/34494. This PR does not need any API changes or whatever.

[nvlsianpu]

I was looking into virtual flash device - looks like it does the job of partition management well, it reduces amount API we have. Can be similar idea be implemented with MTD API?, so we will be able to switch to it smoothly. I think both ideas doesn't collide each other.

[Laczen]

@nvlsianpu, the main difference between virtual flash devices and mtd API is that virtual flash devices create a device for each partition, while mtd only uses (or creates if we would replace the flash device with a mtd device) for the "master" device. This allows support for repartitioning without any changes. This also results in the compiler removing any unused partitions from the image for the mtd API.

The mtd API could (and should) provide support for regions (like in linux) so that page support is provided from the start. It should also be extended to provide a "mtd-compatible" so that master devices can be registered (this would remove the flash API completely). Also it might be a good idea to provide some way of "linking" the different partitions (using a list) so that for each partition it is possible to find the parent and child partitions.

[carlescufi]

API meeting (6th July): no objections.

[de-nordic]

Update to API implementation:

Because adding the special API calls increases size of flash device structure significantly, the independent API calls to provide back-end for the flash_get_page_info, flash_get_page_count and flash_get_size will not be provided, instead single API call, flash_api_get_param will be provided that will be taking the opcode for the parameter to obtain; the increase in access time, to the parameters, should be acceptable as obtaining parameters is not happening often and the significant reduction of size of code is worth it. The flash_api_get_parameters and flash_api_get_layout may be also moved under the single flash_api_get_param API call reducing the API switch table even further.

The

int flash_get_page_info(const struct device *dev, off_t offset, struct flash_page_info *fpi);
ssize_t flash_get_page_count(const struct device *dev);
ssize_t flash_get_size(const struct device *dev);

can be implemented as non-syscalls with such approach.

[Laczen]

@carlescufi, strange that API meeting did not detect the above.

@de-nordic, I support that there would be one function entry in an api to access and modify "configurable items". However I think you are duplicating info to avoid syscalls, this is not a good idea.

[de-nordic]

@de-nordic, I support that there would be one function entry in an api to access and modify "configurable items". However I think you are duplicating info to avoid syscalls, this is not a good idea.

Can you elaborate on that?

[Laczen]

@de-nordic, I support that there would be one function entry in an api to access and modify "configurable items". However I think you are duplicating info to avoid syscalls, this is not a good idea.

Can you elaborate on that?

A flash device (when flash page support is enabled) has a layout structure and an api call to access this layout structure. A virtual flash device needs to follow the api and so also has a layout structure. The information in the layout is duplicated in the flag and the max_page_size.

BTW: It is OK to create a virtual device with uniform block size on top of a flash device that has varying block size. This should be done by defining an appropiate layout.

[de-nordic]

@de-nordic, I support that there would be one function entry in an api to access and modify "configurable items". However I think you are duplicating info to avoid syscalls, this is not a good idea.

Can you elaborate on that?

A flash device (when flash page support is enabled) has a layout structure and an api call to access this layout structure. A virtual flash device needs to follow the api and so also has a layout structure. The information in the layout is duplicated in the flag and the max_page_size.

The max_page_size is gone at this point, I got to the same conclusion as you. The flags remains because It allows you to figure out whether you need to use flash_get_page_info for each page or you can do it once, for uniform devices, and do all calculations by yourself, avoiding further syscalls.

BTW: It is OK to create a virtual device with uniform block size on top of a flash device that has varying block size. This should be done by defining an appropiate layout.

The flash_get_page_info can also do that.

[Laczen]

The max_page_size is gone at this point, I got to the same conclusion as you. The flags remains because It allows you to figure out whether you need to use flash_get_page_info for each page or you can do it once, for uniform devices, and do all calculations by yourself, avoiding further syscalls.

The flags is the same as: layout_size == 1.

BTW: It is OK to create a virtual device with uniform block size on top of a flash device that has varying block size. This should be done by defining an appropiate layout.

The flash_get_page_info can also do that.

??? When you create a virtual flash device it should be done properly, and its layout should reflect what you want it to be. The virtual flash device should not have a uniform layout when using one api call while it has a non-uniform using a different api call.

[de-nordic]

The flags is the same as: layout_size == 1.

This will be retired.

??? When you create a virtual flash device it should be done properly, and its layout should reflect what you want it to be. The virtual flash device should not have a uniform layout when using one api call while it has a non-uniform using a different api call.

The virtual device is the way you implement it, so the flash_get_page_info of your device will do whatever you tell it to do.

The virtual partition device I have demonstrated is very basic, and it gives you layout of the device at specified range: if that specified range is uniform, the virtual partition device will be uniform device even when the real device, in its entirety, has non-uniform layout.

[Laczen]

The virtual device is the way you implement it, so the flash_get_page_info of your device will do whatever you tell it to do.

Yes, but what I am trying to say is that the virtual device should follow the API specifications. The layout is a part of the API specification and should be correct (either a part of what is inside the real device or a new definition).

[de-nordic]

Update to API implementation:

Because adding the special API calls increases size of flash device structure significantly, the independent API calls to provide back-end for the flash_get_page_info, flash_get_page_count and flash_get_size will not be provided,

After experimenting with few approaches, including the direct implementations of 1-1 callback, for example flash_get_page_count calls api->get_page_count(...), and N-1 callbacks, for example flash_get_page_count calls api->get_param(FLASH_PARAM_PAGE_COUNT, ...), I have come to following conclusions:

1) addition of separate calls to API structure does not have that much of an impact, it is only pointer size of bytes per call, and the API structure has only one instance; 2) overhead in drivers, where each callback has to be defined is not so big in comparison to overhead needed to invoke syscall from the user side; for example when N-1 approach is used, the exemplar flash_get_page_count would be inlined as

flash_device_api api = ...;
ssize_t count = 0;
api->get_param(dev, FLASH_PARAM_PAGE_COUNT, &count)
return count;

and it is clearly visible that passing parameters to the get_param and returning value, takes more operations than 1-1 invocation:

flash_device_api api = ...;
ssize_t count = 0;
return api->get_page_count(dev);

3) because syscalls from flash.h are inlined, look at point 2), multiple ivocations may increase binary size when using N-1 approach, although with the simple getters, like proposed here, the large number of independent invocations does not make that impact (and is not real scenario). That observation has been compared against inlined flash_erase, in current form, and with intermediate function defined where flash_erase would call that function: in 'real life' scenario of the littlefs test (for nrf52840) the size in binary change has been less than 16bytes in favor to the current implementation.

Conclusion: we are back to direct, 1-1, API callback implementation.

[zephyrbot]

Hi @de-nordic,

This issue, marked as an RFC, was opened a while ago and did not get any traction. Please confirm the issue is correctly assigned and re-assign it otherwise.

Please take a moment to review if the issue is still relevant to the project. If it is, please provide feedback and direction on how to move forward. If it is not, has already been addressed, is a duplicate, or is no longer relevant, please close it with a short comment explaining the reason.

Thanks!