[PATCH 1/3] arm64: dts: fsl: add clocks property for fman ptp timer node

This patch is to add clocks property for fman ptp timer node.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi b/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi
index a56a408..4664c33 100644
--- a/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi
+++ b/arch/arm64/boot/dts/freescale/qoriq-fman3-0.dtsi

@@ -80,4 +80,5 @@ ptp_timer0: ptp-timer@1afe000 {
 	compatible = "fsl,fman-ptp-timer";
 	reg = <0x0 0x1afe000 0x0 0x1000>;
 	interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+	clocks = <&clockgen 3 0>;
 };

-- 
1.7.1

This patch is to add clocks property for fman ptp timer node.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi   |    1 +
 arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi   |    1 +
 arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi  |    1 +
 arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi  |    1 +
 arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi |    1 +
 5 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi
index 6b124f7..9b6cf91 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman-0.dtsi

@@ -100,4 +100,5 @@ ptp_timer0: ptp-timer@4fe000 {
 	compatible = "fsl,fman-ptp-timer";
 	reg = <0x4fe000 0x1000>;
 	interrupts = <96 2 0 0>;
+	clocks = <&clockgen 3 0>;
 };

diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi
index b80aaf5..e95c11f 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman-1.dtsi

@@ -100,4 +100,5 @@ ptp_timer1: ptp-timer@5fe000 {
 	compatible = "fsl,fman-ptp-timer";
 	reg = <0x5fe000 0x1000>;
 	interrupts = <97 2 0 0>;
+	clocks = <&clockgen 3 1>;
 };

diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi
index d3720fd..d62b36c 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman3-0.dtsi

@@ -105,4 +105,5 @@ ptp_timer0: ptp-timer@4fe000 {
 	compatible = "fsl,fman-ptp-timer";
 	reg = <0x4fe000 0x1000>;
 	interrupts = <96 2 0 0>;
+	clocks = <&clockgen 3 0>;
 };

diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi
index ae34c20..3102324 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman3-1.dtsi

@@ -105,4 +105,5 @@ ptp_timer1: ptp-timer@5fe000 {
 	compatible = "fsl,fman-ptp-timer";
 	reg = <0x5fe000 0x1000>;
 	interrupts = <97 2 0 0>;
+	clocks = <&clockgen 3 1>;
 };

diff --git a/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi b/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi
index 02f2755..c90702b 100644
--- a/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi
+++ b/arch/powerpc/boot/dts/fsl/qoriq-fman3l-0.dtsi

@@ -93,4 +93,5 @@ ptp_timer0: ptp-timer@4fe000 {
 	compatible = "fsl,fman-ptp-timer";
 	reg = <0x4fe000 0x1000>;
 	interrupts = <96 2 0 0>;
+	clocks = <&clockgen 3 0>;
 };

-- 
1.7.1

The ptp_qoriq driver initialized the 1588 timer with the
configurations provided by the properties of device tree
node. For example,

  fsl,tclk-period = <5>;
  fsl,tmr-prsc    = <2>;
  fsl,tmr-add     = <0xaaaaaaab>;
  fsl,tmr-fiper1  = <999999995>;
  fsl,tmr-fiper2  = <99990>;
  fsl,max-adj     = <499999999>;

These things actually were runtime configurations which
were not proper to be put into dts. This patch is to convert
to use module parameters for 1588 timer initialization, and
to support initial register values calculation.
If the parameters are not provided, the driver will calculate
register values with a set of default parameters. With this
patch, those dts properties are no longer needed for new
platform to support 1588 timer, and many QorIQ DPAA platforms
(some P series and T series platforms of PowerPC, and some
LS series platforms of ARM64) could use this driver for their
fman ptp timer with default module parameters. However, this
patch didn't remove the dts method. Because there were still
many old platforms using the dts method. We need to clean up
their dts files, verify module parameters on them, and convert
them to the new method gradually in case of breaking any
function.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
 drivers/ptp/ptp_qoriq.c       |  117 +++++++++++++++++++++++++++++++++++++++-
 include/linux/fsl/ptp_qoriq.h |    1 +
 2 files changed, 115 insertions(+), 3 deletions(-)

diff --git a/drivers/ptp/ptp_qoriq.c b/drivers/ptp/ptp_qoriq.c
index a14c317..22baf83 100644
--- a/drivers/ptp/ptp_qoriq.c
+++ b/drivers/ptp/ptp_qoriq.c

@@ -29,9 +29,30 @@
 #include <linux/of_platform.h>
 #include <linux/timex.h>
 #include <linux/slab.h>
+#include <linux/clk.h>
 
 #include <linux/fsl/ptp_qoriq.h>
 
+static unsigned int cksel = DEFAULT_CKSEL;
+module_param(cksel, uint, 0644);
+MODULE_PARM_DESC(cksel, "Select reference clock");
+
+static unsigned int clk_src;
+module_param(clk_src, uint, 0644);
+MODULE_PARM_DESC(clk_src, "Reference clock frequency (if clocks property not provided in dts)");
+
+static unsigned int tmr_prsc = 2;
+module_param(tmr_prsc, uint, 0644);
+MODULE_PARM_DESC(tmr_prsc, "Output clock division/prescale factor");
+
+static unsigned int tmr_fiper1 = 1000000000;
+module_param(tmr_fiper1, uint, 0644);
+MODULE_PARM_DESC(tmr_fiper1, "Desired fixed interval pulse period (ns)");
+
+static unsigned int tmr_fiper2 = 100000;
+module_param(tmr_fiper2, uint, 0644);
+MODULE_PARM_DESC(tmr_fiper2, "Desired fixed interval pulse period (ns)");
+
 /*
  * Register access functions
  */

@@ -317,6 +338,91 @@ static int ptp_qoriq_enable(struct ptp_clock_info *ptp,
 	.enable		= ptp_qoriq_enable,
 };
 
+/**
+ * qoriq_ptp_nominal_freq - calculate nominal frequency by reference clock
+ *			    frequency
+ *
+ * @clk_src: reference clock frequency
+ *
+ * The nominal frequency is the desired clock frequency.
+ * It should be less than the reference clock frequency.
+ * It should be a factor of 1000MHz.
+ *
+ * Return the nominal frequency
+ */
+static u32 qoriq_ptp_nominal_freq(u32 clk_src)
+{
+	u32 remainder = 0;
+
+	clk_src /= 1000000;
+	remainder = clk_src % 100;
+	if (remainder) {
+		clk_src -= remainder;
+		clk_src += 100;
+	}
+
+	do {
+		clk_src -= 100;
+
+	} while (1000 % clk_src);
+
+	return clk_src * 1000000;
+}
+
+static int qoriq_ptp_config(struct qoriq_ptp *qoriq_ptp,
+			    struct device_node *node)
+{
+	struct clk *clk;
+	u64 freq_comp;
+	u64 max_adj;
+	u32 nominal_freq;
+
+	qoriq_ptp->cksel = cksel;
+
+	if (clk_src) {
+		qoriq_ptp->clk_src = clk_src;
+	} else {
+		clk = of_clk_get(node, 0);
+		if (!IS_ERR(clk)) {
+			qoriq_ptp->clk_src = clk_get_rate(clk);
+			clk_put(clk);
+		}
+	}
+
+	if (qoriq_ptp->clk_src <= 100000000UL) {
+		pr_err("error reference clock value, or lower than 100MHz\n");
+		return -EINVAL;
+	}
+
+	nominal_freq = qoriq_ptp_nominal_freq(qoriq_ptp->clk_src);
+	if (!nominal_freq)
+		return -EINVAL;
+
+	qoriq_ptp->tclk_period = 1000000000UL / nominal_freq;
+	qoriq_ptp->tmr_prsc = tmr_prsc;
+
+	/* Calculate initial frequency compensation value for TMR_ADD register.
+	 * freq_comp = ceil(2^32 / freq_ratio)
+	 * freq_ratio = reference_clock_freq / nominal_freq
+	 */
+	freq_comp = ((u64)1 << 32) * nominal_freq;
+	if (do_div(freq_comp, qoriq_ptp->clk_src))
+		freq_comp++;
+
+	qoriq_ptp->tmr_add = freq_comp;
+	qoriq_ptp->tmr_fiper1 = tmr_fiper1 - qoriq_ptp->tclk_period;
+	qoriq_ptp->tmr_fiper2 = tmr_fiper2 - qoriq_ptp->tclk_period;
+
+	/* max_adj = 1000000000 * (freq_ratio - 1.0) - 1
+	 * freq_ratio = reference_clock_freq / nominal_freq
+	 */
+	max_adj = 1000000000ULL * (qoriq_ptp->clk_src - nominal_freq);
+	max_adj = max_adj / nominal_freq - 1;
+	qoriq_ptp->caps.max_adj = max_adj;
+
+	return 0;
+}
+
 static int qoriq_ptp_probe(struct platform_device *dev)
 {
 	struct device_node *node = dev->dev.of_node;

@@ -332,7 +438,7 @@ static int qoriq_ptp_probe(struct platform_device *dev)
 	if (!qoriq_ptp)
 		goto no_memory;
 
-	err = -ENODEV;
+	err = -EINVAL;
 
 	qoriq_ptp->caps = ptp_qoriq_caps;
 

@@ -351,10 +457,14 @@ static int qoriq_ptp_probe(struct platform_device *dev)
 				 "fsl,tmr-fiper2", &qoriq_ptp->tmr_fiper2) ||
 	    of_property_read_u32(node,
 				 "fsl,max-adj", &qoriq_ptp->caps.max_adj)) {
-		pr_err("device tree node missing required elements\n");
-		goto no_node;
+		pr_warn("device tree node missing required elements, try module param\n");
+
+		if (qoriq_ptp_config(qoriq_ptp, node))
+			goto no_param;
 	}
 
+	err = -ENODEV;
+
 	qoriq_ptp->irq = platform_get_irq(dev, 0);
 
 	if (qoriq_ptp->irq < 0) {

@@ -436,6 +546,7 @@ static int qoriq_ptp_probe(struct platform_device *dev)
 	release_resource(qoriq_ptp->rsrc);
 no_resource:
 	free_irq(qoriq_ptp->irq, qoriq_ptp);
+no_param:
 no_node:
 	kfree(qoriq_ptp);
 no_memory:

diff --git a/include/linux/fsl/ptp_qoriq.h b/include/linux/fsl/ptp_qoriq.h
index dc3dac4..586d430 100644
--- a/include/linux/fsl/ptp_qoriq.h
+++ b/include/linux/fsl/ptp_qoriq.h

@@ -147,6 +147,7 @@ struct qoriq_ptp {
 	u32 cksel;
 	u32 tmr_fiper1;
 	u32 tmr_fiper2;
+	u32 clk_src;
 };
 
 static inline u32 qoriq_read(unsigned __iomem *addr)

-- 
1.7.1

On Mon, Jul 30, 2018 at 06:01:54PM +0800, Yangbo Lu wrote:

The ptp_qoriq driver initialized the 1588 timer with the
configurations provided by the properties of device tree
node. For example,

  fsl,tclk-period = <5>;
  fsl,tmr-prsc    = <2>;
  fsl,tmr-add     = <0xaaaaaaab>;
  fsl,tmr-fiper1  = <999999995>;
  fsl,tmr-fiper2  = <99990>;
  fsl,max-adj     = <499999999>;

These things actually were runtime configurations which
were not proper to be put into dts.

That is debatable.  While I agree that the dts isn't ideal for these,
still it is the lesser of two or more evils.

This patch is to convert
to use module parameters for 1588 timer initialization, and
to support initial register values calculation.

It is hard for me to understand how using module parameters improves
the situation.

If the parameters are not provided, the driver will calculate
register values with a set of default parameters. With this
patch, those dts properties are no longer needed for new
platform to support 1588 timer, and many QorIQ DPAA platforms
(some P series and T series platforms of PowerPC, and some
LS series platforms of ARM64) could use this driver for their
fman ptp timer with default module parameters. However, this
patch didn't remove the dts method. Because there were still
many old platforms using the dts method. We need to clean up
their dts files, verify module parameters on them, and convert
them to the new method gradually in case of breaking any
function.

In addition, like it or not, because the dts is an ABI, you must
continue support of the dts values as a legacy option.

Thanks,
Richard

From: Yangbo Lu <yangbo.lu@nxp.com>
Date: Mon, 30 Jul 2018 18:01:54 +0800

+static unsigned int cksel = DEFAULT_CKSEL;
+module_param(cksel, uint, 0644);
+MODULE_PARM_DESC(cksel, "Select reference clock");
+
+static unsigned int clk_src;
+module_param(clk_src, uint, 0644);
+MODULE_PARM_DESC(clk_src, "Reference clock frequency (if clocks property not provided in dts)");
+
+static unsigned int tmr_prsc = 2;
+module_param(tmr_prsc, uint, 0644);
+MODULE_PARM_DESC(tmr_prsc, "Output clock division/prescale factor");
+
+static unsigned int tmr_fiper1 = 1000000000;
+module_param(tmr_fiper1, uint, 0644);
+MODULE_PARM_DESC(tmr_fiper1, "Desired fixed interval pulse period (ns)");
+
+static unsigned int tmr_fiper2 = 100000;
+module_param(tmr_fiper2, uint, 0644);
+MODULE_PARM_DESC(tmr_fiper2, "Desired fixed interval pulse period (ns)");

Sorry, there is no way I am every applying something like this.  Module
parameters are to be avoided at all costs.

And you don't need it here, you have DTS, please use it.

You are required to support the existing DTS cases, in order to
avoid breaking things, anyways.

Hi Richard,

-----Original Message-----
From: Richard Cochran [mailto:richardcochran@gmail.com]
Sent: Monday, July 30, 2018 10:31 PM
To: Y.b. Lu <yangbo.lu@nxp.com>
Cc: netdev@vger.kernel.org; Madalin-cristian Bucur
[off-list ref]; Rob Herring [off-list ref]; Shawn Guo
[off-list ref]; David S . Miller [off-list ref];
devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] ptp_qoriq: convert to use module parameters for
initialization
=20
On Mon, Jul 30, 2018 at 06:01:54PM +0800, Yangbo Lu wrote:

quoted

The ptp_qoriq driver initialized the 1588 timer with the
configurations provided by the properties of device tree node. For
example,

  fsl,tclk-period =3D <5>;
  fsl,tmr-prsc    =3D <2>;
  fsl,tmr-add     =3D <0xaaaaaaab>;
  fsl,tmr-fiper1  =3D <999999995>;
  fsl,tmr-fiper2  =3D <99990>;
  fsl,max-adj     =3D <499999999>;

These things actually were runtime configurations which were not
proper to be put into dts.

=20
That is debatable.  While I agree that the dts isn't ideal for these, sti=

ll it is the

lesser of two or more evils.

[Y.b. Lu] Ok. You're right indeed :)

=20

quoted

This patch is to convert
to use module parameters for 1588 timer initialization, and to support
initial register values calculation.

=20
It is hard for me to understand how using module parameters improves the
situation.

[Y.b. Lu] Actually I'm not sure whether module_param will be accepted to re=
place dts.
I thought the most possibility would be rejection before sending them out.
Just want suggestion and confirmation whether there is better idea than dts=
 from your comments.

Since we should keep the dts, I will drop the module_param.
Could I add a function to calculate a set of default register values to ini=
tialize ptp timer when dts method failed to get required properties in driv=
er?
I think this will be useful. The ptp timer on new platforms (you may see tw=
o dts patches in this patchset. Many platforms will be affected.) will work=
 without these dts properties. If user want specific setting, they can set =
dts properties.

=20

quoted

If the parameters are not provided, the driver will calculate register
values with a set of default parameters. With this patch, those dts
properties are no longer needed for new platform to support 1588
timer, and many QorIQ DPAA platforms (some P series and T series
platforms of PowerPC, and some LS series platforms of ARM64) could use
this driver for their fman ptp timer with default module parameters.
However, this patch didn't remove the dts method. Because there were
still many old platforms using the dts method. We need to clean up
their dts files, verify module parameters on them, and convert them to
the new method gradually in case of breaking any function.

=20
In addition, like it or not, because the dts is an ABI, you must continue=

 support

of the dts values as a legacy option.

[Y.b. Lu] I get your point now. The dts should be kept :)

=20
Thanks,
Richard

Hi David,

-----Original Message-----
From: David Miller [mailto:davem@davemloft.net]
Sent: Tuesday, July 31, 2018 12:26 AM
To: Y.b. Lu <yangbo.lu@nxp.com>
Cc: netdev@vger.kernel.org; Madalin-cristian Bucur
[off-list ref]; richardcochran@gmail.com; robh+dt@kernel.org;
shawnguo@kernel.org; devicetree@vger.kernel.org;
linuxppc-dev@lists.ozlabs.org; linux-arm-kernel@lists.infradead.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] ptp_qoriq: convert to use module parameters for
initialization
=20
From: Yangbo Lu <yangbo.lu@nxp.com>
Date: Mon, 30 Jul 2018 18:01:54 +0800
=20

quoted

+static unsigned int cksel =3D DEFAULT_CKSEL; module_param(cksel, uint,
+0644); MODULE_PARM_DESC(cksel, "Select reference clock");
+
+static unsigned int clk_src;
+module_param(clk_src, uint, 0644);
+MODULE_PARM_DESC(clk_src, "Reference clock frequency (if clocks
+property not provided in dts)");
+
+static unsigned int tmr_prsc =3D 2;
+module_param(tmr_prsc, uint, 0644);
+MODULE_PARM_DESC(tmr_prsc, "Output clock division/prescale factor");
+
+static unsigned int tmr_fiper1 =3D 1000000000; module_param(tmr_fiper1=

,

quoted

+uint, 0644); MODULE_PARM_DESC(tmr_fiper1, "Desired fixed interval
+pulse period (ns)");
+
+static unsigned int tmr_fiper2 =3D 100000; module_param(tmr_fiper2,
+uint, 0644); MODULE_PARM_DESC(tmr_fiper2, "Desired fixed interval
+pulse period (ns)");

=20
Sorry, there is no way I am every applying something like this.  Module
parameters are to be avoided at all costs.
=20
And you don't need it here, you have DTS, please use it.
=20
You are required to support the existing DTS cases, in order to avoid bre=

aking

things, anyways.

[Y.b. Lu] I get your point. Will drop module_param method.
Thanks a lot for your suggestion.

On Wed, Aug 01, 2018 at 04:36:40AM +0000, Y.b. Lu wrote:

Could I add a function to calculate a set of default register values
to initialize ptp timer when dts method failed to get required
properties in driver?

Yes, it would be ideal if the driver can pick correct values
automatically.

However, the frequency on the FIPER outputs can't be configured
automatically, and we don't have an API for the user to choose this.

I think this will be useful. The ptp timer on new platforms (you may
see two dts patches in this patchset. Many platforms will be
affected.) will work without these dts properties. If user want
specific setting, they can set dts properties.

Sure.

Thanks,
Richard

Hi Richard,

-----Original Message-----
From: Richard Cochran [mailto:richardcochran@gmail.com]
Sent: Wednesday, August 1, 2018 2:15 PM
To: Y.b. Lu <yangbo.lu@nxp.com>
Cc: netdev@vger.kernel.org; Madalin-cristian Bucur
[off-list ref]; Rob Herring [off-list ref]; Shawn Guo
[off-list ref]; David S . Miller [off-list ref];
devicetree@vger.kernel.org; linuxppc-dev@lists.ozlabs.org;
linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] ptp_qoriq: convert to use module parameters for
initialization
=20
On Wed, Aug 01, 2018 at 04:36:40AM +0000, Y.b. Lu wrote:
=20

quoted

Could I add a function to calculate a set of default register values
to initialize ptp timer when dts method failed to get required
properties in driver?

=20
Yes, it would be ideal if the driver can pick correct values automaticall=

y.

=20
However, the frequency on the FIPER outputs can't be configured
automatically, and we don't have an API for the user to choose this.

[Y.b. Lu] Thanks a lot. Just let me send out the v2 patch for your reviewin=
g.

=20

quoted

I think this will be useful. The ptp timer on new platforms (you may
see two dts patches in this patchset. Many platforms will be
affected.) will work without these dts properties. If user want
specific setting, they can set dts properties.

=20
Sure.
=20
Thanks,
Richard