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

Interface for programming Option Bytes on stm32g family. #2493

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

Conversation

blaa
Copy link

@blaa blaa commented Jan 29, 2024

Seems to work for me like this:

unsafe {                                      
    flash::program_option_bytes(|| {          
        pac::FLASH.optr().modify(|r| {        
            r.set_n_boot0(true);              
            r.set_n_swboot0(false);           
        });                                   
        let data = pac::FLASH.optr().read();  
        assert_eq!(data.n_boot0(), true);     
        assert_eq!(data.n_swboot0(), false);  
    });                                       
}                                             

Instead of doing the whole process (as unlock/lock is pub(crate) only):

// Wait, while the memory interface is busy.                
while pac::FLASH.sr().read().bsy() {}                       
                                                            
// Unlock flash                                             
if pac::FLASH.cr().read().lock() {                          
    defmt::info!("Flash is locked, unlocking");             
    /* Magic bytes from embassy-stm32/src/flash/g.rs / RM */
    pac::FLASH.keyr().write_value(0x4567_0123);             
    pac::FLASH.keyr().write_value(0xCDEF_89AB);             
}                                                           
// Check: Should be unlocked.                               
assert_eq!(pac::FLASH.cr().read().lock(), false);           
                                                            
// Unlock Option bytes                                      
if pac::FLASH.cr().read().optlock() {                       
    defmt::info!("Option bytes locked, unlocking");         
                                                            
    /* Source: RM / original HAL */                         
    pac::FLASH.optkeyr().write_value(0x0819_2A3B);          
    pac::FLASH.optkeyr().write_value(0x4C5D_6E7F);          
}                                                           
// Check: Should be unlocked                                
assert_eq!(pac::FLASH.cr().read().optlock(), false);        
                                                            
/* Program boot0 */                                         
pac::FLASH.optr().modify(|r| {                              
    r.set_n_boot0(true);                                    
    r.set_n_swboot0(false);                                 
});                                                         
                                                            
// Check: Should have changed                               
assert_eq!(pac::FLASH.optr().read().n_boot0(), true);       
assert_eq!(pac::FLASH.optr().read().n_swboot0(), false);    
                                                            
// Reload option bytes. This should in general cause RESET. 
pac::FLASH.cr().modify(|w| w.set_optstrt(true));            
while pac::FLASH.sr().read().bsy() {}                       
                                                            
pac::FLASH.cr().modify(|w| w.set_obl_launch(true));         
                                                            
defmt::info!("Relocking");  
// Lock option bytes and flash                              
pac::FLASH.cr().modify(|w| w.set_optlock(true));            
pac::FLASH.cr().modify(|w| w.set_lock(true));               

This should probably work for f family too. Untested.
@Dirbaio
Copy link
Member

Dirbaio commented Feb 4, 2024

  • Do you think it'd be possible to do an API without closures? or at least that doesn't require using the PAC directly. It feels to me the proposed API "leaks" implementation details by only doing "half" of the work: it unlocks stuff but leaves doing the actual register write to you. We should aim for APIs that do "everything" for you.
  • Can you fix CI? cargo +nightly fmt.

@blaa
Copy link
Author

blaa commented Feb 6, 2024

  • Do you think it'd be possible to do an API without closures? or at least that doesn't require using the PAC directly. It feels to me the proposed API "leaks" implementation details by only doing "half" of the work: it unlocks stuff but leaves doing the actual register write to you. We should aim for APIs that do "everything" for you.

Very high level interface would be to create an enum from this table:
scr
And map it to nBOOT1, nBOOT0 and nSWBOOT0.

But I guess too high level? We would have to consult RM per chip and convert it accordingly.
Example usage:

flash::program_boot_configuration(flash::BootConfiguration::MainFlash)

(maybe still unsafe? It might require certain voltage level)

Mid-level abstraction could work like this:

fn program_boot_configuration(nBOOT0: bool, nBOOT1: bool, nSWBOOT0: bool);
...
usage:
program_boot_configuration(true, false, false);

This limits that to the few programmable bits related to boot config, but requires no PAC. Might be more movable between families.

One doesn't exclude the other. There can be low-level interface and high-level interface. Should low-level one use closures though? Or just consist of two methods?

* Can you fix CI? `cargo +nightly fmt`.

Yeah, no problem. I just wonder where to go with this.

@blaa
Copy link
Author

blaa commented Feb 6, 2024

High level API enum draft:

enum BootConfiguration {
    MainFlash,
    EmbeddedSram1,
    SystemMemory,
    PinBoot0MainOrEmbeddedSram1,
    PinBoot0MainOrSystemMemory,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants