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

[scaprile]

Discussed in https://github.com/zephyrproject-rtos/zephyr/discussions/77662

^{Originally posted by **scaprile** August 27, 2024} The following boards stopped building after https://github.com/zephyrproject-rtos/zephyr/commit/84889d478362876e3a2bf4669e205def14c6794b: 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[bot]]

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: https://github.com/zephyrproject-rtos/zephyr/blob/25e90e7bb0ef04ce087fbba2ddc08e918334cfa6/drivers/ethernet/Kconfig.stm32_hal#L61-L65

[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.