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

stm32: add memory-mapped mode for QUADSPI #72339

Merged
merged 6 commits into from
May 16, 2024

Conversation

arbrauns
Copy link
Contributor

@arbrauns arbrauns commented May 6, 2024

Inspired by #68597, cc @FRASTM.

This CONFIG_STM32_MEMMAP is for enabling the MemoryMapped mode
on external octo or quad spi memory.
In this case, the flash_stm32_read is done in mem map mode
the flash_stm32_erase is not available.

Signed-off-by: Francois Ramu <francois.ramu@st.com>
@FRASTM
Copy link
Collaborator

FRASTM commented May 6, 2024

Thanks for implementing the memorymapped mode with the stm32 quadspi flash driver
Aborting the memory-mapped mode is required before erasing and writing (most of the quad flash cannot be written with memcopy).

  • Did you check that access to the quad-NOR is still possible in erase/read/write with your change.
    When I tried this sequence (in stm32 QSPI flash driver enables XiP on external NOR flash #68607), I realized that the samples/drivers/spi_flash fails after successive abort_memorymapped/set_memroymapped/abort_memorymapped. Did you run this example ?

  • Do you confirm that the flash_stm32_qspi.c is also supporting stm32h7 and stm32l4 series that have quad-spi peripherals ?

drivers/flash/flash_stm32_qspi.c Outdated Show resolved Hide resolved
drivers/flash/flash_stm32_qspi.c Outdated Show resolved Hide resolved
@arbrauns
Copy link
Contributor Author

arbrauns commented May 6, 2024

samples/drivers/spi_flash fails after successive abort_memorymapped/set_memroymapped/abort_memorymapped. Did you run this example ?

Yes, it works fine here after fixing the CR/CCR bug you noted. I have found another issue though: if the flash is already set to memory-mapped by e.g. openocd, the init is short-circuited, which causes any flash operations by the firmware to hang (semaphore not available). In my opinion this short-circuiting in flash_stm32_qspi_init() isn't even required:

  • either firmware needs the flash to remain memory-mapped (as setup by the bootloader) continuously, in which case it will simply not include the flash driver,
  • or the firmware uses the flash through flash_*() functions, in which case it doesn't care about it being memory-mapped during boot.

I can't imagine a firmware that requires continuous memory-mapping in order to boot, but then later also uses the flash_*() functions for the same device - do you agree that this use-case is rare enough that it can simply be ignored for now (more elaborate hacks can still be added later should anyone need them)?

Do you confirm that the flash_stm32_qspi.c is also supporting stm32h7 and stm32l4 series that have quad-spi peripherals ?

I initially developed it for a custom stm32l452 board, it works fine there. I don't have an h7 board with QSPI flash to test.

if (ret != 0) {
LOG_ERR("Failed to abort memory-mapped access before write");
goto end;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion : LOG_DBG("Abort Memory-mapped before write");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That feels much less clear to me. For one it doesn't even mention that something went wrong, and it's also not the memory-mapping that's being aborted, just a latent read access, if any.

Copy link
Member

Choose a reason for hiding this comment

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

I think the suggestion was to put it after, once abort is completed.

Copy link
Member

Choose a reason for hiding this comment

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

I think the suggestion was to put it after, once abort is completed.

@arbrauns Can you check this comment ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that makes sense, but then that LOG_DBG() would trigger on every single flash write/erase - would that make sense?

Copy link
Member

Choose a reason for hiding this comment

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

There's already a LOG_DBG on flash_stm32_qspi_read so, that would be coherent.

if (ret != 0) {
LOG_ERR("Failed to abort memory-mapped access before write");
goto end;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

suggestion : LOG_DBG("Abort Memory-mapped before erase");

Copy link
Collaborator

@FRASTM FRASTM left a comment

Choose a reason for hiding this comment

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

Cannot pass the samples/application_development/code_relocation_nocopy/ on stm32h747_disco or stm32h750b_dk.
Succesfully Tested with samples/drivers/spi_flash on stm32f746g_disco (single quad-flash) in memory mapped mode.

@arbrauns
Copy link
Contributor Author

arbrauns commented May 7, 2024

Cannot pass the samples/application_development/code_relocation_nocopy/ on stm32h747_disco or stm32h750b_dk.

As I said, I don't have h7 hardware, could you give some more information about the failures? A good candidate is the ATTR_MPU_EXTMEM QSPI placeholder region, it blocks execution access to the external flash and needs to be overridden with an ATTR_MPU_IO region the size of the actual attached flash, see 78c9ee7.

Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Thanks for putting this all together, some non blocking comments.
Otherwise LGTM once comments from @FRASTM are implemented.

drivers/flash/flash_stm32_qspi.c Outdated Show resolved Hide resolved
boards/st/stm32f769i_disco/stm32f769i_disco.dts Outdated Show resolved Hide resolved
@@ -74,6 +74,13 @@
sw0 = &user_button;
spi-flash0 = &mx25l51245g;
};

quadspi_memory_avail: memory-avail@90000000 {
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice to update board documentation to provide some details on how this could be used.

@erwango
Copy link
Member

erwango commented May 7, 2024

Cannot pass the samples/application_development/code_relocation_nocopy/ on stm32h747_disco or stm32h750b_dk.

Not blocking for the current PR. Let's document clearly what ia available and enter a GH issue on the known issues.

@FRASTM
Copy link
Collaborator

FRASTM commented May 7, 2024

Cannot pass the samples/application_development/code_relocation_nocopy/ on stm32h747_disco or stm32h750b_dk.

As I said, I don't have h7 hardware, could you give some more information about the failures? A good candidate is the ATTR_MPU_EXTMEM QSPI placeholder region, it blocks execution access to the external flash and needs to be overridden with an ATTR_MPU_IO region the size of the actual attached flash, see 78c9ee7.

Building west build -p auto -b stm32h750b_dk samples/application_development/code_relocation_nocopy/
with

	ext_memory: memory@90000000 {
		compatible = "zephyr,memory-region";
		reg = <0x90000000 DT_SIZE_M(64)>; /* max addressable area */
		zephyr,memory-region = "EXTMEM";
		/* The ATTR_MPU_EXTMEM attribut causing a MPU FAULT */
		zephyr,memory-attr = <( DT_MEM_ARM(ATTR_MPU_IO) )>;
	};

and

CONFIG_FLASH=y
CONFIG_STM32_MEMMAP=y

gives the following error:

...
[00:00:00.000,000] <inf> flash_stm32_qspi: Memory-mapped NOR quad-flash at 0x90000000 (0x4000000 bytes)
*** Booting Zephyr OS build v3.6.0-3420-g69d57d068729 ***
Address of main function 0x800066d
[00:00:00.000,000] <err> os: ***** USAGE FAULT *****
[00:00:00.000,000] <err> os:   Attempt to execute undefined instruction
[00:00:00.000,000] <err> os: r0/a1:  0x0000000e  r1/a2:  0x00000000  r2/a3:  0x0000000f
[00:00:00.000,000] <err> os: r3/a4:  0x00000000 r12/ip:  0x00000000 r14/lr:  0x08000681
[00:00:00.000,000] <err> os:  xpsr:  0x61000000
[00:00:00.000,000] <err> os: Faulting instruction address (r15/pc): 0x90000000
[00:00:00.000,000] <err> os: >>> ZEPHYR FATAL ERROR 36: Unknown error on CPU 0
[00:00:00.000,000] <err> os: Current thread: 0x240007f0 (unknown)
[00:00:00.058,000] <err> os: Halting system

And the STM32CubeProgrammer shows 0xFF (blank sector) at 0x90000000

@arbrauns
Copy link
Contributor Author

arbrauns commented May 7, 2024

Since I didn't see any complaints, I've also removed the hack in flash_stm32_qspi_init() now.

@arbrauns
Copy link
Contributor Author

arbrauns commented May 7, 2024

And the STM32CubeProgrammer shows 0xFF (blank sector) at 0x90000000

Are you sure the external flash programming worked then?

@FRASTM
Copy link
Collaborator

FRASTM commented May 7, 2024

And the STM32CubeProgrammer shows 0xFF (blank sector) at 0x90000000

Are you sure the external flash programming worked then?

Yes, with the STM32CubeProgrammer from #68377
At least for stm32h750b_dk the dual quad-flash configuration requires to enable the dual flash mode, just before going to stm32_qspi_set_memory_mapped in the flash_stm32_qspi_init
MODIFY_REG(dev_data->hqspi.Instance->CR, (QUADSPI_CR_DFM), QSPI_DUALFLASH_ENABLE);

Then west build b -p auto -b stm32h750b_dk samples/application_development/code_relocation_nocopy ; west flash
gives:

[00:00:00.000,000] <dbg> flash_stm32_qspi: flash_stm32_qspi_init: Dual Flash Mode                                                                      
[00:00:00.000,000] <dbg> flash_stm32_qspi: stm32_qspi_set_memory_mapped: MemoryMap mode enabled                                                        
[00:00:00.000,000] <inf> flash_stm32_qspi: Memory-mapped NOR quad-flash at 0x90000000 (0x4000000 bytes)                                                
*** Booting Zephyr OS build v3.6.0-3067-gae992b9e37d6 ***                                                                                              
Address of main function 0x800066d                                                                                                                     
Address of function_in_ext_flash 0x90000001                                                                                                            
Address of var_ext_sram_data 0x240000e0 (10)                                                                                                           
Address of function_in_sram 0x24000001                                                                                                                 
Address of var_sram_data 0x240000e4 (10)                                                                                                               
Hello World! stm32h750b_dk  

Refer to #72429

This puts the QSPI peripheral into memory-mapped mode when
CONFIG_STM32_MEMMAP is set. Writes and erase put it back into indirect
write mode.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
According to the datasheet, fRSCLK (Clock Frequency for READ instructions)
is 66MHz; it doesn't mention 72MHz anywhere.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
…alt"

The init script is required for QSPI flash setup.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
By default, the QSPI region is marked as EXTMEM and inaccessible
(see zephyrproject-rtos#57467), mark the first 64MB as IO on stm32f769i_disco.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
This board has memory-mapped QSPI flash.

Signed-off-by: Armin Brauns <armin.brauns@embedded-solutions.at>
Copy link
Member

@erwango erwango left a comment

Choose a reason for hiding this comment

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

Approved on my side as the latest discussion on logging doesn't feel blocking to me.

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

Successfully merging this pull request may close these issues.

6 participants