linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>,
	Prasanna Vengateshan <prasanna.vengateshan@microchip.com>,
	netdev@vger.kernel.org, robh+dt@kernel.org,
	UNGLinuxDriver@microchip.com, Woojung.Huh@microchip.com,
	hkallweit1@gmail.com, davem@davemloft.net, kuba@kernel.org,
	linux-kernel@vger.kernel.org, vivien.didelot@gmail.com,
	f.fainelli@gmail.com, devicetree@vger.kernel.org
Subject: Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x
Date: Sat, 7 Aug 2021 17:40:35 +0200	[thread overview]
Message-ID: <YQ6pc6EZRLftmRh3@lunn.ch> (raw)
In-Reply-To: <20210804104625.d2qw3gr7algzppz5@skbuf>

> I am not even clear what is the expected canonical behavior for a MAC
> driver. It parses rx-internal-delay-ps and tx-internal-delay-ps, and
> then what?

So best practices are based around a MAC-PHY link. phy-mode is passed
to the PHY, and the MAC does not act upon it. MAC rx-internal-delay-ps
and tx-internal-delay-ps can be used to fine tune the link. You can
use them to add and sometimes subtract small amounts of delay.

> It treats all "rgmii*" phy-mode strings identically? Or is it an
> error to have "rgmii-rxid" for phy-mode and non-zero
> rx-internal-delay-ps?

I would say the first is correct, the second statement is false. You
should always be able to fine tune the link, independent of the PHY
mode.

We also have to consider the case when the PHY is not actually able to
implement the delay. It hopefully returns -EOPNOTSUPP for anything
other than "rgmii". You can then put the full 2ns delay into
tx-internal-delay-ps nd rx-internal-delay-ps.

And lastly there is one MAC driver which mostly ignores these best
practices because the vendor crap tree always did the delay in the
MAC. It correctly masks the phy-mode, so the PHY does not add delays.

For MAC-MAC and fixed link best practices are very fuzzily defined.
It is not something we have much of in the kernel. We might also want
to narrow the discussion down to MACs within a switch. MACs within in
NIC should probably follow the best practices for a MAC-PHY link, even
if it is actually a switch on the other end.

I also agree with Russell that mv88e6xxx is probably broken for a
MAC-PHY link. It is known to work for a Marvell DSA in MAC-MAC link,
we have boards doing that.

It seems like a switch MAC should parse rx-internal-delay-ps and
tx-internal-delay-ps and apply them independent of the phy-mode. That
keeps it consistent with MAC-PHY. And if there is a PHY connected,
pass the phy-mode on unmasked.

So that we don't break mv88e6xxx, for a CPU or DSA port, we probably
should continue to locally implement the delay, with the assumption
there is no PHY, it is a MAC-MAC link. We probably want to patch
mv88e6xxx to do nothing for user ports.

I suspect it is a 50/50 roll of a dice what rx and tx actually
mean. Is it from the perspective of the MAC or the PHY? Luckily,
rgmii-rxid and rgmii-txid don't appear in DT very often.

   Andrew

  parent reply	other threads:[~2021-08-07 15:40 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-23 17:30 [PATCH v3 net-next 00/10] net: dsa: microchip: DSA driver support for LAN937x switch Prasanna Vengateshan
2021-07-23 17:30 ` [PATCH v3 net-next 01/10] dt-bindings: net: dsa: dt bindings for microchip lan937x Prasanna Vengateshan
2021-07-26 22:49   ` Rob Herring
2021-07-23 17:31 ` [PATCH v3 net-next 02/10] net: dsa: move mib->cnt_ptr reset code to ksz_common.c Prasanna Vengateshan
2021-07-23 18:53   ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 03/10] net: phy: Add support for LAN937x T1 phy driver Prasanna Vengateshan
2021-08-11 17:52   ` Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 04/10] net: dsa: tag_ksz: add tag handling for Microchip LAN937x Prasanna Vengateshan
2021-07-23 19:23   ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x Prasanna Vengateshan
2021-07-31 15:04   ` Vladimir Oltean
2021-07-31 22:05     ` Andrew Lunn
2021-08-02 21:33       ` Vladimir Oltean
2021-08-03 14:43         ` Andrew Lunn
2021-08-03 15:05           ` Vladimir Oltean
2021-08-02 10:45     ` Prasanna Vengateshan
2021-08-02 12:15       ` Vladimir Oltean
2021-08-02 13:13         ` Andrew Lunn
2021-08-02 13:59           ` Vladimir Oltean
2021-08-02 20:47             ` Andrew Lunn
2021-08-03 16:54             ` Prasanna Vengateshan
2021-08-03 23:54               ` Vladimir Oltean
2021-08-04  9:59                 ` Russell King (Oracle)
2021-08-04 10:46                   ` Vladimir Oltean
2021-08-04 14:28                     ` Prasanna Vengateshan
2021-08-04 14:51                       ` Vladimir Oltean
2021-08-07 15:40                     ` Andrew Lunn [this message]
2021-08-07 17:00                       ` Vladimir Oltean
2021-08-11 17:44                       ` Prasanna Vengateshan
2021-08-11 18:23                         ` Andrew Lunn
2021-08-11 20:14                           ` Russell King (Oracle)
2021-08-11 20:20                             ` Vladimir Oltean
2021-08-11 20:22                               ` Andrew Lunn
2021-07-23 17:31 ` [PATCH v3 net-next 06/10] net: dsa: microchip: add support for phylink management Prasanna Vengateshan
2021-07-31 15:27   ` Vladimir Oltean
2021-08-03 17:04     ` Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 07/10] net: dsa: microchip: add support for ethtool port counters Prasanna Vengateshan
2021-07-23 17:31 ` [PATCH v3 net-next 08/10] net: dsa: microchip: add support for port mirror operations Prasanna Vengateshan
2021-07-31 15:24   ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 09/10] net: dsa: microchip: add support for fdb and mdb management Prasanna Vengateshan
2021-07-31 15:19   ` Vladimir Oltean
2021-07-23 17:31 ` [PATCH v3 net-next 10/10] net: dsa: microchip: add support for vlan operations Prasanna Vengateshan
2021-07-31 15:08   ` Vladimir Oltean
2021-08-02 10:48     ` Prasanna Vengateshan

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=YQ6pc6EZRLftmRh3@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=Woojung.Huh@microchip.com \
    --cc=davem@davemloft.net \
    --cc=devicetree@vger.kernel.org \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=prasanna.vengateshan@microchip.com \
    --cc=robh+dt@kernel.org \
    --cc=vivien.didelot@gmail.com \
    --subject='Re: [PATCH v3 net-next 05/10] net: dsa: microchip: add DSA support for microchip lan937x' \
    /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

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).