linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT
@ 2019-07-01  9:38 Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property Anson.Huang
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Anson.Huang @ 2019-07-01  9:38 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, shawnguo, s.hauer,
	kernel, festevam, leonard.crestez, viresh.kumar, daniel.baluta,
	ping.bai, l.stach, abel.vesa, andrew.smirnov, ccaione, angus,
	agx, linux-kernel, devicetree, linux-arm-kernel
  Cc: Linux-imx

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's timer node, the property name should be "clock-frequency",
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.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V3:
	- use hardcoded "clock-frequency" instead of adding new variable prop_name;
	- add pre-condition check for TIMER_OF_CLOCK and TIMER_OF_CLOCK_FREQUENCY, they MUST be exclusive.
---
 drivers/clocksource/timer-of.c | 29 +++++++++++++++++++++++++++++
 drivers/clocksource/timer-of.h |  7 ++++---
 2 files changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/timer-of.c b/drivers/clocksource/timer-of.c
index 8054228..ab155cc 100644
--- a/drivers/clocksource/timer-of.c
+++ b/drivers/clocksource/timer-of.c
@@ -161,11 +161,30 @@ 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, "clock-frequency", &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)
 {
+	unsigned long clock_flags = TIMER_OF_CLOCK | TIMER_OF_CLOCK_FREQUENCY;
 	int ret = -EINVAL;
 	int flags = 0;
 
+	if (to->flags & clock_flags == clock_flags)
+		return ret;
+
 	if (to->flags & TIMER_OF_BASE) {
 		ret = timer_of_base_init(np, &to->of_base);
 		if (ret)
@@ -180,6 +199,13 @@ int __init timer_of_init(struct device_node *np, struct timer_of *to)
 		flags |= TIMER_OF_CLOCK;
 	}
 
+	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) {
 		ret = timer_of_irq_init(np, &to->of_irq);
 		if (ret)
@@ -201,6 +227,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..a08e108 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;
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property
  2019-07-01  9:38 [PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT Anson.Huang
@ 2019-07-01  9:38 ` Anson.Huang
  2019-07-09 20:30   ` Rob Herring
  2019-07-01  9:38 ` [PATCH V4 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model Anson.Huang
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Anson.Huang @ 2019-07-01  9:38 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, shawnguo, s.hauer,
	kernel, festevam, leonard.crestez, viresh.kumar, daniel.baluta,
	ping.bai, l.stach, abel.vesa, andrew.smirnov, ccaione, angus,
	agx, linux-kernel, devicetree, linux-arm-kernel
  Cc: Linux-imx

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>
---
No change.
---
 .../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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model
  2019-07-01  9:38 [PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property Anson.Huang
@ 2019-07-01  9:38 ` Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 4/5] arm64: dts: imx8mq: Add system counter node Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 5/5] arm64: dts: imx8mm: " Anson.Huang
  3 siblings, 0 replies; 9+ messages in thread
From: Anson.Huang @ 2019-07-01  9:38 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, shawnguo, s.hauer,
	kernel, festevam, leonard.crestez, viresh.kumar, daniel.baluta,
	ping.bai, l.stach, abel.vesa, andrew.smirnov, ccaione, angus,
	agx, linux-kernel, devicetree, linux-arm-kernel
  Cc: Linux-imx

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 V3:
	- remove the .prop_name initialization acording to timer-of changes in V4.
---
 drivers/clocksource/timer-imx-sysctr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..b82a549 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 |
@@ -130,6 +130,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 4/5] arm64: dts: imx8mq: Add system counter node
  2019-07-01  9:38 [PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model Anson.Huang
@ 2019-07-01  9:38 ` Anson.Huang
  2019-07-01  9:38 ` [PATCH V4 5/5] arm64: dts: imx8mm: " Anson.Huang
  3 siblings, 0 replies; 9+ messages in thread
From: Anson.Huang @ 2019-07-01  9:38 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, shawnguo, s.hauer,
	kernel, festevam, leonard.crestez, viresh.kumar, daniel.baluta,
	ping.bai, l.stach, abel.vesa, andrew.smirnov, ccaione, angus,
	agx, linux-kernel, devicetree, linux-arm-kernel
  Cc: Linux-imx

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 2ea600c..477c523 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -634,6 +634,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH V4 5/5] arm64: dts: imx8mm: Add system counter node
  2019-07-01  9:38 [PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT Anson.Huang
                   ` (2 preceding siblings ...)
  2019-07-01  9:38 ` [PATCH V4 4/5] arm64: dts: imx8mq: Add system counter node Anson.Huang
@ 2019-07-01  9:38 ` Anson.Huang
  3 siblings, 0 replies; 9+ messages in thread
From: Anson.Huang @ 2019-07-01  9:38 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, shawnguo, s.hauer,
	kernel, festevam, leonard.crestez, viresh.kumar, daniel.baluta,
	ping.bai, l.stach, abel.vesa, andrew.smirnov, ccaione, angus,
	agx, linux-kernel, devicetree, linux-arm-kernel
  Cc: Linux-imx

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>
---
No changes.
---
 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 9d3bb35..ea15457 100644
--- a/arch/arm64/boot/dts/freescale/imx8mm.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mm.dtsi
@@ -527,6 +527,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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property
  2019-07-01  9:38 ` [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property Anson.Huang
@ 2019-07-09 20:30   ` Rob Herring
  2019-07-10  1:30     ` Anson Huang
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2019-07-09 20:30 UTC (permalink / raw)
  To: Anson Huang
  Cc: Daniel Lezcano, Thomas Gleixner, Mark Rutland, Shawn Guo,
	Sascha Hauer, Sascha Hauer, Fabio Estevam, Leonard Crestez,
	Viresh Kumar, Daniel Baluta, Bai Ping, Lucas Stach, Abel Vesa,
	Andrey Smirnov, Carlo Caione, Angus Ainslie (Purism),
	Guido Günther, linux-kernel, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	NXP Linux Team

On Mon, Jul 1, 2019 at 3:47 AM <Anson.Huang@nxp.com> wrote:
>
> From: Anson Huang <Anson.Huang@nxp.com>

'dt-bindings: timer: ...' for the subject.

>
> 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 a DT binding. What's a platform driver?

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

While you've now given a good explanation why you need this, it all
sounds like linux specific issues and a DT change should not be
necessary.

Presumably, 'clocks' points to a fixed-clock node, right? Just parse
the 'clocks' phandle and fetch the frequency from that node if you
need to get the frequency 'early'.

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> No change.
> ---
>  .../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.

It doesn't really work to say one or the other is needed unless you
make the OS support both cases.

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property
  2019-07-09 20:30   ` Rob Herring
@ 2019-07-10  1:30     ` Anson Huang
  2019-07-10 13:23       ` Rob Herring
  0 siblings, 1 reply; 9+ messages in thread
From: Anson Huang @ 2019-07-10  1:30 UTC (permalink / raw)
  To: Rob Herring
  Cc: Daniel Lezcano, Thomas Gleixner, Mark Rutland, Shawn Guo,
	Sascha Hauer, Sascha Hauer, Fabio Estevam, Leonard Crestez,
	Viresh Kumar, Daniel Baluta, Jacky Bai, Lucas Stach, Abel Vesa,
	Andrey Smirnov, Carlo Caione, Angus Ainslie (Purism),
	Guido Günther, linux-kernel, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dl-linux-imx

Hi, Rob

> On Mon, Jul 1, 2019 at 3:47 AM <Anson.Huang@nxp.com> wrote:
> >
> > From: Anson Huang <Anson.Huang@nxp.com>
> 
> 'dt-bindings: timer: ...' for the subject.

OK, I made a mistake.

> 
> >
> > 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 a DT binding. What's a platform driver?

It is just trying to explain why we need to introduce this "clock-frequency"
property, nothing about the binding and platform driver.

> 
> >
> > 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.
> 
> While you've now given a good explanation why you need this, it all sounds
> like linux specific issues and a DT change should not be necessary.
> 
> Presumably, 'clocks' points to a fixed-clock node, right? Just parse the 'clocks'
> phandle and fetch the frequency from that node if you need to get the
> frequency 'early'.

Sound like a better solution, I will try that, since the system counter's clock is
from osc_24m and divided by 3, since different platforms may have different divider,
so maybe I can create a fixed-clock node in DT, then system counter driver can parse
the clock and fetch the frequency from that node, will redo a V5 patch.

> 
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > No change.
> > ---
> >  .../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.
> 
> It doesn't really work to say one or the other is needed unless you make the
> OS support both cases.

The OS already support both cases now with this patch series.

Thanks,
Anson

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property
  2019-07-10  1:30     ` Anson Huang
@ 2019-07-10 13:23       ` Rob Herring
  2019-07-11  1:04         ` Anson Huang
  0 siblings, 1 reply; 9+ messages in thread
From: Rob Herring @ 2019-07-10 13:23 UTC (permalink / raw)
  To: Anson Huang
  Cc: Daniel Lezcano, Thomas Gleixner, Mark Rutland, Shawn Guo,
	Sascha Hauer, Sascha Hauer, Fabio Estevam, Leonard Crestez,
	Viresh Kumar, Daniel Baluta, Jacky Bai, Lucas Stach, Abel Vesa,
	Andrey Smirnov, Carlo Caione, Angus Ainslie (Purism),
	Guido Günther, linux-kernel, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dl-linux-imx

On Tue, Jul 9, 2019 at 7:30 PM Anson Huang <anson.huang@nxp.com> wrote:
>
> Hi, Rob
>
> > On Mon, Jul 1, 2019 at 3:47 AM <Anson.Huang@nxp.com> wrote:
> > >
> > > From: Anson Huang <Anson.Huang@nxp.com>
> >
> > 'dt-bindings: timer: ...' for the subject.
>
> OK, I made a mistake.
>
> >
> > >
> > > 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 a DT binding. What's a platform driver?
>
> It is just trying to explain why we need to introduce this "clock-frequency"
> property, nothing about the binding and platform driver.
>
> >
> > >
> > > 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.
> >
> > While you've now given a good explanation why you need this, it all sounds
> > like linux specific issues and a DT change should not be necessary.
> >
> > Presumably, 'clocks' points to a fixed-clock node, right? Just parse the 'clocks'
> > phandle and fetch the frequency from that node if you need to get the
> > frequency 'early'.
>
> Sound like a better solution, I will try that, since the system counter's clock is
> from osc_24m and divided by 3, since different platforms may have different divider,
> so maybe I can create a fixed-clock node in DT, then system counter driver can parse
> the clock and fetch the frequency from that node, will redo a V5 patch.

The divide by 3 can be implied by the compatible. If you need a
different divider, add another compatible.

> >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > ---
> > > No change.
> > > ---
> > >  .../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.
> >
> > It doesn't really work to say one or the other is needed unless you make the
> > OS support both cases.
>
> The OS already support both cases now with this patch series.

What about FreeBSD or any other OS?

Rob

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property
  2019-07-10 13:23       ` Rob Herring
@ 2019-07-11  1:04         ` Anson Huang
  0 siblings, 0 replies; 9+ messages in thread
From: Anson Huang @ 2019-07-11  1:04 UTC (permalink / raw)
  To: Rob Herring
  Cc: Daniel Lezcano, Thomas Gleixner, Mark Rutland, Shawn Guo,
	Sascha Hauer, Sascha Hauer, Fabio Estevam, Leonard Crestez,
	Viresh Kumar, Daniel Baluta, Jacky Bai, Lucas Stach, Abel Vesa,
	Andrey Smirnov, Carlo Caione, Angus Ainslie (Purism),
	Guido Günther, linux-kernel, devicetree,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dl-linux-imx

Hi, Rob

> On Tue, Jul 9, 2019 at 7:30 PM Anson Huang <anson.huang@nxp.com> wrote:
> >
> > Hi, Rob
> >
> > > On Mon, Jul 1, 2019 at 3:47 AM <Anson.Huang@nxp.com> wrote:
> > > >
> > > > From: Anson Huang <Anson.Huang@nxp.com>
> > >
> > > 'dt-bindings: timer: ...' for the subject.
> >
> > OK, I made a mistake.
> >
> > >
> > > >
> > > > 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 a DT binding. What's a platform driver?
> >
> > It is just trying to explain why we need to introduce this "clock-frequency"
> > property, nothing about the binding and platform driver.
> >
> > >
> > > >
> > > > 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.
> > >
> > > While you've now given a good explanation why you need this, it all
> > > sounds like linux specific issues and a DT change should not be necessary.
> > >
> > > Presumably, 'clocks' points to a fixed-clock node, right? Just parse the
> 'clocks'
> > > phandle and fetch the frequency from that node if you need to get
> > > the frequency 'early'.
> >
> > Sound like a better solution, I will try that, since the system
> > counter's clock is from osc_24m and divided by 3, since different
> > platforms may have different divider, so maybe I can create a
> > fixed-clock node in DT, then system counter driver can parse the clock and
> fetch the frequency from that node, will redo a V5 patch.
> 
> The divide by 3 can be implied by the compatible. If you need a different
> divider, add another compatible.

Yes, we can consider it later, till now, all the platforms used are with an internal
divider of 3 in system counter block, so I make it fixed divided by 3 in system counter
driver.

> 
> > >
> > > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > > > ---
> > > > No change.
> > > > ---
> > > >  .../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.
> > >
> > > It doesn't really work to say one or the other is needed unless you
> > > make the OS support both cases.
> >
> > The OS already support both cases now with this patch series.
> 
> What about FreeBSD or any other OS?

Now that in V5, I use the fixed clock of OSC as clock input of system counter,
no need to have all these changes now. And also no changes needed in DT
binding, thanks for your review.

Anson.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-07-11  1:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-01  9:38 [PATCH V4 1/5] clocksource: timer-of: Support getting clock frequency from DT Anson.Huang
2019-07-01  9:38 ` [PATCH V4 2/5] clocksource/drivers/sysctr: Add clock-frequency property Anson.Huang
2019-07-09 20:30   ` Rob Herring
2019-07-10  1:30     ` Anson Huang
2019-07-10 13:23       ` Rob Herring
2019-07-11  1:04         ` Anson Huang
2019-07-01  9:38 ` [PATCH V4 3/5] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model Anson.Huang
2019-07-01  9:38 ` [PATCH V4 4/5] arm64: dts: imx8mq: Add system counter node Anson.Huang
2019-07-01  9:38 ` [PATCH V4 5/5] arm64: dts: imx8mm: " Anson.Huang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).