Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

arch/risc-v/mpfs ethernet improvements #15979

Merged
merged 5 commits into from
Mar 24, 2025

Conversation

jlaitine
Copy link
Contributor

@jlaitine jlaitine commented Mar 12, 2025

Summary

This updates the mpfs_ethernet driver by picking the following corrections from TII/SSRC branches:

  • Always use LPWORK queue for txtimeout handling. Txtimeout does lengthy operations such as initializing/polling PHY, so it is never good to put that into HPWORK
  • Change the default workqueue for ethernet driver to LPWORK. Add a Kconfig variable to put it into HPWORK if needed
  • Add a configurable recovery workaround for old LAN8742A cable diagnostics errata
  • Optimize ifup, which did phyinit twice for no reason
  • Set the GMAC_RX_UNITSIZE to the maximum frame length, this can't be configured smaller.

Impact

Impacts only MPFS ethernet, improves reliability.

user changes: NO
build process changes: NO
documentation updates: NO
security implications: NO

Testing

Build Host: Ubuntu 22.04
Target: icicle:hwtest & 3 custom HW designs.

jlaitine and others added 4 commits March 12, 2025 11:31
- Use LPWORK by default if CONFIG_MPFS_ETHMAC_HPWORK is not defined
- Always use LPWORK for timeouts; this makes very lengthy operations such as re-initializing PHY.

Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
…roken PHYs

If the interface is UP, and no packets are received in 30s, re-initialize the interface by calling the
already implemented mpfs_txtimeout_expiry.

This is a workaround for a bug where IF might be UP and working but packets can only
be transmitted. Receive side just doesn't work at all. The bug manifests at least in
older LAN8742A (rev A and B), for which also a silicon errata exists.

The original issue can be re-produced easily by disconnecting and reconnecting the ethernet cable while
the IF is up.

Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
…UG_NET

- Fix compilation failure "error: 'mpfs_phydump' defined but not used [-Werror=unused-function]"
- Add debug dump of phy registers.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
mpfs_phyinit() was called twice during ifup().

Signed-off-by: Jani Paalijarvi <jani.paalijarvi@unikie.com>
@github-actions github-actions bot added Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: M The size of the change in this PR is medium labels Mar 12, 2025
@nuttxpr
Copy link

nuttxpr commented Mar 12, 2025

[Experimental Bot, please feedback here]

Yes, this PR appears to meet the basic NuttX requirements, but could be improved. While it provides a summary of the changes and testing information, it lacks detail in the Impact section. Specifically, it needs to address most of the "NO/YES" questions with more than a single sentence. For example, even though the impact is stated as "only MPFS ethernet," the PR should explicitly state NO for impacts like user changes, build process changes, documentation updates, security implications, etc. If any of these are YES, then a detailed explanation is required. The testing section also needs to include actual logs, not just a description of the test environment.

@jerpelea jerpelea changed the title Mpfs ethernet improvements arch/risc-v/mpfs ethernet improvements Mar 13, 2025
Copy link
Contributor

@jerpelea jerpelea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for 64d2953 please change the title to

arch/risc-v/mpfd: set GMAC_RX_UNITSIZE to max gmac frame len

@jlaitine jlaitine force-pushed the mpfs_ethernet_improvements branch from 64d2953 to 0c06e37 Compare March 14, 2025 06:13
@jlaitine
Copy link
Contributor Author

I believe I have addressed all the review comments, is this good to go in?

@lupyuen
Copy link
Member

lupyuen commented Mar 24, 2025

Sorry @jlaitine could you sign-off this commit with git commit -s? Thanks :-)

Verify that GMAC RX/TX buffers are 64 byte aligned

Signed-off-by: Jukka Laitinen <jukka.laitinen@tii.ae>
@jlaitine jlaitine force-pushed the mpfs_ethernet_improvements branch from 0c06e37 to 7985c55 Compare March 24, 2025 11:23
@xiaoxiang781216 xiaoxiang781216 merged commit 930c4b0 into apache:master Mar 24, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Arch: risc-v Issues related to the RISC-V (32-bit or 64-bit) architecture Size: M The size of the change in this PR is medium
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants