Overriding HAL callbacks in stm32xx_emac.cpp

[pgscada]

Description of defect

It is not possible to provide a custom (overridden) EMAC on STM32 devices without modifying the mbed source files because:

1. If we attempt to write our own ETH_IRQHandler or HAL_ETH_RxCpltCallback functions, the source will not compile because these functions are already defined in stm32xx_emac.cpp.
2. We cannot use the provided implementations of these functions because they ignore any custom EMAC that may have been configured in the program and revert back to the mbed provided EMAC.
3. No macros or any alternatives are provided by the mbed environment to suppress the HAL callbacks defined by mbed in stm32xx_emac.cpp.

A developer wishing to provide a custom EMAC to the NetworkInterface has to modify the mbed source file stm32xx_emac.cpp. This is not desirable and should be avoided.

Target(s) affected by this defect ?

All devices using connectivity/drivers/emac/TARGET_STM/stm32xx_emac.cpp

Toolchain(s) (name and version) displaying this defect ?

All toolchains

What version of Mbed-os are you using (tag or sha) ?

mbed-os-6.2.1

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli 1.8.3

How is this defect reproduced ?

• Create a custom EMAC class and override function MBED_WEAK EMAC &EMAC::get_default_instance() located in stm32xx_emac.cpp
• Create NewtorkInterface using custom EMAC (ie, NetworkInterface iface(&my_emac);)
• Run project and observe error

++ MbedOS Error Info ++
Error Status: 0x80010133 Code: 307 Module: 1
Error Message: Mutex: 0x2000E924, Not allowed in ISR context
Location: 0x800279F
Error Value: 0x2000E924
Current Thread: rtx_idle Id: 0x20001124 Entry: 0x80027AD StackSize: 0x380 StackMem: 0x20001468 SP: 0x2004FF34
For more info, visit: https://mbed.com/s/error?error=0x80010133&osver=60201&core=0x410FC271&comp=2&ver=90300&tgt=DISCO_F746NG
-- MbedOS Error Info --

• Edit stm32xx_emac.cpp, comment out function void HAL_ETH_RxCpltCallback(ETH_HandleTypeDef *heth) and ETH_IRQHandler
• Run project and observe no error

Suggested solutions

• Both HAL callbacks provided by mbed reference STM32_EMAC::get_instance() instead of the weak functions MBED_WEAK EMAC &EMAC::get_default_instance()

void HAL_ETH_RxCpltCallback(ETH_HandleTypeDef *heth)
{
    STM32_EMAC &emac = STM32_EMAC::get_instance();
    if (emac.thread) {
        osThreadFlagsSet(emac.thread, FLAG_RX);
    }
}
void ETH_IRQHandler(void)
{
    STM32_EMAC &emac = STM32_EMAC::get_instance();
    HAL_ETH_IRQHandler(&emac.EthHandle);
}

• Marking the two HAL callbacks as MBED_WEAK solves my issue.
• Otherwise, wrap the HAL callbacks inside #ifndef macros which can be disabled from mbed_app.json

[pgscada]

Created PR : #13555 (comment)

[ciarmcom]

@pgscada thank you for raising this issue.Please take a look at the following comments:

Could you add some more detail to the description? A good description should be at least 25 words.
We cannot automatically identify a release based on the version of Mbed OS that you have provided.
Please provide either a single valid sha of the form #abcde12 or #3b8265d70af32261311a06e423ca33434d8d80de
or a single valid release tag of the form mbed-os-x.y.z .
E.g. 'f46a887d53b72233a9a5dbe173bfd5f51b747f09' has not been matched as a valid tag or sha.
NOTE: If there are fields which are not applicable then please just add 'n/a' or 'None'.This indicates to us that at least all the fields have been considered.
Please update the issue header with the missing information, the issue will not be mirroredto our internal defect tracking system or investigated until this has been fully resolved.

[kjbracey]

Curious as to what the intent is here. You're wanting to use the on-chip Ethernet, but are dissatisfied with the supplied driver, so you're trying to replace it externally?

If there's a particular problem with the driver, wouldn't it be better to patch it and upstream the fix?

[pgscada]

Our board has a LAN9303 3 way switch so in addition to onboard MAC there are three external MACs and 3 external PHY which need to be managed.

So far I haven't been able to make the supplied EMAC work correctly, only solution has been to write my own. That said I have just copied the supplied STM32_EMAC and added some minor tweaks. (Ie, link up/down needs to be read from the external MAC because the internal MAC is always up)

[kjbracey]

So how is the STM32 HAL involved in this? Is the switch downstream from the onboard MAC, so you're wanting to effectively override some of its functionality, because the onboard port is now "internal"? That would be a bit tricky - not sure about it.

Or are these 3 completely separate additional ports? If they are, then you should be operating entirely independently of the driver stack for the existing port.

Simplest case for extra independent Ethernet ports is to start at the C++ EMAC class, and implement that from scratch. Make a LAN9303 class implementing that API, and instantiate that 3 times. That wouldn't conflict with the STM32 EMAC at all.

Or is there some MAC compatibility with the STM MAC hardware, so you're trying to re-use the STM MAC driver code? I can imagine that might be possible, but I doubt the STM HAL would have expected that use case.

[pgscada]

Yes I want to use the network interface API, but I can't do it because mbed has reserved the use of the HAL callback functions.

I'm not sure why every single HAL reserved callback function is not marked MBED_WEAK inside mbed. They really should be because these functions have special meaning to HAL and if we want to override any mbed functionality then you get stuck or, like you say have to reinvent the wheel completely?

[kjbracey]

"the network interface API"

More specifically? If you're implementing an Mbed OS network driver that is nothing to do with the STM32 chip, then you go in at the EMAC C++ level.

I'm not sure why every single HAL reserved callback function is not marked MBED_WEAK inside mbed.

Because the extension mechanisms for the STM HAL are that it has weak defaults, and they need to be overridden by non-weak functions.

You can't override a weak function with a weak function. So that extension mechanism only works once. If Mbed OS is overriding the STM HAL, then an app can't override again. If Mbed OS's own functions were weak, it wouldn't link, as the linker couldn't choose which of 2 weak definitions to use. (Unless the app was overriding, in which case it's non-weak version would win).

This is a non-extensible mechanism, and it's ST's HAL design, not Mbed OS's.

If you want an extensible multi-layer mechanism, you have to do something other than "weak". I was suggesting something along those lines here for a multi-layer RAM init thing, but it was too clever: #13022

reinvent the wheel completely?

I'm not quite clear which if any wheel is being reinvented in this case. These surely aren't STM-hardware-compatible Ethernet controllers? To add an Ethernet MAC to the system, you implement your own EMAC object. The STM32_EMAC object already in the system is specifically a driver for the STM EMAC.

There is a common API to implement (EMAC), and if it's incompatible hardware, then you need a different driver implementing that API.

[ciarmcom]

Thank you for raising this detailed GitHub issue. I am now notifying our internal issue triagers.
Internal Jira reference: https://jira.arm.com/browse/IOTOSM-2249