Re: [PATCH 19/22] dt/bindings: add micrel,rmii_ref_clk_sel_25_mhz to eth-phy binding
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: 2014-11-13 08:09:38
Also in:
lkml, netdev
On Wed, Nov 12, 2014 at 10:19:20AM +0100, Johan Hovold wrote:
On Wed, Nov 12, 2014 at 08:01:27AM +0100, Sascha Hauer wrote:quoted
On Tue, Nov 11, 2014 at 07:18:25PM +0100, Johan Hovold wrote:quoted
On Tue, Nov 11, 2014 at 05:57:42PM +0000, Mark Rutland wrote:quoted
On Tue, Nov 11, 2014 at 05:37:37PM +0000, Johan Hovold wrote:quoted
Add "micrel,rmii_ref_clk_sel_25_mhz" to Micrel ethernet PHY binding documentation. Cc: devicetree@vger.kernel.org Signed-off-by: Johan Hovold <johan@kernel.org> --- Documentation/devicetree/bindings/net/micrel.txt | 5 +++++ 1 file changed, 5 insertions(+)diff --git a/Documentation/devicetree/bindings/net/micrel.txt b/Documentation/devicetree/bindings/net/micrel.txt index a1bab5eaae02..9b08dd6551dd 100644 --- a/Documentation/devicetree/bindings/net/micrel.txt +++ b/Documentation/devicetree/bindings/net/micrel.txt@@ -19,6 +19,11 @@ Optional properties: See the respective PHY datasheet for the mode values. + - micrel,rmii_ref_clk_sel_25_mhz: rmii_ref_clk_sel bit selects 25 MHz mode + + Whether 25 MHz (rather than 50 Mhz) clock mode is selected + when the rmii_ref_clk_sel bit is set.s/_/-/ in property names please.Ouch, copied from variable name, sorry.quoted
That said, I don't follow the meaning. Does this cause the kernel to do something different, or is is simply that a 25MHz ref clock is wired up?Yes, the driver currently sets this configuration bit based on a common clock binding. However, it turns out the meaning of the bit is reversed on some PHY variants. On most PHYs 50 MHz mode is selected by setting this bit, whereas on the PHYs that need this new property, setting it selects 25 MHz mode instead.Maybe rename the property to something like rmii-ref-clk-25mhz-active-high then? Also you should probably make it more explicit that this is a hardware property and not for adjusting the clock.You're right, but how about calling it micrel,rmii-reference-clock-select-inverted Then no one will set it believing it will change the clock mode and without reading the binding doc first. The description could then read something like micrel,rmii-reference-clock-select-inverted: RMII Reference Clock Select bit is inverted The RMII Reference Clock Select bit is inverted so that setting it selects 25 MHz rather than 50 MHz clock mode. Note that this is only needed for PHY variants that has this bit inverted and that a clock reference ("rmii-ref" below) is always needed to select the actual mode.
"Inverted" only has a meaning when everybody agrees what's the normal case. Since that not the case I really prefer talking about "active-high" or "active-low". Sascha -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |