Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 4 Aug 2021 13:46:25 +0300
From:   Vladimir Oltean <olteanv@gmail.com>
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Prasanna Vengateshan <prasanna.vengateshan@microchip.com>,
        Andrew Lunn <andrew@lunn.ch>, 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
Message-ID: <20210804104625.d2qw3gr7algzppz5@skbuf>
References: <20210723173108.459770-1-prasanna.vengateshan@microchip.com>
 <20210723173108.459770-6-prasanna.vengateshan@microchip.com>
 <20210731150416.upe5nwkwvwajhwgg@skbuf>
 <49678cce02ac03edc6bbbd1afb5f67606ac3efc2.camel@microchip.com>
 <20210802121550.gqgbipqdvp5x76ii@skbuf>
 <YQfvXTEbyYFMLH5u@lunn.ch>
 <20210802135911.inpu6khavvwsfjsp@skbuf>
 <50eb24a1e407b651eda7aeeff26d82d3805a6a41.camel@microchip.com>
 <20210803235401.rctfylazg47cjah5@skbuf>
 <20210804095954.GN22278@shell.armlinux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210804095954.GN22278@shell.armlinux.org.uk>
Precedence: bulk

On Wed, Aug 04, 2021 at 10:59:54AM +0100, Russell King (Oracle) wrote:
> This is why we need to have a clear definition of what the various
> RGMII interface types are, how and where they are applied, and make
> sure everyone sticks to that. We have this documented in
> Documentation/networking/phy.rst.

The problem is that I have no clear migration path for the drivers I
maintain, like sja1105, and I suspect that others might be in the exact
same situation.

Currently, if the sja1105 needs to add internal delays in a MAC-to-MAC
(fixed-link) setup, it does that based on the phy-mode string. So
"rgmii-id" + "fixed-link" means for sja1105 "add RX and TX RGMII
internal delays", even though the documentation now says "the MAC should
not add the RX or TX delays in this case".

There are 2 cases to think about, old driver with new DT blob and new
driver with old DT blob. If breakage is involved, I am not actually very
interested in doing the migration, because even though the interpretation
of the phy-mode string is inconsistent between the phy-handle and fixed-link
case (which was deliberate), at least it currently does all that I need it to.

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? 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?
If it is an error, should all MAC drivers check for it? And if it is an
error, does it not make migration even more difficult (adding an
rx-internal-delay-ps property to a MAC OF node which already uses
"rgmii-id" would be preferable to also having to change the "rgmii-id"
to "rgmii", because an old kernel might also need to work with that DT
blob, and that will ignore the new rx-internal-delay-ps property).

Does qca8k_setup_of_rgmii_delay(), a very recent function, even do the
right thing with rx-internal-delay-ps, or is it doing the exact opposite
of the right thing (it applies rx-internal-delay-ps when in rgmii-id or
rgmii-rxid mode).