Linux-Hwmon Archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Choong Yong Liang <yong.liang.choong@linux.intel.com>
Cc: "Voon, Weifeng" <weifeng.voon@intel.com>,
	Rajneesh Bhardwaj <irenic.rajneesh@gmail.com>,
	David E Box <david.e.box@linux.intel.com>,
	Hans de Goede <hdegoede@redhat.com>,
	Mark Gross <markgross@kernel.org>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Jose Abreu <Jose.Abreu@synopsys.com>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Richard Cochran <richardcochran@gmail.com>,
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Jesper Dangaard Brouer <hawk@kernel.org>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Andrew Halaney <ahalaney@redhat.com>,
	Serge Semin <fancer.lancer@gmail.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-stm32@st-md-mailman.stormreply.com"
	<linux-stm32@st-md-mailman.stormreply.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"platform-driver-x86@vger.kernel.org"
	<platform-driver-x86@vger.kernel.org>,
	"linux-hwmon@vger.kernel.org" <linux-hwmon@vger.kernel.org>,
	"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
	"Sit, Michael Wei Hong" <michael.wei.hong.sit@intel.com>,
	"Lai, Peter Jun Ann" <peter.jun.ann.lai@intel.com>,
	"Abdul Rahim, Faizal" <faizal.abdul.rahim@intel.com>
Subject: Re: [PATCH net-next v5 1/9] net: phylink: provide mac_get_pcs_neg_mode() function
Date: Tue, 5 Mar 2024 08:39:30 +0000	[thread overview]
Message-ID: <ZebaQpwyHi0ZoPws@shell.armlinux.org.uk> (raw)
In-Reply-To: <6fbbd36a-20f5-43db-97fc-c8755a82a159@linux.intel.com>

On Tue, Mar 05, 2024 at 12:20:39PM +0800, Choong Yong Liang wrote:
> 
> 
> On 23/2/2024 2:58 pm, Voon, Weifeng wrote:
> > > > For instance, if the interface switches from 2500baseX to SGMII mode,
> > > > and the current link mode is MLO_AN_PHY, calling
> > > 'phylink_pcs_neg_mode'
> > > > would yield PHYLINK_PCS_NEG_OUTBAND. Since the MAC and PCS driver
> > > > require PHYLINK_PCS_NEG_INBAND_ENABLED, the
> > > 'mac_get_pcs_neg_mode'
> > > > function will calculate the mode based on the interface, current link
> > > > negotiation mode, and advertising link mode, returning
> > > > PHYLINK_PCS_NEG_OUTBAND to enable the PCS to configure the correct
> > > settings.
> > > 
> > > This paragraph doesn't make sense - at least to me. It first talks about
> > > requiring PHYLINK_PCS_NEG_INBAND_ENABLED when in SGMII mode. On
> > > this:
> > 
> > The example given here is a very specific condition and that probably why there are some confusions here. Basically, this patch provides an optional function for MAC driver to change the phy interface on-the-fly without the need of reinitialize the Ethernet driver. As we know that the 2500base-x is messy, in our case the 2500base-x does not support inband. To complete the picture, we are using SGMII c37 to handle speed 10/100/1000. Hence, to enable user to switch link speed from 2500 to 1000/100/10 and vice versa on-the-fly, the phy interface need to be configured to inband SGMII for speed 10/100/1000, and outband 2500base-x for speed 2500. Lastly, the newly introduced "mac_get_pcs_neg_mode"callback function enables MAC driver to reconfigure pcs negotiation mode to inband or outband based on the interface mode, current link negotiation mode, and advertising link mode.
> > 
> > > 
> > > 1) are you sure that the hardware can't be programmed for the SGMII
> > > symbol repititions?
> > > 
> > 
> > No, the HW can be program for SGMII symbol repetitions.
> > 
> > > 2) what happens if you're paired with a PHY (e.g. on a SFP module) which
> > > uses SGMII but has no capability of providing the inband data?
> > > (They do exist.) If your hardware truly does require inband data, it is going to
> > > be fundamentally inoperative with these modules.
> > > 
> > 
> > Above explanation should have already cleared your doubts. Inband or outband capability is configured based on the phy interface.
> > 
> > > Next, you then talk about returning PHYLINK_PCS_NEG_OUTBAND for the
> > > "correct settings". How does this relate to the first part where you basically
> > > describe the problem as SGMII requring inband? Basically the two don't
> > > follow.
> > 
> > It should be a typo mistake. SGMII should return PHYLINK_PCS_NEG_INBAND_ENABLED.
> > 
> > > 
> > > How, from a design point of view, because this fundamentally allows drivers
> > > to change how the system behaves, it will allow radically different behaviours
> > > for the same parameters between different drivers.
> > > I am opposed to that - I want to see a situation where we have uniform
> > > behaviour for the same configuration, and where hardware doesn't support
> > > something, we have some way to indicate that via some form of capabilities.
> > > 
> > 
> > Hi Russell,
> > If I understand you correctly, MAC driver should not interfere with pcs negotiation mode and it should be standardized in the generic function, e.g., phylink_pcs_neg_mode()?
> > 
> > > The issue of whether 2500base-X has inband or not is a long standing issue,
> > > and there are arguments (and hardware) that take totally opposing views on
> > > this. There is hardware where 2500base-X inband _must_ be used or the link
> > > doesn't come up. There is also hardware where 2500base-X inband is not
> > > "supported" in documentation but works in practice. There is also hardware
> > > where 2500base-X inband doesn't work. The whole thing is a total mess
> > > (thanks IEEE 802.3 for not getting on top of this early enough... and what's
> > > now stated in 802.3 for 2500base-X is now irrelevant because they were too
> > > late to the
> > > party.)
> > > 
> > 
> > Agreed. And I have also seen some of your comments regarding the 2500SGMII and 2500BASEX.
> > 
> Hi Russell,
> 
> Did the previous reply clear your doubt?

I am working on the problem as and when I can find the time (which is
not much spare time.) I have some prototype code at the moment.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2024-03-05  8:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-15  3:04 [PATCH net-next v5 0/9] Enable SGMII and 2500BASEX interface mode switching for Intel platforms Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 1/9] net: phylink: provide mac_get_pcs_neg_mode() function Choong Yong Liang
2024-02-15 16:26   ` Russell King (Oracle)
2024-02-23  6:58     ` Voon, Weifeng
2024-03-05  4:20       ` Choong Yong Liang
2024-03-05  8:39         ` Russell King (Oracle) [this message]
2024-02-15  3:04 ` [PATCH net-next v5 2/9] net: phylink: add phylink_pcs_neg_mode() declaration into phylink.h Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 3/9] net: stmmac: select PCS negotiation mode according to the interface mode Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 4/9] net: pcs: xpcs: re-initiate clause 37 Auto-negotiation Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 5/9] arch: x86: Add IPC mailbox accessor function and add SoC register access Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 6/9] net: stmmac: configure SerDes on mac_finish Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 7/9] stmmac: intel: configure SerDes according to the interface mode Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 8/9] stmmac: intel: interface switching support for EHL platform Choong Yong Liang
2024-02-15  3:04 ` [PATCH net-next v5 9/9] stmmac: intel: interface switching support for ADL-N platform Choong Yong Liang

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=ZebaQpwyHi0ZoPws@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Jose.Abreu@synopsys.com \
    --cc=ahalaney@redhat.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrew@lunn.ch \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=david.e.box@linux.intel.com \
    --cc=edumazet@google.com \
    --cc=faizal.abdul.rahim@intel.com \
    --cc=fancer.lancer@gmail.com \
    --cc=hawk@kernel.org \
    --cc=hdegoede@redhat.com \
    --cc=hkallweit1@gmail.com \
    --cc=irenic.rajneesh@gmail.com \
    --cc=john.fastabend@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=markgross@kernel.org \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=michael.wei.hong.sit@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=p.zabel@pengutronix.de \
    --cc=pabeni@redhat.com \
    --cc=peter.jun.ann.lai@intel.com \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=weifeng.voon@intel.com \
    --cc=yong.liang.choong@linux.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 read-only IMAP folder(s) and NNTP newsgroup(s).