mbox series

[v24,00/11] acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer

Message ID 20170414184014.8524-1-fu.wei@linaro.org
Headers show
Series acpi, clocksource: add GTDT driver and GTDT support in arm_arch_timer | expand

Message

Fu Wei Fu April 14, 2017, 6:40 p.m. UTC 
From: Fu Wei <fu.wei@linaro.org>


This patchset:
    (1)Preparation for adding GTDT support in arm_arch_timer:
        1. Introduce a MMIO CNTFRQ helper.
        2. separate out device-tree code from arch_timer_detect_rate
        3. replace arch_timer_detect_rate with arch_timer_of_configure_rate
        4. Refactor arch_timer_needs_probing, and move it into DT init call
        5. Introduce some new structs and refactor the MMIO timer init code
        for reusing some common code.

    (2)Introduce ACPI GTDT parser: drivers/acpi/arm64/gtdt.c
    Parse all kinds of timer in GTDT table of ACPI:arch timer,
    memory-mapped timer and SBSA Generic Watchdog timer.
    This driver can help to simplify all the relevant timer drivers,
    and separate all the ACPI GTDT knowledge from them.

    (3)Simplify ACPI code for arm_arch_timer

    (4)Add GTDT support for ARM memory-mapped timer.

This patchset has been tested on the following platforms with ACPI enabled:
    (1)ARM Foundation v8 model

Changelog:
v24: https://lkml.org/lkml/2017/4/14/
     Mark Rutland:
         Rework comments
         Refactor the probing split
         Verify CNTFRQ:arch_timer_mem_verify_cntfrq
         Only register the first frame:arch_timer_mem_find_best_frame
     Fu Wei:
         Fetches from git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/gtdt
         Introduces acpi_unregister_irq function, and use it in drivers/acpi/arm64/gtdt.c

v23: https://lkml.org/lkml/2017/3/31/629
     Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/cleanup
     Improve the data struct of arch_timer_mem and arch_timer_mem_frame to
     improve the parser of GT blocks and arch_timer_mem initualization.
     Improve arch_timer_rate detection: sysreg frequency is primary in DT boot
     Improve some comments in GTDT parser driver.
     Improve acpi_gtdt_init function, and make a comment for the multiple calls.
     Improve the unwinding for the irq of timers, when an error occurs.
     Handle the case of virtual timer GSIV is 0.

v22: https://lkml.org/lkml/2017/3/21/523
     Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/cleanup
     Only Introduce arch_timer_mem_get_cntfrq to get the frequency from mmio.
     Merged patch 2,3(about arch_timer_detect_rate).
     Keep arch_timer_rate, do NOT split it for different types of timer.
     Improve  memory-mapped timer support by comments and variable name:
         data-->timer_mem
         frame-->gtdt_frame
     Delete zero check for SBSA watchdog irq.
     Skip secure SBSA watchdog in GTDT driver.
     Delete Kconfig modification for SBSA watchdog driver.
     Delete no_irq, using nr_res instead.

v21: https://lkml.org/lkml/2017/2/6/734
     Introduce two functions to get the frequency from mmio and sysreg.
     Remove arch_timer_detect_rate use arch_timer_get_*_freq directly
     Split arch_timer_rate for different types of timer.
     Skip secure timer frame in GTDT driver.
     Rebase to git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git arch-timer/cleanup
     (The first 6 patches in v20 have been merged into arch-timer/cleanup branch)

v20: https://lkml.org/lkml/2017/1/18/534
     Reorder the first 4 patches and split the 4th patches.
     Leave CNTHCTL_* as they originally were.
     Fix the bug in arch_timer_select_ppi.
     Split "Rework counter frequency detection" patch.
     Rework the arch_timer_detect_rate function.
     Improve the commit message of "Refactor MMIO timer probing".
     Rebase to 4.10.0-rc4

v19: https://lkml.org/lkml/2016/12/21/25
     Fix a '\n' missing in a error message in arch_timer_mem_init.
     Add "request_mem_region" for ioremapping cntbase, according to
     f947ee1 clocksource/drivers/arm_arch_timer: Map frame with of_io_request_and_map()
     Rebase to 4.9.0-gfb779ff

v18: https://lkml.org/lkml/2016/12/8/446
     Fix 8/15 patch problem of "int ret;" in arch_timer_acpi_init.
     Rebase to 4.9.0-rc8-g9269898

v17: https://lkml.org/lkml/2016/11/25/140
     Take out some cleanups from 4/15.
     Merge 5/15 and 6/15, improve PPI determination code,
     improve commit message.
     Rework counter frequency detection.
     Move arch_timer_needs_of_probing into DT init call.
     Move Platform Timer scan loop back to timer init call to avoid allocating
     and free memory.
     Improve all the exported functions' comment.

v16: https://lkml.org/lkml/2016/11/16/268
     Fix patchset problem about static enum ppi_nr of 01/13 in v15.
     Refactor arch_timer_detect_rate.
     Refactor arch_timer_needs_probing.

v15: https://lkml.org/lkml/2016/11/15/366
     Re-order patches
     Add arm_arch_timer refactoring patches to prepare for GTDT:
         1. rename some  enums and defines, and some cleanups
         2. separate out arch_timer_uses_ppi init code and fix a potential bug
         3. Improve some new structs, refactor the timer init code.
     Since the some structs have been changed, GTDT parser for memory-mapped
     timer and SBSA Generic Watchdog timer have been update.

v14: https://lkml.org/lkml/2016/9/28/573
     Separate memory-mapped timer GTDT support into two patches
         1. Refactor the timer init code to prepare for GTDT
         2. Add GTDT support for memory-mapped timer

v13: http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1231717.html
     Improve arm_arch_timer code for memory-mapped
     timer GTDT support, refactor original memory-mapped timer
     dt support for reusing some common code.

v12: https://lkml.org/lkml/2016/9/13/250
     Rebase to latest Linux 4.8-rc6
     Delete the confusing "skipping" in the error message.

V11: https://lkml.org/lkml/2016/9/6/354
     Rebase to latest Linux 4.8-rc5
     Delete typedef (suggested by checkpatch.pl)

V10: https://lkml.org/lkml/2016/7/26/215
     Drop the "readq" patch.
     Rebase to latest Linux 4.7.

V9: https://lkml.org/lkml/2016/7/25/345
    Improve pr_err message in acpi gtdt driver.
    Update Commit message for 7/9
    shorten the irq mapping function name
    Improve GTDT driver for memory-mapped timer

v8: https://lkml.org/lkml/2016/7/19/660
    Improve "pr_fmt(fmt)" definition: add "ACPI" in front of "GTDT",
    and also improve printk message.
    Simplify is_timer_block and is_watchdog.
    Merge acpi_gtdt_desc_init and gtdt_arch_timer_init into acpi_gtdt_init();
    Delete __init in include/linux/acpi.h for GTDT API
    Make ARM64 select GTDT.
    Delete "#include <linux/module.h>" from acpi_gtdt.c
    Simplify GT block parse code.

v7: https://lkml.org/lkml/2016/7/13/769
    Move the GTDT driver to drivers/acpi/arm64
    Add add the ARM64-specific ACPI Support maintainers in MAINTAINERS
    Merge 3 patches of GTDT parser driver.
    Fix the for_each_platform_timer bug.

v6: https://lkml.org/lkml/2016/6/29/580
    split the GTDT driver to 4 parts: basic, arch_timer, memory-mapped timer,
    and SBSA Generic Watchdog timer
    Improve driver by suggestions and example code from Daniel Lezcano

v5: https://lkml.org/lkml/2016/5/24/356
    Sorting out all patches, simplify the API of GTDT driver:
    GTDT driver just fills the data struct for arm_arch_timer driver.

v4: https://lists.linaro.org/pipermail/linaro-acpi/2016-March/006667.html
    Delete the kvm relevant patches
    Separate two patches for sorting out the code for arm_arch_timer.
    Improve irq info export code to allow missing irq info in GTDT table.

v3: https://lkml.org/lkml/2016/2/1/658
    Improve GTDT driver code:
      (1)improve pr_* by defining pr_fmt(fmt)
      (2)simplify gtdt_sbsa_gwdt_init
      (3)improve gtdt_arch_timer_data_init, if table is NULL, it will try
      to get GTDT table.
    Move enum ppi_nr to arm_arch_timer.h, and add enum spi_nr.
    Add arm_arch_timer get ppi from DT and GTDT support for kvm.

v2: https://lkml.org/lkml/2015/12/2/10
    Rebase to latest kernel version(4.4-rc3).
    Fix the bug about the config problem,
    use CONFIG_ACPI_GTDT instead of CONFIG_ACPI in arm_arch_timer.c

v1: The first upstreaming version: https://lkml.org/lkml/2015/10/28/553

Fu Wei (11):
  clocksource: arm_arch_timer: split dt-only rate handling
  clocksource: arm_arch_timer: refactor arch_timer_needs_probing
  clocksource: arm_arch_timer: move arch_timer_needs_of_probing into DT
    init call
  clocksource: arm_arch_timer: add structs to describe MMIO timer
  clocksource: arm_arch_timer: split MMIO timer probing.
  acpi/arm64: Add GTDT table parse driver
  clocksource: arm_arch_timer: simplify ACPI support code.
  acpi: Add acpi_unregister_irq function
  acpi/arm64: Add memory-mapped timer support in GTDT driver
  clocksource: arm_arch_timer: add GTDT support for memory-mapped timer
  acpi/arm64: Add SBSA Generic Watchdog support in GTDT driver

 arch/arm64/Kconfig                   |   1 +
 drivers/acpi/arm64/Kconfig           |   3 +
 drivers/acpi/arm64/Makefile          |   1 +
 drivers/acpi/arm64/gtdt.c            | 404 +++++++++++++++++++++++++++++++++++
 drivers/acpi/irq.c                   |  10 +
 drivers/clocksource/arm_arch_timer.c | 393 ++++++++++++++++++++++++----------
 include/clocksource/arm_arch_timer.h |  16 ++
 include/linux/acpi.h                 |  14 ++
 8 files changed, 724 insertions(+), 118 deletions(-)
 create mode 100644 drivers/acpi/arm64/gtdt.c

-- 
2.9.3

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Mark Rutland April 18, 2017, 3:58 p.m. UTC | #1 
On Sat, Apr 15, 2017 at 02:40:11AM +0800, fu.wei@linaro.org wrote:
> From: Fu Wei <fu.wei@linaro.org>

> 

> This patch introduces acpi_unregister_irq function to free a

> linux IRQ number<->GSI mapping by a given linux IRQ number.

> 

> Even we have successfully registered the GSI, when some error occurs, we

> may need to unmap it for freeing the IRQ resource. But in some cases, we

> only have IRQ, but not GSI.

> 

> This patch is the preparation for memory-mapped timer support in GTDT

> driver.

> 

> Signed-off-by: Fu Wei <fu.wei@linaro.org>

> ---

>  drivers/acpi/irq.c   | 10 ++++++++++

>  include/linux/acpi.h |  7 +++++++

>  2 files changed, 17 insertions(+)


This will need comments/acks from the ACPI folk.

Lorenzo, do you prefer this to direct use of irq_dispose_mapping() in
the GTDT parsing code?

Thanks,
Mark.

> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c

> index 830299a..59de777 100644

> --- a/drivers/acpi/irq.c

> +++ b/drivers/acpi/irq.c

> @@ -85,6 +85,16 @@ void acpi_unregister_gsi(u32 gsi)

>  EXPORT_SYMBOL_GPL(acpi_unregister_gsi);

>  

>  /**

> + * acpi_unregister_irq() - Free a linux IRQ number<->GSI mapping

> + * @irq: linux IRQ number

> + */

> +void acpi_unregister_irq(int irq)

> +{

> +	irq_dispose_mapping(irq);

> +}

> +EXPORT_SYMBOL_GPL(acpi_unregister_irq);

> +

> +/**

>   * acpi_get_irq_source_fwhandle() - Retrieve fwhandle from IRQ resource source.

>   * @source: acpi_resource_source to use for the lookup.

>   *

> diff --git a/include/linux/acpi.h b/include/linux/acpi.h

> index 4b5c146..728d1ed 100644

> --- a/include/linux/acpi.h

> +++ b/include/linux/acpi.h

> @@ -336,6 +336,13 @@ extern int acpi_get_override_irq(u32 gsi, int *trigger, int *polarity);

>   */

>  void acpi_unregister_gsi (u32 gsi);

>  

> +/*

> + * This function undoes the effect of one call to acpi_register_gsi().

> + * The irq should be the return value of acpi_register_gsi().

> + * If the irq is valid, free a linux IRQ number<->GSI mapping.

> + */

> +void acpi_unregister_irq(int irq);

> +

>  struct pci_dev;

>  

>  int acpi_pci_irq_enable (struct pci_dev *dev);

> -- 

> 2.9.3

> 

--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland April 19, 2017, 3:45 p.m. UTC | #2 
On Wed, Apr 19, 2017 at 04:07:25PM +0100, Mark Rutland wrote:
> On Tue, Apr 18, 2017 at 06:21:07PM +0100, Lorenzo Pieralisi wrote:

> > On Sat, Apr 15, 2017 at 02:40:12AM +0800, fu.wei@linaro.org wrote:

 
> > If yes, why can't it simply be written like this ?

> > 

> > for (; i >= 0; i--, gtdt_frame--) {

> > 	frame = &timer_mem->frame[gtdt_frame->frame_number];

> > 

> > 	/* not sure this check is actually needed */

> > 	if (gtdt_frame->common_flags & ACPI_GTDT_GT_IS_SECURE_TIMER)

> > 		continue;

> > 

> > 	if (frame->phys_irq > 0)

> > 		acpi_unregister_gsi(gtdt_frame->timer_interrupt);

> > 	if (frame->virt_irq > 0)

> > 		acpi_unregister_gsi(gtdt_frame->virtual_timer_interrupt);

> > }

> 

> A reverse loop of this form will work.

> 

> That requires some restructuring, and care to avoid going out of bounds

> instantaneously with the gtdt_frame--, so as to not invoke nasal demons.

> 

> I've attacked this locally, and will send this out after testing. I'll

> drop the new ACPI API patch.


FWIW, I've set this up so the cleanup path is:

do {
        if (gtdt_frame->common_flags & ACPI_GTDT_GT_IS_SECURE_TIMER ||
            gtdt_frame->frame_number >= ARCH_TIMER_MEM_MAX_FRAMES)
                continue;

        frame = &timer_mem->frame[gtdt_frame->frame_number];

        if (frame->phys_irq > 0)
                acpi_unregister_gsi(gtdt_frame->timer_interrupt);
        frame->phys_irq = 0;

        if (frame->virt_irq > 0)
                acpi_unregister_gsi(gtdt_frame->virtual_timer_interrupt);
        frame->virt_irq = 0;
} while (i-- >= 0 && gtdt_frame--);

... the zeroing is to account for duplicate frames, which I now check for in
the probe path (as we do for DT).

Can I take it per your comment on the prior version that with this change I can
take your ack?

I also assume that you're happy for all of the drivers/acpi/arm64/ patches in the
series to go via the clocksource tree?

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Pieralisi April 19, 2017, 3:56 p.m. UTC | #3 
On Wed, Apr 19, 2017 at 04:45:54PM +0100, Mark Rutland wrote:
> On Wed, Apr 19, 2017 at 04:07:25PM +0100, Mark Rutland wrote:

> > On Tue, Apr 18, 2017 at 06:21:07PM +0100, Lorenzo Pieralisi wrote:

> > > On Sat, Apr 15, 2017 at 02:40:12AM +0800, fu.wei@linaro.org wrote:

>  

> > > If yes, why can't it simply be written like this ?

> > > 

> > > for (; i >= 0; i--, gtdt_frame--) {

> > > 	frame = &timer_mem->frame[gtdt_frame->frame_number];

> > > 

> > > 	/* not sure this check is actually needed */

> > > 	if (gtdt_frame->common_flags & ACPI_GTDT_GT_IS_SECURE_TIMER)

> > > 		continue;

> > > 

> > > 	if (frame->phys_irq > 0)

> > > 		acpi_unregister_gsi(gtdt_frame->timer_interrupt);

> > > 	if (frame->virt_irq > 0)

> > > 		acpi_unregister_gsi(gtdt_frame->virtual_timer_interrupt);

> > > }

> > 

> > A reverse loop of this form will work.

> > 

> > That requires some restructuring, and care to avoid going out of bounds

> > instantaneously with the gtdt_frame--, so as to not invoke nasal demons.

> > 

> > I've attacked this locally, and will send this out after testing. I'll

> > drop the new ACPI API patch.

> 

> FWIW, I've set this up so the cleanup path is:

> 

> do {

>         if (gtdt_frame->common_flags & ACPI_GTDT_GT_IS_SECURE_TIMER ||

>             gtdt_frame->frame_number >= ARCH_TIMER_MEM_MAX_FRAMES)

>                 continue;

> 

>         frame = &timer_mem->frame[gtdt_frame->frame_number];

> 

>         if (frame->phys_irq > 0)

>                 acpi_unregister_gsi(gtdt_frame->timer_interrupt);

>         frame->phys_irq = 0;

> 

>         if (frame->virt_irq > 0)

>                 acpi_unregister_gsi(gtdt_frame->virtual_timer_interrupt);

>         frame->virt_irq = 0;

> } while (i-- >= 0 && gtdt_frame--);

> 

> ... the zeroing is to account for duplicate frames, which I now check for in

> the probe path (as we do for DT).

> 

> Can I take it per your comment on the prior version that with this change I can

> take your ack?


Yes, thanks for fixing it up, please add my:

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>


> I also assume that you're happy for all of the drivers/acpi/arm64/ patches in the

> series to go via the clocksource tree?


Yes that's how I expect them to go upstream, that's the simplest way.

Thanks !
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html