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 mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 97347C433F5 for ; Thu, 30 Sep 2021 13:06:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7EE4A615E0 for ; Thu, 30 Sep 2021 13:06:21 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1350144AbhI3NH6 (ORCPT ); Thu, 30 Sep 2021 09:07:58 -0400 Received: from mail.kernel.org ([198.145.29.99]:60528 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1351286AbhI3NHq (ORCPT ); Thu, 30 Sep 2021 09:07:46 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id A6F6761A03; Thu, 30 Sep 2021 13:06:03 +0000 (UTC) Received: from sofa.misterjones.org ([185.219.108.64] helo=why.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 1mVvlB-00DzHF-L3; Thu, 30 Sep 2021 14:06:01 +0100 Date: Thu, 30 Sep 2021 14:06:01 +0100 Message-ID: <874ka2tdqe.wl-maz@kernel.org> From: Marc Zyngier To: Linus Walleij Cc: Daniel Palmer , DTML , Rob Herring , Thomas Gleixner , linux-arm-kernel , Romain Perier Subject: Re: [PATCH 2/3] irqchip: SigmaStar SSD20xD gpi In-Reply-To: References: <20210914100415.1549208-1-daniel@0x0f.com> <20210914100415.1549208-3-daniel@0x0f.com> <87zgs7vavl.wl-maz@kernel.org> <87wnnbv6ac.wl-maz@kernel.org> <87lf3quydn.wl-maz@kernel.org> 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: linus.walleij@linaro.org, daniel@0x0f.com, devicetree@vger.kernel.org, robh+dt@kernel.org, tglx@linutronix.de, linux-arm-kernel@lists.infradead.org, romain.perier@gmail.com 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: devicetree@vger.kernel.org [huh, missed this email...] On Tue, 21 Sep 2021 19:23:04 +0100, Linus Walleij wrote: > > On Tue, Sep 21, 2021 at 10:27 AM Marc Zyngier wrote: > > > Linus: is there a reason why the gpiolib insist on setting its own > > handler while building the hierarchy? > > Is it this? > > /* > * We set handle_bad_irq because the .set_type() should > * always be invoked and set the right type of handler. > */ > irq_domain_set_info(d, > irq, > hwirq, > gc->irq.chip, > gc, > girq->handler, > NULL, NULL); > irq_set_probe(irq); > (...) It is its relative position wrt to irq_domain_alloc_irqs_parent() that has the potential for annoyance. irq_domain_set_info() will trigger an irq startup, which will explode if the parent level hasn't been initialised correctly. > > IIUC it's because sometimes, on elder systems (such as ixp4xx) some machines > are still using boardfiles, and drivers are not obtaining IRQs dynamically > from device tree or ACPI, instead they are set up statically at machine > init. > > I assume it would otherwise be done as part of ops->translate? No, this is the right spot if you really need to set the handler. But it should really be after the parent allocation (see below for something totally untested). Ultimately, setting the flow handler when there is a parent domain is a bit odd, as you'd expect the root domain to be in charge of the overall flow. Thanks, M. diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c index abfbf546d159..53221d54c4be 100644 --- a/drivers/gpio/gpiolib.c +++ b/drivers/gpio/gpiolib.c @@ -1103,19 +1103,6 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d, } chip_dbg(gc, "found parent hwirq %u\n", parent_hwirq); - /* - * We set handle_bad_irq because the .set_type() should - * always be invoked and set the right type of handler. - */ - irq_domain_set_info(d, - irq, - hwirq, - gc->irq.chip, - gc, - girq->handler, - NULL, NULL); - irq_set_probe(irq); - /* This parent only handles asserted level IRQs */ parent_arg = girq->populate_parent_alloc_arg(gc, parent_hwirq, parent_type); if (!parent_arg) @@ -1137,6 +1124,18 @@ static int gpiochip_hierarchy_irq_domain_alloc(struct irq_domain *d, parent_hwirq, hwirq); kfree(parent_arg); + + if (!ret) { + irq_domain_set_info(d, + irq, + hwirq, + gc->irq.chip, + gc, + girq->handler, + NULL, NULL); + irq_set_probe(irq); + } + return ret; } -- Without deviation from the norm, progress is not possible.