On 21/07/2017 16:06, Timur Tabi wrote: > On 7/21/17 8:29 AM, Marc Gonzalez wrote: > >> I don't understand what you're saying. >> >> It is a correct observation that the code enabling >> RGMII RX clock delay is a NOP, since that bit will >> always be set at that point. >> >> The spec for the 8035 (I haven't checked for 8030 and 8031, >> is that what you meant by "other systems"?) states that >> Sel_clk125m_dsp, which is described as: >> "Control bit for rgmii interface rx clock delay" >> is 1 after HW reset, 1 after SW reset. >> >> So my statement "RX clock delay is enabled at reset" >> is universally true. It's not just on some systems. > > Ok, taken out of context, the comment doesn't really explain why the > code is the way it is. I'm not really happy about the word "assumes".
If a HW setting defaults to 0 at reset, and some init is called right after reset, then you know the setting's value is 0. If you need that value to be 1, all you need is a function setting it to 1. This is the current situation. Commit 2e5f9f281ee8 assumes 0 at reset, and provides a function setting the value to 1. Reality is different. The HW setting defaults to 1 at reset. So it turns out that the function setting the value to 1 is pointless, because the value is already 1. There is however no way to set the value to 0. Does that explain why I wrote "assume"? Also the commit message: "The current code supports enabling RGMII RX and TX clock delays. The unstated assumption is that these settings are disabled by default at reset, which is not the case." > Maybe you should add a sentence explaining when the code is NOT a no-op. The code is *NEVER* NOT a no-op. I.e. the code enabling RX clock delay is ALWAYS a no-op. I don't understand what you think is unclear in my comment. Regards.
