[RFC PATCH 0/5] clk: bcm: kona: Add bus clock support and prerequisite clocks

[RFC PATCH 0/5] clk: bcm: kona: Add bus clock support and prerequisite clocks

[Artur Weber]

This patchset does the following:

- Introduce support for bus clocks. These are fairly similar to
  peripheral clocks, but only implement policy, gate and hyst.

- Introduce support for prerequisite clocks; this way we can
  make peripheral clocks automatically enable their corresponding
  bus clocks.

- Add matching bus clocks for BCM21664 peripheral clocks and update
  device tree bindings to match.

The prerequisite clock portion of this patchset is adapted from an
older attempt to add bus clocks[1], submitted by Alex Elder. I've
retained his authorship on that commit.

Notably, Alex's patchset moved clock initialization to the prepare
function. This seems to be incorrect; the prepare function gets called
before the enable function, but not before "set rate"/"set parent"
functions; thus, while clocks enabled fine, any configuration done
before they were first enabled was broken. I ignored that part of
the patchset and only kept the prerequisite clocks.

I would appreciate feedback on the prerequisite clock patch, hence
why this patchset is marked as RFC.

I wasn't able to find any other driver that does something like this,
so I'm not sure if it's correct (especially since I had to switch from
non-locking __clk_prepare and __clk_enable to the regular locking
versions, as the non-locking versions are no longer public - they
appear to have been replaced by clk_core counterparts, but those
functions are not exported anywhere AFAICT).

An alternative way to do this dependency would be to wrap every
component with a relevant bus clock in a "simple-pm-bus" node
with the bus clock in DT, but this seems rather unwieldy.

[1] https://lore.kernel.org/lkml/1402926007-4436-1-git-send-email-elder@linaro.org/

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
Alex Elder (1):
      clk: bcm281xx: implement prerequisite clocks

Artur Weber (4):
      dt-bindings: clock: brcm,kona-ccu: Add BCM21664 bus clocks
      clk: bcm: kona: Add support for bus clocks
      clk: bcm21664: Add matching bus clocks for peripheral clocks
      ARM: dts: bcm2166x-common: Add matching bus clocks for peripheral clocks

 .../devicetree/bindings/clock/brcm,kona-ccu.yaml   |  18 ++-
 arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi    |  28 +++--
 drivers/clk/bcm/clk-bcm21664.c                     | 107 ++++++++++++++++--
 drivers/clk/bcm/clk-kona-setup.c                   | 116 +++++++++++++++++++
 drivers/clk/bcm/clk-kona.c                         | 124 ++++++++++++++++++++-
 drivers/clk/bcm/clk-kona.h                         |  30 ++++-
 include/dt-bindings/clock/bcm21664.h               |  19 +++-
 7 files changed, 411 insertions(+), 31 deletions(-)
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250212-kona-bus-clock-4297eefae940

Best regards,
-- 
Artur Weber <aweber.kernel@gmail.com>

[PATCH RFC 1/5] dt-bindings: clock: brcm,kona-ccu: Add BCM21664 bus clocks

[Artur Weber]

Add bus clocks for hub_timer, sdio, bsc, uart and usb_otg_ahb to the
allowed clock output names for BCM21664 CCUs.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 .../devicetree/bindings/clock/brcm,kona-ccu.yaml       | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml b/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml
index e5656950b3bd0ad44ba47f0ada84b558e71df590..dff04e24e92829b890bf7cd336f0e083bdb30fa6 100644
--- a/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml
+++ b/Documentation/devicetree/bindings/clock/brcm,kona-ccu.yaml
@@ -40,7 +40,7 @@ properties:
 
   clock-output-names:
     minItems: 1
-    maxItems: 10
+    maxItems: 14
 
 required:
   - compatible
@@ -124,7 +124,9 @@ allOf:
     then:
       properties:
         clock-output-names:
-          const: hub_timer
+          items:
+            - const: hub_timer
+            - const: hub_timer_apb
   - if:
       properties:
         compatible:
@@ -142,6 +144,11 @@ allOf:
             - const: sdio2_sleep
             - const: sdio3_sleep
             - const: sdio4_sleep
+            - const: sdio1_ahb
+            - const: sdio2_ahb
+            - const: sdio3_ahb
+            - const: sdio4_ahb
+            - const: usb_otg_ahb
   - if:
       properties:
         compatible:
@@ -158,6 +165,13 @@ allOf:
             - const: bsc2
             - const: bsc3
             - const: bsc4
+            - const: uartb_apb
+            - const: uartb2_apb
+            - const: uartb3_apb
+            - const: bsc1_apb
+            - const: bsc2_apb
+            - const: bsc3_apb
+            - const: bsc4_apb
 
 additionalProperties: false
 

-- 
2.48.1

Re: [PATCH RFC 1/5] dt-bindings: clock: brcm,kona-ccu: Add BCM21664 bus clocks

[Rob Herring (Arm)]

On Sun, 16 Feb 2025 17:12:36 +0100, Artur Weber wrote:
> Add bus clocks for hub_timer, sdio, bsc, uart and usb_otg_ahb to the
> allowed clock output names for BCM21664 CCUs.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
>  .../devicetree/bindings/clock/brcm,kona-ccu.yaml       | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 

Acked-by: Rob Herring (Arm) <robh@kernel.org>

[PATCH RFC 2/5] clk: bcm: kona: Add support for bus clocks

[Artur Weber]

Introduce support for bus clocks into the Broadcom Kona common clock
driver. Most of these functions have been adapted from their peripheral
clock counterparts, as they are nearly identical (bus clocks are just
much more limited in terms of allowed operations).

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 drivers/clk/bcm/clk-kona-setup.c | 116 +++++++++++++++++++++++++++++++++++++++
 drivers/clk/bcm/clk-kona.c       |  62 ++++++++++++++++++++-
 drivers/clk/bcm/clk-kona.h       |  10 ++++
 3 files changed, 187 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/bcm/clk-kona-setup.c b/drivers/clk/bcm/clk-kona-setup.c
index 338558f6fbaec27418497fc246de17e15236ca0d..8b6123e188fbc035cda2cf394035d35197671d06 100644
--- a/drivers/clk/bcm/clk-kona-setup.c
+++ b/drivers/clk/bcm/clk-kona-setup.c
@@ -167,6 +167,58 @@ static bool peri_clk_data_offsets_valid(struct kona_clk *bcm_clk)
 	return true;
 }
 
+static bool bus_clk_data_offsets_valid(struct kona_clk *bcm_clk)
+{
+	struct bus_clk_data *bus;
+	struct bcm_clk_policy *policy;
+	struct bcm_clk_gate *gate;
+	struct bcm_clk_hyst *hyst;
+	const char *name;
+	u32 range;
+	u32 limit;
+
+	BUG_ON(bcm_clk->type != bcm_clk_bus);
+	bus = bcm_clk->u.bus;
+	name = bcm_clk->init_data.name;
+	range = bcm_clk->ccu->range;
+
+	limit = range - sizeof(u32);
+	limit = round_down(limit, sizeof(u32));
+
+	policy = &bus->policy;
+	if (policy_exists(policy)) {
+		if (policy->offset > limit) {
+			pr_err("%s: bad policy offset for %s (%u > %u)\n",
+				__func__, name, policy->offset, limit);
+			return false;
+		}
+	}
+
+	gate = &bus->gate;
+	hyst = &bus->hyst;
+	if (gate_exists(gate)) {
+		if (gate->offset > limit) {
+			pr_err("%s: bad gate offset for %s (%u > %u)\n",
+				__func__, name, gate->offset, limit);
+			return false;
+		}
+
+		if (hyst_exists(hyst)) {
+			if (hyst->offset > limit) {
+				pr_err("%s: bad hysteresis offset for %s "
+					"(%u > %u)\n", __func__,
+					name, hyst->offset, limit);
+				return false;
+			}
+		}
+	} else if (hyst_exists(hyst)) {
+		pr_err("%s: hysteresis but no gate for %s\n", __func__, name);
+		return false;
+	}
+
+	return true;
+}
+
 /* A bit position must be less than the number of bits in a 32-bit register. */
 static bool bit_posn_valid(u32 bit_posn, const char *field_name,
 			const char *clock_name)
@@ -481,9 +533,46 @@ peri_clk_data_valid(struct kona_clk *bcm_clk)
 	return kona_dividers_valid(bcm_clk);
 }
 
+/* Determine whether the set of bus clock registers are valid. */
+static bool
+bus_clk_data_valid(struct kona_clk *bcm_clk)
+{
+	struct bus_clk_data *bus;
+	struct bcm_clk_policy *policy;
+	struct bcm_clk_gate *gate;
+	struct bcm_clk_hyst *hyst;
+	const char *name;
+
+	BUG_ON(bcm_clk->type != bcm_clk_bus);
+
+	if (!bus_clk_data_offsets_valid(bcm_clk))
+		return false;
+
+	bus = bcm_clk->u.bus;
+	name = bcm_clk->init_data.name;
+
+	policy = &bus->policy;
+	if (policy_exists(policy) && !policy_valid(policy, name))
+		return false;
+
+	gate = &bus->gate;
+	if (gate_exists(gate) && !gate_valid(gate, "gate", name))
+		return false;
+
+	hyst = &bus->hyst;
+	if (hyst_exists(hyst) && !hyst_valid(hyst, name))
+		return false;
+
+	return true;
+}
+
 static bool kona_clk_valid(struct kona_clk *bcm_clk)
 {
 	switch (bcm_clk->type) {
+	case bcm_clk_bus:
+		if (!bus_clk_data_valid(bcm_clk))
+			return false;
+		break;
 	case bcm_clk_peri:
 		if (!peri_clk_data_valid(bcm_clk))
 			return false;
@@ -656,6 +745,14 @@ static void peri_clk_teardown(struct peri_clk_data *data,
 	clk_sel_teardown(&data->sel, init_data);
 }
 
+static void bus_clk_teardown(struct bus_clk_data *data,
+				struct clk_init_data *init_data)
+{
+	init_data->num_parents = 0;
+	kfree(init_data->parent_names);
+	init_data->parent_names = NULL;
+}
+
 /*
  * Caller is responsible for freeing the parent_names[] and
  * parent_sel[] arrays in the peripheral clock's "data" structure
@@ -670,9 +767,23 @@ peri_clk_setup(struct peri_clk_data *data, struct clk_init_data *init_data)
 	return clk_sel_setup(data->clocks, &data->sel, init_data);
 }
 
+static int
+bus_clk_setup(struct bus_clk_data *data, struct clk_init_data *init_data)
+{
+	init_data->flags = CLK_IGNORE_UNUSED;
+
+	init_data->parent_names = NULL;
+	init_data->num_parents = 0;
+
+	return 0;
+}
+
 static void bcm_clk_teardown(struct kona_clk *bcm_clk)
 {
 	switch (bcm_clk->type) {
+	case bcm_clk_bus:
+		bus_clk_teardown(bcm_clk->u.data, &bcm_clk->init_data);
+		break;
 	case bcm_clk_peri:
 		peri_clk_teardown(bcm_clk->u.data, &bcm_clk->init_data);
 		break;
@@ -702,6 +813,11 @@ static int kona_clk_setup(struct kona_clk *bcm_clk)
 	struct clk_init_data *init_data = &bcm_clk->init_data;
 
 	switch (bcm_clk->type) {
+	case bcm_clk_bus:
+		ret = bus_clk_setup(bcm_clk->u.data, init_data);
+		if (ret)
+			return ret;
+		break;
 	case bcm_clk_peri:
 		ret = peri_clk_setup(bcm_clk->u.data, init_data);
 		if (ret)
diff --git a/drivers/clk/bcm/clk-kona.c b/drivers/clk/bcm/clk-kona.c
index ec5749e301ba82933144f34acfaf6f3680c443f7..e92d57f3bbb147e72221802175a80502897d7504 100644
--- a/drivers/clk/bcm/clk-kona.c
+++ b/drivers/clk/bcm/clk-kona.c
@@ -961,7 +961,7 @@ static int selector_write(struct ccu_data *ccu, struct bcm_clk_gate *gate,
 	return ret;
 }
 
-/* Clock operations */
+/* Peripheral clock operations */
 
 static int kona_peri_clk_enable(struct clk_hw *hw)
 {
@@ -1233,9 +1233,69 @@ static bool __peri_clk_init(struct kona_clk *bcm_clk)
 	return true;
 }
 
+/* Bus clock operations */
+
+static int kona_bus_clk_enable(struct clk_hw *hw)
+{
+	struct kona_clk *bcm_clk = to_kona_clk(hw);
+	struct bcm_clk_gate *gate = &bcm_clk->u.bus->gate;
+
+	return clk_gate(bcm_clk->ccu, bcm_clk->init_data.name, gate, true);
+}
+
+static void kona_bus_clk_disable(struct clk_hw *hw)
+{
+	struct kona_clk *bcm_clk = to_kona_clk(hw);
+	struct bcm_clk_gate *gate = &bcm_clk->u.bus->gate;
+
+	(void)clk_gate(bcm_clk->ccu, bcm_clk->init_data.name, gate, false);
+}
+
+static int kona_bus_clk_is_enabled(struct clk_hw *hw)
+{
+	struct kona_clk *bcm_clk = to_kona_clk(hw);
+	struct bcm_clk_gate *gate = &bcm_clk->u.bus->gate;
+
+	return is_clk_gate_enabled(bcm_clk->ccu, gate) ? 1 : 0;
+}
+
+struct clk_ops kona_bus_clk_ops = {
+	.enable = kona_bus_clk_enable,
+	.disable = kona_bus_clk_disable,
+	.is_enabled = kona_bus_clk_is_enabled,
+};
+
+/* Put a bus clock into its initial state */
+static bool __bus_clk_init(struct kona_clk *bcm_clk)
+{
+	struct ccu_data *ccu = bcm_clk->ccu;
+	struct bus_clk_data *bus = bcm_clk->u.bus;
+	const char *name = bcm_clk->init_data.name;
+
+	BUG_ON(bcm_clk->type != bcm_clk_bus);
+
+	if (!policy_init(ccu, &bus->policy)) {
+		pr_err("%s: error initializing policy for %s\n",
+			__func__, name);
+		return false;
+	}
+	if (!gate_init(ccu, &bus->gate)) {
+		pr_err("%s: error initializing gate for %s\n", __func__, name);
+		return false;
+	}
+	if (!hyst_init(ccu, &bus->hyst)) {
+		pr_err("%s: error initializing hyst for %s\n", __func__, name);
+		return false;
+	}
+
+	return true;
+}
+
 static bool __kona_clk_init(struct kona_clk *bcm_clk)
 {
 	switch (bcm_clk->type) {
+	case bcm_clk_bus:
+		return __bus_clk_init(bcm_clk);
 	case bcm_clk_peri:
 		return __peri_clk_init(bcm_clk);
 	default:
diff --git a/drivers/clk/bcm/clk-kona.h b/drivers/clk/bcm/clk-kona.h
index e09655024ac2ad42538b924f304b23e87b7db2ce..a5b3d8bdb54eaee9fad80c28796170207b817dfd 100644
--- a/drivers/clk/bcm/clk-kona.h
+++ b/drivers/clk/bcm/clk-kona.h
@@ -390,6 +390,14 @@ struct peri_clk_data {
 	struct bcm_clk_sel sel;
 	const char *clocks[];	/* must be last; use CLOCKS() to declare */
 };
+
+struct bus_clk_data {
+	struct bcm_clk_policy policy;
+	struct bcm_clk_gate gate;
+	struct bcm_clk_hyst hyst;
+	const char *clocks[];	/* must be last; use CLOCKS() to declare */
+};
+
 #define CLOCKS(...)	{ __VA_ARGS__, NULL, }
 #define NO_CLOCKS	{ NULL, }	/* Must use of no parent clocks */
 
@@ -401,6 +409,7 @@ struct kona_clk {
 	union {
 		void *data;
 		struct peri_clk_data *peri;
+		struct bus_clk_data *bus;
 	} u;
 };
 #define to_kona_clk(_hw) \
@@ -488,6 +497,7 @@ struct ccu_data {
 /* Exported globals */
 
 extern struct clk_ops kona_peri_clk_ops;
+extern struct clk_ops kona_bus_clk_ops;
 
 /* Externally visible functions */
 

-- 
2.48.1

Re: [PATCH RFC 2/5] clk: bcm: kona: Add support for bus clocks

[Alex Elder]

On 2/16/25 10:12 AM, Artur Weber wrote:
> Introduce support for bus clocks into the Broadcom Kona common clock
> driver. Most of these functions have been adapted from their peripheral
> clock counterparts, as they are nearly identical (bus clocks are just
> much more limited in terms of allowed operations).
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>

Seems like this was easy to add.  This looks good to me.

Reviewed-by: Alex Elder <elder@riscstar.com>

> ---
>   drivers/clk/bcm/clk-kona-setup.c | 116 +++++++++++++++++++++++++++++++++++++++
>   drivers/clk/bcm/clk-kona.c       |  62 ++++++++++++++++++++-
>   drivers/clk/bcm/clk-kona.h       |  10 ++++
>   3 files changed, 187 insertions(+), 1 deletion(-)
> 
. . .

[PATCH RFC 3/5] clk: bcm281xx: implement prerequisite clocks

[Artur Weber]

From: Alex Elder <elder@kernel.org>

Allow a clock to specify a "prerequisite" clock, identified by its
name.  The prerequisite clock must be prepared and enabled before a
clock that depends on it is used.  In order to simplify locking, we
require a clock and its prerequisite to be associated with the same
CCU.  (We'll just trust--but not verify--that nobody defines a cycle
of prerequisite clocks.)

Rework the KONA_CLK() macro, and define a new KONA_CLK_PREREQ()
variant that allows a prerequisite clock to be specified.

Signed-off-by: Alex Elder <elder@linaro.org>
--- Artur: rebase on v6.13, move prereq prepare/unprepare to main
    prepare/unprepare functions, use locking versions of clk_prepare
    and clk_enable since the non-locking versions are no longer
    public ---
Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 drivers/clk/bcm/clk-kona.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/bcm/clk-kona.h | 20 ++++++++++++---
 2 files changed, 79 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/bcm/clk-kona.c b/drivers/clk/bcm/clk-kona.c
index e92d57f3bbb147e72221802175a80502897d7504..21f925683d0da05ebc97f92236dfb207b1f9c741 100644
--- a/drivers/clk/bcm/clk-kona.c
+++ b/drivers/clk/bcm/clk-kona.c
@@ -9,6 +9,7 @@
 #include <linux/delay.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/clk.h>
 #include <linux/clk-provider.h>
 
 /*
@@ -961,6 +962,63 @@ static int selector_write(struct ccu_data *ccu, struct bcm_clk_gate *gate,
 	return ret;
 }
 
+/*
+ * Common clock prepare/unprepare functions. These implement a "prerequisite"
+ * mechanism; the prerequisite clock is prepared and enabled before the main
+ * clock is prepared.
+ */
+
+static int kona_clk_prepare(struct clk_hw *hw)
+{
+	struct kona_clk *bcm_clk = to_kona_clk(hw);
+	const char *clk_name = bcm_clk->init_data.name;
+	const char *prereq_name = bcm_clk->prereq.name;
+	struct clk *prereq_clk = bcm_clk->prereq.clk;
+	int ret;
+
+	/* If there's no prerequisite clock, there's nothing to do */
+	if (!prereq_name)
+		return 0;
+
+	/* Look up the prerequisite clock if we haven't already */
+	if (!prereq_clk) {
+		prereq_clk = __clk_lookup(prereq_name);
+		if (WARN_ON_ONCE(!prereq_clk))
+			return -ENOENT;
+		bcm_clk->prereq.clk = prereq_clk;
+	}
+
+	ret = clk_prepare(prereq_clk);
+	if (ret) {
+		pr_err("%s: unable to prepare prereq clock %s for %s\n",
+			__func__, prereq_name, clk_name);
+		return ret;
+	}
+
+	ret = clk_enable(prereq_clk);
+	if (ret) {
+		clk_unprepare(prereq_clk);
+		pr_err("%s: unable to enable prereq clock %s for %s\n",
+			__func__, prereq_name, clk_name);
+		return ret;
+	}
+
+	return 0;
+}
+
+static void kona_clk_unprepare(struct clk_hw *hw)
+{
+	struct kona_clk *bcm_clk = to_kona_clk(hw);
+	struct clk *prereq_clk = bcm_clk->prereq.clk;
+
+	/* If there's no prerequisite clock, there's nothing to do */
+	if (!bcm_clk->prereq.name)
+		return;
+
+	clk_disable(prereq_clk);
+	clk_unprepare(prereq_clk);
+}
+
 /* Peripheral clock operations */
 
 static int kona_peri_clk_enable(struct clk_hw *hw)
@@ -1172,6 +1230,8 @@ static int kona_peri_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 }
 
 struct clk_ops kona_peri_clk_ops = {
+	.prepare = kona_clk_prepare,
+	.unprepare = kona_clk_unprepare,
 	.enable = kona_peri_clk_enable,
 	.disable = kona_peri_clk_disable,
 	.is_enabled = kona_peri_clk_is_enabled,
@@ -1260,6 +1320,8 @@ static int kona_bus_clk_is_enabled(struct clk_hw *hw)
 }
 
 struct clk_ops kona_bus_clk_ops = {
+	.prepare = kona_clk_prepare,
+	.unprepare = kona_clk_unprepare,
 	.enable = kona_bus_clk_enable,
 	.disable = kona_bus_clk_disable,
 	.is_enabled = kona_bus_clk_is_enabled,
diff --git a/drivers/clk/bcm/clk-kona.h b/drivers/clk/bcm/clk-kona.h
index a5b3d8bdb54eaee9fad80c28796170207b817dfd..c32c621282ec6dd40fff3f7598ee8aa007fed524 100644
--- a/drivers/clk/bcm/clk-kona.h
+++ b/drivers/clk/bcm/clk-kona.h
@@ -406,6 +406,10 @@ struct kona_clk {
 	struct clk_init_data init_data;	/* includes name of this clock */
 	struct ccu_data *ccu;	/* ccu this clock is associated with */
 	enum bcm_clk_type type;
+	struct {
+		const char *name;
+		struct clk *clk;
+	} prereq;
 	union {
 		void *data;
 		struct peri_clk_data *peri;
@@ -416,16 +420,26 @@ struct kona_clk {
 	container_of(_hw, struct kona_clk, hw)
 
 /* Initialization macro for an entry in a CCU's kona_clks[] array. */
-#define KONA_CLK(_ccu_name, _clk_name, _type)				\
-	{								\
+#define __KONA_CLK_COMMON(_ccu_name, _clk_name, _type)			\
 		.init_data	= {					\
 			.name = #_clk_name,				\
 			.ops = &kona_ ## _type ## _clk_ops,		\
 		},							\
 		.ccu		= &_ccu_name ## _ccu_data,		\
 		.type		= bcm_clk_ ## _type,			\
-		.u.data		= &_clk_name ## _data,			\
+		.u.data		= &_clk_name ## _data
+
+#define KONA_CLK(_ccu_name, _clk_name, _type)				\
+	{								\
+		__KONA_CLK_COMMON(_ccu_name, _clk_name, _type),	\
 	}
+
+#define KONA_CLK_PREREQ(_ccu_name, _clk_name, _type, _prereq)		\
+	{								\
+		.prereq.name	= #_prereq,				\
+		__KONA_CLK_COMMON(_ccu_name, _clk_name, _type),	\
+	}
+
 #define LAST_KONA_CLK	{ .type = bcm_clk_none }
 
 /*

-- 
2.48.1

Re: [PATCH RFC 3/5] clk: bcm281xx: implement prerequisite clocks

[Alex Elder]

On 2/16/25 10:12 AM, Artur Weber wrote:
> From: Alex Elder <elder@kernel.org>
> 
> Allow a clock to specify a "prerequisite" clock, identified by its
> name.  The prerequisite clock must be prepared and enabled before a
> clock that depends on it is used.  In order to simplify locking, we
> require a clock and its prerequisite to be associated with the same
> CCU.  (We'll just trust--but not verify--that nobody defines a cycle
> of prerequisite clocks.)
> 
> Rework the KONA_CLK() macro, and define a new KONA_CLK_PREREQ()
> variant that allows a prerequisite clock to be specified.
> 
> Signed-off-by: Alex Elder <elder@linaro.org>
> --- Artur: rebase on v6.13, move prereq prepare/unprepare to main
>      prepare/unprepare functions, use locking versions of clk_prepare
>      and clk_enable since the non-locking versions are no longer
>      public ---
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>

I'm surprised there is no prepare function for the peripheral
clocks.

The prequisite clock should separate the prepare and the
enable functionality.  Right now you have kona_clk_prepare()
doing both.  Instead, a clock's prepare function should
prepare its prerequisite (if any).  Then its enable function
should enable its parent.

Should all the users of peripheral clocks just also be required
to specify the bus clocks as well?  I suppose that doesn't
encode the prerequisite property (bus comes before peripheral);
is that truly a requirement?

					-Alex

> ---
>   drivers/clk/bcm/clk-kona.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/clk/bcm/clk-kona.h | 20 ++++++++++++---
>   2 files changed, 79 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/bcm/clk-kona.c b/drivers/clk/bcm/clk-kona.c
> index e92d57f3bbb147e72221802175a80502897d7504..21f925683d0da05ebc97f92236dfb207b1f9c741 100644
> --- a/drivers/clk/bcm/clk-kona.c
> +++ b/drivers/clk/bcm/clk-kona.c
> @@ -9,6 +9,7 @@
>   #include <linux/delay.h>
>   #include <linux/io.h>
>   #include <linux/kernel.h>
> +#include <linux/clk.h>
>   #include <linux/clk-provider.h>
>   
>   /*
> @@ -961,6 +962,63 @@ static int selector_write(struct ccu_data *ccu, struct bcm_clk_gate *gate,
>   	return ret;
>   }
>   
> +/*
> + * Common clock prepare/unprepare functions. These implement a "prerequisite"
> + * mechanism; the prerequisite clock is prepared and enabled before the main
> + * clock is prepared.
> + */
> +
> +static int kona_clk_prepare(struct clk_hw *hw)
> +{
> +	struct kona_clk *bcm_clk = to_kona_clk(hw);
> +	const char *clk_name = bcm_clk->init_data.name;
> +	const char *prereq_name = bcm_clk->prereq.name;
> +	struct clk *prereq_clk = bcm_clk->prereq.clk;
> +	int ret;
> +
> +	/* If there's no prerequisite clock, there's nothing to do */
> +	if (!prereq_name)
> +		return 0;
> +
> +	/* Look up the prerequisite clock if we haven't already */
> +	if (!prereq_clk) {
> +		prereq_clk = __clk_lookup(prereq_name);
> +		if (WARN_ON_ONCE(!prereq_clk))
> +			return -ENOENT;
> +		bcm_clk->prereq.clk = prereq_clk;
> +	}
> +
> +	ret = clk_prepare(prereq_clk);
> +	if (ret) {
> +		pr_err("%s: unable to prepare prereq clock %s for %s\n",
> +			__func__, prereq_name, clk_name);
> +		return ret;
> +	}
> +
> +	ret = clk_enable(prereq_clk);
> +	if (ret) {
> +		clk_unprepare(prereq_clk);
> +		pr_err("%s: unable to enable prereq clock %s for %s\n",
> +			__func__, prereq_name, clk_name);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static void kona_clk_unprepare(struct clk_hw *hw)
> +{
> +	struct kona_clk *bcm_clk = to_kona_clk(hw);
> +	struct clk *prereq_clk = bcm_clk->prereq.clk;
> +
> +	/* If there's no prerequisite clock, there's nothing to do */
> +	if (!bcm_clk->prereq.name)
> +		return;
> +
> +	clk_disable(prereq_clk);
> +	clk_unprepare(prereq_clk);
> +}
> +
>   /* Peripheral clock operations */
>   
>   static int kona_peri_clk_enable(struct clk_hw *hw)
> @@ -1172,6 +1230,8 @@ static int kona_peri_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>   }
>   
>   struct clk_ops kona_peri_clk_ops = {
> +	.prepare = kona_clk_prepare,
> +	.unprepare = kona_clk_unprepare,
>   	.enable = kona_peri_clk_enable,
>   	.disable = kona_peri_clk_disable,
>   	.is_enabled = kona_peri_clk_is_enabled,
> @@ -1260,6 +1320,8 @@ static int kona_bus_clk_is_enabled(struct clk_hw *hw)
>   }
>   
>   struct clk_ops kona_bus_clk_ops = {
> +	.prepare = kona_clk_prepare,
> +	.unprepare = kona_clk_unprepare,
>   	.enable = kona_bus_clk_enable,
>   	.disable = kona_bus_clk_disable,
>   	.is_enabled = kona_bus_clk_is_enabled,
> diff --git a/drivers/clk/bcm/clk-kona.h b/drivers/clk/bcm/clk-kona.h
> index a5b3d8bdb54eaee9fad80c28796170207b817dfd..c32c621282ec6dd40fff3f7598ee8aa007fed524 100644
> --- a/drivers/clk/bcm/clk-kona.h
> +++ b/drivers/clk/bcm/clk-kona.h
> @@ -406,6 +406,10 @@ struct kona_clk {
>   	struct clk_init_data init_data;	/* includes name of this clock */
>   	struct ccu_data *ccu;	/* ccu this clock is associated with */
>   	enum bcm_clk_type type;
> +	struct {
> +		const char *name;
> +		struct clk *clk;
> +	} prereq;
>   	union {
>   		void *data;
>   		struct peri_clk_data *peri;
> @@ -416,16 +420,26 @@ struct kona_clk {
>   	container_of(_hw, struct kona_clk, hw)
>   
>   /* Initialization macro for an entry in a CCU's kona_clks[] array. */
> -#define KONA_CLK(_ccu_name, _clk_name, _type)				\
> -	{								\
> +#define __KONA_CLK_COMMON(_ccu_name, _clk_name, _type)			\
>   		.init_data	= {					\
>   			.name = #_clk_name,				\
>   			.ops = &kona_ ## _type ## _clk_ops,		\
>   		},							\
>   		.ccu		= &_ccu_name ## _ccu_data,		\
>   		.type		= bcm_clk_ ## _type,			\
> -		.u.data		= &_clk_name ## _data,			\
> +		.u.data		= &_clk_name ## _data
> +
> +#define KONA_CLK(_ccu_name, _clk_name, _type)				\
> +	{								\
> +		__KONA_CLK_COMMON(_ccu_name, _clk_name, _type),	\
>   	}
> +
> +#define KONA_CLK_PREREQ(_ccu_name, _clk_name, _type, _prereq)		\
> +	{								\
> +		.prereq.name	= #_prereq,				\
> +		__KONA_CLK_COMMON(_ccu_name, _clk_name, _type),	\
> +	}
> +
>   #define LAST_KONA_CLK	{ .type = bcm_clk_none }
>   
>   /*
> 

Re: [PATCH RFC 3/5] clk: bcm281xx: implement prerequisite clocks

[Artur Weber]

On 24.02.2025 17:20, Alex Elder wrote:
> On 2/16/25 10:12 AM, Artur Weber wrote:
>> From: Alex Elder <elder@kernel.org>
>>
>> Allow a clock to specify a "prerequisite" clock, identified by its
>> name.  The prerequisite clock must be prepared and enabled before a
>> clock that depends on it is used.  In order to simplify locking, we
>> require a clock and its prerequisite to be associated with the same
>> CCU.  (We'll just trust--but not verify--that nobody defines a cycle
>> of prerequisite clocks.)
>>
>> Rework the KONA_CLK() macro, and define a new KONA_CLK_PREREQ()
>> variant that allows a prerequisite clock to be specified.
>>
>> Signed-off-by: Alex Elder <elder@linaro.org>
>> --- Artur: rebase on v6.13, move prereq prepare/unprepare to main
>>      prepare/unprepare functions, use locking versions of clk_prepare
>>      and clk_enable since the non-locking versions are no longer
>>      public ---
>> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> 
> I'm surprised there is no prepare function for the peripheral
> clocks.

Not sure I follow - there is a prepare function for peri clocks, the
same one as for all Kona clocks - kona_clk_prepare. Though it only
enables the prereq clock... I assume you mean "prepare function specific
to peripheral clocks", in which case - what should a prepare function
specifically for peripheral clocks do?

As I mentioned in the cover letter, clock initialization has to be done
explicitly at clock init time, since it's required for later operations
like setting the clock rate/parent that are done *before* the first call
to the prepare function happens; otherwise we get errors like this:

[    4.740000] [   T39] kona_peri_clk_set_parent: trigger failed for sdio1
[    4.760000] [   T39] kona_peri_clk_set_rate: gating failure for sdio1
[    5.650000] [   T36] kona_peri_clk_set_parent: trigger failed for sdio3
[    5.670000] [   T36] kona_peri_clk_set_rate: gating failure for sdio3

(I did consider moving the relevant clock initialization to a
"clk_ops.init" function, but left it out of this patchset for brevity.
Might consider actually doing that...)

> The prequisite clock should separate the prepare and the
> enable functionality.  Right now you have kona_clk_prepare()
> doing both.  Instead, a clock's prepare function should
> prepare its prerequisite (if any).  Then its enable function
> should enable its parent.

I copied this behavior from the original patch; there the
prereq_prepare function both prepared and enabled the relevant
prerequisite clock. Indeed doing each step (prepare/enable/disable/
unprepare) in the relevant functions would probably make more sense.

> Should all the users of peripheral clocks just also be required
> to specify the bus clocks as well?  I suppose that doesn't
> encode the prerequisite property (bus comes before peripheral);
> is that truly a requirement?

I see a few problems with that:

- Most drivers only take one clock - that clock is pretty much always
   the peripheral clock (with the only exception being usb_otg_ahb which
   is used for the USB OTG controller).
- Even if they supported both clocks, they would likely just switch them
   on/off at the same time as the equivalent peri clock.

I mostly figured a clock dependency mechanism would work here since it's
what the vendor kernel does - it allows for specifying a dependent clock
and enables/disables it whenever the clock with the dependency is
enabled/disabled.

An alternative option would be to handle the dependency in the device
tree, using "simple-pm-bus" nodes, either:

- Wrapping around each node for a subdevice that uses a peripheral
   clock. This is rather unwieldy since it means a lot of subnodes for
   the most basic of peripherals (sdio1-4, uartb(2,3), etc.). (Also,
   managing the "ranges" properties for all of these sub-busses would
   get annoying unless we specify empty ranges, which according to the
   bindings "is only appropriate if the child node is another 'simple-
   bus' or similar."[1].)

- Using the simple-bus nodes that the BCM2166x DTSI already uses, and
   connecting the relevant bus clock for all the components to them,
   and switching the compatible to "simple-pm-bus". It's less granular,
   but is probably the most sensible option if we go the DT route.

As for whether enabling a bus clock before a peripheral clock is
required... I went ahead and tested it, and the results seem to hint
that it's not.

- I seem to remember that not having the bus clock initialized caused
   some failures on peri clocks, but I can't reproduce this anymore.
   Most likely I'm mistaking it for policy bit setup, which is required
   for gating to work (I have another commit ready to add policy bits
   to all BCM21664 clocks, which I'm planning to send after this
   patchset). And besides, just initializing bus clocks after peri
   clocks seems to work given that's how it's set up right now.

- Also, it seems that all clocks defined with HW_SW_GATE are gated
   on by default, and this state is never cleared afterwards, so all the
   bus clocks I added in the BCM21664 bus clock commit are already
   enabled at startup... after the peri clocks. And given that everything
   works fine, it's probably OK.

Now that I think about it - maybe it would be easier to just keep things
the way they are; drop the prerequisite clock mechanism, let the
driver enable bus clocks by default, then if we wanted to explicitly
control the bus clocks, use the second DT approach I mentioned. If we
ever want to squeeze every last bit of power savings or for any other
potential benefit of keeping bus clocks off, we can revisit this
mechanism later.

Best regards
Artur

[1] https://github.com/devicetree-org/dt-schema/blob/ed9190d20f146d13e262cc9138506326f7d4da91/dtschema/schemas/simple-bus.yaml#L60-L69

Re: [PATCH RFC 3/5] clk: bcm281xx: implement prerequisite clocks

[Alex Elder]

On 2/25/25 12:48 PM, Artur Weber wrote:
> On 24.02.2025 17:20, Alex Elder wrote:
>> On 2/16/25 10:12 AM, Artur Weber wrote:
>>> From: Alex Elder <elder@kernel.org>
>>>
>>> Allow a clock to specify a "prerequisite" clock, identified by its
>>> name.  The prerequisite clock must be prepared and enabled before a
>>> clock that depends on it is used.  In order to simplify locking, we
>>> require a clock and its prerequisite to be associated with the same
>>> CCU.  (We'll just trust--but not verify--that nobody defines a cycle
>>> of prerequisite clocks.)
>>>
>>> Rework the KONA_CLK() macro, and define a new KONA_CLK_PREREQ()
>>> variant that allows a prerequisite clock to be specified.
>>>
>>> Signed-off-by: Alex Elder <elder@linaro.org>
>>> --- Artur: rebase on v6.13, move prereq prepare/unprepare to main
>>>      prepare/unprepare functions, use locking versions of clk_prepare
>>>      and clk_enable since the non-locking versions are no longer
>>>      public ---
>>> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
>>
>> I'm surprised there is no prepare function for the peripheral
>> clocks.
> 
> Not sure I follow - there is a prepare function for peri clocks, the

I meant *before* your patch, there is no prepare function.

> same one as for all Kona clocks - kona_clk_prepare. Though it only
> enables the prereq clock... I assume you mean "prepare function specific
> to peripheral clocks", in which case - what should a prepare function
> specifically for peripheral clocks do?

It's just that the prepare function can block, but the
enable function cannot.  I see __ccu_wait_bit() just
loops until the register indicates an update completed,
so I guess there's no blocking.

> As I mentioned in the cover letter, clock initialization has to be done
> explicitly at clock init time, since it's required for later operations
> like setting the clock rate/parent that are done *before* the first call
> to the prepare function happens; otherwise we get errors like this:
> 
> [    4.740000] [   T39] kona_peri_clk_set_parent: trigger failed for sdio1
> [    4.760000] [   T39] kona_peri_clk_set_rate: gating failure for sdio1
> [    5.650000] [   T36] kona_peri_clk_set_parent: trigger failed for sdio3
> [    5.670000] [   T36] kona_peri_clk_set_rate: gating failure for sdio3
> 
> (I did consider moving the relevant clock initialization to a
> "clk_ops.init" function, but left it out of this patchset for brevity.
> Might consider actually doing that...)
> 
>> The prequisite clock should separate the prepare and the
>> enable functionality.  Right now you have kona_clk_prepare()
>> doing both.  Instead, a clock's prepare function should
>> prepare its prerequisite (if any).  Then its enable function
>> should enable its parent.
> 
> I copied this behavior from the original patch; there the
> prereq_prepare function both prepared and enabled the relevant
> prerequisite clock. Indeed doing each step (prepare/enable/disable/
> unprepare) in the relevant functions would probably make more sense.

I hadn't worked on a clock driver before then, and I think I
got pulled off that project before this series could be accepted
(or even reviewed?).  Maybe I just proposed what the vendor
kernel did.  In any case, *today* I'd say they should be separated.

>> Should all the users of peripheral clocks just also be required
>> to specify the bus clocks as well?  I suppose that doesn't
>> encode the prerequisite property (bus comes before peripheral);
>> is that truly a requirement?
> 
> I see a few problems with that:
> 
> - Most drivers only take one clock - that clock is pretty much always
>    the peripheral clock (with the only exception being usb_otg_ahb which
>    is used for the USB OTG controller).
> - Even if they supported both clocks, they would likely just switch them
>    on/off at the same time as the equivalent peri clock.
> 
> I mostly figured a clock dependency mechanism would work here since it's
> what the vendor kernel does - it allows for specifying a dependent clock
> and enables/disables it whenever the clock with the dependency is
> enabled/disabled.

I don't really expect that the bus clock *must* be enabled
before the peripheral clock.  The requirement is probably
just that both must be (prepared and) enabled before using
the peripheral, and order doesn't matter.  And in that case
there's no need to enforce the ordering.

> An alternative option would be to handle the dependency in the device
> tree, using "simple-pm-bus" nodes, either:
> 
> - Wrapping around each node for a subdevice that uses a peripheral
>    clock. This is rather unwieldy since it means a lot of subnodes for
>    the most basic of peripherals (sdio1-4, uartb(2,3), etc.). (Also,
>    managing the "ranges" properties for all of these sub-busses would
>    get annoying unless we specify empty ranges, which according to the
>    bindings "is only appropriate if the child node is another 'simple-
>    bus' or similar."[1].)

The above sounds wrong to me.

> - Using the simple-bus nodes that the BCM2166x DTSI already uses, and
>    connecting the relevant bus clock for all the components to them,
>    and switching the compatible to "simple-pm-bus". It's less granular,
>    but is probably the most sensible option if we go the DT route.
> 
> As for whether enabling a bus clock before a peripheral clock is
> required... I went ahead and tested it, and the results seem to hint
> that it's not.
> 
> - I seem to remember that not having the bus clock initialized caused
>    some failures on peri clocks, but I can't reproduce this anymore.
>    Most likely I'm mistaking it for policy bit setup, which is required
>    for gating to work (I have another commit ready to add policy bits
>    to all BCM21664 clocks, which I'm planning to send after this
>    patchset). And besides, just initializing bus clocks after peri
>    clocks seems to work given that's how it's set up right now.
> 
> - Also, it seems that all clocks defined with HW_SW_GATE are gated
>    on by default, and this state is never cleared afterwards, so all the
>    bus clocks I added in the BCM21664 bus clock commit are already
>    enabled at startup... after the peri clocks. And given that everything
>    works fine, it's probably OK.
> 
> Now that I think about it - maybe it would be easier to just keep things
> the way they are; drop the prerequisite clock mechanism, let the
> driver enable bus clocks by default, then if we wanted to explicitly
> control the bus clocks, use the second DT approach I mentioned. If we
> ever want to squeeze every last bit of power savings or for any other
> potential benefit of keeping bus clocks off, we can revisit this
> mechanism later.

I'm far out of the loop on the Broadcom drivers.  If the hardware
requires no actual hierarchy/ordering of these clocks, then none
should be implied (by DT, or by the code).  And if that's the case,
having the peripherals depend on both the bus and peripheral clocks
probably makes sense.

					-Alex

> 
> Best regards
> Artur
> 
> [1] https://github.com/devicetree-org/dt-schema/blob/ 
> ed9190d20f146d13e262cc9138506326f7d4da91/dtschema/schemas/simple- 
> bus.yaml#L60-L69

[PATCH RFC 4/5] clk: bcm21664: Add matching bus clocks for peripheral clocks

[Artur Weber]

Now that bus clock support has been implemented into the Broadcom Kona
clock driver, add bus clocks corresponding to HUB_TIMER, SDIO, UART and
BSC, as well as the USB OTG bus clock.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 drivers/clk/bcm/clk-bcm21664.c       | 107 +++++++++++++++++++++++++++++++----
 include/dt-bindings/clock/bcm21664.h |  19 ++++++-
 2 files changed, 111 insertions(+), 15 deletions(-)

diff --git a/drivers/clk/bcm/clk-bcm21664.c b/drivers/clk/bcm/clk-bcm21664.c
index 520c3aeb4ea9c4a431512c0909f9545c1761d17a..c9d15bde1d49c345c160880f72c96aea94dd9f55 100644
--- a/drivers/clk/bcm/clk-bcm21664.c
+++ b/drivers/clk/bcm/clk-bcm21664.c
@@ -39,6 +39,11 @@ static struct peri_clk_data hub_timer_data = {
 	.trig		= TRIGGER(0x0a40, 4),
 };
 
+static struct bus_clk_data hub_timer_apb_data = {
+	.gate		= HW_SW_GATE(0x0414, 18, 3, 2),
+	.hyst		= HYST(0x0414, 10, 11),
+};
+
 static struct ccu_data aon_ccu_data = {
 	BCM21664_CCU_COMMON(aon, AON),
 	.policy		= {
@@ -47,7 +52,9 @@ static struct ccu_data aon_ccu_data = {
 	},
 	.kona_clks	= {
 		[BCM21664_AON_CCU_HUB_TIMER] =
-			KONA_CLK(aon, hub_timer, peri),
+			KONA_CLK_PREREQ(aon, hub_timer, peri, hub_timer_apb),
+		[BCM21664_AON_CCU_HUB_TIMER_APB] =
+			KONA_CLK(aon, hub_timer_apb, bus),
 		[BCM21664_AON_CCU_CLOCK_COUNT] = LAST_KONA_CLK,
 	},
 };
@@ -122,6 +129,26 @@ static struct peri_clk_data sdio4_sleep_data = {
 	.gate		= HW_SW_GATE(0x0360, 18, 2, 3),
 };
 
+static struct bus_clk_data sdio1_ahb_data = {
+	.gate		= HW_SW_GATE(0x0358, 16, 0, 1),
+};
+
+static struct bus_clk_data sdio2_ahb_data = {
+	.gate		= HW_SW_GATE(0x035c, 16, 0, 1),
+};
+
+static struct bus_clk_data sdio3_ahb_data = {
+	.gate		= HW_SW_GATE(0x0364, 16, 0, 1),
+};
+
+static struct bus_clk_data sdio4_ahb_data = {
+	.gate		= HW_SW_GATE(0x0360, 16, 0, 1),
+};
+
+static struct bus_clk_data usb_otg_ahb_data = {
+	.gate		= HW_SW_GATE(0x0348, 16, 0, 1),
+};
+
 static struct ccu_data master_ccu_data = {
 	BCM21664_CCU_COMMON(master, MASTER),
 	.policy		= {
@@ -130,13 +157,13 @@ static struct ccu_data master_ccu_data = {
 	},
 	.kona_clks	= {
 		[BCM21664_MASTER_CCU_SDIO1] =
-			KONA_CLK(master, sdio1, peri),
+			KONA_CLK_PREREQ(master, sdio1, peri, sdio1_ahb),
 		[BCM21664_MASTER_CCU_SDIO2] =
-			KONA_CLK(master, sdio2, peri),
+			KONA_CLK_PREREQ(master, sdio2, peri, sdio2_ahb),
 		[BCM21664_MASTER_CCU_SDIO3] =
-			KONA_CLK(master, sdio3, peri),
+			KONA_CLK_PREREQ(master, sdio3, peri, sdio3_ahb),
 		[BCM21664_MASTER_CCU_SDIO4] =
-			KONA_CLK(master, sdio4, peri),
+			KONA_CLK_PREREQ(master, sdio4, peri, sdio4_ahb),
 		[BCM21664_MASTER_CCU_SDIO1_SLEEP] =
 			KONA_CLK(master, sdio1_sleep, peri),
 		[BCM21664_MASTER_CCU_SDIO2_SLEEP] =
@@ -145,6 +172,16 @@ static struct ccu_data master_ccu_data = {
 			KONA_CLK(master, sdio3_sleep, peri),
 		[BCM21664_MASTER_CCU_SDIO4_SLEEP] =
 			KONA_CLK(master, sdio4_sleep, peri),
+		[BCM21664_MASTER_CCU_SDIO1_AHB] =
+			KONA_CLK(master, sdio1_ahb, bus),
+		[BCM21664_MASTER_CCU_SDIO2_AHB] =
+			KONA_CLK(master, sdio2_ahb, bus),
+		[BCM21664_MASTER_CCU_SDIO3_AHB] =
+			KONA_CLK(master, sdio3_ahb, bus),
+		[BCM21664_MASTER_CCU_SDIO4_AHB] =
+			KONA_CLK(master, sdio4_ahb, bus),
+		[BCM21664_MASTER_CCU_USB_OTG_AHB] =
+			KONA_CLK(master, usb_otg_ahb, bus),
 		[BCM21664_MASTER_CCU_CLOCK_COUNT] = LAST_KONA_CLK,
 	},
 };
@@ -225,6 +262,38 @@ static struct peri_clk_data bsc4_data = {
 	.trig		= TRIGGER(0x0afc, 19),
 };
 
+static struct bus_clk_data uartb_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x0400, 16, 0, 1),
+};
+
+static struct bus_clk_data uartb2_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x0404, 16, 0, 1),
+};
+
+static struct bus_clk_data uartb3_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x0408, 16, 0, 1),
+};
+
+static struct bus_clk_data bsc1_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x0458, 16, 0, 1),
+	.hyst		= HYST(0x0458, 8, 9),
+};
+
+static struct bus_clk_data bsc2_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x045c, 16, 0, 1),
+	.hyst		= HYST(0x045c, 8, 9),
+};
+
+static struct bus_clk_data bsc3_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x0470, 16, 0, 1),
+	.hyst		= HYST(0x0470, 8, 9),
+};
+
+static struct bus_clk_data bsc4_apb_data = {
+	.gate		= HW_SW_GATE_AUTO(0x0474, 16, 0, 1),
+	.hyst		= HYST(0x0474, 8, 9),
+};
+
 static struct ccu_data slave_ccu_data = {
 	BCM21664_CCU_COMMON(slave, SLAVE),
        .policy		= {
@@ -233,19 +302,33 @@ static struct ccu_data slave_ccu_data = {
 	},
 	.kona_clks	= {
 		[BCM21664_SLAVE_CCU_UARTB] =
-			KONA_CLK(slave, uartb, peri),
+			KONA_CLK_PREREQ(slave, uartb, peri, uartb_apb),
 		[BCM21664_SLAVE_CCU_UARTB2] =
-			KONA_CLK(slave, uartb2, peri),
+			KONA_CLK_PREREQ(slave, uartb2, peri, uartb2_apb),
 		[BCM21664_SLAVE_CCU_UARTB3] =
-			KONA_CLK(slave, uartb3, peri),
+			KONA_CLK_PREREQ(slave, uartb3, peri, uartb3_apb),
 		[BCM21664_SLAVE_CCU_BSC1] =
-			KONA_CLK(slave, bsc1, peri),
+			KONA_CLK_PREREQ(slave, bsc1, peri, bsc1_apb),
 		[BCM21664_SLAVE_CCU_BSC2] =
-			KONA_CLK(slave, bsc2, peri),
+			KONA_CLK_PREREQ(slave, bsc2, peri, bsc2_apb),
 		[BCM21664_SLAVE_CCU_BSC3] =
-			KONA_CLK(slave, bsc3, peri),
+			KONA_CLK_PREREQ(slave, bsc3, peri, bsc3_apb),
 		[BCM21664_SLAVE_CCU_BSC4] =
-			KONA_CLK(slave, bsc4, peri),
+			KONA_CLK_PREREQ(slave, bsc4, peri, bsc4_apb),
+		[BCM21664_SLAVE_CCU_UARTB_APB] =
+			KONA_CLK(slave, uartb_apb, bus),
+		[BCM21664_SLAVE_CCU_UARTB2_APB] =
+			KONA_CLK(slave, uartb2_apb, bus),
+		[BCM21664_SLAVE_CCU_UARTB3_APB] =
+			KONA_CLK(slave, uartb3_apb, bus),
+		[BCM21664_SLAVE_CCU_BSC1_APB] =
+			KONA_CLK(slave, bsc1_apb, bus),
+		[BCM21664_SLAVE_CCU_BSC2_APB] =
+			KONA_CLK(slave, bsc2_apb, bus),
+		[BCM21664_SLAVE_CCU_BSC3_APB] =
+			KONA_CLK(slave, bsc3_apb, bus),
+		[BCM21664_SLAVE_CCU_BSC4_APB] =
+			KONA_CLK(slave, bsc4_apb, bus),
 		[BCM21664_SLAVE_CCU_CLOCK_COUNT] = LAST_KONA_CLK,
 	},
 };
diff --git a/include/dt-bindings/clock/bcm21664.h b/include/dt-bindings/clock/bcm21664.h
index 7c7492742f3d4ca439236f2f352e432989409570..8d3e3796c72b02eace84dfb90d6264dee0297a33 100644
--- a/include/dt-bindings/clock/bcm21664.h
+++ b/include/dt-bindings/clock/bcm21664.h
@@ -26,7 +26,8 @@
 /* aon CCU clock ids */
 
 #define BCM21664_AON_CCU_HUB_TIMER		0
-#define BCM21664_AON_CCU_CLOCK_COUNT		1
+#define BCM21664_AON_CCU_HUB_TIMER_APB		1
+#define BCM21664_AON_CCU_CLOCK_COUNT		2
 
 /* master CCU clock ids */
 
@@ -38,7 +39,12 @@
 #define BCM21664_MASTER_CCU_SDIO2_SLEEP		5
 #define BCM21664_MASTER_CCU_SDIO3_SLEEP		6
 #define BCM21664_MASTER_CCU_SDIO4_SLEEP		7
-#define BCM21664_MASTER_CCU_CLOCK_COUNT		8
+#define BCM21664_MASTER_CCU_SDIO1_AHB		8
+#define BCM21664_MASTER_CCU_SDIO2_AHB		9
+#define BCM21664_MASTER_CCU_SDIO3_AHB		10
+#define BCM21664_MASTER_CCU_SDIO4_AHB		11
+#define BCM21664_MASTER_CCU_USB_OTG_AHB		12
+#define BCM21664_MASTER_CCU_CLOCK_COUNT		13
 
 /* slave CCU clock ids */
 
@@ -49,6 +55,13 @@
 #define BCM21664_SLAVE_CCU_BSC2			4
 #define BCM21664_SLAVE_CCU_BSC3			5
 #define BCM21664_SLAVE_CCU_BSC4			6
-#define BCM21664_SLAVE_CCU_CLOCK_COUNT		7
+#define BCM21664_SLAVE_CCU_BSC1_APB		7
+#define BCM21664_SLAVE_CCU_BSC2_APB		8
+#define BCM21664_SLAVE_CCU_BSC3_APB		9
+#define BCM21664_SLAVE_CCU_BSC4_APB		10
+#define BCM21664_SLAVE_CCU_UARTB_APB		11
+#define BCM21664_SLAVE_CCU_UARTB2_APB		12
+#define BCM21664_SLAVE_CCU_UARTB3_APB		13
+#define BCM21664_SLAVE_CCU_CLOCK_COUNT		14
 
 #endif /* _CLOCK_BCM21664_H */

-- 
2.48.1

Re: [PATCH RFC 4/5] clk: bcm21664: Add matching bus clocks for peripheral clocks

[Rob Herring]

On Sun, Feb 16, 2025 at 05:12:39PM +0100, Artur Weber wrote:
> Now that bus clock support has been implemented into the Broadcom Kona
> clock driver, add bus clocks corresponding to HUB_TIMER, SDIO, UART and
> BSC, as well as the USB OTG bus clock.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
>  drivers/clk/bcm/clk-bcm21664.c       | 107 +++++++++++++++++++++++++++++++----
>  include/dt-bindings/clock/bcm21664.h |  19 ++++++-

This is part of the binding, so it goes in the binding patch or on its 
own.

Rob

Re: [PATCH RFC 4/5] clk: bcm21664: Add matching bus clocks for peripheral clocks

[Alex Elder]

On 2/16/25 10:12 AM, Artur Weber wrote:
> Now that bus clock support has been implemented into the Broadcom Kona
> clock driver, add bus clocks corresponding to HUB_TIMER, SDIO, UART and
> BSC, as well as the USB OTG bus clock.
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>

Other than the comment about the header from Rob, this looks
good to me.

Reviewed-by: Alex Elder <elder@riscstar.com>
	
> ---
>   drivers/clk/bcm/clk-bcm21664.c       | 107 +++++++++++++++++++++++++++++++----
>   include/dt-bindings/clock/bcm21664.h |  19 ++++++-
>   2 files changed, 111 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/clk/bcm/clk-bcm21664.c b/drivers/clk/bcm/clk-bcm21664.c
> index 520c3aeb4ea9c4a431512c0909f9545c1761d17a..c9d15bde1d49c345c160880f72c96aea94dd9f55 100644
> --- a/drivers/clk/bcm/clk-bcm21664.c
> +++ b/drivers/clk/bcm/clk-bcm21664.c
> @@ -39,6 +39,11 @@ static struct peri_clk_data hub_timer_data = {
>   	.trig		= TRIGGER(0x0a40, 4),
>   };
>   
> +static struct bus_clk_data hub_timer_apb_data = {
> +	.gate		= HW_SW_GATE(0x0414, 18, 3, 2),
> +	.hyst		= HYST(0x0414, 10, 11),
> +};
> +
>   static struct ccu_data aon_ccu_data = {
>   	BCM21664_CCU_COMMON(aon, AON),
>   	.policy		= {
> @@ -47,7 +52,9 @@ static struct ccu_data aon_ccu_data = {
>   	},
>   	.kona_clks	= {
>   		[BCM21664_AON_CCU_HUB_TIMER] =
> -			KONA_CLK(aon, hub_timer, peri),
> +			KONA_CLK_PREREQ(aon, hub_timer, peri, hub_timer_apb),
> +		[BCM21664_AON_CCU_HUB_TIMER_APB] =
> +			KONA_CLK(aon, hub_timer_apb, bus),
>   		[BCM21664_AON_CCU_CLOCK_COUNT] = LAST_KONA_CLK,
>   	},
>   };
> @@ -122,6 +129,26 @@ static struct peri_clk_data sdio4_sleep_data = {
>   	.gate		= HW_SW_GATE(0x0360, 18, 2, 3),
>   };
>   
> +static struct bus_clk_data sdio1_ahb_data = {
> +	.gate		= HW_SW_GATE(0x0358, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data sdio2_ahb_data = {
> +	.gate		= HW_SW_GATE(0x035c, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data sdio3_ahb_data = {
> +	.gate		= HW_SW_GATE(0x0364, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data sdio4_ahb_data = {
> +	.gate		= HW_SW_GATE(0x0360, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data usb_otg_ahb_data = {
> +	.gate		= HW_SW_GATE(0x0348, 16, 0, 1),
> +};
> +
>   static struct ccu_data master_ccu_data = {
>   	BCM21664_CCU_COMMON(master, MASTER),
>   	.policy		= {
> @@ -130,13 +157,13 @@ static struct ccu_data master_ccu_data = {
>   	},
>   	.kona_clks	= {
>   		[BCM21664_MASTER_CCU_SDIO1] =
> -			KONA_CLK(master, sdio1, peri),
> +			KONA_CLK_PREREQ(master, sdio1, peri, sdio1_ahb),
>   		[BCM21664_MASTER_CCU_SDIO2] =
> -			KONA_CLK(master, sdio2, peri),
> +			KONA_CLK_PREREQ(master, sdio2, peri, sdio2_ahb),
>   		[BCM21664_MASTER_CCU_SDIO3] =
> -			KONA_CLK(master, sdio3, peri),
> +			KONA_CLK_PREREQ(master, sdio3, peri, sdio3_ahb),
>   		[BCM21664_MASTER_CCU_SDIO4] =
> -			KONA_CLK(master, sdio4, peri),
> +			KONA_CLK_PREREQ(master, sdio4, peri, sdio4_ahb),
>   		[BCM21664_MASTER_CCU_SDIO1_SLEEP] =
>   			KONA_CLK(master, sdio1_sleep, peri),
>   		[BCM21664_MASTER_CCU_SDIO2_SLEEP] =
> @@ -145,6 +172,16 @@ static struct ccu_data master_ccu_data = {
>   			KONA_CLK(master, sdio3_sleep, peri),
>   		[BCM21664_MASTER_CCU_SDIO4_SLEEP] =
>   			KONA_CLK(master, sdio4_sleep, peri),
> +		[BCM21664_MASTER_CCU_SDIO1_AHB] =
> +			KONA_CLK(master, sdio1_ahb, bus),
> +		[BCM21664_MASTER_CCU_SDIO2_AHB] =
> +			KONA_CLK(master, sdio2_ahb, bus),
> +		[BCM21664_MASTER_CCU_SDIO3_AHB] =
> +			KONA_CLK(master, sdio3_ahb, bus),
> +		[BCM21664_MASTER_CCU_SDIO4_AHB] =
> +			KONA_CLK(master, sdio4_ahb, bus),
> +		[BCM21664_MASTER_CCU_USB_OTG_AHB] =
> +			KONA_CLK(master, usb_otg_ahb, bus),
>   		[BCM21664_MASTER_CCU_CLOCK_COUNT] = LAST_KONA_CLK,
>   	},
>   };
> @@ -225,6 +262,38 @@ static struct peri_clk_data bsc4_data = {
>   	.trig		= TRIGGER(0x0afc, 19),
>   };
>   
> +static struct bus_clk_data uartb_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x0400, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data uartb2_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x0404, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data uartb3_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x0408, 16, 0, 1),
> +};
> +
> +static struct bus_clk_data bsc1_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x0458, 16, 0, 1),
> +	.hyst		= HYST(0x0458, 8, 9),
> +};
> +
> +static struct bus_clk_data bsc2_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x045c, 16, 0, 1),
> +	.hyst		= HYST(0x045c, 8, 9),
> +};
> +
> +static struct bus_clk_data bsc3_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x0470, 16, 0, 1),
> +	.hyst		= HYST(0x0470, 8, 9),
> +};
> +
> +static struct bus_clk_data bsc4_apb_data = {
> +	.gate		= HW_SW_GATE_AUTO(0x0474, 16, 0, 1),
> +	.hyst		= HYST(0x0474, 8, 9),
> +};
> +
>   static struct ccu_data slave_ccu_data = {
>   	BCM21664_CCU_COMMON(slave, SLAVE),
>          .policy		= {
> @@ -233,19 +302,33 @@ static struct ccu_data slave_ccu_data = {
>   	},
>   	.kona_clks	= {
>   		[BCM21664_SLAVE_CCU_UARTB] =
> -			KONA_CLK(slave, uartb, peri),
> +			KONA_CLK_PREREQ(slave, uartb, peri, uartb_apb),
>   		[BCM21664_SLAVE_CCU_UARTB2] =
> -			KONA_CLK(slave, uartb2, peri),
> +			KONA_CLK_PREREQ(slave, uartb2, peri, uartb2_apb),
>   		[BCM21664_SLAVE_CCU_UARTB3] =
> -			KONA_CLK(slave, uartb3, peri),
> +			KONA_CLK_PREREQ(slave, uartb3, peri, uartb3_apb),
>   		[BCM21664_SLAVE_CCU_BSC1] =
> -			KONA_CLK(slave, bsc1, peri),
> +			KONA_CLK_PREREQ(slave, bsc1, peri, bsc1_apb),
>   		[BCM21664_SLAVE_CCU_BSC2] =
> -			KONA_CLK(slave, bsc2, peri),
> +			KONA_CLK_PREREQ(slave, bsc2, peri, bsc2_apb),
>   		[BCM21664_SLAVE_CCU_BSC3] =
> -			KONA_CLK(slave, bsc3, peri),
> +			KONA_CLK_PREREQ(slave, bsc3, peri, bsc3_apb),
>   		[BCM21664_SLAVE_CCU_BSC4] =
> -			KONA_CLK(slave, bsc4, peri),
> +			KONA_CLK_PREREQ(slave, bsc4, peri, bsc4_apb),
> +		[BCM21664_SLAVE_CCU_UARTB_APB] =
> +			KONA_CLK(slave, uartb_apb, bus),
> +		[BCM21664_SLAVE_CCU_UARTB2_APB] =
> +			KONA_CLK(slave, uartb2_apb, bus),
> +		[BCM21664_SLAVE_CCU_UARTB3_APB] =
> +			KONA_CLK(slave, uartb3_apb, bus),
> +		[BCM21664_SLAVE_CCU_BSC1_APB] =
> +			KONA_CLK(slave, bsc1_apb, bus),
> +		[BCM21664_SLAVE_CCU_BSC2_APB] =
> +			KONA_CLK(slave, bsc2_apb, bus),
> +		[BCM21664_SLAVE_CCU_BSC3_APB] =
> +			KONA_CLK(slave, bsc3_apb, bus),
> +		[BCM21664_SLAVE_CCU_BSC4_APB] =
> +			KONA_CLK(slave, bsc4_apb, bus),
>   		[BCM21664_SLAVE_CCU_CLOCK_COUNT] = LAST_KONA_CLK,
>   	},
>   };
> diff --git a/include/dt-bindings/clock/bcm21664.h b/include/dt-bindings/clock/bcm21664.h
> index 7c7492742f3d4ca439236f2f352e432989409570..8d3e3796c72b02eace84dfb90d6264dee0297a33 100644
> --- a/include/dt-bindings/clock/bcm21664.h
> +++ b/include/dt-bindings/clock/bcm21664.h
> @@ -26,7 +26,8 @@
>   /* aon CCU clock ids */
>   
>   #define BCM21664_AON_CCU_HUB_TIMER		0
> -#define BCM21664_AON_CCU_CLOCK_COUNT		1
> +#define BCM21664_AON_CCU_HUB_TIMER_APB		1
> +#define BCM21664_AON_CCU_CLOCK_COUNT		2
>   
>   /* master CCU clock ids */
>   
> @@ -38,7 +39,12 @@
>   #define BCM21664_MASTER_CCU_SDIO2_SLEEP		5
>   #define BCM21664_MASTER_CCU_SDIO3_SLEEP		6
>   #define BCM21664_MASTER_CCU_SDIO4_SLEEP		7
> -#define BCM21664_MASTER_CCU_CLOCK_COUNT		8
> +#define BCM21664_MASTER_CCU_SDIO1_AHB		8
> +#define BCM21664_MASTER_CCU_SDIO2_AHB		9
> +#define BCM21664_MASTER_CCU_SDIO3_AHB		10
> +#define BCM21664_MASTER_CCU_SDIO4_AHB		11
> +#define BCM21664_MASTER_CCU_USB_OTG_AHB		12
> +#define BCM21664_MASTER_CCU_CLOCK_COUNT		13
>   
>   /* slave CCU clock ids */
>   
> @@ -49,6 +55,13 @@
>   #define BCM21664_SLAVE_CCU_BSC2			4
>   #define BCM21664_SLAVE_CCU_BSC3			5
>   #define BCM21664_SLAVE_CCU_BSC4			6
> -#define BCM21664_SLAVE_CCU_CLOCK_COUNT		7
> +#define BCM21664_SLAVE_CCU_BSC1_APB		7
> +#define BCM21664_SLAVE_CCU_BSC2_APB		8
> +#define BCM21664_SLAVE_CCU_BSC3_APB		9
> +#define BCM21664_SLAVE_CCU_BSC4_APB		10
> +#define BCM21664_SLAVE_CCU_UARTB_APB		11
> +#define BCM21664_SLAVE_CCU_UARTB2_APB		12
> +#define BCM21664_SLAVE_CCU_UARTB3_APB		13
> +#define BCM21664_SLAVE_CCU_CLOCK_COUNT		14
>   
>   #endif /* _CLOCK_BCM21664_H */
> 

[PATCH RFC 5/5] ARM: dts: bcm2166x-common: Add matching bus clocks for peripheral clocks

[Artur Weber]

Following changes in the clock driver, add bus clocks for timer, SDIO,
BSC and UART to the DTS clock output names. Replace the usb_otg_ahb
fixed clock with the real bus clock.

Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
---
 arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi | 28 ++++++++++++++++---------
 1 file changed, 18 insertions(+), 10 deletions(-)

diff --git a/arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi b/arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi
index 87180b7fd695e65b52c52743e6315cbcca385fba..ab6ad8c6d326171a6da1762ecd839bd82e9da482 100644
--- a/arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi
+++ b/arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi
@@ -143,7 +143,7 @@ usbotg: usb@e20000 {
 			compatible = "snps,dwc2";
 			reg = <0x00e20000 0x10000>;
 			interrupts = <GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&usb_otg_ahb_clk>;
+			clocks = <&master_ccu BCM21664_MASTER_CCU_USB_OTG_AHB>;
 			clock-names = "otg";
 			phys = <&usbphy>;
 			phy-names = "usb2-phy";
@@ -248,12 +248,6 @@ var_52m_clk: var_52m {
 			clock-frequency = <52000000>;
 		};
 
-		usb_otg_ahb_clk: usb_otg_ahb {
-			#clock-cells = <0>;
-			compatible = "fixed-clock";
-			clock-frequency = <52000000>;
-		};
-
 		ref_96m_clk: ref_96m {
 			#clock-cells = <0>;
 			compatible = "fixed-clock";
@@ -301,7 +295,8 @@ aon_ccu: aon_ccu@35002000 {
 			compatible = "brcm,bcm21664-aon-ccu";
 			reg = <0x35002000 0x0f00>;
 			#clock-cells = <1>;
-			clock-output-names = "hub_timer";
+			clock-output-names = "hub_timer",
+					     "hub_timer_apb";
 		};
 
 		slave_ccu: slave_ccu@3e011000 {
@@ -314,7 +309,15 @@ slave_ccu: slave_ccu@3e011000 {
 					     "bsc1",
 					     "bsc2",
 					     "bsc3",
-					     "bsc4";
+					     "bsc4",
+					     "uartb_apb",
+					     "uartb2_apb",
+					     "uartb3_apb",
+					     "bsc1_apb",
+					     "bsc2_apb",
+					     "bsc3_apb",
+					     "bsc4_apb";
+
 		};
 
 		master_ccu: master_ccu@3f001000 {
@@ -328,7 +331,12 @@ master_ccu: master_ccu@3f001000 {
 					     "sdio1_sleep",
 					     "sdio2_sleep",
 					     "sdio3_sleep",
-					     "sdio4_sleep";
+					     "sdio4_sleep",
+					     "sdio1_ahb",
+					     "sdio2_ahb",
+					     "sdio3_ahb",
+					     "sdio4_ahb",
+					     "usb_otg_ahb";
 		};
 	};
 };

-- 
2.48.1

Re: [RFC PATCH 0/5] clk: bcm: kona: Add bus clock support and prerequisite clocks

[Alex Elder]

On 2/16/25 10:12 AM, Artur Weber wrote:
> This patchset does the following:
> 
> - Introduce support for bus clocks. These are fairly similar to
>    peripheral clocks, but only implement policy, gate and hyst.
> 
> - Introduce support for prerequisite clocks; this way we can
>    make peripheral clocks automatically enable their corresponding
>    bus clocks.
> 
> - Add matching bus clocks for BCM21664 peripheral clocks and update
>    device tree bindings to match.
> 
> The prerequisite clock portion of this patchset is adapted from an
> older attempt to add bus clocks[1], submitted by Alex Elder. I've
> retained his authorship on that commit.

That was a long time ago!

> Notably, Alex's patchset moved clock initialization to the prepare
> function. This seems to be incorrect; the prepare function gets called
> before the enable function, but not before "set rate"/"set parent"
> functions; thus, while clocks enabled fine, any configuration done
> before they were first enabled was broken. I ignored that part of
> the patchset and only kept the prerequisite clocks.

I think you're right.

> I would appreciate feedback on the prerequisite clock patch, hence
> why this patchset is marked as RFC.
> 
> I wasn't able to find any other driver that does something like this,
> so I'm not sure if it's correct (especially since I had to switch from
> non-locking __clk_prepare and __clk_enable to the regular locking
> versions, as the non-locking versions are no longer public - they
> appear to have been replaced by clk_core counterparts, but those
> functions are not exported anywhere AFAICT).
> 
> An alternative way to do this dependency would be to wrap every
> component with a relevant bus clock in a "simple-pm-bus" node
> with the bus clock in DT, but this seems rather unwieldy.

Yes I had the same thought, and ask about this on patch 3.  I
can't comment on whether this notion of a prerequisite (that
is not its parent) makes sense though.

I didn't look at patch 1 or patch 5.

					-Alex

> 
> [1] https://lore.kernel.org/lkml/1402926007-4436-1-git-send-email-elder@linaro.org/
> 
> Signed-off-by: Artur Weber <aweber.kernel@gmail.com>
> ---
> Alex Elder (1):
>        clk: bcm281xx: implement prerequisite clocks
> 
> Artur Weber (4):
>        dt-bindings: clock: brcm,kona-ccu: Add BCM21664 bus clocks
>        clk: bcm: kona: Add support for bus clocks
>        clk: bcm21664: Add matching bus clocks for peripheral clocks
>        ARM: dts: bcm2166x-common: Add matching bus clocks for peripheral clocks
> 
>   .../devicetree/bindings/clock/brcm,kona-ccu.yaml   |  18 ++-
>   arch/arm/boot/dts/broadcom/bcm2166x-common.dtsi    |  28 +++--
>   drivers/clk/bcm/clk-bcm21664.c                     | 107 ++++++++++++++++--
>   drivers/clk/bcm/clk-kona-setup.c                   | 116 +++++++++++++++++++
>   drivers/clk/bcm/clk-kona.c                         | 124 ++++++++++++++++++++-
>   drivers/clk/bcm/clk-kona.h                         |  30 ++++-
>   include/dt-bindings/clock/bcm21664.h               |  19 +++-
>   7 files changed, 411 insertions(+), 31 deletions(-)
> ---
> base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
> change-id: 20250212-kona-bus-clock-4297eefae940
> 
> Best regards,