devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marek Behún" <kabel@kernel.org>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>,
	netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Rob Herring <robh+dt@kernel.org>,
	devicetree@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Sean Anderson <sean.anderson@seco.com>,
	davem@davemloft.net
Subject: Re: [PATCH net-next v2 4/8] net: phylink: update supported_interfaces with modes from fwnode
Date: Wed, 24 Nov 2021 13:17:03 +0100	[thread overview]
Message-ID: <20211124131703.30176315@thinkpad> (raw)
In-Reply-To: <20211124120441.i7735czjm5k3mkwh@skbuf>

On Wed, 24 Nov 2021 14:04:41 +0200
Vladimir Oltean <olteanv@gmail.com> wrote:

> On Wed, Nov 24, 2021 at 11:04:37AM +0000, Russell King (Oracle) wrote:
> > On Wed, Nov 24, 2021 at 12:54:18AM +0200, Vladimir Oltean wrote:  
> > > This implies that when you bring up a board and write the device tree
> > > for it, you know that PHY mode X works without ever testing it. What if
> > > it doesn't work when you finally add support for it? Now you already
> > > have one DT blob in circulation. That's why I'm saying that maybe it
> > > could be better if we could think in terms that are a bit more physical
> > > and easy to characterize.  
> > 
> > However, it doesn't solve the problem. Let's take an example.
> > 
> > The 3310 supports a mode where it runs in XAUI/5GBASE-R/2500BASE-X/SGMII
> > depending on the negotiated media parameters.
> > 
> > XAUI is four lanes of 3.125Gbaud.
> > 5GBASE-R is one lane of 5.15625Gbaud.
> > 
> > Let's say you're using this, and test the 10G speed using XAUI,
> > intending the other speeds to work. So you put in DT that you support
> > four lanes and up to 5.15625Gbaud.  
> 
> Yes, see, the blame's on you if you do that.You effectively declared
> that the lane is able of sustaining a data rate higher than you've
> actually had proof it does (5.156 vs 3.125).

But the blame is on the DT writer in the same way if they declare
support for a PHY mode that wasn't tested. (Or at least self-tests with
PRBS patterns at given frequency.)

> The reason why I'm making this suggestion is because I think it lends
> itself better to the way in which hardware manufacturers work.
> A hobbyist like me has no choice than to test the highest data rate when
> determining what frequency to declare in the DT (it's the same thing for
> spi-max-frequency and other limilar DT properties, really), but hardware
> people have simulations based on IBIS-AMI models, they can do SERDES
> self-tests using PRBS patterns, lots of stuff to characterize what
> frequency a lane is rated for, without actually speaking any Ethernet
> protocol on it. In fact there are lots of people who can do this stuff
> (which I know mostly nothing about) with precision without even knowing
> how to even type a simple "ls" inside a Linux shell.
>
> > Later, you discover that 5GBASE-R doesn't work because there's an
> > electrical issue with the board. You now have DT in circulation
> > which doesn't match the capabilities of the hardware.
> > 
> > How is this any different from the situation you describe above?
> > To me, it seems to be exactly the same problem.  
> 
> To err is human, of course. But one thing I think we learned from the
> old implementation of phylink_validate is that it gets very tiring to
> keep adding PHY modes, and we always seem to miss some. When that array
> will be described in DT, it could be just a tad more painful to maintain.

The thing is that we will still need the `phy-mode` property, it can't
be deprecated IMO. There are non-SerDes modes, like rgmii, which may
use different pins than SerDes modes.
There may theoretically also be a SoC or PHY where the lanes for XAUI
do not share pins with the lane of 2500base-x, and this lane may not be
wired. Tis true that I don't know of any such hardware and it probably
does not and will not exist, but we don't know that for sure and this is
a case where your proposal will fail and the phy-mode extension would
work nicely.

Maybe we need opinions from other people here.

Marek

  reply	other threads:[~2021-11-24 12:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-23 16:40 [PATCH net-next v2 0/8] Extend `phy-mode` to string array Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 1/8] dt-bindings: ethernet-controller: support multiple PHY connection types Marek Behún
2021-11-30 22:26   ` Rob Herring
2021-11-23 16:40 ` [PATCH net-next v2 2/8] net: Update documentation for *_get_phy_mode() functions Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 3/8] device property: add helper function for getting phy mode bitmap Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 4/8] net: phylink: update supported_interfaces with modes from fwnode Marek Behún
2021-11-23 21:24   ` Vladimir Oltean
2021-11-23 22:27     ` Marek Behún
2021-11-23 22:54       ` Vladimir Oltean
2021-11-24 11:04         ` Russell King (Oracle)
2021-11-24 12:04           ` Vladimir Oltean
2021-11-24 12:17             ` Marek Behún [this message]
2021-11-24 12:31               ` Vladimir Oltean
2021-12-02 22:25                 ` Marek Behún
2021-12-04 12:42                   ` Vladimir Oltean
2021-11-24 11:50         ` Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 5/8] net: phylink: pass supported PHY interface modes to phylib Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 6/8] net: phy: marvell10g: Use generic macro for supported interfaces Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 7/8] net: phy: marvell10g: Use tabs instead of spaces for indentation Marek Behún
2021-11-23 16:40 ` [PATCH net-next v2 8/8] net: phy: marvell10g: select host interface configuration Marek Behún

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=20211124131703.30176315@thinkpad \
    --to=kabel@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=robh+dt@kernel.org \
    --cc=sean.anderson@seco.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).