netdev - Re: [PATCH] Add support for running the Marvell m88e1111 PHY in RGMII mode

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: <20070411010158.GB9526@xi.wantstofly.org>
Date:	Wed, 11 Apr 2007 03:01:58 +0200
From:	Lennert Buytenhek <buytenh@...tstofly.org>
To:	Kim Phillips <kim.phillips@...escale.com>
Cc:	netdev@...r.kernel.org, Jeff Garzik <jgarzik@...ox.com>
Subject: Re: [PATCH] Add support for running the Marvell m88e1111 PHY in RGMII mode

On Tue, Apr 10, 2007 at 04:57:23PM -0500, Kim Phillips wrote:

> also adds RX & TX delay bits to help boards with clock skew problems.

This doesn't make sense at all.

RGMII specifies that clock and data are generated simultaneously.  The
necessary 1.5-2ns of clock delay is either achieved by routing the RGMII
clock net to have that amount of skew, or by programming the RGMII
receiver to add that amount of clock delay internally.

So, boards that have 'clock skew problems' should have RX & TX delay
turned OFF.  Boards that route the clock nets without any skew should
have RX & TX delay turned on.

Your patch unconditionally enables clock delay in the 88e1111, which
only makes sense if the RGMII data and clock nets are routed to have
the approximate same amount of delay.  Which means that it is entirely
board-specific whether this should be enabled or not.  If you enable
the 88e1111 internal RX/TX clock delay feature on a board that also
routes the RGMII clock nets to have the appropriate amount of skew,
things are likely to break entirely.

> [...]
> +
> +		temp |= (MII_M1111_RX_DELAY | MII_M1111_TX_DELAY);

Enabling this unconditionally is just wrong.

I.e. the patch is wrong, and the description is wrong too.
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html