[PATCH] arm64: arch_timer: Fix the lack of set_next_event* for workaround of Cortex-A73 erratum 858921

[PATCH] arm64: arch_timer: Fix the lack of set_next_event* for workaround of Cortex-A73 erratum 858921

[Kunkun Jiang]

The patch a38b71b0833e moves the programming of the timers from
the countdown timer (TVAL) over to the comparator (CVAL). This
makes it necessary to read the counter when setting next event.
However, the workaround of Cortex-A73 erratum 858921 does not
set the corresponding set_next_event_phys and set_next_event_virt.
This patch fixes it.

Fixes: a38b71b0833e ("clocksource/drivers/arm_arch_timer: Move system register timer programming over to CVAL")
Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
---
 drivers/clocksource/arm_arch_timer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 9ab8221ee3c6..ff935efb6a88 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -473,6 +473,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
 		.desc = "ARM erratum 858921",
 		.read_cntpct_el0 = arm64_858921_read_cntpct_el0,
 		.read_cntvct_el0 = arm64_858921_read_cntvct_el0,
+		.set_next_event_phys = erratum_set_next_event_phys,
+		.set_next_event_virt = erratum_set_next_event_virt,
 	},
 #endif
 #ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1
-- 
2.27.0


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

Re: [PATCH] arm64: arch_timer: Fix the lack of set_next_event* for workaround of Cortex-A73 erratum 858921

[Oliver Upton]

On Tue, Sep 13, 2022 at 06:47:23PM +0800, Kunkun Jiang wrote:
> The patch a38b71b0833e moves the programming of the timers from

Please check your patch with scripts/checkpatch.pl next time.
Use this pattern when referring to a commit in the changelog:

commit a38b71b0833e ("clocksource/drivers/arm_arch_timer: Move
system register timer programming over to CVAL") moved the
programming...

> the countdown timer (TVAL) over to the comparator (CVAL). This
> makes it necessary to read the counter when setting next event.
> However, the workaround of Cortex-A73 erratum 858921 does not
> set the corresponding set_next_event_phys and set_next_event_virt.
> This patch fixes it.

Avoid referring to 'this patch', and just state imperatively what the
change does, perhaps:

Add the appropriate hooks to apply the erratum mitigation when
programming the next timer event.

Thanks for catching this!

--
Best,
Oliver

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

Re: [PATCH] arm64: arch_timer: Fix the lack of set_next_event* for workaround of Cortex-A73 erratum 858921

[Marc Zyngier]

On Tue, 13 Sep 2022 11:47:23 +0100,
Kunkun Jiang <jiangkunkun@huawei.com> wrote:
> 
> The patch a38b71b0833e moves the programming of the timers from
> the countdown timer (TVAL) over to the comparator (CVAL). This
> makes it necessary to read the counter when setting next event.
> However, the workaround of Cortex-A73 erratum 858921 does not
> set the corresponding set_next_event_phys and set_next_event_virt.
> This patch fixes it.
> 
> Fixes: a38b71b0833e ("clocksource/drivers/arm_arch_timer: Move system register timer programming over to CVAL")
> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
> ---
>  drivers/clocksource/arm_arch_timer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 9ab8221ee3c6..ff935efb6a88 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -473,6 +473,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
>  		.desc = "ARM erratum 858921",
>  		.read_cntpct_el0 = arm64_858921_read_cntpct_el0,
>  		.read_cntvct_el0 = arm64_858921_read_cntvct_el0,
> +		.set_next_event_phys = erratum_set_next_event_phys,
> +		.set_next_event_virt = erratum_set_next_event_virt,
>  	},
>  #endif
>  #ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1

With the fixed commit message as suggested by Oliver, and a subject
more conform to the style of previous commits (something like:
"clocksource/drivers/arm_arch_timer: Fix handling of ARM erratum 85821"):

Acked-by: Marc Zyngier <maz@kernel.org>

Daniel, I assume you'll pick this?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

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

Re: [PATCH] arm64: arch_timer: Fix the lack of set_next_event* for workaround of Cortex-A73 erratum 858921

[Kunkun Jiang]

Hi Oliver,

Thank you for your suggestion. I've modified it in v2.

Thanks,
Kunkun Jiang

On 2022/9/13 19:26, Oliver Upton wrote:
> On Tue, Sep 13, 2022 at 06:47:23PM +0800, Kunkun Jiang wrote:
>> The patch a38b71b0833e moves the programming of the timers from
> Please check your patch with scripts/checkpatch.pl next time.
> Use this pattern when referring to a commit in the changelog:
>
> commit a38b71b0833e ("clocksource/drivers/arm_arch_timer: Move
> system register timer programming over to CVAL") moved the
> programming...
>
>> the countdown timer (TVAL) over to the comparator (CVAL). This
>> makes it necessary to read the counter when setting next event.
>> However, the workaround of Cortex-A73 erratum 858921 does not
>> set the corresponding set_next_event_phys and set_next_event_virt.
>> This patch fixes it.
> Avoid referring to 'this patch', and just state imperatively what the
> change does, perhaps:
>
> Add the appropriate hooks to apply the erratum mitigation when
> programming the next timer event.
>
> Thanks for catching this!
>
> --
> Best,
> Oliver
> .

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

Re: [PATCH] arm64: arch_timer: Fix the lack of set_next_event* for workaround of Cortex-A73 erratum 858921

[Kunkun Jiang]

Hi Marc,

Thank you. I've modified it in v2.
I'll pay more attention in the future.

Thanks,
Kunkun Jiang

On 2022/9/13 22:12, Marc Zyngier wrote:
> On Tue, 13 Sep 2022 11:47:23 +0100,
> Kunkun Jiang <jiangkunkun@huawei.com> wrote:
>> The patch a38b71b0833e moves the programming of the timers from
>> the countdown timer (TVAL) over to the comparator (CVAL). This
>> makes it necessary to read the counter when setting next event.
>> However, the workaround of Cortex-A73 erratum 858921 does not
>> set the corresponding set_next_event_phys and set_next_event_virt.
>> This patch fixes it.
>>
>> Fixes: a38b71b0833e ("clocksource/drivers/arm_arch_timer: Move system register timer programming over to CVAL")
>> Signed-off-by: Kunkun Jiang <jiangkunkun@huawei.com>
>> ---
>>   drivers/clocksource/arm_arch_timer.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
>> index 9ab8221ee3c6..ff935efb6a88 100644
>> --- a/drivers/clocksource/arm_arch_timer.c
>> +++ b/drivers/clocksource/arm_arch_timer.c
>> @@ -473,6 +473,8 @@ static const struct arch_timer_erratum_workaround ool_workarounds[] = {
>>   		.desc = "ARM erratum 858921",
>>   		.read_cntpct_el0 = arm64_858921_read_cntpct_el0,
>>   		.read_cntvct_el0 = arm64_858921_read_cntvct_el0,
>> +		.set_next_event_phys = erratum_set_next_event_phys,
>> +		.set_next_event_virt = erratum_set_next_event_virt,
>>   	},
>>   #endif
>>   #ifdef CONFIG_SUN50I_ERRATUM_UNKNOWN1
> With the fixed commit message as suggested by Oliver, and a subject
> more conform to the style of previous commits (something like:
> "clocksource/drivers/arm_arch_timer: Fix handling of ARM erratum 85821"):
>
> Acked-by: Marc Zyngier <maz@kernel.org>
>
> Daniel, I assume you'll pick this?
>
> Thanks,
>
> 	M.
>

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