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 <[email protected]>
---
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
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
On Tue, 13 Sep 2022 11:47:23 +0100,
Kunkun Jiang <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>
Daniel, I assume you'll pick this?
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
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
> .
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 <[email protected]> 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 <[email protected]>
>> ---
>> 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 <[email protected]>
>
> Daniel, I assume you'll pick this?
>
> Thanks,
>
> M.
>