From: suravee.suthikulpanit@amd.com (Suravee Suthikulpanit)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2 V4] irqchip: gic: Add supports for ARM GICv2m MSI(-X)
Date: Thu, 28 Aug 2014 03:59:40 -0500 [thread overview]
Message-ID: <53FEEF7C.1090902@amd.com> (raw)
In-Reply-To: <87fvgxrgte.fsf@approximate.cambridge.arm.com>
On 08/15/2014 09:03 AM, Marc Zyngier wrote:
>> +
>> +static struct irq_chip gicv2m_chip;
>> +
>> +#ifdef CONFIG_OF
>
> Is there any reason why this should be guarded by CONFIG_OF? Surely the
> v2m capability should only be enabled if OF is.
[Suravee]
We are also planning to support ACPI in the future also, which will be
using a different init function. Also, there is the same #ifdef in the
irq-gic.c for the gic_of_init(). So, I am just trying to be consistent
here.
>> +
>> + memcpy(&gicv2m_chip, gic->irq_chip, sizeof(struct irq_chip));
>> + gicv2m_chip.name = "GICv2m",
>> + gicv2m_chip.irq_mask = gicv2m_mask_irq;
>> + gicv2m_chip.irq_unmask = gicv2m_unmask_irq;
>> + gic->irq_chip = &gicv2m_chip;
>
> I liked it until this last line. You're overriding the irq_chip for the
> whole GIC. I was expecting you'd only use it for the MSI range
> (basically return a range to the caller, together with your brand new
> irq_chip).
[Suravee]
I'm not sure if I understand you point here. Actually, I don't see the
whole point of the need to have a whole different irq_chip for v2m
stuff. All I need is just a way to overwrite the irq_chip.irq_mask()
and irq_chip.irq_unmask() with the v2m version which should check for
MSI before calling mask/unmask_msi_irq(). I should be able to just do:
gic->irq_chip.irq_mask = gicv2m_mask_irq;
gic->irq_chip.irq_unmask = gicv2m_unmask_irq;
>> @@ -768,19 +768,21 @@ void __init gic_init_physaddr(struct device_node *node)
>> static int gic_irq_domain_map(struct irq_domain *d, unsigned int irq,
>> irq_hw_number_t hw)
>> {
>> + struct gic_chip_data *gic = d->host_data;
>> +
>> if (hw < 32) {
>> irq_set_percpu_devid(irq);
>> - irq_set_chip_and_handler(irq, &gic_chip,
>> + irq_set_chip_and_handler(irq, gic->irq_chip,
>> handle_percpu_devid_irq);
>> set_irq_flags(irq, IRQF_VALID | IRQF_NOAUTOEN);
>> } else {
>> - irq_set_chip_and_handler(irq, &gic_chip,
>> + irq_set_chip_and_handler(irq, gic->irq_chip,
>> handle_fasteoi_irq);
>
> And here you should discriminate on whether this is MSI or not, based on
> the range you got from above.
>
[Suravee]
From above, since we only use one irq_chip (i.e. the gic_chip), there
is no need to differentiate here, and I don't need to make these two
line changes.
>> @@ -1009,6 +1012,16 @@ gic_of_init(struct device_node *node, struct device_node *parent)
>> if (of_property_read_u32(node, "cpu-offset", &percpu_offset))
>> percpu_offset = 0;
>>
>> + gic_data[gic_cnt].irq_chip = &gic_chip;
>> +
>> + /* Currently, we only support one v2m subnode. */
>> + child = of_get_child_by_name(node, "v2m");
>
> If you only support one v2m node, then you should also enforce it for
> potential secondaty GICs (just probing it for gic_cnt == 0 should be
> enough).
[Suravee]
Actually, if we have multiple (N) GICs, we should be able to also
support multiple (N) V2Ms with the followings entries.
gic0 {
.....
v2m {
....
}
}
gic1 {
.....
v2m {
....
}
}
What I am not trying to support at this point is the following:
gic0 {
....
v2m {
....
}
v2m {
....
}
}
>> diff --git a/drivers/irqchip/irq-gic.h b/drivers/irqchip/irq-gic.h
>> new file mode 100644
>> index 0000000..2ec6bc3
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-gic.h
>> @@ -0,0 +1,48 @@
>> +#ifndef _IRQ_GIC_H_
>> +#define _IRQ_GIC_H_
>> +
>> +#include <linux/msi.h>
>> +
>> +union gic_base {
>> + void __iomem *common_base;
>> + void __percpu * __iomem *percpu_base;
>> +};
>> +
>> +#ifdef CONFIG_ARM_GIC_V2M
>> +struct v2m_data {
>> + spinlock_t msi_cnt_lock;
>> + struct msi_chip msi_chip;
>> + struct resource res; /* GICv2m resource */
>> + void __iomem *base; /* GICv2m virt address */
>> + unsigned int spi_start; /* The SPI number that MSIs start */
>> + unsigned int nr_spis; /* The number of SPIs for MSIs */
>> + unsigned long *bm; /* MSI vector bitmap */
>> +};
>> +#endif
>
> So if you put the #ifdef/#endif *inside* the v2m_data structure...
[Suravee] Are you suggesting an empty struct v2m_data{}; Hm.. I guess I
can do that.
Thanks,
Suravee
next prev parent reply other threads:[~2014-08-28 8:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-13 15:00 [PATCH 0/2 V4] irqchip: gic: Introduce ARM GICv2m MSI(-X) support suravee.suthikulpanit at amd.com
2014-08-13 15:00 ` [PATCH 1/2 V4] irqchip: gic: Add supports for ARM GICv2m MSI(-X) suravee.suthikulpanit at amd.com
2014-08-14 2:56 ` Jingoo Han
2014-08-28 9:15 ` Suravee Suthikulpanit
2014-08-14 17:55 ` Mark Rutland
2014-08-28 9:03 ` Suravee Suthikulpanit
2014-09-08 23:05 ` Suravee Suthikulpanit
2014-09-09 11:03 ` Mark Rutland
2014-08-15 14:03 ` Marc Zyngier
2014-08-28 8:59 ` Suravee Suthikulpanit [this message]
2014-09-05 16:15 ` Marc Zyngier
2014-08-13 15:00 ` [PATCH 2/2 V4] irqchip: gicv2m: Add support for multiple MSI for ARM64 GICv2m suravee.suthikulpanit at amd.com
2014-08-15 13:31 ` Marc Zyngier
2014-08-15 14:53 ` Suravee Suthikulanit
2014-08-15 15:08 ` Marc Zyngier
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=53FEEF7C.1090902@amd.com \
--to=suravee.suthikulpanit@amd.com \
--cc=linux-arm-kernel@lists.infradead.org \
/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).