> -----Original Message----- > From: Edwin Peer <edwin.p...@broadcom.com> > Sent: Tuesday, January 26, 2021 7:14 PM > To: Danielle Ratson <daniel...@nvidia.com> > Cc: netdev <netdev@vger.kernel.org>; David S . Miller <da...@davemloft.net>; > Jakub Kicinski <k...@kernel.org>; Jiri Pirko > <j...@nvidia.com>; Andrew Lunn <and...@lunn.ch>; f.faine...@gmail.com; Michal > Kubecek <mkube...@suse.cz>; mlxsw > <ml...@nvidia.com>; Ido Schimmel <ido...@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 <daniel...@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
