Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in GTDT driver

From: Mark Rutland
Date: Thu Apr 06 2017 - 13:53:28 EST


On Fri, Apr 07, 2017 at 01:39:09AM +0800, Fu Wei wrote:
> On 7 April 2017 at 01:24, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> > On Fri, Apr 07, 2017 at 12:47:47AM +0800, Fu Wei wrote:
> >> On 6 April 2017 at 02:38, Mark Rutland <mark.rutland@xxxxxxx> wrote:
> >> > On Sat, Apr 01, 2017 at 01:51:03AM +0800, fu.wei@xxxxxxxxxx wrote:

> > However, I would prefer to simplify this such that we only free the
> > IRQs in the error path.
> >
> > We should be able to iterate over all freams, freeing any non-zero
> > interrupt, since !valid frames shouldn't have non-zero interrupts.
>
> Yes, that is what I am doing :
>
> if (!frame->valid)
> continue;

What I meant was that we won't look at the frame->valid flag at all;
only the interrupts. e.g.

for (int i = 0; i < ARCH_TIMER_MEM_MAX_FRAMES; i++) {
if (frame->phys_irq > 0)
free_the_phys_irq_somehow();
if (frame->virt_irq > 0)
free_the_virt_irq_somehow();
}

... where we somehow figure out the GSI, or we introduce an api like
unregister_gsi_for_irq(irq).

Since the !valid frames should all have zero for their interrupt fields,
no special handling is necessary.

That way, we only free the IRQs in one place, it's obvious that we
consistently free all of them, etc.

> Lorenzo addressed the API issue, we may can fix it by getting GSI info
> from DT, then register it until we figure the best frame.
> It may need some big change in DT code

I'd prefer to keep this constrained to the ACPI code. ;)

Thanks,
Mark.