Skip to content

Flash API: new information getter #36484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 29 commits into from

Conversation

de-nordic
Copy link
Contributor

@de-nordic de-nordic commented Jun 23, 2021

First commit in this PR proposes change to flash API that introduces API calls for obtaining information about flash, like size, page count and page information.

This change, contrary to flash_api_pages_layout, is non optional addition and does not allow directly returning objects from drivers, instead allows to query the driver to fill in information.

The addition of API calls:

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

allows access to information on flash size and page count, and information on single page.
The commit changes also the flash_parameters structure with addition of flags and max_page_size;
there is one flag defined so far: FPF_NON_UNIFORM_LAYOUT that is set for devices with non-uniform layout and should allow code using the flash device to decide how to work with the device.

The second commit is test of changes.

This is proposed implementation of: #36485

Rationale for changes
Original idea has been to implement "virtual" flash device, but the idea has been dropped. Nevertheless the API change allows more efficient checking for device layout.
The main goal of the change is to simplify obtaining information on flash devices, and to be able to define "virtual" devices that could be placed over the other flash devices to perform some actions, which existence of flash_api_pages_layout does allow, as the "virtual" device is not able to easily override values returned by the call.
This change should allow to, for example, define partition or encryption device, that will use the same flash API calls, which means that any function that is able to perform actions on a device of flash type, will be usable with the "virtual" device, without knowing that it is not working on a real device.

Example of the "virtual" device that defines partitions over real device: #36487

TODO:

  • test cases;
  • apply to all drivers.

@github-actions github-actions bot added area: API Changes to public APIs area: native port Host native arch port (native_sim) area: Tests Issues related to a particular existing or missing test labels Jun 23, 2021
@de-nordic de-nordic force-pushed the flash_api_changes branch from 85c201c to a72063e Compare June 23, 2021 12:09
@de-nordic de-nordic changed the title Improvements in flash API that allow definition of "virtual" devices Flash API: new information getter Jun 24, 2021
* -EINVAL if offset is outside of flash device;
* other negative errno code on error.
*/
__syscall int flash_get_page_info(const struct device *dev, off_t off, struct flash_page_info *fpi);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy from: #36487 (comment)
nvlsianpu:
isn't this similar to flash_get_page_info_by_offs()
de-nordic:
I have added the flash_get_page_info, as a part of #36485, so that it would be possible to override results returned by such call, which is not possible with the current flash_api_get_layout, because it gives direct access to objects defined within driver.
I have not removed the original calls that do layout checks, because this is proposal and I did not want to remove them yet.
The flash_get_page_info_by_offs and the flash_get_page_info_by_idx, and the foreachs, in case of the #36485, would be reimplemented as non-system util library, and the CONFIG that enables them would be still available.
All these functions would be reimplemented around the new flash_get_page_info, flash_get_page_count and flash_get_size.
nvlsianpu:
Ok, understood

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all fine to me.

@@ -133,6 +196,9 @@ __subsystem struct flash_driver_api {
flash_api_erase erase;
flash_api_write_protection write_protection;
flash_api_get_parameters get_parameters;
flash_api_get_page_info get_page_info;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy from: #36487 (comment)
nvlsianpu:
this is similar to flash-layout-get implementation api. Shouldn't it be optional?

de-nordic:
I was considering adding this as constant addition as the info is needed anyway, for example you can not do erase without knowing the page info, also this can be used to get alignment as it always returns info for aligned page that contains the offset.
In case of most drivers, the implementation will be quite efficient, because drivers that know about uniform layout will just do something like this:

if (offset < TOTAL_FLASH_SIZE) {
   fpi->offset = offset & ~(CONSTANT_PAGE_SIZE -1);
   fpi->size = CONSTANT_PAGE_SIZE);
}

On the other side the non uniform layout is also quick to obtain:

if (offset < HUGE_PAGE_OFFSET) {
   fpi->offset = offset & ~(CONSTANT_PAGE_SIZE -1);
   fpi->size = CONSTANT_PAGE_SIZE);
} else (offset < TOTAL_FLASH_SIZE) {
   fpi->offset = HUGE_PAGE_OFFSET;
   fpi->size = HUGE_PAGE_SIZE;
}

Because driver exactly knows the layout, in most cases, it does not have to iterate through all pages to get to the offset that is interesting for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right. Option was introduced when not all devices implements the flash-get-layout API. Nowday it is almost mandatory.

nvlsianpu
nvlsianpu previously approved these changes Jul 6, 2021
@de-nordic de-nordic force-pushed the flash_api_changes branch from a72063e to c64bd78 Compare July 15, 2021 13:34
@de-nordic de-nordic force-pushed the flash_api_changes branch 6 times, most recently from eb6e63c to 66d1a01 Compare July 21, 2021 12:34
@de-nordic de-nordic force-pushed the flash_api_changes branch 6 times, most recently from 1b2e4ad to 543c958 Compare July 26, 2021 13:15
@de-nordic de-nordic marked this pull request as ready for review July 28, 2021 07:52
@github-actions github-actions bot added the Stale label Aug 24, 2022
@de-nordic de-nordic removed the Stale label Aug 24, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Oct 24, 2022
@de-nordic de-nordic removed the Stale label Oct 24, 2022
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jun 11, 2023
@de-nordic de-nordic removed the Stale label Jun 12, 2023
@de-nordic de-nordic assigned de-nordic and unassigned nvlsianpu Jun 12, 2023
@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: API Changes to public APIs area: Flash area: native port Host native arch port (native_sim) area: Tests Issues related to a particular existing or missing test platform: ESP32 Espressif ESP32 platform: Microchip SAM Microchip SAM Platform (formerly Atmel SAM) platform: nRF Nordic nRFx platform: NXP NXP platform: Silabs Silicon Labs platform: STM32 ST Micro STM32
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants