[PATCH 0/6] Add CPTS support for AM437x

[PATCH 0/6] Add CPTS support for AM437x

[George Cherian]

The series adds CPTS support for AM4372.

Patch 1 - CPTS clock name harcoding in the driver is removed. 
	  Easier to pass the clock name from dt rather than hardcoding in driver.
	  Also in prepration for DRA7x CPTS support.
Patch 2 - DT changes w.r.t clock changes for AM33xx.
Patch 3 - Enable the CPTS support for both DRA7x and AM4372 in the driver.
Patch 4 - Enable the Annexe F for L2 PTP for AM437x and DRA7x.
Patch 5 - Change the default clocksource to dpll_core_m5 
Patch 6 - DT changes for AM4372.
 
George Cherian (6):
  drivers: net: cpts: Remove hardcoded clock name for CPTS
  ARM: dts: am33xx: Add clock names for cpsw and cpts
  drivers: net: cpsw: Enable CPTS for DRA7xx and AM4372
  drivers: net: cpsw: Enable Annexe F Time sync
  ARM: AM43xx: clk: Change the cpts ref clock source to dpll_core_m5 clk
  ARM: dts: am4372: Add clock names for cpsw and cpts

 arch/arm/boot/dts/am33xx.dtsi  |  2 ++
 arch/arm/boot/dts/am4372.dtsi  |  2 ++
 drivers/clk/ti/clk-43xx.c      | 16 ++++++++++++++++
 drivers/net/ethernet/ti/cpsw.c | 15 ++++++++++-----
 drivers/net/ethernet/ti/cpts.c | 11 ++++-------
 5 files changed, 34 insertions(+), 12 deletions(-)

-- 
1.8.3.1

[PATCH 1/6] drivers: net: cpts: Remove hardcoded clock name for CPTS

[George Cherian]

CPTS refclk name is hardcoded, which makes it fail in case of DRA7x
Remove the hardcoded clock name for CPTS refclk and get the same from DT.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/net/ethernet/ti/cpts.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
index 2435139..0b6f6f7 100644
--- a/drivers/net/ethernet/ti/cpts.c
+++ b/drivers/net/ethernet/ti/cpts.c
@@ -236,13 +236,11 @@ static void cpts_overflow_check(struct work_struct *work)
 	schedule_delayed_work(&cpts->overflow_work, CPTS_OVERFLOW_PERIOD);
 }
 
-#define CPTS_REF_CLOCK_NAME "cpsw_cpts_rft_clk"
-
-static void cpts_clk_init(struct cpts *cpts)
+static void cpts_clk_init(struct device *dev, struct cpts *cpts)
 {
-	cpts->refclk = clk_get(NULL, CPTS_REF_CLOCK_NAME);
+	cpts->refclk = devm_clk_get(dev, "cpts");
 	if (IS_ERR(cpts->refclk)) {
-		pr_err("Failed to clk_get %s\n", CPTS_REF_CLOCK_NAME);
+		pr_err("Failed to get cpts refclk\n");
 		cpts->refclk = NULL;
 		return;
 	}
@@ -252,7 +250,6 @@ static void cpts_clk_init(struct cpts *cpts)
 static void cpts_clk_release(struct cpts *cpts)
 {
 	clk_disable(cpts->refclk);
-	clk_put(cpts->refclk);
 }
 
 static int cpts_match(struct sk_buff *skb, unsigned int ptp_class,
@@ -390,7 +387,7 @@ int cpts_register(struct device *dev, struct cpts *cpts,
 	for (i = 0; i < CPTS_MAX_EVENTS; i++)
 		list_add(&cpts->pool_data[i].list, &cpts->pool);
 
-	cpts_clk_init(cpts);
+	cpts_clk_init(dev, cpts);
 	cpts_write32(cpts, CPTS_EN, control);
 	cpts_write32(cpts, TS_PEND_EN, int_enable);
 
-- 
1.8.3.1

Re: [PATCH 1/6] drivers: net: cpts: Remove hardcoded clock name for CPTS

[Richard Cochran]

On Mon, Apr 28, 2014 at 09:40:20AM +0530, George Cherian wrote:
> CPTS refclk name is hardcoded, which makes it fail in case of DRA7x
> Remove the hardcoded clock name for CPTS refclk and get the same from DT.

Patch ordering - doesn't this patch depend on patch #2?

Thanks,
Richard
--
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 1/6] drivers: net: cpts: Remove hardcoded clock name for CPTS

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

On Mon, Apr 28, 2014 at 09:40:20AM +0530, George Cherian wrote:
> CPTS refclk name is hardcoded, which makes it fail in case of DRA7x
> Remove the hardcoded clock name for CPTS refclk and get the same from DT.
> 
> Signed-off-by: George Cherian <george.cherian@ti.com>
> ---
>  drivers/net/ethernet/ti/cpts.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 2435139..0b6f6f7 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -236,13 +236,11 @@ static void cpts_overflow_check(struct work_struct *work)
>  	schedule_delayed_work(&cpts->overflow_work, CPTS_OVERFLOW_PERIOD);
>  }
>  
> -#define CPTS_REF_CLOCK_NAME "cpsw_cpts_rft_clk"
> -
> -static void cpts_clk_init(struct cpts *cpts)
> +static void cpts_clk_init(struct device *dev, struct cpts *cpts)
>  {
> -	cpts->refclk = clk_get(NULL, CPTS_REF_CLOCK_NAME);
> +	cpts->refclk = devm_clk_get(dev, "cpts");
>  	if (IS_ERR(cpts->refclk)) {
> -		pr_err("Failed to clk_get %s\n", CPTS_REF_CLOCK_NAME);
> +		pr_err("Failed to get cpts refclk\n");

now you have a dev pointer as argument, how about converting this to
dev_err() ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/6] drivers: net: cpts: Remove hardcoded clock name for CPTS

[George Cherian]

On 4/28/2014 8:01 PM, Felipe Balbi wrote:
> On Mon, Apr 28, 2014 at 09:40:20AM +0530, George Cherian wrote:
>> CPTS refclk name is hardcoded, which makes it fail in case of DRA7x
>> Remove the hardcoded clock name for CPTS refclk and get the same from DT.
>>
>> Signed-off-by: George Cherian <george.cherian@ti.com>
>> ---
>>   drivers/net/ethernet/ti/cpts.c | 11 ++++-------
>>   1 file changed, 4 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
>> index 2435139..0b6f6f7 100644
>> --- a/drivers/net/ethernet/ti/cpts.c
>> +++ b/drivers/net/ethernet/ti/cpts.c
>> @@ -236,13 +236,11 @@ static void cpts_overflow_check(struct work_struct *work)
>>   	schedule_delayed_work(&cpts->overflow_work, CPTS_OVERFLOW_PERIOD);
>>   }
>>   
>> -#define CPTS_REF_CLOCK_NAME "cpsw_cpts_rft_clk"
>> -
>> -static void cpts_clk_init(struct cpts *cpts)
>> +static void cpts_clk_init(struct device *dev, struct cpts *cpts)
>>   {
>> -	cpts->refclk = clk_get(NULL, CPTS_REF_CLOCK_NAME);
>> +	cpts->refclk = devm_clk_get(dev, "cpts");
>>   	if (IS_ERR(cpts->refclk)) {
>> -		pr_err("Failed to clk_get %s\n", CPTS_REF_CLOCK_NAME);
>> +		pr_err("Failed to get cpts refclk\n");
> now you have a dev pointer as argument, how about converting this to
> dev_err() ?
Yep will do.. in next version.


-- 
-George

[PATCH 2/6] ARM: dts: am33xx: Add clock names for cpsw and cpts

[George Cherian]

Add CPSW fck and CPTS clock and clock names

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index 9770e35..d1e2b36 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -665,6 +665,8 @@
 		mac: ethernet@4a100000 {
 			compatible = "ti,cpsw";
 			ti,hwmods = "cpgmac0";
+			clocks = <&cpsw_125mhz_gclk>, <&cpsw_cpts_rft_clk>;
+			clock-names = "fck", "cpts";
 			cpdma_channels = <8>;
 			ale_entries = <1024>;
 			bd_ram_size = <0x2000>;
-- 
1.8.3.1

[PATCH 3/6] drivers: net: cpsw: Enable CPTS for DRA7xx and AM4372

[George Cherian]

Enable cpts hardware time stamping for Dra7xx and AM4372.
This enables PTPv2 for DRA7xx and AM4372.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 36aa109..085ffb5 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -1398,7 +1398,8 @@ static int cpsw_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
 	struct hwtstamp_config cfg;
 
 	if (priv->version != CPSW_VERSION_1 &&
-	    priv->version != CPSW_VERSION_2)
+	    priv->version != CPSW_VERSION_2 &&
+	    priv->version != CPSW_VERSION_3)
 		return -EOPNOTSUPP;
 
 	if (copy_from_user(&cfg, ifr->ifr_data, sizeof(cfg)))
@@ -1443,6 +1444,7 @@ static int cpsw_hwtstamp_set(struct net_device *dev, struct ifreq *ifr)
 		cpsw_hwtstamp_v1(priv);
 		break;
 	case CPSW_VERSION_2:
+	case CPSW_VERSION_3:
 		cpsw_hwtstamp_v2(priv);
 		break;
 	default:
@@ -1459,7 +1461,8 @@ static int cpsw_hwtstamp_get(struct net_device *dev, struct ifreq *ifr)
 	struct hwtstamp_config cfg;
 
 	if (priv->version != CPSW_VERSION_1 &&
-	    priv->version != CPSW_VERSION_2)
+	    priv->version != CPSW_VERSION_2 &&
+	    priv->version != CPSW_VERSION_3)
 		return -EOPNOTSUPP;
 
 	cfg.flags = 0;
-- 
1.8.3.1

[PATCH 4/6] drivers: net: cpsw: Enable Annexe F Time sync

[George Cherian]

Enable the Annex F Time Sync explicitly for DRA7x and AM4372.
With this enabled the L2 PTP is working.

while at that rename TS_BIT8 to TS_TTL_NONZERO

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/net/ethernet/ti/cpsw.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 085ffb5..af1423b 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -248,7 +248,8 @@ struct cpsw_ss_regs {
 #define TS_131              (1<<11) /* Time Sync Dest IP Addr 131 enable */
 #define TS_130              (1<<10) /* Time Sync Dest IP Addr 130 enable */
 #define TS_129              (1<<9)  /* Time Sync Dest IP Addr 129 enable */
-#define TS_BIT8             (1<<8)  /* ts_ttl_nonzero? */
+#define TS_TTL_NONZERO      (1<<8)  /* Time Sync Time To Live Non-zero enable */
+#define TS_ANNEX_F_EN       (1<<6)  /* Time Sync Annex F enable */
 #define TS_ANNEX_D_EN       (1<<4)  /* Time Sync Annex D enable */
 #define TS_LTYPE2_EN        (1<<3)  /* Time Sync LTYPE 2 enable */
 #define TS_LTYPE1_EN        (1<<2)  /* Time Sync LTYPE 1 enable */
@@ -256,8 +257,9 @@ struct cpsw_ss_regs {
 #define TS_RX_EN            (1<<0)  /* Time Sync Receive Enable */
 
 #define CTRL_TS_BITS \
-	(TS_320 | TS_319 | TS_132 | TS_131 | TS_130 | TS_129 | TS_BIT8 | \
-	 TS_ANNEX_D_EN | TS_LTYPE1_EN)
+	(TS_320 | TS_319 | TS_132 | TS_131 | TS_130 | TS_129 |\
+	 TS_TTL_NONZERO | TS_ANNEX_F_EN | TS_ANNEX_D_EN |\
+	 TS_LTYPE1_EN)
 
 #define CTRL_ALL_TS_MASK (CTRL_TS_BITS | TS_TX_EN | TS_RX_EN)
 #define CTRL_TX_TS_BITS  (CTRL_TS_BITS | TS_TX_EN)
-- 
1.8.3.1

Re: [PATCH 4/6] drivers: net: cpsw: Enable Annexe F Time sync

[Richard Cochran]

On Mon, Apr 28, 2014 at 09:40:23AM +0530, George Cherian wrote:
> Enable the Annex F Time Sync explicitly for DRA7x and AM4372.
> With this enabled the L2 PTP is working.

L2 works fine without this bit. If this is needed for V3 hardware,
then it should have its own code variant.
 
> while at that rename TS_BIT8 to TS_TTL_NONZERO

Is this bit finally documented for am335x?

Thanks,
Richard

--
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 4/6] drivers: net: cpsw: Enable Annexe F Time sync

[George Cherian]

On 4/28/2014 1:25 PM, Richard Cochran wrote:
> On Mon, Apr 28, 2014 at 09:40:23AM +0530, George Cherian wrote:
>> Enable the Annex F Time Sync explicitly for DRA7x and AM4372.
>> With this enabled the L2 PTP is working.
> L2 works fine without this bit. If this is needed for V3 hardware,
> then it should have its own code variant.
okay
>   
>> while at that rename TS_BIT8 to TS_TTL_NONZERO
> Is this bit finally documented for am335x?
Not for am335x, but for other SoC's it s documented.
> Thanks,
> Richard
>


-- 
-George

[PATCH 5/6] ARM: AM43xx: clk: Change the cpts ref clock source to dpll_core_m5 clk

[George Cherian]

cpsw_cpts_rft_clk has got the choice of 3 clocksources
 -dpll_core_m4_ck
 -dpll_core_m5_ck
 -dpll_disp_m2_ck

By default dpll_core_m4_ck is selected, witn this as clock
source the CPTS doesnot work properly. It gives clockcheck errors
while running PTP.

 clockcheck: clock jumped backward or running slower than expected!

By selecting dpll_core_m5_ck as the clocksource fixes this issue.
In AM335x dpll_core_m5_ck is the default clocksource.

Signed-off-by: George Cherian <george.cherian@ti.com>
---
 drivers/clk/ti/clk-43xx.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/clk/ti/clk-43xx.c b/drivers/clk/ti/clk-43xx.c
index 67c8de5..b4877e0 100644
--- a/drivers/clk/ti/clk-43xx.c
+++ b/drivers/clk/ti/clk-43xx.c
@@ -110,9 +110,25 @@ static struct ti_dt_clk am43xx_clks[] = {
 
 int __init am43xx_dt_clk_init(void)
 {
+	struct clk *clk1, *clk2;
+
 	ti_dt_clocks_register(am43xx_clks);
 
 	omap2_clk_disable_autoidle_all();
 
+	/*
+	 * cpsw_cpts_rft_clk  has got the choice of 3 clocksources
+	 * dpll_core_m4_ck, dpll_core_m5_ck and dpll_disp_m2_ck.
+	 * By default dpll_core_m4_ck is selected, witn this as clock
+	 * source the CPTS doesnot work properly. It gives clockcheck errors
+	 * while running PTP.
+	 * clockcheck: clock jumped backward or running slower than expected!
+	 * By selecting dpll_core_m5_ck as the clocksource fixes this issue.
+	 * In AM335x dpll_core_m5_ck is the default clocksource.
+	 */
+	clk1 = clk_get_sys(NULL, "cpsw_cpts_rft_clk");
+	clk2 = clk_get_sys(NULL, "dpll_core_m5_ck");
+	clk_set_parent(clk1, clk2);
+
 	return 0;
 }
-- 
1.8.3.1

Re: [PATCH 5/6] ARM: AM43xx: clk: Change the cpts ref clock source to dpll_core_m5 clk

[Richard Cochran]

On Mon, Apr 28, 2014 at 09:40:24AM +0530, George Cherian wrote:
> cpsw_cpts_rft_clk has got the choice of 3 clocksources
>  -dpll_core_m4_ck
>  -dpll_core_m5_ck
>  -dpll_disp_m2_ck
> 
> By default dpll_core_m4_ck is selected, witn this as clock
> source the CPTS doesnot work properly. It gives clockcheck errors
> while running PTP.
> 
>  clockcheck: clock jumped backward or running slower than expected!

It is strange that I have never seen this error, since I have often
tested linuxptp on a beagle bone white.

Can you please explain why this clock doesn't work correctly?

> By selecting dpll_core_m5_ck as the clocksource fixes this issue.
> In AM335x dpll_core_m5_ck is the default clocksource.

The choice of clock source in the CPTS driver originally came from
TI. It would be nice to know why that was the wrong choice.

Thanks,
Richard

Re: [PATCH 5/6] ARM: AM43xx: clk: Change the cpts ref clock source to dpll_core_m5 clk

[George Cherian]

On 4/28/2014 12:40 PM, Richard Cochran wrote:
> On Mon, Apr 28, 2014 at 09:40:24AM +0530, George Cherian wrote:
>> cpsw_cpts_rft_clk has got the choice of 3 clocksources
>>   -dpll_core_m4_ck
>>   -dpll_core_m5_ck
>>   -dpll_disp_m2_ck
>>
>> By default dpll_core_m4_ck is selected, witn this as clock
>> source the CPTS doesnot work properly. It gives clockcheck errors
>> while running PTP.
>>
>>   clockcheck: clock jumped backward or running slower than expected!
> It is strange that I have never seen this error, since I have often
> tested linuxptp on a beagle bone white.
In beagle bone white (AM335x)  CPTS has a choice of 2 clocksource
-dpll_core_m5_ck
-dpll_core_m4_ck
and by default  dpll_core_m5_ck is used. Where as in AM437x the default 
clocksource used is dpll_core_m4_ck .

You can change the clocksource in beagle bone white by writing  1 to 
0x44e00520 (By default its 0).
>
> Can you please explain why this clock doesn't work correctly?
>
>> By selecting dpll_core_m5_ck as the clocksource fixes this issue.
>> In AM335x dpll_core_m5_ck is the default clocksource.
> The choice of clock source in the CPTS driver originally came from
> TI. It would be nice to know why that was the wrong choice.
>
> Thanks,
> Richard


-- 
-George

Re: [PATCH 5/6] ARM: AM43xx: clk: Change the cpts ref clock source to dpll_core_m5 clk

[Richard Cochran]

On Mon, Apr 28, 2014 at 06:25:56PM +0530, George Cherian wrote:
> In beagle bone white (AM335x)  CPTS has a choice of 2 clocksource
> -dpll_core_m5_ck
> -dpll_core_m4_ck
> and by default  dpll_core_m5_ck is used. Where as in AM437x the
> default clocksource used is dpll_core_m4_ck .

Is your patch changing the default clock for am335x?

If yes, it shouldn't.
If no, then the patch description should say so.

Thanks,
Richard

 

Re: [PATCH 5/6] ARM: AM43xx: clk: Change the cpts ref clock source to dpll_core_m5 clk

[George Cherian]

Hi Richard,

On 4/28/2014 9:48 PM, Richard Cochran wrote:
> On Mon, Apr 28, 2014 at 06:25:56PM +0530, George Cherian wrote:
>> In beagle bone white (AM335x)  CPTS has a choice of 2 clocksource
>> -dpll_core_m5_ck
>> -dpll_core_m4_ck
>> and by default  dpll_core_m5_ck is used. Where as in AM437x the
>> default clocksource used is dpll_core_m4_ck .
> Is your patch changing the default clock for am335x?
No
>
> If yes, it shouldn't.
> If no, then the patch description should say so.
I am modifying the file

drivers/clk/ti/clk-43xx.c for am33xx its drivers/clk/ti/clk-33xx.c

> Thanks,
> Richard
>
>   


-- 
-George

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

[PATCH 6/6] ARM: dts: am4372: Add clock names for cpsw and cpts

[George Cherian]

Add CPSW fck and CPTS clock and clock names for AM4372

Signed-off-by: George Cherian <george.cherian-l0cyMroinI0@public.gmane.org>
---
 arch/arm/boot/dts/am4372.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/am4372.dtsi b/arch/arm/boot/dts/am4372.dtsi
index 36d523a..c2779f6 100644
--- a/arch/arm/boot/dts/am4372.dtsi
+++ b/arch/arm/boot/dts/am4372.dtsi
@@ -489,6 +489,8 @@
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ti,hwmods = "cpgmac0";
+			clocks = <&cpsw_125mhz_gclk>, <&cpsw_cpts_rft_clk>;
+			clock-names = "fck", "cpts";
 			status = "disabled";
 			cpdma_channels = <8>;
 			ale_entries = <1024>;
-- 
1.8.3.1

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