EthernetInterface fix detecting change of connection status on ARCH_MAX

[zhiyong-ft]

Summary of changes

Fixed bug that EthernetInterface doesn't detect change of connection status on ARCH_MAX boards, and also provided facility to change/override default Ethernet PHY address for STM32 based customized boards.

Bug was described in the following thread.
#12367

Impact of changes

None

Migration actions required

None

Documentation

To change default Ethernet PHY address, add the following entry into mbed_app.json:

 "ARCH_MAX": {
     "stm32-emac.eth-phyaddr": 1
 }

---

Pull request type

[X] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

---

Test results

[] No Tests required for this change (E.g docs only update)
[] Covered by existing mbed-os tests (Greentea or Unittest)
[X] Tests / results supplied as part of this PR

Bug on ARCH_MAX board was fixed by adding overriding entry described in documentation section. The same test code compiled for NUCLEO_F429ZI also works.

---

Reviewers

---

@eg-astrouka @toyowata @ARMmbed/mbed-os-ipcore

[ciarmcom]

@zhiyong80, thank you for your changes.
@toyowata @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

[michalpasztamobica]

Looks like a smart fix to me, but let's improve the mbed_lib.json a little bit, please.

[michalpasztamobica]

Wouldn't it be reasonable to add a target_override below for ARCH_MAX? Like this:

 "target_overrides": {
        ...
        "ARCH_MAX": {
            "eth-phyaddr": 0
        }
    }

This way user wouldn't have to figure things out on his/her own.
Also - Some documentation explaining what this option exactly stands for would be a good idea. For example:

"config": {
        "eth-rxbufnb": 4,
        "eth-phyaddr": {
            "help" : "Configures the expected value of PHYAD0 pin (0 for pulled-down or 1 for pulled-up",
            "options": [0, 1],
            "value" : 0
        }

Feel free to come up with a better description or remove the option limitation, if I misunderstood how this pin works.

[zhiyong-ft]

This makes sense, I would change the default for ARCH_MAX to 1 and options can be an array of different values depending on PHY used. DP83848 can use any value between 0 ~ 31. LAN8742 uses only 0 and 1 though.

[michalpasztamobica]

Right, ARCH_MAX should use 1 by default, not 0.
I am not sure if there is a way to define a range of numbers neatly in mbed_lib.json. I haven't found such configuration in any of the existing mbed_lib.jsons and from json docs I read that you'd have to set a "minimum" and "maximum" values. If you feel like experimenting with this, go ahead, but it's also fine to give up on the "options" field and just specify the valid values in the "help" message.

[zhiyong-ft]

I just took out the options line. If someone is adventurous enough to play with with PHYAD pins, he/she will figure out what value to put there:)

[zhiyong-ft]

Can anyone test the status change bug on other STM32 based Ethernet boards? ODIN-W2 and u-blox C030 seem need to use 1 as well.

[michalpasztamobica]

@zhiyong80 , to my understanding the PHYADDR stays the same (0) for all boards apart from ARCH_MAX? I don't think it is necessary to test them all, if nothing changed for them. Our CI will run quite some STM boards, once started, so we will be able to see any potential failures.

[michalpasztamobica]

And one more thing - if the PHYADDR has a value dependent on a pull-up/pull-down configuration then how come it can have any value other than [0, 1] (up to 31, you said)? Is there something else influencing this value? Perhaps the "help" field needs a further update?

[zhiyong-ft]

@michalpasztamobica Once the board is designed and manufactured, phy address will stay the same. But people may design their own boards and change the strapping of PHYAD pin(s), therefore need to override it.

[michalpasztamobica]

The current version looks good to me.

[zhiyong-ft]

UBox boards seem to pullup PHYAD0 during initialization. But you are right all 144pin nucleo boards have PHYAD0 pulled down, and DISCO boards don't pullup/down PHYAD0 so default 0 will be used as well.

[michalpasztamobica]

@zhiyong80 , I guess CI might not have a test which plugs the ethernet cable in and out ;-). I will test UBLOX_EVK_ODIN_W2 on my desk and come back to you with the results. Perhaps we need to add another "target_override" for Ublox.

[jeromecoutant]

You could remove ETH_PHY_ADDRESS definition,
and use /
EthHandle.Init.PhyAddress = MBED_CONF_STM32_EMAC_ETH_PHYADDR;

[michalpasztamobica]

@zhiyong80 , regarding UBLOX_EVK_ODIN_W2, with the current master branch it works fine:

210: Network Status = 3
211: Network Status = 3
// Here I disconnected the network cable
212: Network Status = 1
213: Network Status = 1

So I guess it needs 0 in PHYADDR or perhaps this value gets overridden somewhere else in the code. Anyway - it should work with the current setting and no need to add a separate "target_override" for it.

[zhiyong-ft]

And one more thing - if the PHYADDR has a value dependent on a pull-up/pull-down configuration then how come it can have any value other than [0, 1] (up to 31, you said)? Is there something else influencing this value? Perhaps the "help" field needs a further update?

Because some Phy have more than 1 PHYAD pin, DP83848 has 5, but other Phy used by STM32 based Ethernet boards do have only 1 PHYAD pin. Unfortunately pulling PHYAD0 on DP83848 has other implications.

[zhiyong-ft]

You could remove ETH_PHY_ADDRESS definition,
and use /
EthHandle.Init.PhyAddress = MBED_CONF_STM32_EMAC_ETH_PHYADDR;

I would prefer to keep this extra line, otherwise it breaks the pattern of original code.

[0xc0170]

CI started

[mbed-ci]

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

• jenkins-ci/mbed-os-ci_dynamic-memory-usage

[0xc0170]

Job restarted (internal fault)