From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BB777C433F5 for ; Sun, 12 Dec 2021 18:49:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Subject:Cc:To:From:Message-ID:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=8oSdFxD2yyGuA/u5155ZFbGXhmVeGizK45arR+V7PLA=; b=HgvJ+MgmviZJay s/GPPjgIpid8Kya0TD72iMzyWIP8Ic6NMh/aSypqQJX71aRa9E0O0hkRj6z7TG9toY04JRSwCTDCj Kj115fs0shRBh19xu48lVN5JdY5/U0Ngl/M9K60fyS6+enZknKH9YdX07zc89ZxmOpTni2uWzQf6g sZO7sjkR9h3FUI9bIz4fnUAXeGGp+lWXaYL6dxWP3x8h8f4gXtQZxK3NFZxzdJw4e9tEJvV+R5O4D NfvqXaHhnXs0QMnjvaTKvrpEmBpCmFfVeyd2q7GGIMq6QlE8PpO0+P9tRHvMTLxPwhZAA7N1RhMFc DU/GZ/+NEGezDc/XsMfw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mwTsj-006ziY-9j; Sun, 12 Dec 2021 18:47:33 +0000 Received: from ams.source.kernel.org ([145.40.68.75]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mwTse-006zhx-UX for linux-arm-kernel@lists.infradead.org; Sun, 12 Dec 2021 18:47:30 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 689C1B80D53; Sun, 12 Dec 2021 18:47:27 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 1DE88C341C6; Sun, 12 Dec 2021 18:47:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1639334846; bh=O/JyBKDviYTgGsgLittlUm6OxD4QDh85881BgFqsTKU=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=WX/y1lYaHaFoupdA2l3qY/o62B9ghJ1IZjsd980RDX7/rerIrgKUr/gKFGDRUsXV2 cxA+NocdVBq76GzjVjt4tY7lS8nvBP4xsy7WO/n3QdmNuvhDgte4BLRGeRP71hpHRk rX7OqafhqSxHHSqSlc+G6bH+1TYZreTPU4zPmpOnkwEKwDgYKh009U3e5EKVl5LlMH N5UrNhGOpnz3xjKx17qpeElHC35qo8Rrf4yVyisfL5oTDyYsa/wZBYaP91+NXrgS+v EQZ9Ms3Q2q/c5vhwQEzwxgXKA3dfwMg4oZk7c1uR5W2S94lavOEsxAROT5oWH7un0x bCNo2+TEWQ2CA== Received: from sofa.misterjones.org ([185.219.108.64] helo=wait-a-minute.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1mwTsa-00Bd2s-4Z; Sun, 12 Dec 2021 18:47:24 +0000 Date: Sun, 12 Dec 2021 18:47:23 +0000 Message-ID: <87o85lu0ck.wl-maz@kernel.org> From: Marc Zyngier To: Hector Martin Cc: Thomas Gleixner , Rob Herring , Sven Peter , Alyssa Rosenzweig , 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 In-Reply-To: <20211209043249.65474-7-marcan@marcan.st> References: <20211209043249.65474-1-marcan@marcan.st> <20211209043249.65474-7-marcan@marcan.st> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: marcan@marcan.st, tglx@linutronix.de, robh+dt@kernel.org, sven@svenpeter.dev, alyssa@rosenzweig.io, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, devicetree@vger.kernel.org X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20211212_104729_293137_FBE16C1D X-CRM114-Status: GOOD ( 39.99 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, 09 Dec 2021 04:32:49 +0000, Hector Martin 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 > --- > 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