RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550 devices

[Jeff Daly <jeffd@silicom-usa.com>]

> -----Original Message-----
> From: Zhang, Qi Z <qi.z.zhang@intel.com>
> Sent: Sunday, May 29, 2022 6:49 PM
> To: Jeff Daly <jeffd@silicom-usa.com>; dev@dpdk.org
> Cc: Stephen Douthit <stephend@silicom-usa.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: RE: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550
> devices
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments.
> 
> 
> > -----Original Message-----
> > From: Jeff Daly <jeffd@silicom-usa.com>
> > Sent: Friday, May 27, 2022 4:44 AM
> > To: dev@dpdk.org
> > Cc: Stephen Douthit <stephend@silicom-usa.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> > Subject: [PATCH v2] net/ixgbe: Treat 1G Cu SFPs as 1G SX on the X550
> > devices
> >
> > 1G Cu SFPs are not officially supported on the X552/X553 family of
> > devices but create an option cu_sfp_as_sx to treat them as 1G SX
> > modules since they usually work.  Print a warning though since support
> > isn't validated, similar to what already happens for other
> > unofficially supported SFPs enabled via the allow_unsupported_sfps
> parameter inherited from the mainline Linux driver.
> >
> > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > Suggested-by: Stephen Douthit <stephend@silicom-usa.com>
> > ---
> > v2:
> > * Introduced cu_sfp_as_sx option, default off.
> > ---
> >  doc/guides/nics/ixgbe.rst           | 16 ++++++++++++++
> >  drivers/net/ixgbe/base/ixgbe_type.h |  1 +
> > drivers/net/ixgbe/base/ixgbe_x550.c | 12 ++++++++++-
> >  drivers/net/ixgbe/ixgbe_ethdev.c    | 33
> +++++++++++++++++++++++++++++
> >  drivers/net/ixgbe/ixgbe_ethdev.h    |  3 +++
> >  5 files changed, 64 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/guides/nics/ixgbe.rst b/doc/guides/nics/ixgbe.rst
> > index
> > 82fa453fa28e..5db63083eef8 100644
> > --- a/doc/guides/nics/ixgbe.rst
> > +++ b/doc/guides/nics/ixgbe.rst
> > @@ -101,6 +101,22 @@ To guarantee the constraint, capabilities in
> > dev_conf.rxmode.offloads will be ch
> >
> >  fdir_conf->mode will also be checked.
> >
> > +Runtime Options
> > +^^^^^^^^^^^^^^^^^^
> > +
> > +The following ``devargs`` options can be enabled at runtime. They
> > +must be passed as part of EAL arguments. For example,
> > +
> > +.. code-block:: console
> > +
> > +   dpdk-testpmd -a af:10.0,cu_sfp_as_sx=1 -- -i
> > +
> > +- ``cu_sfp_as_sx`` (default **0**)
> 
> Can we make this devargs more generic e.g.: "allow_unsupported_phy"
> So we don't need to add a devarg for similar requirement case by case in
> future, of cause we still need to well explain all the unsupported cases in the
> document.
> 
> 

this patch is specifically to change the driver's recognition of Cu transceivers
and treat them as optical transceivers.  so should we consider this an 
unsupported phy and use that same switch 'allow_unsupported_phy' or are
you looking for a more generic name than 'cu_sfp_as_sx'?  if you are looking
for a more generic name vs just reusing allow_unsupported_phy, then please
pick something and I'll submit a new patch, but I don't want to guess what
would be ok by submitting patches.