Skip to content

Enable consumer of ec-slimloader-imxrt to construct FlashJournal #29

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.

Sign up for GitHub

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

Draft
ayodejiige wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Enable consumer of ec-slimloader-imxrt to construct FlashJournal #29

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions libs/ec-slimloader-imxrt/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,18 +43,19 @@ const MAX_SLOT_COUNT: usize = 7;
pub type ExternalStorage = BlockingAsync<FlexSpiNorStorage<'static, READ_ALIGNMENT, WRITE_ALIGNMENT, ERASE_SIZE>>;

pub struct Partitions {
pub state: Partition<'static, ExternalStorage, RW, NoopRawMutex>,
pub journal: FlashJournal<Partition<'static, ExternalStorage, RW>>,
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

The FlashJournal type is missing the mutex type parameter that was present in the original Partition (NoopRawMutex). This creates an inconsistency with the slots field on line 47 which still specifies NoopRawMutex. Verify that FlashJournal's type parameters are correct and consistent with the partition's mutex requirements.

Suggested change
pub journal: FlashJournal<Partition<'static, ExternalStorage, RW>>,
pub journal: FlashJournal<Partition<'static, ExternalStorage, RW, NoopRawMutex>>,

Copilot uses AI. Check for mistakes.
pub slots: Vec<Partition<'static, ExternalStorage, RO, NoopRawMutex>, MAX_SLOT_COUNT>,
}

Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

The #[allow(async_fn_in_trait)] attribute is added without explanation. Consider adding a comment explaining why this lint suppression is necessary, especially since async functions in traits have specific stability and compatibility implications.

Suggested change
// Allow async functions in traits for ImxrtConfig, as async trait methods are required for this API.
// Note: async functions in traits are currently unstable in Rust. This lint suppression is intentional,
// and should be revisited when the feature is stabilized or if trait design changes.

Copilot uses AI. Check for mistakes.
#[allow(async_fn_in_trait)]
pub trait ImxrtConfig {
/// Minimum and maximum image size contained within a slot.
const SLOT_SIZE_RANGE: Range<usize>;

/// The memory range an image is allowed to be copied to.
const LOAD_RANGE: Range<*mut u32>;

Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

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

Making this trait method async is a breaking change that moves error handling responsibility to trait implementers. The original implementation had explicit panic handling with a descriptive message. Consider documenting the expected error handling behavior for FlashJournal::new failures, or provide guidance on how implementers should handle initialization errors.

Suggested change
/// Initializes the partitions required for bootloading.
///
/// # Error Handling
/// Implementers are responsible for handling any errors that may occur during
/// initialization (e.g., failures in `FlashJournal::new`). It is recommended to
/// log errors with a descriptive message and either panic or return a default value,
/// depending on the application's requirements. Consistent error handling should be
/// ensured across all implementations.

Copilot uses AI. Check for mistakes.
fn partitions(&self, flash: &'static mut PartitionManager<ExternalStorage, NoopRawMutex>) -> Partitions;
async fn partitions(&self, flash: &'static mut PartitionManager<ExternalStorage, NoopRawMutex>) -> Partitions;
}

#[allow(dead_code)]
Expand Down Expand Up @@ -105,12 +106,7 @@ impl<C: ImxrtConfig> Board for Imxrt<C> {
let ext_flash_manager =
EXT_FLASH.init_with(|| PartitionManager::<_, NoopRawMutex>::new(BlockingAsync::new(ext_flash)));

let Partitions { state, slots } = config.partitions(ext_flash_manager);

let journal = match FlashJournal::new::<JOURNAL_BUFFER_SIZE>(state).await {
Ok(journal) => journal,
Err(e) => panic!("Failed to initialize the flash state journal: {:?}", e),
};
let Partitions { journal, slots } = config.partitions(ext_flash_manager).await;

Self {
journal,
Expand Down
Loading