On Tue, 2019-09-10 at 10:00 +0200, Michal Kubecek wrote: > [External] > > On Mon, Sep 09, 2019 at 04:12:50PM +0300, Alexandru Ardelean wrote: > > The `phy_tunable_id` has been named `ETHTOOL_PHY_EDPD` since it looks like > > this feature is common across other PHYs (like EEE), and defining > > `ETHTOOL_PHY_ENERGY_DETECT_POWER_DOWN` seems too long. > > > > The way EDPD works, is that the RX block is put to a lower power mode, > > except for link-pulse detection circuits. The TX block is also put to low > > power mode, but the PHY wakes-up periodically to send link pulses, to avoid > > lock-ups in case the other side is also in EDPD mode. > > > > Currently, there are 2 PHY drivers that look like they could use this new > > PHY tunable feature: the `adin` && `micrel` PHYs. > > > > The ADIN's datasheet mentions that TX pulses are at intervals of 1 second > > default each, and they can be disabled. For the Micrel KSZ9031 PHY, the > > datasheet does not mention whether they can be disabled, but mentions that > > they can modified. > > > > The way this change is structured, is similar to the PHY tunable downshift > > control: > > * a `ETHTOOL_PHY_EDPD_DFLT_TX_INTERVAL` value is exposed to cover a default > > TX interval; some PHYs could specify a certain value that makes sense > > * `ETHTOOL_PHY_EDPD_NO_TX` would disable TX when EDPD is enabled > > * `ETHTOOL_PHY_EDPD_DISABLE` will disable EDPD > > > > This should allow PHYs to: > > * enable EDPD and not enable TX pulses (interval would be 0) > > * enable EDPD and configure TX pulse interval; note that TX interval units > > would be PHY specific; we could consider `seconds` as units, but it could > > happen that some PHYs would be prefer milliseconds as a unit; > > a maximum of 65533 units should be sufficient > > Sorry for missing the discussion on previous version but I don't really > like the idea of leaving the choice of units to PHY. Both for manual > setting and system configuration, it would be IMHO much more convenient > to have the interpretation universal for all NICs. > I was also a bit uncertain/undecided here about letting PHYs decide units. And I also wasn't sure what to propose. Not proposing a unit now, would have allowed us to decide later based on what PHYs implement (generally) in the future. I know that may make me look a bit lazy [for not proposing units in ethtool], but tbh it's more of lack-of-experience with ethtool (or these APIs) evolve over time. > Seconds as units seem too coarse and maximum of ~18 hours way too big. > Milliseconds would be more practical from granularity point of view, > would maximum of ~65 seconds be sufficient? I think 65 seconds is more than enough. I feel that, if you plug-in an eth cable and don't have a link in a minute, then it's not great (no matter how much of a power-saver you are). Actually, voicing out these units makes it simpler for a decision to go in favor for milliseconds. So: thank you :) Alex > > Michal Kubecek > > > * disable EDPD > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@xxxxxxxxxx>
