[PATCH 1/2] timer: mtk_timer: initialize the timer before use

[PATCH 1/2] timer: mtk_timer: initialize the timer before use

[Weijie Gao]

The timer being used by this driver may have already been used by first
stage bootloader (e.g. ATF/preloader), and it's settings may differ from
what this driver is going to use.

This may cause issues, such as inaccurate timer frequency due to
incorrect clock divider.

This patch adds the initialization code to avoid them.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
 drivers/timer/mtk_timer.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/timer/mtk_timer.c b/drivers/timer/mtk_timer.c
index 448a76a7e1..f6b97f868c 100644
--- a/drivers/timer/mtk_timer.c
+++ b/drivers/timer/mtk_timer.c
@@ -61,6 +61,16 @@ static int mtk_timer_probe(struct udevice *dev)
 	if (!uc_priv->clock_rate)
 		return -EINVAL;
 
+	/*
+	 * Initialize the timer:
+	 * 1. set clock source to system clock with clock divider setting to 1
+	 * 2. set timer mode to free running
+	 * 3. reset timer counter to 0 then enable the timer
+	 */
+	writel(GPT4_CLK_SYS | GPT4_CLK_DIV1, priv->base + MTK_GPT4_CLK);
+	writel(GPT4_FREERUN | GPT4_CLEAR | GPT4_ENABLE,
+	       priv->base + MTK_GPT4_CTRL);
+
 	return 0;
 }
 
-- 
2.17.1

[PATCH 2/2] dts: mt7622: use accurate clock source fot mtk_timer

[Weijie Gao]

The input system clock for mt7622 timer is 10MHz and can be retrieved
through the clk driver.

Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
---
 arch/arm/dts/mt7622.dtsi | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/arch/arm/dts/mt7622.dtsi b/arch/arm/dts/mt7622.dtsi
index d888545809..5c2e0251de 100644
--- a/arch/arm/dts/mt7622.dtsi
+++ b/arch/arm/dts/mt7622.dtsi
@@ -71,16 +71,10 @@
 		compatible = "mediatek,timer";
 		reg = <0x10004000 0x80>;
 		interrupts = <GIC_SPI 152 IRQ_TYPE_LEVEL_LOW>;
-		clocks = <&system_clk>;
+		clocks = <&infracfg CLK_INFRA_APXGPT_PD>;
 		clock-names = "system-clk";
 	};
 
-	system_clk: dummy13m {
-		compatible = "fixed-clock";
-		clock-frequency = <13000000>;
-		#clock-cells = <0>;
-	};
-
 	infracfg: infracfg at 10000000 {
 		compatible = "mediatek,mt7622-infracfg",
 			     "syscon";
-- 
2.17.1

[PATCH 1/2] timer: mtk_timer: initialize the timer before use

[Matthias Brugger]

On Tue, Jan 12, 2021 at 01:44:02PM +0800, Weijie Gao wrote:
> The timer being used by this driver may have already been used by first
> stage bootloader (e.g. ATF/preloader), and it's settings may differ from
> what this driver is going to use.
> 
> This may cause issues, such as inaccurate timer frequency due to
> incorrect clock divider.
> 
> This patch adds the initialization code to avoid them.
> 
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> ---
>  drivers/timer/mtk_timer.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/timer/mtk_timer.c b/drivers/timer/mtk_timer.c
> index 448a76a7e1..f6b97f868c 100644
> --- a/drivers/timer/mtk_timer.c
> +++ b/drivers/timer/mtk_timer.c
> @@ -61,6 +61,16 @@ static int mtk_timer_probe(struct udevice *dev)
>  	if (!uc_priv->clock_rate)
>  		return -EINVAL;
>  
> +	/*
> +	 * Initialize the timer:
> +	 * 1. set clock source to system clock with clock divider setting to 1
> +	 * 2. set timer mode to free running
> +	 * 3. reset timer counter to 0 then enable the timer
> +	 */
> +	writel(GPT4_CLK_SYS | GPT4_CLK_DIV1, priv->base + MTK_GPT4_CLK);
> +	writel(GPT4_FREERUN | GPT4_CLEAR | GPT4_ENABLE,

GPT4_FREERUN is defined as GENMASK(5,4) while in the Linux kernel it has
the value of 3. Can you explain where this difference comes from?

Regards,
Matthias

> +	       priv->base + MTK_GPT4_CTRL);
> +
>  	return 0;
>  }
>  
> -- 
> 2.17.1

[PATCH 1/2] timer: mtk_timer: initialize the timer before use

[Weijie Gao]

On Fri, 2021-01-15 at 17:36 +0100, Matthias Brugger wrote:
> On Tue, Jan 12, 2021 at 01:44:02PM +0800, Weijie Gao wrote:
> > The timer being used by this driver may have already been used by first
> > stage bootloader (e.g. ATF/preloader), and it's settings may differ from
> > what this driver is going to use.
> > 
> > This may cause issues, such as inaccurate timer frequency due to
> > incorrect clock divider.
> > 
> > This patch adds the initialization code to avoid them.
> > 
> > Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> > ---
> >  drivers/timer/mtk_timer.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/timer/mtk_timer.c b/drivers/timer/mtk_timer.c
> > index 448a76a7e1..f6b97f868c 100644
> > --- a/drivers/timer/mtk_timer.c
> > +++ b/drivers/timer/mtk_timer.c
> > @@ -61,6 +61,16 @@ static int mtk_timer_probe(struct udevice *dev)
> >  	if (!uc_priv->clock_rate)
> >  		return -EINVAL;
> >  
> > +	/*
> > +	 * Initialize the timer:
> > +	 * 1. set clock source to system clock with clock divider setting to 1
> > +	 * 2. set timer mode to free running
> > +	 * 3. reset timer counter to 0 then enable the timer
> > +	 */
> > +	writel(GPT4_CLK_SYS | GPT4_CLK_DIV1, priv->base + MTK_GPT4_CLK);
> > +	writel(GPT4_FREERUN | GPT4_CLEAR | GPT4_ENABLE,
> 
> GPT4_FREERUN is defined as GENMASK(5,4) while in the Linux kernel it has
> the value of 3. Can you explain where this difference comes from?

GENMASK(5,4) == 3 << 4, which is exactly the same as what's being used
in kernel:

#define GPT_CTRL_OP(val)        (((val) & 0x3) << 4)
#define GPT_CTRL_OP_FREERUN     (3)

> 
> Regards,
> Matthias
> 
> > +	       priv->base + MTK_GPT4_CTRL);
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.17.1

[PATCH 1/2] timer: mtk_timer: initialize the timer before use

[Matthias Brugger]

On Mon, Jan 18, 2021 at 08:44:42AM +0800, Weijie Gao wrote:
> On Fri, 2021-01-15 at 17:36 +0100, Matthias Brugger wrote:
> > On Tue, Jan 12, 2021 at 01:44:02PM +0800, Weijie Gao wrote:
> > > The timer being used by this driver may have already been used by first
> > > stage bootloader (e.g. ATF/preloader), and it's settings may differ from
> > > what this driver is going to use.
> > > 
> > > This may cause issues, such as inaccurate timer frequency due to
> > > incorrect clock divider.
> > > 
> > > This patch adds the initialization code to avoid them.
> > > 
> > > Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>
> > > ---
> > >  drivers/timer/mtk_timer.c | 10 ++++++++++
> > >  1 file changed, 10 insertions(+)
> > > 
> > > diff --git a/drivers/timer/mtk_timer.c b/drivers/timer/mtk_timer.c
> > > index 448a76a7e1..f6b97f868c 100644
> > > --- a/drivers/timer/mtk_timer.c
> > > +++ b/drivers/timer/mtk_timer.c
> > > @@ -61,6 +61,16 @@ static int mtk_timer_probe(struct udevice *dev)
> > >  	if (!uc_priv->clock_rate)
> > >  		return -EINVAL;
> > >  
> > > +	/*
> > > +	 * Initialize the timer:
> > > +	 * 1. set clock source to system clock with clock divider setting to 1
> > > +	 * 2. set timer mode to free running
> > > +	 * 3. reset timer counter to 0 then enable the timer
> > > +	 */
> > > +	writel(GPT4_CLK_SYS | GPT4_CLK_DIV1, priv->base + MTK_GPT4_CLK);
> > > +	writel(GPT4_FREERUN | GPT4_CLEAR | GPT4_ENABLE,
> > 
> > GPT4_FREERUN is defined as GENMASK(5,4) while in the Linux kernel it has
> > the value of 3. Can you explain where this difference comes from?
> 
> GENMASK(5,4) == 3 << 4, which is exactly the same as what's being used
> in kernel:
> 
> #define GPT_CTRL_OP(val)        (((val) & 0x3) << 4)
> #define GPT_CTRL_OP_FREERUN     (3)
> 

You are right, sorry my fault:
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>

> > 
> > Regards,
> > Matthias
> > 
> > > +	       priv->base + MTK_GPT4_CTRL);
> > > +
> > >  	return 0;
> > >  }
> > >  
> > > -- 
> > > 2.17.1
> 

[PATCH 1/2] timer: mtk_timer: initialize the timer before use

[Tom Rini]

On Tue, Jan 12, 2021 at 01:44:02PM +0800, Weijie Gao wrote:

> The timer being used by this driver may have already been used by first
> stage bootloader (e.g. ATF/preloader), and it's settings may differ from
> what this driver is going to use.
> 
> This may cause issues, such as inaccurate timer frequency due to
> incorrect clock divider.
> 
> This patch adds the initialization code to avoid them.
> 
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210119/1d04380c/attachment.sig>

[PATCH 2/2] dts: mt7622: use accurate clock source fot mtk_timer

[Tom Rini]

On Tue, Jan 12, 2021 at 01:44:11PM +0800, Weijie Gao wrote:

> The input system clock for mt7622 timer is 10MHz and can be retrieved
> through the clk driver.
> 
> Signed-off-by: Weijie Gao <weijie.gao@mediatek.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210119/ef7666bd/attachment.sig>