On Sun, Dec 18, 2016 at 4:49 PM, David Miller <davem@xxxxxxxxxxxxx> wrote: > From: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> > Date: Sat, 17 Dec 2016 19:21:19 +0100 > >> Prior to this patch we were using a hardcoded RGMII TX clock delay of >> 2ns (= 1/4 cycle of the 125MHz RGMII TX clock). This value works for >> many boards, but unfortunately not for all (due to the way the actual >> circuit is designed, sometimes because the TX delay is enabled in the >> PHY, etc.). Making the TX delay on the MAC side configurable allows us >> to support all possible hardware combinations. >> >> This allows fixing a compatibility issue on some boards, where the >> RTL8211F PHY is configured to generate the TX delay. We can now turn >> off the TX delay in the MAC, because otherwise we would be applying the >> delay twice (which results in non-working TX traffic). >> >> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> >> Tested-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx> > > Is this really the safest thing to do? > > If you say the existing hard-coded setting of 1/4 cycle works on most > boards, and what you're trying to do is override it with an OF > property value for boards where the existing setting does not work, > then you _must_ use a default value that corresponds to what the > existing code does not when you don't see this new OF property. it's a bit more complicated in reality: 1/4 cycle works when the TX delay of the RTL8211F PHY is turned off (until recently it was always enabled for phy-mode RGMII). > So please retain the current behavior of the 1/4 cycle TX delay > setting when you don't see the amlogic,tx-delay-ns property. > > I really think you risk breaking existing boards by not doing so, > unless you can have this patch tested on every such board that exists > and I don't think you really can feasibly and rigorously do that. there's a patch in my follow-up series which adds the 2ns to the .dts for all RGMII based boards: [0] (and I would keep these even if we had a default value, just to make it explicit and thus easier to understand for other people). however, we can add the 2ns default back (I can do this if you want - Rob Herring was unhappy with the missing documentation of this default value [1] - so note to myself: take care of that as well). but then we have to decide when to apply this default value: only when we're in RGMII mode or also in any of the RGMII_*ID modes? please let me know how we should proceed Regards, Martin [0] http://lists.infradead.org/pipermail/linux-amlogic/2016-December/001838.html [1] http://lists.infradead.org/pipermail/linux-amlogic/2016-November/001817.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
