DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: Jeff Daly <jeffd@silicom-usa.com>
Cc: "Zhang, Qi Z" <qi.z.zhang@intel.com>,
	"Yang, Qiming" <qiming.yang@intel.com>,
	"Wu, Wenjun1" <wenjun1.wu@intel.com>,
	"Zhao1, Wei" <wei.zhao1@intel.com>,
	Xiao Zhang <xiao.zhang@intel.com>,
	Xiaolong Ye <xiaolong.ye@intel.com>,
	Lunyuan Cui <lunyuanx.cui@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"stable@dpdk.org" <stable@dpdk.org>
Subject: Re: [PATCH] net/ixgbe: Limit SDP3 check of TX_DISABLE to appropriate devices
Date: Thu, 12 May 2022 21:15:54 +0200	[thread overview]
Message-ID: <2941580.687JKscXgg@thomas> (raw)
In-Reply-To: <VI1PR0402MB3517AFE8F21CE7D23670E7D5EACB9@VI1PR0402MB3517.eurprd04.prod.outlook.com>

12/05/2022 19:01, Jeff Daly:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 11/05/2022 13:43, Zhang, Qi Z:
> > > From: Jeff Daly <jeffd@silicom-usa.com>
> > > >
> > > > 1ca05831b9b added a check that SDP3 (used as a TX_DISABLE output to
> > > > the SFP cage on these cards) is not asserted to avoid incorrectly
> > > > reporting link up when the SFP's laser is turned off.
> > > >
> > > > ff8162cb957 limited this workaround to fiber ports
> > > >
> > > > This patch:
> > > > * Adds devarg 'fiber_sdp3_no_tx_disable' not all fiber ixgbe devs use
> > > >   SDP3 as TX_DISABLE
> > > >
> > > > Fixes: 1ca05831b9b ("net/ixgbe: fix link status")
> > > > Fixes: ff8162cb957 ("net/ixgbe: fix link status")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Jeff Daly <jeffd@silicom-usa.com>
> > >
> > > Acked-by: Qi Zhang <qi.z.zhang@intel.com>
> > >
> > > Applied to dpdk-next-net-intel.
> > 
> > There is a lack of context in this description.
> > I don't know what SDP3 and TX_DISABLE refers to.
> > Please make more complete sentences, thanks.
> > 
> 
> I don't want to sound obtuse here, but this is a fix to a specific Intel NIC driver.  Any symbols or abbreviations or definitions used in a device driver are almost always in the manual.  While SDP3 means something specific to the Intel 82599 and X550 (and probably others), it probably doesn't appear in a driver from Marvell for example.  So, in this case [S]oftware [D]efined [P]ins [3] (out of 0-3) is specifically talking about the Intel X550.  I'm familiar enough with the hardware to recognize that, but if I was to look at a Marvell driver and saw something I didn't recognize like that, I'd be checking the Marvell manual.

Of course we can spend hours checking code and manuals.
But if we need to look to a lot of commits,
it is a lot more convenient to have a summary of this information
in the commit log.

> What I'm describing here is the fact that the TX_DISABLE signal (a signal defined in the SFP spec) from the NIC as implemented by the Software Defined Pin (3) by many (most?) implementations that are using this Intel driver, is not specifically the *only* use of SDP3.  A later patch limited the check (correctly) to fiber implementations (which is the only thing that makes sense), and *this* patch adds a module switch for platforms to disable this check in the event that they (as they are perfectly allowed to) don't use SDP3 as TX_DISABLE.

Thank you for the explanation.



      reply	other threads:[~2022-05-12 19:16 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-10 18:57 Jeff Daly
2022-05-11 11:43 ` Zhang, Qi Z
2022-05-11 14:50   ` Thomas Monjalon
2022-05-12 17:01     ` Jeff Daly
2022-05-12 19:15       ` Thomas Monjalon [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2941580.687JKscXgg@thomas \
    --to=thomas@monjalon.net \
    --cc=dev@dpdk.org \
    --cc=jeffd@silicom-usa.com \
    --cc=lunyuanx.cui@intel.com \
    --cc=qi.z.zhang@intel.com \
    --cc=qiming.yang@intel.com \
    --cc=stable@dpdk.org \
    --cc=wei.zhao1@intel.com \
    --cc=wenjun1.wu@intel.com \
    --cc=xiao.zhang@intel.com \
    --cc=xiaolong.ye@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).