Skip to content

Conversation

@panekmaciej
Copy link
Contributor

@panekmaciej panekmaciej commented Aug 30, 2024

  • Fix the issue that eth_adin2111_oa_data_read returning error would cause the offload thread to exit and hence RTOS crash, as the offload thread is an essential one. It looks like this was a result of invalid merge or copy paste, the break statements in lines 705 and 711 were meant for internal loop that keeps reading the RX FIFO until is empty. Added the missing loop. Also refactored the code so it is common with OA and non-OA access and the FIFOs are read sequentially port 1 / port 2.
  • Fix the issue in eth_adin2111_send_oa_frame with improper EBO calculation. Sending more than 1472 byte packet would cause the frame to exceed maximum Ethernet frame size and cause the packet to be dropped. Without this fix the TCP communication did not work unless TCP send window was lowered with CONFIG_NET_TCP_MAX_SEND_WINDOW_SIZE, but that caused poor TCP performance.

@github-actions
Copy link

Hello @panekmaciej, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

@panekmaciej panekmaciej force-pushed the adin2111-fixes branch 3 times, most recently from b618bc7 to b85cbb5 Compare September 3, 2024 10:52
@panekmaciej panekmaciej requested a review from pdgendt September 10, 2024 16:38
Copy link
Member

@MaureenHelm MaureenHelm left a comment

Choose a reason for hiding this comment

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

@jpmur can you take a look?

MaureenHelm
MaureenHelm previously approved these changes Sep 23, 2024
@MaureenHelm MaureenHelm added this to the v4.0.0 milestone Oct 21, 2024
@MaureenHelm
Copy link
Member

Needs rebase

if (i == chunks) {
hdr |= ADIN2111_OA_DATA_HDR_EV;
hdr |= (ctx->oa_cps - 1) << ADIN2111_OA_DATA_HDR_EBO;
hdr |= ebo << ADIN2111_OA_DATA_HDR_EBO;
Copy link
Contributor

Choose a reason for hiding this comment

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

When at the last chunk, you should consider adding ebo to cur instead of cur += ctx->oa_cps; (line 397). This would ensure that the frame length passed to the SPI layer is exactly correct, although I don't think this is a functional issue.

@mmahadevan108 mmahadevan108 modified the milestones: v4.0.0, 4.1 Nov 8, 2024
@nashif nashif modified the milestones: 4.1, v4.1.0 Nov 8, 2024
@kartben
Copy link
Contributor

kartben commented Dec 12, 2024

@panekmaciej will you be able to come back to this to address the merge conflicts?

@panekmaciej
Copy link
Contributor Author

@panekmaciej will you be able to come back to this to address the merge conflicts?

Yes, but after the new year.

@maass-hamburg
Copy link
Member

@panekmaciej please also create a issue for this, so this can also be backported

@fabiobaltieri fabiobaltieri merged commit d611e28 into zephyrproject-rtos:main Feb 19, 2025
136 of 193 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: Ethernet bug The issue is a bug, or the PR is fixing a bug platform: ADI Analog Devices, Inc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants