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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (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 DCF07C46CD2 for ; Tue, 9 Jan 2024 16:53:51 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:Content-Type: Content-Transfer-Encoding:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:From:References:CC:To:Subject: MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=KhD4DL5CGW6C/V99HYAhf1y19BLlPW73cBJUlcry7ks=; b=PgHVYaCeSqprWw OYmmde+wr5+nQ66CXlP/+CRc7ZgvLm7igftiBnIPCZblFW6uE4HFM3EOf8KOxtIwzp+/Y8aB/QIJI GOtYEnWVsjmgg+yAB1mZUa773re0UvSQCMQy8idVat0UWqVZf79PPlfVT7hFzNaD0UOun5P+cXQyZ GozOj/yCxrOIMKEpNQjecmCrXcbzLZnOpTnw4DBZkf9HPaH/E5KNW8cgpf4AJJWJlLmt8KvbcundN JZZS+Fgzv0dnQbJ2HnelK0SzLaLTpgjE0iZzKp2FXSZWZnCIFXYIh330QwQ13flEyLwHrNx5d6h06 XqHcqQwgMKDnmPmfMiuA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1rNFM1-008ukV-1m; Tue, 09 Jan 2024 16:53:29 +0000 Received: from fllv0016.ext.ti.com ([198.47.19.142]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1rNFLy-008ujc-1a for linux-arm-kernel@lists.infradead.org; Tue, 09 Jan 2024 16:53:28 +0000 Received: from lelv0266.itg.ti.com ([10.180.67.225]) by fllv0016.ext.ti.com (8.15.2/8.15.2) with ESMTP id 409Gr5Xm016878; Tue, 9 Jan 2024 10:53:05 -0600 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1704819185; bh=iHiTjx8NHZJnPof+njozBL2G+dd0BsVHIjNWCy6rIek=; h=Date:Subject:To:CC:References:From:In-Reply-To; b=Yy3bMx4/Yzf97w86aOfBS4akux9QB2Kuo5MatkVOI+0K9M8kchw4Oqto7TtkbrtMr 2tL48v1DNa2NdNWNJhQccC9k8qy3/pfg9p32n89F6nuLNtgezOFNwohAzj0oVfL94Z NuMDjG5qfsIWUm0HKnL/gbstXMeEoeb6OaUgdHkA= Received: from DFLE112.ent.ti.com (dfle112.ent.ti.com [10.64.6.33]) by lelv0266.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 409Gr57x031093 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Tue, 9 Jan 2024 10:53:05 -0600 Received: from DFLE112.ent.ti.com (10.64.6.33) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Tue, 9 Jan 2024 10:53:05 -0600 Received: from lelvsmtp6.itg.ti.com (10.180.75.249) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Tue, 9 Jan 2024 10:53:05 -0600 Received: from [10.249.40.136] ([10.249.40.136]) by lelvsmtp6.itg.ti.com (8.15.2/8.15.2) with ESMTP id 409Gr3gb015005; Tue, 9 Jan 2024 10:53:03 -0600 Message-ID: Date: Tue, 9 Jan 2024 10:53:03 -0600 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH RFC v2 02/11] dt-bindings: gpu: Add PowerVR Series5 SGX GPUs Content-Language: en-US To: Krzysztof Kozlowski , Frank Binns , Matt Coster , "H . Nikolaus Schaller" , Adam Ford , Ivaylo Dimitrov , Maarten Lankhorst , Maxime Ripard , Thomas Zimmermann , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , =?UTF-8?Q?Beno=C3=AEt_Cousson?= , Tony Lindgren , Nishanth Menon , Vignesh Raghavendra , Tero Kristo , Paul Cercueil CC: , , , , , , References: <20240108183302.255055-1-afd@ti.com> <20240108183302.255055-3-afd@ti.com> From: Andrew Davis In-Reply-To: X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240109_085326_617403_DC7ADD89 X-CRM114-Status: GOOD ( 25.05 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 1/9/24 5:32 AM, Krzysztof Kozlowski wrote: > On 08/01/2024 19:32, Andrew Davis wrote: >> The Imagination PowerVR Series5 "SGX" GPU is part of several SoCs from >> multiple vendors. Describe how the SGX GPU is integrated in these SoC, >> including register space and interrupts. Clocks, reset, and power domain >> information is SoC specific. >> >> Signed-off-by: Andrew Davis >> --- >> .../bindings/gpu/img,powervr-sgx.yaml | 124 ++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 125 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml >> >> diff --git a/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml b/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml >> new file mode 100644 >> index 0000000000000..bb821e1184de9 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/gpu/img,powervr-sgx.yaml >> @@ -0,0 +1,124 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +# Copyright (c) 2023 Imagination Technologies Ltd. > > Your email has @TI domain, are you sure you attribute your copyrights to > Imagination? > The file started as a copy/paste from a IMG copyrighted file, even though it is now almost completely re-written I've left their (c) for good measure. I'll add an additional TI (c). > ... > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: true > > Missing min/maxItems > These are set in the allOf/if/then blocks below, seems if I don't set them to at least something here then I get a warning: 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+' even if I define them in the allOf block below. I don't know what the min/max should be until I check the compatible in the allOf block. >> + >> + clock-names: >> + minItems: 1 >> + items: >> + - const: core >> + - const: mem >> + - const: sys >> + >> + power-domains: >> + maxItems: 1 >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + >> +additionalProperties: false > > This goes after allOf: block. > ACK >> + >> +allOf: >> + - if: >> + properties: >> + compatible: >> + contains: >> + const: ti,am6548-gpu >> + then: >> + required: >> + - power-domains >> + else: >> + properties: >> + power-domains: false >> + - if: >> + properties: >> + compatible: >> + contains: >> + enum: >> + - allwinner,sun6i-a31-gpu >> + - ingenic,jz4780-gpu >> + then: >> + allOf: >> + - if: > > I don't understand why do you need to embed allOf inside another allOf. > The upper (outer) if:then: looks entirely useless. > It is so that both compatibles falls through to having clock being required. Logic in YAML always seems messy to me, here it is in pseudo C: if (compatible == allwinner,sun6i-a31-gpu || compatible == ingenic,jz4780-gpu) { if (compatible == allwinner,sun6i-a31-gpu) clocks: ... if (compatible == ingenic,jz4780-gpu) clocks: ... required: - clocks - clock-names } else { /* disallow for all others */ properties: clocks: false clock-names: false } Now if I had an "else if" that didn't force the indention to keep growing I would have used that. (does one exist?) I also cannot simply add the clock properties only for the two compats need them for the reasons above and so must add them unconditionally before then explicitly disable them in a catch-all else path. Andrew >> + properties: >> + compatible: >> + contains: >> + const: allwinner,sun6i-a31-gpu >> + then: >> + properties: >> + clocks: >> + minItems: 2 >> + maxItems: 2 >> + clock-names: >> + minItems: 2 >> + maxItems: 2 > > > Best regards, > Krzysztof > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel