[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230512-traffic-popsicle-5c3423b37fab@wendy>
Date: Fri, 12 May 2023 14:49:27 +0100
From: Conor Dooley <conor.dooley@...rochip.com>
To: Xingyu Wu <xingyu.wu@...rfivetech.com>
CC: <linux-riscv@...ts.infradead.org>, <devicetree@...r.kernel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Philipp Zabel <p.zabel@...gutronix.de>,
Conor Dooley <conor@...nel.org>,
Emil Renner Berthing <kernel@...il.dk>,
Rob Herring <robh+dt@...nel.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Albert Ou <aou@...s.berkeley.edu>,
Hal Feng <hal.feng@...rfivetech.com>,
William Qiu <william.qiu@...rfivetech.com>,
<linux-kernel@...r.kernel.org>, <linux-clk@...r.kernel.org>
Subject: Re: [PATCH v4 3/7] dt-bindings: clock: jh7110-syscrg: Add PLL clock
inputs
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@...aro.org>
> >> >> Signed-off-by: Xingyu Wu <xingyu.wu@...rfivetech.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@...20000: 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@...20000: 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@...20000: 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@...20000: 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?
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists