From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oo1-f41.google.com (mail-oo1-f41.google.com [209.85.161.41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 09F62188; Tue, 5 Dec 2023 13:02:03 -0800 (PST) Received: by mail-oo1-f41.google.com with SMTP id 006d021491bc7-58e30de3933so2124922eaf.3; Tue, 05 Dec 2023 13:02:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701810122; x=1702414922; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=IdYRNh7Vlob6vHvVQj3FqYlHUzVIZf5EcZbK3xhngEg=; b=bmC304Ybt/lLceVWsw9hKVUQc4tT6c/EXo7VyB5kCoSkYY0yZjwiSp4NnPrF3DcVkq FgzKQWcJzl5meVCTC5gWPcrCLLu4UXYOug+FP9Z4IW6V8DiBGcxkRuZPBOgloLh0NFwG tbm/EccMFRZubUff9J/Ld8m5NI34i9Uhs2fTOXVeEZQEEwUmBXR/wO3BKzP6ZIvChgL2 8Qm2WURSfz2w8UHNltYlX2GYG5gPSk5nj0hFgvX2XzKSuH5TMsYBKw+gNJ2ZKTByGqBs 2VKCDlxhwtcb7SDpG5L/keprKM9Y1s8IkFGhu8G8Zde8UB8wmUL4IELj11KDuGxWCl+R wF7w== X-Gm-Message-State: AOJu0Yx3jMCB6MTfbiz6YoxNEFiflpptJw8OVrDild9hhu+Qx1YWL02C 1gISSb38dpxlUmw/K98hrQ== X-Google-Smtp-Source: AGHT+IEELo2vI+di+qlC0rsG6Gdv4w/6prGhHS2OYuIkz413JcirnWVw4QzKEsB7bi8jrOvnQ/zU7Q== X-Received: by 2002:a05:6870:b50e:b0:1fb:75c:3feb with SMTP id v14-20020a056870b50e00b001fb075c3febmr7677773oap.75.1701810122176; Tue, 05 Dec 2023 13:02:02 -0800 (PST) Received: from herring.priv (66-90-144-107.dyn.grandenetworks.net. [66.90.144.107]) by smtp.gmail.com with ESMTPSA id ps14-20020a0568709e0e00b001fb4aaf261csm1037762oab.32.2023.12.05.13.01.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Dec 2023 13:02:01 -0800 (PST) Received: (nullmailer pid 3792495 invoked by uid 1000); Tue, 05 Dec 2023 21:01:58 -0000 Date: Tue, 5 Dec 2023 15:01:58 -0600 From: Rob Herring To: Yoshinori Sato Cc: linux-sh@vger.kernel.org, Damien Le Moal , Krzysztof Kozlowski , Conor Dooley , Geert Uytterhoeven , Michael Turquette , Stephen Boyd , David Airlie , Daniel Vetter , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Thomas Gleixner , Lorenzo Pieralisi , =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , Bjorn Helgaas , Greg Kroah-Hartman , Jiri Slaby , Magnus Damm , Daniel Lezcano , Rich Felker , John Paul Adrian Glaubitz , Lee Jones , Helge Deller , Heiko Stuebner , Jernej Skrabec , Chris Morgan , Linus Walleij , Randy Dunlap , Arnd Bergmann , Hyeonggon Yoo <42.hyeyoo@gmail.com>, David Rientjes , Vlastimil Babka , Baoquan He , Andrew Morton , Guenter Roeck , Stephen Rothwell , Guo Ren , Javier Martinez Canillas , Azeem Shaikh , Palmer Dabbelt , Bin Meng , Max Filippov , Tom Rix , Herve Codina , Jacky Huang , Lukas Bulwahn , Jonathan Corbet , Biju Das , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Sam Ravnborg , Michael Karcher , Sergey Shtylyov , Laurent Pinchart , linux-ide@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-pci@vger.kernel.org, linux-serial@vger.kernel.org, linux-fbdev@vger.kernel.org Subject: Re: [DO NOT MERGE v5 19/37] dt-bindings: interrupt-controller: renesas,sh7751-irl-ext: Add json-schema Message-ID: <20231205210158.GA3774253-robh@kernel.org> References: <1623383c89532994218795cd3755c37819be426b.1701768028.git.ysato@users.sourceforge.jp> Precedence: bulk X-Mailing-List: linux-clk@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1623383c89532994218795cd3755c37819be426b.1701768028.git.ysato@users.sourceforge.jp> On Tue, Dec 05, 2023 at 06:45:38PM +0900, Yoshinori Sato wrote: > Renesas SH7751 external interrupt encoder json-schema. > > Signed-off-by: Yoshinori Sato > --- > .../renesas,sh7751-irl-ext.yaml | 83 +++++++++++++++++++ > 1 file changed, 83 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > new file mode 100644 > index 000000000000..ba4fe2e4d749 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > @@ -0,0 +1,83 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,sh7751-irl-ext.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas SH7751 IRL external encoder with enable regs. IRL? In Real Life? > + > +maintainers: > + - Yoshinori Sato > + > +description: | Don't need '|' if no formatting to preserve. > + This is the generally used external interrupt encoder on SH7751 based boards. > + > +properties: > + compatible: > + items: > + - const: renesas,sh7751-irl-ext > + > + reg: > + minItems: 1 maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 1 > + > + '#address-cells': > + const: 0 > + > + '#size-cells': > + const: 0 Don't need #size-cells. > + > + renesas,width: > + description: Enable register width > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [8, 16, 32] reg-io-width is the standard property for this purpose. > + > + renesas,set-to-disable: > + $ref: /schemas/types.yaml#/definitions/flag > + description: Setting this flag to 1 disables it. You can't set a boolean to 1. What is 'it' here? > + > + renesas,enable-bit: > + $ref: /schemas/types.yaml#/definitions/uint32-array You've described a 2 entry matrix, not an array. > + description: | > + IRL enable register bit mapping > + 1st word IRL > + 2nd word bit index of enable register Needs a better description of what this is for. If it is per SoC then it should be implied from the compatible string. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - renesas,width > + - renesas,enable-bit > + > +additionalProperties: false > + > +examples: > + - | > + r2dintc: sh7751irl_encoder@a4000000 { interrupt-controller@... Drop unused labels. > + compatible = "renesas,sh7751-irl-ext"; > + reg = <0xa4000000 0x02>; > + interrupt-controller; > + #address-cells = <0>; > + #size-cells = <0>; > + #interrupt-cells = <1>; > + renesas,width = <16>; > + renesas,enable-bit = <0 11>, /* PCI INTD */ > + <1 9>, /* CF IDE */ > + <2 8>, /* CF CD */ > + <3 12>, /* PCI INTC */ > + <4 10>, /* SM501 */ > + <5 6>, /* KEY */ > + <6 5>, /* RTC ALARM */ > + <7 4>, /* RTC T */ > + <8 7>, /* SDCARD */ > + <9 14>, /* PCI INTA */ > + <10 13>, /* PCI INTB */ > + <11 0>, /* EXT */ > + <12 15>; /* TP */ Looks like the first value is just the index of the entry, so drop it and use the index. But better yet, these are all per interrupt values. Put them into the interrupt cell values instead. For example the RTC would have something like: interrupts = <6 5>, <7 4>; Though I do wonder if you really need the first value, or that was just interrupt numbers you made up and then created this mapping? Rob 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 gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id AE53BC4167B for ; Tue, 5 Dec 2023 21:02:05 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 2101E10E61A; Tue, 5 Dec 2023 21:02:05 +0000 (UTC) Received: from mail-oi1-f170.google.com (mail-oi1-f170.google.com [209.85.167.170]) by gabe.freedesktop.org (Postfix) with ESMTPS id 202A510E61A for ; Tue, 5 Dec 2023 21:02:03 +0000 (UTC) Received: by mail-oi1-f170.google.com with SMTP id 5614622812f47-3b845ba9ba9so3870356b6e.3 for ; Tue, 05 Dec 2023 13:02:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701810122; x=1702414922; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=IdYRNh7Vlob6vHvVQj3FqYlHUzVIZf5EcZbK3xhngEg=; b=ivqHAp7PwNdwz+Zcrfd2QU4qWYhNn2CSHyGgRyI/AXAAs1gkRkSu6d6a172WpG8gTo xOlhdZvnoVjJwtU3SiQxen5lE3q/WD8bNJJKKEQ5AS5KF85gMt8sQ/edDLBhAdjTghFl S42RnFbSnM/SuGlsFxmgWVST/slkChimP50v9E4kzgBTnG6W2GxGGXy3CXKsL2Rq7DsS bNtByf3eSsnXeX8ZUqyM1adZnAtOdG1O6Fm7XRceDrUDhT75FodwZmiPE/jgbCR7L0E+ xhIwleprYsfQYyGcx75UvK4sC/gZIiMdVniwxKfb4SZeK0EXpAfdOtGW5U3EQQ4UCbTq AXzw== X-Gm-Message-State: AOJu0YzpbGFh+Btm/pfd57mX6nMgwVHU+dHyx/LUXDre/ZkQi8bqOOm7 EESwzNLfi97XkcPYSAGczg== X-Google-Smtp-Source: AGHT+IEELo2vI+di+qlC0rsG6Gdv4w/6prGhHS2OYuIkz413JcirnWVw4QzKEsB7bi8jrOvnQ/zU7Q== X-Received: by 2002:a05:6870:b50e:b0:1fb:75c:3feb with SMTP id v14-20020a056870b50e00b001fb075c3febmr7677773oap.75.1701810122176; Tue, 05 Dec 2023 13:02:02 -0800 (PST) Received: from herring.priv (66-90-144-107.dyn.grandenetworks.net. [66.90.144.107]) by smtp.gmail.com with ESMTPSA id ps14-20020a0568709e0e00b001fb4aaf261csm1037762oab.32.2023.12.05.13.01.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 05 Dec 2023 13:02:01 -0800 (PST) Received: (nullmailer pid 3792495 invoked by uid 1000); Tue, 05 Dec 2023 21:01:58 -0000 Date: Tue, 5 Dec 2023 15:01:58 -0600 From: Rob Herring To: Yoshinori Sato Subject: Re: [DO NOT MERGE v5 19/37] dt-bindings: interrupt-controller: renesas,sh7751-irl-ext: Add json-schema Message-ID: <20231205210158.GA3774253-robh@kernel.org> References: <1623383c89532994218795cd3755c37819be426b.1701768028.git.ysato@users.sourceforge.jp> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1623383c89532994218795cd3755c37819be426b.1701768028.git.ysato@users.sourceforge.jp> X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= , linux-fbdev@vger.kernel.org, Rich Felker , Herve Codina , Geert Uytterhoeven , linux-sh@vger.kernel.org, Bin Meng , Michael Turquette , linux-pci@vger.kernel.org, Jacky Huang , Palmer Dabbelt , Max Filippov , Guo Ren , Lee Jones , Krzysztof Kozlowski , Hyeonggon Yoo <42.hyeyoo@gmail.com>, Jiri Slaby , linux-clk@vger.kernel.org, Stephen Rothwell , Laurent Pinchart , Baoquan He , Jonathan Corbet , Helge Deller , Daniel Lezcano , Magnus Damm , Javier Martinez Canillas , Jernej Skrabec , linux-serial@vger.kernel.org, David Rientjes , Lukas Bulwahn , Lorenzo Pieralisi , Guenter Roeck , devicetree@vger.kernel.org, Conor Dooley , Thomas Zimmermann , Arnd Bergmann , =?UTF-8?Q?Uwe_Kleine=2DK=C3=B6nig?= , Maxime Ripard , Sam Ravnborg , Damien Le Moal , dri-devel@lists.freedesktop.org, Chris Morgan , John Paul Adrian Glaubitz , Bjorn Helgaas , Thomas Gleixner , Vlastimil Babka , Sergey Shtylyov , linux-ide@vger.kernel.org, Stephen Boyd , Greg Kroah-Hartman , Randy Dunlap , Biju Das , linux-kernel@vger.kernel.org, Azeem Shaikh , linux-renesas-soc@vger.kernel.org, Tom Rix , Michael Karcher , Andrew Morton Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Tue, Dec 05, 2023 at 06:45:38PM +0900, Yoshinori Sato wrote: > Renesas SH7751 external interrupt encoder json-schema. > > Signed-off-by: Yoshinori Sato > --- > .../renesas,sh7751-irl-ext.yaml | 83 +++++++++++++++++++ > 1 file changed, 83 insertions(+) > create mode 100644 Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > > diff --git a/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > new file mode 100644 > index 000000000000..ba4fe2e4d749 > --- /dev/null > +++ b/Documentation/devicetree/bindings/interrupt-controller/renesas,sh7751-irl-ext.yaml > @@ -0,0 +1,83 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/interrupt-controller/renesas,sh7751-irl-ext.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Renesas SH7751 IRL external encoder with enable regs. IRL? In Real Life? > + > +maintainers: > + - Yoshinori Sato > + > +description: | Don't need '|' if no formatting to preserve. > + This is the generally used external interrupt encoder on SH7751 based boards. > + > +properties: > + compatible: > + items: > + - const: renesas,sh7751-irl-ext > + > + reg: > + minItems: 1 maxItems: 1 > + > + interrupt-controller: true > + > + '#interrupt-cells': > + const: 1 > + > + '#address-cells': > + const: 0 > + > + '#size-cells': > + const: 0 Don't need #size-cells. > + > + renesas,width: > + description: Enable register width > + $ref: /schemas/types.yaml#/definitions/uint32 > + enum: [8, 16, 32] reg-io-width is the standard property for this purpose. > + > + renesas,set-to-disable: > + $ref: /schemas/types.yaml#/definitions/flag > + description: Setting this flag to 1 disables it. You can't set a boolean to 1. What is 'it' here? > + > + renesas,enable-bit: > + $ref: /schemas/types.yaml#/definitions/uint32-array You've described a 2 entry matrix, not an array. > + description: | > + IRL enable register bit mapping > + 1st word IRL > + 2nd word bit index of enable register Needs a better description of what this is for. If it is per SoC then it should be implied from the compatible string. > + > +required: > + - compatible > + - reg > + - interrupt-controller > + - '#interrupt-cells' > + - renesas,width > + - renesas,enable-bit > + > +additionalProperties: false > + > +examples: > + - | > + r2dintc: sh7751irl_encoder@a4000000 { interrupt-controller@... Drop unused labels. > + compatible = "renesas,sh7751-irl-ext"; > + reg = <0xa4000000 0x02>; > + interrupt-controller; > + #address-cells = <0>; > + #size-cells = <0>; > + #interrupt-cells = <1>; > + renesas,width = <16>; > + renesas,enable-bit = <0 11>, /* PCI INTD */ > + <1 9>, /* CF IDE */ > + <2 8>, /* CF CD */ > + <3 12>, /* PCI INTC */ > + <4 10>, /* SM501 */ > + <5 6>, /* KEY */ > + <6 5>, /* RTC ALARM */ > + <7 4>, /* RTC T */ > + <8 7>, /* SDCARD */ > + <9 14>, /* PCI INTA */ > + <10 13>, /* PCI INTB */ > + <11 0>, /* EXT */ > + <12 15>; /* TP */ Looks like the first value is just the index of the entry, so drop it and use the index. But better yet, these are all per interrupt values. Put them into the interrupt cell values instead. For example the RTC would have something like: interrupts = <6 5>, <7 4>; Though I do wonder if you really need the first value, or that was just interrupt numbers you made up and then created this mapping? Rob