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

Support of the flash for SiWx917 #64

Merged
merged 6 commits into from
Nov 22, 2024

Conversation

Martinhoff-maker
Copy link
Collaborator

@Martinhoff-maker Martinhoff-maker commented Nov 14, 2024

Multiple commits :

  • Add flash driver for the SiWx917
  • Add tests concerning flash driver
  • Add code-partition to specify NWP firmware partition and Zephyr application code partition

@Martinhoff-maker Martinhoff-maker force-pushed the siwx917-flash branch 2 times, most recently from 4a274c8 to 50c64c7 Compare November 14, 2024 16:39
@Martinhoff-maker Martinhoff-maker marked this pull request as ready for review November 14, 2024 16:45
drivers/flash/Kconfig.siwx917 Outdated Show resolved Hide resolved
drivers/flash/soc_flash_silabs_siwx917.c Show resolved Hide resolved
#size-cells = <1>;

// NWP (Network Processor) partition (do not overwrite)
nwp_firmware_partition: partition@0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? The NWP sees the flash at a different address (in the 0x4xxx range), where it has a 1.8 MB image that the M4 can't access. As I understand it, the memory map as seen by the M4 actually does start at 0x08202000, not 0x08200000.

Copy link
Contributor

@jerome-pouiller jerome-pouiller Nov 15, 2024

Choose a reason for hiding this comment

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

I have not found proper reference for memory mapping.

If I understand right, the "917 reference manual" says the flash start at 0x08000000 (4.3.4 Address Mapping). The size NWP firmware (in fact the size of the .rps file) is 1.6MB. So, it would probably makes sense to reserve 2MB and place the M4 firmware at 0x08200000. However, isp_prepare.py prefixes the firmware with 4096 bytes for the rps header (I believe it could be decreased). So, the firmware should be placed at 0x08201000.

BTW, the datasheet says siwg917m111mgtba has 8MB of flash.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

After an investigation, our thought are that the flash start at 0x08000000 and has a size of 8MB. The NWP firmware partition would reserved 2MB of flash memory from 0x8000000 to 0x8200000. The code partition will start at 0x8200000 and will have a size of 6MB (we reserved 8KB for rps header at the start). As we are in a common flash mode, we supposed to be able to write from 0x8000000 to 0x8800000 (and eventually deliberately write on the nwp firmware in case of a test) but what we see is that we can only write in flash from 0x8200200 to 0x8400000. (like we only have 4KB of flash memory. discussion is ongoing with SMalae to understand what happens on NWP side with WiseConnect.

Copy link
Contributor

@asmellby asmellby Nov 19, 2024

Choose a reason for hiding this comment

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

The important thing to realize is that in common flash mode, the NWP owns the flash. The M4 is a subordinate that is only allowed to use the subset the NWP gives it access to. This is opposite from all other Silabs devices, where the user-facing CPU is the owner of its memory.

#address-cells = <1>;
#size-cells = <1>;

flash0: flash@8200000 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should actually be moved to the board dts, since the address of the flash changes depending on whether the board uses common (internal) flash or dual flash. For now, I see that the flash driver only supports common flash, so it doesn't really matter, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also believe the size of the flash depends of the part.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

From my point of view and what I see in the other .dts, start address of the flash will be in siwg917.dtsi and the size of the flash will be in the part specific siwg917m111mgtba.dtsi. We then declare flash partition in the board specific .dts.

Copy link
Contributor

@asmellby asmellby Nov 19, 2024

Choose a reason for hiding this comment

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

SiWx917 is unique in that the start address changes depending on common vs. dual flash mode -- the memory mapping of "M4 gets access to a slice of the flash owned by the NWP" (common flash) vs. "M4 has its own dedicated flash" (dual flash) is completely different. It's really weird, but it's how it works. And the choice of whether the second flash chip is added is a board-level decision.

drivers/flash/soc_flash_silabs_siwx917.c Show resolved Hide resolved
drivers/flash/soc_flash_silabs_siwx917.c Outdated Show resolved Hide resolved
drivers/flash/soc_flash_silabs_siwx917.c Outdated Show resolved Hide resolved
drivers/flash/soc_flash_silabs_siwx917.c Outdated Show resolved Hide resolved
drivers/flash/Kconfig.siwx917 Show resolved Hide resolved
soc/silabs/silabs_siwx917/siwg917/nwp_init.c Outdated Show resolved Hide resolved
#address-cells = <1>;
#size-cells = <1>;

flash0: flash@8200000 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I also believe the size of the flash depends of the part.

#size-cells = <1>;

// NWP (Network Processor) partition (do not overwrite)
nwp_firmware_partition: partition@0 {
Copy link
Contributor

@jerome-pouiller jerome-pouiller Nov 15, 2024

Choose a reason for hiding this comment

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

I have not found proper reference for memory mapping.

If I understand right, the "917 reference manual" says the flash start at 0x08000000 (4.3.4 Address Mapping). The size NWP firmware (in fact the size of the .rps file) is 1.6MB. So, it would probably makes sense to reserve 2MB and place the M4 firmware at 0x08200000. However, isp_prepare.py prefixes the firmware with 4096 bytes for the rps header (I believe it could be decreased). So, the firmware should be placed at 0x08201000.

BTW, the datasheet says siwg917m111mgtba has 8MB of flash.

If the DT node chosen/zephyr,flash is defined, the reg address of this
device is used as default value for FLASH_BASE_ADDRESS.

Using this feature the user does not have to repeat the same information
in Device Tree and in the configuration.

Signed-off-by: Jérôme Pouiller <[email protected]>
The flash driver need to initialize NetWork coProcessor (NWP) without
BLE or WiFi.

In addition, if neither BLE or WiFi is selected, compiler complain cfg
is not used. So slightly change the code to make de compiler happy.

Signed-off-by: Jérôme Pouiller <[email protected]>
@Martinhoff-maker Martinhoff-maker force-pushed the siwx917-flash branch 2 times, most recently from 0712e79 to 8f4b85f Compare November 19, 2024 15:20
Copy link
Contributor

@jerome-pouiller jerome-pouiller left a comment

Choose a reason for hiding this comment

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

Mostly fine.

drivers/flash/Kconfig.siwx917 Outdated Show resolved Hide resolved
dts/arm/silabs/siwg917m111mgtba.dtsi Outdated Show resolved Hide resolved
@Martinhoff-maker Martinhoff-maker force-pushed the siwx917-flash branch 4 times, most recently from 471087a to 0bb0826 Compare November 20, 2024 10:09
@Martinhoff-maker
Copy link
Collaborator Author

Martinhoff-maker commented Nov 20, 2024

With the help of SMalae, a confluence page (https://confluence.silabs.com/pages/viewpage.action?spaceKey=EN&title=9117B0+Flash+Map#id-9117B0FlashMap-M4regioninCommonflash-8MBGreenInuse) and this AN (https://www.silabs.com/documents/public/application-notes/an1416-siwx917-soc-memory-map.pdf) , it is much clearer now for me. I understand the fact that the flash start address will change with the dual flash mode and then the flash start address must be declared in the board.dts file. For the moment, the only mode supported is "common" so as you said @asmellby, it will be okay. The support of the dual flash memory might be done in another PR ?

Copy link
Contributor

@jerome-pouiller jerome-pouiller left a comment

Choose a reason for hiding this comment

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

The memory mapping is still not accurate. However, I believe we can move on.
I may submit a new PR with the changes I have in mind but it is not a priority for now.

Copy link
Collaborator

@jhedberg jhedberg left a comment

Choose a reason for hiding this comment

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

Looks generally fine, but please fix the return value type of flash_siwx917_range_is_in_bounds()

drivers/flash/soc_flash_silabs_siwx917.c Outdated Show resolved Hide resolved
jerome-pouiller and others added 4 commits November 22, 2024 09:27
Add flash driver for SiWx917.

Co-authored-by: Martin Hoff <[email protected]>

Signed-off-by: Jérôme Pouiller <[email protected]>
Signed-off-by: Martin Hoff <[email protected]>
Simplify code usage.

Signed-off-by: Jérôme Pouiller <[email protected]>
A specific section of the flash on the SiWx917 is reserved for the
Network co-processor (nwp). This zone needs to be delimited in order to
not overwrite it. We then need to use "code-partition" to provide Zephyr
the application code flash location.

Signed-off-by: Martin Hoff <[email protected]>
Allow CI to build tests concerning flash driver.
Extra-arg in the twister command line (DTC_OVERLAY_FILE) allows
to have the .overlay file in zephyr-silabs repository and not in the
official zephyr repository.

Co-authored-by: Martin Hoff <[email protected]>

Signed-off-by: Jérôme Pouiller <[email protected]>
Signed-off-by: Martin Hoff <[email protected]>
@jhedberg jhedberg merged commit 39da958 into SiliconLabsSoftware:main Nov 22, 2024
3 checks passed
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.

4 participants