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.

Reply via email to