Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Tue, 12 Apr 2022 15:10:36 +0200
From:   Andrew Lunn <andrew@lunn.ch>
To:     Grygorii Strashko <grygorii.strashko@ti.com>
Cc:     Puranjay Mohan <p-mohan@ti.com>, linux-kernel@vger.kernel.org,
        bjorn.andersson@linaro.org, mathieu.poirier@linaro.org,
        krzysztof.kozlowski+dt@linaro.org,
        linux-remoteproc@vger.kernel.org, devicetree@vger.kernel.org,
        nm@ti.com, ssantosh@kernel.org, s-anna@ti.com,
        linux-arm-kernel@lists.infradead.org, davem@davemloft.net,
        kuba@kernel.org, netdev@vger.kernel.org, vigneshr@ti.com,
        kishon@ti.com, Roger Quadros <rogerq@kernel.org>
Subject: Re: [RFC 13/13] net: ti: icssg-prueth: Add ICSSG ethernet driver
Message-ID: <YlV6TKTl5uQGZrQX@lunn.ch>
References: <20220406094358.7895-1-p-mohan@ti.com>
 <20220406094358.7895-14-p-mohan@ti.com>
 <Yk3d/cC36fhNmfY2@lunn.ch>
 <468d4d9b-44b4-2894-2a75-4caab1e72147@ti.com>
 <f3ae48a8-7177-3a92-bdfd-3b243a5527c1@ti.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <f3ae48a8-7177-3a92-bdfd-3b243a5527c1@ti.com>
Precedence: bulk

> > > > +	if (phy_if == PHY_INTERFACE_MODE_RGMII_ID ||
> > > > +	    phy_if == PHY_INTERFACE_MODE_RGMII_TXID)
> > > > +		rgmii_tx_id |= ICSSG_CTRL_RGMII_ID_MODE;
> > > > +
> > > > +	regmap_update_bits(ctrl_mmr, icssgctrl_reg, ICSSG_CTRL_RGMII_ID_MODE, rgmii_tx_id);
> > > > +
> > > > +	return 0;
> > > > +}
> > > > 
> > > 
> > > O.K, so this does not do what i initially thought it was doing. I was
> > > thinking it was to fine tune the delay, ti,syscon-rgmii-delay would be
> > > a small pico second value to allow the 2ns delay to be tuned to the
> > > board.
> > > 
> > > But now i think this is actually inserting the full 2ns delay?
> > > 
> > > The problem is, you also pass phy_if to of_phy_connect() so the PHY
> > > will also insert the delay if requested. So you end up with double
> > > delays for rgmii_id and rgmii_txid.
> 
> It's misunderstanding here. The bit field name in TRM is RGMII0_ID_MODE
> and meaning:
> 0h - Internal transmit delay is enabled
> 1h - Internal transmit delay is not enabled.
> 
> So here internal delay will be disabled for RGMII_ID/RGMII_TXID.

And enabled for the others?

Why don't you always disable the delays and let the PHY do it? That is
what pretty much every other MAC/PHY combination does.

    Andrew