linux-kernel.vger.kernel.org archive mirror
 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 related	[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 related	[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).