feat(dfu_ext): Add ext dfu flash

[Schievel1]

No description provided.

[Schievel1]

honestly, best I can do is hiding the type behind a FlashType and importing the concrete type depending on feature set.

The problem is, that the flash must be stored in a static for the usb registration, but static can't be generics.
So in order to have this as impl NorFlash the user would have to make this static, which overcomplicates the user facing API imo.

Maybe you have a better idea.

[HaoboGu]

The current design can be simplified a lot by using NorFlash, and 'static is not needed actually.

We can just have:

pub struct RmkDfuInterface<'d, DFU: NorFlash, STATE: NorFlash> {
    central: DfuState<RmkDfuHandler<FirmwareHandler<'d, DFU, STATE, ResetImmediate, BLOCK_SIZE_DFU>>>,
    #[cfg(feature = "dfu_split")]
    passthrough: [Option<DfuState<RmkDfuHandler<PassthroughDfuHandler>>>; MAX_PASSTHROUGH_ALTS],
    #[cfg(feature = "dfu_split")]
    num_passthrough: usize,
    current_alt: u8,
}
impl<'d, DFU: NorFlash, STATE: NorFlash> RmkDfuInterface<'d, DFU, STATE> {
    pub fn new(
        dfu: DFU,
        state: STATE,
        aligned: &'d mut AlignedBuffer<DFU_ALIGN>,
        #[cfg(feature = "dfu_split")] num_peripherals: usize,
    ) -> Self {
        const { assert!(STATE::WRITE_SIZE <= DFU_ALIGN) };
        let updater = BlockingFirmwareUpdater::new(
            FirmwareUpdaterConfig { dfu, state },
            &mut aligned.0[..STATE::WRITE_SIZE],
        );
        // ...
    }
}

pub fn mark_booted(state: impl NorFlash, aligned: &mut AlignedBuffer<DFU_ALIGN>);

And in the user code it becomes:

let flash = Mutex::<CriticalSectionRawMutex, _>::new(RefCell::new(Nvmc::new(p.NVMC)));
let mut aligned = AlignedBuffer([0; 32]);

rmk::dfu::mark_booted(BlockingPartition::new(&flash, STATE_OFFSET, STATE_SIZE), &mut aligned);

let mut dfu = RmkDfuInterface::new(
    BlockingPartition::new(&flash, DFU_OFFSET, DFU_SIZE),
    BlockingPartition::new(&flash, STATE_OFFSET, STATE_SIZE),
    &mut aligned,
    SPLIT_PERIPHERALS_NUM,
);

let storage = async_flash_wrapper(BlockingPartition::new(&flash, STORAGE_OFFSET, STORAGE_SIZE));

[Schievel1]

The UsbTransport requires things to have static lifetime.
src/usb/mod.rs:

pub struct UsbTransport<'a, D: Driver<'static>> {

and

impl<'a, D: Driver<'static>> UsbTransport<'a, D> {
    pub fn new(driver: D, device_config: DeviceConfig<'static>) -> Self {

Only way to do static I think is when the type is known, and it's only known in user code. So we have to shift what we do here into user code. (the let flash_mutex: &'static MutexType = FLASH_CELL.init(Mutex::new(RefCell::new(flash)));)

Like this:

// types only added to make things a bit easier to overlook
type InternalFlashMutex =
    Mutex<CriticalSectionRawMutex, RefCell<Flash<'static, FLASH, Blocking, { rmk::dfu::FLASH_SIZE }>>>;
type InternalFlashPartition =
    BlockingPartition<'static, CriticalSectionRawMutex, Flash<'static, FLASH, Blocking, { rmk::dfu::FLASH_SIZE }>>;


    static FLASH_MUTEX: StaticCell<InternalFlashMutex> = StaticCell::new();
    let flash_mutex = FLASH_MUTEX.init(InternalFlashMutex::new(RefCell::new(embassy_rp::flash::Flash::<
        _,
        embassy_rp::flash::Blocking,
        { rmk::dfu::FLASH_SIZE },
    >::new_blocking(p.FLASH))));

    let state_partition =
        InternalFlashPartition::new(flash_mutex, dfu_flash_layout.state_offset, dfu_flash_layout.state_size);
    let flash = async_flash_wrapper(InternalFlashPartition::new(
        flash_mutex,
        dfu_flash_layout.storage_offset,
        dfu_flash_layout.storage_size,
    ));

I will try to make UsbTransport & co a shorter lifetime, but that is a bigger change.

[Schievel1]

I will try to make UsbTransport & co a shorter lifetime, but that is a bigger change.

I tinkered around with this, but I think it only gets worse from here.
So I think we should be going what you proposed, instead of hiding the staticcell complexity behind a function that needs concrete types and therefore must be implemented separately for each.

type ExternalFlash = W25qNorFlash<spi::Spi<'static, peripherals::SPI0, spi::Blocking>, Output<'static>>;
type ExternalPartition = BlockingPartition<'static, CriticalSectionRawMutex, ExternalFlash>;
type InternalFlashMutex =
    Mutex<CriticalSectionRawMutex, RefCell<Flash<'static, FLASH, Blocking, { rmk::dfu::FLASH_SIZE }>>>;
type InternalFlashPartition =
    BlockingPartition<'static, CriticalSectionRawMutex, Flash<'static, FLASH, Blocking, { rmk::dfu::FLASH_SIZE }>>;


#[embassy_executor::main]
async fn main(_spawner: Spawner) {
    static DFU_MUTEX: StaticCell<Mutex<CriticalSectionRawMutex, RefCell<ExternalFlash>>> = StaticCell::new();
    let dfu_mutex = DFU_MUTEX.init(Mutex::new(RefCell::new(ext_flash)));
    let dfu_partition = ExternalPartition::new(dfu_mutex, 0, dfu_mutex.lock(|c| c.borrow().capacity() as u32));

    let dfu_flash_layout = dfu_flash_layout();
    static FLASH_MUTEX: StaticCell<InternalFlashMutex> = StaticCell::new();
    let flash_mutex = FLASH_MUTEX.init(InternalFlashMutex::new(RefCell::new(embassy_rp::flash::Flash::<
        _,
        embassy_rp::flash::Blocking,
        { rmk::dfu::FLASH_SIZE },
    >::new_blocking(p.FLASH))));
    let mut state_partition =
        InternalFlashPartition::new(flash_mutex, dfu_flash_layout.state_offset, dfu_flash_layout.state_size);
    let flash = async_flash_wrapper(InternalFlashPartition::new(
        flash_mutex,
        dfu_flash_layout.storage_offset,
        dfu_flash_layout.storage_size,
    ));


    rmk::dfu::mark_booted(&mut state_partition);


    static DFU_IFACE: StaticCell<rmk::dfu::RmkDfuInterface<ExternalPartition, InternalFlashPartition>> =
        StaticCell::new();
    let dfu_iface = DFU_IFACE.init(rmk::dfu::RmkDfuInterface::new(dfu_partition, state_partition));
    let mut usb_transport =
        UsbTransport::new_with_dfu(driver, rmk_config.device_config, dfu_iface).with_host_service(&host_service);

// ...
}

The handler is then generic over norflash:

pub struct RmkDfuInterface<'d, DFU: NorFlash, STATE: NorFlash> {
    central: DfuState<RmkDfuHandler<FirmwareHandler<'d, DFU, STATE, ResetImmediate, BLOCK_SIZE_DFU>>>,
    #[cfg(feature = "dfu_split")]
    passthrough: [Option<DfuState<RmkDfuHandler<PassthroughDfuHandler>>>; MAX_PASSTHROUGH_ALTS],
    #[cfg(feature = "dfu_split")]
    num_passthrough: usize,
    current_alt: u8,
}

impl<'d, DFU: NorFlash, STATE: NorFlash> RmkDfuInterface<'d, DFU, STATE> {
    /// Build the DFU interface from a DFU download partition and a boot state
    /// partition.
    pub fn new(dfu: DFU, state: STATE, #[cfg(feature = "dfu_split")] num_peripherals: usize) -> Self {
...

[HaoboGu]

What I don't understand is, why UsbTransport needs dfu_iface? The transport should do the transport thing only, forward the data to the DFU part. So it should not own dfu info.