LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow
@ 2018-03-26 7:47 Anson Huang
2018-03-26 7:47 ` [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support Anson Huang
2018-03-27 15:06 ` [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Rob Herring
0 siblings, 2 replies; 6+ messages in thread
From: Anson Huang @ 2018-03-26 7:47 UTC (permalink / raw)
To: daniel.lezcano, tglx, robh+dt, mark.rutland, aisheng.dong
Cc: Linux-imx, linux-kernel, devicetree
According to i.MX7ULP reference manual, TPM_SC_CPWMS can ONLY be
written when counter is disabled, TPM_SC_TOF is write-1-clear,
TPM_C0SC_CHF is also write-1-clear, correct these registers
initialization flow;
Replace incorret clock name igp with ipg.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt | 2 +-
drivers/clocksource/timer-imx-tpm.c | 10 ++++++++--
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
index b4aa7dd..f82087b 100644
--- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
+++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
@@ -15,7 +15,7 @@ Required properties:
- interrupts : Should be the clock event device interrupt.
- clocks : The clocks provided by the SoC to drive the timer, must contain
an entry for each entry in clock-names.
-- clock-names : Must include the following entries: "igp" and "per".
+- clock-names : Must include the following entries: "ipg" and "per".
Example:
tpm5: tpm@40260000 {
diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index 21bffdc..7403e49 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -20,6 +20,7 @@
#define TPM_SC 0x10
#define TPM_SC_CMOD_INC_PER_CNT (0x1 << 3)
#define TPM_SC_CMOD_DIV_DEFAULT 0x3
+#define TPM_SC_TOF_MASK (0x1 << 7)
#define TPM_CNT 0x14
#define TPM_MOD 0x18
#define TPM_STATUS 0x1c
@@ -29,6 +30,7 @@
#define TPM_C0SC_MODE_SHIFT 2
#define TPM_C0SC_MODE_MASK 0x3c
#define TPM_C0SC_MODE_SW_COMPARE 0x4
+#define TPM_C0SC_CHF_MASK (0x1 << 7)
#define TPM_C0V 0x24
static void __iomem *timer_base;
@@ -179,7 +181,7 @@ static int __init tpm_timer_init(struct device_node *np)
ipg = of_clk_get_by_name(np, "ipg");
per = of_clk_get_by_name(np, "per");
if (IS_ERR(ipg) || IS_ERR(per)) {
- pr_err("tpm: failed to get igp or per clk\n");
+ pr_err("tpm: failed to get ipg or per clk\n");
ret = -ENODEV;
goto err_clk_get;
}
@@ -205,9 +207,13 @@ static int __init tpm_timer_init(struct device_node *np)
* 4) Channel0 disabled
* 5) DMA transfers disabled
*/
+ /* make sure counter is disabled */
writel(0, timer_base + TPM_SC);
+ /* TOF is W1C */
+ writel(TPM_SC_TOF_MASK, timer_base + TPM_SC);
writel(0, timer_base + TPM_CNT);
- writel(0, timer_base + TPM_C0SC);
+ /* CHF is W1C */
+ writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
/* increase per cnt, div 8 by default */
writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support
2018-03-26 7:47 [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Anson Huang
@ 2018-03-26 7:47 ` Anson Huang
2018-03-26 14:10 ` Daniel Lezcano
2018-03-27 15:06 ` [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Rob Herring
1 sibling, 1 reply; 6+ messages in thread
From: Anson Huang @ 2018-03-26 7:47 UTC (permalink / raw)
To: daniel.lezcano, tglx, robh+dt, mark.rutland, aisheng.dong
Cc: Linux-imx, linux-kernel, devicetree
Different TPM modules have different width counters which
is 16-bit or 32-bit, the counter width can be read from
TPM_PARAM register bit[23:16], this patch adds dynamic
check for counter width to support both 16-bit and 32-bit
TPM modules.
Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
drivers/clocksource/timer-imx-tpm.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
index 7403e49..5063b77 100644
--- a/drivers/clocksource/timer-imx-tpm.c
+++ b/drivers/clocksource/timer-imx-tpm.c
@@ -17,6 +17,9 @@
#include <linux/of_irq.h>
#include <linux/sched_clock.h>
+#define TPM_PARAM 0x4
+#define TPM_PARAM_WIDTH_SHIFT 16
+#define TPM_PARAM_WIDTH_MASK (0xff << 16)
#define TPM_SC 0x10
#define TPM_SC_CMOD_INC_PER_CNT (0x1 << 3)
#define TPM_SC_CMOD_DIV_DEFAULT 0x3
@@ -33,6 +36,7 @@
#define TPM_C0SC_CHF_MASK (0x1 << 7)
#define TPM_C0V 0x24
+static int counter_width;
static void __iomem *timer_base;
static struct clock_event_device clockevent_tpm;
@@ -66,7 +70,7 @@ static struct delay_timer tpm_delay_timer;
static inline unsigned long tpm_read_counter(void)
{
- return readl(timer_base + TPM_CNT);
+ return readl(timer_base + TPM_CNT) & GENMASK(counter_width - 1, 0);
}
static unsigned long tpm_read_current_timer(void)
@@ -85,10 +89,11 @@ static int __init tpm_clocksource_init(unsigned long rate)
tpm_delay_timer.freq = rate;
register_current_timer_delay(&tpm_delay_timer);
- sched_clock_register(tpm_read_sched_clock, 32, rate);
+ sched_clock_register(tpm_read_sched_clock, counter_width, rate);
return clocksource_mmio_init(timer_base + TPM_CNT, "imx-tpm",
- rate, 200, 32, clocksource_mmio_readl_up);
+ rate, 200, counter_width,
+ clocksource_mmio_readl_up);
}
static int tpm_set_next_event(unsigned long delta,
@@ -153,8 +158,8 @@ static int __init tpm_clockevent_init(unsigned long rate, int irq)
clockevent_tpm.cpumask = cpumask_of(0);
clockevent_tpm.irq = irq;
- clockevents_config_and_register(&clockevent_tpm,
- rate, 300, 0xfffffffe);
+ clockevents_config_and_register(&clockevent_tpm, rate, 300,
+ GENMASK(counter_width - 1, 1));
return ret;
}
@@ -199,6 +204,8 @@ static int __init tpm_timer_init(struct device_node *np)
goto err_per_clk_enable;
}
+ counter_width = (readl(timer_base + TPM_PARAM) & TPM_PARAM_WIDTH_MASK)
+ >> TPM_PARAM_WIDTH_SHIFT;
/*
* Initialize tpm module to a known state
* 1) Counter disabled
@@ -220,7 +227,7 @@ static int __init tpm_timer_init(struct device_node *np)
timer_base + TPM_SC);
/* set MOD register to maximum for free running mode */
- writel(0xffffffff, timer_base + TPM_MOD);
+ writel(GENMASK(counter_width - 1, 0), timer_base + TPM_MOD);
rate = clk_get_rate(per) >> 3;
ret = tpm_clocksource_init(rate);
--
2.7.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support
2018-03-26 7:47 ` [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support Anson Huang
@ 2018-03-26 14:10 ` Daniel Lezcano
2018-03-28 2:24 ` Anson Huang
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2018-03-26 14:10 UTC (permalink / raw)
To: Anson Huang, tglx, robh+dt, mark.rutland, aisheng.dong
Cc: Linux-imx, linux-kernel, devicetree
On 26/03/2018 09:47, Anson Huang wrote:
> Different TPM modules have different width counters which
> is 16-bit or 32-bit, the counter width can be read from
> TPM_PARAM register bit[23:16], this patch adds dynamic
> check for counter width to support both 16-bit and 32-bit
> TPM modules.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
Are you sure you shouldn't use a prescalar with 16b and reduce its
rating? What will be the duration of the clocksource before wrapping up?
Also, can you do the change to prevent computing the mask at each
'read_count' call ?
> ---
> drivers/clocksource/timer-imx-tpm.c | 19 +++++++++++++------
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
> index 7403e49..5063b77 100644
> --- a/drivers/clocksource/timer-imx-tpm.c
> +++ b/drivers/clocksource/timer-imx-tpm.c
> @@ -17,6 +17,9 @@
> #include <linux/of_irq.h>
> #include <linux/sched_clock.h>
>
> +#define TPM_PARAM 0x4
> +#define TPM_PARAM_WIDTH_SHIFT 16
> +#define TPM_PARAM_WIDTH_MASK (0xff << 16)
> #define TPM_SC 0x10
> #define TPM_SC_CMOD_INC_PER_CNT (0x1 << 3)
> #define TPM_SC_CMOD_DIV_DEFAULT 0x3
> @@ -33,6 +36,7 @@
> #define TPM_C0SC_CHF_MASK (0x1 << 7)
> #define TPM_C0V 0x24
>
> +static int counter_width;
> static void __iomem *timer_base;
> static struct clock_event_device clockevent_tpm;
>
> @@ -66,7 +70,7 @@ static struct delay_timer tpm_delay_timer;
>
> static inline unsigned long tpm_read_counter(void)
> {
> - return readl(timer_base + TPM_CNT);
> + return readl(timer_base + TPM_CNT) & GENMASK(counter_width - 1, 0);
> }
>
> static unsigned long tpm_read_current_timer(void)
> @@ -85,10 +89,11 @@ static int __init tpm_clocksource_init(unsigned long rate)
> tpm_delay_timer.freq = rate;
> register_current_timer_delay(&tpm_delay_timer);
>
> - sched_clock_register(tpm_read_sched_clock, 32, rate);
> + sched_clock_register(tpm_read_sched_clock, counter_width, rate);
>
> return clocksource_mmio_init(timer_base + TPM_CNT, "imx-tpm",
> - rate, 200, 32, clocksource_mmio_readl_up);
> + rate, 200, counter_width,
> + clocksource_mmio_readl_up);
> }
>
> static int tpm_set_next_event(unsigned long delta,
> @@ -153,8 +158,8 @@ static int __init tpm_clockevent_init(unsigned long rate, int irq)
>
> clockevent_tpm.cpumask = cpumask_of(0);
> clockevent_tpm.irq = irq;
> - clockevents_config_and_register(&clockevent_tpm,
> - rate, 300, 0xfffffffe);
> + clockevents_config_and_register(&clockevent_tpm, rate, 300,
> + GENMASK(counter_width - 1, 1));
>
> return ret;
> }
> @@ -199,6 +204,8 @@ static int __init tpm_timer_init(struct device_node *np)
> goto err_per_clk_enable;
> }
>
> + counter_width = (readl(timer_base + TPM_PARAM) & TPM_PARAM_WIDTH_MASK)
> + >> TPM_PARAM_WIDTH_SHIFT;
> /*
> * Initialize tpm module to a known state
> * 1) Counter disabled
> @@ -220,7 +227,7 @@ static int __init tpm_timer_init(struct device_node *np)
> timer_base + TPM_SC);
>
> /* set MOD register to maximum for free running mode */
> - writel(0xffffffff, timer_base + TPM_MOD);
> + writel(GENMASK(counter_width - 1, 0), timer_base + TPM_MOD);
>
> rate = clk_get_rate(per) >> 3;
> ret = tpm_clocksource_init(rate);
>
--
<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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow
2018-03-26 7:47 [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Anson Huang
2018-03-26 7:47 ` [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support Anson Huang
@ 2018-03-27 15:06 ` Rob Herring
2018-03-28 1:46 ` Anson Huang
1 sibling, 1 reply; 6+ messages in thread
From: Rob Herring @ 2018-03-27 15:06 UTC (permalink / raw)
To: Anson Huang
Cc: daniel.lezcano, tglx, mark.rutland, aisheng.dong, Linux-imx,
linux-kernel, devicetree
On Mon, Mar 26, 2018 at 03:47:40PM +0800, Anson Huang wrote:
> According to i.MX7ULP reference manual, TPM_SC_CPWMS can ONLY be
> written when counter is disabled, TPM_SC_TOF is write-1-clear,
> TPM_C0SC_CHF is also write-1-clear, correct these registers
> initialization flow;
>
> Replace incorret clock name igp with ipg.
incorrect
This looks like an unrelated change.
>
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
> Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt | 2 +-
Though you have the same typo, this should be a separate change from the
driver.
> drivers/clocksource/timer-imx-tpm.c | 10 ++++++++--
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> index b4aa7dd..f82087b 100644
> --- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> +++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> @@ -15,7 +15,7 @@ Required properties:
> - interrupts : Should be the clock event device interrupt.
> - clocks : The clocks provided by the SoC to drive the timer, must contain
> an entry for each entry in clock-names.
> -- clock-names : Must include the following entries: "igp" and "per".
> +- clock-names : Must include the following entries: "ipg" and "per".
>
> Example:
> tpm5: tpm@40260000 {
> diff --git a/drivers/clocksource/timer-imx-tpm.c b/drivers/clocksource/timer-imx-tpm.c
> index 21bffdc..7403e49 100644
> --- a/drivers/clocksource/timer-imx-tpm.c
> +++ b/drivers/clocksource/timer-imx-tpm.c
> @@ -20,6 +20,7 @@
> #define TPM_SC 0x10
> #define TPM_SC_CMOD_INC_PER_CNT (0x1 << 3)
> #define TPM_SC_CMOD_DIV_DEFAULT 0x3
> +#define TPM_SC_TOF_MASK (0x1 << 7)
> #define TPM_CNT 0x14
> #define TPM_MOD 0x18
> #define TPM_STATUS 0x1c
> @@ -29,6 +30,7 @@
> #define TPM_C0SC_MODE_SHIFT 2
> #define TPM_C0SC_MODE_MASK 0x3c
> #define TPM_C0SC_MODE_SW_COMPARE 0x4
> +#define TPM_C0SC_CHF_MASK (0x1 << 7)
> #define TPM_C0V 0x24
>
> static void __iomem *timer_base;
> @@ -179,7 +181,7 @@ static int __init tpm_timer_init(struct device_node *np)
> ipg = of_clk_get_by_name(np, "ipg");
> per = of_clk_get_by_name(np, "per");
> if (IS_ERR(ipg) || IS_ERR(per)) {
> - pr_err("tpm: failed to get igp or per clk\n");
> + pr_err("tpm: failed to get ipg or per clk\n");
> ret = -ENODEV;
> goto err_clk_get;
> }
> @@ -205,9 +207,13 @@ static int __init tpm_timer_init(struct device_node *np)
> * 4) Channel0 disabled
> * 5) DMA transfers disabled
> */
> + /* make sure counter is disabled */
> writel(0, timer_base + TPM_SC);
> + /* TOF is W1C */
> + writel(TPM_SC_TOF_MASK, timer_base + TPM_SC);
> writel(0, timer_base + TPM_CNT);
> - writel(0, timer_base + TPM_C0SC);
> + /* CHF is W1C */
> + writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
>
> /* increase per cnt, div 8 by default */
> writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow
2018-03-27 15:06 ` [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Rob Herring
@ 2018-03-28 1:46 ` Anson Huang
0 siblings, 0 replies; 6+ messages in thread
From: Anson Huang @ 2018-03-28 1:46 UTC (permalink / raw)
To: Rob Herring
Cc: daniel.lezcano, tglx, mark.rutland, A.s. Dong, dl-linux-imx,
linux-kernel, devicetree
Anson Huang
Best Regards!
> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Tuesday, March 27, 2018 11:07 PM
> To: Anson Huang <anson.huang@nxp.com>
> Cc: daniel.lezcano@linaro.org; tglx@linutronix.de; mark.rutland@arm.com; A.s.
> Dong <aisheng.dong@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: Re: [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers
> operation flow
>
> On Mon, Mar 26, 2018 at 03:47:40PM +0800, Anson Huang wrote:
> > According to i.MX7ULP reference manual, TPM_SC_CPWMS can ONLY be
> > written when counter is disabled, TPM_SC_TOF is write-1-clear,
> > TPM_C0SC_CHF is also write-1-clear, correct these registers
> > initialization flow;
> >
> > Replace incorret clock name igp with ipg.
>
> incorrect
>
> This looks like an unrelated change.
>
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> > Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt | 2 +-
>
> Though you have the same typo, this should be a separate change from the
> driver.
Thanks, I will do separate patches to fix the typo.
Anson.
>
> > drivers/clocksource/timer-imx-tpm.c | 10
> ++++++++--
> > 2 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> > b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> > index b4aa7dd..f82087b 100644
> > --- a/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> > +++ b/Documentation/devicetree/bindings/timer/nxp,tpm-timer.txt
> > @@ -15,7 +15,7 @@ Required properties:
> > - interrupts : Should be the clock event device interrupt.
> > - clocks : The clocks provided by the SoC to drive the timer, must contain
> > an entry for each entry in clock-names.
> > -- clock-names : Must include the following entries: "igp" and "per".
> > +- clock-names : Must include the following entries: "ipg" and "per".
> >
> > Example:
> > tpm5: tpm@40260000 {
> > diff --git a/drivers/clocksource/timer-imx-tpm.c
> > b/drivers/clocksource/timer-imx-tpm.c
> > index 21bffdc..7403e49 100644
> > --- a/drivers/clocksource/timer-imx-tpm.c
> > +++ b/drivers/clocksource/timer-imx-tpm.c
> > @@ -20,6 +20,7 @@
> > #define TPM_SC 0x10
> > #define TPM_SC_CMOD_INC_PER_CNT (0x1 << 3)
> > #define TPM_SC_CMOD_DIV_DEFAULT 0x3
> > +#define TPM_SC_TOF_MASK (0x1 << 7)
> > #define TPM_CNT 0x14
> > #define TPM_MOD 0x18
> > #define TPM_STATUS 0x1c
> > @@ -29,6 +30,7 @@
> > #define TPM_C0SC_MODE_SHIFT 2
> > #define TPM_C0SC_MODE_MASK 0x3c
> > #define TPM_C0SC_MODE_SW_COMPARE 0x4
> > +#define TPM_C0SC_CHF_MASK (0x1 << 7)
> > #define TPM_C0V 0x24
> >
> > static void __iomem *timer_base;
> > @@ -179,7 +181,7 @@ static int __init tpm_timer_init(struct device_node
> *np)
> > ipg = of_clk_get_by_name(np, "ipg");
> > per = of_clk_get_by_name(np, "per");
> > if (IS_ERR(ipg) || IS_ERR(per)) {
> > - pr_err("tpm: failed to get igp or per clk\n");
> > + pr_err("tpm: failed to get ipg or per clk\n");
> > ret = -ENODEV;
> > goto err_clk_get;
> > }
> > @@ -205,9 +207,13 @@ static int __init tpm_timer_init(struct device_node
> *np)
> > * 4) Channel0 disabled
> > * 5) DMA transfers disabled
> > */
> > + /* make sure counter is disabled */
> > writel(0, timer_base + TPM_SC);
> > + /* TOF is W1C */
> > + writel(TPM_SC_TOF_MASK, timer_base + TPM_SC);
> > writel(0, timer_base + TPM_CNT);
> > - writel(0, timer_base + TPM_C0SC);
> > + /* CHF is W1C */
> > + writel(TPM_C0SC_CHF_MASK, timer_base + TPM_C0SC);
> >
> > /* increase per cnt, div 8 by default */
> > writel(TPM_SC_CMOD_INC_PER_CNT | TPM_SC_CMOD_DIV_DEFAULT,
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe devicetree"
> > in the body of a message to majordomo@vger.kernel.org More majordomo
> > info at
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fvger
> > .kernel.org%2Fmajordomo-info.html&data=02%7C01%7CAnson.Huang%40nx
> p.com
> > %7C9de93358525a4e55fd7e08d593f45e3b%7C686ea1d3bc2b4c6fa92cd99c5
> c301635
> > %7C0%7C0%7C636577600126807143&sdata=D1NrhkZ2nret5XXTDoszUslt3W
> FjS0pMGh
> > NA1Ws2mcM%3D&reserved=0
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support
2018-03-26 14:10 ` Daniel Lezcano
@ 2018-03-28 2:24 ` Anson Huang
0 siblings, 0 replies; 6+ messages in thread
From: Anson Huang @ 2018-03-28 2:24 UTC (permalink / raw)
To: Daniel Lezcano, tglx, robh+dt, mark.rutland, A.s. Dong
Cc: dl-linux-imx, linux-kernel, devicetree
Anson Huang
Best Regards!
> -----Original Message-----
> From: Daniel Lezcano [mailto:daniel.lezcano@linaro.org]
> Sent: Monday, March 26, 2018 10:11 PM
> To: Anson Huang <anson.huang@nxp.com>; tglx@linutronix.de;
> robh+dt@kernel.org; mark.rutland@arm.com; A.s. Dong
> <aisheng.dong@nxp.com>
> Cc: dl-linux-imx <linux-imx@nxp.com>; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter
> width support
>
> On 26/03/2018 09:47, Anson Huang wrote:
> > Different TPM modules have different width counters which is 16-bit or
> > 32-bit, the counter width can be read from TPM_PARAM register
> > bit[23:16], this patch adds dynamic check for counter width to support
> > both 16-bit and 32-bit TPM modules.
> >
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
>
> Are you sure you shouldn't use a prescalar with 16b and reduce its rating? What
> will be the duration of the clocksource before wrapping up?
The duration currently is 0xffff / 8MHz, ~8.2mS, yes, it is too short, I will make the prescalar
higher to increase the duration.
>
> Also, can you do the change to prevent computing the mask at each
> 'read_count' call ?
I think we can just return the value read from the CNT register, even the width
is 16, the upper 16 bits always 0, so it does NOT matter, I will remove the mask computation
in V2 patch.
Thanks.
Anson.
>
> > ---
> > drivers/clocksource/timer-imx-tpm.c | 19 +++++++++++++------
> > 1 file changed, 13 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/clocksource/timer-imx-tpm.c
> > b/drivers/clocksource/timer-imx-tpm.c
> > index 7403e49..5063b77 100644
> > --- a/drivers/clocksource/timer-imx-tpm.c
> > +++ b/drivers/clocksource/timer-imx-tpm.c
> > @@ -17,6 +17,9 @@
> > #include <linux/of_irq.h>
> > #include <linux/sched_clock.h>
> >
> > +#define TPM_PARAM 0x4
> > +#define TPM_PARAM_WIDTH_SHIFT 16
> > +#define TPM_PARAM_WIDTH_MASK (0xff << 16)
> > #define TPM_SC 0x10
> > #define TPM_SC_CMOD_INC_PER_CNT (0x1 << 3)
> > #define TPM_SC_CMOD_DIV_DEFAULT 0x3
> > @@ -33,6 +36,7 @@
> > #define TPM_C0SC_CHF_MASK (0x1 << 7)
> > #define TPM_C0V 0x24
> >
> > +static int counter_width;
> > static void __iomem *timer_base;
> > static struct clock_event_device clockevent_tpm;
> >
> > @@ -66,7 +70,7 @@ static struct delay_timer tpm_delay_timer;
> >
> > static inline unsigned long tpm_read_counter(void) {
> > - return readl(timer_base + TPM_CNT);
> > + return readl(timer_base + TPM_CNT) & GENMASK(counter_width - 1, 0);
> > }
> >
> > static unsigned long tpm_read_current_timer(void) @@ -85,10 +89,11 @@
> > static int __init tpm_clocksource_init(unsigned long rate)
> > tpm_delay_timer.freq = rate;
> > register_current_timer_delay(&tpm_delay_timer);
> >
> > - sched_clock_register(tpm_read_sched_clock, 32, rate);
> > + sched_clock_register(tpm_read_sched_clock, counter_width, rate);
> >
> > return clocksource_mmio_init(timer_base + TPM_CNT, "imx-tpm",
> > - rate, 200, 32, clocksource_mmio_readl_up);
> > + rate, 200, counter_width,
> > + clocksource_mmio_readl_up);
> > }
> >
> > static int tpm_set_next_event(unsigned long delta, @@ -153,8 +158,8
> > @@ static int __init tpm_clockevent_init(unsigned long rate, int irq)
> >
> > clockevent_tpm.cpumask = cpumask_of(0);
> > clockevent_tpm.irq = irq;
> > - clockevents_config_and_register(&clockevent_tpm,
> > - rate, 300, 0xfffffffe);
> > + clockevents_config_and_register(&clockevent_tpm, rate, 300,
> > + GENMASK(counter_width - 1, 1));
> >
> > return ret;
> > }
> > @@ -199,6 +204,8 @@ static int __init tpm_timer_init(struct device_node
> *np)
> > goto err_per_clk_enable;
> > }
> >
> > + counter_width = (readl(timer_base + TPM_PARAM) &
> TPM_PARAM_WIDTH_MASK)
> > + >> TPM_PARAM_WIDTH_SHIFT;
> > /*
> > * Initialize tpm module to a known state
> > * 1) Counter disabled
> > @@ -220,7 +227,7 @@ static int __init tpm_timer_init(struct device_node
> *np)
> > timer_base + TPM_SC);
> >
> > /* set MOD register to maximum for free running mode */
> > - writel(0xffffffff, timer_base + TPM_MOD);
> > + writel(GENMASK(counter_width - 1, 0), timer_base + TPM_MOD);
> >
> > rate = clk_get_rate(per) >> 3;
> > ret = tpm_clocksource_init(rate);
> >
>
>
> --
>
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.li
> naro.org%2F&data=02%7C01%7CAnson.Huang%40nxp.com%7C34d41253162e
> 4956105a08d593236499%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636576702585890585&sdata=td%2FN%2BpDuc%2BeFn5SZjAS6u6v6NgCQ
> AcZkNr9KS%2B8XiK0%3D&reserved=0> Linaro.org │ Open source software for
> ARM SoCs
>
> Follow Linaro:
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.f
> acebook.com%2Fpages%2FLinaro&data=02%7C01%7CAnson.Huang%40nxp.co
> m%7C34d41253162e4956105a08d593236499%7C686ea1d3bc2b4c6fa92cd99c
> 5c301635%7C0%7C0%7C636576702585890585&sdata=CePZL2guHTui9SbxWpY
> S2V%2FRqTZsIUEgPVLz6m8794E%3D&reserved=0> Facebook |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Ftwitter
> .com%2F%23!%2Flinaroorg&data=02%7C01%7CAnson.Huang%40nxp.com%7C
> 34d41253162e4956105a08d593236499%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C636576702585890585&sdata=f37TVduDxk3ihZ8NQA2O1Mq
> LBB5wdLLhbehnQLkQKFA%3D&reserved=0> Twitter |
> <https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.li
> naro.org%2Flinaro-blog%2F&data=02%7C01%7CAnson.Huang%40nxp.com%7C
> 34d41253162e4956105a08d593236499%7C686ea1d3bc2b4c6fa92cd99c5c301
> 635%7C0%7C0%7C636576702585890585&sdata=%2BAOgLPpZHN%2FjX66pyo8
> TW5RhybKrPYaijWs4z8OYP48%3D&reserved=0> Blog
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-28 2:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-26 7:47 [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Anson Huang
2018-03-26 7:47 ` [PATCH 2/2] clocksource/drivers/imx-tpm: add different counter width support Anson Huang
2018-03-26 14:10 ` Daniel Lezcano
2018-03-28 2:24 ` Anson Huang
2018-03-27 15:06 ` [PATCH 1/2] clocksource/drivers/imx-tpm: correct some registers operation flow Rob Herring
2018-03-28 1:46 ` 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).