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 F1AD7C433F5 for ; Tue, 25 Jan 2022 10:14:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238826AbiAYKNd (ORCPT ); Tue, 25 Jan 2022 05:13:33 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:32956 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242088AbiAYKIF (ORCPT ); Tue, 25 Jan 2022 05:08:05 -0500 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DB6A3C061749 for ; Tue, 25 Jan 2022 02:08:03 -0800 (PST) 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 dfw.source.kernel.org (Postfix) with ESMTPS id 3B639615F0 for ; Tue, 25 Jan 2022 10:08:03 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A19DAC340E0; Tue, 25 Jan 2022 10:08:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1643105282; bh=55mtZOKFeyp+SDLZZ67pEsuV8ej24DyQkF/qOQmEonE=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=q+oPq44ojvLTeTCZQhjgGRkBAsTiBSRWSXr15J0VXdiilnPjgT1ipa37higB19i12 EueAVyzzGaHw3L9V8XT+9ff1c8A2Q5eL3mDXNqk87FoGXEaugxXfTS13Zvs/eShvim C+4mc+0jEAO8Nssx5f0CiPpV8mVeJ5BC6CjQwx07ydkQAYGGas46gd+CmRL4ZrQYTF NWuyecQz6E8PBMrHI9SIibHuRAkrZrXL3n6Rj6ExTeHD5xKoOudYPrG8XCmrUY8HCA nLk6+NvwZVa1MNCp8rlWU5Rqlatc3r8cgXa0d59b2DH9m1pEDSO/wTlRmxeHWoe9PW N6W5P0L08xW0w== 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 1nCIk4-002ogN-Kz; Tue, 25 Jan 2022 10:08:00 +0000 Date: Tue, 25 Jan 2022 10:08:00 +0000 Message-ID: <871r0w86wv.wl-maz@kernel.org> From: Marc Zyngier To: Nikita Yushchenko Cc: Thomas Gleixner , Geert Uytterhoeven , Eugeniu Rosca , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] drivers: irqchip: add irq-type-changer In-Reply-To: References: <20220124095652.522099-1-nikita.yoush@cogentembedded.com> <878rv58ghy.wl-maz@kernel.org> <874k5s8a32.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: nikita.yoush@cogentembedded.com, tglx@linutronix.de, geert@linux-m68k.org, erosca@de.adit-jv.com, linux-kernel@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 Tue, 25 Jan 2022 09:35:22 +0000, Nikita Yushchenko wrote: > > >>> I also don't see why you model this as the actual device that triggers > >>> the interrupt. > >> > >> Well, that somehow matches the physical reality. In the case of wl18xx > >> on KF, physically the interrupt signal indeed originates from the > >> intermediate chip - the inverting level-shifter. > > > > Reality? By allowing something like an edge-to-level conversion? How > > can that work? > > Edge to level can be problematic, but level to edge does not cause any > difficulties, nor in generating nor in handling. Hmmm. A whole industry seems to disagree with you. > > >> In contrary, an abstraction of intermediate entity that does a static > >> conversion of the trigger type and does not need any software control, > >> looks sane. And, hardware designers do strange things sometimes, I > >> won't be surprised observing a change from level to edge one day. > > > > If you think that it can happen without a HW register that latches the > > edge and requires an ack, you need to question your understanding of > > an interrupt life cycle, and of the properties of the various trigger > > types. > > There are plenty of devices capable to signal both level and edge > interrupts, configurable by a register. Basic handling is always the > same, and involves masking the interrupt at interrupt controller while > IRQs disabled, then enabling IRQs, then programming the device to > clear the interrupt condition, then unmasking the interrupt at > interrupt controller. If the trigger type is level or edge, is only > interesting to interrupt controller driver, but not to a wider scope. > > Nothing stops hw designers from doing all sort of strange things with > interrupt signals. Right now I have a board on my desk where interrupt > signals from several chips are wired to inputs of a logical AND > element and the output of that is wired to SoC's gpio. I don't see > what stops them tomorrow from setting up their CPLDs to issue a short > impulse at output in return to a level change on input. And that will > be a level to edge converter. And a broken one, as you'd have nothing to regenerate an edge if the input stays high. Such HW can stay in the bin. > > >> In driver's init, I only call of_irq_parse_one() for interrupt defined > >> in the changer's node. This does not create a mapping for it. The > >> mapping is only created when changer's "interrupt-child" creates a > >> mapping for their interrupt - then changer's alloc() routine calls > >> irq_domain_alloc_irqs_parent() in the same way as all other > >> hierarchical irqchips do. > >> > >> I don't see where double mapping can appear here. Please explain. > > > > Just look at your code. You start probing a device that has an > > 'interrupts' property. This triggers the allocation of an interrupt. > > Where does it trigger it? I'll let you follow the path from of_device_alloc() all the way to the actual DT parsing of the interrupt specifier, allocation of the interrupt descriptor and mapping in the parent domain. You then override the mapping (I really should put a WARN_ON() in the irqdomain code to catch this sort of things). > And what is this "allocation", after all? Is it allocation of virq > number? And descriptor, and mapping in the domain. > That allocation happens when irq_create_fwspec_mapping() calls > irq_domain_alloc_irqs(). But this path does not necessary gets > executed in probing. In the current irq_tyoe_changer driver, it is > not. I don't think you see the full extent of the problem. M. -- Without deviation from the norm, progress is not possible.