> -----Original Message----- > From: Edwin Peer <[email protected]> > Sent: Tuesday, January 26, 2021 7:14 PM > To: Danielle Ratson <[email protected]> > Cc: netdev <[email protected]>; David S . Miller <[email protected]>; > Jakub Kicinski <[email protected]>; Jiri Pirko > <[email protected]>; Andrew Lunn <[email protected]>; [email protected]; Michal > Kubecek <[email protected]>; mlxsw > <[email protected]>; Ido Schimmel <[email protected]> > 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 <[email protected]> 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
