stm32-rs / stm32-eth

Embedded Ethernet driver in Rust
Apache License 2.0
147 stars 47 forks source link

Support for any other mode than 100 Mbps/FD #24

Closed astro closed 1 year ago

astro commented 4 years ago

This driver assumes that PHY autonegotiation always results in 100 MBps/FD.

We should poll the PHY at the right moments to set eth_mac maccr fes and dm accordingly. When?

adamgreig commented 4 years ago

Usually I'd have a state machine where you start out disconnected, enable the MDIO interface and use it to reset the PHY, then wait for link negotiation (or use a hard-coded configuration), then once the link becomes up you can configure and enable the rest of the MAC and the system is active. In that case, once the PHY reports autonegotiation is complete, you can configure the MAC with the reported link properties.

However, I don't think stm32-eth really provides such a concept; you initialise it once (which starts the MAC at 100M FD) and from then on it's running, irrespective of the link status. That's definitely simpler to get running and requires less work from the application code, so it might not be the end of the world to leave that as the default state of affairs.

Perhaps the simplest thing for now is to add the methods to configure the MAC for whatever operation mode is requested, and then add a helper method that takes a PHY status struct and configures the MAC according to the PHY's reported mode? That way users could init as currently (perhaps new could take an operation mode instead of assuming 100M FD, but perhaps it doesn't really matter); then the user can poll for link established themselves and call a 'reconfigure' method once the link is up?

astro commented 4 years ago

It would be great if the driver would support replugging the Ethernet interface (with different link speed).

If we don't want to introduce new API, we could poll the link status and reconfigure whenever recv_next() has no new packet. Admittedly, that would introduce unexpected functionality in the RX path. It may even turn out to be a performance problem.

I guess that leaves us at new API functions.

adamgreig commented 4 years ago

We could add a new 'poll_link' method which checks if the link is up, configures the MAC to match the PHY's status, and returns the current link status: down/10/100/hd/fd/manual/auto. Then we suggest the user poll it on a regular but infrequent interval (say every 100ms or whatever). Then the user could either never poll (device only works in 100FD), poll at the same time they poll for received packets (causes extra overhead checking the PHY over MDIO, but not catastrophic), or ideally polls infrequently in a timer or in a main loop or whatever. The returned status is useful for the user application too.

I think adding it to recv_next() isn't great, because reading a couple of registers from the PHY over MDIO takes quite a bit of time.

datdenkikniet commented 1 year ago

Basic support added in #53. While not optimal (as it requires the user to do some manual intervention and we're not taking care of it super gracefully), it's better than not being able to configure it at all :)