Re: [PATCH 11/17] clk: sunxi-ng: add support for Allwinner R329 CCU

On Thu, Aug 19, 2021 at 09:41:26PM -0500, Samuel Holland wrote:

On 8/2/21 1:22 AM, Icenowy Zheng wrote:

quoted

Allwinner R329 has a CCU that is similar to the H616 one, but it's cut
down and have PLLs moved out.

Add support for it.

Signed-off-by: Icenowy Zheng <redacted>
---
 drivers/clk/sunxi-ng/Kconfig                |   5 +
 drivers/clk/sunxi-ng/Makefile               |   1 +
 drivers/clk/sunxi-ng/ccu-sun50i-r329.c      | 526 ++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu-sun50i-r329.h      |  32 ++
 include/dt-bindings/clock/sun50i-r329-ccu.h |  73 +++
 include/dt-bindings/reset/sun50i-r329-ccu.h |  45 ++
 6 files changed, 682 insertions(+)
 create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-r329.c
 create mode 100644 drivers/clk/sunxi-ng/ccu-sun50i-r329.h
 create mode 100644 include/dt-bindings/clock/sun50i-r329-ccu.h
 create mode 100644 include/dt-bindings/reset/sun50i-r329-ccu.h

diff --git a/drivers/clk/sunxi-ng/Kconfig b/drivers/clk/sunxi-ng/Kconfig
index e49b2c2fa5b7..4b32d5f81ea8 100644
--- a/drivers/clk/sunxi-ng/Kconfig
+++ b/drivers/clk/sunxi-ng/Kconfig

@@ -42,6 +42,11 @@ config SUN50I_H6_R_CCU
 	default ARM64 && ARCH_SUNXI
 	depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST
 
+config SUN50I_R329_CCU
+	bool "Support for the Allwinner R329 CCU"
+	default ARM64 && ARCH_SUNXI
+	depends on (ARM64 && ARCH_SUNXI) || COMPILE_TEST
+
 config SUN50I_R329_R_CCU
 	bool "Support for the Allwinner R329 PRCM CCU"
 	default ARM64 && ARCH_SUNXI

diff --git a/drivers/clk/sunxi-ng/Makefile b/drivers/clk/sunxi-ng/Makefile
index db338a2188fd..62f3c5bf331c 100644
--- a/drivers/clk/sunxi-ng/Makefile
+++ b/drivers/clk/sunxi-ng/Makefile

@@ -28,6 +28,7 @@ obj-$(CONFIG_SUN50I_A100_R_CCU)	+= ccu-sun50i-a100-r.o
 obj-$(CONFIG_SUN50I_H6_CCU)	+= ccu-sun50i-h6.o
 obj-$(CONFIG_SUN50I_H616_CCU)	+= ccu-sun50i-h616.o
 obj-$(CONFIG_SUN50I_H6_R_CCU)	+= ccu-sun50i-h6-r.o
+obj-$(CONFIG_SUN50I_R329_CCU)	+= ccu-sun50i-r329.o
 obj-$(CONFIG_SUN50I_R329_R_CCU)	+= ccu-sun50i-r329-r.o
 obj-$(CONFIG_SUN4I_A10_CCU)	+= ccu-sun4i-a10.o
 obj-$(CONFIG_SUN5I_CCU)		+= ccu-sun5i.o

diff --git a/drivers/clk/sunxi-ng/ccu-sun50i-r329.c b/drivers/clk/sunxi-ng/ccu-sun50i-r329.c
new file mode 100644
index 000000000000..a0b4cfd6e1db
--- /dev/null
+++ b/drivers/clk/sunxi-ng/ccu-sun50i-r329.c

@@ -0,0 +1,526 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Based on the H616 CCU driver, which is:
+ *   Copyright (c) 2020 Arm Ltd.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+
+#include "ccu_common.h"
+#include "ccu_reset.h"
+
+#include "ccu_div.h"
+#include "ccu_gate.h"
+#include "ccu_mp.h"
+#include "ccu_mult.h"
+#include "ccu_nk.h"
+#include "ccu_nkm.h"
+#include "ccu_nkmp.h"
+#include "ccu_nm.h"
+
+#include "ccu-sun50i-r329.h"
+
+/*
+ * An external divider of PLL-CPUX is controlled here. As it's similar to
+ * the external divider of PLL-CPUX on previous SoCs (only usable under
+ * 288MHz}, ignore it.

Mismatched (braces} here

quoted

+ */
+static const char * const cpux_parents[] = { "osc24M", "osc32k", "iosc",
+					     "pll-cpux", "pll-periph",
+					     "pll-periph-2x",
+					     "pll=periph-800m" };

= should be a -.

Now that these PLLs are in a different device, how is this supposed to affect
the DT binding? Do we put all of them in the clocks property?

If so, we can use .fw_name at some point. If not, why bother with the clocks
property at all? This is another part of the "let's get the clock tree right
from the start" discussion.

Agreed

Maxime