From: Marc Zyngier <maz@kernel.org>
To: Hector Martin <marcan@marcan.st>
Cc: Thomas Gleixner <tglx@linutronix.de>,
Rob Herring <robh+dt@kernel.org>, Sven Peter <sven@svenpeter.dev>,
Alyssa Rosenzweig <alyssa@rosenzweig.io>,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
Subject: Re: [PATCH 6/6] irqchip/apple-aic: Add support for AICv2
Date: Sun, 12 Dec 2021 18:47:23 +0000 [thread overview]
Message-ID: <87o85lu0ck.wl-maz@kernel.org> (raw)
In-Reply-To: <20211209043249.65474-7-marcan@marcan.st>
On Thu, 09 Dec 2021 04:32:49 +0000,
Hector Martin <marcan@marcan.st> wrote:
>
> Introduce support for the new AICv2 hardware block in t6000/t6001 SoCs.
>
> It seems these blocks are missing the information required to compute
> the event register offset in the capability registers, so we specify
> that in the DT.
>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
> drivers/irqchip/irq-apple-aic.c | 146 ++++++++++++++++++++++++++++----
> 1 file changed, 128 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c
> index 46b7750548a0..226d5232dd14 100644
> --- a/drivers/irqchip/irq-apple-aic.c
> +++ b/drivers/irqchip/irq-apple-aic.c
> @@ -101,6 +101,57 @@
>
> #define AIC_MAX_IRQ 0x400
>
> +/*
> + * AIC v2 registers (MMIO)
> + */
> +
> +#define AIC2_VERSION 0x0000
> +#define AIC2_VERSION_VER GENMASK(7, 0)
> +
> +#define AIC2_INFO1 0x0004
> +#define AIC2_INFO1_NR_IRQ GENMASK(15, 0)
> +#define AIC2_INFO1_LAST_DIE GENMASK(27, 24)
> +
> +#define AIC2_INFO2 0x0008
> +
> +#define AIC2_INFO3 0x000c
> +#define AIC2_INFO3_MAX_IRQ GENMASK(15, 0)
> +#define AIC2_INFO3_MAX_DIE GENMASK(27, 24)
> +
> +#define AIC2_RESET 0x0010
> +#define AIC2_RESET_RESET BIT(0)
> +
> +#define AIC2_CONFIG 0x0014
> +#define AIC2_CONFIG_ENABLE BIT(0)
> +#define AIC2_CONFIG_PREFER_PCPU BIT(28)
> +
> +#define AIC2_TIMEOUT 0x0028
> +#define AIC2_CLUSTER_PRIO 0x0030
> +#define AIC2_DELAY_GROUPS 0x0100
> +
> +#define AIC2_IRQ_CFG 0x2000
> +
> +/*
> + * AIC2 registers are laid out like this, starting at AIC2_IRQ_CFG:
> + *
> + * Repeat for each die:
> + * IRQ_CFG: u32 * MAX_IRQS
> + * SW_SET: u32 * (MAX_IRQS / 32)
> + * SW_CLR: u32 * (MAX_IRQS / 32)
> + * MASK_SET: u32 * (MAX_IRQS / 32)
> + * MASK_CLR: u32 * (MAX_IRQS / 32)
> + * HW_STATE: u32 * (MAX_IRQS / 32)
> + *
> + * This is followed by a set of event registers, each 16K page aligned.
> + * The first one is the AP event register we will use. Unfortunately,
> + * the actual implemented die count is not specified anywhere in the
> + * capability registers, so we have to explcitly specify the event
explicitly
> + * register offset in the device tree to remain forward-compatible.
Do the current machines actually have more than a single die?
> + */
> +
> +#define AIC2_IRQ_CFG_TARGET GENMASK(3, 0)
> +#define AIC2_IRQ_CFG_DELAY_IDX GENMASK(7, 5)
> +
> #define MASK_REG(x) (4 * ((x) >> 5))
> #define MASK_BIT(x) BIT((x) & GENMASK(4, 0))
>
> @@ -187,6 +238,7 @@ struct aic_info {
> /* Register offsets */
> u32 event;
> u32 target_cpu;
> + u32 irq_cfg;
> u32 sw_set;
> u32 sw_clr;
> u32 mask_set;
> @@ -214,6 +266,14 @@ static const struct aic_info aic1_fipi_info = {
> .fast_ipi = true,
> };
>
> +static const struct aic_info aic2_info = {
> + .version = 2,
> +
> + .irq_cfg = AIC2_IRQ_CFG,
> +
> + .fast_ipi = true,
> +};
> +
> static const struct of_device_id aic_info_match[] = {
> {
> .compatible = "apple,t8103-aic",
> @@ -223,6 +283,10 @@ static const struct of_device_id aic_info_match[] = {
> .compatible = "apple,aic",
> .data = &aic1_info,
> },
> + {
> + .compatible = "apple,aic2",
> + .data = &aic2_info,
> + },
> {}
> };
>
> @@ -368,6 +432,14 @@ static struct irq_chip aic_chip = {
> .irq_set_type = aic_irq_set_type,
> };
>
> +static struct irq_chip aic2_chip = {
> + .name = "AIC2",
> + .irq_mask = aic_irq_mask,
> + .irq_unmask = aic_irq_unmask,
> + .irq_eoi = aic_irq_eoi,
> + .irq_set_type = aic_irq_set_type,
> +};
How is the affinity managed if you don't have a callback? A number of
things are bound to break if you don't have one. And a description of
how an interrupt gets routed wouldn't go amiss!
> +
> /*
> * FIQ irqchip
> */
> @@ -524,10 +596,15 @@ static struct irq_chip fiq_chip = {
> static int aic_irq_domain_map(struct irq_domain *id, unsigned int irq,
> irq_hw_number_t hw)
> {
> + struct aic_irq_chip *ic = id->host_data;
> u32 type = FIELD_GET(AIC_EVENT_TYPE, hw);
> + struct irq_chip *chip = &aic_chip;
> +
> + if (ic->info.version == 2)
> + chip = &aic2_chip;
>
> if (type == AIC_EVENT_TYPE_HW) {
> - irq_domain_set_info(id, irq, hw, &aic_chip, id->host_data,
> + irq_domain_set_info(id, irq, hw, chip, id->host_data,
> handle_fasteoi_irq, NULL, NULL);
> irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(irq)));
> } else if (type == AIC_EVENT_TYPE_FIQ) {
> @@ -882,23 +959,25 @@ static int aic_init_cpu(unsigned int cpu)
> /* Commit all of the above */
> isb();
>
> - /*
> - * Make sure the kernel's idea of logical CPU order is the same as AIC's
> - * If we ever end up with a mismatch here, we will have to introduce
> - * a mapping table similar to what other irqchip drivers do.
> - */
> - WARN_ON(aic_ic_read(aic_irqc, AIC_WHOAMI) != smp_processor_id());
> + if (aic_irqc->info.version == 1) {
> + /*
> + * Make sure the kernel's idea of logical CPU order is the same as AIC's
> + * If we ever end up with a mismatch here, we will have to introduce
> + * a mapping table similar to what other irqchip drivers do.
> + */
> + WARN_ON(aic_ic_read(aic_irqc, AIC_WHOAMI) != smp_processor_id());
>
> - /*
> - * Always keep IPIs unmasked at the hardware level (except auto-masking
> - * by AIC during processing). We manage masks at the vIPI level.
> - */
> - aic_ic_write(aic_irqc, AIC_IPI_ACK, AIC_IPI_SELF | AIC_IPI_OTHER);
> - if (!aic_irqc->info.fast_ipi) {
> - aic_ic_write(aic_irqc, AIC_IPI_MASK_SET, AIC_IPI_SELF);
> - aic_ic_write(aic_irqc, AIC_IPI_MASK_CLR, AIC_IPI_OTHER);
> - } else {
> - aic_ic_write(aic_irqc, AIC_IPI_MASK_SET, AIC_IPI_SELF | AIC_IPI_OTHER);
> + /*
> + * Always keep IPIs unmasked at the hardware level (except auto-masking
> + * by AIC during processing). We manage masks at the vIPI level.
> + */
> + aic_ic_write(aic_irqc, AIC_IPI_ACK, AIC_IPI_SELF | AIC_IPI_OTHER);
> + if (!aic_irqc->info.fast_ipi) {
> + aic_ic_write(aic_irqc, AIC_IPI_MASK_SET, AIC_IPI_SELF);
> + aic_ic_write(aic_irqc, AIC_IPI_MASK_CLR, AIC_IPI_OTHER);
> + } else {
> + aic_ic_write(aic_irqc, AIC_IPI_MASK_SET, AIC_IPI_SELF | AIC_IPI_OTHER);
> + }
Why is this specific to v1 and not affecting v2? I'm sure there is a
good reason, but documenting these differences would certainly help
reviewing (which version implement which registers, for example).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-12-12 18:49 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-09 4:32 [PATCH 0/6] irqchip/apple-aic: Add support for AICv2 Hector Martin
2021-12-09 4:32 ` [PATCH 1/6] dt-bindings: interrupt-controller: apple, aic: Add apple, aic2 support Hector Martin
2021-12-09 17:28 ` [PATCH 1/6] dt-bindings: interrupt-controller: apple,aic: Add apple,aic2 support Rob Herring
2021-12-11 12:28 ` Hector Martin
2021-12-11 12:44 ` [PATCH 1/6] dt-bindings: interrupt-controller: apple, aic: Add apple, aic2 support Marc Zyngier
2021-12-11 12:52 ` [PATCH 1/6] dt-bindings: interrupt-controller: apple,aic: Add apple,aic2 support Hector Martin
2021-12-11 12:49 ` Mark Kettenis
2021-12-09 4:32 ` [PATCH 2/6] irqchip/apple-aic: Add Fast IPI support Hector Martin
2021-12-12 12:21 ` Marc Zyngier
2021-12-18 5:31 ` Hector Martin
2021-12-20 12:43 ` Marc Zyngier
2021-12-09 4:32 ` [PATCH 3/6] irqchip/apple-aic: Switch to irq_domain_create_tree and sparse hwirqs Hector Martin
2021-12-12 14:37 ` Marc Zyngier
2021-12-18 5:36 ` Hector Martin
2021-12-09 4:32 ` [PATCH 4/6] irqchip/apple-aic: Dynamically compute register offsets Hector Martin
2021-12-12 18:26 ` Marc Zyngier
2021-12-18 5:37 ` Hector Martin
2021-12-09 4:32 ` [PATCH 5/6] irqchip/apple-aic: Support multiple dies Hector Martin
2021-12-13 16:10 ` Marc Zyngier
2021-12-18 5:39 ` Hector Martin
2021-12-20 13:38 ` Marc Zyngier
2021-12-09 4:32 ` [PATCH 6/6] irqchip/apple-aic: Add support for AICv2 Hector Martin
2021-12-12 18:47 ` Marc Zyngier [this message]
2021-12-18 6:02 ` Hector Martin
2021-12-20 13:52 ` 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=87o85lu0ck.wl-maz@kernel.org \
--to=maz@kernel.org \
--cc=alyssa@rosenzweig.io \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marcan@marcan.st \
--cc=robh+dt@kernel.org \
--cc=sven@svenpeter.dev \
--cc=tglx@linutronix.de \
/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).