All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-17  9:21 ` Keqian Zhu
  0 siblings, 0 replies; 12+ messages in thread
From: Keqian Zhu @ 2020-07-17  9:21 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, kvmarm, kvm
  Cc: Marc Zyngier, Catalin Marinas, Will Deacon, James Morse,
	Suzuki K Poulose, wanghaibin.wang, Keqian Zhu

ARM virtual counter supports event stream. It can only trigger an event
when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
example, when the trigger bit is 0, then it triggers an event for every
two cycles.

Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
 drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ecf7b7db2d05..06d99a4b1b9b 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
 static void arch_timer_evtstrm_enable(int divider)
 {
 	u32 cntkctl = arch_timer_get_cntkctl();
+	int cntkctl_evnti;
+
+	/*
+	 * Note that it can only trigger an event when the trigger bit
+	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
+	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
+	 */
+	cntkctl_evnti = divider - 1;
+	cntkctl_evnti = min(cntkctl_evnti, 15);
+	cntkctl_evnti = max(cntkctl_evnti, 0);
 
 	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
 	/* Set the divider and enable virtual event stream */
-	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
 			| ARCH_TIMER_VIRT_EVT_EN;
 	arch_timer_set_cntkctl(cntkctl);
 	arch_timer_set_evtstrm_feature();
@@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
 	/* Find the closest power of two to the divisor */
 	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
 	pos = fls(evt_stream_div);
-	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
 		pos--;
+
 	/* enable event stream */
-	arch_timer_evtstrm_enable(min(pos, 15));
+	arch_timer_evtstrm_enable(pos);
 }
 
 static void arch_counter_set_user_access(void)
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-17  9:21 ` Keqian Zhu
  0 siblings, 0 replies; 12+ messages in thread
From: Keqian Zhu @ 2020-07-17  9:21 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, kvmarm, kvm
  Cc: Marc Zyngier, Catalin Marinas, Will Deacon

ARM virtual counter supports event stream. It can only trigger an event
when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
example, when the trigger bit is 0, then it triggers an event for every
two cycles.

Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
 drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ecf7b7db2d05..06d99a4b1b9b 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
 static void arch_timer_evtstrm_enable(int divider)
 {
 	u32 cntkctl = arch_timer_get_cntkctl();
+	int cntkctl_evnti;
+
+	/*
+	 * Note that it can only trigger an event when the trigger bit
+	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
+	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
+	 */
+	cntkctl_evnti = divider - 1;
+	cntkctl_evnti = min(cntkctl_evnti, 15);
+	cntkctl_evnti = max(cntkctl_evnti, 0);
 
 	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
 	/* Set the divider and enable virtual event stream */
-	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
 			| ARCH_TIMER_VIRT_EVT_EN;
 	arch_timer_set_cntkctl(cntkctl);
 	arch_timer_set_evtstrm_feature();
@@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
 	/* Find the closest power of two to the divisor */
 	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
 	pos = fls(evt_stream_div);
-	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
 		pos--;
+
 	/* enable event stream */
-	arch_timer_evtstrm_enable(min(pos, 15));
+	arch_timer_evtstrm_enable(pos);
 }
 
 static void arch_counter_set_user_access(void)
-- 
2.19.1

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-17  9:21 ` Keqian Zhu
  0 siblings, 0 replies; 12+ messages in thread
From: Keqian Zhu @ 2020-07-17  9:21 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, kvmarm, kvm
  Cc: Suzuki K Poulose, Marc Zyngier, Keqian Zhu, James Morse,
	Catalin Marinas, wanghaibin.wang, Will Deacon

ARM virtual counter supports event stream. It can only trigger an event
when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
example, when the trigger bit is 0, then it triggers an event for every
two cycles.

Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
---
 drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index ecf7b7db2d05..06d99a4b1b9b 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
 static void arch_timer_evtstrm_enable(int divider)
 {
 	u32 cntkctl = arch_timer_get_cntkctl();
+	int cntkctl_evnti;
+
+	/*
+	 * Note that it can only trigger an event when the trigger bit
+	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
+	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
+	 */
+	cntkctl_evnti = divider - 1;
+	cntkctl_evnti = min(cntkctl_evnti, 15);
+	cntkctl_evnti = max(cntkctl_evnti, 0);
 
 	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
 	/* Set the divider and enable virtual event stream */
-	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
+	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
 			| ARCH_TIMER_VIRT_EVT_EN;
 	arch_timer_set_cntkctl(cntkctl);
 	arch_timer_set_evtstrm_feature();
@@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
 	/* Find the closest power of two to the divisor */
 	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
 	pos = fls(evt_stream_div);
-	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
+	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
 		pos--;
+
 	/* enable event stream */
-	arch_timer_evtstrm_enable(min(pos, 15));
+	arch_timer_evtstrm_enable(pos);
 }
 
 static void arch_counter_set_user_access(void)
-- 
2.19.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
  2020-07-17  9:21 ` Keqian Zhu
  (?)
@ 2020-07-28  7:39   ` zhukeqian
  -1 siblings, 0 replies; 12+ messages in thread
From: zhukeqian @ 2020-07-28  7:39 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, kvmarm, kvm
  Cc: Marc Zyngier, Catalin Marinas, Will Deacon, James Morse,
	Suzuki K Poulose, wanghaibin.wang

Friendly ping.
Is this an effective bugfix?

On 2020/7/17 17:21, Keqian Zhu wrote:
> ARM virtual counter supports event stream. It can only trigger an event
> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
> example, when the trigger bit is 0, then it triggers an event for every
> two cycles.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index ecf7b7db2d05..06d99a4b1b9b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>  static void arch_timer_evtstrm_enable(int divider)
>  {
>  	u32 cntkctl = arch_timer_get_cntkctl();
> +	int cntkctl_evnti;
> +
> +	/*
> +	 * Note that it can only trigger an event when the trigger bit
> +	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
> +	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
> +	 */
> +	cntkctl_evnti = divider - 1;
> +	cntkctl_evnti = min(cntkctl_evnti, 15);
> +	cntkctl_evnti = max(cntkctl_evnti, 0);
>  
>  	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>  	/* Set the divider and enable virtual event stream */
> -	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>  			| ARCH_TIMER_VIRT_EVT_EN;
>  	arch_timer_set_cntkctl(cntkctl);
>  	arch_timer_set_evtstrm_feature();
> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>  	/* Find the closest power of two to the divisor */
>  	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>  	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>  		pos--;
> +
>  	/* enable event stream */
> -	arch_timer_evtstrm_enable(min(pos, 15));
> +	arch_timer_evtstrm_enable(pos);
>  }
>  
>  static void arch_counter_set_user_access(void)
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-28  7:39   ` zhukeqian
  0 siblings, 0 replies; 12+ messages in thread
From: zhukeqian @ 2020-07-28  7:39 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, kvmarm, kvm
  Cc: Marc Zyngier, Catalin Marinas, Will Deacon

Friendly ping.
Is this an effective bugfix?

On 2020/7/17 17:21, Keqian Zhu wrote:
> ARM virtual counter supports event stream. It can only trigger an event
> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
> example, when the trigger bit is 0, then it triggers an event for every
> two cycles.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index ecf7b7db2d05..06d99a4b1b9b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>  static void arch_timer_evtstrm_enable(int divider)
>  {
>  	u32 cntkctl = arch_timer_get_cntkctl();
> +	int cntkctl_evnti;
> +
> +	/*
> +	 * Note that it can only trigger an event when the trigger bit
> +	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
> +	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
> +	 */
> +	cntkctl_evnti = divider - 1;
> +	cntkctl_evnti = min(cntkctl_evnti, 15);
> +	cntkctl_evnti = max(cntkctl_evnti, 0);
>  
>  	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>  	/* Set the divider and enable virtual event stream */
> -	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>  			| ARCH_TIMER_VIRT_EVT_EN;
>  	arch_timer_set_cntkctl(cntkctl);
>  	arch_timer_set_evtstrm_feature();
> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>  	/* Find the closest power of two to the divisor */
>  	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>  	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>  		pos--;
> +
>  	/* enable event stream */
> -	arch_timer_evtstrm_enable(min(pos, 15));
> +	arch_timer_evtstrm_enable(pos);
>  }
>  
>  static void arch_counter_set_user_access(void)
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-28  7:39   ` zhukeqian
  0 siblings, 0 replies; 12+ messages in thread
From: zhukeqian @ 2020-07-28  7:39 UTC (permalink / raw)
  To: linux-kernel, linux-arm-kernel, kvmarm, kvm
  Cc: Suzuki K Poulose, Marc Zyngier, James Morse, Catalin Marinas,
	wanghaibin.wang, Will Deacon

Friendly ping.
Is this an effective bugfix?

On 2020/7/17 17:21, Keqian Zhu wrote:
> ARM virtual counter supports event stream. It can only trigger an event
> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
> example, when the trigger bit is 0, then it triggers an event for every
> two cycles.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index ecf7b7db2d05..06d99a4b1b9b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>  static void arch_timer_evtstrm_enable(int divider)
>  {
>  	u32 cntkctl = arch_timer_get_cntkctl();
> +	int cntkctl_evnti;
> +
> +	/*
> +	 * Note that it can only trigger an event when the trigger bit
> +	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
> +	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
> +	 */
> +	cntkctl_evnti = divider - 1;
> +	cntkctl_evnti = min(cntkctl_evnti, 15);
> +	cntkctl_evnti = max(cntkctl_evnti, 0);
>  
>  	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>  	/* Set the divider and enable virtual event stream */
> -	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>  			| ARCH_TIMER_VIRT_EVT_EN;
>  	arch_timer_set_cntkctl(cntkctl);
>  	arch_timer_set_evtstrm_feature();
> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>  	/* Find the closest power of two to the divisor */
>  	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>  	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>  		pos--;
> +
>  	/* enable event stream */
> -	arch_timer_evtstrm_enable(min(pos, 15));
> +	arch_timer_evtstrm_enable(pos);
>  }
>  
>  static void arch_counter_set_user_access(void)
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
  2020-07-17  9:21 ` Keqian Zhu
  (?)
@ 2020-07-28 10:16   ` Marc Zyngier
  -1 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2020-07-28 10:16 UTC (permalink / raw)
  To: Keqian Zhu
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Catalin Marinas,
	Will Deacon, James Morse, Suzuki K Poulose, wanghaibin.wang

On 2020-07-17 10:21, Keqian Zhu wrote:
> ARM virtual counter supports event stream. It can only trigger an event
> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 
> 0),
> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
> example, when the trigger bit is 0, then it triggers an event for every
> two cycles.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index ecf7b7db2d05..06d99a4b1b9b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>  static void arch_timer_evtstrm_enable(int divider)
>  {
>  	u32 cntkctl = arch_timer_get_cntkctl();
> +	int cntkctl_evnti;
> +
> +	/*
> +	 * Note that it can only trigger an event when the trigger bit
> +	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
> +	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
> +	 */
> +	cntkctl_evnti = divider - 1;
> +	cntkctl_evnti = min(cntkctl_evnti, 15);
> +	cntkctl_evnti = max(cntkctl_evnti, 0);
> 
>  	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>  	/* Set the divider and enable virtual event stream */
> -	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>  			| ARCH_TIMER_VIRT_EVT_EN;
>  	arch_timer_set_cntkctl(cntkctl);
>  	arch_timer_set_evtstrm_feature();
> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>  	/* Find the closest power of two to the divisor */
>  	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>  	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>  		pos--;
> +
>  	/* enable event stream */
> -	arch_timer_evtstrm_enable(min(pos, 15));
> +	arch_timer_evtstrm_enable(pos);
>  }
> 
>  static void arch_counter_set_user_access(void)

This looks like a very convoluted fix. If the problem you are
trying to fix is that the event frequency is at most half of
that of the counter, why isn't the patch below the most
obvious fix?

Thanks,

         M.

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index 6c3e84180146..0a65414b781f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -824,8 +824,12 @@ static void arch_timer_configure_evtstream(void)
  {
  	int evt_stream_div, pos;

-	/* Find the closest power of two to the divisor */
-	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	/*
+	 * Find the closest power of two to the divisor. As the event
+	 * stream can at most be generated at half the frequency of the
+	 * counter, use half the frequency when computing the divider.
+	 */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
  	pos = fls(evt_stream_div);
  	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
  		pos--;

-- 
Jazz is not dead. It just smells funny...

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-28 10:16   ` Marc Zyngier
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2020-07-28 10:16 UTC (permalink / raw)
  To: Keqian Zhu
  Cc: kvm, Catalin Marinas, linux-kernel, Will Deacon, kvmarm,
	linux-arm-kernel

On 2020-07-17 10:21, Keqian Zhu wrote:
> ARM virtual counter supports event stream. It can only trigger an event
> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 
> 0),
> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
> example, when the trigger bit is 0, then it triggers an event for every
> two cycles.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index ecf7b7db2d05..06d99a4b1b9b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>  static void arch_timer_evtstrm_enable(int divider)
>  {
>  	u32 cntkctl = arch_timer_get_cntkctl();
> +	int cntkctl_evnti;
> +
> +	/*
> +	 * Note that it can only trigger an event when the trigger bit
> +	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
> +	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
> +	 */
> +	cntkctl_evnti = divider - 1;
> +	cntkctl_evnti = min(cntkctl_evnti, 15);
> +	cntkctl_evnti = max(cntkctl_evnti, 0);
> 
>  	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>  	/* Set the divider and enable virtual event stream */
> -	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>  			| ARCH_TIMER_VIRT_EVT_EN;
>  	arch_timer_set_cntkctl(cntkctl);
>  	arch_timer_set_evtstrm_feature();
> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>  	/* Find the closest power of two to the divisor */
>  	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>  	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>  		pos--;
> +
>  	/* enable event stream */
> -	arch_timer_evtstrm_enable(min(pos, 15));
> +	arch_timer_evtstrm_enable(pos);
>  }
> 
>  static void arch_counter_set_user_access(void)

This looks like a very convoluted fix. If the problem you are
trying to fix is that the event frequency is at most half of
that of the counter, why isn't the patch below the most
obvious fix?

Thanks,

         M.

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index 6c3e84180146..0a65414b781f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -824,8 +824,12 @@ static void arch_timer_configure_evtstream(void)
  {
  	int evt_stream_div, pos;

-	/* Find the closest power of two to the divisor */
-	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	/*
+	 * Find the closest power of two to the divisor. As the event
+	 * stream can at most be generated at half the frequency of the
+	 * counter, use half the frequency when computing the divider.
+	 */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
  	pos = fls(evt_stream_div);
  	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
  		pos--;

-- 
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-28 10:16   ` Marc Zyngier
  0 siblings, 0 replies; 12+ messages in thread
From: Marc Zyngier @ 2020-07-28 10:16 UTC (permalink / raw)
  To: Keqian Zhu
  Cc: kvm, Suzuki K Poulose, Catalin Marinas, linux-kernel,
	James Morse, wanghaibin.wang, Will Deacon, kvmarm,
	linux-arm-kernel

On 2020-07-17 10:21, Keqian Zhu wrote:
> ARM virtual counter supports event stream. It can only trigger an event
> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 
> 0),
> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
> example, when the trigger bit is 0, then it triggers an event for every
> two cycles.
> 
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index ecf7b7db2d05..06d99a4b1b9b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>  static void arch_timer_evtstrm_enable(int divider)
>  {
>  	u32 cntkctl = arch_timer_get_cntkctl();
> +	int cntkctl_evnti;
> +
> +	/*
> +	 * Note that it can only trigger an event when the trigger bit
> +	 * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
> +	 * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
> +	 */
> +	cntkctl_evnti = divider - 1;
> +	cntkctl_evnti = min(cntkctl_evnti, 15);
> +	cntkctl_evnti = max(cntkctl_evnti, 0);
> 
>  	cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>  	/* Set the divider and enable virtual event stream */
> -	cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
> +	cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>  			| ARCH_TIMER_VIRT_EVT_EN;
>  	arch_timer_set_cntkctl(cntkctl);
>  	arch_timer_set_evtstrm_feature();
> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>  	/* Find the closest power of two to the divisor */
>  	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>  	pos = fls(evt_stream_div);
> -	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
> +	if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>  		pos--;
> +
>  	/* enable event stream */
> -	arch_timer_evtstrm_enable(min(pos, 15));
> +	arch_timer_evtstrm_enable(pos);
>  }
> 
>  static void arch_counter_set_user_access(void)

This looks like a very convoluted fix. If the problem you are
trying to fix is that the event frequency is at most half of
that of the counter, why isn't the patch below the most
obvious fix?

Thanks,

         M.

diff --git a/drivers/clocksource/arm_arch_timer.c 
b/drivers/clocksource/arm_arch_timer.c
index 6c3e84180146..0a65414b781f 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -824,8 +824,12 @@ static void arch_timer_configure_evtstream(void)
  {
  	int evt_stream_div, pos;

-	/* Find the closest power of two to the divisor */
-	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
+	/*
+	 * Find the closest power of two to the divisor. As the event
+	 * stream can at most be generated at half the frequency of the
+	 * counter, use half the frequency when computing the divider.
+	 */
+	evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
  	pos = fls(evt_stream_div);
  	if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
  		pos--;

-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
  2020-07-28 10:16   ` Marc Zyngier
  (?)
@ 2020-07-28 11:59     ` zhukeqian
  -1 siblings, 0 replies; 12+ messages in thread
From: zhukeqian @ 2020-07-28 11:59 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-kernel, linux-arm-kernel, kvmarm, kvm, Catalin Marinas,
	Will Deacon, James Morse, Suzuki K Poulose, wanghaibin.wang

Hi Marc,

On 2020/7/28 18:16, Marc Zyngier wrote:
> On 2020-07-17 10:21, Keqian Zhu wrote:
>> ARM virtual counter supports event stream. It can only trigger an event
>> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
>> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
>> example, when the trigger bit is 0, then it triggers an event for every
>> two cycles.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c
>> b/drivers/clocksource/arm_arch_timer.c
>> index ecf7b7db2d05..06d99a4b1b9b 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>>  static void arch_timer_evtstrm_enable(int divider)
>>  {
>>      u32 cntkctl = arch_timer_get_cntkctl();
>> +    int cntkctl_evnti;
>> +
>> +    /*
>> +     * Note that it can only trigger an event when the trigger bit
>> +     * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
>> +     * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
>> +     */
>> +    cntkctl_evnti = divider - 1;
>> +    cntkctl_evnti = min(cntkctl_evnti, 15);
>> +    cntkctl_evnti = max(cntkctl_evnti, 0);
>>
>>      cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>>      /* Set the divider and enable virtual event stream */
>> -    cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>> +    cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>>              | ARCH_TIMER_VIRT_EVT_EN;
>>      arch_timer_set_cntkctl(cntkctl);
>>      arch_timer_set_evtstrm_feature();
>> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>>      /* Find the closest power of two to the divisor */
>>      evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>>      pos = fls(evt_stream_div);
>> -    if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
>> +    if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>>          pos--;
>> +
>>      /* enable event stream */
>> -    arch_timer_evtstrm_enable(min(pos, 15));
>> +    arch_timer_evtstrm_enable(pos);
>>  }
>>
>>  static void arch_counter_set_user_access(void)
> 
> This looks like a very convoluted fix. If the problem you are
> trying to fix is that the event frequency is at most half of
> that of the counter, why isn't the patch below the most
> obvious fix?
> 
> Thanks,
> 
>         M.
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6c3e84180146..0a65414b781f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -824,8 +824,12 @@ static void arch_timer_configure_evtstream(void)
>  {
>      int evt_stream_div, pos;
> 
> -    /* Find the closest power of two to the divisor */
> -    evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> +    /*
> +     * Find the closest power of two to the divisor. As the event
> +     * stream can at most be generated at half the frequency of the
> +     * counter, use half the frequency when computing the divider.
> +     */
> +    evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
>      pos = fls(evt_stream_div);
>      if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
I think here does not consider the case of pos==1 (though it will not occur...)
>          pos--;
> 

It looks good to me.

Thanks,
Keqian

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-28 11:59     ` zhukeqian
  0 siblings, 0 replies; 12+ messages in thread
From: zhukeqian @ 2020-07-28 11:59 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvm, Catalin Marinas, linux-kernel, Will Deacon, kvmarm,
	linux-arm-kernel

Hi Marc,

On 2020/7/28 18:16, Marc Zyngier wrote:
> On 2020-07-17 10:21, Keqian Zhu wrote:
>> ARM virtual counter supports event stream. It can only trigger an event
>> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
>> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
>> example, when the trigger bit is 0, then it triggers an event for every
>> two cycles.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c
>> b/drivers/clocksource/arm_arch_timer.c
>> index ecf7b7db2d05..06d99a4b1b9b 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>>  static void arch_timer_evtstrm_enable(int divider)
>>  {
>>      u32 cntkctl = arch_timer_get_cntkctl();
>> +    int cntkctl_evnti;
>> +
>> +    /*
>> +     * Note that it can only trigger an event when the trigger bit
>> +     * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
>> +     * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
>> +     */
>> +    cntkctl_evnti = divider - 1;
>> +    cntkctl_evnti = min(cntkctl_evnti, 15);
>> +    cntkctl_evnti = max(cntkctl_evnti, 0);
>>
>>      cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>>      /* Set the divider and enable virtual event stream */
>> -    cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>> +    cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>>              | ARCH_TIMER_VIRT_EVT_EN;
>>      arch_timer_set_cntkctl(cntkctl);
>>      arch_timer_set_evtstrm_feature();
>> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>>      /* Find the closest power of two to the divisor */
>>      evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>>      pos = fls(evt_stream_div);
>> -    if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
>> +    if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>>          pos--;
>> +
>>      /* enable event stream */
>> -    arch_timer_evtstrm_enable(min(pos, 15));
>> +    arch_timer_evtstrm_enable(pos);
>>  }
>>
>>  static void arch_counter_set_user_access(void)
> 
> This looks like a very convoluted fix. If the problem you are
> trying to fix is that the event frequency is at most half of
> that of the counter, why isn't the patch below the most
> obvious fix?
> 
> Thanks,
> 
>         M.
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6c3e84180146..0a65414b781f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -824,8 +824,12 @@ static void arch_timer_configure_evtstream(void)
>  {
>      int evt_stream_div, pos;
> 
> -    /* Find the closest power of two to the divisor */
> -    evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> +    /*
> +     * Find the closest power of two to the divisor. As the event
> +     * stream can at most be generated at half the frequency of the
> +     * counter, use half the frequency when computing the divider.
> +     */
> +    evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
>      pos = fls(evt_stream_div);
>      if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
I think here does not consider the case of pos==1 (though it will not occur...)
>          pos--;
> 

It looks good to me.

Thanks,
Keqian
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI
@ 2020-07-28 11:59     ` zhukeqian
  0 siblings, 0 replies; 12+ messages in thread
From: zhukeqian @ 2020-07-28 11:59 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: kvm, Suzuki K Poulose, Catalin Marinas, linux-kernel,
	James Morse, wanghaibin.wang, Will Deacon, kvmarm,
	linux-arm-kernel

Hi Marc,

On 2020/7/28 18:16, Marc Zyngier wrote:
> On 2020-07-17 10:21, Keqian Zhu wrote:
>> ARM virtual counter supports event stream. It can only trigger an event
>> when the trigger bit of CNTVCT_EL0 changes from 0 to 1 (or from 1 to 0),
>> so the actual period of event stream is 2 ^ (cntkctl_evnti + 1). For
>> example, when the trigger bit is 0, then it triggers an event for every
>> two cycles.
>>
>> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
>> ---
>>  drivers/clocksource/arm_arch_timer.c | 17 ++++++++++++++---
>>  1 file changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c
>> b/drivers/clocksource/arm_arch_timer.c
>> index ecf7b7db2d05..06d99a4b1b9b 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -799,10 +799,20 @@ static void __arch_timer_setup(unsigned type,
>>  static void arch_timer_evtstrm_enable(int divider)
>>  {
>>      u32 cntkctl = arch_timer_get_cntkctl();
>> +    int cntkctl_evnti;
>> +
>> +    /*
>> +     * Note that it can only trigger an event when the trigger bit
>> +     * of CNTVCT_EL0 changes from 1 to 0 (or from 0 to 1), so the
>> +     * actual period of event stream is 2 ^ (cntkctl_evnti + 1).
>> +     */
>> +    cntkctl_evnti = divider - 1;
>> +    cntkctl_evnti = min(cntkctl_evnti, 15);
>> +    cntkctl_evnti = max(cntkctl_evnti, 0);
>>
>>      cntkctl &= ~ARCH_TIMER_EVT_TRIGGER_MASK;
>>      /* Set the divider and enable virtual event stream */
>> -    cntkctl |= (divider << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>> +    cntkctl |= (cntkctl_evnti << ARCH_TIMER_EVT_TRIGGER_SHIFT)
>>              | ARCH_TIMER_VIRT_EVT_EN;
>>      arch_timer_set_cntkctl(cntkctl);
>>      arch_timer_set_evtstrm_feature();
>> @@ -816,10 +826,11 @@ static void arch_timer_configure_evtstream(void)
>>      /* Find the closest power of two to the divisor */
>>      evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
>>      pos = fls(evt_stream_div);
>> -    if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
>> +    if ((pos == 1) || (pos > 1 && !(evt_stream_div & (1 << (pos - 2)))))
>>          pos--;
>> +
>>      /* enable event stream */
>> -    arch_timer_evtstrm_enable(min(pos, 15));
>> +    arch_timer_evtstrm_enable(pos);
>>  }
>>
>>  static void arch_counter_set_user_access(void)
> 
> This looks like a very convoluted fix. If the problem you are
> trying to fix is that the event frequency is at most half of
> that of the counter, why isn't the patch below the most
> obvious fix?
> 
> Thanks,
> 
>         M.
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6c3e84180146..0a65414b781f 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -824,8 +824,12 @@ static void arch_timer_configure_evtstream(void)
>  {
>      int evt_stream_div, pos;
> 
> -    /* Find the closest power of two to the divisor */
> -    evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ;
> +    /*
> +     * Find the closest power of two to the divisor. As the event
> +     * stream can at most be generated at half the frequency of the
> +     * counter, use half the frequency when computing the divider.
> +     */
> +    evt_stream_div = arch_timer_rate / ARCH_TIMER_EVT_STREAM_FREQ / 2;
>      pos = fls(evt_stream_div);
>      if (pos > 1 && !(evt_stream_div & (1 << (pos - 2))))
I think here does not consider the case of pos==1 (though it will not occur...)
>          pos--;
> 

It looks good to me.

Thanks,
Keqian

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-07-28 12:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-17  9:21 [RESEND PATCH] drivers: arm arch timer: Correct fault programming of CNTKCTL_EL1.EVNTI Keqian Zhu
2020-07-17  9:21 ` Keqian Zhu
2020-07-17  9:21 ` Keqian Zhu
2020-07-28  7:39 ` zhukeqian
2020-07-28  7:39   ` zhukeqian
2020-07-28  7:39   ` zhukeqian
2020-07-28 10:16 ` Marc Zyngier
2020-07-28 10:16   ` Marc Zyngier
2020-07-28 10:16   ` Marc Zyngier
2020-07-28 11:59   ` zhukeqian
2020-07-28 11:59     ` zhukeqian
2020-07-28 11:59     ` zhukeqian

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.