From: Andrew Lunn <email@example.com> To: Vladimir Oltean <firstname.lastname@example.org> Cc: "Russell King (Oracle)" <email@example.com>, Prasanna Vengateshan <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, UNGLinuxDriver@microchip.com, Woojung.Huh@microchip.com, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com 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
next prev 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 \ --firstname.lastname@example.org \ --cc=UNGLinuxDriver@microchip.com \ --cc=Woojung.Huh@microchip.com \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --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).