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 2279FC28D13 for ; Thu, 25 Aug 2022 13:02:43 +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-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc: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=i3ve4+46THAjR1JIpqlisuRi6By7T8C7srPSZDB9o1k=; b=Xf3BBsH3AKRzXT kZgzwtCaXaREQkywynSas86/U79Vt9oNo89PbLK33iaWU3mgYz4mWdBGYiE7yjwREAyPwpDd7Mn+i fdTSAMuB4Zuab5VPRdftD4eTjtoNDu/Eh4sJxnhbFKlTLpoHVHMuE3sIfwVz/zudD7l4e18Y9CZlb eeTzddgdiLJ0Rzt1OLSm7BhJ9VLyG+1etsjHcum/McxpurY8c7WHn5/QhWcUPQhoAc63Cb8TEJkqe LVFzkzA2Uq1eu4M+gbTBXZhMpj0NjovHSZoEWQYvL4W9EXjZzNpYr8kbmceGSwMvb6s22HzreR679 3FyI+qayTrDPsyRGWAHA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oRCTy-00EGUT-0U; Thu, 25 Aug 2022 13:01:14 +0000 Received: from mail-lf1-x134.google.com ([2a00:1450:4864:20::134]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oRCTu-00EGRY-EC for linux-arm-kernel@lists.infradead.org; Thu, 25 Aug 2022 13:01:12 +0000 Received: by mail-lf1-x134.google.com with SMTP id s6so16902850lfo.11 for ; Thu, 25 Aug 2022 06:01:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc; bh=HXz5r5nARBDUrWmq5bMXUBc88dL0Gf53fHnzPY0C8Cw=; b=IjmH3eKJZsmSiWXRk8UD08OLLxfyc+keRxdrSyGSHQ8Kff1Q/6Sl/2/m8sqXaXtvht UUxmjcry5A4cPm7BAu22lFkLN1r7vxyr73y7AZcakr+awqp3Vtwxo1ps50ip7cp1Y2hQ QsAQ75lX1XILbbUqI2CIZCahqKuNRzapBWsPLLy6+Vo2KA0hqRVIWcbULMrxgJQwkSut +2uqpY1fbk20Jsi/PEb3B87aqI6jrCMswf14s//6+KwYiHHZXN5zWZ1fsIzoIsaqCeAr nCgEqb7tC+DzAPax4zjzPTuKIZBNOWj7REm3X00HpWgNa4COV+vcOiFkMbrz1ULx0+V3 ZlmQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=HXz5r5nARBDUrWmq5bMXUBc88dL0Gf53fHnzPY0C8Cw=; b=6FxDcW9L8CG0/fpuX5szAXAMXFaBT4Y9j1MUlQhwnNuPB6NFKOY4/l91BpaSvFd/bU 9uiZfD6chpWdT81CJMEef/f1YDH77XZGcMpgx503ySLokue+gSKC8o1sXInWCXDpZg7S BQIDdYEUWxaDh+9a7K2wvsBK+1ns7iwNyKqhEXcfSufJz6Pp/RxUVaN1vWaQC/wV6rnr cSgJgbKe/cAc4mzXNFEJIfVu8kW93MSYPataNKOjESI7u2ZE4Ig9dOA7tahk/UxJLYiO sAg5Htwy7QGjVwfAItMfcb55HNTfrf5qzTFz2UKxsqtvvTuRsDx8Xzgobyleyg7vICby ZwEA== X-Gm-Message-State: ACgBeo1jE4BxrqAnuRndzuxyRwPe3uplD6NbVYXKwX+kdLfRl/deKbVs u+4MCG2DxknqQSeCQfPkGrc= X-Google-Smtp-Source: AA6agR5E2VLJ311lYr2Z6ZX+fqdNCc+e5s1M3RyjO6AfhBJqTxKKgSoQHjbqmVNivcVedLapoj+S1Q== X-Received: by 2002:a05:6512:c10:b0:492:d263:f918 with SMTP id z16-20020a0565120c1000b00492d263f918mr1092408lfu.501.1661432468639; Thu, 25 Aug 2022 06:01:08 -0700 (PDT) Received: from mobilestation ([95.79.140.178]) by smtp.gmail.com with ESMTPSA id b9-20020ac25e89000000b00489e812f05asm497832lfq.21.2022.08.25.06.01.06 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Aug 2022 06:01:07 -0700 (PDT) Date: Thu, 25 Aug 2022 16:01:05 +0300 From: Serge Semin To: Alexander Stein Cc: Serge Semin , Rob Herring , Rob Herring , Krzysztof Kozlowski , Bjorn Helgaas , Lorenzo Pieralisi , Jingoo Han , Gustavo Pimentel , Richard Zhu , Lucas Stach , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , NXP Linux Team , linux-arm-kernel@lists.infradead.org, Alexey Malahov , Pavel Parkhomenko , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Frank Li , Manivannan Sadhasivam , linux-pci@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v5 01/20] dt-bindings: imx6q-pcie: Fix clock names for imx6sx and imx8mq Message-ID: <20220825130105.s5lj3h3pqlntns2f@mobilestation> References: <20220822184701.25246-1-Sergey.Semin@baikalelectronics.ru> <20220822184701.25246-2-Sergey.Semin@baikalelectronics.ru> <8354660.EvYhyI6sBW@steina-w> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <8354660.EvYhyI6sBW@steina-w> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220825_060110_555108_1E9E7889 X-CRM114-Status: GOOD ( 27.58 ) 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-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 Hi Alexander, On Thu, Aug 25, 2022 at 09:55:56AM +0200, Alexander Stein wrote: > Hello Serge, > > Am Montag, 22. August 2022, 20:46:42 CEST schrieb Serge Semin: > > Originally as it was defined the legacy bindings the pcie_inbound_axi and > > pcie_aux clock names were supposed to be used in the fsl,imx6sx-pcie and > > fsl,imx8mq-pcie devices respectively. But the bindings conversion has been > > incorrectly so now the fourth clock name is defined as "pcie_inbound_axi > > for imx6sx-pcie, pcie_aux for imx8mq-pcie", which is completely wrong. > > Let's fix that by conditionally apply the clock-names constraints based on > > the compatible string content. > > > > Fixes: 751ca492f131 ("dt-bindings: PCI: imx6: convert the imx pcie > > controller to dtschema") > > Signed-off-by: Serge Semin > > > > > > --- > > > > Changelog v5: > > - This is a new patch added on the v5 release of the patchset. > > --- > > .../bindings/pci/fsl,imx6q-pcie.yaml | 47 +++++++++++++++++-- > > 1 file changed, 42 insertions(+), 5 deletions(-) > > > > diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml index > > 376e739bcad4..ebfe75f1576e 100644 > > --- a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > +++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.yaml > > @@ -16,6 +16,47 @@ description: |+ > > > > allOf: > > - $ref: /schemas/pci/snps,dw-pcie.yaml# > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx6sx-pcie > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pcie > > + - const: pcie_bus > > + - const: pcie_phy > > + - const: pcie_inbound_axi > > + - if: > > + properties: > > + compatible: > > + contains: > > + const: fsl,imx8mq-pcie > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pcie > > + - const: pcie_bus > > + - const: pcie_phy > > + - const: pcie_aux > > + - if: > > + properties: > > + compatible: > > + not: > > + contains: > > + enum: > > + - fsl,imx6sx-pcie > > + - fsl,imx8mq-pcie > > I'm not so sure about this list essentially copying the (for now) two > compatibles from above, but no hard from my side. I have a quite similar patch > nesting the following structure: > if imx6sx > then > <4 clocks including pcie_inbound_axi> > else if imx8mq > then > <4 clocks including pcie_aux> > else > <3 clocks> The schema above looks a bit different in your case: + if: + then: + else: + if: + then: + else: Anyway the main point is each new statement adds one more indentation level, which in case of updating the schema with new clocks setup will get to be even more right shifted. Note for that reason you'd need to fully update the last else block. So the corresponding patch will get to be bulky and less readable. Another point for my approach is that the if-else-if-else-etc statement much harder to read than just multiple if-statements combined in the allOf property. IMO that's why more often you get to see the allOf-if-if-etc pattern than the allOf-if-else-if-else one. > > In the end I'm fine with both approaches, whatever DT bindings maintainer find > superior. > Acked-by: Alexander Stein Thanks. -Sergey > > > > + then: > > + properties: > > + clock-names: > > + items: > > + - const: pcie > > + - const: pcie_bus > > + - const: pcie_phy > > > > properties: > > compatible: > > @@ -57,11 +98,7 @@ properties: > > > > clock-names: > > minItems: 3 > > - items: > > - - const: pcie > > - - const: pcie_bus > > - - const: pcie_phy > > - - const: pcie_inbound_axi for imx6sx-pcie, pcie_aux for imx8mq-pcie > > + maxItems: 4 > > > > num-lanes: > > const: 1 > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel