[PATCH v2] dt/bindings: fsl-fec: add clock properties

[PATCH v2] dt/bindings: fsl-fec: add clock properties

[Shawn Guo]

Update fsl-fec.txt to add 'clocks' and 'clock-names' properties.

Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Changes since v1:
 - Leave compatible change out, which should probably be addressed by
   another patch
 - Move clock properties into 'Optional properties:' section

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

diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
index 845ff84..468736d 100644
--- a/Documentation/devicetree/bindings/net/fsl-fec.txt
+++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
@@ -16,6 +16,15 @@ Optional properties:
   will have the duration be 1 millisecond.  Numbers greater than 1000 are
   invalid and 1 millisecond will be used instead.
 - phy-supply: regulator that powers the Ethernet PHY.
+- clocks: the clocks feeding the FEC controller and phy.
+   - "ipg": the peripheral access clock
+   - "ahb": the bus clock for MAC
+   - "ptp": the sampling clock for PTP (IEEE 1588).  On SoC like i.MX6Q,
+     the clock could come from either the internal clock control module
+     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.
+- clock-names: Must contain the clock names described just above
 
 Example:
 
-- 
1.7.9.5


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

Re: [PATCH v2] dt/bindings: fsl-fec: add clock properties

[Sascha Hauer]

On Mon, Feb 24, 2014 at 09:48:10AM +0800, Shawn Guo wrote:
> Update fsl-fec.txt to add 'clocks' and 'clock-names' properties.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Changes since v1:
>  - Leave compatible change out, which should probably be addressed by
>    another patch
>  - Move clock properties into 'Optional properties:' section

As mentioned in the reply to v1: Marking the clocks as optional doesn't
make sense when the driver actually requires them (Or be prepared when
somebody sends in a patch 'fixing' the driver according to the binding).
IMO we should just mention the clocks are optional on PowerPC.

Sascha

> 
>  Documentation/devicetree/bindings/net/fsl-fec.txt |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index 845ff84..468736d 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -16,6 +16,15 @@ Optional properties:
>    will have the duration be 1 millisecond.  Numbers greater than 1000 are
>    invalid and 1 millisecond will be used instead.
>  - phy-supply: regulator that powers the Ethernet PHY.
> +- clocks: the clocks feeding the FEC controller and phy.
> +   - "ipg": the peripheral access clock
> +   - "ahb": the bus clock for MAC
> +   - "ptp": the sampling clock for PTP (IEEE 1588).  On SoC like i.MX6Q,
> +     the clock could come from either the internal clock control module
> +     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.
> +- clock-names: Must contain the clock names described just above
>  
>  Example:
>  
> -- 
> 1.7.9.5
> 
> 
> --
> 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
> 

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

Re: [PATCH v2] dt/bindings: fsl-fec: add clock properties

[Shawn Guo]

On Mon, Feb 24, 2014 at 08:53:37AM +0100, Sascha Hauer wrote:
> On Mon, Feb 24, 2014 at 09:48:10AM +0800, Shawn Guo wrote:
> > Update fsl-fec.txt to add 'clocks' and 'clock-names' properties.
> > 
> > Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > ---
> > Changes since v1:
> >  - Leave compatible change out, which should probably be addressed by
> >    another patch
> >  - Move clock properties into 'Optional properties:' section
> 
> As mentioned in the reply to v1: Marking the clocks as optional doesn't
> make sense when the driver actually requires them (Or be prepared when
> somebody sends in a patch 'fixing' the driver according to the binding).
> IMO we should just mention the clocks are optional on PowerPC.

Okay, the whole thing becomes quite uncertain to me.  The reason of this
v2 is that I got reminded by Gerhard's comment that we've had a time the
driver works with DT without these properties.  At that time, the clock
driver provides the clock lookups for the device by calling
clk_register_clkdev().  But on the other hand, I agree with you that it
does not make sense to binding users, because the IP requires these
clock inputs when it gets integrated on a SoC, and the driver requires
these clocks to work.

Shawn

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

Re: [PATCH v2] dt/bindings: fsl-fec: add clock properties

[Gerhard Sittig]

On Mon, Feb 24, 2014 at 09:48 +0800, Shawn Guo wrote:
> 
> Update fsl-fec.txt to add 'clocks' and 'clock-names' properties.
> 
> Signed-off-by: Shawn Guo <shawn.guo-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Changes since v1:
>  - Leave compatible change out, which should probably be addressed by
>    another patch
>  - Move clock properties into 'Optional properties:' section
> 
>  Documentation/devicetree/bindings/net/fsl-fec.txt |    9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/fsl-fec.txt b/Documentation/devicetree/bindings/net/fsl-fec.txt
> index 845ff84..468736d 100644
> --- a/Documentation/devicetree/bindings/net/fsl-fec.txt
> +++ b/Documentation/devicetree/bindings/net/fsl-fec.txt
> @@ -16,6 +16,15 @@ Optional properties:
>    will have the duration be 1 millisecond.  Numbers greater than 1000 are
>    invalid and 1 millisecond will be used instead.
>  - phy-supply: regulator that powers the Ethernet PHY.
> +- clocks: the clocks feeding the FEC controller and phy.
> +   - "ipg": the peripheral access clock
> +   - "ahb": the bus clock for MAC
> +   - "ptp": the sampling clock for PTP (IEEE 1588).  On SoC like i.MX6Q,
> +     the clock could come from either the internal clock control module
> +     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.
> +- clock-names: Must contain the clock names described just above
>  
>  Example:

I can agree with that change.  It improves the situation for i.MX
(provides missing information for ARM), and doesn't break MPC,
while I'm not aware of other FEC (fast ethernet controller) uses.


For MPC though, I will have to provide a followup.

Starting with v3.12 (2771399a "fs_enet: cleanup clock API use")
the Linux drivers/net/ethernet/freescale/fs_enet/fs_enet-main.c
implementation looks up the "per" clock, while failure isn't
fatal.  I will change this to lookup "ipg" first and fallback to
"per" then.  Up to and including v3.13 no PowerPC .dts file
provides clock specs for FEC nodes, so lookup always fails.  I'm
guilty of not having provided a bindings update back then.

Starting with v3.14-rc1 the MPC5121/23/25 platform does provide
proper COMMON_CLK support, but comes with full backwards compat
support, too.  So failure to lookup the clock spec still isn't
fatal, the code works with both updated as well as pre-existing
device trees.  And it will keep working across "incompatible"
changes in the .dts, switching from "per" to "ipg" names for the
FEC nodes.

So there are three changes that I will have to prepare:
- documenting "per" as an (obsolete) alias for "ipg" in the
  binding
- looking up "ipg" before "per" in one of the FEC drivers
- and switching from "per" to "ipg" in MPC .dts files

As outlined above, none of these changes depends on a specific
order, nothing needs to get rushed into v3.14, although it would
be nice to start shipping with a consistent state.


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-ynQEQJNshbs@public.gmane.org
--
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

Re: [PATCH v2] dt/bindings: fsl-fec: add clock properties

[Gerhard Sittig]

On Mon, Feb 24, 2014 at 09:51 +0100, Gerhard Sittig wrote:
> 
> For MPC though, I will have to provide a followup.
> 
> [ ... ]
> 
> So there are three changes that I will have to prepare:
> - documenting "per" as an (obsolete) alias for "ipg" in the
>   binding
> - looking up "ipg" before "per" in one of the FEC drivers
> - and switching from "per" to "ipg" in MPC .dts files
> 
> As outlined above, none of these changes depends on a specific
> order, nothing needs to get rushed into v3.14, although it would
> be nice to start shipping with a consistent state.

For the record, I have sent a three patch series starting in
<1393237557-31406-2-git-send-email-gsi-ynQEQJNshbs@public.gmane.org>, which brings the
MPC5121 implementation in line with what i.MX does and what the
updated binding document specifies.


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-ynQEQJNshbs@public.gmane.org
--
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