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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04CEFC433FE for ; Sun, 12 Dec 2021 14:37:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231442AbhLLOhd (ORCPT ); Sun, 12 Dec 2021 09:37:33 -0500 Received: from ams.source.kernel.org ([145.40.68.75]:33676 "EHLO ams.source.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230161AbhLLOhc (ORCPT ); Sun, 12 Dec 2021 09:37:32 -0500 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 A2E07B80D11; Sun, 12 Dec 2021 14:37:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 32479C341C6; Sun, 12 Dec 2021 14:37:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1639319849; bh=+IeHqoX4dUsugp5slAC1d/RVfMq9l+2uaUrzisdEH18=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=R8ByD3Nt7GwtIUJtrJIJ6Na5E1RH7WQZDGU7ApEk+UnWc0if9zQXjsyUhjq6+VNyv 6eTApg7BJex8qW5j9PNn4dBWtFXf9upQBtMe6yH7+yIqlk77U7Jq2Nc4DMBRHN5YMN 8qCc+OLyoncA8rst7fG+3/eB/p2Qn8sdUDiNSrdUCtrNGjuEl3pUgu/OIX+B75pumQ 7r7b501M4uDLZNEIQg2r8HlDPTt9KVajy3JNhHhul9YxMWN6n1N+5PLuO48QUeEr82 FXrXqsuiEuCqN0oNcgw5wNx906zqPOoexVl34i3DBGdbkvcZJn3tluKdkkxJyHIyX1 UviWd/DDUqfvQ== 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 1mwPyh-00BbZ8-5i; Sun, 12 Dec 2021 14:37:27 +0000 Date: Sun, 12 Dec 2021 14:37:26 +0000 Message-ID: <87r1ahubx5.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 3/6] irqchip/apple-aic: Switch to irq_domain_create_tree and sparse hwirqs In-Reply-To: <20211209043249.65474-4-marcan@marcan.st> References: <20211209043249.65474-1-marcan@marcan.st> <20211209043249.65474-4-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") Content-Type: text/plain; charset=US-ASCII 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 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 09 Dec 2021 04:32:46 +0000, Hector Martin wrote: > > This allows us to directly use the hardware event number as the hwirq > number. Since IRQ events have bit 16 set (type=1), FIQs now move to > starting at hwirq number 0. > > This will become more important once multi-die support is introduced in > a later commit. > > Signed-off-by: Hector Martin > --- > drivers/irqchip/irq-apple-aic.c | 67 ++++++++++++++++++--------------- > 1 file changed, 36 insertions(+), 31 deletions(-) > > diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c > index 1aa63580cae4..572d1af175fc 100644 > --- a/drivers/irqchip/irq-apple-aic.c > +++ b/drivers/irqchip/irq-apple-aic.c > @@ -66,7 +66,7 @@ > */ > > #define AIC_INFO 0x0004 > -#define AIC_INFO_NR_HW GENMASK(15, 0) > +#define AIC_INFO_NR_IRQ GENMASK(15, 0) > > #define AIC_CONFIG 0x0010 > > @@ -75,6 +75,7 @@ > #define AIC_EVENT_TYPE GENMASK(31, 16) > #define AIC_EVENT_NUM GENMASK(15, 0) > > +#define AIC_EVENT_TYPE_FIQ 0 /* Software use */ What does 'SW use' mean? Are you using the fact that the event register never returns 0 in the top bits? > #define AIC_EVENT_TYPE_HW 1 > #define AIC_EVENT_TYPE_IPI 4 > #define AIC_EVENT_IPI_OTHER 1 > @@ -158,6 +159,8 @@ > #define MPIDR_CPU GENMASK(7, 0) > #define MPIDR_CLUSTER GENMASK(15, 8) > > +#define AIC_FIQ_HWIRQ(x) (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_FIQ) | \ > + FIELD_PREP(AIC_EVENT_NUM, x)) > #define AIC_NR_FIQ 4 > #define AIC_NR_SWIPI 32 > > @@ -209,7 +212,7 @@ struct aic_irq_chip { > void __iomem *base; > struct irq_domain *hw_domain; > struct irq_domain *ipi_domain; > - int nr_hw; > + int nr_irq; > > struct aic_info info; > }; > @@ -239,18 +242,22 @@ static void aic_ic_write(struct aic_irq_chip *ic, u32 reg, u32 val) > > static void aic_irq_mask(struct irq_data *d) > { > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > > - aic_ic_write(ic, AIC_MASK_SET + MASK_REG(irqd_to_hwirq(d)), > - MASK_BIT(irqd_to_hwirq(d))); > + u32 irq = FIELD_GET(AIC_EVENT_NUM, hwirq); This expression is used quite a few times, and could use a helper clarifying its purpose (converting the event/hwirq to an index?). 'irq' is a bit of a misnomer too, but I struggle to find another name... > + > + aic_ic_write(ic, AIC_MASK_SET + MASK_REG(irq), MASK_BIT(irq)); > } > > static void aic_irq_unmask(struct irq_data *d) > { > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > > - aic_ic_write(ic, AIC_MASK_CLR + MASK_REG(d->hwirq), > - MASK_BIT(irqd_to_hwirq(d))); > + u32 irq = FIELD_GET(AIC_EVENT_NUM, hwirq); > + > + aic_ic_write(ic, AIC_MASK_CLR + MASK_REG(irq), MASK_BIT(irq)); > } > > static void aic_irq_eoi(struct irq_data *d) > @@ -278,7 +285,7 @@ static void __exception_irq_entry aic_handle_irq(struct pt_regs *regs) > irq = FIELD_GET(AIC_EVENT_NUM, event); > > if (type == AIC_EVENT_TYPE_HW) > - generic_handle_domain_irq(aic_irqc->hw_domain, irq); > + generic_handle_domain_irq(aic_irqc->hw_domain, event); > else if (type == AIC_EVENT_TYPE_IPI && irq == 1) > aic_handle_ipi(regs); > else if (event != 0) > @@ -310,7 +317,7 @@ static int aic_irq_set_affinity(struct irq_data *d, > else > cpu = cpumask_any_and(mask_val, cpu_online_mask); > > - aic_ic_write(ic, AIC_TARGET_CPU + hwirq * 4, BIT(cpu)); > + aic_ic_write(ic, AIC_TARGET_CPU + FIELD_GET(AIC_EVENT_NUM, hwirq) * 4, BIT(cpu)); > irq_data_update_effective_affinity(d, cpumask_of(cpu)); > > return IRQ_SET_MASK_OK; > @@ -340,9 +347,7 @@ static struct irq_chip aic_chip = { > > static unsigned long aic_fiq_get_idx(struct irq_data *d) > { > - struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > - > - return irqd_to_hwirq(d) - ic->nr_hw; > + return FIELD_GET(AIC_EVENT_NUM, irqd_to_hwirq(d)); > } > > static void aic_fiq_set_mask(struct irq_data *d) > @@ -430,11 +435,11 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs) > > if (TIMER_FIRING(read_sysreg(cntp_ctl_el0))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL0_PHYS); > + AIC_FIQ_HWIRQ(AIC_TMR_EL0_PHYS)); > > if (TIMER_FIRING(read_sysreg(cntv_ctl_el0))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL0_VIRT); > + AIC_FIQ_HWIRQ(AIC_TMR_EL0_VIRT)); > > if (is_kernel_in_hyp_mode()) { > uint64_t enabled = read_sysreg_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2); > @@ -442,12 +447,12 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs) > if ((enabled & VM_TMR_FIQ_ENABLE_P) && > TIMER_FIRING(read_sysreg_s(SYS_CNTP_CTL_EL02))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL02_PHYS); > + AIC_FIQ_HWIRQ(AIC_TMR_EL02_PHYS)); > > if ((enabled & VM_TMR_FIQ_ENABLE_V) && > TIMER_FIRING(read_sysreg_s(SYS_CNTV_CTL_EL02))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL02_VIRT); > + AIC_FIQ_HWIRQ(AIC_TMR_EL02_VIRT)); > } > > if ((read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) & (PMCR0_IMODE | PMCR0_IACT)) == > @@ -492,13 +497,13 @@ 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); > > - if (hw < ic->nr_hw) { > + if (type == AIC_EVENT_TYPE_HW) { > irq_domain_set_info(id, irq, hw, &aic_chip, id->host_data, > handle_fasteoi_irq, NULL, NULL); > irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(irq))); > - } else { > + } else if (type == AIC_EVENT_TYPE_FIQ) { Do we need to check for FIQ? This should be the case by construction, right? If there is a risk that it isn't the case, then we probably need a default case (and the whole thing would be better written as a switch() statement). > irq_set_percpu_devid(irq); > irq_domain_set_info(id, irq, hw, &fiq_chip, id->host_data, > handle_percpu_devid_irq, NULL, NULL); > @@ -519,14 +524,15 @@ static int aic_irq_domain_translate(struct irq_domain *id, > > switch (fwspec->param[0]) { > case AIC_IRQ: > - if (fwspec->param[1] >= ic->nr_hw) > + if (fwspec->param[1] >= ic->nr_irq) > return -EINVAL; > - *hwirq = fwspec->param[1]; > + *hwirq = (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) | > + FIELD_PREP(AIC_EVENT_NUM, fwspec->param[1])); > break; > case AIC_FIQ: > if (fwspec->param[1] >= AIC_NR_FIQ) > return -EINVAL; > - *hwirq = ic->nr_hw + fwspec->param[1]; > + *hwirq = AIC_FIQ_HWIRQ(fwspec->param[1]); > > /* > * In EL1 the non-redirected registers are the guest's, > @@ -535,10 +541,10 @@ static int aic_irq_domain_translate(struct irq_domain *id, > if (!is_kernel_in_hyp_mode()) { > switch (fwspec->param[1]) { > case AIC_TMR_GUEST_PHYS: > - *hwirq = ic->nr_hw + AIC_TMR_EL0_PHYS; > + *hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_PHYS); > break; > case AIC_TMR_GUEST_VIRT: > - *hwirq = ic->nr_hw + AIC_TMR_EL0_VIRT; > + *hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_VIRT); > break; > case AIC_TMR_HV_PHYS: > case AIC_TMR_HV_VIRT: > @@ -894,11 +900,10 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p > aic_irqc = irqc; > > info = aic_ic_read(irqc, AIC_INFO); > - irqc->nr_hw = FIELD_GET(AIC_INFO_NR_HW, info); > + irqc->nr_irq = FIELD_GET(AIC_INFO_NR_IRQ, info); > > - irqc->hw_domain = irq_domain_create_linear(of_node_to_fwnode(node), > - irqc->nr_hw + AIC_NR_FIQ, > - &aic_irq_domain_ops, irqc); > + irqc->hw_domain = irq_domain_create_tree(of_node_to_fwnode(node), > + &aic_irq_domain_ops, irqc); > if (WARN_ON(!irqc->hw_domain)) { > iounmap(irqc->base); > kfree(irqc); > @@ -917,11 +922,11 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p > set_handle_irq(aic_handle_irq); > set_handle_fiq(aic_handle_fiq); > > - for (i = 0; i < BITS_TO_U32(irqc->nr_hw); i++) > + for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++) > aic_ic_write(irqc, AIC_MASK_SET + i * 4, U32_MAX); > - for (i = 0; i < BITS_TO_U32(irqc->nr_hw); i++) > + for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++) > aic_ic_write(irqc, AIC_SW_CLR + i * 4, U32_MAX); > - for (i = 0; i < irqc->nr_hw; i++) > + for (i = 0; i < irqc->nr_irq; i++) > aic_ic_write(irqc, AIC_TARGET_CPU + i * 4, 1); > > if (!is_kernel_in_hyp_mode()) > @@ -937,7 +942,7 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p > vgic_set_kvm_info(&vgic_info); > > pr_info("Initialized with %d IRQs, %d FIQs, %d vIPIs\n", > - irqc->nr_hw, AIC_NR_FIQ, AIC_NR_SWIPI); > + irqc->nr_irq, AIC_NR_FIQ, AIC_NR_SWIPI); > > return 0; > } Thanks, M. -- Without deviation from the norm, progress is not possible. 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 E4966C433F5 for ; Sun, 12 Dec 2021 14:39:12 +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=9XTj9pnnHyewWD22Ffagm0fglG4oWh3gH3jBBHh4rRg=; b=OJiueG3sAnk+zr 81Za+lwNSm34gQqqKaP/bG89FuzqQ3uiaMHe1Lh9R6CV2+rUlivUUwNerpAtS/WXRgUoZzbCOxmfP C4HqGzuttm6o9Is1Ya9utIh5nvUqX68WnQl8AQNkvN/+G+XytnoF5iNBzFR83J9sKCASs7PGqr4GQ KqiPhHykc8jUDMSEr5qYPM+i6UXOFPwDZv0gGBw7CiWpnwyrzXMq9S2OkMd0MebCfOCCR4m7I4nvf K6UjmnCD0knXsjbZmz0MPsOfzRY+udFhLgwa1ktMKsINte7sdQ3wVJg2nziIk3wr7sVuvMK+7fIYT Jz42xJisAvsf0KHtsm/A==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mwPyq-006fC7-WB; Sun, 12 Dec 2021 14:37:37 +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 1mwPym-006fBd-GS for linux-arm-kernel@lists.infradead.org; Sun, 12 Dec 2021 14:37:34 +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 8A7CEB80CF1; Sun, 12 Dec 2021 14:37:30 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 32479C341C6; Sun, 12 Dec 2021 14:37:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1639319849; bh=+IeHqoX4dUsugp5slAC1d/RVfMq9l+2uaUrzisdEH18=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=R8ByD3Nt7GwtIUJtrJIJ6Na5E1RH7WQZDGU7ApEk+UnWc0if9zQXjsyUhjq6+VNyv 6eTApg7BJex8qW5j9PNn4dBWtFXf9upQBtMe6yH7+yIqlk77U7Jq2Nc4DMBRHN5YMN 8qCc+OLyoncA8rst7fG+3/eB/p2Qn8sdUDiNSrdUCtrNGjuEl3pUgu/OIX+B75pumQ 7r7b501M4uDLZNEIQg2r8HlDPTt9KVajy3JNhHhul9YxMWN6n1N+5PLuO48QUeEr82 FXrXqsuiEuCqN0oNcgw5wNx906zqPOoexVl34i3DBGdbkvcZJn3tluKdkkxJyHIyX1 UviWd/DDUqfvQ== 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 1mwPyh-00BbZ8-5i; Sun, 12 Dec 2021 14:37:27 +0000 Date: Sun, 12 Dec 2021 14:37:26 +0000 Message-ID: <87r1ahubx5.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 3/6] irqchip/apple-aic: Switch to irq_domain_create_tree and sparse hwirqs In-Reply-To: <20211209043249.65474-4-marcan@marcan.st> References: <20211209043249.65474-1-marcan@marcan.st> <20211209043249.65474-4-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_063732_856600_98468823 X-CRM114-Status: GOOD ( 39.39 ) 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:46 +0000, Hector Martin wrote: > > This allows us to directly use the hardware event number as the hwirq > number. Since IRQ events have bit 16 set (type=1), FIQs now move to > starting at hwirq number 0. > > This will become more important once multi-die support is introduced in > a later commit. > > Signed-off-by: Hector Martin > --- > drivers/irqchip/irq-apple-aic.c | 67 ++++++++++++++++++--------------- > 1 file changed, 36 insertions(+), 31 deletions(-) > > diff --git a/drivers/irqchip/irq-apple-aic.c b/drivers/irqchip/irq-apple-aic.c > index 1aa63580cae4..572d1af175fc 100644 > --- a/drivers/irqchip/irq-apple-aic.c > +++ b/drivers/irqchip/irq-apple-aic.c > @@ -66,7 +66,7 @@ > */ > > #define AIC_INFO 0x0004 > -#define AIC_INFO_NR_HW GENMASK(15, 0) > +#define AIC_INFO_NR_IRQ GENMASK(15, 0) > > #define AIC_CONFIG 0x0010 > > @@ -75,6 +75,7 @@ > #define AIC_EVENT_TYPE GENMASK(31, 16) > #define AIC_EVENT_NUM GENMASK(15, 0) > > +#define AIC_EVENT_TYPE_FIQ 0 /* Software use */ What does 'SW use' mean? Are you using the fact that the event register never returns 0 in the top bits? > #define AIC_EVENT_TYPE_HW 1 > #define AIC_EVENT_TYPE_IPI 4 > #define AIC_EVENT_IPI_OTHER 1 > @@ -158,6 +159,8 @@ > #define MPIDR_CPU GENMASK(7, 0) > #define MPIDR_CLUSTER GENMASK(15, 8) > > +#define AIC_FIQ_HWIRQ(x) (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_FIQ) | \ > + FIELD_PREP(AIC_EVENT_NUM, x)) > #define AIC_NR_FIQ 4 > #define AIC_NR_SWIPI 32 > > @@ -209,7 +212,7 @@ struct aic_irq_chip { > void __iomem *base; > struct irq_domain *hw_domain; > struct irq_domain *ipi_domain; > - int nr_hw; > + int nr_irq; > > struct aic_info info; > }; > @@ -239,18 +242,22 @@ static void aic_ic_write(struct aic_irq_chip *ic, u32 reg, u32 val) > > static void aic_irq_mask(struct irq_data *d) > { > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > > - aic_ic_write(ic, AIC_MASK_SET + MASK_REG(irqd_to_hwirq(d)), > - MASK_BIT(irqd_to_hwirq(d))); > + u32 irq = FIELD_GET(AIC_EVENT_NUM, hwirq); This expression is used quite a few times, and could use a helper clarifying its purpose (converting the event/hwirq to an index?). 'irq' is a bit of a misnomer too, but I struggle to find another name... > + > + aic_ic_write(ic, AIC_MASK_SET + MASK_REG(irq), MASK_BIT(irq)); > } > > static void aic_irq_unmask(struct irq_data *d) > { > + irq_hw_number_t hwirq = irqd_to_hwirq(d); > struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > > - aic_ic_write(ic, AIC_MASK_CLR + MASK_REG(d->hwirq), > - MASK_BIT(irqd_to_hwirq(d))); > + u32 irq = FIELD_GET(AIC_EVENT_NUM, hwirq); > + > + aic_ic_write(ic, AIC_MASK_CLR + MASK_REG(irq), MASK_BIT(irq)); > } > > static void aic_irq_eoi(struct irq_data *d) > @@ -278,7 +285,7 @@ static void __exception_irq_entry aic_handle_irq(struct pt_regs *regs) > irq = FIELD_GET(AIC_EVENT_NUM, event); > > if (type == AIC_EVENT_TYPE_HW) > - generic_handle_domain_irq(aic_irqc->hw_domain, irq); > + generic_handle_domain_irq(aic_irqc->hw_domain, event); > else if (type == AIC_EVENT_TYPE_IPI && irq == 1) > aic_handle_ipi(regs); > else if (event != 0) > @@ -310,7 +317,7 @@ static int aic_irq_set_affinity(struct irq_data *d, > else > cpu = cpumask_any_and(mask_val, cpu_online_mask); > > - aic_ic_write(ic, AIC_TARGET_CPU + hwirq * 4, BIT(cpu)); > + aic_ic_write(ic, AIC_TARGET_CPU + FIELD_GET(AIC_EVENT_NUM, hwirq) * 4, BIT(cpu)); > irq_data_update_effective_affinity(d, cpumask_of(cpu)); > > return IRQ_SET_MASK_OK; > @@ -340,9 +347,7 @@ static struct irq_chip aic_chip = { > > static unsigned long aic_fiq_get_idx(struct irq_data *d) > { > - struct aic_irq_chip *ic = irq_data_get_irq_chip_data(d); > - > - return irqd_to_hwirq(d) - ic->nr_hw; > + return FIELD_GET(AIC_EVENT_NUM, irqd_to_hwirq(d)); > } > > static void aic_fiq_set_mask(struct irq_data *d) > @@ -430,11 +435,11 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs) > > if (TIMER_FIRING(read_sysreg(cntp_ctl_el0))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL0_PHYS); > + AIC_FIQ_HWIRQ(AIC_TMR_EL0_PHYS)); > > if (TIMER_FIRING(read_sysreg(cntv_ctl_el0))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL0_VIRT); > + AIC_FIQ_HWIRQ(AIC_TMR_EL0_VIRT)); > > if (is_kernel_in_hyp_mode()) { > uint64_t enabled = read_sysreg_s(SYS_IMP_APL_VM_TMR_FIQ_ENA_EL2); > @@ -442,12 +447,12 @@ static void __exception_irq_entry aic_handle_fiq(struct pt_regs *regs) > if ((enabled & VM_TMR_FIQ_ENABLE_P) && > TIMER_FIRING(read_sysreg_s(SYS_CNTP_CTL_EL02))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL02_PHYS); > + AIC_FIQ_HWIRQ(AIC_TMR_EL02_PHYS)); > > if ((enabled & VM_TMR_FIQ_ENABLE_V) && > TIMER_FIRING(read_sysreg_s(SYS_CNTV_CTL_EL02))) > generic_handle_domain_irq(aic_irqc->hw_domain, > - aic_irqc->nr_hw + AIC_TMR_EL02_VIRT); > + AIC_FIQ_HWIRQ(AIC_TMR_EL02_VIRT)); > } > > if ((read_sysreg_s(SYS_IMP_APL_PMCR0_EL1) & (PMCR0_IMODE | PMCR0_IACT)) == > @@ -492,13 +497,13 @@ 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); > > - if (hw < ic->nr_hw) { > + if (type == AIC_EVENT_TYPE_HW) { > irq_domain_set_info(id, irq, hw, &aic_chip, id->host_data, > handle_fasteoi_irq, NULL, NULL); > irqd_set_single_target(irq_desc_get_irq_data(irq_to_desc(irq))); > - } else { > + } else if (type == AIC_EVENT_TYPE_FIQ) { Do we need to check for FIQ? This should be the case by construction, right? If there is a risk that it isn't the case, then we probably need a default case (and the whole thing would be better written as a switch() statement). > irq_set_percpu_devid(irq); > irq_domain_set_info(id, irq, hw, &fiq_chip, id->host_data, > handle_percpu_devid_irq, NULL, NULL); > @@ -519,14 +524,15 @@ static int aic_irq_domain_translate(struct irq_domain *id, > > switch (fwspec->param[0]) { > case AIC_IRQ: > - if (fwspec->param[1] >= ic->nr_hw) > + if (fwspec->param[1] >= ic->nr_irq) > return -EINVAL; > - *hwirq = fwspec->param[1]; > + *hwirq = (FIELD_PREP(AIC_EVENT_TYPE, AIC_EVENT_TYPE_HW) | > + FIELD_PREP(AIC_EVENT_NUM, fwspec->param[1])); > break; > case AIC_FIQ: > if (fwspec->param[1] >= AIC_NR_FIQ) > return -EINVAL; > - *hwirq = ic->nr_hw + fwspec->param[1]; > + *hwirq = AIC_FIQ_HWIRQ(fwspec->param[1]); > > /* > * In EL1 the non-redirected registers are the guest's, > @@ -535,10 +541,10 @@ static int aic_irq_domain_translate(struct irq_domain *id, > if (!is_kernel_in_hyp_mode()) { > switch (fwspec->param[1]) { > case AIC_TMR_GUEST_PHYS: > - *hwirq = ic->nr_hw + AIC_TMR_EL0_PHYS; > + *hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_PHYS); > break; > case AIC_TMR_GUEST_VIRT: > - *hwirq = ic->nr_hw + AIC_TMR_EL0_VIRT; > + *hwirq = AIC_FIQ_HWIRQ(AIC_TMR_EL0_VIRT); > break; > case AIC_TMR_HV_PHYS: > case AIC_TMR_HV_VIRT: > @@ -894,11 +900,10 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p > aic_irqc = irqc; > > info = aic_ic_read(irqc, AIC_INFO); > - irqc->nr_hw = FIELD_GET(AIC_INFO_NR_HW, info); > + irqc->nr_irq = FIELD_GET(AIC_INFO_NR_IRQ, info); > > - irqc->hw_domain = irq_domain_create_linear(of_node_to_fwnode(node), > - irqc->nr_hw + AIC_NR_FIQ, > - &aic_irq_domain_ops, irqc); > + irqc->hw_domain = irq_domain_create_tree(of_node_to_fwnode(node), > + &aic_irq_domain_ops, irqc); > if (WARN_ON(!irqc->hw_domain)) { > iounmap(irqc->base); > kfree(irqc); > @@ -917,11 +922,11 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p > set_handle_irq(aic_handle_irq); > set_handle_fiq(aic_handle_fiq); > > - for (i = 0; i < BITS_TO_U32(irqc->nr_hw); i++) > + for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++) > aic_ic_write(irqc, AIC_MASK_SET + i * 4, U32_MAX); > - for (i = 0; i < BITS_TO_U32(irqc->nr_hw); i++) > + for (i = 0; i < BITS_TO_U32(irqc->nr_irq); i++) > aic_ic_write(irqc, AIC_SW_CLR + i * 4, U32_MAX); > - for (i = 0; i < irqc->nr_hw; i++) > + for (i = 0; i < irqc->nr_irq; i++) > aic_ic_write(irqc, AIC_TARGET_CPU + i * 4, 1); > > if (!is_kernel_in_hyp_mode()) > @@ -937,7 +942,7 @@ static int __init aic_of_ic_init(struct device_node *node, struct device_node *p > vgic_set_kvm_info(&vgic_info); > > pr_info("Initialized with %d IRQs, %d FIQs, %d vIPIs\n", > - irqc->nr_hw, AIC_NR_FIQ, AIC_NR_SWIPI); > + irqc->nr_irq, AIC_NR_FIQ, AIC_NR_SWIPI); > > return 0; > } 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