Hi, Ferruh > -----Original Message----- > From: Yigit, Ferruh <ferruh.yi...@intel.com> > Sent: Thursday, June 18, 2020 7:12 PM > To: Zhao1, Wei <wei.zh...@intel.com>; i.dyu...@samsung.com; > dev@dpdk.org; v.kurams...@samsung.com; tho...@monjalon.net; > david.march...@redhat.com; arybche...@solarflare.com; Guo, Jia > <jia....@intel.com>; Xing, Beilei <beilei.x...@intel.com>; Yang, Qiming > <qiming.y...@intel.com>; Lu, Wenzhuo <wenzhuo...@intel.com> > Subject: Re: [PATCH v3 5/7] net/ixgbe: return unknown speed in status > > On 6/18/2020 2:23 AM, Zhao1, Wei wrote: > > Hi, ferruh > > > >> -----Original Message----- > >> From: Yigit, Ferruh <ferruh.yi...@intel.com> > >> Sent: Thursday, June 18, 2020 12:51 AM > >> To: i.dyu...@samsung.com; dev@dpdk.org; v.kurams...@samsung.com; > >> tho...@monjalon.net; david.march...@redhat.com; > >> arybche...@solarflare.com; Zhao1, Wei <wei.zh...@intel.com>; Guo, Jia > >> <jia....@intel.com>; Xing, Beilei <beilei.x...@intel.com>; Yang, > >> Qiming <qiming.y...@intel.com>; Lu, Wenzhuo <wenzhuo...@intel.com> > >> Subject: Re: [PATCH v3 5/7] net/ixgbe: return unknown speed in status > >> > >> On 6/15/2020 10:01 AM, Ivan Dyukov wrote: > >>> rte_ethdev has declared new NUM_UNKNOWN speed which could be used > in > >>> case when no speed information is available > >>> > >>> Signed-off-by: Ivan Dyukov <i.dyu...@samsung.com> > >>> --- > >>> drivers/net/ixgbe/ixgbe_ethdev.c | 6 +----- > >>> 1 file changed, 1 insertion(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > >>> b/drivers/net/ixgbe/ixgbe_ethdev.c > >>> index a4e5c539d..5b9b13058 100644 > >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c > >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > >>> @@ -4311,11 +4311,7 @@ ixgbe_dev_link_update_share(struct > >> rte_eth_dev *dev, > >>> switch (link_speed) { > >>> default: > >>> case IXGBE_LINK_SPEED_UNKNOWN: > >>> -if (hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T || > >>> -hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T_L) > >>> -link.link_speed = ETH_SPEED_NUM_10M; -else -link.link_speed = > >>> ETH_SPEED_NUM_100M; > > > > For ixgbe x553(IXGBE_DEV_ID_X550EM_A_1G_T), we must do some > adaption, we can not delete these specific code for the kind of ixgbe nic. > > Hi Wei, > > These checks are done when 'link_speed' is 'IXGBE_LINK_SPEED_UNKNOWN'. > > I assume we are setting some default values based on device type when link > speed is unknown. Using new 'ETH_SPEED_NUM_UNKNOWN' type when link > speed is unknown can be more accurate. > > For 'IXGBE_DEV_ID_X550EM_A_1G_T', is link speed > 'IXGBE_LINK_SPEED_UNKNOWN' > explicitly means 'ETH_SPEED_NUM_10M'? > If so why it doesn't return 'IXGBE_LINK_SPEED_10_FULL' instead?
After do a double check, it seems base code ixgbe_check_mac_link_generic() has do the adaption, if (hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T || hw->device_id == IXGBE_DEV_ID_X550EM_A_1G_T_L) *speed = IXGBE_LINK_SPEED_10_FULL; so we only need do some upodate in ixgbe_dev_link_update_share(), add the case IXGBE_LINK_SPEED_10_FULL. So, this patch set is ok now, I think. > > > > > > > >>> +link.link_speed = ETH_SPEED_NUM_UNKNOWN; > >>> break; > > > > > > > >>> > >>> case IXGBE_LINK_SPEED_100_FULL: > >>> > >> > >> looks good to me.