devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Marek Behún" <kabel@kernel.org>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Russell King <rmk+kernel@armlinux.org.uk>,
	Rob Herring <robh@kernel.org>,
	devicetree@vger.kernel.org, pali@kernel.org
Subject: Re: [PATCH net-next 0/2] dt-bindings: define property describing supported ethernet PHY modes
Date: Thu, 25 Mar 2021 00:45:25 +0100	[thread overview]
Message-ID: <20210325004525.734f3040@thinkpad> (raw)
In-Reply-To: <755130b4-2fab-2b53-456f-e2304b1922f2@gmail.com>

On Wed, 24 Mar 2021 16:16:41 -0700
Florian Fainelli <f.fainelli@gmail.com> wrote:

> On 3/24/2021 4:00 PM, Marek Behún wrote:
> > On Wed, 24 Mar 2021 14:19:28 -0700
> > Florian Fainelli <f.fainelli@gmail.com> wrote:
> >   
> >>> Another problem is that if lower modes are supported, we should
> >>> maybe use them in order to save power.    
> >>
> >> That is an interesting proposal but if you want it to be truly valuable,
> >> does not that mean that an user ought to be able to switch between any
> >> of the supported PHY <=> MAC interfaces at runtime, and then within
> >> those interfaces to the speeds that yield the best power savings?  
> > 
> > If the code determines that there are multiple working configurations,
> > it theoretically could allow the user to switch between them.
> > 
> > My idea was that this should be done by kernel, though.
> > 
> > But power saving is not the main problem I am trying to solve.
> > What I am trying to solve is that if a board does not support all modes
> > supported by the MAC and PHY, because they are not wired or something,
> > we need to know about that so that we can select the correct mode for
> > PHYs that change this mode at runtime.  
> 
> OK so the runtime part comes from plugging in various SFP modules into a
> cage but other than that, for a "fixed" link such as a SFF or a soldered
> down PHY, do we agree that there would be no runtime changing of the
> 'phy-mode'?

No, we do not. The PHY can be configured (by strapping pins or by
sw) to change phy-mode depending on the autonegotiated copper speed.

So if you plug in an ethernet cable where on the otherside is only 1g
capable device, the PHY will change mode to sgmii. But if you then plug
a 5g capable device, the PHY will change mode to 5gbase-r.

This happens if the PHY is configured into one of these changing
configurations. It can also be configured to USXGMII, or 10GBASER with
rate matching.

Not many MACs in kernel support USXGMII currently.

And if you use rate matching mode, and the copper side is
linked in lower speed (2.5g for example), and the MAC will start
sending too many packets, the internal buffer in the PHY is only 16 KB,
so it will fill up quickly. So you need pause frames support. But this
is broken for speeds <= 1g, according to erratum.

So you really want to change modes. The rate matching mode is
basically useless.

> 
> What I am trying to understand is why this needs to be added to the
> Device Tree as opposed to a bitmask within the PHY driver that indicates
> the various interface mode capabilities which, looking at the code you
> shared below, is how you make decisions ultimately.

Because someone can create a board with a SOC where MAC is capable of
all of the following modes: 10gbase-r, xaui, rxaui, 5gbase-r,
2.5gbase-x, sgmii.

And use Marvell 88X3310 PHY to translate to copper.

But only wire the PHY to the MAC with one SerDes lane. So for 10g,
10gbase-r mode must be used, xaui and rxaui cannot.
Or wire the PHY to the MAC with 2 SerDes lanes, but both lanes capable
only of 6 GHz freq. So for 10g, rxaui must be used.

And then make the mistake of wiring the strapping pins to the
rate-matching mode, which is useless.

So we need to know which modes are supported if we want to change the
configuration to a working one.

> >   
> >>>
> >>> But for this we need to know which phy-modes are supported on the
> >>> board.
> >>>
> >>> This series adds documentation for a new ethernet PHY property,
> >>> called `supported-mac-connection-types`.    
> >>
> >> That naming does not quite make sense to me, if we want to describe the
> >> MAC supported connection types, then those would naturally be within the
> >> Ethernet MAC Device Tree node, no? If we are describing what the PHY is
> >> capable, then we should be dropping "mac" from the property name not to
> >> create confusion.  
> > 
> > I put "mac" there to indicate that this is the SerDes to the MAC (i.e.
> > host side in Marvell PHY). 88X3310 has another SerDes side (Fiber Side).
> > I guess I put "mac" there so that if in the future we wanted to specify
> > supported modes for the fiber side, we could add
> > `supported-fiber-connection-types`.  
> 
> You would traditionally find the words "line side" (copper, optical,
> etc.) and "MAC side" being used in datasheets, maybe you can use a
> similar naming here?

So
  supported-connection-types-mac-side
  supported-connection-types-line-side
or maybe media-side?

I am still exploring whether this could be simply defined in the
ethernet controllers `phy-mode` property, as Rob Herring says. It would
be simpler...

Marek

  reply	other threads:[~2021-03-24 23:46 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-24 10:35 [PATCH net-next 0/2] dt-bindings: define property describing supported ethernet PHY modes Marek Behún
2021-03-24 10:35 ` [PATCH net-next 1/2] dt-bindings: ethernet-controller: create a type for PHY interface modes Marek Behún
2021-03-24 20:07   ` Rob Herring
2021-03-24 20:59     ` Marek Behún
2021-03-24 10:35 ` [PATCH net-next 2/2] dt-bindings: ethernet-phy: define `supported-mac-connection-types` property Marek Behún
2021-03-24 21:19 ` [PATCH net-next 0/2] dt-bindings: define property describing supported ethernet PHY modes Florian Fainelli
2021-03-24 23:00   ` Marek Behún
2021-03-24 23:16     ` Florian Fainelli
2021-03-24 23:45       ` Marek Behún [this message]
2021-03-25  0:11         ` Florian Fainelli
2021-03-25  0:30           ` Russell King - ARM Linux admin
2021-03-25  0:43           ` Marek Behún
2021-03-25  0:28         ` Russell King - ARM Linux admin

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=20210325004525.734f3040@thinkpad \
    --to=kabel@kernel.org \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pali@kernel.org \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=robh@kernel.org \
    /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).