Re: [dpdk-dev] [PATCH 2/5] net/txgbe: update link setup process of backplane NICs

["Jiawen Wu" <jiawenwu@trustnetic.com>]

On Tuesday, March 9, 2021 10:33 PM, Ferruh Yigit wrote:
> On 3/5/2021 11:23 AM, Jiawen Wu wrote:
> > Use some configuration to control the link setup flow, to adapt to
> > different NIC's construction. Use firmware version to control the
> > impact of firmware update. And fix some left bugs.
> >
> > Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>
> > ---
> >   config/rte_config.h                   |   7 ++
> >   doc/guides/nics/txgbe.rst             |  20 ++++
> >   drivers/net/txgbe/base/txgbe_eeprom.h |   3 +
> >   drivers/net/txgbe/base/txgbe_hw.c     |  85 +---------------
> >   drivers/net/txgbe/base/txgbe_osdep.h  |   1 +
> >   drivers/net/txgbe/base/txgbe_phy.c    | 137 +++++++++++++++-----------
> >   drivers/net/txgbe/base/txgbe_phy.h    |  90 +++++++++++++++--
> >   drivers/net/txgbe/base/txgbe_type.h   |  24 ++++-
> >   drivers/net/txgbe/txgbe_ethdev.c      |   6 ++
> >   drivers/net/txgbe/txgbe_logs.h        |   9 ++
> >   10 files changed, 239 insertions(+), 143 deletions(-)
> >
> > diff --git a/config/rte_config.h b/config/rte_config.h index
> > 55a2fc50e..834b52245 100644
> > --- a/config/rte_config.h
> > +++ b/config/rte_config.h
> > @@ -131,6 +131,13 @@
> >   #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VF 4
> >   #define RTE_LIBRTE_I40E_QUEUE_NUM_PER_VM 4
> >
> > +/* txgbe defines */
> > +#undef RTE_TXGBE_DEBUG_BP
> > +#define RTE_TXGBE_BP_AUTO 1
> > +#undef RTE_TXGBE_KR_POLL
> > +#define RTE_TXGBE_KR_PRESENT 1
> > +#undef RTE_TXGBE_KX_SGMII
> > +
> 
> We should prevent adding more compile time options as much as possible.
> 
> >   /* Ring net PMD settings */
> >   #define RTE_PMD_RING_MAX_RX_RINGS 16
> >   #define RTE_PMD_RING_MAX_TX_RINGS 16 diff --git
> > a/doc/guides/nics/txgbe.rst b/doc/guides/nics/txgbe.rst index
> > e520f13f3..52e23942b 100644
> > --- a/doc/guides/nics/txgbe.rst
> > +++ b/doc/guides/nics/txgbe.rst
> > @@ -67,6 +67,26 @@ Please note that enabling debugging options may
> affect system performance.
> >
> >     Decide to enable or disable HW CRC in VF PMD.
> >
> > +- ``RTE_TXGBE_DEBUG_BP`` (undefined by default)
> > +
> > +  Toggle display of auto-negtiation process for backplane NICs.
> > +
> > +- ``RTE_TXGBE_BP_AUTO`` (defined by default)
> > +
> > +  Decide to use auto-negtiation mode or force mode to link up backplane
> NICs.
> > +
> > +- ``RTE_TXGBE_KR_POLL`` (undefined by default)
> > +
> > +  Enable or disable polling mode to receive AN interrupt for backplane NICs.
> > +
> > +- ``RTE_TXGBE_KR_PRESENT`` (defined by default)
> > +
> > +  Decide to use present mode or init mode for backplane NICs.
> > +
> > +- ``RTE_TXGBE_KX_SGMII`` (undefined by default)
> > +
> > +  Special treatment for KX SGMII cards.
> > +
> 
> Why these compile time flags are added?
> 
> I see one of them is for the logging, which can be converted into the dynamic
> debug already. The existing compile time debug flags are only for the datapath.
> 
> And for other compile time flags, can you please convert them to the device
> arguments, you can see multiple samples on other drivers, like
> 'IXGBEVF_DEVARG_PFLINK_FULLCHK' in ixgbe.
> 
> There are multiple problem with the compile time flags, they are harder to fully
> tested, and easy to leave broken code. Also it is problematic for distributing
> software, since these options can't be modified on binary and not that useful
> for end users.

Ok, I will convert them.

These compile time flags are added because users who buy our chips to produce
their own backplane cards want to support some extended features by changing
parameters.