From: Christoffer Dall <christoffer.dall@linaro.org>
To: Julien Grall <julien.grall@arm.com>
Cc: kvmarm@lists.cs.columbia.edu, marc.zyngier@arm.com,
fu.wei@linaro.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, wei@redhat.com,
al.stone@linaro.org, Thomas Gleixner <tglx@linutronix.de>,
Jason Cooper <jason@lakedaemon.net>
Subject: Re: [PATCH v3 5/9] irqchip/gic-v3: Gather all ACPI specific data in a single structure
Date: Tue, 8 Mar 2016 21:39:30 -0800 [thread overview]
Message-ID: <20160309053930.GH26583@lvm> (raw)
In-Reply-To: <1457436573-6180-6-git-send-email-julien.grall@arm.com>
On Tue, Mar 08, 2016 at 11:29:29AM +0000, Julien Grall wrote:
> Even though all the variables aren't marked with __initdata, they are
> only used during initialization. So the structure is marked with
> __initdata.
Not sure I understand this commit message.
As I see it, this commit includes two changes:
1. Mark the variables only used during init with __initdata
2. Move the variables into a structure
If I get that right, can you argue for both changes?
Thanks,
-Christoffer
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>
>
> ---
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
>
> Changes in v3:
> - Patch added
> ---
> drivers/irqchip/irq-gic-v3.c | 60 ++++++++++++++++++++++++--------------------
> 1 file changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index 5b7d3c2..50e87e6 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -968,19 +968,22 @@ out_unmap_dist:
> IRQCHIP_DECLARE(gic_v3, "arm,gic-v3", gic_of_init);
>
> #ifdef CONFIG_ACPI
> -static void __iomem *dist_base;
> -static struct redist_region *redist_regs __initdata;
> -static u32 nr_redist_regions __initdata;
> -static bool single_redist;
> +static struct
> +{
> + void __iomem *dist_base;
> + struct redist_region *redist_regs;
> + u32 nr_redist_regions;
> + bool single_redist;
> +} acpi_data __initdata;
>
> static void __init
> gic_acpi_register_redist(phys_addr_t phys_base, void __iomem *redist_base)
> {
> static int count = 0;
>
> - redist_regs[count].phys_base = phys_base;
> - redist_regs[count].redist_base = redist_base;
> - redist_regs[count].single_redist = single_redist;
> + acpi_data.redist_regs[count].phys_base = phys_base;
> + acpi_data.redist_regs[count].redist_base = redist_base;
> + acpi_data.redist_regs[count].single_redist = acpi_data.single_redist;
> count++;
> }
>
> @@ -1008,7 +1011,7 @@ gic_acpi_parse_madt_gicc(struct acpi_subtable_header *header,
> {
> struct acpi_madt_generic_interrupt *gicc =
> (struct acpi_madt_generic_interrupt *)header;
> - u32 reg = readl_relaxed(dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> + u32 reg = readl_relaxed(acpi_data.dist_base + GICD_PIDR2) & GIC_PIDR2_ARCH_MASK;
> u32 size = reg == GIC_PIDR2_ARCH_GICv4 ? SZ_64K * 4 : SZ_64K * 2;
> void __iomem *redist_base;
>
> @@ -1025,7 +1028,7 @@ static int __init gic_acpi_collect_gicr_base(void)
> acpi_tbl_entry_handler redist_parser;
> enum acpi_madt_type type;
>
> - if (single_redist) {
> + if (acpi_data.single_redist) {
> type = ACPI_MADT_TYPE_GENERIC_INTERRUPT;
> redist_parser = gic_acpi_parse_madt_gicc;
> } else {
> @@ -1076,14 +1079,14 @@ static int __init gic_acpi_count_gicr_regions(void)
> count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_REDISTRIBUTOR,
> gic_acpi_match_gicr, 0);
> if (count > 0) {
> - single_redist = false;
> + acpi_data.single_redist = false;
> return count;
> }
>
> count = acpi_table_parse_madt(ACPI_MADT_TYPE_GENERIC_INTERRUPT,
> gic_acpi_match_gicc, 0);
> if (count > 0)
> - single_redist = true;
> + acpi_data.single_redist = true;
>
> return count;
> }
> @@ -1103,7 +1106,7 @@ static bool __init acpi_validate_gic_table(struct acpi_subtable_header *header,
> if (count <= 0)
> return false;
>
> - nr_redist_regions = count;
> + acpi_data.nr_redist_regions = count;
> return true;
> }
>
> @@ -1114,25 +1117,28 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
> {
> struct acpi_madt_generic_distributor *dist;
> struct fwnode_handle *domain_handle;
> + size_t size;
> int i, err;
>
> /* Get distributor base address */
> dist = (struct acpi_madt_generic_distributor *)header;
> - dist_base = ioremap(dist->base_address, ACPI_GICV3_DIST_MEM_SIZE);
> - if (!dist_base) {
> + acpi_data.dist_base = ioremap(dist->base_address,
> + ACPI_GICV3_DIST_MEM_SIZE);
> + if (!acpi_data.dist_base) {
> pr_err("Unable to map GICD registers\n");
> return -ENOMEM;
> }
>
> - err = gic_validate_dist_version(dist_base);
> + err = gic_validate_dist_version(acpi_data.dist_base);
> if (err) {
> - pr_err("No distributor detected at @%p, giving up", dist_base);
> + pr_err("No distributor detected at @%p, giving up",
> + acpi_data.dist_base);
> goto out_dist_unmap;
> }
>
> - redist_regs = kzalloc(sizeof(*redist_regs) * nr_redist_regions,
> - GFP_KERNEL);
> - if (!redist_regs) {
> + size = sizeof(*acpi_data.redist_regs) * acpi_data.nr_redist_regions;
> + acpi_data.redist_regs = kzalloc(size, GFP_KERNEL);
> + if (!acpi_data.redist_regs) {
> err = -ENOMEM;
> goto out_dist_unmap;
> }
> @@ -1141,14 +1147,14 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
> if (err)
> goto out_redist_unmap;
>
> - domain_handle = irq_domain_alloc_fwnode(dist_base);
> + domain_handle = irq_domain_alloc_fwnode(acpi_data.dist_base);
> if (!domain_handle) {
> err = -ENOMEM;
> goto out_redist_unmap;
> }
>
> - err = gic_init_bases(dist_base, redist_regs, nr_redist_regions, 0,
> - domain_handle);
> + err = gic_init_bases(acpi_data.dist_base, acpi_data.redist_regs,
> + acpi_data.nr_redist_regions, 0, domain_handle);
> if (err)
> goto out_fwhandle_free;
>
> @@ -1158,12 +1164,12 @@ gic_acpi_init(struct acpi_subtable_header *header, const unsigned long end)
> out_fwhandle_free:
> irq_domain_free_fwnode(domain_handle);
> out_redist_unmap:
> - for (i = 0; i < nr_redist_regions; i++)
> - if (redist_regs[i].redist_base)
> - iounmap(redist_regs[i].redist_base);
> - kfree(redist_regs);
> + for (i = 0; i < acpi_data.nr_redist_regions; i++)
> + if (acpi_data.redist_regs[i].redist_base)
> + iounmap(acpi_data.redist_regs[i].redist_base);
> + kfree(acpi_data.redist_regs);
> out_dist_unmap:
> - iounmap(dist_base);
> + iounmap(acpi_data.dist_base);
> return err;
> }
> IRQCHIP_ACPI_DECLARE(gic_v3, ACPI_MADT_TYPE_GENERIC_DISTRIBUTOR,
> --
> 1.9.1
>
next prev parent reply other threads:[~2016-03-09 5:40 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-08 11:29 [PATCH v3 0/9] arm64: Add support of KVM with ACPI Julien Grall
2016-03-08 11:29 ` [PATCH v3 1/9] clocksource: arm_arch_timer: Gather KVM specific information in a structure Julien Grall
2016-03-09 3:23 ` Christoffer Dall
2016-03-09 5:35 ` Julien Grall
2016-03-08 11:29 ` [PATCH v3 2/9] clocksource: arm_arch_timer: Extend arch_timer_kvm_info to get the virtual IRQ Julien Grall
2016-03-09 3:27 ` Christoffer Dall
2016-03-09 5:52 ` Julien Grall
2016-03-08 11:29 ` [PATCH v3 3/9] irqchip/gic-v2: Gather ACPI specific data in a single structure Julien Grall
2016-03-09 5:47 ` Christoffer Dall
2016-03-09 6:18 ` Julien Grall
2016-03-13 18:19 ` Christoffer Dall
2016-03-08 11:29 ` [PATCH v3 4/9] irqchip/gic-v2: Parse and export virtual GIC information Julien Grall
2016-03-09 5:14 ` Christoffer Dall
2016-03-14 12:19 ` Julien Grall
2016-03-08 11:29 ` [PATCH v3 5/9] irqchip/gic-v3: Gather all ACPI specific data in a single structure Julien Grall
2016-03-09 5:39 ` Christoffer Dall [this message]
2016-03-15 12:26 ` Julien Grall
2016-03-18 9:42 ` Christoffer Dall
2016-03-08 11:29 ` [PATCH v3 6/9] irqchip/gic-v3: Parse and export virtual GIC information Julien Grall
2016-03-09 5:53 ` Christoffer Dall
2016-03-22 11:27 ` Graeme Gregory
2016-03-22 12:02 ` Julien Grall
2016-03-08 11:29 ` [PATCH v3 7/9] KVM: arm/arm64: arch_timer: Rely on the arch timer to parse the firmware tables Julien Grall
2016-03-09 5:55 ` Christoffer Dall
2016-03-08 11:29 ` [PATCH v3 8/9] KVM: arm/arm64: vgic: Rely on the GIC driver " Julien Grall
2016-03-09 6:02 ` Christoffer Dall
2016-03-08 11:29 ` [PATCH v3 9/9] clocksource: arm_arch_timer: Remove arch_timer_get_timecounter Julien Grall
2016-03-09 6:02 ` Christoffer Dall
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160309053930.GH26583@lvm \
--to=christoffer.dall@linaro.org \
--cc=al.stone@linaro.org \
--cc=fu.wei@linaro.org \
--cc=jason@lakedaemon.net \
--cc=julien.grall@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marc.zyngier@arm.com \
--cc=tglx@linutronix.de \
--cc=wei@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).