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