Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 5 Jan 2023 18:13:53 +0100
From:   Andrew Lunn <andrew@lunn.ch>
To:     Roger Quadros <rogerq@kernel.org>
Cc:     Md Danish Anwar <danishanwar@ti.com>,
        "Andrew F. Davis" <afd@ti.com>, Tero Kristo <t-kristo@ti.com>,
        Suman Anna <s-anna@ti.com>, YueHaibing <yuehaibing@huawei.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Rob Herring <robh+dt@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>, nm@ti.com,
        ssantosh@kernel.org, srk@ti.com, linux-kernel@vger.kernel.org,
        devicetree@vger.kernel.org, netdev@vger.kernel.org,
        linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 1/2] dt-bindings: net: Add ICSSG Ethernet Driver
 bindings
Message-ID: <Y7cFUW6dFRaI+nPV@lunn.ch>
References: <20221223110930.1337536-1-danishanwar@ti.com>
 <20221223110930.1337536-2-danishanwar@ti.com>
 <Y6W7FNzJEHYt6URg@lunn.ch>
 <620ce8e6-2b40-1322-364a-0099a6e2af26@kernel.org>
 <Y7Mjx8ZEVEcU2mK8@lunn.ch>
 <b55dec4b-4fd5-71fa-4073-b5793cafdee7@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <b55dec4b-4fd5-71fa-4073-b5793cafdee7@kernel.org>
Precedence: bulk

> >> On 23/12/2022 16:28, Andrew Lunn wrote:
> >>>> +        ethernet-ports {
> >>>> +            #address-cells = <1>;
> >>>> +            #size-cells = <0>;
> >>>> +            pruss2_emac0: port@0 {
> >>>> +                reg = <0>;
> >>>> +                phy-handle = <&pruss2_eth0_phy>;
> >>>> +                phy-mode = "rgmii-rxid";
> >>>
> >>> That is unusual. Where are the TX delays coming from?
> >>
> >> >From the below property
> >>
> >> +                ti,syscon-rgmii-delay = <&scm_conf 0x4120>;
> >>
> >> The TX delay can be enabled/disabled from within the ICSSG block.
> >>
> >> If this property exists and PHY mode is neither PHY_INTERFACE_MODE_RGMII_ID
> >> nor PHY_INTERFACE_MODE_RGMII_TXID then the internal delay is enabled.
> >>
> >> This logic is in prueth_config_rgmiidelay() function in the introduced driver.
> > 
> > What nearly every other MAC driver does is pass the phy-mode to the
> > PHY and lets the PHY add the delays. I would recommend you do that,
> > rather than be special and different.
> 
> 
> If I remember right we couldn't disable MAC TX delay on some earlier silicon
> so had to take this route. I don't remember why we couldn't disable it though.
> 
> In more recent Silicon Manuals I do see that MAC TX delay can be enabled/disabled.
> If this really is the case then we should change to
> 
>  phy-mode = "rgmii-id";
> 
> And let PHY handle the TX+RX delays.

DT describes the board. PHY mode indicates what delays the board
requires, because the board itself is not performing the delays by
using extra long lines. So typically, phy-mode is rgmii-id, indicating
delays need to be added somewhere in both directions.

Who adds the delays is then between the MAC and the PHY. In most
cases, the MAC does nothing, and passes phy-mode to the PHY and the
PHY does it.

But it is also possible for the MAC to do the delay. So if you cannot
actually disable the TX delay in the MAC, that is O.K. But you need to
modify phy-mode you pass to the PHY to indicate the MAC is doing the
delay, otherwise the PHY will additionally do the delay. So your DT
will contain rgmii-id, because that is what the board requires, but
the MAC will pass rmgii-rxid to the PHY, since that is what the PHY
needs to add.

	Andrew