stm32: eth: build issue after PHY address resolution fix (84889d4)

[scaprile]

Discussed in #77662

^{Originally posted by scaprile August 27, 2024}
The following boards stopped building after 84889d4:

nucleo_h563zi
stm32h573i_dk
nucleo_h743zi
stm32h735g_disco
stm32h745i_disco/stm32h745xx/m7
stm32h747i_disco/stm32h747xx/m7
nucleo_h753zi
nucleo_h755zi_q/stm32h755xx/m7

The error is this one:

zephyr/drivers/ethernet/eth_stm32_hal.c:62:92: error: pasting ")" and "_ORD" does not give a valid preprocessing token
   62 |             DEVICE_DT_GET(DT_CHILD(DT_INST_CHILD(n, mdio), __CONCAT(ethernet_phy_, PHY_ADDR)))

looks like CONFIG_ETH_STM32_HAL_PHY_ADDRESS does not get a value (hence also PHY_ADDR)

nucleo_h563zi, for example, builds again when rolling back that commit.

My prj.conf is bare minimum

CONFIG_NETWORKING=y
CONFIG_NET_IPV4=y
CONFIG_NET_IPV6=y
CONFIG_NET_TCP=y
CONFIG_NET_UDP=y
CONFIG_NET_DHCPV4=y
CONFIG_NET_SOCKETS=y
CONFIG_NET_SOCKETS_POLL_MAX=32
CONFIG_POSIX_API=y
CONFIG_POSIX_MAX_FDS=32
CONFIG_NET_MAX_CONN=10
CONFIG_NET_MAX_CONTEXTS=10
CONFIG_NET_CONFIG_SETTINGS=y
CONFIG_NET_CONNECTION_MANAGER=y
CONFIG_NET_LOG=y

CONFIG_LOG=y
CONFIG_ISR_STACK_SIZE=2048
CONFIG_MAIN_STACK_SIZE=8192
CONFIG_IDLE_STACK_SIZE=1024

CONFIG_MINIMAL_LIBC=y
CONFIG_MINIMAL_LIBC_RAND=y
CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=32768

Is this an error on my side ? Something essential is missing that breaks Kconfig reading ?

These boards, nevertheless, build OK with that very same prj.conf:

nucleo_f207zg
nucleo_f429zi
nucleo_f746zg
nucleo_f767zi

[github-actions]

Hi @scaprile! 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]

@AlexFabre Can you have a look ?

[AlexFabre]

Hi @scaprile,

In the previous PR we highlighted a point regarding the provenance of the PHY_ADDR value.

If you add this definition in your config file, does it build properly ?

CONFIG_ETH_STM32_HAL_PHY_ADDRESS=0

[AlexFabre]

These boards, nevertheless, build OK with that very same prj.conf:

nucleo_f207zg
nucleo_f429zi
nucleo_f746zg
nucleo_f767zi

Only the STM32 H5 and H7 are concerned because #71012 introduced this new mdio address resolution only for those two families.

[AlexFabre]

@scaprile are you building a sample or a custom project ?

I didn't mange to reproduce your problem yet.

[erwango]

@scaprile are you building a sample or a custom project ?

I didn't mange to reproduce your problem yet.

Same here. CONFIG_ETH_STM32_HAL_PHY_ADDRESS always get a value, even if not defined.
Which is expected given the way it is defined unconditionally:

zephyr/drivers/ethernet/Kconfig.stm32_hal

Lines 61 to 65 in 25e90e7

	config ETH_STM32_HAL_PHY_ADDRESS
	int "Phy address"
	default 0
	help
	The phy address to use.

[scaprile]

@scaprile are you building a sample or a custom project ?

@AlexFabre : custom project, I posted my prj.conf at the discussion.

I can see CONFIG_ETH_STM32_HAL_PHY_ADDRESS defined at build/zephyr/.config :

$ grep CONFIG_ETH_STM32_HAL_PHY_ADDRESS build/zephyr/.config 
CONFIG_ETH_STM32_HAL_PHY_ADDRESS=0

Adding it to my prj.conf makes no difference.

Here is my prj.conf:

CONFIG_NETWORKING=y
CONFIG_NET_IPV4=y
CONFIG_NET_IPV6=y
CONFIG_NET_TCP=y
CONFIG_NET_UDP=y
CONFIG_NET_DHCPV4=y
CONFIG_NET_SOCKETS=y
CONFIG_NET_SOCKETS_POLL_MAX=32
CONFIG_POSIX_API=y
CONFIG_POSIX_MAX_FDS=32
CONFIG_NET_MAX_CONN=10
CONFIG_NET_MAX_CONTEXTS=10
CONFIG_NET_CONFIG_SETTINGS=y
CONFIG_NET_CONNECTION_MANAGER=y
CONFIG_NET_LOG=y

CONFIG_LOG=y
CONFIG_ISR_STACK_SIZE=2048
CONFIG_MAIN_STACK_SIZE=8192
CONFIG_IDLE_STACK_SIZE=1024

CONFIG_MINIMAL_LIBC=y
CONFIG_MINIMAL_LIBC_RAND=y
CONFIG_COMMON_LIBC_MALLOC_ARENA_SIZE=32768

and I build with west build -b nucleo_h563zi -p auto $(realpath $(CURDIR)) inside a Makefile, using your Docker image zephyrprojectrtos/ci (this is part of a CI/CD test setup)
(previously I run west init west update, latest one was yesterday around noon, UTC)
Project attached: h563.zip
All projects are basically equal, only hal.h changes, except for those where there is no RNG or it is not yet supported

[AlexFabre]

All right, it looks like GCC complaint about token pasting comes from a wrong macro call.

Instead of "__CONCAT" it should be "_CONCAT" (only one leading underscore).

[scaprile]

I changed that locally, and I can confirm that all the boards now build OK.
Thanks a lot !

[scaprile]

@AlexFabre stm32h745i_disco/stm32h745xx/m7 has its PHY hardwired for address 1:

I would expect Zephyr to configure for that board without user intervention; though I see CONFIG_ETH_STM32_HAL_PHY_ADDRESS=0 in build/zephyr/.config.
Technically, one could force that PHY pin low on reset by setting the GPIO to an output low, releasing reset, and then configuring as an input; I don't know if you folks do something like that, I hope you don't, and I don't currently have a board with me to test if it actually works.

Setting CONFIG_ETH_STM32_HAL_PHY_ADDRESS=1 in prj.conf breaks the build, now the error is this one:

                 from zephyr/drivers/ethernet/eth_stm32_hal.c:13:
zephyr/include/zephyr/device.h:92:41: error: '__device_dts_ord_DT_N_S_soc_S_ethernet_40028000_S_mdio_S_ethernet_phy_1_ORD' undeclared here (not in a function); did you mean 'DT_N_S_soc_S_ethernet_40028000_S_mdio_S_ethernet_phy_0_ORD'?
   92 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
      |                                         ^~~~~~~~~
zephyr/include/zephyr/toolchain/common.h:137:26: note: in definition of macro '_DO_CONCAT'
  137 | #define _DO_CONCAT(x, y) x ## y
      |                          ^
zephyr/include/zephyr/device.h:92:33: note: in expansion of macro '_CONCAT'
   92 | #define DEVICE_NAME_GET(dev_id) _CONCAT(__device_, dev_id)
      |                                 ^~~~~~~

Please note that not adding that line, or even setting CONFIG_ETH_STM32_HAL_PHY_ADDRESS=0 builds OK.
Let me know if you would like this to be in a separate issue.

[AlexFabre]

That is the point we highlighted in #76978 that still needs to be addressed.

The PHY address has yet to be the same for both the DTS label and the CONFIG_ETH_STM32_HAL_PHY_ADDRESS definition.

A possible simplification would be to restrict the PHY address resolution to the DTS label only, and delete CONFIG_ETH_STM32_HAL_PHY_ADDRESS. To be discussed.

Regarding the stm32h745i_disco

stm32h745i_disco/stm32h745xx/m7 has its PHY hardwired for address 1. But in its dts the PHY node label address is: ethernet-phy@0

&mdio {
	status = "okay";
	pinctrl-0 = <&eth_mdio_pa2 &eth_mdc_pc1>;
	pinctrl-names = "default";

	ethernet-phy@0 {
		compatible = "ethernet-phy";
		reg = <0x00>;
		status = "okay";
	};
};

As @erwango said, by default, when not set, CONFIG_ETH_STM32_HAL_PHY_ADDRESS will be 0.

That's why the code will build.

However, to fulfill the hardware schematic, CONFIG_ETH_STM32_HAL_PHY_ADDRESS has to be set to 1 and the dts label modified to ethernet-phy@1.

[AlexFabre]

@erwango I started discussion #77782 to talk about that PHY address resolution inside the HAL.

[scaprile]

That is the point we highlighted in #76978 that still needs to be addressed.

Oh, thanks, now I understand. I didn't get that, I'm not familiar with your internals and I bypass ST's HAL as much as possible.