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 memory read / write issues? #81

Closed
amcelroy opened this issue Sep 11, 2023 · 12 comments
Closed

Flash memory read / write issues? #81

amcelroy opened this issue Sep 11, 2023 · 12 comments

Comments

@amcelroy
Copy link
Contributor

Howdy,

I was wondering if anyone has had any luck with the flash memory module for reading and writing? I am using the Nucleo-G474RE and am trying to write some data to flash memory using RTIC 1.x as the "RTOS". The main issue seems to be when calling the FlashWriter::write function that core::ptr::write_volatile doesn't write, and when the subsequent core::ptr::read_volatile is called, the results don't match and the function returns with Err(Error::VerifyError);

There isn't a working flash example, so here is a summary of my attempt that hopefully can be added to the examples when things are up and running. The PLL config is being used to bump the clock speed up to 128MHz and get the ADC running at a 1MHz (not included in the test). Removing this code and checking clock domains will be the next step.

The example project should compile and run using the Segger J-Link tools using VS Code (tested on my Macbook Pro M2).

@usbalbin
Copy link
Contributor

usbalbin commented Sep 12, 2023

(Just a user here)

Note sure if this is the problem, however from what I can tell from the datasheet, max fVCO_OUT is 344MHz.

pll_config.mux = stm32g4xx_hal::rcc::PLLSrc::HSI; // 16MHz
pll_config.n = stm32g4xx_hal::rcc::PllNMul::MUL_64;
pll_config.m = stm32g4xx_hal::rcc::PllMDiv::DIV_2; // f(vco) = 16MHz*64/2 = 512MHz <----- This is more than 344MHz
pll_config.r = Some(stm32g4xx_hal::rcc::PllRDiv::DIV_4); // f(sysclock) = 512MHz/4 = 128MHz

and the above seems to set it to 512MHz.

I usually use CubeMX's "Clock Configuration" to double check :)

amcelroy added a commit to amcelroy/stm32g474-flash-test that referenced this issue Sep 12, 2023
Reconfigured clock based on the discussion here: stm32-rs/stm32g4xx-hal#81
@amcelroy
Copy link
Contributor Author

Thanks for taking a look, that is a good idea about using CubeMX. I tried going with the default clocking solution and that didn't seem to work either. The test project is updated based on your suggestions to reduce the f(vco) from 512MHz -> 256MHz by halving PLL_N and PLL_R to still maintain the 128MHz sysclock speed.

@usbalbin
Copy link
Contributor

usbalbin commented Sep 14, 2023

I am probably reading something wrong but does it not say:

Perform the data write operation at the desired memory address, inside main memory
block or OTP area. Only double word can be programmed.

Then why does the code say:

// Flash is written 16 bits at a time, so combine two bytes to get a
// half-word

Again sorry if I am completely misunderstanding things. :)


I am also wondering a bit about the flash wait states at higher system frequencies, this does not seem to match RM0440, again from what I can tell... It seems to be ok for default clock settings though.


Edit: Anyone please, let me know if I am wrong :)

@amcelroy
Copy link
Contributor Author

amcelroy commented Sep 14, 2023

Thanks for continuing to dig on this. You may be on to something, after the core::voltaile_write the Status Register bit 6 SIZERR is set.

Update:
I changed the code to write a double word (u64) and it worked. I'll work on a better solution and submit a pull request.
Screenshot 2023-09-14 at 8 44 03 AM

@amcelroy
Copy link
Contributor Author

Another update, the Nucleo-STM32G474 on our boards is the STM32G474RE6TU, which has a different flash layout (see "Embedded Flash memory for Category 3 devices" chapter). Unfortunately, the HAL doesn't expose the DBANK bit in the FLASH_OPTR register; but upon manual inspection this bit is set. This means that this chip has 2 banks of 256kB banks with a page size of 2k instead of 4k. This is all to say the erase function isn't working either on the Nucleo boards.

I think the best solution is to configure any STM32G474 with a DBANK of 0, as the STM32G474 SVD file that I have seems to be missing DBANK. Alternatively, we could add new part numbers and more granular configurations.

Any thoughts?

@amcelroy
Copy link
Contributor Author

amcelroy commented Sep 14, 2023

Proposed fixes: #83

Changing from the intended double bank to single bank really messed up subsequent flash attempts, so I left the device in whatever bank configuration is default. FlashWriter now requires the page size to be passed in for certain chips instead of assuming the page size.

Screenshot 2023-09-14 at 2 25 40 PM

@usbalbin
Copy link
Contributor

Regardning DBANK

Maybe something like this which generates these extra fields for cat 3 devices (stmg471, 473, 474, 483 and 484):

             #[doc = "Bit 19 - Window watchdog selection"]
             #[inline(always)]
             #[must_use]
             pub fn wwdg_sw(&mut self) -> WWDG_SW_W<19> {
                 WWDG_SW_W::new(self)
             }
+            #[doc = "Bit 20 - Bank to boot from"]
+            #[inline(always)]
+            #[must_use]
+            pub fn bfb2(&mut self) -> BFB2_W<20> {
+                BFB2_W::new(self)
+            }
+            #[doc = "Bit 22 - Single or dual bank mode"]
+            #[inline(always)]
+            #[must_use]
+            pub fn dbank(&mut self) -> DBANK_W<22> {
+                DBANK_W::new(self)
+            }
             #[doc = "Bit 23 - Boot configuration"]
             #[inline(always)]
             #[must_use]
             pub fn n_boot1(&mut self) -> N_BOOT1_W<23> {
                 N_BOOT1_W::new(self)
             }

@amcelroy
Copy link
Contributor Author

amcelroy commented Sep 14, 2023

That seems like a good idea. I'm still pretty new to Rust and the generation of the HALs for this chipset, where in the pipeline would this run?

Edit: Also, is there a way to change the Segger page size? When I went from double bank to single bank, subsequent flashes using the Segger were not correct and looked like it was writing the first half of a 4096 byte page, which makes sense if the Segger was working with 2048 byte pages.

@usbalbin
Copy link
Contributor

stm32g4 is a dependency of stm32g4xx-hal. stm32g4 however is an autogenerated crate that is generated by stm32-rs/stm32-rs which is a repo full of patches and things to generate correct pac crates from the (not always so correct) svd files

@amcelroy
Copy link
Contributor Author

stm32g4 is a dependency of stm32g4xx-hal. stm32g4 however is an autogenerated crate that is generated by stm32-rs/stm32-rs which is a repo full of patches and things to generate correct pac crates from the (not always so correct) svd files

At what stage does this crate pull and compile new yaml or SVD files? The cargo file for the stm32g4xx-hal uses version stm32g4 = "0.15.1", would this need to be updated or is the yaml file patch not big enough to cause a version uprev in the stm32-rs/stm32 crate?

@usbalbin
Copy link
Contributor

I can not speak as to when(or if) stm32-rs/stm32-rs#880 might get merged, that would depend on when someone has time to review it. Whether it would be big enough for a new release, I dont know.. I am quite new here too :) I also believe there might have been some breaking changes since the release used by stm32g4xx-hal.

In the meantime, smt32-rs/stm32-rs is not itself a crate, just a tool/configuration to autogenerate crates. instructions here for how to build. This will, among a lot of other crates, generate a /some/path/stm32-rs/stm32g4 which can be specified as dependency instead of "0.15.1".

@amcelroy
Copy link
Contributor Author

Closed: #83 (comment)

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

No branches or pull requests

2 participants