[PATCH (net.git)] stmmac: ptp: fix the reference clock

[PATCH (net.git)] stmmac: ptp: fix the reference clock

[Giuseppe Cavallaro]

The PTP reference clock, used for setting the addend in the Timestamp Addend
Register, was erroneously hard-coded (as reported in the databook just as
example).

The patch removes the macro named: STMMAC_SYSCLOCK and allows to use a
reference clock (clk_ptp_ref_i) that can be passed from the platform.

If not passed, the main driver clock will be used as default; note that
this can be fine on some platforms.

Note that, prior this patch, using the old STMMAC_SYSCLOCK on some platforms,
as side effect, the ptp clock can move faster/slower than the system clock.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
 Documentation/devicetree/bindings/net/stmmac.txt  |    4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |    2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   19 +++++++++++++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h  |    2 --
 4 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 9b03c57..eeed4e30 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -39,6 +39,10 @@ Optional properties:
   further clocks may be specified in derived bindings.
 - clock-names: One name for each entry in the clocks property, the
   first one should be "stmmaceth".
+- clk_ptp_ref: this is the PTP reference clock; in case of the PTP is
+  available this clock is used for programming the Timestamp Addend Register.
+  If not passed then the system clock will be used and this is fine on some
+  platforms.
 
 Examples:
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index ca01035..128a0b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -105,6 +105,8 @@ struct stmmac_priv {
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_clock_ops;
 	unsigned int default_addend;
+	struct clk *clk_ptp_ref;
+	unsigned int clk_ptp_rate;
 	u32 adv_ts;
 	int use_riwt;
 	int irq_wake;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 51a89d4..8dbd9c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -603,16 +603,16 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 		/* calculate default added value:
 		 * formula is :
 		 * addend = (2^32)/freq_div_ratio;
-		 * where, freq_div_ratio = STMMAC_SYSCLOCK/50MHz
-		 * hence, addend = ((2^32) * 50MHz)/STMMAC_SYSCLOCK;
-		 * NOTE: STMMAC_SYSCLOCK should be >= 50MHz to
+		 * where, freq_div_ratio = clk_ptp_ref_i/50MHz
+		 * hence, addend = ((2^32) * 50MHz)/clk_ptp_ref_i;
+		 * NOTE: clk_ptp_ref_i should be >= 50MHz to
 		 *       achive 20ns accuracy.
 		 *
 		 * 2^x * y == (y << x), hence
 		 * 2^32 * 50000000 ==> (50000000 << 32)
 		 */
 		temp = (u64) (50000000ULL << 32);
-		priv->default_addend = div_u64(temp, STMMAC_SYSCLOCK);
+		priv->default_addend = div_u64(temp, priv->clk_ptp_rate);
 		priv->hw->ptp->config_addend(priv->ioaddr,
 					     priv->default_addend);
 
@@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
 	if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
 		return -EOPNOTSUPP;
 
+	/* Fall-back to main clock in case of no PTP ref is passed */
+	priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref");
+	if (IS_ERR(priv->clk_ptp_ref)) {
+		priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk);
+	} else {
+		clk_prepare_enable(priv->clk_ptp_ref);
+		priv->clk_ptp_rate = clk_get_rate(priv->clk_ptp_ref);
+	}
+
 	priv->adv_ts = 0;
 	if (priv->dma_cap.atime_stamp && priv->extend_desc)
 		priv->adv_ts = 1;
@@ -657,6 +666,8 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
 
 static void stmmac_release_ptp(struct stmmac_priv *priv)
 {
+	if (priv->clk_ptp_ref)
+		clk_disable_unprepare(priv->clk_ptp_ref);
 	stmmac_ptp_unregister(priv);
 }
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
index 3dbc047..4535df3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
@@ -25,8 +25,6 @@
 #ifndef __STMMAC_PTP_H__
 #define __STMMAC_PTP_H__
 
-#define STMMAC_SYSCLOCK 62500000
-
 /* IEEE 1588 PTP register offsets */
 #define PTP_TCR		0x0700	/* Timestamp Control Reg */
 #define PTP_SSIR	0x0704	/* Sub-Second Increment Reg */
-- 
1.7.4.4

[PATCH (net.git) (v2)] stmmac: ptp: fix the reference clock

[Giuseppe Cavallaro]

The PTP reference clock, used for setting the addend in the Timestamp Addend
Register, was erroneously hard-coded (as reported in the databook just as
example).

The patch removes the macro named: STMMAC_SYSCLOCK and allows to use a
reference clock (clk_ptp_ref_i) that can be passed from the platform.

If not passed, the main driver clock will be used as default; note that
this can be fine on some platforms.

Note that, prior this patch, using the old STMMAC_SYSCLOCK on some platforms,
as side effect, the ptp clock can move faster/slower than the system clock.

Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
---
v2: fixed the release function in case of the new clock is not provided by the
    platform.

 Documentation/devicetree/bindings/net/stmmac.txt  |    4 ++++
 drivers/net/ethernet/stmicro/stmmac/stmmac.h      |    2 ++
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c |   20 ++++++++++++++++----
 drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h  |    2 --
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 9b03c57..e45ac3f 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -39,6 +39,10 @@ Optional properties:
   further clocks may be specified in derived bindings.
 - clock-names: One name for each entry in the clocks property, the
   first one should be "stmmaceth".
+- clk_ptp_ref: this is the PTP reference clock; in case of the PTP is
+  available this clock is used for programming the Timestamp Addend Register.
+  If not passed then the system clock will be used and this is fine on some
+  platforms.
 
 Examples:
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
index ca01035..128a0b7 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
@@ -105,6 +105,8 @@ struct stmmac_priv {
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_clock_ops;
 	unsigned int default_addend;
+	struct clk *clk_ptp_ref;
+	unsigned int clk_ptp_rate;
 	u32 adv_ts;
 	int use_riwt;
 	int irq_wake;
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 51a89d4..0365289 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -603,16 +603,16 @@ static int stmmac_hwtstamp_ioctl(struct net_device *dev, struct ifreq *ifr)
 		/* calculate default added value:
 		 * formula is :
 		 * addend = (2^32)/freq_div_ratio;
-		 * where, freq_div_ratio = STMMAC_SYSCLOCK/50MHz
-		 * hence, addend = ((2^32) * 50MHz)/STMMAC_SYSCLOCK;
-		 * NOTE: STMMAC_SYSCLOCK should be >= 50MHz to
+		 * where, freq_div_ratio = clk_ptp_ref_i/50MHz
+		 * hence, addend = ((2^32) * 50MHz)/clk_ptp_ref_i;
+		 * NOTE: clk_ptp_ref_i should be >= 50MHz to
 		 *       achive 20ns accuracy.
 		 *
 		 * 2^x * y == (y << x), hence
 		 * 2^32 * 50000000 ==> (50000000 << 32)
 		 */
 		temp = (u64) (50000000ULL << 32);
-		priv->default_addend = div_u64(temp, STMMAC_SYSCLOCK);
+		priv->default_addend = div_u64(temp, priv->clk_ptp_rate);
 		priv->hw->ptp->config_addend(priv->ioaddr,
 					     priv->default_addend);
 
@@ -638,6 +638,16 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
 	if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
 		return -EOPNOTSUPP;
 
+	/* Fall-back to main clock in case of no PTP ref is passed */
+	priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref");
+	if (IS_ERR(priv->clk_ptp_ref)) {
+		priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk);
+		priv->clk_ptp_ref = NULL;
+	} else {
+		clk_prepare_enable(priv->clk_ptp_ref);
+		priv->clk_ptp_rate = clk_get_rate(priv->clk_ptp_ref);
+	}
+
 	priv->adv_ts = 0;
 	if (priv->dma_cap.atime_stamp && priv->extend_desc)
 		priv->adv_ts = 1;
@@ -657,6 +667,8 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
 
 static void stmmac_release_ptp(struct stmmac_priv *priv)
 {
+	if (priv->clk_ptp_ref)
+		clk_disable_unprepare(priv->clk_ptp_ref);
 	stmmac_ptp_unregister(priv);
 }
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
index 3dbc047..4535df3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ptp.h
@@ -25,8 +25,6 @@
 #ifndef __STMMAC_PTP_H__
 #define __STMMAC_PTP_H__
 
-#define STMMAC_SYSCLOCK 62500000
-
 /* IEEE 1588 PTP register offsets */
 #define PTP_TCR		0x0700	/* Timestamp Control Reg */
 #define PTP_SSIR	0x0704	/* Sub-Second Increment Reg */
-- 
1.7.4.4

Re: [PATCH (net.git) (v2)] stmmac: ptp: fix the reference clock

[David Miller]

From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Wed, 27 Aug 2014 10:37:49 +0200

> The PTP reference clock, used for setting the addend in the Timestamp Addend
> Register, was erroneously hard-coded (as reported in the databook just as
> example).
> 
> The patch removes the macro named: STMMAC_SYSCLOCK and allows to use a
> reference clock (clk_ptp_ref_i) that can be passed from the platform.
> 
> If not passed, the main driver clock will be used as default; note that
> this can be fine on some platforms.
> 
> Note that, prior this patch, using the old STMMAC_SYSCLOCK on some platforms,
> as side effect, the ptp clock can move faster/slower than the system clock.
> 
> Signed-off-by: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> ---
> v2: fixed the release function in case of the new clock is not provided by the
>     platform.

Applied.

Re: [PATCH (net.git)] stmmac: ptp: fix the reference clock

[David Miller]

From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Date: Tue, 26 Aug 2014 08:04:32 +0200

> @@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
>  	if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
>  		return -EOPNOTSUPP;
>  
> +	/* Fall-back to main clock in case of no PTP ref is passed */
> +	priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref");
> +	if (IS_ERR(priv->clk_ptp_ref)) {
> +		priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk);

This code path will leave an IS_ERR() value in clk_ptp_ref.
 ...
>  static void stmmac_release_ptp(struct stmmac_priv *priv)
>  {
> +	if (priv->clk_ptp_ref)
> +		clk_disable_unprepare(priv->clk_ptp_ref);
>  	stmmac_ptp_unregister(priv);
>  }

Which this code will erroneously pass into clk_disable_unprepare().

Please NULL the pointer out properl when devm_clk_get() errors.

Thanks.

Re: [PATCH (net.git)] stmmac: ptp: fix the reference clock

[Giuseppe CAVALLARO]

Hi David

On 8/28/2014 1:26 AM, David Miller wrote:
> From: Giuseppe Cavallaro <peppe.cavallaro@st.com>
> Date: Tue, 26 Aug 2014 08:04:32 +0200
>
>> @@ -638,6 +638,15 @@ static int stmmac_init_ptp(struct stmmac_priv *priv)
>>   	if (!(priv->dma_cap.time_stamp || priv->dma_cap.atime_stamp))
>>   		return -EOPNOTSUPP;
>>
>> +	/* Fall-back to main clock in case of no PTP ref is passed */
>> +	priv->clk_ptp_ref = devm_clk_get(priv->device, "clk_ptp_ref");
>> +	if (IS_ERR(priv->clk_ptp_ref)) {
>> +		priv->clk_ptp_rate = clk_get_rate(priv->stmmac_clk);
>
> This code path will leave an IS_ERR() value in clk_ptp_ref.
>   ...
>>   static void stmmac_release_ptp(struct stmmac_priv *priv)
>>   {
>> +	if (priv->clk_ptp_ref)
>> +		clk_disable_unprepare(priv->clk_ptp_ref);
>>   	stmmac_ptp_unregister(priv);
>>   }
>
> Which this code will erroneously pass into clk_disable_unprepare().
>
> Please NULL the pointer out properl when devm_clk_get() errors.

I had sent the v2, let me know if it's ok for you.

peppe

>
> Thanks.
>