| Message ID | 1393237557-31406-2-git-send-email-gsi@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Feb 24, 2014 at 11:25 +0100, Gerhard Sittig wrote: > > a recent FEC binding document update that was motivated by i.MX > development revealed that ARM and PowerPC implementations in Linux > did not agree on the clock names to use for the FEC nodes > > change clock names from "per" to "ipg" in the FEC nodes of the > mpc5121.dtsi include file such that the .dts specs comply with > the common FEC binding > > this "incompatible" change does not break operation, because > - COMMON_CLK support for MPC5121/23/25 and adjusted .dts files > were only introduced in Linux v3.14-rc1, no mainline release > provided these specs before > - if this change won't make it for v3.14, the MPC512x CCF support > provides full backwards compability, and keeps operating with > device trees which lack clock specs or don't match in the names > > Signed-off-by: Gerhard Sittig <gsi@denx.de> ping Are there opinions about making PowerPC users of FEC use the same clock names as ARM users do, to re-use (actually: keep sharing) the FEC binding? The alternative would be to fragment the FEC binding into several bindings for ARM and PowerPC, which I feel would be undesirable, and is not necessary. It would be nice to ship v3.14 (the first release with proper CCF support for MPC512x) in a shape that is consistent with other FEC users (ARM i.MX). We are lucky to have noticed the difference before the release, and need not introduce the incompatibility, and need not document and support obsolete conditions but can fix them instead. > --- > arch/powerpc/boot/dts/mpc5121.dtsi | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi > index 2c0e1552d20b..a5a375598ed8 100644 > --- a/arch/powerpc/boot/dts/mpc5121.dtsi > +++ b/arch/powerpc/boot/dts/mpc5121.dtsi > @@ -281,7 +281,7 @@ > #address-cells = <1>; > #size-cells = <0>; > clocks = <&clks MPC512x_CLK_FEC>; > - clock-names = "per"; > + clock-names = "ipg"; > }; > > eth0: ethernet@2800 { > @@ -291,7 +291,7 @@ > local-mac-address = [ 00 00 00 00 00 00 ]; > interrupts = <4 0x8>; > clocks = <&clks MPC512x_CLK_FEC>; > - clock-names = "per"; > + clock-names = "ipg"; > }; > > /* USB1 using external ULPI PHY */ > -- > 1.7.10.4 virtually yours Gerhard Sittig
On Mon, Mar 03, 2014 at 10:22:31AM +0100, Gerhard Sittig wrote: > On Mon, Feb 24, 2014 at 11:25 +0100, Gerhard Sittig wrote: > > > > a recent FEC binding document update that was motivated by i.MX > > development revealed that ARM and PowerPC implementations in Linux > > did not agree on the clock names to use for the FEC nodes > > > > change clock names from "per" to "ipg" in the FEC nodes of the > > mpc5121.dtsi include file such that the .dts specs comply with > > the common FEC binding > > > > this "incompatible" change does not break operation, because > > - COMMON_CLK support for MPC5121/23/25 and adjusted .dts files > > were only introduced in Linux v3.14-rc1, no mainline release > > provided these specs before > > - if this change won't make it for v3.14, the MPC512x CCF support > > provides full backwards compability, and keeps operating with > > device trees which lack clock specs or don't match in the names > > > > Signed-off-by: Gerhard Sittig <gsi@denx.de> > > ping > > Are there opinions about making PowerPC users of FEC use the same > clock names as ARM users do, to re-use (actually: keep sharing) > the FEC binding? The alternative would be to fragment the FEC > binding into several bindings for ARM and PowerPC, which I feel > would be undesirable, and is not necessary. As I already said, Documentation/devicetree/bindings/net/fsl-fec.txt was created specifically for i.MX FEC controller from day one. And even as of today, it doesn't serve PowerPC, because for example the property 'phy-mode' documented as required one is not required by PowerPC FEC. My opinion would be to patch fsl-fec.txt a little bit to make it clear that it's a binding doc for i.MX FEC, and create the other one for PowerPC FEC. This is the way less confusing to people and easier for binding maintenance. Shawn
On Wed, Mar 05, 2014 at 09:48 +0800, Shawn Guo wrote: > > On Mon, Mar 03, 2014 at 10:22:31AM +0100, Gerhard Sittig wrote: > > On Mon, Feb 24, 2014 at 11:25 +0100, Gerhard Sittig wrote: > > > > > > a recent FEC binding document update that was motivated by i.MX > > > development revealed that ARM and PowerPC implementations in Linux > > > did not agree on the clock names to use for the FEC nodes > > > > > > change clock names from "per" to "ipg" in the FEC nodes of the > > > mpc5121.dtsi include file such that the .dts specs comply with > > > the common FEC binding > > > > > > this "incompatible" change does not break operation, because > > > - COMMON_CLK support for MPC5121/23/25 and adjusted .dts files > > > were only introduced in Linux v3.14-rc1, no mainline release > > > provided these specs before > > > - if this change won't make it for v3.14, the MPC512x CCF support > > > provides full backwards compability, and keeps operating with > > > device trees which lack clock specs or don't match in the names > > > > > > Signed-off-by: Gerhard Sittig <gsi@denx.de> > > > > ping > > > > Are there opinions about making PowerPC users of FEC use the same > > clock names as ARM users do, to re-use (actually: keep sharing) > > the FEC binding? The alternative would be to fragment the FEC > > binding into several bindings for ARM and PowerPC, which I feel > > would be undesirable, and is not necessary. > > As I already said, Documentation/devicetree/bindings/net/fsl-fec.txt > was created specifically for i.MX FEC controller from day one. And even > as of today, it doesn't serve PowerPC, because for example the property > 'phy-mode' documented as required one is not required by PowerPC FEC. > My opinion would be to patch fsl-fec.txt a little bit to make it clear > that it's a binding doc for i.MX FEC, and create the other one for > PowerPC FEC. This is the way less confusing to people and easier for > binding maintenance. Should we still try to have a common behaviour where possible? Such that even if there are two bindings, they don't diverge in "unnecessary" ways? But given that we already are past -rc5, I guess the suggested change is too late for v3.14 anyway. So we have to live with the fact of a mainline release of different behaviour. And the backwards compatibility support in the MPC512x CCF implementation allows to cope with a potential future "ipg" unification while still working with former "per" using device trees. There's no blocker. So nevermind. virtually yours Gerhard Sittig
On Wed, Mar 05, 2014 at 11:52:09AM +0100, Gerhard Sittig wrote: > On Wed, Mar 05, 2014 at 09:48 +0800, Shawn Guo wrote: > > > > On Mon, Mar 03, 2014 at 10:22:31AM +0100, Gerhard Sittig wrote: > > > On Mon, Feb 24, 2014 at 11:25 +0100, Gerhard Sittig wrote: > > > > > > > > a recent FEC binding document update that was motivated by i.MX > > > > development revealed that ARM and PowerPC implementations in Linux > > > > did not agree on the clock names to use for the FEC nodes > > > > > > > > change clock names from "per" to "ipg" in the FEC nodes of the > > > > mpc5121.dtsi include file such that the .dts specs comply with > > > > the common FEC binding > > > > > > > > this "incompatible" change does not break operation, because > > > > - COMMON_CLK support for MPC5121/23/25 and adjusted .dts files > > > > were only introduced in Linux v3.14-rc1, no mainline release > > > > provided these specs before > > > > - if this change won't make it for v3.14, the MPC512x CCF support > > > > provides full backwards compability, and keeps operating with > > > > device trees which lack clock specs or don't match in the names > > > > > > > > Signed-off-by: Gerhard Sittig <gsi@denx.de> > > > > > > ping > > > > > > Are there opinions about making PowerPC users of FEC use the same > > > clock names as ARM users do, to re-use (actually: keep sharing) > > > the FEC binding? The alternative would be to fragment the FEC > > > binding into several bindings for ARM and PowerPC, which I feel > > > would be undesirable, and is not necessary. > > > > As I already said, Documentation/devicetree/bindings/net/fsl-fec.txt > > was created specifically for i.MX FEC controller from day one. And even > > as of today, it doesn't serve PowerPC, because for example the property > > 'phy-mode' documented as required one is not required by PowerPC FEC. > > My opinion would be to patch fsl-fec.txt a little bit to make it clear > > that it's a binding doc for i.MX FEC, and create the other one for > > PowerPC FEC. This is the way less confusing to people and easier for > > binding maintenance. > > Should we still try to have a common behaviour where possible? > Such that even if there are two bindings, they don't diverge in > "unnecessary" ways? Maybe the long term goal should be to share the code. The MPC5200 FEC and the i.MX FEC are very similar. Only the DMA engine is quite different. Sascha
diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi index 2c0e1552d20b..a5a375598ed8 100644 --- a/arch/powerpc/boot/dts/mpc5121.dtsi +++ b/arch/powerpc/boot/dts/mpc5121.dtsi @@ -281,7 +281,7 @@ #address-cells = <1>; #size-cells = <0>; clocks = <&clks MPC512x_CLK_FEC>; - clock-names = "per"; + clock-names = "ipg"; }; eth0: ethernet@2800 { @@ -291,7 +291,7 @@ local-mac-address = [ 00 00 00 00 00 00 ]; interrupts = <4 0x8>; clocks = <&clks MPC512x_CLK_FEC>; - clock-names = "per"; + clock-names = "ipg"; }; /* USB1 using external ULPI PHY */
a recent FEC binding document update that was motivated by i.MX development revealed that ARM and PowerPC implementations in Linux did not agree on the clock names to use for the FEC nodes change clock names from "per" to "ipg" in the FEC nodes of the mpc5121.dtsi include file such that the .dts specs comply with the common FEC binding this "incompatible" change does not break operation, because - COMMON_CLK support for MPC5121/23/25 and adjusted .dts files were only introduced in Linux v3.14-rc1, no mainline release provided these specs before - if this change won't make it for v3.14, the MPC512x CCF support provides full backwards compability, and keeps operating with device trees which lack clock specs or don't match in the names Signed-off-by: Gerhard Sittig <gsi@denx.de> --- arch/powerpc/boot/dts/mpc5121.dtsi | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)