| Message ID | 0dc6f5e159ec0bc471483c77192237f42dbcc646.1610424657.git.weijie.gao@mediatek.com |
|---|---|
| State | Accepted |
| Commit | 63779b240711cb1a0761bbceb323f5e9558394cc |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/2] timer: mtk_timer: initialize the timer before use | expand |
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
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
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 >
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!
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; }
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(+)