Re: PHY reset question

From: Marco Felsch
Date: Wed Oct 07 2020 - 06:48:05 EST


On 20-10-07 11:20, Marek Vasut wrote:
> On 10/7/20 11:06 AM, Marco Felsch wrote:

...

> > You're right, just wanted to provide you a link :)
>
> Can you CC me on the next version of those patches ? I seems the LAN8710
> is causing grief to many.

No need to since this serie was already applied.

> >> but isn't the
> >> last patch 5/5 breaking existing setups ?
> >
> > IMHO the solution proposed using the PHY_RST_AFTER_CLK_EN was wrong so
> > we needed to fix that. Yes we need to take care of DT backward
> > compatibility but we still must be able to fix wrong behaviours within
> > the driver. I could also argue that PHY_RST_AFTER_CLK_EN solution was
> > breaking exisitng setups too.
> >
> >> The LAN8710 surely does need
> >> clock enabled before the reset line is toggled.
> >
> > Yep and therefore you can specify it yet within the DT.
>
> So the idea is that the PHY enables the clock for itself . And if the
> MAC doesn't export these clock as clk to which you can refer to in DT,
> then you still need the PHY_RST_AFTER_CLK_EN flag, so the MAC can deal
> with enabling the clock ? Or is the idea to fix the MAC drivers too ?

First we need to consider that the PHY_RST_AFTER_CLK_EN flag gets only
handled by the imx-fec driver currently. This particular MAC driver
don't provide clks instead it is just a clock consumer. The clock is
coming from the clock driver and the clk-id is IMX6QDL_CLK_ENET_REF. If
the imx host is the clock provider for smsc-phy you need to specify that
clock-id. There are other phy-drivers using the same mechanism currently.
But yes, you need at least one clock provider. My solution is not
complete as Florian proposal [1] since it rely on the fact that the MAC
enables all clocks before probing the mdio bus. Luckily the imx-fec
driver has this behaviour and my patches are valid till Florian's
patches are merged. Resetting the phy should only be done within the phy
state machine and not from outside without respecting the phy state
since this can cause undefined behaviours.

Florian did you send a new version of those patches?

[1] https://www.spinics.net/lists/netdev/msg680412.html