Skip to content
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

flash_partitions: a flash_map alternative #71158

Closed
wants to merge 2 commits into from

Conversation

Laczen
Copy link
Collaborator

@Laczen Laczen commented Apr 5, 2024

Working with partitions on a flash device has been done using the flash_map subsystem. The flash_map subsystem has been limiting (e.g. #52395) and does not support support for flash devices that do not need an erase (non flash devices that are supported as if they are flash).

The PR proposes an alternative for flash_map that supports classic flash devices, "no-erase" flash devices. Partitions can be read-only, and it is possible to provide an "override" erase-block-size.

The support for "no-erase" flash devices is achieved by the definition of a "zephyr,flash-no-erase" compatible that is added to the flash device.

The PR is a simplified version of #70979 and only targeting flash devices.

Signed-off-by: Laczen JMS laczenjms@gmail.com

@zephyrbot zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Storage Storage subsystem labels Apr 5, 2024
@zephyrbot zephyrbot requested a review from galak April 5, 2024 16:26
Working with partitions on a flash device has been done using the
flash_map subsystem. The flash_map subsystem has been limiting
(e.g. zephyrproject-rtos#52395) and does not support support for flash devices that
do not need an erase (non flash devices that are supported as if they
are flash).

The PR proposes an alternative for flash_map that supports classic flash
devices, "no-erase" flash devices. Partitions can be read-only, and it
is possible to provide an "override" erase-block-size.

The support for "no-erase" flash devices is achieved by the definition
of a "zephyr,flash-no-erase" compatible that is added to the flash
device.

Signed-off-by: Laczen JMS <laczenjms@gmail.com>
Add tests for the flash_partitions subsystem

Signed-off-by: Laczen JMS <laczenjms@gmail.com>
@MaureenHelm
Copy link
Member

@de-nordic please take a look

Copy link
Collaborator

@de-nordic de-nordic left a comment

Choose a reason for hiding this comment

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

NAK.

First of all it incorrectly uses the compatible:
image
https://github.com/devicetree-org/devicetree-specification/releases/download/v0.4/devicetree-specification-v0.4.pdf
Second this keeps attaching its own api object to each partition; fash map has already been able to work with devices just by having the struct device object pointer from original device with constrains added for partition.

The layer also bypasses (skips) the fixed-partition definitions to directly access DTS definitions of them, basically skipping one layer, which is design issue. This follows lasagna anti-pattern, is hackish and does not well establish what the fixed partition object represents or is defined by.

We already have support of fixed-partition, that comes from Linuxhttps://github.com/zephyrproject-rtos/zephyr/blob/main/dts/bindings/mtd/fixed-partitions.yaml, I seen o reason to have another Zephyr specific definition for this purpose.
There is some rework needed in Flash Map, but this not good solution.

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 Aug 14, 2024
@Laczen Laczen closed this Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Devicetree Binding PR modifies or adds a Device Tree binding area: Storage Storage subsystem Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants