drivers/enc28j60: disable flow control#19845
Merged
bors[bot] merged 1 commit intoRIOT-OS:masterfrom Jul 24, 2023
Merged
Conversation
benpicco
added
Type: bug
Member
|
I can confirm this, setting the bit indeed blindly sends pause frames periodically:
It appears that the author of the driver assumed that flow control would only send pause frames when the RX buffer of the ENC28J60 would run out of space. To add proper flow control to the driver, we would have to enable and disable sending pause frames depending on the network load. I agree that having no flow control is much better than having broken flow control. |
maribu
approved these changes
Jul 24, 2023
Member
|
bors merge |
Contributor
|
Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Contribution description
This PR disables flow control in the ENC28J60 driver init function.
Setting FCEN1 in the EFLOCON register causes the ENC28J60 to periodically send pause frames. It should only be set when the receiver is running out of buffer space [1].
(Also, the FULDPXS bit in EFLOCON is read-only.)
[1] https://ww1.microchip.com/downloads/en/devicedoc/39662c.pdf
Testing procedure
Launch Wireshark / tshark on an interface connected to an ENC28J60 controlled by RIOT. Without this PR you should see a pause frame being sent about every 100 ms (~10x per second):
With this PR no pause frames are sent.
Issues/PRs references
None