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

cpu/sam0_eth: interrupt based link detection/auto-negotiation #19703

Merged
merged 5 commits into from
Jun 14, 2023

Conversation

benpicco
Copy link
Contributor

@benpicco benpicco commented Jun 2, 2023

Contribution description

The PHY interrupt line is connected to the MCU, so we can just generate a GPIO interrupt when a PHY event orrurs.

This allows us to have LINK_UP/LINK_DOWN events which are needed to start / stop sending router solicitations.

We can also use the result of the auto-negotiation process to configure the MAC accordingly.

There is a bit of a hack inside: When we have an auto-negotiation capable link partner, but auto-negotiation fails after a timeout, we just re-start the auto-negotiation process.
This is not needed on same54-xpro where auto-negotiation always works on the first try, but with our custom board, this is not always the case - it's still unclear as to why, but the link is stable afterwards and we don't have any packet loss.
So Having this added robustness is IMHO fine.

Testing procedure

ifconfig should now show the real link status.
When you start the board with the Ethernet cable unplugged, router solicitations will only start once you plug in the cable.

Issues/PRs references

similar to #14688

@benpicco benpicco requested review from dylad and keestux as code owners June 2, 2023 15:04
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Jun 2, 2023
@benpicco benpicco changed the title cpu/sam0_eth: monitor link status by polling PHY @benpicco cpu/sam0_eth: monitor link status by polling PHY Jun 2, 2023
@benpicco benpicco requested a review from maribu June 2, 2023 15:09
cpu/sam0_common/sam0_eth/eth-netdev.c Outdated Show resolved Hide resolved
@benpicco benpicco added the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 2, 2023
@benpicco
Copy link
Contributor Author

benpicco commented Jun 2, 2023

As @dylad pointed out we actually do have a PHY IRQ - it's just a normal GPIO (sam_gmac_config[0].int_pin).

I'll re-write the code to use that and also do proper auto-negotiation as in 5f9b55a

@dylad
Copy link
Member

dylad commented Jun 4, 2023

Is it still WIP ?

@maribu
Copy link
Member

maribu commented Jun 4, 2023

It is still using the timer rather then the GPIO IRQ

@benpicco
Copy link
Contributor Author

benpicco commented Jun 4, 2023

It uses the IRQ, the timer is just a fallback for when it doesn’t work on the first try - not sure why our board is so ' special' there.

Still WIP because I don’t clean up the code/commits yet.

@benpicco benpicco force-pushed the cpu/sam0_eth-link branch from 3a97148 to e0d955b Compare June 5, 2023 10:01
@benpicco benpicco removed the State: WIP State: The PR is still work-in-progress and its code is not in its final presentable form yet label Jun 5, 2023
@benpicco benpicco changed the title cpu/sam0_eth: monitor link status by polling PHY cpu/sam0_eth: interrupt based link detection/auto-negotiation Jun 5, 2023
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 5, 2023
@riot-ci
Copy link

riot-ci commented Jun 5, 2023

Murdock results

✔️ PASSED

2a255ff cpu/sam0_eth: interrupt based link detection/auto-negotiation

Success Failures Total Runtime
6934 0 6934 10m:39s

Artifacts

@benpicco benpicco force-pushed the cpu/sam0_eth-link branch from e0d955b to 63d0695 Compare June 5, 2023 10:46
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Feels free to squash directly.

cpu/sam0_common/sam0_eth/eth-netdev.c Outdated Show resolved Hide resolved
cpu/sam0_common/sam0_eth/eth-netdev.c Show resolved Hide resolved
@benpicco benpicco force-pushed the cpu/sam0_eth-link branch from 3bd9457 to 9d40bd6 Compare June 12, 2023 09:40
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

ACK

@dylad
Copy link
Member

dylad commented Jun 12, 2023

bors merge

bors bot added a commit that referenced this pull request Jun 12, 2023
19703: cpu/sam0_eth: interrupt based link detection/auto-negotiation r=dylad a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
@bors
Copy link
Contributor

bors bot commented Jun 12, 2023

Build failed:

@maribu
Copy link
Member

maribu commented Jun 13, 2023

The usual KConfig forgotten issue 😉

@benpicco benpicco force-pushed the cpu/sam0_eth-link branch from 9d40bd6 to 508d64a Compare June 14, 2023 11:24
@github-actions github-actions bot added the Area: Kconfig Area: Kconfig integration label Jun 14, 2023
@benpicco
Copy link
Contributor Author

bors merge

bors bot added a commit that referenced this pull request Jun 14, 2023
19703: cpu/sam0_eth: interrupt based link detection/auto-negotiation r=benpicco a=benpicco



19724: dist/tools/openocd: add OPENOCD_SERVER_ADDRESS variable r=benpicco a=fabian18



19735: nrf5x_common: Clear I2C periph shorts r=benpicco a=bergzand

### Contribution description

The I2C peripheral's shortcuts are used with the read and write register to automatically stop the I2C transaction or to continue with the next stage.

With simple I2C read and write bytes these shorts are not used, but are also not cleared by the function in all cases, causing it to use the shortcut configuration set by a previous function call. This patch ensures that the shorts are always set by the read and write functions

### Testing procedure

Should be possible to spot with a logic analyzer and the I2C periph test. Maybe the HIL test can also detect it :)

### Issues/PRs references

None

Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Co-authored-by: Fabian Hüßler <fabian.huessler@ml-pa.com>
Co-authored-by: Koen Zandberg <koen@bergzand.net>
@bors
Copy link
Contributor

bors bot commented Jun 14, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Jun 14, 2023
19703: cpu/sam0_eth: interrupt based link detection/auto-negotiation r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <benjamin.valentin@ml-pa.com>
@benpicco benpicco force-pushed the cpu/sam0_eth-link branch from 508d64a to 2a255ff Compare June 14, 2023 12:22
@bors
Copy link
Contributor

bors bot commented Jun 14, 2023

Canceled.

@benpicco
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 14, 2023

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.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 829af7c into RIOT-OS:master Jun 14, 2023
@benpicco benpicco deleted the cpu/sam0_eth-link branch June 14, 2023 15:42
@benpicco benpicco added this to the Release 2023.07 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants