All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.