Message ID | 20230512022036.97987-4-xingyu.wu@starfivetech.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp4801330vqo; Thu, 11 May 2023 19:24:11 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4Xc1xSkreTW4qrn4pTLGRqImo7avrUSyeZy/5F+yu7Pzd2s691K3Wa0zT0m8OLzvvRk4kM X-Received: by 2002:a17:90a:e545:b0:24e:2e86:5465 with SMTP id ei5-20020a17090ae54500b0024e2e865465mr23165974pjb.31.1683858250932; Thu, 11 May 2023 19:24:10 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683858250; cv=none; d=google.com; s=arc-20160816; b=c8K3X/gkztSPsJM0R9YNu8oTWl0hKv0aTWfrSs31XnnotEM6xxLZOkGRtP1lmxWr9N 4Df9PQz5xVJEtfjnySk6wtfB4vNQ3gP7dTirihP1nAZmNOnhNWdqlaw/LJP7m9JaI8er JHIkkCz+LKW3xBAZv0fbb4IAu4Nu4cButZOAFywn+xhOGDiwTn5x11WPV8E9YnGeDrNs ampuYyq+N5ot/1oBOeZmwUyibHgCq9nJgPASsDfFnzVhmaudErEHa+mzV2z9xVieJQpo wf9zBLwu/T5CrNWtAzovNclhYt9jgcolnlj5aH4RrSmNC7ySA003E5iJssucIB9apQJG krKw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from; bh=PwbGvwuLjPEWuiiw4Gz44ViK028lH+sHoEfkYYz3Blw=; b=w/vydRopzxsKAJsBkdctbTXgF9SpfwZ9tQ5BbRIrscbr+AeuemVmJv8g7Ee0uUs1bO T7CsAgot4kRzN+h5Dhg7hHwtoC+PV9fYQaqEz3YSqFs7ne+yDCHxOzuujb0vCQwNcByE 90yW8jbtjq9aATrNBuGC5ZKFBQZaUiFyEVWuu2HUXQ06sisir2TVbEi38OPZjNsEerp7 ehLXc2D74dOyEzEJ0jlIABbzkgwgTwbtL25fGioWE/rbUqL+J88OM70dDn4fP3Pp5PwT aBXM83CU772XmD5e1jQn9Bw6kW0CzsktqH/fzMICqBvLHf0uq7nfQW2dxpBSynTw5v38 teww== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id s20-20020a63af54000000b005192d51325dsi7667520pgo.42.2023.05.11.19.23.58; Thu, 11 May 2023 19:24:10 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S239752AbjELCWf convert rfc822-to-8bit (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Thu, 11 May 2023 22:22:35 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:58176 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231799AbjELCWX (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 11 May 2023 22:22:23 -0400 Received: from ex01.ufhost.com (ex01.ufhost.com [61.152.239.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 208D159C5; Thu, 11 May 2023 19:22:22 -0700 (PDT) Received: from EXMBX165.cuchost.com (unknown [175.102.18.54]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "EXMBX165", Issuer "EXMBX165" (not verified)) by ex01.ufhost.com (Postfix) with ESMTP id B900324E291; Fri, 12 May 2023 10:22:15 +0800 (CST) Received: from EXMBX061.cuchost.com (172.16.6.61) by EXMBX165.cuchost.com (172.16.6.75) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Fri, 12 May 2023 10:22:15 +0800 Received: from localhost.localdomain (113.72.146.187) by EXMBX061.cuchost.com (172.16.6.61) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Fri, 12 May 2023 10:22:14 +0800 From: Xingyu Wu <xingyu.wu@starfivetech.com> To: <linux-riscv@lists.infradead.org>, <devicetree@vger.kernel.org>, "Michael Turquette" <mturquette@baylibre.com>, Stephen Boyd <sboyd@kernel.org>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Philipp Zabel <p.zabel@pengutronix.de>, Conor Dooley <conor@kernel.org>, "Emil Renner Berthing" <kernel@esmil.dk> CC: Rob Herring <robh+dt@kernel.org>, Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, Albert Ou <aou@eecs.berkeley.edu>, Hal Feng <hal.feng@starfivetech.com>, Xingyu Wu <xingyu.wu@starfivetech.com>, William Qiu <william.qiu@starfivetech.com>, <linux-kernel@vger.kernel.org>, <linux-clk@vger.kernel.org> Subject: [PATCH v4 3/7] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs Date: Fri, 12 May 2023 10:20:32 +0800 Message-ID: <20230512022036.97987-4-xingyu.wu@starfivetech.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20230512022036.97987-1-xingyu.wu@starfivetech.com> References: <20230512022036.97987-1-xingyu.wu@starfivetech.com> MIME-Version: 1.0 Content-Type: text/plain X-Originating-IP: [113.72.146.187] X-ClientProxiedBy: EXCAS062.cuchost.com (172.16.6.22) To EXMBX061.cuchost.com (172.16.6.61) X-YovoleRuleAgent: yovoleflag Content-Transfer-Encoding: 8BIT X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765653349213080463?= X-GMAIL-MSGID: =?utf-8?q?1765653349213080463?= |
Series |
Add PLL clocks driver and syscon for StarFive JH7110 SoC
|
|
Commit Message
Xingyu Wu
May 12, 2023, 2:20 a.m. UTC
Add PLL clock inputs from PLL clock generator. Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> --- .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
Comments
On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: > Add PLL clock inputs from PLL clock generator. > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > --- > .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- > 1 file changed, 18 insertions(+), 2 deletions(-) /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short 'i2stx_bclk_ext' was expected 'i2stx_lrck_ext' was expected 'i2srx_bclk_ext' was expected 'i2srx_lrck_ext' was expected 'tdm_ext' was expected 'mclk_ext' was expected 'pll0_out' was expected From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short 'i2stx_bclk_ext' was expected 'i2stx_lrck_ext' was expected 'i2srx_bclk_ext' was expected 'i2srx_lrck_ext' was expected 'tdm_ext' was expected 'mclk_ext' was expected 'pll0_out' was expected Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml This binding change is incompatible with the existing devicetrees for the visionfive 2.
On 2023/5/12 14:47, Conor Dooley wrote: > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: >> Add PLL clock inputs from PLL clock generator. >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> --- >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- >> 1 file changed, 18 insertions(+), 2 deletions(-) > > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > 'i2stx_bclk_ext' was expected > 'i2stx_lrck_ext' was expected > 'i2srx_bclk_ext' was expected > 'i2srx_lrck_ext' was expected > 'tdm_ext' was expected > 'mclk_ext' was expected > 'pll0_out' was expected > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > 'i2stx_bclk_ext' was expected > 'i2stx_lrck_ext' was expected > 'i2srx_bclk_ext' was expected > 'i2srx_lrck_ext' was expected > 'tdm_ext' was expected > 'mclk_ext' was expected > 'pll0_out' was expected > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > This binding change is incompatible with the existing devicetrees for > the visionfive 2. This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. Best regards, Xingyu Wu
On Fri, May 12, 2023 at 04:07:47PM +0800, Xingyu Wu wrote: > On 2023/5/12 14:47, Conor Dooley wrote: > > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: > >> Add PLL clock inputs from PLL clock generator. > >> > >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > >> --- > >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- > >> 1 file changed, 18 insertions(+), 2 deletions(-) > > > > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > > 'i2stx_bclk_ext' was expected > > 'i2stx_lrck_ext' was expected > > 'i2srx_bclk_ext' was expected > > 'i2srx_lrck_ext' was expected > > 'tdm_ext' was expected > > 'mclk_ext' was expected > > 'pll0_out' was expected > > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > > 'i2stx_bclk_ext' was expected > > 'i2stx_lrck_ext' was expected > > 'i2srx_bclk_ext' was expected > > 'i2srx_lrck_ext' was expected > > 'tdm_ext' was expected > > 'mclk_ext' was expected > > 'pll0_out' was expected > > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > > > > This binding change is incompatible with the existing devicetrees for > > the visionfive 2. > > This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. The existing devicetree is a valid, albeit limited, description of the hardware. After your changes to the clock driver in this series, but *without* the changes to the devicetrees, does the system still function? From a quick check of 4/7, it looks like it will not?
On 2023/5/12 17:35, Conor Dooley wrote: > On Fri, May 12, 2023 at 04:07:47PM +0800, Xingyu Wu wrote: >> On 2023/5/12 14:47, Conor Dooley wrote: >> > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: >> >> Add PLL clock inputs from PLL clock generator. >> >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> --- >> >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- >> >> 1 file changed, 18 insertions(+), 2 deletions(-) >> > >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short >> > 'i2stx_bclk_ext' was expected >> > 'i2stx_lrck_ext' was expected >> > 'i2srx_bclk_ext' was expected >> > 'i2srx_lrck_ext' was expected >> > 'tdm_ext' was expected >> > 'mclk_ext' was expected >> > 'pll0_out' was expected >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short >> > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short >> > 'i2stx_bclk_ext' was expected >> > 'i2stx_lrck_ext' was expected >> > 'i2srx_bclk_ext' was expected >> > 'i2srx_lrck_ext' was expected >> > 'tdm_ext' was expected >> > 'mclk_ext' was expected >> > 'pll0_out' was expected >> > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> > >> > This binding change is incompatible with the existing devicetrees for >> > the visionfive 2. >> >> This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. > > The existing devicetree is a valid, albeit limited, description of the > hardware. > After your changes to the clock driver in this series, but *without* the > changes to the devicetrees, does the system still function? > From a quick check of 4/7, it looks like it will not? I just tested it on the board and the system still worked without the changes about devicetree. But these clocks' rate were 0 because these could not get the PLL clocks from devicetree. Best regards, Xingyu Wu
On Fri, May 12, 2023 at 05:56:16PM +0800, Xingyu Wu wrote: > On 2023/5/12 17:35, Conor Dooley wrote: > > On Fri, May 12, 2023 at 04:07:47PM +0800, Xingyu Wu wrote: > >> On 2023/5/12 14:47, Conor Dooley wrote: > >> > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: > >> >> Add PLL clock inputs from PLL clock generator. > >> >> > >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > >> >> --- > >> >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- > >> >> 1 file changed, 18 insertions(+), 2 deletions(-) > >> > > >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > >> > 'i2stx_bclk_ext' was expected > >> > 'i2stx_lrck_ext' was expected > >> > 'i2srx_bclk_ext' was expected > >> > 'i2srx_lrck_ext' was expected > >> > 'tdm_ext' was expected > >> > 'mclk_ext' was expected > >> > 'pll0_out' was expected > >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > >> > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > >> > 'i2stx_bclk_ext' was expected > >> > 'i2stx_lrck_ext' was expected > >> > 'i2srx_bclk_ext' was expected > >> > 'i2srx_lrck_ext' was expected > >> > 'tdm_ext' was expected > >> > 'mclk_ext' was expected > >> > 'pll0_out' was expected > >> > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> > > >> > This binding change is incompatible with the existing devicetrees for > >> > the visionfive 2. > >> > >> This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. > > > > The existing devicetree is a valid, albeit limited, description of the > > hardware. > > After your changes to the clock driver in this series, but *without* the > > changes to the devicetrees, does the system still function? > > From a quick check of 4/7, it looks like it will not? > > I just tested it on the board and the system still worked without the changes > about devicetree. But these clocks' rate were 0 because these could not get > the PLL clocks from devicetree. Hmm, that sounds like an issue to me. If all of the clock rates are computed based off of parents that incorrectly report 0, are we not in for trouble? Should the fixed-factor clocks be retained as a fallback for the sake of compatibility? Emil, Stephen?
On 2023/5/12 21:49, Conor Dooley wrote: > On Fri, May 12, 2023 at 05:56:16PM +0800, Xingyu Wu wrote: >> On 2023/5/12 17:35, Conor Dooley wrote: >> > On Fri, May 12, 2023 at 04:07:47PM +0800, Xingyu Wu wrote: >> >> On 2023/5/12 14:47, Conor Dooley wrote: >> >> > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: >> >> >> Add PLL clock inputs from PLL clock generator. >> >> >> >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> >> --- >> >> >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- >> >> >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> > >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: >> >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short >> >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: >> >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short >> >> > 'i2stx_bclk_ext' was expected >> >> > 'i2stx_lrck_ext' was expected >> >> > 'i2srx_bclk_ext' was expected >> >> > 'i2srx_lrck_ext' was expected >> >> > 'tdm_ext' was expected >> >> > 'mclk_ext' was expected >> >> > 'pll0_out' was expected >> >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: >> >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short >> >> > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: >> >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short >> >> > 'i2stx_bclk_ext' was expected >> >> > 'i2stx_lrck_ext' was expected >> >> > 'i2srx_bclk_ext' was expected >> >> > 'i2srx_lrck_ext' was expected >> >> > 'tdm_ext' was expected >> >> > 'mclk_ext' was expected >> >> > 'pll0_out' was expected >> >> > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> > >> >> > This binding change is incompatible with the existing devicetrees for >> >> > the visionfive 2. >> >> >> >> This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. >> > >> > The existing devicetree is a valid, albeit limited, description of the >> > hardware. >> > After your changes to the clock driver in this series, but *without* the >> > changes to the devicetrees, does the system still function? >> > From a quick check of 4/7, it looks like it will not? >> >> I just tested it on the board and the system still worked without the changes >> about devicetree. But these clocks' rate were 0 because these could not get >> the PLL clocks from devicetree. > > Hmm, that sounds like an issue to me. If all of the clock rates are > computed based off of parents that incorrectly report 0, are we not in > for trouble? > Should the fixed-factor clocks be retained as a fallback for the sake of > compatibility? > Emil, Stephen? I got your concern. Actually, I can add a check in driver to see if the dts has pll clocks and then decide whether to use fixed-factor clocks or pll clocks from syscon. But eventually we have to use pll clocks and dts has to add it. Then the binding should add it synchronously, right? Best regards, Xingyu Wu
On Fri, May 19, 2023 at 03:59:19PM +0800, Xingyu Wu wrote: > On 2023/5/12 21:49, Conor Dooley wrote: > > On Fri, May 12, 2023 at 05:56:16PM +0800, Xingyu Wu wrote: > >> On 2023/5/12 17:35, Conor Dooley wrote: > >> > On Fri, May 12, 2023 at 04:07:47PM +0800, Xingyu Wu wrote: > >> >> On 2023/5/12 14:47, Conor Dooley wrote: > >> >> > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: > >> >> >> Add PLL clock inputs from PLL clock generator. > >> >> >> > >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > >> >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> > >> >> >> --- > >> >> >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- > >> >> >> 1 file changed, 18 insertions(+), 2 deletions(-) > >> >> > > >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > >> >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > >> >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > >> >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > >> >> > 'i2stx_bclk_ext' was expected > >> >> > 'i2stx_lrck_ext' was expected > >> >> > 'i2srx_bclk_ext' was expected > >> >> > 'i2srx_lrck_ext' was expected > >> >> > 'tdm_ext' was expected > >> >> > 'mclk_ext' was expected > >> >> > 'pll0_out' was expected > >> >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: > >> >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short > >> >> > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: > >> >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short > >> >> > 'i2stx_bclk_ext' was expected > >> >> > 'i2stx_lrck_ext' was expected > >> >> > 'i2srx_bclk_ext' was expected > >> >> > 'i2srx_lrck_ext' was expected > >> >> > 'tdm_ext' was expected > >> >> > 'mclk_ext' was expected > >> >> > 'pll0_out' was expected > >> >> > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml > >> >> > > >> >> > This binding change is incompatible with the existing devicetrees for > >> >> > the visionfive 2. > >> >> > >> >> This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. > >> > > >> > The existing devicetree is a valid, albeit limited, description of the > >> > hardware. > >> > After your changes to the clock driver in this series, but *without* the > >> > changes to the devicetrees, does the system still function? > >> > From a quick check of 4/7, it looks like it will not? > >> > >> I just tested it on the board and the system still worked without the changes > >> about devicetree. But these clocks' rate were 0 because these could not get > >> the PLL clocks from devicetree. > > > > Hmm, that sounds like an issue to me. If all of the clock rates are > > computed based off of parents that incorrectly report 0, are we not in > > for trouble? > > Should the fixed-factor clocks be retained as a fallback for the sake of > > compatibility? > > Emil, Stephen? > > I got your concern. Actually, I can add a check in driver to see if the dts > has pll clocks and then decide whether to use fixed-factor clocks or pll clocks > from syscon. But eventually we have to use pll clocks and dts has to add it. > Then the binding should add it synchronously, right? IMO, it is okay to change the bindings to only allow the "correct" representation of the clock tree, but the driver should fall back to the fixed factor clocks if it detects the old/limited configuration. Cheers, Conor.
On 2023/5/19 16:12, Conor Dooley wrote: > On Fri, May 19, 2023 at 03:59:19PM +0800, Xingyu Wu wrote: >> On 2023/5/12 21:49, Conor Dooley wrote: >> > On Fri, May 12, 2023 at 05:56:16PM +0800, Xingyu Wu wrote: >> >> On 2023/5/12 17:35, Conor Dooley wrote: >> >> > On Fri, May 12, 2023 at 04:07:47PM +0800, Xingyu Wu wrote: >> >> >> On 2023/5/12 14:47, Conor Dooley wrote: >> >> >> > On Fri, May 12, 2023 at 10:20:32AM +0800, Xingyu Wu wrote: >> >> >> >> Add PLL clock inputs from PLL clock generator. >> >> >> >> >> >> >> >> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> >> >> >> Signed-off-by: Xingyu Wu <xingyu.wu@starfivetech.com> >> >> >> >> --- >> >> >> >> .../clock/starfive,jh7110-syscrg.yaml | 20 +++++++++++++++++-- >> >> >> >> 1 file changed, 18 insertions(+), 2 deletions(-) >> >> >> > >> >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: >> >> >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short >> >> >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.3b.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: >> >> >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short >> >> >> > 'i2stx_bclk_ext' was expected >> >> >> > 'i2stx_lrck_ext' was expected >> >> >> > 'i2srx_bclk_ext' was expected >> >> >> > 'i2srx_lrck_ext' was expected >> >> >> > 'tdm_ext' was expected >> >> >> > 'mclk_ext' was expected >> >> >> > 'pll0_out' was expected >> >> >> > From schema: /Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clocks: 'oneOf' conditional failed, one must be fixed: >> >> >> > [[19], [20], [21], [22], [23], [24], [25], [26], [27]] is too short >> >> >> > From schema: Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> >> > /tmp/tmp.KDlzwQM5ma/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2-v1.2a.dtb: clock-controller@13020000: clock-names: 'oneOf' conditional failed, one must be fixed: >> >> >> > ['osc', 'gmac1_rmii_refin', 'gmac1_rgmii_rxin', 'i2stx_bclk_ext', 'i2stx_lrck_ext', 'i2srx_bclk_ext', 'i2srx_lrck_ext', 'tdm_ext', 'mclk_ext'] is too short >> >> >> > 'i2stx_bclk_ext' was expected >> >> >> > 'i2stx_lrck_ext' was expected >> >> >> > 'i2srx_bclk_ext' was expected >> >> >> > 'i2srx_lrck_ext' was expected >> >> >> > 'tdm_ext' was expected >> >> >> > 'mclk_ext' was expected >> >> >> > 'pll0_out' was expected >> >> >> > Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml >> >> >> > >> >> >> > This binding change is incompatible with the existing devicetrees for >> >> >> > the visionfive 2. >> >> >> >> >> >> This looks like less clocks about PLL in SYSCRG node. And I add this in patch 7. >> >> > >> >> > The existing devicetree is a valid, albeit limited, description of the >> >> > hardware. >> >> > After your changes to the clock driver in this series, but *without* the >> >> > changes to the devicetrees, does the system still function? >> >> > From a quick check of 4/7, it looks like it will not? >> >> >> >> I just tested it on the board and the system still worked without the changes >> >> about devicetree. But these clocks' rate were 0 because these could not get >> >> the PLL clocks from devicetree. >> > >> > Hmm, that sounds like an issue to me. If all of the clock rates are >> > computed based off of parents that incorrectly report 0, are we not in >> > for trouble? >> > Should the fixed-factor clocks be retained as a fallback for the sake of >> > compatibility? >> > Emil, Stephen? >> >> I got your concern. Actually, I can add a check in driver to see if the dts >> has pll clocks and then decide whether to use fixed-factor clocks or pll clocks >> from syscon. But eventually we have to use pll clocks and dts has to add it. >> Then the binding should add it synchronously, right? > > IMO, it is okay to change the bindings to only allow the "correct" > representation of the clock tree, but the driver should fall back to the > fixed factor clocks if it detects the old/limited configuration. > Great, I will follow it. Best regards, Xingyu Wu
diff --git a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml index 84373ae31644..fcb363353050 100644 --- a/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml +++ b/Documentation/devicetree/bindings/clock/starfive,jh7110-syscrg.yaml @@ -27,6 +27,9 @@ properties: - description: External I2S RX left/right channel clock - description: External TDM clock - description: External audio master clock + - description: PLL0 + - description: PLL1 + - description: PLL2 - items: - description: Main Oscillator (24 MHz) @@ -38,6 +41,9 @@ properties: - description: External I2S RX left/right channel clock - description: External TDM clock - description: External audio master clock + - description: PLL0 + - description: PLL1 + - description: PLL2 clock-names: oneOf: @@ -52,6 +58,9 @@ properties: - const: i2srx_lrck_ext - const: tdm_ext - const: mclk_ext + - const: pll0_out + - const: pll1_out + - const: pll2_out - items: - const: osc @@ -63,6 +72,9 @@ properties: - const: i2srx_lrck_ext - const: tdm_ext - const: mclk_ext + - const: pll0_out + - const: pll1_out + - const: pll2_out '#clock-cells': const: 1 @@ -93,12 +105,16 @@ examples: <&gmac1_rgmii_rxin>, <&i2stx_bclk_ext>, <&i2stx_lrck_ext>, <&i2srx_bclk_ext>, <&i2srx_lrck_ext>, - <&tdm_ext>, <&mclk_ext>; + <&tdm_ext>, <&mclk_ext>, + <&pllclk 0>, + <&pllclk 1>, + <&pllclk 2>; clock-names = "osc", "gmac1_rmii_refin", "gmac1_rgmii_rxin", "i2stx_bclk_ext", "i2stx_lrck_ext", "i2srx_bclk_ext", "i2srx_lrck_ext", - "tdm_ext", "mclk_ext"; + "tdm_ext", "mclk_ext", + "pll0_out", "pll1_out", "pll2_out"; #clock-cells = <1>; #reset-cells = <1>; };