Re: [RFC/PATCH] net: nixge: Add PHYLINK support

[Moritz Fischer]

Hi Florian,

On Tue, Sep 4, 2018 at 5:27 PM, Florian Fainelli <f.fainelli@xxxxxxxxx> wrote:
> On 09/04/2018 05:15 PM, Moritz Fischer wrote:
>> Add basic PHYLINK support to driver.
>>
>> Suggested-by: Andrew Lunn <andrew@xxxxxxx>
>> Signed-off-by: Moritz Fischer <mdf@xxxxxxxxxx>
>> ---
>>
>> Hi all,
>>
>> as Andrew suggested in order to enable SFP as
>> well as fixed-link support add PHYLINK support.
>>
>> A couple of questions are still open (hence the RFC):
>>
>> 1) It seems odd to implement PHYLINK callbacks that
>>    are all empty? If so, should we have generic empty
>>    ones in drivers/net/phy/phylink.c like we have for
>>    genphys?
>
> Yes it is odd, the validate callback most certainly should not be empty,
> neither should the mac_config and mac_link_{up,down}, but, with some
> luck, you can get things to just work, typically with MDIO PHYs, since a
> large amount of what they can do is discoverable.
>
> If you had an existing adjust_link callback from PHYLIB, it's really
> about breaking it down such that the MAC configuration of
> speed/duplex/pause happens in mac_config, and the link setting (if
> necessary), happens in mac_link_{up,down}, and that's pretty much it for
> MLO_AN_PHY cases.

Let me check, it seems there is a register that indicates whether the MAC can
do either 1G or 10G. I might be able to use that for some of the above, but
there is not really much in terms of writable registers there.
It's like a DMA engine with a bit of MDIO on the side. Let me see if I can make
it look less weird with that. If not I'll go with a comment explaining
that there
isn't much to do for the MLO_AN_PHY case and the MLO_FIXED cases?

Cheers and thanks for your feedback,
Moritz