Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1618211

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

Show all headers | View raw

On Fri, Apr 07, 2017 at 12:47:47AM +0800, Fu Wei wrote:
> On 6 April 2017 at 02:38, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Sat, Apr 01, 2017 at 01:51:03AM +0800, fu.wei@linaro.org wrote:
> >> +     /*
> >> +      * Get the GT timer Frame data for every GT Block Timer
> >> +      */
> >> +     for (i = 0; i < block->timer_count; i++, gtdt_frame++) {
> >> +             if (gtdt_frame->common_flags & ACPI_GTDT_GT_IS_SECURE_TIMER)
> >> +                     continue;
> >> +
> >> +             if (!gtdt_frame->base_address || !gtdt_frame->timer_interrupt)
> >> +                     goto error;
> >> +
> >> +             frame = &timer_mem->frame[gtdt_frame->frame_number];
> >> +             frame->phys_irq = map_gt_gsi(gtdt_frame->timer_interrupt,
> >> +                                          gtdt_frame->timer_flags);
> >> +             if (frame->phys_irq <= 0) {
> >> +                     pr_warn("failed to map physical timer irq in frame %d.\n",
> >> +                             gtdt_frame->frame_number);
> >> +                     goto error;
> >> +             }
> >> +
> >> +             if (gtdt_frame->virtual_timer_interrupt) {
> >> +                     frame->virt_irq =
> >> +                             map_gt_gsi(gtdt_frame->virtual_timer_interrupt,
> >> +                                        gtdt_frame->virtual_timer_flags);
> >> +                     if (frame->virt_irq <= 0) {
> >> +                             pr_warn("failed to map virtual timer irq in frame %d.\n",
> >> +                                     gtdt_frame->frame_number);
> >> +                             acpi_unregister_gsi(gtdt_frame->timer_interrupt);
> >> +                             goto error;
> >> +                     }
> >> +             } else {
> >> +                     frame->virt_irq = 0;
> >> +                     pr_debug("virtual timer in frame %d not implemented.\n",
> >> +                              gtdt_frame->frame_number);
> >> +             }
> >> +
> >> +             frame->cntbase = gtdt_frame->base_address;
> >> +             /*
> >> +              * The CNTBaseN frame is 4KB (register offsets 0x000 - 0xFFC).
> >> +              * See ARM DDI 0487A.k_iss10775, page I1-5130, Table I1-4
> >> +              * "CNTBaseN memory map".
> >> +              */
> >> +             frame->size = SZ_4K;
> >> +             frame->valid = true;
> >> +     }
> >> +
> >> +     return 0;
> >> +
> >> +error:
> >> +     for (i = 0; i < ARCH_TIMER_MEM_MAX_FRAMES; i++) {
> >> +             frame = &timer_mem->frame[i];
> >> +             if (!frame->valid)
> >> +                     continue;
> >> +             irq_dispose_mapping(frame->phys_irq);
> >> +             if (frame->virt_irq)
> >> +                     irq_dispose_mapping(frame->virt_irq);
> >> +     }
> >
> > We assign interrupts and may goto error before setting valid, so here we
> 
> yes, I mean to do it.(setting valid at the end of loop)
> 
> > won't free the interrupts of the last frame we parsed.
> 
> that won't  be a problem, we may assign two interrupts in a round:
> First of all, if  the assignment goes  wrong, that means the current
> interrupt haven't been successfully assigned.
> (1)if the first goes wrong, the we goto error to unwind  the irqs
> assigned in previous rounds.
> (2)if the second one goes wrong , we acpi_unregister_gsi the first one
> and then  goto error to unwind  the irqs assigned in previous rounds.
> (3)If the two assignments are successful, set up valid flag
> 
> So we won't miss freeing the interrupts of the last frame we parsed.
> 
> Did I miss something?

No; you are correct, and I was mistaken.

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.

I can make that update locally; no need to respin.

Thanks,
Mark.

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v23 00/11] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 06/11] clocksource: arm_arch_timer: refactor MMIO timer probing. fu.wei@linaro.org - 2017-03-31 20:00 +0200
    Re: [PATCH v23 06/11] clocksource: arm_arch_timer: refactor MMIO  timer probing. Mark Rutland <mark.rutland@arm.com> - 2017-04-05 20:50 +0200
      Re: [PATCH v23 06/11] clocksource: arm_arch_timer: refactor MMIO  timer probing. Fu Wei <fu.wei@linaro.org> - 2017-04-06 12:50 +0200
  [PATCH v23 04/11] clocksource: arm_arch_timer: move arch_timer_needs_of_probing into DT init call fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 08/11] clocksource: arm_arch_timer: simplify ACPI support code. fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in GTDT driver fu.wei@linaro.org - 2017-03-31 20:00 +0200
    Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Will Deacon <will.deacon@arm.com> - 2017-04-03 11:50 +0200
    Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-04-03 12:50 +0200
      Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Fu Wei <fu.wei@linaro.org> - 2017-04-06 19:20 +0200
    Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Mark Rutland <mark.rutland@arm.com> - 2017-04-05 20:40 +0200
      Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Mark Rutland <mark.rutland@arm.com> - 2017-04-06 12:10 +0200
      Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Fu Wei <fu.wei@linaro.org> - 2017-04-06 18:50 +0200
        Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Mark Rutland <mark.rutland@arm.com> - 2017-04-06 19:30 +0200
          Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Fu Wei <fu.wei@linaro.org> - 2017-04-06 19:50 +0200
            Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Mark Rutland <mark.rutland@arm.com> - 2017-04-06 20:00 +0200
              Re: [PATCH v23 09/11] acpi/arm64: Add memory-mapped timer support in  GTDT driver Fu Wei <fu.wei@linaro.org> - 2017-04-06 20:10 +0200
  [PATCH v23 07/11] acpi/arm64: Add GTDT table parse driver fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 05/11] clocksource: arm_arch_timer: add structs to describe MMIO timer fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 01/11] clocksource: arm_arch_timer: add MMIO CNTFRQ helper fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 11/11] acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 02/11] clocksource: arm_arch_timer: split dt-only rate handling fu.wei@linaro.org - 2017-03-31 20:00 +0200
  [PATCH v23 03/11] clocksource: arm_arch_timer: refactor arch_timer_needs_probing fu.wei@linaro.org - 2017-03-31 20:00 +0200
  Re: [PATCH v23 00/11] acpi, clocksource: add GTDT driver and GTDT  support in arm_arch_timer Xiongfeng Wang <wangxiongfeng2@huawei.com> - 2017-04-01 04:20 +0200
    Re: [PATCH v23 00/11] acpi, clocksource: add GTDT driver and GTDT  support in arm_arch_timer Fu Wei <fu.wei@linaro.org> - 2017-04-01 05:50 +0200
  Re: [PATCH v23 00/11] acpi, clocksource: add GTDT driver and GTDT  support in arm_arch_timer Timur Tabi <timur@codeaurora.org> - 2017-04-04 22:40 +0200

csiph-web