From mboxrd@z Thu Jan  1 00:00:00 1970
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Subject: Re: Link modes representation in phylib
Date: Fri, 29 Jun 2018 15:26:13 +0200
Message-ID: <20180629152613.43fcba8d@bootlin.com>
References: <20180618170224.321f8264@bootlin.com>
        <20180618154018.GB5865@lunn.ch>
        <20180619113053.11df78a2@bootlin.com>
        <20180619152155.GC26796@lunn.ch>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: davem@davemloft.net,
        Russell King - ARM Linux <linux@armlinux.org.uk>,
        Florian Fainelli <f.fainelli@gmail.com>,
        netdev <netdev@vger.kernel.org>,
        Antoine Tenart <antoine.tenart@bootlin.com>,
        "thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
        Gregory CLEMENT <gregory.clement@bootlin.com>,
        Miquel Raynal <miquel.raynal@bootlin.com>
To: Andrew Lunn <andrew@lunn.ch>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail.bootlin.com ([62.4.15.54]:54795 "EHLO mail.bootlin.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752242AbeF2N0R (ORCPT <rfc822;netdev@vger.kernel.org>);
        Fri, 29 Jun 2018 09:26:17 -0400
In-Reply-To: <20180619152155.GC26796@lunn.ch>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Hello Andrew,

On Tue, 19 Jun 2018 17:21:55 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

>> What I propose is that we add 3 link_mode fields in phy_device, and keep
>> the legacy fields for now. It would be up to the driver to fill the new
>> "supported" field in config_init, kind of like what's done in the
>> marvell10g driver.  
>
>Hi Maxime
>
>You can do this conversion in the core. If features == 0, and some
>bits are set in the features link_mode, do the conversion at probe
>time. The same can be done for lp_advertising, when the call into the
>drivers read_status() has completed.

Thanks for the suggestion. I see how this can be done with
phydrv->supported and phydev->lp_advertising, however I'm not sure how
we should deal with the fact that ethernet drivers directly access
fields such as "advertising" or "supported".

Should we update the new fields in phy_device when functions such as
"phy_start" or "phy_start_aneg" are called, just in case the ethernet
driver modified the phydev->supported / phydev->advertising fields
in the meantime ?

My concern is that phylink will rely on the new link_mode
representation to be up-to-date, and ethernet drivers that aren't
converted to phylink, but link to a new PHY will rely on the legacy
representation.

That might be just fine if we take good care making sure both the
legacy and the new representation are well sync'd, but since I don't
have a good big-picture view of all this, I prefer having your opinion
first :)

>> Would that be acceptable ?  
>
>It sounds reasonable. Lets see what the code looks like.

Ok I'll be glad to send a series for that, I just want to make sure I
go in the right direction

Thanks,

Maxime