Re: [PATCH 2/2] dsa: mv88e6131: support fixed PHYs

2015-02-21 2:30 GMT-08:00 Tobias Waldekranz [off-list ref]:

On Thu, Feb 12, 2015 at 08:13:28AM -0800, Florian Fainelli wrote:

quoted

2015-02-12 6:13 GMT-08:00 Tobias Waldekranz [off-list ref]:

quoted

Statically setup the PCS Control on the MAC to match the fixed PHY.

bcm_sf2 supports both fixed PHYs and regular PHYs, yet we do not need
to get access to the fixed PHY status from the adjust_link callback
because you could implement a separate fixed_link_update callback for
that purpose.

Did not that work for you?

That was my first approach and it worked fine. The only issue I saw
was that the callback was continously called at each poll cycle even
though the link state had not changed.

Right, we poll using this callback fairly often. Just like the PHY
libraries adjust_link, drivers are responsible for implementing
"caching" and avoiding the callback to be invoked too frequently.

So then I implemented the same check for updates that was in the
regular adjust_link callback. But before I submitted that version of
the patch I looked att the sf2 code, and it seemed as though this code
uses the callback to update the phy status based on the chip state and
not the other way around. Did I misunderstand the code?

It is a two step process:

- fixed_link_update updates the fixed PHY's status member to reflect
the HW changes (link change mostly), updates the PHY device
speed/duplex/pause parameters

- adjust_link reads the PHY device speed/duplex/pause and applies
these to the HW registers

Not wanting to break your code, I went with this approach instead. But
if you're fine with it, I'm more than happy to go with that version.

I think it is a little cleaner since the adjust_link callback does not
need to know what kind of PHY device it is dealing with, while the
fixed_link_update() one only takes care of fixed PHYs.

Thanks
-- 
Florian