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

- From: Florian Fainelli \<f.fainelli@gmail.com\>
- Date: 2015-02-21 18:57:07 
- List: netdev
- Page: https://ratatoskr.run/netdev/2015/02/10978497
- Thread: https://ratatoskr.run/netdev/2015/02/10977993/t.md

```text
2015-02-21 2:30 GMT-08:00 Tobias Waldekranz [ref]:
> On Thu, Feb 12, 2015 at 08:13:28AM -0800, Florian Fainelli wrote:
>> 2015-02-12 6:13 GMT-08:00 Tobias Waldekranz [ref]:
>> > 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
```