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 B3F5CC433EF for ; Tue, 25 Jan 2022 09:12:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1456505AbiAYJLm (ORCPT ); Tue, 25 Jan 2022 04:11:42 -0500 Received: from ams.source.kernel.org ([145.40.68.75]:44736 "EHLO ams.source.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1454453AbiAYI7j (ORCPT ); Tue, 25 Jan 2022 03:59:39 -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 4496EB8172C for ; Tue, 25 Jan 2022 08:59:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id F2FC5C340E0; Tue, 25 Jan 2022 08:59:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1643101172; bh=kCO7IxDgXPqShA70BUyMi5yfB+SyJ4wrM6l24c7nuMk=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=btNm7SA7UW4BAL9DclNC89iLciCZUGUH3hv38j3oye+XQIcAr31XkL7AZMKUR2ue1 f0cdh0FXxhy9TVGIjwrdLMiL0Q4PfbG307e/wxkTVE9+5ZKTkQvHcAJsl/ASiMNowb b2abNNSdR6ydJlgVUf/dxoq9/giJDj7lLJAdKxR6I42iuho9zxDUxta0STGU7PlBhH Fm+LaPBQK6MceVBfmYaCXrnXeexdm62pyousRVobJANtuekqjsyMwZPXl9skOpFJbq 4OB2uAqy8Tzv69oKAviZjB64+7kOrsyELklW5IgJ9f/lzafoRPVoirOzDhifk859Gq BAcZ6M2oYmMWw== 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 1nCHfm-002nTB-2t; Tue, 25 Jan 2022 08:59:30 +0000 Date: Tue, 25 Jan 2022 08:59:29 +0000 Message-ID: <874k5s8a32.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> 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 05:47:37 +0000, Nikita Yushchenko wrote: > > Hi > > > 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? > > I remember your suggestion to configure interrupt source's node with > interrupt-parent set to inverter and interrupt details for inverter's > parent. But this looks hacky and inconsistent for me. We'll have to agree to disagree. > > 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. > Thus it looked a good idea not to limit the conversion to inversion, > but support arbitrary one. Then, irqspec inside the node for the > intermediate entity obtains a meaning, making the entire model > consistent. Again, this cannot work, because the very *semantics* of an edge interrupt (event) cannot directly be converted in a level (state). > > I don't strongly insist on this approach, it just looks cleaner for me. > > > > I'm also pretty sure that with the current code, > > you end-up with *two* interrupts (one for the inverter and one for the > > end-point). > > 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. Then the endpoint also has an 'interrupts' property, also allocating an interrupt. You then happily override the mapping of the first IRQ with the second one in the parent irqchip. That's just broken. M. -- Without deviation from the norm, progress is not possible.