On 2017/2/21 3:00, Marc Zyngier wrote: > On 10/02/17 07:10, Hanjun Guo wrote: >> Hi Marc, >> >> On 2017/1/24 19:32, Marc Zyngier wrote: >>> On 24/01/17 10:57, Mark Rutland wrote: >>>> On Tue, Jan 24, 2017 at 06:39:51PM +0800, Hanjun Guo wrote: >>>>> From: Hanjun Guo <hanjun.guo@xxxxxxxxxx> >>>>> >>>>> Add hisilicon timer specific erratum fixes. >>>>> >>>>> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx> >>>>> --- >>>>> drivers/clocksource/arm_arch_timer.c | 22 ++++++++++++++++++++++ >>>>> 1 file changed, 22 insertions(+) >>>>> >>>>> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c >>>>> index 80d6f76..3e62a09 100644 >>>>> --- a/drivers/clocksource/arm_arch_timer.c >>>>> +++ b/drivers/clocksource/arm_arch_timer.c >>>>> @@ -1156,10 +1156,32 @@ struct gtdt_arch_timer_fixup { >>>>> void *context; >>>>> }; >>>>> >>>>> +#ifdef CONFIG_HISILICON_ERRATUM_161010101 >>>>> +static void __init hisi_erratum_workaroud_enable(void *context) >>>>> +{ >>>>> + int i; >>>>> + >>>>> + for (i = 0; i < ARRAY_SIZE(ool_workarounds); i++) { >>>>> + if (!strcmp(context, ool_workarounds[i].id)) { >>>>> + timer_unstable_counter_workaround = &ool_workarounds[i]; >>>>> + static_branch_enable(&arch_timer_read_ool_enabled); >>>>> + pr_info("arch_timer: Enabling workaround for %s\n", >>>>> + timer_unstable_counter_workaround->id); >>>>> + break; >>>>> + } >>>>> + } >>>>> +} >>>>> +#endif >>>>> + >>>>> /* note: this needs to be updated according to the doc of OEM ID >>>>> * and TABLE ID for different board. >>>>> */ >>>>> static struct gtdt_arch_timer_fixup arch_timer_quirks[] __initdata = { >>>>> +#ifdef CONFIG_HISILICON_ERRATUM_161010101 >>>>> + {"HISI ", "HIP05 ", 0, &hisi_erratum_workaroud_enable, "hisilicon,erratum-161010101"}, >>>>> + {"HISI ", "HIP06 ", 0, &hisi_erratum_workaroud_enable, "hisilicon,erratum-161010101"}, >>>>> + {"HISI ", "HIP07 ", 0, &hisi_erratum_workaroud_enable, "hisilicon,erratum-161010101"}, >>>>> +#endif >>>>> }; >>>> NAK. This duplicates logic unnecessarily (for enabling the workaround), >>>> and (ab)uses the id, which was intended to be specific to DT (since it >>>> is a DT property name). >>> Agreed, that's properly revolting. >>> >>>> We should split the matching from the particular workaround (and >>>> enabling thereof), so that we can go straight from ACPI match to >>>> workaround (without having to use the DT id in this manner), and don't >>>> have to duplicate the logic to enable the workaround. >>>> >>>> I believe Marc is looking at some rework in this area which may enable >>>> this, so please wait for that to appear. >>> Yeah, I'm implementing a semi-flexible way to add new quirk types, and >>> the last thing I want to see is two (or more) tables describing the same >>> thing. >> >> Kindly ping, if you have patches in hand, I can test against our platforms, >> thank you very much. > > I've just pushed out a branch based on arm64/for-next/core, plus the > HiSi timer workaround: > > https://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=timers/errata-rework > > I'll post the patches for review once the merge window is open, but > hopefully that should get you going. > > Thanks, > Test this branch and could work fine for Hisilicon board, great work ! Still need time to review the details, but this should not seem very difficult. :) Thanks Ding > M. > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html