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 004DCC77B7F for ; Fri, 19 May 2023 08:02:15 +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: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=7b5M12y1wHvYGWAJzPd5BSDWpNo0ocSIv2eyYZObX9g=; b=tbjwVQ41vfLcqP qa1dYlhQ80Ch82zW+4UP9rJZjMYqLjliKyt4Pk7uNmgk+JkRosXzNg+6O3DUeaHAXCghiwhrPasTi IQgG1ZU9xYd7PPd/357tZXzs8vyXTF0NMlQJyFHUtL4BJTsYjsBi8S5zZY3C48fksMFmqzoAZV5mq Dpmv3VtMBxlPIioWnfXyTNfZ8Rn1WBWTc9W1nWra3iXd6S17QdtEsLxpjZILdqqe+z+MAxUG6mcy9 MxWrqaWXcYHijySc5lJ0DJ/arNEPnbvZ0JbDJE615BYxIpF3wXKQdL/aF1jXMz6kF7G3TtVoauFmt jjPA3qIz+ryAFsjZOvfg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pzv3r-00FRtX-2C; Fri, 19 May 2023 08:02:03 +0000 Received: from desiato.infradead.org ([90.155.92.199]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1pzv3p-00FRt3-38 for linux-riscv@bombadil.infradead.org; Fri, 19 May 2023 08:02:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Content-Transfer-Encoding:Content-Type :In-Reply-To:From:References:CC:To:Subject:MIME-Version:Date:Message-ID: Sender:Reply-To:Content-ID:Content-Description; bh=qkwyrQSJurhsumPh+Stw0XP8g8shGm4lTnxbuj3cbRQ=; b=UshzyB+pW4bGs7A1Vh1zUKCGyB 7Hyn01kpxIVeEY7JMOw6XlICICVDMPu7u1pHPvkQ4pq1ACm/dvSnISR7GT8iN2iYCobXNysCcDlyt EKk3jEhpQOpueyYpArZGGNdEliqag1vxmsgQZwRRlPC2qIj7jVOvXYKbfs9Kcm5lt9oLGxkc1+DuY zSV68iUCs47ZFXP/7zHZ6V7/SseIN05Ekm5chQY0rt5148U12wqWLMA9dfGyQx3YyEIci0louTa+W IWHDUyRpRjCRiEViuvd0i2uNtWdpC3kQnsOVdjH/rLp8lBl72HMAyamjUyGcsKXib2svkvx5m7f00 5ZQnHL7w==; Received: from fd01.gateway.ufhost.com ([61.152.239.71]) by desiato.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1pzv3j-00Fp1y-0z for linux-riscv@lists.infradead.org; Fri, 19 May 2023 08:01:59 +0000 Received: from EXMBX166.cuchost.com (unknown [175.102.18.54]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "EXMBX166", Issuer "EXMBX166" (not verified)) by fd01.gateway.ufhost.com (Postfix) with ESMTP id AEF6B24DB84; Fri, 19 May 2023 16:01:03 +0800 (CST) Received: from EXMBX061.cuchost.com (172.16.6.61) by EXMBX166.cuchost.com (172.16.6.76) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Fri, 19 May 2023 16:01:03 +0800 Received: from [192.168.125.131] (113.72.146.100) by EXMBX061.cuchost.com (172.16.6.61) with Microsoft SMTP Server (TLS) id 15.0.1497.42; Fri, 19 May 2023 16:01:02 +0800 Message-ID: <906cec55-e438-0eca-618c-4f29b2642fcb@starfivetech.com> Date: Fri, 19 May 2023 15:59:19 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v4 3/7] dt-bindings: clock: jh7110-syscrg: Add PLL clock inputs Content-Language: en-US To: Conor Dooley CC: , , "Michael Turquette" , Stephen Boyd , Krzysztof Kozlowski , Philipp Zabel , Conor Dooley , "Emil Renner Berthing" , Rob Herring , Paul Walmsley , Palmer Dabbelt , Albert Ou , Hal Feng , William Qiu , , References: <20230512022036.97987-1-xingyu.wu@starfivetech.com> <20230512022036.97987-4-xingyu.wu@starfivetech.com> <20230512-uproar-external-49a9e793fbc4@wendy> <91e4fd3c-20cb-724b-c9a8-e038600aabb7@starfivetech.com> <20230512-backlit-radiated-ded0b38b4a94@wendy> <20230512-traffic-popsicle-5c3423b37fab@wendy> From: Xingyu Wu In-Reply-To: <20230512-traffic-popsicle-5c3423b37fab@wendy> X-Originating-IP: [113.72.146.100] X-ClientProxiedBy: EXCAS061.cuchost.com (172.16.6.21) To EXMBX061.cuchost.com (172.16.6.61) X-YovoleRuleAgent: yovoleflag X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230519_090156_213654_A347109E X-CRM114-Status: GOOD ( 21.54 ) X-BeenThere: linux-riscv@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-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org 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 >> >> >> Signed-off-by: Xingyu Wu >> >> >> --- >> >> >> .../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 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv