netdev - Re: [PATCH] net: renesas: Fix rgmii-id delays

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdXiMhpU0vDV3KaOg4DY59cszAtoG1sDOgnTRY6C6cyitQ@mail.gmail.com>
Date:   Tue, 19 Oct 2021 22:05:41 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Cc:     Andrew Lunn <andrew@...n.ch>,
        Köry Maincent <kory.maincent@...tlin.com>,
        netdev <netdev@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Sergey Shtylyov <s.shtylyov@....ru>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>,
        Biju Das <biju.das.jz@...renesas.com>,
        Sergei Shtylyov <sergei.shtylyov@...il.com>,
        Adam Ford <aford173@...il.com>,
        Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
        Yang Yingliang <yangyingliang@...wei.com>
Subject: Re: [PATCH] net: renesas: Fix rgmii-id delays

Hi Thomas,

On Tue, Oct 19, 2021 at 5:57 PM Thomas Petazzoni
<thomas.petazzoni@...tlin.com> wrote:
> On Tue, 19 Oct 2021 17:41:49 +0200
> Andrew Lunn <andrew@...n.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@...ux-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