@@ -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.+ - clocks, clock-names: contains clocks according to the common clock bindings. supported clocks:
@@ -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.
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?
Surely that should be described via the common clock bindings? Or if
internal through a clock-frequency property?
Mark.
@@ -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.
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.
Surely that should be described via the common clock bindings? Or if
internal through a clock-frequency property?
The driver currently selects the mode using the common clock bindings,
but this new property is needed to properly handle those PHY variants on
which the clock configuration bit has the reverse meaning.
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
@@ -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.
It's not very nice from Micrel to create Phys with exactly the same
device id but the meaning of this bit reversed.
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 |
@@ -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.
It's not very nice from Micrel to create Phys with exactly the same
device id but the meaning of this bit reversed.
@@ -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 |
@@ -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".
Yes, but I'm reluctant to using "active-high" and "active-low" as this
is not a signal (or IO pin) we're dealing with.
It's a configuration bit, which (if set) selects 25 MHz mode. Hence I
still think something like
micrel,rmii_ref_clk_sel_25_mhz
or
micrel,rmii_reference_clock_select_selects_25_mhz
is preferred. The binding documentation will still make it clear that a
clocks reference is also needed.
Johan