STM32H7S78-DK Ethernet support

[bklaric1]

Is your feature request related to a problem? Please describe.

I wanted to implement Ethernet support for the STM32H7S78-DK board, which under Zephyr at the moment doesn't officially support Ethernet.

I based my work on the STM32H7S3L8 board (with adapted pins) and successfully tested Ethernet using the dumb_http_server sample.

Before opening a PR, I’d like feedback on one implementation detail.

The setup for Ethernet DMA in mpu_regions.c depends on sram2:

zephyr/soc/st/stm32/stm32h7rsx/mpu_regions.c

Lines 33 to 42 in 91b1b84

	#if DT_NODE_HAS_STATUS_OKAY(DT_NODELABEL(mac))
	#define sram_eth_node DT_NODELABEL(sram2)
	#if DT_NODE_HAS_STATUS_OKAY(sram_eth_node)
	/* Region 5 - Ethernet DMA buffer RAM */
	MPU_REGION_ENTRY("SRAM_ETH_BUF", DT_REG_ADDR(sram_eth_node),
	REGION_RAM_NOCACHE_ATTR(REGION_16K)),
	/* Region 6 - Ethernet DMA descriptor RAM (overlays the first 256B of SRAM_ETH_BUF)*/
	MPU_REGION_ENTRY("SRAM_ETH_DESC", DT_REG_ADDR(sram_eth_node), REGION_PPB_ATTR(REGION_256B)),
	#endif
	#endif

But sram2 is currently disabled, so the MPU regions are not created unless the user manually enables it:

zephyr/dts/arm/st/h7rs/stm32h7rs.dtsi

Lines 67 to 77 in 91b1b84

	/* System data RAM accessible over AHB bus: SRAM2 in D2 domain */
	sram2: memory@30004000 {
	compatible = "zephyr,memory-region", "mmio-sram";
	reg = <0x30004000 DT_SIZE_K(16)>;
	zephyr,memory-region = "SRAM2";
	/* Disable SRAM2 by default to avoid unintended access.
	* To enable it, explicitly define zephyr,memory-attr
	* to configure MPU attributes.
	*/
	status = "disabled";
	};

Describe the solution you'd like

There are two different ways of solving this, at least known to me at this moment:

1. Enabling sram2 in stm32h7s78_dk-common.dtsi and reducing the sram0 by 16KB:

&sram0 {
	reg = <0x24000000 DT_SIZE_K(440)>;
};

...

&sram2 {
	status = "okay";
	zephyr,memory-attr = <DT_MEM_ARM(ATTR_MPU_RAM)>;
};

2. Replacing sram2 with sram1 here (or creating a fallback, if sram2 is not available):
   

   zephyr/soc/st/stm32/stm32h7rsx/mpu_regions.c

   Lines 33 to 42 in 91b1b84

   	#if DT_NODE_HAS_STATUS_OKAY(DT_NODELABEL(mac))
   	#define sram_eth_node DT_NODELABEL(sram2)
   	#if DT_NODE_HAS_STATUS_OKAY(sram_eth_node)
   	/* Region 5 - Ethernet DMA buffer RAM */
   	MPU_REGION_ENTRY("SRAM_ETH_BUF", DT_REG_ADDR(sram_eth_node),
   	REGION_RAM_NOCACHE_ATTR(REGION_16K)),
   	/* Region 6 - Ethernet DMA descriptor RAM (overlays the first 256B of SRAM_ETH_BUF)*/
   	MPU_REGION_ENTRY("SRAM_ETH_DESC", DT_REG_ADDR(sram_eth_node), REGION_PPB_ATTR(REGION_256B)),
   	#endif
   	#endif

I think this ties, at least partially, to this issue: #98866, since I needed to enable CONFIG_SPEED_OPTIMIZATIONS=y in order to avoid MPU FAULT.

If this is a good way to implement Ethernet support for this board, I will gladly open a PR with these and other needed changes to enable Ethernet.

Thanks in advance.

Describe alternatives you've considered

No response

[github-actions]

Hi @bklaric1! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[erwango]

See #97364 and #98866

[bklaric1]

Thanks for the links. I missed the PR commit one.

As another test, I tried just enabling mac and mdio in stm32h7s78_dk-common.dtsi and leaving sram2 as is (disabled in stm32h7rs.dtsi).
My setup looks like this (the relevant parts):

• sample: http_server

• prj.conf: same as http_server with addition of this:

  CONFIG_SPEED_OPTIMIZATIONS=y
  CONFIG_ETH_DRIVER=y
  

  Without the first config, the app crashes immediately (even with sram2 enabled), as mentioned in the issue you and me both linked.

• stm32h7s78_dk-common.dtsi: enabled mac and mdio like this:

  &mac {
  	pinctrl-0 = <&eth_rmii_ref_clk_pd7
  			&eth_rmii_crs_dv_pa7
  			&eth_rmii_rxd0_pc4
  			&eth_rmii_rxd1_pc5
  			&eth_rmii_tx_en_pg11
  			&eth_rmii_txd0_pb0
  			&eth_rmii_txd1_pb1>;
  	pinctrl-names = "default";
  	phy-connection-type = "rmii";
  	phy-handle = <&eth_phy>;
  	status = "okay";
  };
  
  &mdio {
  	pinctrl-0 = <&eth_mdio_pa2 &eth_mdc_pc1>;
  	pinctrl-names = "default";
  	status = "okay";
  
  	eth_phy: ethernet-phy@0 {
  		compatible = "ethernet-phy";
  		reg = <0x00>;
  	};
  };
  

  Note that the pins are correct for this board and the pc1 jumper is set.

The build is successful and doesn't log any errors or warnings. I used this setting when running west build: -b stm32h7s78_dk/stm32h7s7xx/ext_flash_app so to flash the app to external flash.

After flashing and opening the serial terminal, this is what I see when the app boots:

...
I: Jumping to the first image slot
I: PHY (0) ID 7C131
E: ***** BUS FAULT *****
E:   Imprecise data bus error
E: r0/a1:  0x00000000  r1/a2:  0x00000000  r2/a3:  0x00000018
E: r3/a4:  0x7004e3d8 r12/ip:  0x40029000 r14/lr:  0x70035b63
E:  xpsr:  0x81000000
E: Faulting instruction address (r15/pc): 0x70035bce
E: >>> ZEPHYR FATAL ERROR 26: Unknown error on CPU 0
E: Current thread: 0x24005110 (main)
E: Halting system

Trying to build and flash with the same setup, with the sram2 enabled (and sram0 size reduced), the app starts as normal and I can ping the board.

Therefore, I can confirm that without enabling sram2 on stm32h7s78_dk board, the Ethernet can't be used.

The mpu_regions.c for h7x series even has a fallback, as I mentioned as a possible solution:
https://github.com/JB-Lighting/zephyr/blob/13daf245a801a0b3d79a67221e4b599fd1cb3ad9/soc/st/stm32/stm32h7x/mpu_regions.c#L24-L41

I am sorry if I am missing something very obvious. I am relatively new to Zephyr and am trying to understand/contribute to the project, at least for this board.

[erwango]

Sorry, I read your description too quickly and missed that you already had some insight on this.
Btw, it could be useful to make your title bit more precise to be sure reader got the point quickly.

Therefore, I can confirm that without enabling sram2 on stm32h7s78_dk board, the Ethernet can't be used.

Agreed. @TD-JBL Is that a possible leftover from your implementation ?

[bklaric1]

I agree. The title is a bit misleading. Sorry. I'll keep that in mind for future issues.
If @TD-JBL confirms it, I can open a PR with needed changes, since I already have them implemented locally.

[TD-JBL]

Yes, sram2 needs to be enabled for ethernet. But the sram2 node must not have the zephyr,memory-attr attribute set. This would have a higher priority in the mpu regions configuration than the setup defined in mpu_regions.c. That's why it was disabled by default in PR #97364 and one must enable it in the board's dt and set the zephyr,memory-attr attribute if used for non-ethernet stuff.
See my comment for more details on this.

Yes, h7x has a list of srams and the first available will be used (typically AHB sram is chosen - somewhere in the h7x ethernet PRs I read that the used sram for ethernet was changed to use AHB sram to get rid of some strange bugs). As AHB sram1 and sram2 in h7rsx are of the same size and configuration, I only implemented sram2 in mpu_regions.c.

There is no need to reduce sram0 in size when using sram1/2, because sram1 and sram2 are the AHB SRAMs, sram0 is the AXI SRAM1-4.

I didn't had the time yet to look into the memory access faults, I don't get them in my configuration.

Regarding your proposed solution:
From my view there is no point in switching sram1/2 in the mpu_regions.c. Maybe add sram1 as fallback (but keep the needed dt node/properties in mind).

Instead of having sram2 disabled by default, we could enable it and set the zephyr,memory-attr attribute in the soc's dt, but then the attribute must be removed in the board's dt when using ethernet?

Maybe I'm also missing something here, as this is quite a complicated topic ;)

[bklaric1]

Okay, this topic is more complex than originally thought.

@TD-JBL I don't quite understand, why setting zephyr,memory-attr attribute for sram2 is not allowed when using Ethernet.
I tested the following:

1. Enabled sram2 in stm32h7rs.dtsi like this:

	/* System data RAM accessible over AHB bus: SRAM2 in D2 domain */
	sram2: memory@30004000 {
		compatible = "zephyr,memory-region", "mmio-sram";
		reg = <0x30004000 DT_SIZE_K(16)>;
		zephyr,memory-region = "SRAM2";
		/* Disable SRAM2 by default to avoid unintended access.
		 * To enable it, explicitly define zephyr,memory-attr
		 * to configure MPU attributes.
		 */
		zephyr,memory-attr = <DT_MEM_ARM(ATTR_MPU_RAM)>;
		//status = "disabled";
	};

2. Added this to mpu_regions.c:

#if DT_NODE_HAS_STATUS_OKAY(DT_NODELABEL(sram2))
#define sram_eth_node DT_NODELABEL(sram2)
#else
#define sram_eth_node DT_NODELABEL(sram1)
#endif

3. Built and flashed the http_server sample. The build log shows that SRAM2 is fully utilized:

Memory region         Used Size  Region Size  %age Used
           FLASH:      320584 B        60 MB      0.51%
             RAM:      122528 B       456 KB     26.24%
           SRAM0:          0 GB       456 KB      0.00%
           SRAM1:          0 GB        16 KB      0.00%
           SRAM2:         16 KB        16 KB    100.00%
            DTCM:          0 GB       128 KB      0.00%
            ITCM:          0 GB        64 KB      0.00%
          EXTMEM:          0 GB       128 MB      0.00%
           PSRAM:          0 GB        32 MB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%

The app starts just fine after flashing and I can ping the board (and open the website).

Maybe I too am missing something here :)

[TD-JBL]

My understanding is that Zephyr now has multiple mpu regions defined for sram2. The one created because of zephyr,memory-attr = <DT_MEM_ARM(ATTR_MPU_RAM)>; and the 2 other ones coming from the mpu_regions.c. And the one from the node's property has a higher priority telling the mpu that it can cache/buffer/whatever this region used by DMA ....

Note: sram1 must also be added to sections.ld when added to mpu_regions.c (see h7x version of this).

[bklaric1]

Thanks for the note. I forgot to add that to sections.ld.

After adding it, I decided to test without enabling sram2 (so with sram1), and it worked exactly the same. After building, the memory regions look like this:

Memory region         Used Size  Region Size  %age Used
           FLASH:      320528 B        60 MB      0.51%
             RAM:      122528 B       456 KB     26.24%
           SRAM0:          0 GB       456 KB      0.00%
           SRAM1:         16 KB        16 KB    100.00%
            DTCM:          0 GB       128 KB      0.00%
            ITCM:          0 GB        64 KB      0.00%
          EXTMEM:          0 GB       128 MB      0.00%
           PSRAM:          0 GB        32 MB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%

Now I don't have enough knowledge about the arm core to correctly say how enabling sram2 (with DT_MEM_ARM(ATTR_MPU_RAM) attribute) reflects on other samples and user code. Therefore I can't say if this is the correct way to implement Ethernet support (hence the issue rather than PR).

My opinion on this is that not enabling Ethernet support for this board because of this reason (which after local testing isn't causing any unexpected errors) isn't enough to drop this feature.

I agree with you that this can cause problems under specific conditions.

Maybe someone with deeper knowledge on this topic can shed some light on this?

[TD-JBL]

I really expected ethernet to fail when having the zephyr,memory-attr = <DT_MEM_ARM(ATTR_MPU_RAM)>; property set on the sram where mpu_regions.c also define the SRAM_ETH_BUF and SRAM_ETH_DESC regions.

I looked into the MPU region configuration on the running board and it is as expected: The regions from mpu_region.c are overlayed by a new region defining the sram2 as standard cachable ram. So I would have expected that ethernet is not working anymore. I'm not sure why it is working (tested also on my side). Maybe AHB SRAM is not cachable by hardware design? I found a note telling this, but don't know where I found this information.