> -----Original Message-----
> From: Michal Kubecek <mkube...@suse.cz>
> Sent: Thursday, October 22, 2020 7:28 PM
> To: Danielle Ratson <daniel...@nvidia.com>
> Cc: Jiri Pirko <j...@resnulli.us>; Andrew Lunn <and...@lunn.ch>; Jakub
> Kicinski <k...@kernel.org>; Ido Schimmel
> <ido...@idosch.org>; netdev@vger.kernel.org; da...@davemloft.net; Jiri Pirko
> <j...@nvidia.com>; f.faine...@gmail.com; mlxsw
> <ml...@nvidia.com>; Ido Schimmel <ido...@nvidia.com>;
> johan...@sipsolutions.net
> Subject: Re: [PATCH net-next 1/6] ethtool: Extend link modes settings uAPI
> with lanes
>
> On Thu, Oct 22, 2020 at 06:15:48AM +0000, Danielle Ratson wrote:
> > > -----Original Message-----
> > > From: Michal Kubecek <mkube...@suse.cz>
> > > Sent: Wednesday, October 21, 2020 11:48 AM
> > >
> > > Ah, right, it does. But as you extend struct ethtool_link_ksettings
> > > and drivers will need to be updated to provide this information,
> > > wouldn't it be more useful to let the driver provide link mode in
> > > use instead (and derive number of lanes from it)?
> >
> > This is the way it is done with the speed parameter, so I have aligned
> > it to it. Why the lanes should be done differently comparing to the
> > speed?
>
> Speed and duplex have worked this way since ages and the interface was
> probably introduced back in times when combination of
> speed and duplex was sufficient to identify the link mode. This is no longer
> the case and even adding number of lanes wouldn't make
> the combination unique. So if we are going to extend the interface now and
> update drivers to provide extra information, I believe it
> would be more useful to provide full information.
>
> Michal
Hi Michal,
What do you think of passing the link modes you have suggested as a bitmask,
similar to "supported", that contains only one positive bit?
Something like that:
diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index afae2beacbc3..dd946c88daa3 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -127,6 +127,7 @@ struct ethtool_link_ksettings {
__ETHTOOL_DECLARE_LINK_MODE_MASK(supported);
__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising);
__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising);
+ __ETHTOOL_DECLARE_LINK_MODE_MASK(chosen);
} link_modes;
u32 lanes;
};
Do you have perhaps a better suggestion?
And the speed and duplex parameters should be removed from being passed like as
well, right?
Thanks,
Danielle
