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;
MIME-Version: 1.0
References: <20211019145719.122751-1-kory.maincent@bootlin.com>
 <CAMuHMdWghZ7HM5RRFRsZu8P_ikna0QWoRfCKeym61N-Lv-v4Xw@mail.gmail.com>
 <20211019173520.0154a8cb@kmaincent-XPS-13-7390> <YW7nPfzjstmeoMbf@lunn.ch> <20211019175746.11b388ce@windsurf>
In-Reply-To: <20211019175746.11b388ce@windsurf>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Tue, 19 Oct 2021 22:05:41 +0200
Message-ID: <CAMuHMdXiMhpU0vDV3KaOg4DY59cszAtoG1sDOgnTRY6C6cyitQ@mail.gmail.com>
Subject: Re: [PATCH] net: renesas: Fix rgmii-id delays
To:     Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        =?UTF-8?Q?K=C3=B6ry_Maincent?= <kory.maincent@bootlin.com>,
        netdev <netdev@vger.kernel.org>,
        Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Sergey Shtylyov <s.shtylyov@omp.ru>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        Biju Das <biju.das.jz@bp.renesas.com>,
        Sergei Shtylyov <sergei.shtylyov@gmail.com>,
        Adam Ford <aford173@gmail.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>,
        Yang Yingliang <yangyingliang@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Thomas,

On Tue, Oct 19, 2021 at 5:57 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
> On Tue, 19 Oct 2021 17:41:49 +0200
> Andrew Lunn <andrew@lunn.ch> wrote:
> > > When people update the kernel version don't they update also the devicetree?
> >
> > DT is ABI. Driver writers should not break old blobs running on new
> > kernels. Often the DT blob is updated with the kernel, but it is not
> > required. It could be stored in a hard to reach place, shared with
> > u-boot etc.
>
> Right, but conversely if someone reads the DT bindings that exists
> today, specifies phy-mode = "rgmii-rxid" or phy-mmode = "rmgii-txid",

Today == v5.10-rc1 and later?

> this person will get incorrect behavior. Sure a behavior that is
> backward compatible with older DTs, but a terribly wrong one when you
> write a new DT and read the DT binding documentation. This is exactly
> the problem that happened to us.

If you write a new DT, you read the DT binding documentation, and
"make dtbs_check" will inform you politely if you use the legacy/wrong
DT (i.e. lacking "[rt]x-internal-delay-ps")?

> I know that those properties are considered obsolete, but even though
> they are considered as such, they are still supported, but for this
> particular MAC driver, with an inverted meaning compared to what the DT
> binding documentation says.
>
> What wins: DT ABI backward compatibility, or correctness of the DT
> binding ? :-)

Both ;-)

The current driver is backwards-compatible with the legacy/wrong DTB.
The current DT bindings (as of v5.10-rc1), using "[rt]x-internal-delay-ps"
are correct.
Or am I missing something here?

BTW, it's still not clear to me why the inversion would be needed.
Cfr. Andrew's comment:

| So with rgmii-rxid, what is actually passed to the PHY? Is your
| problem you get twice the delay in one direction, and no delay in the
| other?

We know the ravb driver misbehaved in the past by applying the
rgmii-*id values to the MAC, while they are meant for the PHY, thus
causing bad interaction with PHY drivers.  But that was fixed
by commit 9b23203c32ee02cd ("ravb: Mask PHY mode to avoid inserting
delays twice") and a6f51f2efa742df0 ("ravb: Add support for explicit
internal clock delay configuration").

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds