Received: by 2002:a05:6a10:7420:0:0:0:0 with SMTP id hk32csp3567135pxb; Mon, 21 Feb 2022 00:32:36 -0800 (PST) X-Google-Smtp-Source: ABdhPJwL+8PRt34XcqfxF2zFlxsYf7P0h+MUZZnpLP34I1WdCtRjcdmwTqWiAoPLrVaZ+0ciuFkl X-Received: by 2002:a17:907:2087:b0:6bb:1525:dfb2 with SMTP id pv7-20020a170907208700b006bb1525dfb2mr15036074ejb.504.1645432356125; Mon, 21 Feb 2022 00:32:36 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1645432356; cv=none; d=google.com; s=arc-20160816; b=rbZKH8EkP/qnlRAq0WLFbf7TqmnEhxA09n6zupsM2ZBd6u+W3PaCqamXQQj3fVlRrc UCo3I9jmX8vliDDCg9YqNwjA8nqKxOcXH8QisiJnRD5JRwFDh7/UYS5WYV4BajSb+Fz4 4vWHQHYicCKmU1Dtfn0/lnCtTlhGmfj6EG5rWz+KBKL1ayA2ZI4RXin3Dto0jOeDqsIX qQyqj7oLxstMLCwjGjd4cjMXBRjKG/xIL6atpWGvRTsypP2tGNzjTfrFU/E4Rn6r0pUT Iwo10bQprsfyckLofr5Fq+QdizbajIUpQydvbEbPo/JOOnl1VRpaoPfTuKFN8k4vk1AG I4vQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:cc:to:subject:message-id:date:from:in-reply-to :references:mime-version:dkim-signature; bh=nedZE9CYo/O4rESVv1Nf5juJloXpQE/HOA07tzdjzh4=; b=Jl4ya7P2aTj5+NV53Tj1OiDnxk2+AFRcjp2+pmRyk85bQmUqGzNigQj1GBOwM/fkO/ cZokSLtTgT2rVL9AXDNmjjFZpT7fuUjiaynJhAwLx59sLu2DExgHerabSAckoRz7ReCb GGtkBfNomPkb/gnTtEUATuoDFY5aBYMPA/h6sunu4d3ipvzEhntcvGd6KIpgXwIPfSU4 aFnPhnF4jknzBDXgoQ2W+GoW0+k+422MHzQNDrA+TpNO+5k+/XfzqrssJ68ak982ZxvM nWnPhvLFPN2tFfs3iW2WkGXDX8sxQi7v1/Vxvb+1KUGTo/9lUWB1pDjM/KvOJSaCpfnL T7xg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@brainfault-org.20210112.gappssmtp.com header.s=20210112 header.b=Ybq8d9MV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id cq2si10237091edb.26.2022.02.21.00.32.12; Mon, 21 Feb 2022 00:32:36 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@brainfault-org.20210112.gappssmtp.com header.s=20210112 header.b=Ybq8d9MV; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240940AbiBSDjR (ORCPT + 99 others); Fri, 18 Feb 2022 22:39:17 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:54500 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231258AbiBSDjQ (ORCPT ); Fri, 18 Feb 2022 22:39:16 -0500 Received: from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com [IPv6:2a00:1450:4864:20::32e]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C682450B38 for ; Fri, 18 Feb 2022 19:38:57 -0800 (PST) Received: by mail-wm1-x32e.google.com with SMTP id bg21-20020a05600c3c9500b0035283e7a012so7708739wmb.0 for ; Fri, 18 Feb 2022 19:38:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=brainfault-org.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=nedZE9CYo/O4rESVv1Nf5juJloXpQE/HOA07tzdjzh4=; b=Ybq8d9MVycFAHyQOoZAXdJHdp57iiyFesmQLye9XQO+8Tuzi92/XZNcHen3n2QEn7l dKSl6bBRcP6IjnUSCQtc6n3sUmxOLtb+d+C4jXkuphJuwW51jF1QSG7fIx1t+mCnPK2d swg1A+QvUIR8lPQ3MY+HVpejsoLRJOKxy4hz6pQA7N8+io9imHN762onltdMYUyGY4Rq TxIboGOG7wUM76Lt2is70XtAD6rtXzMVcS684QF7atMz/ajYdrPohUdnFPrXiU31R/cj 58MEjctQZYr/HWa5mp9M1pab0LlWRuN14hZ9DDYMh26GVxGs4wS7ItdzyBLGmz0txrVP UVFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=nedZE9CYo/O4rESVv1Nf5juJloXpQE/HOA07tzdjzh4=; b=i6Tz5hBzFVyDwtICCMcaouhQAGpSHnUZnR7dLk1U9vmwDYYYaW6yxBED+TNdGfGm0n qf6HoqjMKmCTMX0BdtXiLuhuCFocnOcq6iddtaSq6HRyvVZunrMw5aBXRgmE9w1Vp0cc iiFWbqn85CY8+d707ocK1Dkn0jBmTFbOUY7HPDkzCkgAcw628BSKwDFeRu/rhiRV6lbX rsBb7rFzJnqt24ZRupiNcHkajn2KB28Cr7bekwG2F663Vv8B9tE6mkLc5iLDHnBp3tOt 8gXGEXc/AO9yF0oAyVkdTS6d6wnzD3vl/urlpwkRg7fpehI+wcfA7iGoh/sRTbpDd9tf Gvvg== X-Gm-Message-State: AOAM533RErH0loYy6NWy0OHscpqeFyFWJV+R7fweK6PFX1KWN3ZLa7Kt X5e/jUitEEmodmGNy++660+QEDKaY3Qx7ChYo9pvAg== X-Received: by 2002:a7b:cb99:0:b0:34e:b056:91ed with SMTP id m25-20020a7bcb99000000b0034eb05691edmr9424307wmi.137.1645241936103; Fri, 18 Feb 2022 19:38:56 -0800 (PST) MIME-Version: 1.0 References: <20220128052505.859518-1-apatel@ventanamicro.com> <20220128052505.859518-3-apatel@ventanamicro.com> <063b8a5636d6372f37029946b2c3e0f4@kernel.org> In-Reply-To: <063b8a5636d6372f37029946b2c3e0f4@kernel.org> From: Anup Patel Date: Sat, 19 Feb 2022 09:08:43 +0530 Message-ID: Subject: Re: [PATCH v2 2/6] irqchip/riscv-intc: Create domain using named fwnode To: Marc Zyngier Cc: Anup Patel , Palmer Dabbelt , Paul Walmsley , Thomas Gleixner , Daniel Lezcano , Atish Patra , Alistair Francis , linux-riscv , "linux-kernel@vger.kernel.org List" Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 17, 2022 at 8:42 PM Marc Zyngier wrote: > > On 2022-01-28 05:25, Anup Patel wrote: > > We should create INTC domain using a synthetic fwnode which will allow > > drivers (such as RISC-V SBI IPI driver, RISC-V timer driver, RISC-V > > PMU driver, etc) not having dedicated DT/ACPI node to directly create > > interrupt mapping for standard local interrupt numbers defined by the > > RISC-V privileged specification. > > > > Signed-off-by: Anup Patel > > --- > > arch/riscv/include/asm/irq.h | 2 ++ > > arch/riscv/kernel/irq.c | 13 +++++++++++++ > > drivers/clocksource/timer-clint.c | 13 +++++++------ > > drivers/clocksource/timer-riscv.c | 11 ++--------- > > drivers/irqchip/irq-riscv-intc.c | 12 ++++++++++-- > > drivers/irqchip/irq-sifive-plic.c | 19 +++++++++++-------- > > 6 files changed, 45 insertions(+), 25 deletions(-) > > > > diff --git a/arch/riscv/include/asm/irq.h > > b/arch/riscv/include/asm/irq.h > > index e4c435509983..f85ebaf07505 100644 > > --- a/arch/riscv/include/asm/irq.h > > +++ b/arch/riscv/include/asm/irq.h > > @@ -12,6 +12,8 @@ > > > > #include > > > > +extern struct fwnode_handle *riscv_intc_fwnode(void); > > + > > extern void __init init_IRQ(void); > > > > #endif /* _ASM_RISCV_IRQ_H */ > > diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c > > index 7207fa08d78f..f2fed78ab659 100644 > > --- a/arch/riscv/kernel/irq.c > > +++ b/arch/riscv/kernel/irq.c > > @@ -7,9 +7,22 @@ > > > > #include > > #include > > +#include > > +#include > > #include > > #include > > > > +static struct fwnode_handle *intc_fwnode; > > + > > +struct fwnode_handle *riscv_intc_fwnode(void) > > +{ > > + if (!intc_fwnode) > > + intc_fwnode = irq_domain_alloc_named_fwnode("RISCV-INTC"); > > + > > + return intc_fwnode; > > +} > > +EXPORT_SYMBOL_GPL(riscv_intc_fwnode); > > Why is this created outside of the root interrupt controller driver? > Furthermore, why do you need to create a new fwnode the first place? > As far as I can tell, the INTC does have a node, and what you don't > have is the firmware linkage between PMU (an others) and the INTC. Fair enough, I will update this patch to not create a synthetic fwnode. The issue is not with INTC driver. We have other drivers and places (such as SBI IPI driver, SBI PMU driver, and KVM RISC-V AIA support) where we don't have a way to locate INTC fwnode. > > what you should have instead is something like: > > static struct fwnode_handle *(*__get_root_intc_node)(void); > struct fwnode_handle *riscv_get_root_intc_hwnode(void) > { > if (__get_root_intc_node) > return __get_root_intc_node(); > > return NULL; > } > > and the corresponding registration interface. Thanks, I will follow this suggestion. This is a much better approach and it will avoid touching existing drivers. > > But either way, something breaks: the INTC has one node per CPU, and > expect one irqdomain per CPU. Having a single fwnode completely breaks > the INTC driver (and probably the irqdomain list, as we don't check for > duplicate entries). > > > diff --git a/drivers/irqchip/irq-riscv-intc.c > > b/drivers/irqchip/irq-riscv-intc.c > > index b65bd8878d4f..26ed62c11768 100644 > > --- a/drivers/irqchip/irq-riscv-intc.c > > +++ b/drivers/irqchip/irq-riscv-intc.c > > @@ -112,8 +112,16 @@ static int __init riscv_intc_init(struct > > device_node *node, > > if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) > > return 0; > > > > - intc_domain = irq_domain_add_linear(node, BITS_PER_LONG, > > - &riscv_intc_domain_ops, NULL); > > + /* > > + * Create INTC domain using a synthetic fwnode which will allow > > + * drivers (such as RISC-V SBI IPI driver, RISC-V timer driver, > > + * RISC-V PMU driver, etc) not having dedicated DT/ACPI node to > > + * directly create interrupt mapping for standard local interrupt > > + * numbers defined by the RISC-V privileged specification. > > + */ > > + intc_domain = irq_domain_create_linear(riscv_intc_fwnode(), > > + BITS_PER_LONG, > > + &riscv_intc_domain_ops, NULL); > > This is what I'm talking about. It is simply broken. So either you don't > need a per-CPU node (and the DT was bad the first place), or you > absolutely need > one (and the whole 'well-known/default domain' doesn't work at all). > > Either way, this patch is plain wrong. Okay, I will update this patch with the new approach which you suggested. Regards, Anup > > > M. > -- > Jazz is not dead. It just smells funny...