[RFC] Introduce Clock Management Subsystem (Clock Driver Based) #72102

danieldegrasse · 2024-04-29T14:44:11Z

Introduction

This PR proposes a clock management subsystem. It is opened as an alternative to #70467. The eventual goal would be to replace the existing clock control drivers with implementations using the clock management subsystem. This subsystem defines clock control hardware within the devicetree, and abstracts configuration of clocks to "clock states", which reference the clock control devicetree nodes in order to configure the clock tree.

Problem description

The core goal of this change is to provide a more device-agnostic way to manage clocks. Although the clock control subsystem does define clocks as an opaque type, drivers themselves still often need to be aware of the underlying type behind this opaque definition, and there is no standard for how many cells will be present on a given clock controller, so implementation details of the clock driver are prone to leaking into drivers. This presents a problem for vendors that reuse IP blocks across SOC lines with different clock controller drivers.

Beyond this, the clock control subsystem doesn't provide a simple way to configure clocks. clock_control_configure and clock_control_set_rate are both available, but clock_control_configure is ripe for leaking implementation details to the driver, and clock_control_set_rate will likely require runtime calculations to achieve requested clock rates that aren't realistic for small embedded devices (or leak implementation details, if clock_control_subsys_rate_t isn't an integer)

Proposed change

This proposal provides the initial infrastructure for clock management, as well as an implementation on the LPC55S69 and an initial driver conversion for the Flexcomm serial driver (mostly for demonstration purposes). Long term, the goal would be to transition all SOCs to this subsystem, and deprecate the clock control API. The subsystem has been designed so it can exist alongside clock control (much like pinmux and pinctrl) in order to make this transition smoother.

The application is expected to assign clock states within devicetree, so the driver should have no awareness of the contents of a clock state, only the target state name. Clock outputs are also assigned within the SOC devicetree, so drivers do not see the details of these either.

In order to fully abstract clock management details from consumers, the clock management layer is split into two layers:

clock management layer: public facing, used by consumers to query rates from and reconfigure their clocks
clock driver layer: internal to clock management, used by clock drivers to query rates from and reconfigure their parent sources

This split is required because not all applications want the flash overhead of enabling runtime rate resolution, so clock states need to be opaque to the consumer. When a consumer requests a rate directly via clock_mgmt_req_rate, the request will be satisfied by one of the predefined states for the clock, unless runtime rate resolution is enabled. Consumers can also apply states directly via clock_mgmt_apply_state.

Detailed RFC

Clock Management Layer

The clock management layer is the public API that consumers use to interact with clocks. Each consumer will have a set of clock states defined, along with an array of clock outputs. Consumers can query rates of their output clocks, and apply new clock states at any time.

The Clock Management API exposes the following functions:

clock_mgmt_get_rate: Reads a clock rate from a given clock output in Hz
clock_mgmt_apply_state: Applies a new clock state from those defined in the consumer's devicetree node
clock_mgmt_set_callback: Sets a callback to fire before any of the clock outputs defined for this consumer are reconfigured. A negative return value from this callback will prevent the clock from being reconfigured.
clock_mgmt_disabled_unused: Disable any clock sources that are not actively in use
clock_mgmt_req_rate: Request a frequency range from a clock output

A given clock consumer might define clocks states and outputs like so:

&mydev_clock_source {
    mydev_state_default: mydev-state-default {
        compatible = "clock-state";
	clock-frequency = <DT_FREQ_M(10)>;
	clocks = <&mydev_div 1 &mydev_mux 3>;
    };
    
    mydev_state_sleep: mydev-state-sleep {
        compatible = "clock-state";
	clock-frequency = <DT_FREQ_M(1)>;
	clocks = <&mydev_div 2 &mydev_mux 0>;
    };
}

mydev {
    ...
    compatible = "vnd,mydev";
    clock-outputs = <&mydev_clock_source>;
    clock-output-names = "default";
    clock-state-0 = <&mydev_state_default>;
    clock-state-1 = <&mydev_state_sleep>;
    clock-state-names = "default", "sleep";
    ...
};

Note that the cells for each node within the clocks property of a state are specific to that node's compatible. It is expected that this values will be used to configure settings like multiplexer selections or divider values directly.

The consumer's driver would then interact with the clock management API like so:

/* A driver for the "vnd,mydev" compatible device */
#define DT_DRV_COMPAT vnd_mydev

...
#include <zephyr/drivers/clock_mgmt.h>
...

struct mydev_config {
    ...
    /* Reference to default clock */
    const struct clock_output *clk;
    /* clock default state */
    const clock_mgmt_state_t default_state;
    /* clock sleep state */
    const clock_mgmt_state_t sleep_state;
    ...
};

...

int mydev_clock_cb(const struct clock_mgmt_event *ev, const void *data)
{
    const struct device *dev = (const struct device *)data;

    if (ev->new_rate > HS_MAX_CLK_RATE) {
        /* Can't support this new rate */
        return -ENOTSUP;
    }
    if (ev->type == CLOCK_MGMT_POST_RATE_CHANGE) {
        /* Handle clock rate change ... */
    }
    return 0;
}

static int mydev_init(const struct device *dev)
{
    const struct mydev_config *config = dev->config;
    int clk_rate;
    ...
    /* Set default clock to default state */
    hs_clock_rate = clock_mgmt_apply_state(config->clk, config->default_state);
    if (hs_clock_rate < 0) {
        return hs_clock_rate;
    }
    /* Register for a callback if default clock changes rate */
    ret = clock_mgmt_set_callback(config->hs_clk, hs_clock_cb, dev);
    if (ret < 0) {
        return ret;
    }
    ...
}

#define MYDEV_DEFINE(i)                                                    \
    /* Define clock output for default clock */                            \
    CLOCK_MGMT_DT_INST_DEFINE_OUTPUT_BY_NAME(i, default);                  \
    ...                                                                    \
    static const struct mydev_config mydev_config_##i = {                  \
        ...                                                                \
        /* Initialize clock output */                                      \
        .clk = CLOCK_MGMT_DT_INST_GET_OUTPUT_BY_NAME(i, default),          \
        /* Read states for default and sleep */                            \
        .default_state = CLOCK_MGMT_DT_INST_GET_STATE(i, default,          \
						         default),         \
        .sleep_state = CLOCK_MGMT_DT_INST_GET_STATE(i, default,            \
						       sleep),             \
        ...                                                                \
    };                                                                     \
    static struct mydev_data mydev_data##i;                                \
    ...                                                                    \
								           \
    DEVICE_DT_INST_DEFINE(i, mydev_init, NULL, &mydev_data##i,             \
		          &mydev_config##i, ...);

    DT_INST_FOREACH_STATUS_OKAY(MYDEV_DEFINE)

Requesting Clock Rates versus Configuring the Clock Tree

Clock states can be defined using one of two methods: either clock rates can be requested from using clock_mgmt_req_rate, or states can be applied directly using clock_mgmt_apply_state. If CONFIG_CLOCK_MGMT_SET_RATE is enabled, clock rate requests can also be handled at runtime, which may result in more accurate clocks for a given request. However, some clock configurations may only be possibly by directly applying a state using clock_mgmt_apply_state.

Directly Configuring Clock Tree

For flash optimization or advanced use cases, the devicetree can be used to configure clock nodes directly with driver-specific data. Each clock node in the tree defines a set of specifiers within its compatible, which can be used to configure node specific settings. Each node defines two macros to parse these specifiers, based on its compatible: Z_CLOCK_MGMT_xxx_DATA_DEFINE and Z_CLOCK_MGMT_xxx_DATA_GET (where xxx is the device compatible string as an uppercase token). The expansion of Z_CLOCK_MGMT_xxx_DATA_GET for a given node and set of specifiers will be passed to the clock_configure function as a void * when that clock state is applied. This allows the user to configure clock node specific settings directly (for example, the precision targeted by a given oscillator or the frequency generation method used by a PLL). It can also be used to reduce flash usage, as parameters like PLL multiplication and division factors can be set in the devicetree, rather than being calculated at runtime.

Defining a clock state that directly configures the clock tree might look like so:

&mydev_clock_source {
    mydev_state_default: mydev-state-default {
            compatible = "clock-state";
	    clock-frequency = <DT_FREQ_M(10)>;
	    clocks = <&mydev_div 1 &mydev_mux 3>;
    };
};

This would setup the mydev_mux and mydev_div using hardware specific settings (given by their specifiers). In this case, these settings might be selected so that the clock output of mydev_clock_source would be 1MHz.

Runtime Clock Requests

When CONFIG_CLOCK_MGMT_RUNTIME is enabled, clock requests issued via clock_mgmt_req_rate will be aggregated, so that each request from a consumer is combined into one set of clock constraints. This means that if a consumer makes a request, that request is "sticky", and the clock output will reject an attempt to reconfigure it to a range outside of the requested frequency. For clock states in devicetree, the same "sticky" behavior can be achieved by adding the locking-state property to the state definition. This should be done for states on critical clocks, such as the CPU core clock, that should not be reconfigured due to another consumer applying a clock state.

Clock Driver Layer

The clock driver layer describes clock producers available on the system. Within an SOC clock tree, individual clock nodes (IE clock multiplexers, dividers, and PLLs) are considered separate producers, and should have separate devicetree definitions and drivers. Clock drivers can implement the following API functions:

notify: Called by parent clock to notify child it is about to reconfigure to a new clock rate. Child clock can return error if this rate is not supported, or simply calculate its new rate and forward the notification to its own children
get_rate: Called by child clock to request frequency of this clock in Hz
configure: Called directly by clock management subsystem to reconfigure the clock node. Clock node should notify children of its new rate
round_rate: Called by a child clock to request best frequency in Hz a parent can produce, given a requested target frequency
set_rate: Called by a child clock to set parent to best frequency in Hz it can produce, given a requested target frequency

To implement these APIs, the clock drivers are expected to make use of the clock driver API. This API has the following functions:

clock_get_rate: Read the rate of a given clock
clock_round_rate: Get the best clock frequency a clock can produce given a requested target frequency
clock_set_rate: Set a clock to the best clock frequency it can produce given a requested target frequency. Also calls clock_lock on the clock to prevent future reconfiguration by clocks besides the one taking ownership
clock_notify_children: Notify all clock children that this clock is about to reconfigure to produce a new rate.
clock_children_check_rate: Verify that children can accept a new rate
clock_children_notify_pre_change: Notify children a clock is about to reconfigure
clock_children_notify_post_change: Notify children a clock has reconfigured

As an example, a vendor multiplexer driver might get its rate like so:

static int vendor_mux_get_rate(const struct clk *clk_hw)
{
	const struct vendor_mux_get_rate *data = clk_hw->hw_data;
	int sel = *data->reg & VND_MUX_MASK;
	
	/* Return rate of active parent */
	return clock_get_rate(data->parents[sel]);
}

SOC devicetrees must define all clock outputs in devicetree. This approach is required because clock producers can reference each node directly in a clock state, in order to configure the clock tree without needing to request a clock frequency and have it applied at runtime.

An SOC clock tree therefore might look like the following:

mydev_clock_mux: mydev-clock-mux@400002b0 {
	compatible = "vnd,clock-mux";
	#clock-cells = <1>;
	reg = <0x400002b0 0x3>;
	offset = <0x0>;
	/* Other clock nodes that provide inputs to this multiplexer */
	input-sources = <&fixed_12m_clk &pll &fixed_96m_clk &no_clock>;
	#address-cells = <1>;
	#size-cells = <1>;

	/* Divider whose parent is this multiplexer */
	mydev_clock_div: mydev-clock-div@40000300 {
		compatible = "vnd,clock-div";
		#clock-cells = <1>;
		reg = <0x40000300 0x8>;

		mydev_clock_source: mydev-clock-source {
			compatible = "clock-output";
			#clock-cells = <1>;
		};
	};
};

Producers can provide specifiers when configuring a node, which will be used by the clock subsystem to determine how to configure the clock. For a clock node with the compatible vnd,clock-compat, the following
macros must be defined:

Z_CLOCK_MGMT_VND_CLOCK_COMPAT_DATA_DEFINE: Defines any static data that is needed to configure this clock
Z_CLOCK_MGMT_VND_CLOCK_COMPAT_DATA_GET: Gets reference to previously defined static data to configure this clock. Cast to a void* and passed to clock_configure. For simple clock drivers, this may be the only definition needed.

For example, the vnd,clock-mux compatible might have one specifier: "selector", and the following macros defined:

/* No data structure needed for mux */
#define Z_CLOCK_MGMT_VND_CLOCK_MUX_DATA_DEFINE(node_id, prop, idx)
/* Get mux configuration value */
#define Z_CLOCK_MGMT_VND_CLOCK_MUX_DATA_GET(node_id, prop, idx)         \
	DT_PHA_BY_IDX(node_id, prop, idx, selector)

The value that Z_CLOCK_MGMT_VND_CLOCK_MUX_DATA_GET expands to will be passed to the clock_configure API call for the driver implementing the vnd,clock-mux compatible. Such an implementation might look like the following:

static int vendor_mux_configure(const struct clk *clk_hw, const void *mux)
{
	const struct vendor_mux_get_rate *data = clk_hw->hw_data;
	int ret;
	uint32_t mux_val = (uint32_t)mux;
	int current_rate = clock_get_rate(clk_hw);
	int new_rate;

	if (mux_val > data->parent_count) {
		return -EINVAL;
	}
	
	new_rate = clock_get_rate(data->parents[mux_val]);

	/* Notify children of new rate, and check if they can accept it */
	ret = clock_children_check_rate(clk_hw, new_rate);
	if (ret < 0) {
		return ret;
	}
	
	/* Notify children we are about to change rate */
	ret = clock_children_notify_pre_change(clk_hw, current_rate, new_rate);
	if (ret < 0) {
		return ret;
	}

	(*data->reg) = mux_val;
	
	/* Notify children we have changed rate */
	ret = clock_children_notify_post_change(clk_hw, current_rate, new_rate);
	if (ret < 0) {
		return ret;
	}
	return 0;
}

A clock state to set the mydev_clock_mux to use the pll clock as input would then look like this:

clocks = <&mydev_clock_mux 1>;

Note the mydev_clock_source leaf node in the clock tree above. These nodes must exist as children of any clock node that can be used by a peripheral, and the peripheral must reference the mydev_clock_source node in its clock-outputs property. The clock management subsystem implements clock drivers for nodes with the clock-output compatible, which handles mapping the clock management APIs to internal clock driver APIs.

Framework Configuration

Since the clock management framework would likely be included with every SOC build, several Kconfigs are defined to enable/disable features that will not be needed for every application, and increase flash usage when enabled. These Kconfig are the following:

CONFIG_CLOCK_MGMT_RUNTIME: Enables clocks to notify children of reconfiguration. Needed any time that peripherals will reconfigure clocks at runtime, or if clock_mgmt_disable_unused is used. Also makes requests from consumers to clock_mgmt_req_rate aggregate, so that if a customer makes a request that the clock accepts it is guaranteed the clock will not change frequency outside of those parameters.
CONFIG_CLOCK_MGMT_SET_RATE: Enables clocks to calculate a new rate and apply it at runtime. When enabled, clock_mgmt_req_rate will use
runtime rate resolution if no statically defined clock states satisfy a request. Also enables CONFIG_CLOCK_MGMT_RUNTIME.

Dependencies

This is of course a large change. I'm opening the RFC early for review, but if we choose this route for clock management we will need to create a tracking issue and follow a transition process similar to how we did for pin control.

Beyond this, there are a few key dependencies I'd like to highlight:

in order to reduce flash utilization (by avoiding linking in children clock nodes an application does not need), I am referencing child clocks by clock handles (which work in a manner similar to device handles). This requires a 2 stage link process for runtime clock support as the first stage link must discard unused clock structures and the second stage link adds clock handles where needed
Some minimal devicetree scripting changes are needed to handle clock states

Flash usage

NOTE: these numbers are subject to change! This is simply present to provide a benchmark of the rough flash impact of the clock framework with/without certain features

The below builds all configure the clock tree to output 96MHz using the internal oscillator to drive the flexcomm0 serial, and configure the LPC55S69's PLL1 to output a core clock at 144MHz (derived from the 16MHz external crystal)

# Baseline, without clock management enabled
$ west build -p always -b lpcxpresso55s69//cpu0 samples/hello_world/ -DCONFIG_CLOCK_MANAGEMENT=n -DCONFIG_CLOCK_CONTROL=y
Memory region         Used Size  Region Size  %age Used
           FLASH:       16578 B       160 KB     10.12%
             RAM:        4240 B       192 KB      2.16%
        USB_SRAM:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%
# Disable clock control, enable clock management. 170 additional bytes of FLASH, 16 bytes of extra RAM
$ west build -p always -b lpcxpresso55s69//cpu0 samples/hello_world/ -DCONFIG_CLOCK_MANAGEMENT=y -DCONFIG_CLOCK_CONTROL=n
Memory region         Used Size  Region Size  %age Used
           FLASH:       16748 B       160 KB     10.22%
             RAM:        4256 B       192 KB      2.16%
        USB_SRAM:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%
# Clock Management with notification support. 2356 additional bytes of FLASH, 56 bytes of RAM		
$ west build -p always -b lpcxpresso55s69//cpu0 samples/hello_world/ -DCONFIG_CLOCK_MANAGEMENT=y -DCONFIG_CLOCK_CONTROL=n -DCONFIG_CLOCK_MANAGEMENT_RUNTIME=y
Memory region         Used Size  Region Size  %age Used
           FLASH:       19104 B       160 KB     11.66%
             RAM:        4312 B       192 KB      2.19%
        USB_SRAM:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%
# Clock Management with runtime rate setting and notification support. 4632 additional bytes of FLASH, 0 bytes of RAM		
$ west build -p always -b lpcxpresso55s69//cpu0 samples/hello_world/ -DCONFIG_CLOCK_MANAGEMENT=y -DCONFIG_CLOCK_CONTROL=n -DCONFIG_CLOCK_MANAGEMENT_RUNTIME=y -DCONFIG_CLOCK_MANAGEMENT_SET_RATE=y
Memory region         Used Size  Region Size  %age Used
           FLASH:       23736 B       160 KB     14.49%
             RAM:        4312 B       192 KB      2.19%
        USB_SRAM:          0 GB        16 KB      0.00%
        IDT_LIST:          0 GB        32 KB      0.00%

Concerns and Unresolved Questions

I'm unsure what the implications of requiring a 2 stage link process for all builds with the clock control framework that have runtime clocking enabled will be for build/testing time overhead.

In many ways, the concept of clock states duplicates operating points in Linux. I'm not sure if we want to instead define clock states as operating points. The benefit of this would be for peripherals (or CPU cores) that support multiple operating points and use a regulator to select them, since we could define the target voltage with the clock state.

Currently, we aggregate clock requests sent via clock_mgmt_req_rate within the clock output driver, and the clock output driver will handle rejecting any attempt to configure a frequency outside of the constraints that have been set on it. While this results in simple application usage, I am not sure if it would instead be better to rely on consumers to reject rates they cannot handle. An approach like this would likely use less flash.

Currently we also issue callbacks at three points:

to validate children can accept a new frequency
before setting a new rate for the clock
after setting a new rate for the clock

We could potentially issue only one callback, directly before reconfiguring the clock. The question here is if this would satisfy all use cases, or are there consumers that need to take a certain action right before their clock reconfigures, and a different action after? One example I can think of is a CPU that needs to raise core voltage before its frequency rises, but would reduce core voltage after its core frequency drops

Alternatives

The primary alternative to this PR would be #70467. That PR implements uses functions to apply clock states, while this PR implements the SOC clock backend using a method much more similar to the common clock framework, but wraps the implementation in a clock management subsystem using states similar to how #70467 does. This allows us to work around the "runtime rate setting" issue, since this feature can now be optional

danieldegrasse · 2025-09-17T23:20:52Z

Nacking the PR as I think the complexity induced on device tree side is not acceptable as is.
Since clock configuration is a base of a zephyr target configuration, this change will hit all users similarly to changes as hwmv2 or pinctrl introduction with the difference that it is significantly more complex.
I want to avoid this is merged too quickly because a smooth transition from clock_control is possible and hence the risk seen lower compared to changes previously mentioned.

@erwango I think this is the last outstanding issue we don't have a great solution for. Something @mathieuchopstm has suggested would be using a format like this for devicetree description:
clocks = <&pll pdiv: 3 mdiv: 4 sscg0: 2 &mux input: 3>
(Note that the meaning of the above values isn't significant- the idea is that we have some kind of labels in phandles).
Do you think this description method would make sense? @decsny, do you think this deviates too far from normal devicetree language to be acceptable? As @mathieuchopstm pointed out, labels can appear anywhere in a property value: devicetree-specification.readthedocs.io/en/stable/source-language.html, so something like reg = reglabel: <0 sizelabel: 0x1000000>; is legal (which makes me think the above format also would be)

What was the purpose of why you are suggesting using the property labels, I don't get it

Essentially to improve readability, and ideally also to add validation. The big difference between what we have now and what this framework proposes is that a clock node defined for the clock control framework can utilize the enum type to enforce a limited range of values for a given devicetree property. This isn't currently possible with phandles. If we had some sort of labels like proposed, we could start enforcing property ranges (and readability would be improved)

BTW did you check the new macros I added lately for phandle arrays? I was imagining/ intended them to be used for clock control specifically (and also am using them for the MUX RFC)

I've looked at them in the context of your MUX RFC, yeah. My understanding is that the macros there are used to encode register writes and the values within one phandle, like so:

mux-states = <&inputmux 0x744 0xc 0x3>; /* Using mask of 0xc, write 0x3 to offset 0x744 within inputmux` */

Were you envisioning a similar usage here, or something different? Clock drivers could definitely define their configuration as a series of register writes, but I'm not sure how that solves the readability/validation problem we have here, unless you were thinking of using those macros differently?

the only thing dts can't do, and never could, is validate the individual values and if they work together, that's up the the driver to do. Unless I'm missing something.

I think the enum level validation is essentially the only thing missing here- you can indeed do all kind of tricks to encode property names in a phandle (up to something like this):

clocks = <&pll0 PDIV 1 MDIV 2 SSCG 2>; /* vendor macro just ignores every other phandle index */

mathieuchopstm · 2025-09-18T12:15:22Z

I think the enum level validation is essentially the only thing missing here- you can indeed do all kind of tricks to encode property names in a phandle (up to something like this):
clocks = <&pll0 PDIV 1 MDIV 2 SSCG 2>; /* vendor macro just ignores every other phandle index */

FWIW, you don't even need to over-declare clock cells, which would make the bindings ugly: you can just #define PDIV /* nothing */ (there should not be any cross-vendor naming conflicts since, by definition, DTS macros (for HW-specific stuff) are not shared between vendors)

@ erwango I think this is the last outstanding issue we don't have a great solution for. Something @ mathieuchopstm has suggested would be using a format like this for devicetree description:
clocks = <&pll pdiv: 3 mdiv: 4 sscg0: 2 &mux input: 3>
(Note that the meaning of the above values isn't significant- the idea is that we have some kind of labels in phandles).

This is a super no, both because it will easily lead to conflicts as a label is a global symbol, so if there already is an input nodelabel somwhere else, it will silently override it and produce really strange errors (node is now suddenly an integer constant?), also, because it is entirely redundant, /* */ comments work in devicetree, and are already used everywhere to document anything.

clocks = <&pll /*pdiv*/ 3 /* mdiv */ 4 sscg0: 2 &mux /* input */ 3>

FTR this suggestion was more of a "this is possible" idea than something serious; as you point out it is definitely a bad idea as labels must be unique in global namespace. (also, I haven't verified whether edtlib accepts freeform labels rather than just nodelabels)

To summarize the main points of a discussion with @danieldegrasse, the main issues with the phandle-array/clock-cells based approach are:

bindings must (almost?) always be cross-referenced to understand a configuration
- unlike properties which are somewhat self-documenting
it is easy to provide an incorrect configuration which can result in various issues (because the initialization order is the order of cells in phandle-array)
- configuration only partially or not at all applied, HW deadlock, ...
- e.g., PLL on STM32H7 (pll_on() will deadlock if source clock isn't enabled)

Using properties - as implemented by myself - helps mitigate these issues but also brings some of its own:

DT_DEP_ORD-ordered initialization is not always correct
each node gets only "one chance" at initializing
- Use case Daniel had in mind is PLLs where the following sequence may be required (e.g., in case a bootloader starts Zephyr with unpredictable clock configuration)
  - Switch MUX source to FIXED_CLK
  - Turn off PLL
  - Configure PLL
  - Turn on PLL
  - Switch MUX source to PLL

We did brainstorm a few ideas to tackle these shortcomings, but nothing has been implemented so far - I plan to revisit this topic when time allows. (which is another reason why going for a collab branch first seems like a better idea to me: there are still many fine print details to discuss, which could require some reworking of the subsystem, but wouldn't hinder mostly-complete implementation of clock drivers from vendors - being a collab branch would allow incrementally refining the subsystem more easily that if working on main)

Note by the way that the properties-based system would only work for boot-time initialization. Anything dynamic will have to be done through phandle-arrays/clock-states/etc (but my assumption is that boot-time init should be sufficient for most scenarios, allowing most users to be blissfully unaware of gory details)

mathieuchopstm · 2025-09-18T15:29:15Z

A few points after reading the documentation:

as far as I can tell, CMS is still missing a clock_control_get_status() equivalent?
as remarked by @etienne-lms internally, sometimes we want to perform off/on from ISR
- right now, the whole subsystem is locked via a k_mutex (which can't be taken while in ISR as far as I know?); moving to a k_spinlock is not ideal though
- other solution is to forbid off/on from ISR, but I think this is a somewhat common pattern? (to be confirmed)
I think others made this remark too: the clock-output naming on consumer-side is quite confusing
Quoting documentation: Clock drivers will must hold a reference to their parent clock device, if one exists.
- for a multiplexer, one might want to disable some of the inputs (e.g., to save footprint) - should this be allowed? if yes, subsystem should provide a mechanism for that

Overall, the documentation seems fair: there are a few typos here and there, and some things could be reworked a bit, but it gives a reasonable overview. I added some feedback on #94679 to avoid cluttering this PR's discussion feed too much.

Somewhat unrelated but another thing that came to my mind is a fairly common situation on STM32: some IPs have two clock inputs (per_bus_ck and per_ker_ck), but both inputs are gated by "the same gate"[1]. I'm wondering how this kind of gates should be represented in CMS.

[1] Not physically but the same bit in RCC controls both gates - see below example where ADC12EN would be "one gate":

decsny · 2025-09-18T18:18:13Z

Nacking the PR as I think the complexity induced on device tree side is not acceptable as is.
Since clock configuration is a base of a zephyr target configuration, this change will hit all users similarly to changes as hwmv2 or pinctrl introduction with the difference that it is significantly more complex.
I want to avoid this is merged too quickly because a smooth transition from clock_control is possible and hence the risk seen lower compared to changes previously mentioned.

@erwango I think this is the last outstanding issue we don't have a great solution for. Something @mathieuchopstm has suggested would be using a format like this for devicetree description:
clocks = <&pll pdiv: 3 mdiv: 4 sscg0: 2 &mux input: 3>
(Note that the meaning of the above values isn't significant- the idea is that we have some kind of labels in phandles).
Do you think this description method would make sense? @decsny, do you think this deviates too far from normal devicetree language to be acceptable? As @mathieuchopstm pointed out, labels can appear anywhere in a property value: devicetree-specification.readthedocs.io/en/stable/source-language.html, so something like reg = reglabel: <0 sizelabel: 0x1000000>; is legal (which makes me think the above format also would be)

What was the purpose of why you are suggesting using the property labels, I don't get it

Essentially to improve readability, and ideally also to add validation. The big difference between what we have now and what this framework proposes is that a clock node defined for the clock control framework can utilize the enum type to enforce a limited range of values for a given devicetree property. This isn't currently possible with phandles. If we had some sort of labels like proposed, we could start enforcing property ranges (and readability would be improved)

BTW did you check the new macros I added lately for phandle arrays? I was imagining/ intended them to be used for clock control specifically (and also am using them for the MUX RFC)

I've looked at them in the context of your MUX RFC, yeah. My understanding is that the macros there are used to encode register writes and the values within one phandle, like so:
mux-states = <&inputmux 0x744 0xc 0x3>; /* Using mask of 0xc, write 0x3 to offset 0x744 within inputmux` */
Were you envisioning a similar usage here, or something different? Clock drivers could definitely define their configuration as a series of register writes, but I'm not sure how that solves the readability/validation problem we have here, unless you were thinking of using those macros differently?

No, the PHA macros I add are to put any arbitrary data in phandle specifiers. On that PR, there is 3 drivers being added, and only one of them is "encoding register writes" in the specifier. One of the others is just an index into a list on a different node, and the last is corresponding to a number representing what should the gpio select lines of an external circuit should be.

So, I don't know what data would be useful to put in the specifier here, but it doesn't strictly have to be register addresses and values, although that is an option.

decsny · 2025-09-18T18:28:07Z

@ erwango I think this is the last outstanding issue we don't have a great solution for. Something @ mathieuchopstm has suggested would be using a format like this for devicetree description:
clocks = <&pll pdiv: 3 mdiv: 4 sscg0: 2 &mux input: 3>
(Note that the meaning of the above values isn't significant- the idea is that we have some kind of labels in phandles).

This is a super no, both because it will easily lead to conflicts as a label is a global symbol, so if there already is an input nodelabel somwhere else, it will silently override it and produce really strange errors (node is now suddenly an integer constant?), also, because it is entirely redundant, /* */ comments work in devicetree, and are already used everywhere to document anything.
clocks = <&pll /*pdiv*/ 3 /* mdiv */ 4 sscg0: 2 &mux /* input */ 3>

FTR this suggestion was more of a "this is possible" idea than something serious; as you point out it is definitely a bad idea as labels must be unique in global namespace. (also, I haven't verified whether edtlib accepts freeform labels rather than just nodelabels)

To summarize the main points of a discussion with @danieldegrasse, the main issues with the phandle-array/clock-cells based approach are:
* bindings must (almost?) always be cross-referenced to understand a configuration

In my opinion , this is a non-issue. We already had plenty of phandle arrays in tree in zephyr for all sorts of things, and not to mention every other project that uses DT such as linux or uboot, and nobody ever complained about this.

  * unlike properties which are somewhat self-documenting

In my opinion, the explosion of so many properties and clock bindings in the way this PR is right now, is more of a confusing thing, it's so much chaos to sort through from my perspective. I would like to see some more monolithic clock controller devices and less bindings and properties.

* it is easy to provide an incorrect configuration which can result in various issues (because the initialization order is the order of cells in `phandle-array`)
  * configuration only partially or not at all applied, HW deadlock, ...
  * e.g., PLL on STM32H7 (`pll_on()` will deadlock if source clock isn't enabled)

This seems like an implementation problem, not a design problem with using phandle specifiers. Can you give an example of what you mean.

We did brainstorm a few ideas to tackle these shortcomings, but nothing has been implemented so far - I plan to revisit this topic when time allows. (which is another reason why going for a collab branch first seems like a better idea to me: there are still many fine print details to discuss, which could require some reworking of the subsystem, but wouldn't hinder mostly-complete implementation of clock drivers from vendors - being a collab branch would allow incrementally refining the subsystem more easily that if working on main)

Eventually, I imagine this will turn into a collab branch, but even a collab branch will be sticky because major changes will require reworking every other SOC vendor SOC. So until we have the clear idea that everybody mostly agrees on of what we want to do, since this is such a herculean task, then we shouldn't make any shared branch at all IMO.

Now also I am not saying that it's up to Daniel to be reworking this PR until infinity during this whole thing - I would like to see some more people making their own branches and showing PoCs of their ideas, this is something I was going to do if I find time but, yes, this is a herculean task so that is asking a lot. In fact, I haven't had appropriate time to even review this PR.

Note by the way that the properties-based system would only work for boot-time initialization. Anything dynamic will have to be done through phandle-arrays/clock-states/etc (but my assumption is that boot-time init should be sufficient for most scenarios, allowing most users to be blissfully unaware of gory details)

Exactly another point of using phandle specifiers, you can use *-names property to correlate, store, and, arbitrarily reference different configurations from DT to switch between at runtime. With properties, everything seems a lot less flexible, and to get any flexibility, you have to describe these weird gigantic "states" descriptions.

decsny · 2025-09-18T18:33:22Z

I also think this RFC might be trying to do too much right now. I don't think we should be trying to negotiate the configuration of PLLs and oscillators between different consumers at runtime right now, it's just adding to the complexity a lot. Reconfiguring them based on power states seems like a reasonable goal right now, and is the main reason I see anybody even wanting to reconfigure them. And even tying them into the Zephyr PM domains and runtime device PM to place constraints on the clocks being disabled using pm_device_put/get instead of going through such a complicated callback process, seems like a better idea.

Maybe instead of getting lost in implementation discussions, we need to back up here and ask, what even are the specific requirements and use cases of a "clock manager" that we are trying to meet, that we can't do today. Because right now I am confused about that, this discussion has been all over the place with people saying a load of different stuff for over a year now, it doesn't seem focused on specific goals to me.

danieldegrasse · 2025-09-18T21:30:21Z

Somewhat unrelated but another thing that came to my mind is a fairly common situation on STM32: some IPs have two clock inputs (per_bus_ck and per_ker_ck), but both inputs are gated by "the same gate"[1]. I'm wondering how this kind of gates should be represented in CMS.

Currently you'd have to define the gate twice, something like this:

The framework doesn't have a concept of multi-parent clocks that don't permit selection currently, and IMO it shouldn't. Otherwise how would you query the rate of this clock?

If both gate clocks share the same set of children, then they would be disabled by clock_management_off at the same time, so writing to the register twice would be fine.

danieldegrasse · 2025-09-18T21:41:07Z

as far as I can tell, CMS is still missing a clock_control_get_status() equivalent?

It is- but what would adding a function like this accomplish? In my vision, a user would consume the API like this:

# Turn on clock. When this returns, clock and its dependencies will be enabled.
clock_management_on()
# Query clock rate. If the return value is 0 or negative, the clock is in a bad state
clock_management_get_rate()

I'm unclear what status the user would need besides:

clock is on and running: clock_management_get_rate() returns positive value
clock is gated: clock_management_get_rate() returns 0
clock is in a bad state: clock_management_get_rate() returns negative value

as remarked by @etienne-lms internally, sometimes we want to perform off/on from ISR

I guess we need to figure out if this is a common use case. My expectation was that clocks would usually be turned on at init, and disabled in power management handlers

Quoting documentation: Clock drivers will must hold a reference to their parent clock device, if one exists. for a multiplexer, one might want to disable some of the inputs (e.g., to save footprint) - should this be allowed? if yes, subsystem should provide a mechanism for that

I'll be honest- I don't think this is something we should support. If a user really wants to optimize their build like this, they can overwrite the dt property of a multiplexer to remove parent clock nodes. I don't think we should allow clock drivers themselves to optimize parents away.

Overall, the documentation seems fair: there are a few typos here and there, and some things could be reworked a bit, but it gives a reasonable overview. I added some feedback on #94679 to avoid cluttering this PR's discussion feed too much.

Appreciate the feedback- will work on making updates in response when I have some BW.

decsny · 2025-09-18T21:47:30Z

The core goal of this change is to provide a more device-agnostic way to manage clocks. Although the clock control subsystem does define clocks as an opaque type, drivers themselves still often need to be aware of the underlying type behind this opaque definition, and there is no standard for how many cells will be present on a given clock controller, so implementation details of the clock driver are prone to leaking into drivers. This presents a problem for vendors that reuse IP blocks across SOC lines with different clock controller drivers.

The actual main problem description in this PR is actually solved by just refactoring the clock control system we already have to just use macros in the similar way to how I did in the mux RFC, which makes it totally generic to consumers. And these macros are already merged in tree available to use in a similar fashion. So, we don't need a whole new clock framework just to solve this particular problem.

Beyond this, the clock control subsystem doesn't provide a simple way to configure clocks. clock_control_configure and clock_control_set_rate are both available, but clock_control_configure is ripe for leaking implementation details to the driver,

Same problem as above actually, it's just naming the particular function which requires the opaque information.

and clock_control_set_rate will likely require runtime calculations to achieve requested clock rates that aren't realistic for small embedded devices

What makes them unrealistic with clock_control but realistic with this "clock management subsystem"? Which BTW, is not even architected like a classical subsystem with self-contained central logic, but rather a bunch of tiny drivers doing callbacks to each other and a some scripting hacks which are hard to understand. So again, I ask the question, what exactly are we trying to achieve here, if runtime reconfiguring is the goal, what are the use cases we actually desire to support? Let's focus the discussion.

(or leak implementation details, if clock_control_subsys_rate_t isn't an integer)

Ditto again from above, solved by already existing DT macros and already possible consumer paradigms.

decsny

Not clear what we are even trying to accomplish with a clock manager framework, the problem description in the original post is mostly already achievable by just refactoring the ways consumers retrieve the opaque DT data to use the existing clock control API.

I would like to see a clear list of requirements and use cases that a clock manager framework is actually supposed to be meeting and solving that we cannot do today. I am sure there are some, but we need to be focused on the problems, not lost in our keyboards continually redefining an implementation for something that we are not even clear what is the goal of.

decsny · 2025-08-29T15:23:17Z

soc/nxp/lpc/lpc55xxx/soc.c

+	(DT_NODE_HAS_COMPAT_STATUS(DT_NODELABEL(flexcomm4), nxp_lpc_spi, okay) && \
+	CONFIG_SPI_MCUX_FLEXCOMM) || \
+	(DT_NODE_HAS_COMPAT_STATUS(DT_NODELABEL(flexcomm4), nxp_lpc_usart, okay) && \
+	(CONFIG_UART_MCUX_FLEXCOMM && !defined(CONFIG_CLOCK_MANAGEMENT)))


I guess you are keeping the old default clocking code for backwards compatibility if someone with this board doesnt want to use clock management?

It seems like instead of appending && !defined(CONFIG_CLOCK_MANAGEMENT) on every ifdef , you can just wrap the all if it in one #if defined(CONFIG_CLOCK_MANAGEMENT)

Yeah, can update this next time I push. Did this for backwards compatibility and to be sure the clock framework was actually configuring the UART clock.

decsny · 2025-08-29T15:26:42Z

dts/bindings/clock-management/clock-device.yaml

+  clock-state-0:
+    type: phandles
+    description: |
+      Clock configuration/s for the first state. Content should be a series of
+      references to the clock nodes declared as children of the clock
+      controller. The specifier cells to these clock nodes are specific to the
+      implementation of the system's clock controller
+
+  clock-state-1:
+    type: phandles
+    description: |
+      Clock configuration/s for the second state. See clock-state-0.
+
+  clock-state-2:
+    type: phandles
+    description: |
+      Clock configuration/s for the third state. See clock-state-0.
+
+  clock-state-3:
+    type: phandles
+    description: |
+      Clock configuration/s for the fourth state. See clock-state-0.
+
+  clock-state-4:
+    type: phandles
+    description: |
+      Clock configuration/s for the fifth state. See clock-state-0.


why is it like this? just put an arbitrary number of phandles on one clock-states property?

Did this to match the way that pinctrl is defined. Also, we have the ability to have multiple output clocks per node. So something like the following is possible:

emul_dev1: emul-dev1 { compatible = "vnd,emul-clock-consumer"; clock-outputs = <&dev1_out_slow &dev1_out_fast>; clock-output-names = "slow", "fast"; clock-state-0 = <&dev1_10mhz &dev1_100mhz>; clock-state-1 = <&dev1_3mhz &dev1_25mhz>; clock-state-names = "default", "sleep"; };

dev1_10mhz corresponds to the default state for clock output dev1_out_slow, and dev1_100mhz corresponds to the default state for clock output dev1_out_fast

danieldegrasse · 2025-09-18T21:53:35Z

I also think this RFC might be trying to do too much right now. I don't think we should be trying to negotiate the configuration of PLLs and oscillators between different consumers at runtime right now, it's just adding to the complexity a lot. Reconfiguring them based on power states seems like a reasonable goal right now, and is the main reason I see anybody even wanting to reconfigure them. And even tying them into the Zephyr PM domains and runtime device PM to place constraints on the clocks being disabled using pm_device_put/get instead of going through such a complicated callback process, seems like a better idea.

I'll be honest, that's the core goal of this framework- I want a clock consumer to be able to request a new clock setting, and have other consumers get notified about that change (and optionally reject it)

The other approach (as you mentioned) is monolithic clock power states. I a while back in #70467, using the nomenclature of "setpoints". There was some good discussion there. In essence the use case that drove the move to this framework was something like what is described here: #70467

Essentially:

clock consumers: X, Y. Clock sources A, B, C
If X and Y are running, source C must be used (high power)
If X is running and Y is off, source A can be used
If Y is running and X is off, source B can be used

There isn't really a good way to do this with monolithic power states- you are going to end up with a huge matrix of static clock tree definitions for every dependency like this in a system.

danieldegrasse · 2025-09-18T22:22:41Z

Not clear what we are even trying to accomplish with a clock manager framework, the problem description in the original post is mostly already achievable by just refactoring the ways consumers retrieve the opaque DT data to use the existing clock control API.

You're right- the initial post doesn't really describe the problem well. The initial post was also written 1.5 years ago, and last edited 9 months back. The framework has undergone at least 2 major reworks since then in response to feedback. I'll attempt to describe the goals of the framework as it exists now below. If you'd like me to edit the description of the PR itself to integrate this, I can do so.

Introduction

This PR proposes a clock management subsystem. The core goal of the subsystem is to support runtime reconfiguration of the clock tree, by handling dependencies between clock producers and notifying clock consumers as appropriate. This subsystem continues to support static configuration like what is possible using the clock control framework today, but encapsulates SOC clock trees into a series of clock producer drivers, which are opaque to clock consumers and managed by the subsystem itself.

Problem description

Currently, we support some level of runtime clock configuration in Zephyr- however this configuration is accomplished by two functions: clock_control_set_rate and clock_control_configure. Both of these functions take in driver specific data for clock management, which fundamentally couples clock consumer code to the clock producer driver.

Beyond this, there is no generic framework for selecting the best clock given a requested rate. This is a relatively generic problem, with an implementation like the following:

query all parent clocks to see what rate they can produce given a requested rate
validate that the most accurate rate offered by a parent works for all consumers of that parent
set the clock to the rate it offered

If SOC clock drivers want to implement this functionality, they will need to do it for their specific clock tree. Moreover, the fact that clock APIs don't include any sort of "query rate" API means that clock drivers are inherently going to be monolithic, limiting code reuse and precluding generic clock drivers entirely.

To be specific, a request that warrants this type of runtime resolution might be something like the following:

Consumer X and Y both source from the same multiplexer, selecting sources A, B and C.
Consumer X can clock from source A (low power) or C (high power)
Consumer Y can clock from source B (low power) or C (high power)
When X and Y are on, source C must be on. Otherwise we should use the lower power source

In the current clock control driver design, code that should otherwise be generic would need to be placed in the vendor's driver to perform this kind of resolution. With the clock management framework, the resolution might look like this:

Consumer X starts on clock A
Consumer Y boots, requests clock B. Consumer X rejects clock A, so clock C is used (as both consumers accept it)
Consumer X powers down. It clears its clock request, and the clock framework moves consumer Y to clock B

Proposed change

The core of this subsystem is splitting clock producers into "clock drivers". Each driver implements a clock API described here: https://builds.zephyrproject.io/zephyr/pr/94679/docs/hardware/clock_management/index.html#implementation-guidelines. Drivers can be configured using vendor-specific data encoded into devicetree in "clock states", or can be configured by a runtime request.

The clock management subsystem handles generic tasks like selecting the best clock producer for a given request, and validating a clock rate with consumers.

The subsystem is designed with 3 levels of configuration:

basic: only static clock configuration is supported. This configuration aims to use the lowest flash footprint possible versus static clock configuration with the existing clock control drivers while providing feature parity
runtime: Clock states can be applied at runtime. Other clock consumers affected by the clock state will be notified of the reconfiguration, and can reject it
set rate: Clock consumers can request a specific integer frequency range at runtime, and the clock management framework will interface with clock drivers to provide the most accurate (or highest ranked) clock for the frequency request

mathieuchopstm · 2025-09-19T09:32:17Z

* it is easy to provide an incorrect configuration which can result in various issues (because the initialization order is the order of cells in `phandle-array`)
  * configuration only partially or not at all applied, HW deadlock, ...
  * e.g., PLL on STM32H7 (`pll_on()` will deadlock if source clock isn't enabled)

This seems like an implementation problem, not a design problem with using phandle specifiers. Can you give an example of what you mean.

I'll use H7 as example again which has three PLLs:

There's a shared "PLL SRC MUX" which selects the clock source for the 3 PLLs
- This mux can only be configured when all PLLs are off
- This mux does not switch source if both old and new are not active
Each PLL has a "prescaler+VCO" block sourced from the mux and three outputs
- f_PLLn_PVCO = f_PLLSRCMUX * (PLLn_divn / PLLn_divm)
- f_PLLn_OUTx = f_PLLn_PVCO / PLLn_OUTx_div if enabled
- PLL output blocks can be disabled/enabled only when PVCO block is disabled

The DTS would be something like:

pll_src_mux {
    inputs = <&generator ...>;

    plln_pvco {
        plln_outx { /* ... */ };
        plln_outy { /* ... */ };
        plln_outz { /* ... */ };
    };

    pllm_pvco { /* ... */ };
    pllo_pvco { /* ... */ };
};

And the initialization sequence:

clocks = <&generator ...>,   /* (a) */
         <&pll_src_mux ...>, /* (b) */
         <&plln_outx ...>,   /* |
         <&plln_outy ...>,    * |--> (c)
         <&plln_outz ...>,    * |
                              */
         <&plln_pvco ...>;   /* (d) */

If (a) is not before (b) in phandle-array, a deadlock occurs (mux will not change source). If (b) is not before (d), a deadlock may occur depending on RESET configuration (PLL fails to lock if source is not active), and either way the PLLs won't run with the intended source. If (c) is not before (d), the PLL will run but the expected configuration won't be applied.

The advantage of a properties-based approach is that we (as in, whoever implements the SoC side) could enforce a specific initialization order, ensuring that such issues cannot occur.

Note by the way that the properties-based system would only work for boot-time initialization. Anything dynamic will have to be done through phandle-arrays/clock-states/etc (but my assumption is that boot-time init should be sufficient for most scenarios, allowing most users to be blissfully unaware of gory details)

Exactly another point of using phandle specifiers, you can use *-names property to correlate, store, and, arbitrarily reference different configurations from DT to switch between at runtime. With properties, everything seems a lot less flexible, and to get any flexibility, you have to describe these weird gigantic "states" descriptions.

Besides what I described above, the big reason for a properties-based proposal was also that a single boot-time configuration is what (almost?) everyone uses today in Zephyr - unless I missed it and there's already a platform with dynamic management?
I recall a discussion with @henrikbrixandersen about this topic, though I don't know if we have any metric about how many users shipped products with a static configuration vs. something dynamic (how much of that is due to Zephyr lacking a framework for dynamic configuration is up for debate - I do think we had someone saying here they had their own dynamic framework, collecting their feedback could be interesting)

as far as I can tell, CMS is still missing a clock_control_get_status() equivalent?

It is- but what would adding a function like this accomplish?

The use case I keep thinking of is the STM32 TRNG driver where we query the clock state to know if driver is being called in re-entrant fashion from ISR (well, really, that's not how it's done today but was how I planned to do it). I guess a "refcount" would be an acceptable alternative though.

I must admit I didn't think about using clock_management_get_rate() as an indirect way to obtain status though - it could be acceptable.

as remarked by @etienne-lms internally, sometimes we want to perform off/on from ISR

I guess we need to figure out if this is a common use case. My expectation was that clocks would usually be turned on at init, and disabled in power management handlers

For context, again this came mostly from the STM32 TRNG driver which automatically starts/stops itself depending on the state of its internal entropy pools. Maybe this implementation is not correct w.r.t. Zephyr's PM framework.

The core goal of this change is to provide a more device-agnostic way to manage clocks. Although the clock control subsystem does define clocks as an opaque type, drivers themselves still often need to be aware of the underlying type behind this opaque definition, and there is no standard for how many cells will be present on a given clock controller, so implementation details of the clock driver are prone to leaking into drivers. This presents a problem for vendors that reuse IP blocks across SOC lines with different clock controller drivers.

The actual main problem description in this PR is actually solved by just refactoring the clock control system we already have to just use macros in the similar way to how I did in the mux RFC, which makes it totally generic to consumers. And these macros are already merged in tree available to use in a similar fashion. So, we don't need a whole new clock framework just to solve this particular problem.

Even ignoring the whole RUNTIME aspect, changing how clocks are described in Zephyr as done in this RFC solves more than just the "generic consumers" problem. The two other main issues that have been brought up are that, in Clock Control, you effectively have one giant per-SoC(-series) driver that is a pain to write and maintain; in addition, you end up encoding the device's clock tree in code anyways because it's needed for clock_control_get_rate().

By having the clock tree in DTS instead, you get clock_management_get_rate() for "free", and your drivers are now small, simple and reusable across SoC series (e.g., I only had to write two additional drivers to add support for STM32H7 on top of STM32C0 in my CMS implementation - all other drivers were reused). For the record, though these figures should be taken with extreme caution:

$ wc -l zephyr/drivers/clock_management/stm32/*
  186 clock_management_stm32.h
   20 CMakeLists.txt
   89 Kconfig
  106 stm32_bus_prescaler.c
   66 stm32c0_hsisys_div.c
   73 stm32_clock_gate.c
  104 stm32_clock_generator.c
  285 stm32_clock_management_common.c
  126 stm32_clock_management_common.h
   90 stm32_clock_multiplexer.c
   76 stm32_h7_pll_output.c
  171 stm32_h7_pll_pvco.c
  124 stm32_internal_clkgen.c
   72 stm32_sysclk_prescaler.c
   89 stm32_timer_freqmul.c
 1677 total

$ wc -l zephyr/drivers/clock_control/clock_stm32_ll_*
  119 clock_stm32_ll_common.h       <--- shared by all clock_control STM32 drivers
 1090 clock_stm32_ll_common.c       <--- shared by a few series because clock tree is identical (C0, F1, ...)
 1165 clock_stm32_ll_h7.c
 2374 total

The downside is that this does increase FLASH footprint: some figures have been posted several times in this thread, the ballpark is around 300~400B on a "simple" product like STM32C0. A change in scope and paradigm can definitely reduce that (e.g., "each DT node must describe operation performed on input clock" ==> solve entire clock tree at build time ==> leave only leaf nodes (clock gates) as instances in the final image + write-what-where sequence executed during initialization); this brings back your legitimate question about "what are we trying to achieve" which may need to be discussed more formally again.

decsny · 2025-09-19T13:37:55Z

drivers/clock_management/clock_management_common.c

+ * @param ev_type Type of clock notification event
+ * @return 0 if notification chain succeeded, or error if not
+ */
+static int clock_notify_children(const struct clk *clk_hw,


no offense, but in my opinion this function is written terribly, there is WAY too much block nesting and cognitive complexity, it needs to be refactored. I suggest looking at the SonarQubeCloud suggestions as it has found 41 issues on this PR which is the most I've ever seen (probably because it is a huge PR)

Sure, I can refactor this into sub functions for each clock type. I'll take a look through the sonarcube- some of those suggestions are very good (I'll admit I initially discounted it when I saw that because it was complaining about recursion, which this PR needs to function)

decsny · 2025-09-19T14:51:36Z

Not clear what we are even trying to accomplish with a clock manager framework, the problem description in the original post is mostly already achievable by just refactoring the ways consumers retrieve the opaque DT data to use the existing clock control API.

You're right- the initial post doesn't really describe the problem well. The initial post was also written 1.5 years ago, and last edited 9 months back. The framework has undergone at least 2 major reworks since then in response to feedback. I'll attempt to describe the goals of the framework as it exists now below. If you'd like me to edit the description of the PR itself to integrate this, I can do so.

If the original post is way out of date, then yes, I think you should update it.

Introduction

This PR proposes a clock management subsystem. The core goal of the subsystem is to support runtime reconfiguration of the clock tree, by handling dependencies between clock producers and notifying clock consumers as appropriate. This subsystem continues to support static configuration like what is possible using the clock control framework today, but encapsulates SOC clock trees into a series of clock producer drivers, which are opaque to clock consumers and managed by the subsystem itself.

I am still confused what is meant by "managed by the subsystem itself", maybe I am just missing something, but I see no subsystem being added on this PR, only a driver class.

Problem description

Currently, we support some level of runtime clock configuration in Zephyr- however this configuration is accomplished by two functions: clock_control_set_rate and clock_control_configure. Both of these functions take in driver specific data for clock management, which fundamentally couples clock consumer code to the clock producer driver.

I don't agree still with this problem description, this is just an implementation problem, see again what I said above about we can use the now-existing macros to form opaque arrays of arbitrary data from DT. So IMO the existing clock control API does not "fundamentally" have a design which couples consumers to specific drivers. If I am not being clear about this, maybe I can try to make a PR to add some helper macros and show some kind of example (although I admit, refactoring some of these platform to use the clock control API properly, is a big task of its own).

Beyond this, there is no generic framework for selecting the best clock given a requested rate. This is a relatively generic problem, with an implementation like the following:
* query all parent clocks to see what rate they can produce given a requested rate

* validate that the most accurate rate offered by a parent works for all consumers of that parent

* set the clock to the rate it offered
If SOC clock drivers want to implement this functionality, they will need to do it for their specific clock tree.

Instead of focusing on the implementation in the problem description, I think it should be describing the requirements of rate requesting. I am not interested in the implementation details when I am reviewing this architecture proposal, I want to know, what exact problem(s) are we trying to solve and what are the characteristics of any solution that would address them.

Moreover, the fact that clock APIs don't include any sort of "query rate" API means that clock drivers are inherently going to be monolithic, limiting code reuse and precluding generic clock drivers entirely.

I don't know what this is supposed to mean, honestly. What does having a query rate API have to do with precluding generic clock drivers and code reuse? What even is a "generic clock driver"? You need to define these terms you're using.

To be specific, a request that warrants this type of runtime resolution might be something like the following:
* Consumer X and Y both source from the same multiplexer, selecting sources A, B and C.

* Consumer X can clock from source A (low power) or C (high power)

* Consumer Y can clock from source B (low power) or C (high power)

* When X and Y are on, source C must be on. Otherwise we should use the lower power source
In the current clock control driver design, code that should otherwise be generic would need to be placed in the vendor's driver to perform this kind of resolution. With the clock management framework, the resolution might look like this:
* Consumer X starts on clock A

* Consumer Y boots, requests clock B. Consumer X rejects clock A, so clock C is used (as both consumers accept it)

* Consumer X powers down. It clears its clock request, and the clock framework moves consumer Y to clock B

I will say, this sounds very similar to power domains structure that PM subsystem has, and I suggest to take some inspiration from how that is designed, because IMO it has (or is in the process of) being designed very coherently. It is essentially another example of a "constraint based dependency tree manager". @bjarki-andreasen maybe can comment about this.

Proposed change

I am just wondering, have you considered putting clock object or callback in the device model, similar to PM?

The subsystem is designed with 3 levels of configuration:

* basic: only static clock configuration is supported. This configuration aims to use the lowest flash footprint possible versus static clock configuration with the existing clock control drivers while providing feature parity

For the "basic configuration" I wonder why we can't just use the existing clock control drivers and API, if it's supposed to be the same.

* runtime: Clock states can be applied at runtime. Other clock consumers affected by the clock state will be notified of the reconfiguration, and can reject it

It seems like this can be done by extending the device model with a clock callback and extending the clock control API by having it be allowed to perform this behavior when clock_control_configure is called.

And I understand you have a notion of "clock devices" which are miniature polymorphic objects to optimize instead of using full "devices" for every clock. But my comment again has been and still is, that we should not be having a "driver" for every single clock source, mux, div, whatever, in the SOC. The driver should be for a reusable and cohesive IP module from a hardware vendor, not trying to slice it up into all these mini drivers, which, if you're worried about ROM usage, IMO is probably going to be worse. The current clock control drivers are mostly written badly and use #ifdef to control rom usage, but they can be written much more simply if they just made use of DT phandles properly and if consumers did not intentionally couple themself to a driver, when it's not even necessary.

I think the issue you are trying to solve is that a lot of current clock control driver, cast an integer into a void *, which is just wrong and not allowing them to actually use the existing flexibility of clock control API at all. So why don't we try to first use the clock control API as it was intended and see what extensions are needed instead of reinventing everything from scratch?

* set rate: Clock consumers can request a specific integer frequency range at runtime, and the clock management framework will interface with clock drivers to provide the most accurate (or highest ranked) clock for the frequency request

Yeah, this , is more complicated, but again my question still stands from before, which is what are the actual use cases for all of this that we are trying to do. You gave a lot of abstract examples in the form of talking about a theoretical X, Y, A, B, C, okay, but I am not interested in letters and academic derivations, what I want us to be focused on here is what are the actual technical use cases we are trying to accomplish with a clock control or manager framework. WE need to stay focused on these actual goals when we are considering doing this major architectural change, it should not be theoretical and academic in motivation.

decsny · 2025-09-19T16:04:49Z

* it is easy to provide an incorrect configuration which can result in various issues (because the initialization order is the order of cells in `phandle-array`)
  * configuration only partially or not at all applied, HW deadlock, ...
  * e.g., PLL on STM32H7 (`pll_on()` will deadlock if source clock isn't enabled)
This seems like an implementation problem, not a design problem with using phandle specifiers. Can you give an example of what you mean.
I'll use H7 as example again which has three PLLs:
* There's a shared "PLL SRC MUX" which selects the clock source for the 3 PLLs
  
  * This mux can only be configured when all PLLs are off
  * _This mux does not switch source if both old and new are not active_

* Each PLL has a "prescaler+VCO" block sourced from the mux and three outputs
  
  * `f_PLLn_PVCO = f_PLLSRCMUX * (PLLn_divn / PLLn_divm)`
  * `f_PLLn_OUTx = f_PLLn_PVCO / PLLn_OUTx_div` _if enabled_
  * PLL output blocks can be disabled/enabled only when PVCO block is disabled
The DTS would be something like:
pll_src_mux {
    inputs = <&generator ...>;

    plln_pvco {
        plln_outx { /* ... */ };
        plln_outy { /* ... */ };
        plln_outz { /* ... */ };
    };

    pllm_pvco { /* ... */ };
    pllo_pvco { /* ... */ };
};
And the initialization sequence:
clocks = <&generator ...>,   /* (a) */
         <&pll_src_mux ...>, /* (b) */
         <&plln_outx ...>,   /* |
         <&plln_outy ...>,    * |--> (c)
         <&plln_outz ...>,    * |
                              */
         <&plln_pvco ...>;   /* (d) */
If (a) is not before (b) in phandle-array, a deadlock occurs (mux will not change source). If (b) is not before (d), a deadlock may occur depending on RESET configuration (PLL fails to lock if source is not active), and either way the PLLs won't run with the intended source. If (c) is not before (d), the PLL will run but the expected configuration won't be applied.

The advantage of a properties-based approach is that we (as in, whoever implements the SoC side) could enforce a specific initialization order, ensuring that such issues cannot occur.

This is not a problem with the concept of using phandles instead of nodes-based hierarchies and properties. This is a problem, again, which I have said, of having WAY too many "clock drivers" because of the way this RFC is designed right now. A clock consumer referencing it's clocks with a clocks property should not have to reference 500 clock nodes just to configure it's one clock input. All of this stuff, should be figured out either hierarchically (which I THOUGHT was the design goal of this PR), or by a monolithic clock device driver representing an actual hardware IP block which is providing multiple outputs (what is MEANT to be how zephyr clock_control works, but pretty much every current implementation is a terrible example), or a mixture of both (linux approach). Not "every device has to have a giant phandle array referencing 500 clocks they need to configure". That is way overly complicated. This again is still looking like at the very least an implementation problem, or, a fundamental design problem with the RFC. It is hard for me to tell which at this point because this is such a gargantuan PR with so many things and tricks and crossplay going on. Can't we try to keep things simple?

Note by the way that the properties-based system would only work for boot-time initialization. Anything dynamic will have to be done through phandle-arrays/clock-states/etc (but my assumption is that boot-time init should be sufficient for most scenarios, allowing most users to be blissfully unaware of gory details)

Exactly another point of using phandle specifiers, you can use *-names property to correlate, store, and, arbitrarily reference different configurations from DT to switch between at runtime. With properties, everything seems a lot less flexible, and to get any flexibility, you have to describe these weird gigantic "states" descriptions.

Besides what I described above, the big reason for a properties-based proposal was also that a single boot-time configuration is what (almost?) everyone uses today in Zephyr - unless I missed it and there's already a platform with dynamic management? I recall a discussion with @henrikbrixandersen about this topic, though I don't know if we have any metric about how many users shipped products with a static configuration vs. something dynamic (how much of that is due to Zephyr lacking a framework for dynamic configuration is up for debate - I do think we had someone saying here they had their own dynamic framework, collecting their feedback could be interesting)

I don't understand, are you trying to say the phandles specifier form would not allow for static configuration? Because that's how we actually are doing static configuration right now for NXP. So I don't get your point here.

The core goal of this change is to provide a more device-agnostic way to manage clocks. Although the clock control subsystem does define clocks as an opaque type, drivers themselves still often need to be aware of the underlying type behind this opaque definition, and there is no standard for how many cells will be present on a given clock controller, so implementation details of the clock driver are prone to leaking into drivers. This presents a problem for vendors that reuse IP blocks across SOC lines with different clock controller drivers.

The actual main problem description in this PR is actually solved by just refactoring the clock control system we already have to just use macros in the similar way to how I did in the mux RFC, which makes it totally generic to consumers. And these macros are already merged in tree available to use in a similar fashion. So, we don't need a whole new clock framework just to solve this particular problem.

Even ignoring the whole RUNTIME aspect, changing how clocks are described in Zephyr as done in this RFC solves more than just the "generic consumers" problem. The two other main issues that have been brought up are that, in Clock Control, you effectively have one giant per-SoC(-series) driver that is a pain to write and maintain; in addition, you end up encoding the device's clock tree in code anyways because it's needed for clock_control_get_rate().

By having the clock tree in DTS instead, you get clock_management_get_rate() for "free", and your drivers are now small, simple and reusable across SoC series (e.g., I only had to write two additional drivers to add support for STM32H7 on top of STM32C0 in my CMS implementation - all other drivers were reused). For the record, though these figures should be taken with extreme caution:
$ wc -l zephyr/drivers/clock_management/stm32/*
  186 clock_management_stm32.h
   20 CMakeLists.txt
   89 Kconfig
  106 stm32_bus_prescaler.c
   66 stm32c0_hsisys_div.c
   73 stm32_clock_gate.c
  104 stm32_clock_generator.c
  285 stm32_clock_management_common.c
  126 stm32_clock_management_common.h
   90 stm32_clock_multiplexer.c
   76 stm32_h7_pll_output.c
  171 stm32_h7_pll_pvco.c
  124 stm32_internal_clkgen.c
   72 stm32_sysclk_prescaler.c
   89 stm32_timer_freqmul.c
 1677 total

$ wc -l zephyr/drivers/clock_control/clock_stm32_ll_*
  119 clock_stm32_ll_common.h       <--- shared by all clock_control STM32 drivers
 1090 clock_stm32_ll_common.c       <--- shared by a few series because clock tree is identical (C0, F1, ...)
 1165 clock_stm32_ll_h7.c
 2374 total

You can add reusable code to be shared between drivers without creating a super complex architecture around that. What you just showed, is not an improvement, it is just more confusion. Because these are actually all now their own components and there is so many side effects and cross-component communications going on, it is getting out of hand in my opinion.

The downside is that this does increase FLASH footprint: some figures have been posted several times in this thread, the ballpark is around 300~400B on a "simple" product like STM32C0. A change in scope and paradigm can definitely reduce that (e.g., "each DT node must describe operation performed on input clock" ==> solve entire clock tree at build time ==> leave only leaf nodes (clock gates) as instances in the final image + write-what-where sequence executed during initialization); this brings back your legitimate question about "what are we trying to achieve" which may need to be discussed more formally again.

Yes, this is exactly one or two of the points I was making, basically. You have to also realize, back to the phandles, that phandles form a tree too. The "child/parent node" DTS structure is specifically meant for describing buses, although it is also commonly used for describing subcomponents of devices, which is fine. But what we are talking about here is describing a tree of inter-device dependencies, which, is not supposed to be solved with child/parent DTS nodes, it's supposed to be solved by phandles. The DT spec clearly talks about use cases of phandles this way, such as the "interrupt tree" which is described through "interrupts" properties. And the way clocks are done in every other project that uses DT is through a "clocks tree" which is described through "clocks" properties. So I think we should be consistent with how DT is used across all projects, and not be doing things differently in a weird unintended way for Zephyr which IMO is not even justifiable. From what I have heard, the arguments for doing this tortured DT scheme do not make any actual sense, and are mostly pointing at design failures of the actual subsystem in code, not DT. The reason you are running into this problem is because you are tightly coupling DT structurally to subsystem implementation, this is wrong, and this is why we always have the mantra of "don't put software configuration in DT". In this case, sure you're trying to describe hardware, but you're trying to do it in a way that is tightly coupled to your "subsystem" implementation (note: you are not even coupling it to the subsystem design, but actually to the implementation, this is horribly terrible). Each DT node should describe a modular IP, and have a driver that can program to that hardware interface. If there is any dependencies between them they should be expressed by phandles. Having nodes for every single mux, div, gate, and bit within an IP, is completely not the point of what we should be doing in DT, it's totally out of line in my opinion.

decsny · 2025-09-20T03:34:04Z

Just to show, I'm not just being hand wavy about what I'm talking about with how we can already solve the opaqueness/coupling problem by just adding some useful macrobatics, look at this WIP PR: #96315

All I did with it so far is hello world on frdm_rw612, it's totally incomplete, but I am probably going to push this PR to completion. And it is not going to solve most of the problems we are talking about here but my point is we do not need a gigantic architectural rework just to solve the issue of consistently stupid implementation problems in existing clock control drivers, which is what was CLEARLY the original motivation of this PR (besides the original post focusing on that so much, I KNOW that was @danieldegrasse original motivation because when he was at NXP that is the main problem we were having that motivated him to even be looking at it in the first place). As far as the new motiviations that have cropped up and expanded the scope of this architecturally hyperinflated discussions, I want to know what are the actual use case that is motivating them, because all I have seen in the last year is a bunch of silicon vendors pontificating academically about all the things they could theoretically do with their clock trees and musing over implementation after implementation and architecture after architecture, with no clear focused guiding requirements, IMO that is not how we should be approaching this AT ALL.

And why am I not hearing any alternative proposal that actually considered, what if we just try incrementally extending appropriately and actually implementing correctly the existing clock control API? I mean this is just looking to me like a classic example of software developer natural tendency (not really the best link I could find on the phenomenon but I think anybody that has been in the industry for any amount of time knows what I'm talking about:) to want to "rewrite" an entirely new "glorious" thing from scratch instead of fix and improve the existing stuff.

danieldegrasse · 2025-09-21T02:01:31Z

I am still confused what is meant by "managed by the subsystem itself", maybe I am just missing something, but I see no subsystem being added on this PR, only a driver class.

Maybe it is time to restructure the code then. In the initial implementation of this PR, most of the clock management logic resided in drivers, and a few common helpers were defined in clock_management_common.c. Now, clock_management_common.c has become a pretty large file (~1000 lines).

Would it make sense to instead split the code in clock_management_common.c into a subsys folder across a few files? That would probably better delineate what code is part of the "subsystem" versus drivers.

I don't agree still with this problem description, this is just an implementation problem, see again what I said above about we can use the now-existing macros to form opaque arrays of arbitrary data from DT. So IMO the existing clock control API does not "fundamentally" have a design which couples consumers to specific drivers

clock_control_set_rate and clock_control_configure absolutely couple consumer drivers and producer drivers. Both take void * arguments, which means that you have no restrictions on what type of data the consumer passes to the producer when making a request.

We can define some form of generic macro that creates a data type to pass to these functions which is more generic, but the APIs themselves place no restriction on implementers- a vendor can choose to use these generic macros, or they can bypass them entirely. The key point is that the consumer facing API takes opaque arguments, which by definition makes it possible to leak implementation details between the clock producer and consumer.

Instead of focusing on the implementation in the problem description, I think it should be describing the requirements of rate requesting. I am not interested in the implementation details when I am reviewing this architecture proposal, I want to know, what exact problem(s) are we trying to solve and what are the characteristics of any solution that would address them.

Perhaps this explanation dives too deep into implementation, sure. I think the core problem is runtime resolution, which I described in the comment above (see the section about consumer X and Y)

And I understand you have a notion of "clock devices" which are miniature polymorphic objects to optimize instead of using full "devices" for every clock. But my comment again has been and still is, that we should not be having a "driver" for every single clock source, mux, div, whatever, in the SOC. The driver should be for a reusable and cohesive IP module from a hardware vendor, not trying to slice it up into all these mini drivers, which, if you're worried about ROM usage, IMO is probably going to be worse. The current clock control drivers are mostly written badly and use #ifdef to control rom usage, but they can be written much more simply if they just made use of DT phandles properly and if consumers did not intentionally couple themself to a driver, when it's not even necessary.

I don't agree with the argument here. The clock tree of an SOC does not constitute a single "reusable or cohesive IP module". It is a set of reusable IP blocks. Let's take the clock tree of the LPC55S69 versus the iMXRT595 as an example:

Both the LPC and iMX part share the same register layouts/programming semantics for their dividers and multiplexers
The PLL programming interface is vastly different. The LPC PLLs are unified blocks, whereas the iMX PLLs have multiple PFD outputs.
The overarching clock tree structure is vastly different between the iMX and LPC part

If we want to maximize code reuse, we need to split drivers into components:

driver for the multiplexer
driver for the divider
driver for the LPC PLL
driver for the iMX PLL

We then can reuse the divider and multiplexer drivers across the clock tree for each SOC.

Yeah, this , is more complicated, but again my question still stands from before, which is what are the actual use cases for all of this that we are trying to do. You gave a lot of abstract examples in the form of talking about a theoretical X, Y, A, B, C, okay, but I am not interested in letters and academic derivations, what I want us to be focused on here is what are the actual technical use cases we are trying to accomplish with a clock control or manager framework. WE need to stay focused on these actual goals when we are considering doing this major architectural change, it should not be theoretical and academic in motivation.

I don't have a concrete example for you unfortunately, this was a usecase that came from @hubertmis. I would love if @bjarki-andreasen or @hubertmis could chime in with a specific case here. NXP parts don't have a usecase like this I am aware of. Here's a marginally more concrete example though:

UART and SPI both source from MUX A
MUX A can select between:
- CLK_12M (10uA, 12MHz)
- CLK_96M (30uA, 96MHz)
- PLL0 (100uA, 50-150MHz)
UART starts. Requests the lowest power clock that:
- is between 10-100MHz
- consumes 30uA or less.
MUX A is configured to select CLK_12M based on this request.
SPI starts. Requests the fastest clock that:
- is between 50-150MHz
- consumes 1A or less (effectively any clock)
MUX A is now configured to select CLK_96M, because PLL0 is too high power for UART source
SPI stops. MUX A is now configured back CLK_12M, because that is best for UART.

I want to be clear here. This is the type of usecase the framework is designed to solve in its current form. Static boot time configuration is a problem we can solve a myriad of other (simpler) ways. The only goals this framework has for static configuration are:

feature parity with existing clock control framework
limit the additional ROM overhead incurred by switching to this framework from clock control

Just to show, I'm not just being hand wavy about what I'm talking about with how we can already solve the opaqueness/coupling problem by just adding some useful macrobatics, look at this WIP PR: #96315

I appreciate you doing this- it is good to try to move things forward in incremental steps, and it is absolutely possible to solve NXP's static clock configuration issue without the clock management PR. It would be great to see that move forwards.

As I hope is clear, this PR goes well beyond just solving NXP's static clock configuration problem, which was indeed the initial reason I started work on this- at this point though the proposal is for a subsystem, and a subsystem needs to work for more usecases than NXP's alone.

And why am I not hearing any alternative proposal that actually considered, what if we just try incrementally extending appropriately and actually implementing correctly the existing clock control API? I mean this is just looking to me like a classic example of software developer natural tendency (not really the best link I could find on the phenomenon but I think anybody that has been in the industry for any amount of time knows what I'm talking about:) to want to "rewrite" an entirely new "glorious" thing from scratch instead of fix and improve the existing stuff.

I'm unclear what you mean here. #70467 was an alternate proposal. It was considered. If you'd like to make another alternate proposal, please do- there is no need for this PR to be the only proposed solution. This PR has also gone through several reworks- some large enough I would argue they constitute a new proposed approach to the problem.

Based on your comments I'm concerned you are focused on this PR as just a static clock framework. It isn't that- if it was, this would be vastly overcomplicated. I've implemented the runtime features that exist in response to feedback from @bjarki-andreasen and @hubertmis. If either of you could add a concrete example to help highlight why we need runtime rate resolution features, that might be helpful

CkovMk · 2025-09-22T07:00:38Z

I agree that current separation of individual "mux" and "div" are way too complicated. MPUs can have 100+ clock cells, resulting in giant clock trees in dts.

If we want to maximize code reuse, we need to split drivers into components:
driver for the multiplexer
driver for the divider
driver for the LPC PLL
driver for the iMX PLL

Usually the code reuse is limited within vendor's scope, would it be possible to design an higher level abstraction in the subsystem's view, and allow for vendor specific implementation for clock cell components? I think we are not expecting clock config being portable across vendors anyway.

In my concern, the set rate capability is really useful for video, display and audio applications, where the clock frequency often depends on frame rate, resolution or sample rate. I'm really looking forward this built-in function to provide a standard approach. Also the notify-accept mechanism is nice to have.

mathieuchopstm · 2025-09-22T07:51:59Z

This is not a problem with the concept of using phandles instead of nodes-based hierarchies and properties. This is a problem, again, which I have said, of having WAY too many "clock drivers" because of the way this RFC is designed right now. A clock consumer referencing it's clocks with a clocks property should not have to reference 500 clock nodes just to configure it's one clock input. All of this stuff, should be figured out either hierarchically (which I THOUGHT was the design goal of this PR), or by a monolithic clock device driver representing an actual hardware IP block which is providing multiple outputs (what is MEANT to be how zephyr clock_control works, but pretty much every current implementation is a terrible example), or a mixture of both (linux approach). Not "every device has to have a giant phandle array referencing 500 clocks they need to configure". That is way overly complicated. This again is still looking like at the very least an implementation problem, or, a fundamental design problem with the RFC. It is hard for me to tell which at this point because this is such a gargantuan PR with so many things and tricks and crossplay going on. Can't we try to keep things simple?

FTR, there is probably a bit of confusion here: the clocks I was describing would be on a clock-state and not on a peripheral, like this:

zephyr/boards/nxp/lpcxpresso55s69/lpcxpresso55s69_lpc55s69_cpu0.dts

Lines 131 to 143 in 7764d24

    
           &system_clock { 
        
           	/* 144 MHz setpoint for CPU core clock */ 
        
           	sys_clk_144mhz: sys_clk_144mhz { 
        
           		compatible = "clock-state"; 
        
           		clocks = <&ahbclkdiv 1 &fro_12m 1 &mainclkselb 0 &mainclksela 0 
        
           			&xtal32m 1 &clk_in_en 1 &pll1clksel 1 
        
           			&pll1_pdec 2 &pll1 8 144 0 53 31 
        
           			&pll1_directo 0 &pll1_bypass 0 
        
           			&mainclkselb 2>; 
        
           		clock-frequency = <DT_FREQ_M(144)>; 
        
           		locking-state; 
        
           	}; 
        
           };

Each device should indeed only refer to its parents, I was not suggesting otherwise.

Besides what I described above, the big reason for a properties-based proposal was also that a single boot-time configuration is what (almost?) everyone uses today in Zephyr - unless I missed it and there's already a platform with dynamic management? I recall a discussion with @henrikbrixandersen about this topic, though I don't know if we have any metric about how many users shipped products with a static configuration vs. something dynamic (how much of that is due to Zephyr lacking a framework for dynamic configuration is up for debate - I do think we had someone saying here they had their own dynamic framework, collecting their feedback could be interesting)

I don't understand, are you trying to say the phandles specifier form would not allow for static configuration? Because that's how we actually are doing static configuration right now for NXP. So I don't get your point here.

No, I was merely trying to say that static initialization is presumably what most people use, hence why we should try to make it as easy to use as possible (e.g., properties-based).

Even ignoring the whole RUNTIME aspect, changing how clocks are described in Zephyr as done in this RFC solves more than just the "generic consumers" problem. The two other main issues that have been brought up are that, in Clock Control, you effectively have one giant per-SoC(-series) driver that is a pain to write and maintain; in addition, you end up encoding the device's clock tree in code anyways because it's needed for clock_control_get_rate().
By having the clock tree in DTS instead, you get clock_management_get_rate() for "free", and your drivers are now small, simple and reusable across SoC series (e.g., I only had to write two additional drivers to add support for STM32H7 on top of STM32C0 in my CMS implementation - all other drivers were reused). For the record, though these figures should be taken with extreme caution:
$ wc -l zephyr/drivers/clock_management/stm32/*
  186 clock_management_stm32.h
   20 CMakeLists.txt
   89 Kconfig
  106 stm32_bus_prescaler.c
   66 stm32c0_hsisys_div.c
   73 stm32_clock_gate.c
  104 stm32_clock_generator.c
  285 stm32_clock_management_common.c
  126 stm32_clock_management_common.h
   90 stm32_clock_multiplexer.c
   76 stm32_h7_pll_output.c
  171 stm32_h7_pll_pvco.c
  124 stm32_internal_clkgen.c
   72 stm32_sysclk_prescaler.c
   89 stm32_timer_freqmul.c
 1677 total

$ wc -l zephyr/drivers/clock_control/clock_stm32_ll_*
  119 clock_stm32_ll_common.h       <--- shared by all clock_control STM32 drivers
 1090 clock_stm32_ll_common.c       <--- shared by a few series because clock tree is identical (C0, F1, ...)
 1165 clock_stm32_ll_h7.c
 2374 total
You can add reusable code to be shared between drivers without creating a super complex architecture around that. What you just showed, is not an improvement, it is just more confusion.

That's kinda what we have with clock_stm32_ll_common, but it didn't scale to support both tiny SoCs with AHB/APB and large devices with separate domains, multiple bus matrices and 20 source muxes. I'm not saying it's impossible though, just that we haven't found a way to make it work - suggestions welcome.

Because these are actually all now their own components and there is so many side effects and cross-component communications going on, it is getting out of hand in my opinion.

Which side effects and cross-component communications? Drivers don't talk to each other, the subsystem manages that now.

The downside is that this does increase FLASH footprint: some figures have been posted several times in this thread, the ballpark is around 300~400B on a "simple" product like STM32C0. A change in scope and paradigm can definitely reduce that (e.g., "each DT node must describe operation performed on input clock" ==> solve entire clock tree at build time ==> leave only leaf nodes (clock gates) as instances in the final image + write-what-where sequence executed during initialization); this brings back your legitimate question about "what are we trying to achieve" which may need to be discussed more formally again.

Yes, this is exactly one or two of the points I was making, basically. You have to also realize, back to the phandles, that phandles form a tree too. The "child/parent node" DTS structure is specifically meant for describing buses, although it is also commonly used for describing subcomponents of devices, which is fine. But what we are talking about here is describing a tree of inter-device dependencies, which, is not supposed to be solved with child/parent DTS nodes, it's supposed to be solved by phandles.

That was one of the solutions I was thinking about to solve the initialization order issue, actually - having properties somewhere to describe that some nodes must be init'ed in a specific order relative to each other. Though, for this very instance, since the PLL configuration is (mostly) locked after being enabled, the entire configuration could be done by the PVCO node (the outputs would still need separate nodes). This doesn't apply to all series; some of them allow specific outputs to be disabled on-the-fly.

bjarki-andreasen · 2025-09-22T11:40:02Z

To summarize the requirements from Nordic:

We need to be able to select clock power modes at runtime
We need to be able to define required clocks for a peripheral to be operational
We need to be able to request a specific output frequency of a clock needed by a peripheral

For us, just defining every clock as a device, and having every configuration it can be in as child nodes we can select, we have all we need.

clocks {
        /* low power */
        lfxo {
                lfxo_mope_0: mode-0 {
                        accuracy-ppb = <200>;
                        precision-error-ppb = <200>;
                };
        };

        /* high power */
        hfxo {
                hfxo_mope_0: mode-0 {
                        accuracy-ppb = <20>;
                        precision-error-ppb = <20>;
                };
        };

        global_hsfll {
                /* modes sorted by power consumption */
                global_hsfll_mode_0: mode-0 {
                        accuracy-ppb = <400>;
                        error-ppb = <400>;
                        /* uses internal rc */
                };
               global_hsfll_mode_1:  mode-1 {
                        accuracy-ppb = <250>; /* accuracy etc. is not a simple addition sadly, it has to be measured */
                        error-ppb = <250>;
                        required-clocks = <&lfxo>;
                };
                global_hsfll_mode_2: mode-2 {
                        accuracy-ppb = <30>;
                        error-ppb = <30>;
                        required-clocks = <&hfxo>;
                };
        };

        spi120 {
                required-clocks = <&global_hsfll>; /* any mode of global_hsfll will work */
        };
};

On our hardware, the "modes" do not affect frequency, only accuracy, so we just need to be able to specify an output frequency from drivers, the application can then determine, at runtime, which power mode to use.

danieldegrasse · 2025-09-22T21:56:13Z

I agree that current separation of individual "mux" and "div" are way too complicated. MPUs can have 100+ clock cells, resulting in giant clock trees in dts.
If we want to maximize code reuse, we need to split drivers into components:
driver for the multiplexer
driver for the divider
driver for the LPC PLL
driver for the iMX PLL
Usually the code reuse is limited within vendor's scope, would it be possible to design an higher level abstraction in the subsystem's view, and allow for vendor specific implementation for clock cell components? I think we are not expecting clock config being portable across vendors anyway.

Sure- a vendor is already able to implement clocks within this framework at less granularity than I described. It is possible to simply implement an entire clock tree as a series of "root clocks" if desired. The downside to using this approach is that the vendor will be responsible for handling negotiations between clocks, since the framework won't know how the tree is structured.

One thing I'll note- the API as proposed here is now very close to the Linux common clock framework, which is a proven working framework on SOCs much more complex than Zephyr supports. The main difference is that we describe our clock tree in DTS, while Linux describes the tree in C. I chose this approach because we need the ability to reference individual nodes in DTS to configure them, which isn't really possible if we only have the clock tree defined in C.

Essentially our runtime configuration API (set_rate and similar) is the same as the Linux CCF, and the only variation is that we support this configure API- which is required because we don't want runtime rate setting code to be enabled for builds that only perform static clock configuration.

decsny · 2025-09-22T23:12:33Z

I am still confused what is meant by "managed by the subsystem itself", maybe I am just missing something, but I see no subsystem being added on this PR, only a driver class.

Maybe it is time to restructure the code then. In the initial implementation of this PR, most of the clock management logic resided in drivers, and a few common helpers were defined in clock_management_common.c. Now, clock_management_common.c has become a pretty large file (~1000 lines).

Would it make sense to instead split the code in clock_management_common.c into a subsys folder across a few files? That would probably better delineate what code is part of the "subsystem" versus drivers.

Yeah probably I would prefer to see it this way, and would lead the development (both on this PR and in the future) also down a path which I think is more sensible, which is to have a more central clock managing logic.

I don't agree still with this problem description, this is just an implementation problem, see again what I said above about we can use the now-existing macros to form opaque arrays of arbitrary data from DT. So IMO the existing clock control API does not "fundamentally" have a design which couples consumers to specific drivers

clock_control_set_rate and clock_control_configure absolutely couple consumer drivers and producer drivers. Both take void * arguments, which means that you have no restrictions on what type of data the consumer passes to the producer when making a request.

I don't agree with this really that coupling is required with void *. My experience with void * in C is that the purpose for using it is when either the caller or the callee doesn't know the format of the data that is being passed around. Such as, a void * to an opaque state handle only the callee knows about the format, or a void * to some user data that will get sent in a callback, that only the caller knows about the format. So if both the callee and the caller know about the format, then you don't need to use a void * in the first place.

In fact, I guess that is actually what I am trying to do on the PR I linked before, I am proposing a unification of the clock control API so that all client and implementation codes for clock control know the same format - the arbitrary DT spec format, which is the most general way to pass around the data we are actually trying to pass around - the DT specifier cells. So maybe you are right that void * was not an ideal choice here but I think the existing API can be made generic and unified with some trivial (but monotonous) refactoring. Actually it's a good use case for AI.

We can define some form of generic macro that creates a data type to pass to these functions which is more generic, but the APIs themselves place no restriction on implementers- a vendor can choose to use these generic macros, or they can bypass them entirely. The key point is that the consumer facing API takes opaque arguments, which by definition makes it possible to leak implementation details between the clock producer and consumer.

Again I don't think opaque argument leaks implementation details at all actually, that's the meaning of opaque. What we are seeing actually is that this current void * usage is NOT being used as opaque. An alternative to the PR I posted above is actually doing something similar to pinctrl_soc.h where, instead of a unified data scheme like the clock_control_dt_spec, it could be that each SOC implement it's own clock getter and definer and type, just like with pinctrl.

Instead of focusing on the implementation in the problem description, I think it should be describing the requirements of rate requesting. I am not interested in the implementation details when I am reviewing this architecture proposal, I want to know, what exact problem(s) are we trying to solve and what are the characteristics of any solution that would address them.

Perhaps this explanation dives too deep into implementation, sure. I think the core problem is runtime resolution, which I described in the comment above (see the section about consumer X and Y)

I am still looking to focus this RFC on use cases and not theoretical situations.

And I understand you have a notion of "clock devices" which are miniature polymorphic objects to optimize instead of using full "devices" for every clock. But my comment again has been and still is, that we should not be having a "driver" for every single clock source, mux, div, whatever, in the SOC. The driver should be for a reusable and cohesive IP module from a hardware vendor, not trying to slice it up into all these mini drivers, which, if you're worried about ROM usage, IMO is probably going to be worse. The current clock control drivers are mostly written badly and use #ifdef to control rom usage, but they can be written much more simply if they just made use of DT phandles properly and if consumers did not intentionally couple themself to a driver, when it's not even necessary.

I don't agree with the argument here. The clock tree of an SOC does not constitute a single "reusable or cohesive IP module". It is a set of reusable IP blocks. Let's take the clock tree of the LPC55S69 versus the iMXRT595 as an example:

First of all, at no point did I say that the "entire clock tree of an SOC is a single reusable IP module". I said, that any IP which are reuseable (even if they are just different configurations of the same IP) should be getting a driver and compatible. Which I would expect that there would be multiple clock drivers per SOC.

* Both the LPC and iMX part share the same register layouts/programming semantics for their dividers and multiplexers
* The PLL programming interface is vastly different. The LPC PLLs are unified blocks, whereas the iMX PLLs have multiple PFD outputs.

Because they are both syscon "clkctl"/CCM (clock control module) IP. So what I am saying is there should be one driver for the "clkctl", not a driver for a mux and a driver for a div. I know the LPC doesn't call it that, and groups it in the same chapter and same memory map, but I can tell you a secret which is not surprising which is if I look at the actual chip data, I can clearly see there is a "CCM" IP and a "PLL" IP which are separate. So what I am saying is that we should not need multiple drivers for one IP, such as the CCM. But yes, the PLL would be a different driver than the CCM.

* The overarching clock tree structure is vastly different between the iMX and LPC part

That's fine again, the platform differences are as always going to be described in DT. We are not debating if the different platforms are different, but how to describe them in DT.

If we want to maximize code reuse, we need to split drivers into components:
* driver for the multiplexer

* driver for the divider

* driver for the LPC PLL

* driver for the iMX PLL
We then can reuse the divider and multiplexer drivers across the clock tree for each SOC.

I think driver for the CCM, driver for the LPC PLL, and driver for the iMX PLL.

I want to be clear here. This (runtime configuration) is the type of usecase the framework is designed to solve in its current form. Static boot time configuration is a problem we can solve a myriad of other (simpler) ways. The only goals this framework has for static configuration are:
* feature parity with existing clock control framework

* limit the additional ROM overhead incurred by switching to this framework from clock control

What I am trying to see is if we can instead of replacing (even with feature parity) the existing clock control, is if we can improve the clock control and build on top of it instead. I am not saying, to throw out everything in this PR, but I would like to see if it can be adjusted to be simpler, and not requiring a switch and deprecation.

Just to show, I'm not just being hand wavy about what I'm talking about with how we can already solve the opaqueness/coupling problem by just adding some useful macrobatics, look at this WIP PR: #96315

I appreciate you doing this- it is good to try to move things forward in incremental steps, and it is absolutely possible to solve NXP's static clock configuration issue without the clock management PR. It would be great to see that move forwards.

cc @EmilioCBen (he was wanting to know if you will block my PR)

As I hope is clear, this PR goes well beyond just solving NXP's static clock configuration problem, which was indeed the initial reason I started work on this- at this point though the proposal is for a subsystem, and a subsystem needs to work for more usecases than NXP's alone.

I am completely aware you are going beyond the original motivation, and I am not saying not to try to do that, my points I am making here are basically these things:

focus the RFC on actual use cases for these runtime configurings so we know we are actually doing what we need to be doing
consider if there is any way to integrate with existing clock control (even if we need to improve it)
try to match linux style of describing clock trees in DT
make sure we are keeping it as simple as we can

And why am I not hearing any alternative proposal that actually considered, what if we just try incrementally extending appropriately and actually implementing correctly the existing clock control API? I mean this is just looking to me like a classic example of software developer natural tendency (not really the best link I could find on the phenomenon but I think anybody that has been in the industry for any amount of time knows what I'm talking about:) to want to "rewrite" an entirely new "glorious" thing from scratch instead of fix and improve the existing stuff.

I'm unclear what you mean here. #70467 was an alternate proposal. It was considered. If you'd like to make another alternate proposal, please do- there is no need for this PR to be the only proposed solution. This PR has also gone through several reworks- some large enough I would argue they constitute a new proposed approach to the problem.

You are right, which is what I said above, I would like to see some alternative proposals from more people and I considered doing something myself. However, the reason I said what you quoted here is because if you are saying we need to deprecate the old APIs instead of improving/extending them, I feel like you need a justification for saying why we can't be considering doing that. And the old one you linked also was trying to replace it.

Based on your comments I'm concerned you are focused on this PR as just a static clock framework. It isn't that- if it was, this would be vastly overcomplicated. I've implemented the runtime features that exist in response to feedback from @bjarki-andreasen and @hubertmis. If either of you could add a concrete example to help highlight why we need runtime rate resolution features, that might be helpful

No, I'm not focusing it on that, in fact, I want to solve the static clocking thing separately in the meantime, and what I am actually saying here is that I want this PR to be more focused on the dynamic clocking, with intentionality around specific use cases. And it would be nice if it can build on top of some intermediate work that solves the static problem, but this is where I AM being handwavey, because I do not know how that would look.

decsny · 2025-09-22T23:14:19Z

I agree that current separation of individual "mux" and "div" are way too complicated. MPUs can have 100+ clock cells, resulting in giant clock trees in dts.
If we want to maximize code reuse, we need to split drivers into components:
driver for the multiplexer
driver for the divider
driver for the LPC PLL
driver for the iMX PLL
Usually the code reuse is limited within vendor's scope, would it be possible to design an higher level abstraction in the subsystem's view, and allow for vendor specific implementation for clock cell components? I think we are not expecting clock config being portable across vendors anyway.

In my concern, the set rate capability is really useful for video, display and audio applications, where the clock frequency often depends on frame rate, resolution or sample rate. I'm really looking forward this built-in function to provide a standard approach. Also the notify-accept mechanism is nice to have.

One thing I will point out is that there is already a set_rate function in the current clock control API. But it suffers from the same problem as the configure function. Yet I think it can also be solved in similar ways.

How I would like to see this PR adding value is by creating a subsystem layer that lets the clock control drivers call on a clock manager subsystem to ensure the clock values they get requested is OK, then they are responsible for actually changing the hardware state.

CkovMk · 2025-09-23T08:19:15Z

Usually the code reuse is limited within vendor's scope, would it be possible to design an higher level abstraction in the subsystem's view, and allow for vendor specific implementation for clock cell components? I think we are not expecting clock config being portable across vendors anyway.

Sure- a vendor is already able to implement clocks within this framework at less granularity than I described. It is possible to simply implement an entire clock tree as a series of "root clocks" if desired. The downside to using this approach is that the vendor will be responsible for handling negotiations between clocks, since the framework won't know how the tree is structured.

I think the problem here is really this granularity: the framework explicitly defines standard clock device, mux clock device and root clock device. Many SoCs are designed to have 1 gate, 1 mux and 1 divider per IP to allow maximum flexibility. This framework enforces 3 separate DT nodes instead of 1. I'd like to see composed clock devices for simpler DT hierarchy, less layers of callbacks as well as code size reduction (possibly).

Some other comments:

For external clocks like mclk, its possible it gets clock from an configurable external clock source (e.g. an on-board I2C clock generator or timer, or even another on-chip counter). The frequency of it thus cannot be determined during build time. Purpose an separate external-clock compatible to simply forward the frequency of another clock-output.

Also added comments for struct clock_management_event.

CkovMk · 2025-09-23T08:40:41Z

include/zephyr/drivers/clock_management.h

+struct clock_management_event {
+	/** Type of event */
+	enum clock_management_event_type type;
+	/** Old clock rate */
+	clock_freq_t old_rate;
+	/** New clock rate */
+	clock_freq_t new_rate;
+};


Some clock drivers requires RPC to another "management" core in order to get their current config (e.g. mux option or divider), which can be slow compared to direct register access. During the 1st call of CLOCK_MANAGEMENT_QUERY_RATE_CHANGE event, the underlying driver has already gathered some info. Would it be possible to add some kind of hint that subsequent CLOCK_MANAGEMENT_PRE_RATE_CHANGE and CLOCK_MANAGEMENT_POST_RATE_CHANGE event for the same clock device comes from the same request, and the info gathered from first call could be reused?

mathieuchopstm · 2025-09-23T12:37:54Z

* Both the LPC and iMX part share the same register layouts/programming semantics for their dividers and multiplexers
* The PLL programming interface is vastly different. The LPC PLLs are unified blocks, whereas the iMX PLLs have multiple PFD outputs.
Because they are both syscon "clkctl"/CCM (clock control module) IP. So what I am saying is there should be one driver for the "clkctl", not a driver for a mux and a driver for a div. I know the LPC doesn't call it that, and groups it in the same chapter and same memory map, but I can tell you a secret which is not surprising which is if I look at the actual chip data, I can clearly see there is a "CCM" IP and a "PLL" IP which are separate. So what I am saying is that we should not need multiple drivers for one IP, such as the CCM. But yes, the PLL would be a different driver than the CCM.
If we want to maximize code reuse, we need to split drivers into components:
* driver for the multiplexer
* driver for the divider
* driver for the LPC PLL
* driver for the iMX PLL
We then can reuse the divider and multiplexer drivers across the clock tree for each SOC.
I think driver for the CCM, driver for the LPC PLL, and driver for the iMX PLL.

I'm not super familiar with NXP HW but I assume this CCM is basically a block where you can control most clock tree elements (gates, muxes and prescalers)... which leads me to believe that while the functional interface (i.e., how to edit bits in MMIO to do specific operations) is the same across all products, the actual implementation differs (e.g., because product A has 10 gates, 5 muxes and 3 prescalers, but product B has 15 gates, 7 muxes and 5 prescalers) and so I struggle to see how one could write a unified driver for such a block -unless your suggestion is that we would describe this whole hierarchy in the DT à la

ccm {
    mux1 {
        prescaler1 { 
            gate1 { /* ... */ };
            gate2 { /* ... */ };
        };
        prescaler2 {
            gate3 { /* ... */ };
        };
    };
    mux2 { /* ... */ };
};

and somehow the CCM driver would manage all the nodes under it? Otherwise, it just sounds like the current STM32 implementation (i.e., one per-series "RCC block" gigadriver). Also, how would the CCM and PLL drivers interact?

(Such an hybrid approach of one-driver-manages-many-devices could be interesting, I'm just having trouble seeing how it could be done with today's tools)

I want to be clear here. This (runtime configuration) is the type of usecase the framework is designed to solve in its current form. Static boot time configuration is a problem we can solve a myriad of other (simpler) ways. The only goals this framework has for static configuration are:
* feature parity with existing clock control framework
* limit the additional ROM overhead incurred by switching to this framework from clock control
What I am trying to see is if we can instead of replacing (even with feature parity) the existing clock control, is if we can improve the clock control and build on top of it instead. I am not saying, to throw out everything in this PR, but I would like to see if it can be adjusted to be simpler, and not requiring a switch and deprecation.

There is still room for improvement within the Clock Control implementation, but some things will require more than little API-compatible edits.

For example, the data argument of clock_control_configure() is basically useless because the only thing you can serialize from DT is the clock_control_subsys_t; this means that you have to use the same structure for clock_control_{off,on}(), clock_control_configure() and clock_control_get_rate() by design. This leads to frankenstein-esque implementations where the clock_control_subsys_t, supposed to merely identify a clock tree element, is overloaded to convey additional information (how this overload is performed varies from vendor to vendor too 🙂)

In the end, clock_control_get_rate() will always more or less be a multi-level LUT lookup; the question is whether we want to continue having hardcoded LUTs in drivers/HALs, or if we want to express them through the Device Tree (My understanding being that the Linux CCF opted for the first option?)

By the way, with regard to footprint, I would like to point out that - at least for STM32 - the results are somewhat biased; we are "cheating" by effectively having two different formats for the clock cells, depending on whether they will be provided to on/off (gates only!) or get_rate/configure (latter is for muxes only). This saves one LUT level[1] but has unwanted side effects, notably on DT side where two clock cells must be provided on each device whose driver may perform a get_rate(), even if there is no multiplexer associated to the device (this also inhibits trivially adding get_rate() calls to drivers)

[1] The usual clock_control_get_rate() implementations receive a "device identifier", which goes through a first LUT to obtain a "clock source identifier", which is then used to look up the frequency through SoC-specific LUTs; our implementation provides the "clock source identifier" directly.

I agree that current separation of individual "mux" and "div" are way too complicated. MPUs can have 100+ clock cells, resulting in giant clock trees in dts.

Bring back to the same point: the separation will be done, the question is whether it should be in DTS or in C code (Linux does it in C code - example). It is true that DTS has a downside of being quite verbose, but @danieldegrasse demonstrated that it could be generated automatically via scripting (though one may argue you could generate C code this way too).

Based on your comments I'm concerned you are focused on this PR as just a static clock framework. It isn't that- if it was, this would be vastly overcomplicated. I've implemented the runtime features that exist in response to feedback from @bjarki-andreasen and @hubertmis. If either of you could add a concrete example to help highlight why we need runtime rate resolution features, that might be helpful

No, I'm not focusing it on that, in fact, I want to solve the static clocking thing separately in the meantime, and what I am actually saying here is that I want this PR to be more focused on the dynamic clocking, with intentionality around specific use cases. And it would be nice if it can build on top of some intermediate work that solves the static problem, but this is where I AM being handwavey, because I do not know how that would look.

+

Usually the code reuse is limited within vendor's scope, would it be possible to design an higher level abstraction in the subsystem's view, and allow for vendor specific implementation for clock cell components? I think we are not expecting clock config being portable across vendors anyway.
In my concern, the set rate capability is really useful for video, display and audio applications, where the clock frequency often depends on frame rate, resolution or sample rate. I'm really looking forward this built-in function to provide a standard approach. Also the notify-accept mechanism is nice to have.

One thing I will point out is that there is already a set_rate function in the current clock control API. But it suffers from the same problem as the configure function. Yet I think it can also be solved in similar ways.

How I would like to see this PR adding value is by creating a subsystem layer that lets the clock control drivers call on a clock manager subsystem to ensure the clock values they get requested is OK, then they are responsible for actually changing the hardware state.

I do have a similar mindset, i.e. focus should be placed on having a solid framework for static configuration but also designed such that one may easily "plug in" a management framework on top if desired; whether this framework should be implemented completely, partially or not at all by Zephyr being another topic; the "plug in" mechanism must however be thought about carefully.

decsny · 2025-10-10T13:31:19Z

dts/bindings/clock-management/clock-state.yaml

+# Copyright 2024 NXP
+# SPDX-License-Identifier: Apache-2.0
+
+compatible: "clock-state"


this needs to be prefixed with zephyr, since it is not describing hardware at all but only is a mechanism for doing programming in DT.

decsny · 2025-10-10T13:32:08Z

dts/bindings/clock-management/clock-output.yaml

+  may also be defined as children of these nodes, and can be applied directly
+  by consumers.
+
+compatible: "clock-output"


this similarly should probably be prefixed with zephyr, , even though it is technically descriptive of hardware, it's only needed to host the clock-state nodes, and therefore is unique to zephyr

While we're at, maybe also worth renaming as the output usage is counter intuitive: We're talking about the entry clock of the node defining this property.

danieldegrasse force-pushed the rfc/clock-mgmt-drivers branch 9 times, most recently from b4d7f94 to 8cc246a Compare April 29, 2024 21:08

danieldegrasse marked this pull request as ready for review April 29, 2024 22:07

danieldegrasse requested review from dleach02 and mmahadevan108 as code owners April 29, 2024 22:07

zephyrbot requested review from 57300, DerekSnell, EmilioCBen, aescolar, dbaluta, decsny, galak, jeremybettis, manuargue and nashif April 29, 2024 22:08

decsny requested changes Sep 18, 2025

View reviewed changes

decsny reviewed Sep 19, 2025

View reviewed changes

CkovMk reviewed Sep 23, 2025

View reviewed changes

decsny requested changes Oct 10, 2025

View reviewed changes

dleach02 unassigned mmahadevan108 and dleach02 Nov 10, 2025

[RFC] Introduce Clock Management Subsystem (Clock Driver Based) #72102

Are you sure you want to change the base?

[RFC] Introduce Clock Management Subsystem (Clock Driver Based) #72102

Conversation

danieldegrasse commented Apr 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Introduction

Problem description

Proposed change

Detailed RFC

Clock Management Layer

Requesting Clock Rates versus Configuring the Clock Tree

Directly Configuring Clock Tree

Runtime Clock Requests

Clock Driver Layer

Framework Configuration

Dependencies

Flash usage

Concerns and Unresolved Questions

Alternatives

Uh oh!

danieldegrasse commented Sep 17, 2025

Uh oh!

mathieuchopstm commented Sep 18, 2025

Uh oh!

mathieuchopstm commented Sep 18, 2025

Uh oh!

decsny commented Sep 18, 2025

Uh oh!

decsny commented Sep 18, 2025

Uh oh!

decsny commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danieldegrasse commented Sep 18, 2025

Uh oh!

danieldegrasse commented Sep 18, 2025

Uh oh!

decsny commented Sep 18, 2025

Uh oh!

decsny left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danieldegrasse commented Sep 18, 2025

Uh oh!

danieldegrasse commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Introduction

Problem description

Proposed change

Uh oh!

mathieuchopstm commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

decsny Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

decsny commented Sep 19, 2025

Introduction

Problem description

Proposed change

Uh oh!

decsny commented Sep 19, 2025

Uh oh!

decsny commented Sep 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danieldegrasse commented Sep 21, 2025

Uh oh!

danieldegrasse commented Apr 29, 2024 •

edited

Loading

decsny commented Sep 18, 2025 •

edited

Loading

danieldegrasse commented Sep 18, 2025 •

edited

Loading

mathieuchopstm commented Sep 19, 2025 •

edited

Loading

decsny Sep 19, 2025 •

edited

Loading

decsny commented Sep 20, 2025 •

edited

Loading

mathieuchopstm commented Sep 22, 2025 •

edited

Loading

bjarki-andreasen commented Sep 22, 2025 •

edited

Loading