Hi Laurent, thanks for your patches :) Can you check your setup since we get 6 individual emails: 'git send-email --thread ...' ;) On 21-01-18 16:57, Badel, Laurent wrote: > Description: > External PHY reset from the FEC driver was introduced in commit [1] to > mitigate an issue with iMX SoCs and LAN87xx PHYs. The issue occurs > because the FEC driver turns off the reference clock for power saving > reasons [2], which doesn't work out well with LAN87xx PHYs which require > a running REF_CLK during the power-up sequence. Not only during the power-up sequence. The complete phy internal state machine (the hardware state machine) gets confused if the clock is turned off randomly. > As a result, the PHYs > occasionally (and unpredictably) fail to establish a stable link and > require a hardware reset to work reliably. > > As previously noted [3], the solution in [1] integrates poorly with the > PHY abstraction layer, and it also performs many unnecessary resets. This > patch series suggests a simpler solution to this problem, namely to hold > the PHY in reset during the time between the PHY driver probe and the first > opening of the FEC driver. Holding the Phy within reset during the FEC is in reset seems wrong to me because: The clock can be supplied by an external crystal/oszi. This would add unnecessary delays. Also this is again a FEC/SMSC combination fix again. The phy has the same problem on other hosts if they are the clock provider and toggling the ref-clk. > To illustrate why this is sufficient, below is a representation of the PHY > RST and REF_CLK status at relevant time points (note that RST signal is > active-low for LAN87xx): > > 1. During system boot when the PHY is probed: > RST 111111111111111111111000001111111111111 > CLK 000011111111111111111111111111111000000 > REF_CLK is enabled during fec_probe(), and there is a short reset pulse > due to mdiobus_register_gpiod() which calls gpiod_get_optional() with There is also a deprecated "phy-reset-gpios" did you test this as well? > the GPIOD_OUT_LOW flag, which sets the initial value to 0. The reset is > deasserted by phy_device_register() shortly after. After that, the PHY > runs without clock until the FEC is opened, which causes the unstable > link issue. Nope that's not true, you can specify the clock within the device-tree so the fec-ref-clk isn't disabled anymore. > 2. At first opening of the FEC: ... > Extensive testing with LAN8720 confirmed that the REF_CLK can be disabled > without problems as long as the PHY is either in reset or in power-down > mode (which is relevant for suspend-to-ram as well). You can't disable the clock. What you listing here means that the smsc phy needs to be re-initialized after such an clock loss. If we can disbale the clock randomly we wouldn't need to re-initialize the phy again. Regards, Marco
