[PATCH V3 1/5] clocksource: timer-of: Support getting clock frequency from DT

[PATCH V3 1/5] clocksource: timer-of: Support getting clock frequency from DT

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

More and more platforms use platform driver model for clock driver,
so the clock driver is NOT ready during timer initialization phase,
it will cause timer initialization failed.

To support those platforms with upper scenario, introducing a new
flag TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
TIMER_OF_CLOCK flag to support getting timer clock frequency from
DT, then of_clk operations can be skipped.

User needs to select either TIMER_OF_CLOCK_FREQUENCY or TIMER_OF_CLOCK
flag if want to use timer-of driver to initialize the clock rate,
and the corresponding clock name or property name needs to be specified.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
New patch:
	- Add new flag of TIMER_OF_CLOCK_FREQUENCY, mutually exclusive with TIMER_OF_CLOCK, to support
	  getting clock frequency from DT directly;
	- Add prop_name to of_timer_clk structure, if using TIMER_OF_CLOCK_FREQUENCY flag, user needs
	  to pass the property name for timer-of driver to get clock frequency from DT, this is to avoid
	  the couple of timer-of driver and DT, so timer-of driver does NOT use a fixed property name.
---
 drivers/clocksource/timer-of.c | 23 +++++++++++++++++++++++
 drivers/clocksource/timer-of.h |  8 +++++---
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 8054228..c91a8b6 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -161,6 +161,21 @@ static __init int timer_of_base_init(struct device_node *np,
 	return 0;
 }
 
+static __init int timer_of_clk_frequency_init(struct device_node *np,
+					      struct of_timer_clk *of_clk)
+{
+	int ret;
+	u32 rate;
+
+	ret = of_property_read_u32(np, of_clk->prop_name, &rate);
+	if (!ret) {
+		of_clk->rate = rate;
+		of_clk->period = DIV_ROUND_UP(rate, HZ);
+	}
+
+	return ret;
+}
+
 int __init timer_of_init(struct device_node *np, struct timer_of *to)
 {
 	int ret = -EINVAL;
@@ -178,6 +193,11 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
 		if (ret)
 			goto out_fail;
 		flags |= TIMER_OF_CLOCK;
+	} else if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
+		ret = timer_of_clk_frequency_init(np, &to->of_clk);
+		if (ret)
+			goto out_fail;
+		flags |= TIMER_OF_CLOCK_FREQUENCY;
 	}
 
 	if (to->flags & TIMER_OF_IRQ) {
@@ -201,6 +221,9 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
 	if (flags & TIMER_OF_CLOCK)
 		timer_of_clk_exit(&to->of_clk);
 
+	if (flags & TIMER_OF_CLOCK_FREQUENCY)
+		to->of_clk.rate = 0;
+
 	if (flags & TIMER_OF_BASE)
 		timer_of_base_exit(&to->of_base);
 	return ret;
diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
index a5478f3..f1a083e 100644
--- a/drivers/clocksource/timer-of.h
+++ b/drivers/clocksource/timer-of.h
@@ -4,9 +4,10 @@
 
 #include <linux/clockchips.h>
 
-#define TIMER_OF_BASE	0x1
-#define TIMER_OF_CLOCK	0x2
-#define TIMER_OF_IRQ	0x4
+#define TIMER_OF_BASE			0x1
+#define TIMER_OF_CLOCK			0x2
+#define TIMER_OF_IRQ			0x4
+#define TIMER_OF_CLOCK_FREQUENCY	0x8
 
 struct of_timer_irq {
 	int irq;
@@ -26,6 +27,7 @@ struct of_timer_base {
 struct of_timer_clk {
 	struct clk *clk;
 	const char *name;
+	const char *prop_name;
 	int index;
 	unsigned long rate;
 	unsigned long period;
-- 
2.7.4

[PATCH V3 2/5] clocksource/drivers/sysctr: Add clock-frequency property

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Systems which use platform driver model for clock driver require the
clock frequency to be supplied via device tree when system counter
driver is enabled.

This is necessary as in the platform driver model the of_clk operations
do not work correctly because system counter driver is initialized in
early phase of system boot up, and clock driver using platform driver
model is NOT ready at that time, it will cause system counter driver
initialization failed.

Add clock-frequency property to the device tree bindings of the NXP
system counter, so the driver can tell timer-of driver to get clock
frequency from DT directly instead of doing of_clk operations via
clk APIs.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V2:
	- make clock-frequency property as required property, mutually exclusive with clocks/clock-names.
	- update the example using the DT node added in this patch series.
---
 .../devicetree/bindings/timer/nxp,sysctr-timer.txt        | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..7088a0e 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -11,15 +11,18 @@ Required properties:
 - reg :             Specifies the base physical address and size of the comapre
                     frame and the counter control, read & compare.
 - interrupts :      should be the first compare frames' interrupt
-- clocks : 	    Specifies the counter clock.
-- clock-names: 	    Specifies the clock's name of this module
+- clocks :          Specifies the counter clock, mutually exclusive with clock-frequency.
+- clock-names :     Specifies the clock's name of this module, mutually exclusive with
+		    clock-frequency.
+- clock-frequency : Specifies system counter clock frequency, mutually exclusive with
+		    clocks/clock-names.
 
 Example:
 
 	system_counter: timer@306a0000 {
 		compatible = "nxp,sysctr-timer";
-		reg = <0x306a0000 0x20000>;/* system-counter-rd & compare */
-		clocks = <&clk_8m>;
-		clock-names = "per";
-		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+		reg = <0x306a0000 0x30000>;
+		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+		clock-frequency = <8333333>;
 	};
-- 
2.7.4

[PATCH V3 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

On some i.MX8M platforms, clock driver uses platform driver
model and it is NOT ready during timer initialization phase,
the clock operations will fail and system counter driver will
fail too. As all the i.MX8M platforms' system counter clock
are from OSC which is always enabled, so it is no need to enable
clock for system counter driver, the ONLY thing is to pass
clock frequence to driver.

To make system counter driver work for upper scenario, if DT's
system counter node has property "clock-frequency" present,
setting TIMER_OF_CLOCK_FREQUENCY flag to indicate timer-of driver
to get clock frequency from DT directly instead of of_clk operation
via clk APIs.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V2:
	- do runtime check to decide whether using TIMER_OF_CLOCK_FREQUENCY or TIMER_OF_CLOCK
	  according to DT node settings.
---
 drivers/clocksource/timer-imx-sysctr.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..73e3193 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -98,7 +98,7 @@ static irqreturn_t sysctr_timer_interrupt(int irq, void *dev_id)
 }
 
 static struct timer_of to_sysctr = {
-	.flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE,
+	.flags = TIMER_OF_IRQ | TIMER_OF_BASE,
 	.clkevt = {
 		.name			= "i.MX system counter timer",
 		.features		= CLOCK_EVT_FEAT_ONESHOT |
@@ -114,6 +114,7 @@ static struct timer_of to_sysctr = {
 	},
 	.of_clk = {
 		.name = "per",
+		.prop_name = "clock-frequency",
 	},
 };
 
@@ -130,6 +131,9 @@ static int __init sysctr_timer_init(struct device_node *np)
 {
 	int ret = 0;
 
+	to_sysctr.flags |= of_find_property(np, "clock-frequency", NULL) ?
+			   TIMER_OF_CLOCK_FREQUENCY : TIMER_OF_CLOCK;
+
 	ret = timer_of_init(np, &to_sysctr);
 	if (ret)
 		return ret;
-- 
2.7.4

[PATCH V3 4/5] arm64: dts: imx8mq: Add system counter node

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Add i.MX8MQ system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
No changes.
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index b1114a6..bea53bc 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -636,6 +636,14 @@
 				#pwm-cells = <2>;
 				status = "disabled";
 			};
+
+			system_counter: timer@306a0000 {
+				compatible = "nxp,sysctr-timer";
+				reg = <0x306a0000 0x30000>;
+				interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				clock-frequency = <8333333>;
+			};
 		};
 
 		bus@30800000 { /* AIPS3 */
-- 
2.7.4

[PATCH V3 5/5] arm64: dts: imx8mm: Add system counter node

[Anson.Huang]

From: Anson Huang <Anson.Huang@nxp.com>

Add i.MX8MM system counter node to enable timer-imx-sysctr
broadcast timer driver.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
New patch:
	- As i.MX8MM clock driver will be soon moved to using platform driver model, so the patch
	  series I sent out for i.MX8MM system counter driver support will need rework accordingly,
	  so I add the i.MX8MM DT support in this patch series, it uses same method as i.MX8MQ's
	  system counter driver.
---
 arch/arm64/boot/dts/freescale/imx8mm.dtsi | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mm.dtsi b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
index 63f4731..aa985a0 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -528,6 +528,14 @@
 				#pwm-cells = <2>;
 				status = "disabled";
 			};
+
+			system_counter: timer@306a0000 {
+				compatible = "nxp,sysctr-timer";
+				reg = <0x306a0000 0x30000>;
+				interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>,
+					     <GIC_SPI 48 IRQ_TYPE_LEVEL_HIGH>;
+				clock-frequency = <8000000>;
+			};
 		};
 
 		aips3: bus@30800000 {
-- 
2.7.4

Re: [PATCH V3 1/5] clocksource: timer-of: Support getting clock frequency from DT

[Daniel Lezcano]

Hi Anson,

thanks for taking care of adding the clock-frequency handling in the
timer-of.

On 28/06/2019 05:30, Anson.Huang@nxp.com wrote:
> From: Anson Huang <Anson.Huang@nxp.com>
> 
> More and more platforms use platform driver model for clock driver,
> so the clock driver is NOT ready during timer initialization phase,
> it will cause timer initialization failed.
> 
> To support those platforms with upper scenario, introducing a new
> flag TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
> TIMER_OF_CLOCK flag to support getting timer clock frequency from
> DT, then of_clk operations can be skipped.
> 
> User needs to select either TIMER_OF_CLOCK_FREQUENCY or TIMER_OF_CLOCK
> flag if want to use timer-of driver to initialize the clock rate,
> and the corresponding clock name or property name needs to be specified.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> New patch:
> 	- Add new flag of TIMER_OF_CLOCK_FREQUENCY, mutually exclusive with TIMER_OF_CLOCK, to support
> 	  getting clock frequency from DT directly;
> 	- Add prop_name to of_timer_clk structure, if using TIMER_OF_CLOCK_FREQUENCY flag, user needs
> 	  to pass the property name for timer-of driver to get clock frequency from DT, this is to avoid
> 	  the couple of timer-of driver and DT, so timer-of driver does NOT use a fixed property name.
> ---
>  drivers/clocksource/timer-of.c | 23 +++++++++++++++++++++++
>  drivers/clocksource/timer-of.h |  8 +++++---
>  2 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
> index 8054228..c91a8b6 100644
> --- a/drivers/clocksource/timer-of.c
> +++ b/drivers/clocksource/timer-of.c
> @@ -161,6 +161,21 @@ static __init int timer_of_base_init(struct device_node *np,
>  	return 0;
>  }
>  
> +static __init int timer_of_clk_frequency_init(struct device_node *np,
> +					      struct of_timer_clk *of_clk)
> +{
> +	int ret;
> +	u32 rate;
> +
> +	ret = of_property_read_u32(np, of_clk->prop_name, &rate);
> +	if (!ret) {
> +		of_clk->rate = rate;
> +		of_clk->period = DIV_ROUND_UP(rate, HZ);
> +	}
> +
> +	return ret;
> +}
> +
>  int __init timer_of_init(struct device_node *np, struct timer_of *to)
>  {
>  	int ret = -EINVAL;
> @@ -178,6 +193,11 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
>  		if (ret)
>  			goto out_fail;
>  		flags |= TIMER_OF_CLOCK;
> +	} else if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
> +		ret = timer_of_clk_frequency_init(np, &to->of_clk);
> +		if (ret)
> +			goto out_fail;
> +		flags |= TIMER_OF_CLOCK_FREQUENCY;
>  	}

/* Pre-condition */

if (to->flags & (TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY))
	return -EINVAL;

[ ... ]

if (to->flags & TIMER_OF_CLOCK) {
}

if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
}

>  	if (to->flags & TIMER_OF_IRQ) {
> @@ -201,6 +221,9 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
>  	if (flags & TIMER_OF_CLOCK)
>  		timer_of_clk_exit(&to->of_clk);
>  
> +	if (flags & TIMER_OF_CLOCK_FREQUENCY)
> +		to->of_clk.rate = 0;
> +
>  	if (flags & TIMER_OF_BASE)
>  		timer_of_base_exit(&to->of_base);
>  	return ret;
> diff --git a/drivers/clocksource/timer-of.h b/drivers/clocksource/timer-of.h
> index a5478f3..f1a083e 100644
> --- a/drivers/clocksource/timer-of.h
> +++ b/drivers/clocksource/timer-of.h
> @@ -4,9 +4,10 @@
>  
>  #include <linux/clockchips.h>
>  
> -#define TIMER_OF_BASE	0x1
> -#define TIMER_OF_CLOCK	0x2
> -#define TIMER_OF_IRQ	0x4
> +#define TIMER_OF_BASE			0x1
> +#define TIMER_OF_CLOCK			0x2
> +#define TIMER_OF_IRQ			0x4
> +#define TIMER_OF_CLOCK_FREQUENCY	0x8
>  
>  struct of_timer_irq {
>  	int irq;
> @@ -26,6 +27,7 @@ struct of_timer_base {
>  struct of_timer_clk {
>  	struct clk *clk;
>  	const char *name;
> +	const char *prop_name;

For the moment, keep it hardcoded with "clock-frequency" directly in the
function.

>  	int index;
>  	unsigned long rate;
>  	unsigned long period;
> 


-- 
 <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

RE: [PATCH V3 1/5] clocksource: timer-of: Support getting clock frequency from DT

[Anson Huang]

Hi, Daniel

> Subject: Re: [PATCH V3 1/5] clocksource: timer-of: Support getting clock
> frequency from DT
> 
> 
> Hi Anson,
> 
> thanks for taking care of adding the clock-frequency handling in the timer-of.

Sure.

> 
> On 28/06/2019 05:30, Anson.Huang@nxp.com wrote:
> > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > More and more platforms use platform driver model for clock driver, so
> > the clock driver is NOT ready during timer initialization phase, it
> > will cause timer initialization failed.
> >
> > To support those platforms with upper scenario, introducing a new flag
> > TIMER_OF_CLOCK_FREQUENCY which is mutually exclusive with
> > TIMER_OF_CLOCK flag to support getting timer clock frequency from DT,
> > then of_clk operations can be skipped.
> >
> > User needs to select either TIMER_OF_CLOCK_FREQUENCY or
> TIMER_OF_CLOCK
> > flag if want to use timer-of driver to initialize the clock rate, and
> > the corresponding clock name or property name needs to be specified.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > New patch:
> > 	- Add new flag of TIMER_OF_CLOCK_FREQUENCY, mutually exclusive
> with TIMER_OF_CLOCK, to support
> > 	  getting clock frequency from DT directly;
> > 	- Add prop_name to of_timer_clk structure, if using
> TIMER_OF_CLOCK_FREQUENCY flag, user needs
> > 	  to pass the property name for timer-of driver to get clock frequency
> from DT, this is to avoid
> > 	  the couple of timer-of driver and DT, so timer-of driver does NOT
> use a fixed property name.
> > ---
> >  drivers/clocksource/timer-of.c | 23 +++++++++++++++++++++++
> > drivers/clocksource/timer-of.h |  8 +++++---
> >  2 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-of.c
> > b/drivers/clocksource/timer-of.c index 8054228..c91a8b6 100644
> > --- a/drivers/clocksource/timer-of.c
> > +++ b/drivers/clocksource/timer-of.c
> > @@ -161,6 +161,21 @@ static __init int timer_of_base_init(struct
> device_node *np,
> >  	return 0;
> >  }
> >
> > +static __init int timer_of_clk_frequency_init(struct device_node *np,
> > +					      struct of_timer_clk *of_clk) {
> > +	int ret;
> > +	u32 rate;
> > +
> > +	ret = of_property_read_u32(np, of_clk->prop_name, &rate);
> > +	if (!ret) {
> > +		of_clk->rate = rate;
> > +		of_clk->period = DIV_ROUND_UP(rate, HZ);
> > +	}
> > +
> > +	return ret;
> > +}
> > +
> >  int __init timer_of_init(struct device_node *np, struct timer_of *to)
> > {
> >  	int ret = -EINVAL;
> > @@ -178,6 +193,11 @@ int __init timer_of_init(struct device_node *np,
> struct timer_of *to)
> >  		if (ret)
> >  			goto out_fail;
> >  		flags |= TIMER_OF_CLOCK;
> > +	} else if (to->flags & TIMER_OF_CLOCK_FREQUENCY) {
> > +		ret = timer_of_clk_frequency_init(np, &to->of_clk);
> > +		if (ret)
> > +			goto out_fail;
> > +		flags |= TIMER_OF_CLOCK_FREQUENCY;
> >  	}
> 
> /* Pre-condition */
> 
> if (to->flags & (TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY))
> 	return -EINVAL;
> 
> [ ... ]
> 
> if (to->flags & TIMER_OF_CLOCK) {
> }
> 
> if (to->flags & TIMER_OF_CLOCK_FREQUENCY) { }
> 

Ah, make sense, they are exclusive, will add it in next version.

> >  	if (to->flags & TIMER_OF_IRQ) {
> > @@ -201,6 +221,9 @@ int __init timer_of_init(struct device_node *np,
> struct timer_of *to)
> >  	if (flags & TIMER_OF_CLOCK)
> >  		timer_of_clk_exit(&to->of_clk);
> >
> > +	if (flags & TIMER_OF_CLOCK_FREQUENCY)
> > +		to->of_clk.rate = 0;
> > +
> >  	if (flags & TIMER_OF_BASE)
> >  		timer_of_base_exit(&to->of_base);
> >  	return ret;
> > diff --git a/drivers/clocksource/timer-of.h
> > b/drivers/clocksource/timer-of.h index a5478f3..f1a083e 100644
> > --- a/drivers/clocksource/timer-of.h
> > +++ b/drivers/clocksource/timer-of.h
> > @@ -4,9 +4,10 @@
> >
> >  #include <linux/clockchips.h>
> >
> > -#define TIMER_OF_BASE	0x1
> > -#define TIMER_OF_CLOCK	0x2
> > -#define TIMER_OF_IRQ	0x4
> > +#define TIMER_OF_BASE			0x1
> > +#define TIMER_OF_CLOCK			0x2
> > +#define TIMER_OF_IRQ			0x4
> > +#define TIMER_OF_CLOCK_FREQUENCY	0x8
> >
> >  struct of_timer_irq {
> >  	int irq;
> > @@ -26,6 +27,7 @@ struct of_timer_base {  struct of_timer_clk {
> >  	struct clk *clk;
> >  	const char *name;
> > +	const char *prop_name;
> 
> For the moment, keep it hardcoded with "clock-frequency" directly in the
> function.

OK, then I will NOT add any dt-binding for this property. The reason to use prop_name
instead of hardcode is I don't want to create a binding doc just for this property.

Thanks,
Anson.