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 X-Spam-Level: X-Spam-Status: No, score=-9.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F1431C433DF for ; Thu, 27 Aug 2020 10:08:10 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id BFA81207F7 for ; Thu, 27 Aug 2020 10:08:10 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="vUpR9EL2"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ZwVP72Wy" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org BFA81207F7 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=i5ztINxds2wDvfRVnsK2+Sdh6hLPF2owz2SCxypHJ+w=; b=vUpR9EL2+P6q13R+rAC8ptxB8 IHv7hOk5W7K+2tts1NcNrmZkT/kX7EWXyqy7VUqchBiPeOE3L6LZBZlTAmPH4/wR9PvON2L7MZS9j yR8OMeBa/CoM6yX7QuodVn/t/CLKaFddJuEKarIMt0taaylivzu6R4L0K7p6JK4sPEhlqL9ZP3F18 tCYbRNIkTp6cWf13F2viYKsB4IMx16qxmV6R1DdihnQx4vH35dfRAhcC6yEyytxbTSNq2lWLX5wnK 1yrVW/TlT/4l1/pC54tA52dojvIc7S5U/i2Y19G5drVLc6t7A9hEvC2gXArsfE3JC/J9ZVb5h+fPX 8K5qNx+fQ==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kBEnl-0004AN-FF; Thu, 27 Aug 2020 10:06:37 +0000 Received: from mail-ej1-x644.google.com ([2a00:1450:4864:20::644]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1kBEnh-00049f-SO; Thu, 27 Aug 2020 10:06:35 +0000 Received: by mail-ej1-x644.google.com with SMTP id d11so6805280ejt.13; Thu, 27 Aug 2020 03:06:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=H5b7vH/Mm3tUXXI+YL/JUy2ooYJLh277NmvDbO34a90=; b=ZwVP72WyTO8UGptv0QVO5pjPiiQxg3QQS7Gv/qsfr7frOnlHhnafxMvJ8GUvauUwiy uLalmuljZwpayEuok0FSqKxa3vNn3g9hbEurMvRAEEt9YK4xMSqhF007MB8FOS8k+neX MrQNRIYxhZSENGn4U1CHf1keMtchPVHuE9FmnnSN5gu4LZs9Z/N8b9gtgsWQRoS7ItP4 NVK7W2+VyAfHr0FKGGHjtf7WV+CPvJi/uK0F7APbInCebpnB0qY37oDwnt9xzrdLCpv6 VWgOzd0sLDg0vqc/ojtdllNADxIgE2hEd7nV4JB1/1L9C6ZeZdOO5ZSztGXoo8XXV8eJ EqaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=H5b7vH/Mm3tUXXI+YL/JUy2ooYJLh277NmvDbO34a90=; b=S9Is2TE4L2LhOIKZVnI16hZiU2sIKfIc+eVYOBeFW8kJbkBAMO7RLwfJGxFaecfKdv A2QQa4uTBjAenKsRRSE/mPSLUoS0J4AG+epAfPHc14ChZMysqkGR2e1ltpcPGPQVFqL9 l6IFL1V+LXw5iyetIBszUhQpk1Q2m/CRIiM4ye/pUPJBCDGO5QLlxIyMAwPCy/Ht8hgg 1t1mnbGXf83SHGrgpmE5ue5OF9KJlQ9FbB7aMpYbduLSy1qeUA1YD2LpU7QaW7qrn0BH GmfM8MdIUm+k6xG66V/EOtD+aq1cTiRpHpzy+OUybCVLzZWhNB5u3UOg7L+C3owZuKNC Z3eQ== X-Gm-Message-State: AOAM530uq/kO9b8iKgZEbxEP16J8AGoiEratOF1XSWuOKl7XD0QrrTon d8dUSwkbjbdW55Iy8/58vNI= X-Google-Smtp-Source: ABdhPJwYe3czk57emax7/H0cocpDsdNj6ermBTHkChVY5+jGk5Ax7YgihFrLjoGyeAX88M/oCzwkVg== X-Received: by 2002:a17:906:af41:: with SMTP id ly1mr19513097ejb.418.1598522792401; Thu, 27 Aug 2020 03:06:32 -0700 (PDT) Received: from BV030612LT ([188.24.159.61]) by smtp.gmail.com with ESMTPSA id cz21sm1191148edb.45.2020.08.27.03.06.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 27 Aug 2020 03:06:31 -0700 (PDT) Date: Thu, 27 Aug 2020 13:06:29 +0300 From: Cristian Ciocaltea To: Rob Herring Subject: Re: [PATCH v5 1/3] dt-bindings: interrupt-controller: Add Actions SIRQ controller binding Message-ID: <20200827100629.GA2451538@BV030612LT> References: <6bd99d4a7e50904b57bb3ad050725fbb418874b7.1597852360.git.cristian.ciocaltea@gmail.com> <20200825220913.GA1423455@bogus> <20200826214220.GA2444747@BV030612LT> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200827_060633_964846_EAC47090 X-CRM114-Status: GOOD ( 38.36 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: devicetree@vger.kernel.org, Jason Cooper , Marc Zyngier , linux-actions@lists.infradead.org, "linux-kernel@vger.kernel.org" , Manivannan Sadhasivam , Thomas Gleixner , Andreas =?iso-8859-1?Q?F=E4rber?= , "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 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 Wed, Aug 26, 2020 at 04:48:38PM -0600, Rob Herring wrote: > On Wed, Aug 26, 2020 at 3:42 PM Cristian Ciocaltea > wrote: > > > > Hi Rob, > > > > Thanks for the review! > > > > On Tue, Aug 25, 2020 at 04:09:13PM -0600, Rob Herring wrote: > > > On Wed, Aug 19, 2020 at 07:37:56PM +0300, Cristian Ciocaltea wrote: > > > > Actions Semi Owl SoCs SIRQ interrupt controller is found in S500, S700 > > > > and S900 SoCs and provides support for handling up to 3 external > > > > interrupt lines. > > > > > > > > Signed-off-by: Cristian Ciocaltea > > > > --- > > > > Changes in v5: > > > > - Updated controller description statements both in the commit message > > > > and the binding doc > > > > > > > > .../actions,owl-sirq.yaml | 68 +++++++++++++++++++ > > > > 1 file changed, 68 insertions(+) > > > > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > > > > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > new file mode 100644 > > > > index 000000000000..cf9b7a514e4e > > > > --- /dev/null > > > > +++ b/Documentation/devicetree/bindings/interrupt-controller/actions,owl-sirq.yaml > > > > @@ -0,0 +1,68 @@ > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > > > > +%YAML 1.2 > > > > +--- > > > > +$id: http://devicetree.org/schemas/interrupt-controller/actions,owl-sirq.yaml# > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > > + > > > > +title: Actions Semi Owl SoCs SIRQ interrupt controller > > > > + > > > > +maintainers: > > > > + - Manivannan Sadhasivam > > > > + - Cristian Ciocaltea > > > > + > > > > +description: | > > > > + This interrupt controller is found in the Actions Semi Owl SoCs (S500, S700 > > > > + and S900) and provides support for handling up to 3 external interrupt lines. > > > > + > > > > +properties: > > > > + compatible: > > > > + oneOf: > > > > + - items: > > > > + - enum: > > > > + - actions,s500-sirq > > > > + - actions,s700-sirq > > > > + - actions,s900-sirq > > > > + - const: actions,owl-sirq > > > > + - const: actions,owl-sirq > > > > > > This should be dropped. You should always have the SoC specific > > > compatible. > > > > Sure, I will get rid of the 'owl-sirq' compatible. > > > > > > + > > > > + reg: > > > > + maxItems: 1 > > > > + > > > > + interrupt-controller: true > > > > + > > > > + '#interrupt-cells': > > > > + const: 2 > > > > + description: > > > > + The first cell is the input IRQ number, between 0 and 2, while the second > > > > + cell is the trigger type as defined in interrupt.txt in this directory. > > > > + > > > > + 'actions,ext-interrupts': > > > > + description: | > > > > + Contains the GIC SPI IRQ numbers mapped to the external interrupt > > > > + lines. They shall be specified sequentially from output 0 to 2. > > > > + $ref: /schemas/types.yaml#/definitions/uint32-array > > > > + minItems: 3 > > > > + maxItems: 3 > > > > > > Can't you use 'interrupts' here? > > > > This was actually my initial idea, but it might confuse the users since > > this is not following the parent controller IRQ specs, i.e. the trigger > > type is set internally by the SIRQ driver, it's not taken from DT. > > Then what's the 2nd cell for? I should have added also a child device sample to make it more clear how this is supposed to work: &i2c0 { atc260x: pmic@65 { [...] interrupt-parent = <&sirq>; interrupts = <2 IRQ_TYPE_LEVEL_HIGH>; }; }; The PMIC above uses the SIRQ2 pin of the SIRQ controller to trigger interrupts, while the controller is responsible for proper translation before sending to GIC, i.e. converting falling edge to rising edge signal and active low to active high signal. > > Please see the DTS sample bellow where both devices are on the same > > level and have GIC as interrupt parent. The 'interrupts' property > > in the sirq node looks incomplete now. That is why I decided to use > > a custom name for it, although I'm not sure it's the most relevant one, > > I am open to any other suggestion. > > > > i2c0: i2c@b0170000 { > > [...] > > interrupts = ; > > [...] > > }; > > > > sirq: interrupt-controller@b01b0200 { > > [...] > > interrupt-controller; > > #interrupt-cells = <2>; > > interrupts = <13>, /* SIRQ0 */ > > <14>, /* SIRQ1 */ > > <15>; /* SIRQ2 */ > > This isn't valid if the GIC is the parent as you have to have 3 cells > for each interrupt. Right, that's the reason of replacing 'interrupts' with 'actions,ext-interrupts'. > Ultimately the GIC trigger type has to be > something. Is it fixed or passed thru? If the latter, just use 0 > (IRQ_TYPE_NONE) if the GIC trigger mode is not fixed. Having some sort > of translation of the trigger is pretty common. Yes, as explained above, the SIRQ controller performs indeed the translation of the incoming signal. So if I understand correctly, your suggestion would be to use the following inside the sirq node: interrupts = , /* SIRQ0 */ [...] > Rob Thanks, Cristi _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel