Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <fa4fca9c-5950-8ec1-5746-91e51085cecf@gmail.com>
References: <20180905001535.19168-1-mdf@kernel.org> <fa4fca9c-5950-8ec1-5746-91e51085cecf@gmail.com>
From:   Moritz Fischer <moritz.fischer@ettus.com>
Date:   Tue, 4 Sep 2018 21:05:45 -0700
Message-ID: <CAAtXAHdZxgvb7=DG0B2irB4wJ+taJaOYbxhGjNtjWvaAQP8_nA@mail.gmail.com>
Subject: Re: [RFC/PATCH] net: nixge: Add PHYLINK support
To:     Florian Fainelli <f.fainelli@gmail.com>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Andrew Lunn <andrew@lunn.ch>,
        Alex Williams <alex.williams@ni.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Florian,

On Tue, Sep 4, 2018 at 5:27 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 09/04/2018 05:15 PM, Moritz Fischer wrote:
>> Add basic PHYLINK support to driver.
>>
>> Suggested-by: Andrew Lunn <andrew@lunn.ch>
>> Signed-off-by: Moritz Fischer <mdf@kernel.org>
>> ---
>>
>> 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