STM32 EMAC : add configuration choice and connection check #12464

jeromecoutant · 2020-02-19T08:43:05Z

Summary of changes

Request/question started in https://forums.mbed.com/t/stm32f767-ethernet-with-dp83640/7149

This also fixes #12416

So few corrections are proposed:

Add some flexibility and be able to configure locally parameters value depending on chosen PHY
Add several check before setting OK init and connection procedures

Impact of changes

Migration actions required

STM32 EMAC is more configurable for each application.
Default setting have been set for the supported ST boards.

Documentation

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)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

ciarmcom · 2020-02-19T10:00:47Z

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

JojoS62 · 2020-02-19T13:39:25Z

features/netsocket/emac-drivers/TARGET_STM/mbed_lib.json

+        },
+        "eth-phy-reset-delay": {
+            "help" : "Reset process time - Default value: 0.5s as specified in LAN8742A datasheet",
+            "value" : "5000"


5000 ms is 5 s, not 0.5 s.

JojoS62 · 2020-02-19T17:45:21Z

features/netsocket/emac-drivers/TARGET_STM/stm32xx_emac.cpp

@@ -668,14 +713,18 @@ void STM32_EMAC::phy_task()
    uint32_t status;

    if (HAL_ETH_ReadPHYRegister(&EthHandle, PHY_BSR, &status) == HAL_OK) {
-        if (emac_link_state_cb) {
+        if ((emac_link_state_cb) && (status != 0xFFFF)) {


~~this does not work on my target with LAN8720~~, still trying to figure out why.

Edit:
fixed, that was a damned wiring error now. Checking against 0xffff works as intended. Wiring error was a fixed level on MDIO pin, and every phy_read returned 0xffff. This is not detected by the interface.

timovanasten · 2020-02-20T08:45:06Z

features/netsocket/emac-drivers/TARGET_STM/stm32xx_emac.cpp

    EthHandle.Init.Speed = ETH_SPEED_100M;
    EthHandle.Init.DuplexMode = ETH_MODE_FULLDUPLEX;


Why not make this configurable too?

If you think it can be useful, I can add this parameter ?

Yes, that would be useful for us :)

JojoS62 · 2020-02-20T11:28:01Z

Another suggestion: a PHY ID can be read from registers 2 and 3. This could be read out and provided in an API or printed in the debug/trace log. When this ID is invalid, a warning can be issued to find a wrong PHY address easier.

JojoS62 · 2020-02-20T11:34:33Z

features/netsocket/emac-drivers/TARGET_STM/mbed_lib.json

-        "eth-phyaddr": {
+        "thread-stacksize": {
+            "help": "Stack size for stm32_emac_thread",
+            "value": 512


how about increasing the value automatically when the trace is enabled? I think for an user it is difficult to guess a valid stacksize when it is configurable.

During my tests, I didn't see any big difference...

I think the adjustable stacksize come in from this:
#12416 (comment)
usually its a system thread and should have a fixed threadsize. Are there other cases than this special with trace enabled where stacksize can be a problem?

jeromecoutant · 2020-02-20T13:17:47Z

Another suggestion: a PHY ID can be read from registers 2 and 3.

I don't see any register 2 and 3 in LAN8742a...

JojoS62 · 2020-02-20T13:29:50Z

pls check http://ww1.microchip.com/downloads/en/DeviceDoc/DS_LAN8742_00001989A.pdf
page 58ff. This register exists in LAN8742, 8720 and DP83848 and hopefully all others.

jeromecoutant · 2020-02-20T13:56:34Z

pls check http://ww1.microchip.com/downloads/en/DeviceDoc/DS_LAN8742_00001989A.pdf
page 58ff. This register exists in LAN8742, 8720 and DP83848 and hopefully all others.

OK, what you call "register 2 and 3" are "PHY Identifier 1 and 2" ? :-)

This could be read out and provided in an API or printed in the debug/trace log

OK to print in the debug log. Here is what I get:

[INFO][STE1]: PHY IDENTIFIER 1 REGISTER 0x7
[INFO][STE1]: PHY IDENTIFIER 2 REGISTER 0xc131

JojoS62 · 2020-02-20T14:04:37Z

Yes! The guys from nxp are also reading the ID:

mbed-os/features/netsocket/emac-drivers/TARGET_NXP_EMAC/TARGET_LPCTarget/lpc_phy_dp83848.cpp

Lines 107 to 109 in 3662759

    
           #define DP83848C_ID         0x20005C90  /**< PHY Identifier - DP83848C */ 
        
           #define LAN8720_ID          0x0007C0F0  /**< PHY Identifier - LAN8720  */ 
        
           #define KSZ8041_ID          0x00221510  /**< PHY Identifier - KSZ8041  */

jeromecoutant · 2020-02-20T14:34:49Z

PHY ID debug log added.

0xc0170 · 2020-02-24T15:55:03Z

CI started

mbed-ci · 2020-02-24T16:30:17Z

Test run: FAILED

Summary: 3 of 4 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-IAR

0xc0170 · 2020-02-27T13:27:05Z

baremetal example failed,

[Error] stm32f4xx_hal_eth.c@294,40: [Pe020]: identifier "MBED_CONF_STM32_EMAC_ETH_PHY_RESET_DELAY" is undefined
[Error] stm32f4xx_hal_eth.c@369,19: [Pe020]: identifier "MBED_CONF_STM32_EMAC_ETH_PHY_STATUS_REGISTER" is undefined
[Error] stm32f4xx_hal_eth.c@385,19: [Pe020]: identifier "MBED_CONF_STM32_EMAC_ETH_PHY_DUPLEX_STATUS" is undefined
[Error] stm32f4xx_hal_eth.c@396,103: [Pe020]: identifier "MBED_CONF_STM32_EMAC_ETH_PHY_SPEED_STATUS" is undefined
[Error] stm32f4xx_hal_eth.c@849,0: [Pe020]: identifier "MBED_CONF_STM32_EMAC_ETH_RXBUFNB" is undefined

mergify · 2020-02-28T08:46:25Z

This PR cannot be merged due to conflicts. Please rebase to resolve them.

- PHY default configuration can be changed - AutoNegotiation - Speed - DuplexMode - PHY register offset can be updated depending on chosen PHY All unused parameters are cleaned.

Pull request has been modified.

jeromecoutant · 2020-03-02T16:19:13Z

baremetal example failed,

OK, I am not used yet to check this.
Should be corrected now.

0xc0170 · 2020-03-02T16:52:37Z

CI restarted

mbed-ci · 2020-03-02T20:15:01Z

Test run: SUCCESS

Summary: 8 of 8 test jobs passed
Build number : 2
Build artifacts

0xc0170 · 2020-03-03T15:58:57Z

@jeromecoutant Is this ready for integration? It looks like to me it is (ready for merge applied).

0xc0170 · 2020-03-03T16:04:15Z

Received OK! Merging now

This was referenced Feb 19, 2020

Ethernet: devices with LAN8742 and using STM32 HAL work only accidentaly #11929

Closed

NUCLEO_F429ZI occassionally fails to set up ethernet #9470

Closed

ciarmcom requested review from a team February 19, 2020 10:00

ciarmcom added the needs: review label Feb 19, 2020

JojoS62 reviewed Feb 19, 2020

View reviewed changes

timovanasten reviewed Feb 20, 2020

View reviewed changes

jeromecoutant force-pushed the PR_ETHERNET branch 2 times, most recently from fee61d3 to 8ea4422 Compare February 20, 2020 10:17

JojoS62 reviewed Feb 20, 2020

View reviewed changes

jeromecoutant mentioned this pull request Feb 20, 2020

EMACInterface::connect() always succeeds with an incorrect PHY address #12416

Closed

0xc0170 previously approved these changes Feb 24, 2020

View reviewed changes

mergify bot added needs: CI needs: work and removed needs: review needs: CI labels Feb 24, 2020

jeromecoutant added 2 commits March 2, 2020 16:19

STM32 EMAC : more configurable

1b40076

- PHY default configuration can be changed - AutoNegotiation - Speed - DuplexMode - PHY register offset can be updated depending on chosen PHY All unused parameters are cleaned.

STM32 EMAC : check driver function status before returning success

c3653c6

jeromecoutant added 4 commits March 2, 2020 16:20

STM32 EMAC : check PHY_BSR value before connect status

89a537b

STM32 EMAC : enable mbed_trace

c3c0928

STM32 EMAC : thread size is configurable

01a186a

STM32 EMAC : add PHY ID information

b15dffa

jeromecoutant force-pushed the PR_ETHERNET branch from 02f3698 to b15dffa Compare March 2, 2020 16:18

0xc0170 added needs: CI and removed needs: work labels Mar 2, 2020

0xc0170 added needs: review and removed needs: CI labels Mar 3, 2020

0xc0170 approved these changes Mar 3, 2020

View reviewed changes

jeromecoutant mentioned this pull request Mar 3, 2020

mbed-trace enable makes thread overflow in GCC #12561

Closed

bulislaw approved these changes Mar 3, 2020

View reviewed changes

0xc0170 added release-version: 6.0.0-alpha-3 ready for merge and removed needs: review labels Mar 3, 2020

0xc0170 merged commit b3583f0 into ARMmbed:master Mar 3, 2020

mergify bot removed the ready for merge label Mar 3, 2020

jeromecoutant deleted the PR_ETHERNET branch March 3, 2020 16:07

artokin mentioned this pull request Mar 5, 2020

STM32 EMAC: Ethernet driver initialisation fails in DISCO_F769NI #12577

Closed

jeromecoutant mentioned this pull request Mar 30, 2020

Ethernet on a custom board failing #12709

Closed

JojoS62 mentioned this pull request Apr 2, 2020

add config option for phy media interface #12745

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

STM32 EMAC : add configuration choice and connection check #12464

STM32 EMAC : add configuration choice and connection check #12464

jeromecoutant commented Feb 19, 2020

ciarmcom commented Feb 19, 2020

JojoS62 Feb 19, 2020

JojoS62 Feb 19, 2020 •

edited

Loading

timovanasten Feb 20, 2020

jeromecoutant Feb 20, 2020

timovanasten Feb 20, 2020

JojoS62 commented Feb 20, 2020

JojoS62 Feb 20, 2020

jeromecoutant Feb 20, 2020

JojoS62 Feb 20, 2020

jeromecoutant commented Feb 20, 2020

JojoS62 commented Feb 20, 2020

jeromecoutant commented Feb 20, 2020

JojoS62 commented Feb 20, 2020 •

edited

Loading

jeromecoutant commented Feb 20, 2020

0xc0170 commented Feb 24, 2020

mbed-ci commented Feb 24, 2020

0xc0170 commented Feb 27, 2020

mergify bot commented Feb 28, 2020

jeromecoutant commented Mar 2, 2020

0xc0170 commented Mar 2, 2020

mbed-ci commented Mar 2, 2020

0xc0170 commented Mar 3, 2020

0xc0170 commented Mar 3, 2020

		EthHandle.Init.Speed = ETH_SPEED_100M;
		EthHandle.Init.DuplexMode = ETH_MODE_FULLDUPLEX;

STM32 EMAC : add configuration choice and connection check #12464

STM32 EMAC : add configuration choice and connection check #12464

Conversation

jeromecoutant commented Feb 19, 2020

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Feb 19, 2020

Choose a reason for hiding this comment

JojoS62 Feb 19, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JojoS62 commented Feb 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeromecoutant commented Feb 20, 2020

JojoS62 commented Feb 20, 2020

jeromecoutant commented Feb 20, 2020

JojoS62 commented Feb 20, 2020 • edited Loading

jeromecoutant commented Feb 20, 2020

0xc0170 commented Feb 24, 2020

mbed-ci commented Feb 24, 2020

0xc0170 commented Feb 27, 2020

mergify bot commented Feb 28, 2020

jeromecoutant commented Mar 2, 2020

0xc0170 commented Mar 2, 2020

mbed-ci commented Mar 2, 2020

0xc0170 commented Mar 3, 2020

0xc0170 commented Mar 3, 2020

JojoS62 Feb 19, 2020 •

edited

Loading

JojoS62 commented Feb 20, 2020 •

edited

Loading