RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters

From: Danielle Ratson <danieller@nvidia.com>
To: Edwin Peer <edwin.peer@broadcom.com>, Michal Kubecek <mkubecek@suse.cz>
Cc: netdev <netdev@vger.kernel.org>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Jiri Pirko <jiri@nvidia.com>,
	Andrew Lunn <andrew@lunn.ch>,
	"f.fainelli@gmail.com" <f.fainelli@gmail.com>,
	mlxsw <mlxsw@nvidia.com>, Ido Schimmel <idosch@nvidia.com>
Subject: RE: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
Date: Wed, 27 Jan 2021 13:22:02 +0000	[thread overview]
Message-ID: <DM6PR12MB45161FF65D43867C9ED96B6ED8BB9@DM6PR12MB4516.namprd12.prod.outlook.com> (raw)
In-Reply-To: <CAKOOJTyRyz+KTZvQ8XAZ+kehjbTtqeA3qv+r9DJmS-f9eC6qWg@mail.gmail.com>

> -----Original Message-----
> From: Edwin Peer <edwin.peer@broadcom.com>
> Sent: Tuesday, January 26, 2021 7:14 PM
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netdev <netdev@vger.kernel.org>; David S . Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jiri Pirko
> <jiri@nvidia.com>; Andrew Lunn <andrew@lunn.ch>; f.fainelli@gmail.com; Michal Kubecek <mkubecek@suse.cz>; mlxsw
> <mlxsw@nvidia.com>; Ido Schimmel <idosch@nvidia.com>
> Subject: Re: [PATCH net-next v3 2/7] ethtool: Get link mode in use instead of speed and duplex parameters
> 
> On Tue, Jan 26, 2021 at 9:09 AM Danielle Ratson <danieller@nvidia.com> wrote:
> 
> > > I understand the benefit of deriving the dependent fields in core code
> > > rather than in each driver, I just don't think this is necessarily
> > > mutually exclusive with being able to force a particular link mode at
> > > the driver API, making link_mode R/W (and even extend this interface
> > > to user space). For a driver that works internally in terms of the
> > > link_mode it's returning, this would be more natural.
> >
> > I am not sure I fully understood you, but it seems like some expansion that can be
> > done in the future if needed, and doesn't need to hold that patchset back.
> 
> For one thing, it's cleaner if the driver API is symmetric. The
> proposed solution sets attributes in terms of speeds and lanes, etc.,
> but it gets them in terms of a compound link_info. But, this asymmetry
> aside, if link_mode may eventually become R/W at the driver API, as
> you suggest, then it is more appropriate to guard it with a capability
> bit, as has been done for lanes, rather than use the -1 special value
> to indicate that the driver did not set it.
> 
> Regards,
> Edwin Peer

This patchset adds lanes parameter, not link_mode. The link_mode addition was added as a read-only parameter for the reasons we mentioned, and I am not sure that implementing the symmetric side is relevant for this patchset.

Michal, do you think we will use the Write side of the link_mode parameter? And if so, do you think it is relevant for this specific patchset?

Thanks,
Danielle