[PATCH 1/3] fs_enet: update clock names to comply with FEC binding

[PATCH 1/3] fs_enet: update clock names to comply with FEC binding

[Gerhard Sittig]

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 the OF clock lookup to prefer "ipg" over "per", which
improves compliance with the binding, and keeps compatibility
with former device trees

Signed-off-by: Gerhard Sittig <gsi@denx.de>
---
 drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
index 62f042d4aaa9..ce20184b96cb 100644
--- a/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
+++ b/drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
@@ -1037,11 +1037,20 @@ static int fs_enet_probe(struct platform_device *ofdev)
 			fpi->use_rmii = 1;
 	}
 
-	/* make clock lookup non-fatal (the driver is shared among platforms),
+	/* the driver is shared across several PowerPC platforms, not all
+	 * of them provide COMMON_CLK support, and newer kernels are supposed
+	 * to keep working with older DT blobs, so clock lookup is non-fatal
+	 *
 	 * but require enable to succeed when a clock was specified/found,
 	 * keep a reference to the clock upon successful acquisition
+	 *
+	 * the FEC binding is shared with ARM platforms, so we lookup several
+	 * clock names to prefer the common naming convention yet support
+	 * names that were used before unification
 	 */
-	clk = devm_clk_get(&ofdev->dev, "per");
+	clk = devm_clk_get(&ofdev->dev, "ipg");
+	if (IS_ERR(clk))
+		clk = devm_clk_get(&ofdev->dev, "per");
 	if (!IS_ERR(clk)) {
 		err = clk_prepare_enable(clk);
 		if (err) {
-- 
1.7.10.4

[PATCH 2/3] dts: mpc512x: adjust clock specs for FEC nodes

[Gerhard Sittig]

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(-)

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

Re: [PATCH 2/3] dts: mpc512x: adjust clock specs for FEC nodes

[Gerhard Sittig]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

Re: [PATCH 2/3] dts: mpc512x: adjust clock specs for FEC nodes

[Shawn Guo]

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

Re: [PATCH 2/3] dts: mpc512x: adjust clock specs for FEC nodes

[Gerhard Sittig]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

Re: [PATCH 2/3] dts: mpc512x: adjust clock specs for FEC nodes

[Sascha Hauer]

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

-- 
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 |

[PATCH 3/3] dt/bindings: fsl-fec: add "per" to clock properties

[Gerhard Sittig]

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

update the FEC (fast ethernet controller) binding to document the
"per" clock name as an obsolete alias for "ipg"

Signed-off-by: Gerhard Sittig <gsi@denx.de>
---

this patch depends on "dt/bindings: fsl-fec: add clock properties"
by Shawn Guo which introduces the context of this patch

the patch only is necessary if the MPC5121 .dtsi update (switch
FEC nodes from "per" to "ipg") won't make it for v3.14

---
 Documentation/devicetree/bindings/net/fsl-fec.txt |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 468736d4323d..f59b58e29da6 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -24,6 +24,8 @@ Optional properties:
      or external oscillator via pad depending on board design.
    - "enet_out": the phy reference clock provided by SoC via pad, which
      is available on SoC like i.MX28.
+   - "per": obsolete alias for "ipg" for compatibility with early
+     MPC5121 implementations, not recommended for new .dts files
 - clock-names: Must contain the clock names described just above
 
 Example:
-- 
1.7.10.4