On 2020/12/17 19:17, Stephen Boyd wrote:

Quoting Damien Le Moal (2020-12-17 00:13:57)

quoted

On 2020/12/17 17:10, Stephen Boyd wrote:

quoted

Quoting Damien Le Moal (2020-12-13 05:50:42)

quoted

diff --git a/include/dt-bindings/clock/k210-clk.h b/include/dt-bindings/clock/k210-clk.h
index 5a2fd64d1a49..b2de702cbf75 100644
--- a/include/dt-bindings/clock/k210-clk.h
+++ b/include/dt-bindings/clock/k210-clk.h

@@ -3,18 +3,51 @@
  * Copyright (C) 2019-20 Sean Anderson <seanga2@gmail.com>
  * Copyright (c) 2020 Western Digital Corporation or its affiliates.
  */
-#ifndef K210_CLK_H
-#define K210_CLK_H
+#ifndef CLOCK_K210_CLK_H
+#define CLOCK_K210_CLK_H
 
 /*
- * Arbitrary identifiers for clocks.
- * The structure is: in0 -> pll0 -> aclk -> cpu
- *
- * Since we use the hardware defaults for now, set all these to the same clock.
+ * Kendryte K210 SoC clock identifiers (arbitrary values).
  */
-#define K210_CLK_PLL0   0
-#define K210_CLK_PLL1   0
-#define K210_CLK_ACLK   0
-#define K210_CLK_CPU    0

This seems to open a bisection hole. I see that ACLK is used in the
existing dtsi file, and that is the same as CLK_CPU, but after this
patch it will change to not exist anymore. Can we leave ACLK around
defined to be 0? I imagine it won't be used in the future so we can
remove it later. I can then apply this for v5.11-rc1 and then merge the
clk driver patch in clk tree.

quoted

+#define K210_CLK_CPU   0
+#define K210_CLK_SRAM0 1
+#define K210_CLK_SRAM1 2

Patch 6 of the series removes the use of K210_CLK_CPU and K210_CLK_ACLK from the
device trees. I added that patch as the DT modification proper comes only at
patch 16. Maybe I should squash patch 6 into this one ?

Preferably the defines are just left alone forever and then forgotten.
The dt-bindings directory is almost ABI and so changing numbers or
removing defines is hard to do. Usually patches in this directory are an
additive thing.

I just did that. It works.

Ideally, patches 7, 8 and 9 need to go in together with the clk driver patch.
Since the builtin DTB patch precedes the clock driver patch that touches the
sysctl driver, I need to rework it a little, keeping the
SOC_DECLARE_BUILTIN_DTB() for now. And finally, a small DTS update patch needs
to be added too for the sysctl & sysclk nodes update. That would make it a 5
patch series for the clock driver addition. Would this work ?

Or, you just take patch 9 (clk doc) and patch 13 (clk driver), slightly modified
to move the sysctl register definitions into a common header (currently part of
patch 7). 2 patches only, without any other change, resulting in the clock
driver not being used until the rest of the series goes into 5.12. Do you prefer
that solution ?


-- 
Damien Le Moal
Western Digital Research