[PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional 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 the optinal clock-frequency to the device tree bindings of the NXP
system counter, so the frequency can be handed in and the of_clk
operations can be skipped.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V1:
	- improve commit log, no content change.
---
 Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
index d576599..c9907a0 100644
--- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
@@ -14,6 +14,11 @@ Required properties:
 - clocks : 	    Specifies the counter clock.
 - clock-names: 	    Specifies the clock's name of this module
 
+Optional properties:
+
+- clock-frequency : Specifies system counter clock frequency and indicates system
+		    counter driver to skip clock operations.
+
 Example:
 
 	system_counter: timer@306a0000 {
@@ -22,4 +27,5 @@ Example:
 		clocks = <&clk_8m>;
 		clock-names = "per";
 		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
+		clock-frequency = <8333333>;
 	};
-- 
2.7.4

[PATCH RESEND V2 2/3] 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, add an
option of skipping of_clk operation for system counter driver,
an optional property "clock-frequency" is introduced to pass
the frequency value to system counter driver and indicate driver
to skip of_clk operations.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Changes since V1:
	- improve commit log, no content change.
---
 drivers/clocksource/timer-imx-sysctr.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
index fd7d680..8ff3d7e 100644
--- a/drivers/clocksource/timer-imx-sysctr.c
+++ b/drivers/clocksource/timer-imx-sysctr.c
@@ -129,6 +129,14 @@ static void __init sysctr_clockevent_init(void)
 static int __init sysctr_timer_init(struct device_node *np)
 {
 	int ret = 0;
+	u32 rate;
+
+	if (!of_property_read_u32(np, "clock-frequency",
+				  &rate)) {
+		to_sysctr.of_clk.rate = rate;
+		to_sysctr.of_clk.period = DIV_ROUND_UP(rate, HZ);
+		to_sysctr.flags &= ~TIMER_OF_CLOCK;
+	}
 
 	ret = timer_of_init(np, &to_sysctr);
 	if (ret)
-- 
2.7.4

[PATCH RESEND V2 3/3] 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 change.
---
 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 d09b808..9d99191 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -635,6 +635,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

Re: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Daniel Lezcano]

On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> 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 the optinal clock-frequency to the device tree bindings of the NXP
> system counter, so the frequency can be handed in and the of_clk
> operations can be skipped.

Isn't it possible to create a fixed-clock and refer to it? So no need to
create a specific action before calling timer_of_init() ?

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- improve commit log, no content change.
> ---
>  Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> index d576599..c9907a0 100644
> --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> @@ -14,6 +14,11 @@ Required properties:
>  - clocks : 	    Specifies the counter clock.
>  - clock-names: 	    Specifies the clock's name of this module
>  
> +Optional properties:
> +
> +- clock-frequency : Specifies system counter clock frequency and indicates system
> +		    counter driver to skip clock operations.
> +
>  Example:
>  
>  	system_counter: timer@306a0000 {
> @@ -22,4 +27,5 @@ Example:
>  		clocks = <&clk_8m>;
>  		clock-names = "per";
>  		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
> +		clock-frequency = <8333333>;
>  	};
> 


-- 
 <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 RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Anson Huang]

Hi, Daniel

> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> > 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 the optinal clock-frequency to the device tree bindings of the NXP
> > system counter, so the frequency can be handed in and the of_clk
> > operations can be skipped.
> 
> Isn't it possible to create a fixed-clock and refer to it? So no need to create a
> specific action before calling timer_of_init() ?
> 

As the clock must be ready before the TIMER_OF_DECLARE, so adding a CLK_OF_DECLARE_DRIVER in
clock driver to ONLY register a fixed-clock? The system counter's frequency are different on different
platforms, so adding fixed clock in system counter driver is NOT a good idea, ONLY the DT node or the
clock driver can create this fixed clock according to platforms, can you advise where to create this fixed
clock is better?

Thanks,
Anson 

Re: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Daniel Lezcano]

On 26/06/2019 03:42, Anson Huang wrote:
> Hi, Daniel
> 
>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
>>> 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 the optinal clock-frequency to the device tree bindings of the NXP
>>> system counter, so the frequency can be handed in and the of_clk
>>> operations can be skipped.
>>
>> Isn't it possible to create a fixed-clock and refer to it? So no need to create a
>> specific action before calling timer_of_init() ?
>>
> 
> As the clock must be ready before the TIMER_OF_DECLARE, so adding a CLK_OF_DECLARE_DRIVER in
> clock driver to ONLY register a fixed-clock? The system counter's frequency are different on different
> platforms, so adding fixed clock in system counter driver is NOT a good idea, ONLY the DT node or the
> clock driver can create this fixed clock according to platforms, can you advise where to create this fixed
> clock is better?

Can you point me to a DT with the "nxp,sysctr-timer" ?


-- 
 <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 RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Anson Huang]

Hi, Daniel

> On 26/06/2019 03:42, Anson Huang wrote:
> > Hi, Daniel
> >
> >> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> >>> 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 the optinal clock-frequency to the device tree bindings of the
> >>> NXP system counter, so the frequency can be handed in and the of_clk
> >>> operations can be skipped.
> >>
> >> Isn't it possible to create a fixed-clock and refer to it? So no need
> >> to create a specific action before calling timer_of_init() ?
> >>
> >
> > As the clock must be ready before the TIMER_OF_DECLARE, so adding a
> > CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock?
> > The system counter's frequency are different on different platforms,
> > so adding fixed clock in system counter driver is NOT a good idea,
> > ONLY the DT node or the clock driver can create this fixed clock according to
> platforms, can you advise where to create this fixed clock is better?
> 
> Can you point me to a DT with the "nxp,sysctr-timer" ?

The DT node of system counter is new added in 3/3 of this patch series, also can be found
from below link:
https://patchwork.kernel.org/patch/11011703/

thanks,
Anson

Re: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Daniel Lezcano]

On 27/06/2019 02:43, Anson Huang wrote:
> Hi, Daniel
> 
>> On 26/06/2019 03:42, Anson Huang wrote:
>>> Hi, Daniel
>>>
>>>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
>>>>> 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 the optinal clock-frequency to the device tree bindings of the
>>>>> NXP system counter, so the frequency can be handed in and the of_clk
>>>>> operations can be skipped.
>>>>
>>>> Isn't it possible to create a fixed-clock and refer to it? So no need
>>>> to create a specific action before calling timer_of_init() ?
>>>>
>>>
>>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a
>>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock?
>>> The system counter's frequency are different on different platforms,
>>> so adding fixed clock in system counter driver is NOT a good idea,
>>> ONLY the DT node or the clock driver can create this fixed clock according to
>> platforms, can you advise where to create this fixed clock is better?
>>
>> Can you point me to a DT with the "nxp,sysctr-timer" ?
> 
> The DT node of system counter is new added in 3/3 of this patch series, also can be found
> from below link:
> https://patchwork.kernel.org/patch/11011703/

Sorry, I was unclear. I meant a patch with the timer defined using a
clock as defined currently in the binding (no clock-frequency).


-- 
 <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 RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Anson Huang]

Hi, Daniel

> On 27/06/2019 02:43, Anson Huang wrote:
> > Hi, Daniel
> >
> >> On 26/06/2019 03:42, Anson Huang wrote:
> >>> Hi, Daniel
> >>>
> >>>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> >>>>> 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 the optinal clock-frequency to the device tree bindings of the
> >>>>> NXP system counter, so the frequency can be handed in and the
> >>>>> of_clk operations can be skipped.
> >>>>
> >>>> Isn't it possible to create a fixed-clock and refer to it? So no
> >>>> need to create a specific action before calling timer_of_init() ?
> >>>>
> >>>
> >>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a
> >>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock?
> >>> The system counter's frequency are different on different platforms,
> >>> so adding fixed clock in system counter driver is NOT a good idea,
> >>> ONLY the DT node or the clock driver can create this fixed clock
> >>> according to
> >> platforms, can you advise where to create this fixed clock is better?
> >>
> >> Can you point me to a DT with the "nxp,sysctr-timer" ?
> >
> > The DT node of system counter is new added in 3/3 of this patch
> > series, also can be found from below link:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fpatch%2F11011703%2F&amp;data=02%7C01%7Canson.
> huang%
> >
> 40nxp.com%7C8b9519ecceb346712be808d6fad675e4%7C686ea1d3bc2b4c6f
> a92cd99
> >
> c5c301635%7C0%7C0%7C636972196338405582&amp;sdata=sOQQzDFxoCqe
> VuHFuYPHh
> > F8Bdj2Zu9WS7Go%2FV9lrWa8%3D&amp;reserved=0
> 
> Sorry, I was unclear. I meant a patch with the timer defined using a clock as
> defined currently in the binding (no clock-frequency).

OK, for i.MX8MM, we use clocks, check below patch series:

https://patchwork.kernel.org/patch/11008519/

code piece as below:

+			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>;
+				clocks = <&clk IMX8MM_CLK_SYS_CTR>;
+				clock-names = "per";
+			};

Thanks,
Anson.

Re: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Daniel Lezcano]

Hi Anson,

On 27/06/2019 10:11, Anson Huang wrote:
> Hi, Daniel
> 
>> On 27/06/2019 02:43, Anson Huang wrote:
>>> Hi, Daniel
>>>
>>>> On 26/06/2019 03:42, Anson Huang wrote:
>>>>> Hi, Daniel
>>>>>
>>>>>> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
>>>>>>> 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 the optinal clock-frequency to the device tree bindings of the
>>>>>>> NXP system counter, so the frequency can be handed in and the
>>>>>>> of_clk operations can be skipped.
>>>>>>
>>>>>> Isn't it possible to create a fixed-clock and refer to it? So no
>>>>>> need to create a specific action before calling timer_of_init() ?
>>>>>>
>>>>>
>>>>> As the clock must be ready before the TIMER_OF_DECLARE, so adding a
>>>>> CLK_OF_DECLARE_DRIVER in clock driver to ONLY register a fixed-clock?
>>>>> The system counter's frequency are different on different platforms,
>>>>> so adding fixed clock in system counter driver is NOT a good idea,
>>>>> ONLY the DT node or the clock driver can create this fixed clock
>>>>> according to
>>>> platforms, can you advise where to create this fixed clock is better?
>>>>
>>>> Can you point me to a DT with the "nxp,sysctr-timer" ?
>>>
>>> The DT node of system counter is new added in 3/3 of this patch
>>> series, also can be found from below link:
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
>>>
>> hwork.kernel.org%2Fpatch%2F11011703%2F&amp;data=02%7C01%7Canson.
>> huang%
>>>
>> 40nxp.com%7C8b9519ecceb346712be808d6fad675e4%7C686ea1d3bc2b4c6f
>> a92cd99
>>>
>> c5c301635%7C0%7C0%7C636972196338405582&amp;sdata=sOQQzDFxoCqe
>> VuHFuYPHh
>>> F8Bdj2Zu9WS7Go%2FV9lrWa8%3D&amp;reserved=0
>>
>> Sorry, I was unclear. I meant a patch with the timer defined using a clock as
>> defined currently in the binding (no clock-frequency).
> 
> OK, for i.MX8MM, we use clocks, check below patch series:
> 
> https://patchwork.kernel.org/patch/11008519/
> 
> code piece as below:
> 
> +			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>;
> +				clocks = <&clk IMX8MM_CLK_SYS_CTR>;
> +				clock-names = "per";
> +			};

Thanks,

The fixed-clock can help to keep the code and the DT definition for the
timer untouched as the 'clocks' above will refer to it. But that means
we describe a fake clock. So it is up to you to decide if you want to
stick the clock-frequency or use a fixed-clock.




-- 
 <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 RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Daniel Lezcano]

On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> 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 the optinal clock-frequency to the device tree bindings of the NXP
> system counter, so the frequency can be handed in and the of_clk
> operations can be skipped.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- improve commit log, no content change.
> ---
>  Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> index d576599..c9907a0 100644
> --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> @@ -14,6 +14,11 @@ Required properties:
>  - clocks : 	    Specifies the counter clock.
>  - clock-names: 	    Specifies the clock's name of this module
>  
> +Optional properties:
> +
> +- clock-frequency : Specifies system counter clock frequency and indicates system
> +		    counter driver to skip clock operations.
> +

Shouldn't it be required and mutually exclusive with clocks/clock-names?

>  Example:
>  
>  	system_counter: timer@306a0000 {
> @@ -22,4 +27,5 @@ Example:
>  		clocks = <&clk_8m>;
>  		clock-names = "per";
>  		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
> +		clock-frequency = <8333333>;
>  	};
> 


-- 
 <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 RESEND V2 2/3] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

[Daniel Lezcano]

On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> 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, add an
> option of skipping of_clk operation for system counter driver,
> an optional property "clock-frequency" is introduced to pass
> the frequency value to system counter driver and indicate driver
> to skip of_clk operations.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Changes since V1:
> 	- improve commit log, no content change.
> ---
>  drivers/clocksource/timer-imx-sysctr.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/clocksource/timer-imx-sysctr.c b/drivers/clocksource/timer-imx-sysctr.c
> index fd7d680..8ff3d7e 100644
> --- a/drivers/clocksource/timer-imx-sysctr.c
> +++ b/drivers/clocksource/timer-imx-sysctr.c
> @@ -129,6 +129,14 @@ static void __init sysctr_clockevent_init(void)
>  static int __init sysctr_timer_init(struct device_node *np)
>  {
>  	int ret = 0;
> +	u32 rate;
> +
> +	if (!of_property_read_u32(np, "clock-frequency",
> +				  &rate)) {
> +		to_sysctr.of_clk.rate = rate;
> +		to_sysctr.of_clk.period = DIV_ROUND_UP(rate, HZ);
> +		to_sysctr.flags &= ~TIMER_OF_CLOCK;
> +	}

Please take the opportunity to add the TIMER_OF_CLOCK_FREQUENCY flag in
timer-of and the corresponding code above.

Then check the clock-frequency presence and add TIMER_OF_CLOCK or
TIMER_OF_CLOCK_FREQUENCY flag to the timer-of structure.

eg:

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


-- 
 <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 RESEND V2 2/3] clocksource: imx-sysctr: Make timer work with clock driver using platform driver model

[Anson Huang]

Hi, Daniel

> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> > 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, add an option
> > of skipping of_clk operation for system counter driver, an optional
> > property "clock-frequency" is introduced to pass the frequency value
> > to system counter driver and indicate driver to skip of_clk
> > operations.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V1:
> > 	- improve commit log, no content change.
> > ---
> >  drivers/clocksource/timer-imx-sysctr.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/clocksource/timer-imx-sysctr.c
> > b/drivers/clocksource/timer-imx-sysctr.c
> > index fd7d680..8ff3d7e 100644
> > --- a/drivers/clocksource/timer-imx-sysctr.c
> > +++ b/drivers/clocksource/timer-imx-sysctr.c
> > @@ -129,6 +129,14 @@ static void __init sysctr_clockevent_init(void)
> > static int __init sysctr_timer_init(struct device_node *np)  {
> >  	int ret = 0;
> > +	u32 rate;
> > +
> > +	if (!of_property_read_u32(np, "clock-frequency",
> > +				  &rate)) {
> > +		to_sysctr.of_clk.rate = rate;
> > +		to_sysctr.of_clk.period = DIV_ROUND_UP(rate, HZ);
> > +		to_sysctr.flags &= ~TIMER_OF_CLOCK;
> > +	}
> 
> Please take the opportunity to add the TIMER_OF_CLOCK_FREQUENCY flag in
> timer-of and the corresponding code above.

OK, so another patch for timer-of is necessary, if TIMER_OF_CLOCK_FREQUENCY flag
is present, just read the "clock-frequency" from DT instead of getting clock rate from
clock tree, right? I think this becomes a common case for timer driver, as more and more
platforms will use platform driver model for clock driver, it would be good if timer-of can
provide solution.

> 
> Then check the clock-frequency presence and add TIMER_OF_CLOCK or
> TIMER_OF_CLOCK_FREQUENCY flag to the timer-of structure.
> 
> eg:
> 
>     to_sysctr.flags |= of_find_property(np, "clock-frequency", NULL) ?
> 		TIMER_OF_CLOCK_FREQUENCY : TIMER_OF_CLOCK;
> 

OK.

Thanks,
Anson.

RE: [PATCH RESEND V2 1/3] clocksource/drivers/sysctr: Add optional clock-frequency property

[Anson Huang]

Hi, Daniel

> On 23/06/2019 14:38, Anson.Huang@nxp.com wrote:
> > 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 the optinal clock-frequency to the device tree bindings of the NXP
> > system counter, so the frequency can be handed in and the of_clk
> > operations can be skipped.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Changes since V1:
> > 	- improve commit log, no content change.
> > ---
> >  Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt | 6
> > ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > index d576599..c9907a0 100644
> > --- a/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > +++ b/Documentation/devicetree/bindings/timer/nxp,sysctr-timer.txt
> > @@ -14,6 +14,11 @@ Required properties:
> >  - clocks : 	    Specifies the counter clock.
> >  - clock-names: 	    Specifies the clock's name of this module
> >
> > +Optional properties:
> > +
> > +- clock-frequency : Specifies system counter clock frequency and indicates
> system
> > +		    counter driver to skip clock operations.
> > +
> 
> Shouldn't it be required and mutually exclusive with clocks/clock-names?
>
Yes, make sense, I ever thought about it when doing this patch, but eventually I picked
the optional...will fix it in next version.

Thanks,
Anson

> >  Example:
> >
> >  	system_counter: timer@306a0000 {
> > @@ -22,4 +27,5 @@ Example:
> >  		clocks = <&clk_8m>;
> >  		clock-names = "per";
> >  		interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
> > +		clock-frequency = <8333333>;
> >  	};
> >
> 
>