2016-10-26 21:28, Harish Patil: > > >2016-10-18 21:11, Rasesh Mody: > >> From: Harish Patil <harish.patil at qlogic.com> > >> > >> This patch fixes the missing 100G link speed advertisement > >> when the 100G support was initially added. > >> > >> Fixes: 2af14ca79c0a ("net/qede: support 100G") > >> > >> Signed-off-by: Harish Patil <harish.patil at qlogic.com> > >[...] > >> [Features] > >> +Speed capabilities = Y > > > >This feature should be checked only when it is fully implemented, > >i.e. when you return the real capabilities of the device. > > > >> --- a/drivers/net/qede/qede_ethdev.c > >> +++ b/drivers/net/qede/qede_ethdev.c > >> @@ -599,7 +599,8 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev, > >> DEV_TX_OFFLOAD_UDP_CKSUM | > >> DEV_TX_OFFLOAD_TCP_CKSUM); > >> > >> - dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G; > >> + dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G | > >> + ETH_LINK_SPEED_100G; > >> } > > > >It is only faking the capabilities at driver-level. > >You should check if the underlying device is able to achieve 100G > >before advertising this flag to the application. > > > >I suggest to update this patch to remove the doc update. > >The contract is to fill it only when the code is fixed. > >By the way, we must call every other drivers to properly implement > >this feature. > > > > Hi Thomas, > Its not really a faking. The same driver supports all three link speeds. > The required support for 100G was already present in the 16.07 inbox > driver. > We just had missed out advertising 100G link speed via > dev_info->speed_capa. > Hence it is - Fixes: 2af14ca79c0a ("net/qede: support 100G?). > Hope it is okay.
Yes the code is okay. But it is not complete. Please tell me you understand that you must fill speed_capa depending of the device capabilities. Then you will be able to fill "Speed capabilities" box in the doc.