Re: [PATCH 2/3] clk: thead: add support for the T-HEAD TH1520 CCU

From: Drew Fustini <dfustini@baylibre.com>
To: Jisheng Zhang <jszhang@kernel.org>
Cc: Yangtao Li <frank.li@vivo.com>,
	devicetree@vger.kernel.org, Conor Dooley <conor+dt@kernel.org>,
	sboyd@kernel.org, mturquette@baylibre.com,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	linux-riscv@lists.infradead.org, linux-clk@vger.kernel.org,
	Wei Fu <wefu@redhat.com>
Subject: Re: [PATCH 2/3] clk: thead: add support for the T-HEAD TH1520 CCU
Date: Sun, 3 Sep 2023 15:37:30 -0700	[thread overview]
Message-ID: <ZPUKqsEglxC5+Ocb@x1> (raw)
In-Reply-To: <ZOsnYagqFz6atfw2@xhacker>

On Sun, Aug 27, 2023 at 06:37:21PM +0800, Jisheng Zhang wrote:
> On Mon, May 15, 2023 at 01:44:00PM +0800, Yangtao Li wrote:
> > Add support for th1520 in the clock framework.
> 
> Hi Yangtao,
> 
> I didn't see any new version of this series in this development window,
> and we have already talked about it several times on wechat, so do
> you still plan to renew this series? If not, I will update it and may
> rewrite the code once next development window is open.
> 
> Thanks in advance

I tried this clock driver on the BeagleV Ahead board and I got the
following during boot [1]:

  __clk_core_init: ahb2 must implement .set_parent & .determine_rate
  th1520-clk: probe of ffef010000.clock-controller failed with error -22

I am guessing this has to do with how ahb2_clk is defined?

 432 static struct ccu_div ahb2_clk = {
 433         .div            = TH_CCU_DIV_FLAGS(0, 3, CLK_DIVIDER_ONE_BASED),
 434         .mux            = TH_CCU_ARG(5, 1),
 435         .common         = {
 436                 .reg            = 0x120,                      
 437                 .hw.init        = CLK_HW_INIT_PARENTS("ahb2",
 438                                                       ahb2_parents,
 439                                                       &ccu_div_ops,
 440                                                       0),
 441         },
 442 };

It passes ccu_div_ops in CLK_HW_INIT_PARENTS(). ccu_div_ops does
implement .set_parent but it doesn't implement .determine_rate. Maybe
that is the problem?

Yangtao or Jisheng: would you be able to share the device tree you're
using?

I added this based on the example in the device tree binding:
=============================================================

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 18761a06c866..8153bf73e8c0 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -313,6 +313,14 @@ dmac0: dma-controller@ffefc00000 {
                        status = "disabled";
                };
 
+               clk: clock-controller@ffef010000 {
+                       compatible = "thead,th1520-ccu";
+                       reg = <0xff 0xef010000 0x0 0x1000>;
+                       clocks = <&osc_32k>, <&osc>;
+                       clock-names = "losc", "hosc";
+                       #clock-cells = <1>;
+               };
+
                mmc0: mmc@ffe7080000 {
                        compatible = "thead,th1520-dwcmshc";
                        reg = <0xff 0xe7080000 0x0 0x10000

=============================================================

Thank you,
Drew

[1] https://gist.github.com/pdp7/b585a6dad5134fc8773e53ad48600fef
