Messages in this thread | | | Date | Thu, 17 Nov 2022 12:40:21 +0800 | Subject | Re: [PATCH 2/2] net: fec: Create device link between phy dev and mac dev | From | "Wang, Xiaolei" <> |
| |
On 11/17/2022 8:21 AM, Florian Fainelli wrote: > CAUTION: This email comes from a non Wind River email account! > Do not click links or open attachments unless you recognize the sender > and know the content is safe. > > On 11/16/22 15:57, Andrew Lunn wrote: >> On Wed, Nov 16, 2022 at 03:27:39PM -0800, Florian Fainelli wrote: >>> On 11/16/22 07:07, Andrew Lunn wrote: >>>> On Wed, Nov 16, 2022 at 10:43:05PM +0800, Xiaolei Wang wrote: >>>>> On imx6sx, there are two fec interfaces, but the external >>>>> phys can only be configured by fec0 mii_bus. That means >>>>> the fec1 can't work independently, it only work when the >>>>> fec0 is active. It is alright in the normal boot since the >>>>> fec0 will be probed first. But then the fec0 maybe moved >>>>> behind of fec1 in the dpm_list due to various device link. >>> >>> Humm, but if FEC1 depends upon its PHY to be available by the FEC0 >>> MDIO bus >>> provider, then surely we will need to make sure that FEC0's MDIO bus is >>> always functional, and that includes surviving re-ordering as well >>> as any >>> sort of run-time power management that can occur. >> >> Runtime PM is solved for the FECs MDIO bus, because there are switches >> hanging off it, which have their own life cycle independent of the >> MAC. This is something i had to fix many moons ago, when the FEC would >> power off the MDIO bus when the interface is admin down, stopping >> access to the switch. The mdio read and write functions now do run >> time pm get and put as needed. >> >> I've never done suspend/resume with a switch, it is not something >> needed in the use cases i've covered. > > All of the systems with integrated I worked on had to support > suspend/resume both with HW maintaining the state, and with HW losing > the state because of being powered off. The whole thing is IMHO still > not quite well supported upstream if you have applied some configuration > more complicated than a bridge or standalone ports. Anyway, this is a > topic for another 10 years to come :) > >> >>>>> So in system suspend and resume, we would get the following >>>>> warning when configuring the external phy of fec1 via the >>>>> fec0 mii_bus due to the inactive of fec0. In order to fix >>>>> this issue, we create a device link between phy dev and fec0. >>>>> This will make sure that fec0 is always active when fec1 >>>>> is in active mode. >>> >>> Still not clear to me how the proposed fix works, let alone how it >>> does not >>> leak device links since there is no device_link_del(), also you are >>> going to >>> be creating guaranteed regressions by putting that change in the PHY >>> library. >> >> The reference leak is bad, but i think phylib is the correct place to >> fix this general issue. It is not specific to the FEC. There are other >> boards with dual MAC SoCs and they save a couple of pins by putting >> both PHYs on one MDIO bus. Having the link should help better >> represent the device tree so that suspend/resume can do stuff in the >> right order. > > My concern is that we already have had a hard time solving the proper > suspend/resume sequence whether the MAC suspends the PHY or the MDIO bus > suspends the PHY and throwing device links will either change the > ordering in subtle ways, or hopefully just provide the same piece of > information we already have via mac_managed_pm. > > It seems like in Xiaolei's case, the MDIO bus should suspend the PHY and > that ought to take care of all dependencies, one would think.
Hi
mac_managed_pm solves the soft reset triggered during aeg. If you modify it back to MDIO bus to suspend phy, you still need to solve the problem of auto-negotiation,
thanks
xiaolei
> -- > Florian >
| |