adsp: Introduce ADSP SoC CDU clock driver - #3456
Open
qasim-ijaz wants to merge 24 commits into
Open
Conversation
Author
|
Tested the driver on all SoCs, boots successully and clock trees seem valid. Clock tree dumps for all SoCs below: POSTreserved sel parents in original is wrong, mine fixes: |
Collaborator
|
@ukleinek just so you're aware of this since it is a big change and may impact your work |
Collaborator
|
@qasim-ijaz Check https://github.com/analogdevicesinc/linux/actions/runs/29926080863/job/88943461564?pr=3456#step:9:72 for checkpatch, cocci, etc |
Collaborator
|
Not sure if checkpatch can check for it, but it looks like the commit descriptions are wrapped at 50 characters. |
Contributor
LLM reviewThis series adds a shared ADSP SoC CDU (Clock Distribution Unit) clock run: 29930235555
|
pamolloy
reviewed
Jul 22, 2026
pamolloy
reviewed
Jul 22, 2026
qasim-ijaz
force-pushed
the
cdu
branch
2 times, most recently
from
July 23, 2026 13:16
79662d5 to
55209e3
Compare
|
tested with RevE+RevD boots, and clock tree in clk_summary looks ok |
qasim-ijaz
force-pushed
the
cdu
branch
2 times, most recently
from
July 23, 2026 14:55
b546642 to
3080660
Compare
Author
Fixed most of the checkpatch issues:
As for the rest they seem to be about 1) line lengths of 101/102/103 exceeding the 100 line max, i think this should be fine since its only 1-2 chars extra (if i was to change this i'd probably have to make the names shorter or something so I think readability > worrying about 1-2 extra chars, also it seems other clock drivers also exceed this limit upstream) 2) parenthesis alignment, once again i think this is fine since this is a style employed by other upstream clock drivers (https://github.com/torvalds/linux/blob/master/drivers/clk/rockchip/clk-rk3588.c for example) and i think the current styling looks quite clean. |
Collaborator
|
The wrapping of the commit descriptions should be made consistent. The description of |
sipraga
reviewed
Aug 3, 2026
sipraga
left a comment
sipraga
left a comment
Contributor
There was a problem hiding this comment.
Overall great job and the structure is very sound! I am looking forward to the CGU driver getting the same loving treatment - things should look much cleaner after!
I have left quite a lot of comments but they are a mix of subjective opinions (mostly in the interest of naming consistency, as we've discussed before), general code improvements, and only a few fundamental issues. Take your time and feel free to call me on teams if you think it's easier to chat than take it here.
I got a bit tired of reviewing, so this is a bit of a partial review. But hopefully enough for you to work with for now. Of course push back on my comments if I got something wrong or you disagree - sometimes I'm just thinking out loud :-D
| #clock-cells = <1>; | ||
|
|
||
| clocks = <&sys_clkin0>, | ||
| <&sys_clkin1>, |
Contributor
There was a problem hiding this comment.
make sure to run dt_binding_check. it can take an eternity to check the whole tree, so you can specify a specific bindings file to check:
make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/adi,sc5xx-cdu.yaml
Author
There was a problem hiding this comment.
I ran the dt_binding_check on Documentation/devicetree/bindings/clock/adi,adsp-cdu.yaml it passes cleanly with no errors.
qasim-ijaz
force-pushed
the
cdu
branch
2 times, most recently
from
August 16, 2026 22:18
555cbc5 to
4884b3d
Compare
sipraga
reviewed
Aug 17, 2026
| * This header defines the clocks that the ADSP-SC573 | ||
| * CDU clock controller provides. | ||
| * | ||
| * Author: Qasim Ijaz <qasim.ijaz@analog.com> |
Contributor
There was a problem hiding this comment.
I think just drop author here and elsewhere, since copyright is usually where it goes but that's attributed to ADI. we also have git-blame
just my 2c
|
|
||
| // Extra half rate clocks generated in the CDU | ||
| clks[ADSP_SC57X_CLK_OCLK0_HALF] = clk_register_fixed_factor(NULL, "oclk_0_half", | ||
| clks[ADSP_SC57X_CLK_CGU0_OCLK_HALF] = clk_register_fixed_factor(NULL, "oclk_0_half", |
Contributor
There was a problem hiding this comment.
for this commit's message, restrict to 72 columns
Contributor
There was a problem hiding this comment.
and same for a few of the other commit mesages
Author
There was a problem hiding this comment.
I went over all the commits and wrapped them, so they don't exceed 72 chars.
| clk->cdu_clko, clk->parent_data, | ||
| clk->mux_table, clk->num_parents, | ||
| clk->clock_flags, &cdu_lock, | ||
| cdu_match_data->reserved_mask); |
Contributor
There was a problem hiding this comment.
wouldn't it make more sense to just pass a struct adsp_cdu_clks pointer to adsp_cdu_register() rather than unpacking it all here?
| return !!(reg & ADSP_CDU_CFG_EN); | ||
| } | ||
|
|
||
| #ifdef CONFIG_DEBUG_FS |
Contributor
There was a problem hiding this comment.
I think you can just stick it at the top without the ifdef guard as most ppl do
Author
There was a problem hiding this comment.
dropped this and move the headers to the top.
| .is_enabled = adsp_cdu_is_enabled, | ||
| #ifdef CONFIG_DEBUG_FS | ||
| .debug_init = adsp_cdu_debug_init, | ||
| #endif |
Contributor
There was a problem hiding this comment.
seems most drivers forego the ifdef guard here too
|
|
||
| spin_lock_irqsave(cdu->lock, flags); | ||
| reg = adsp_cdu_readl(cdu, adsp_cdu_cfg(cdu)); | ||
| spin_unlock_irqrestore(cdu->lock, flags); |
Contributor
There was a problem hiding this comment.
as discussed already, I'm skeptical you need much, if any, locking in this driver. get_parent and set_parent are serialized in the clock core, no?
| select COMMON_CLK_ADI_CDU | ||
| help | ||
| This enables the ADI SC594 clock driver. The driver provides | ||
| This enables the ADI SC598 clock driver. The driver provides |
Contributor
There was a problem hiding this comment.
You write
clk: adi: enable common CDU driver for SC598
Select COMMON_CLK_ADI_CDU from COMMON_CLK_ADI_SC598 so the common Clock Distribution Unit
driver is built with SC598 clock support.
but doesn't the CDU driver always support sc598?
in the end (didn't get there yet) I suppose we just want the Kconfig.platforms entry to select COMMON_CLK_ADI_ADSP_CDU
Contributor
There was a problem hiding this comment.
oh yeah, also, should the kconfig entry be called COMMON_CLK_ADI_ADSP_CDU (note the _ADSP) for consistency?
Author
There was a problem hiding this comment.
Good suggestion, I have renamed it to COMMON_CLK_ADI_ADSP_CDU and reworded the commits to be a little clearer. In terms of the Kconfig.platforms change, yes I think that would be ideal but I think it would be better to do with the other Kconfig changes (after this is done/merged) we discussed earlier (depends on ARCH_ADSP, etc.)
| <&clk ADSP_SC598_CLK_CGU0_DCLK>, | ||
| <&clk ADSP_SC598_CLK_CGU0_DCLK_HALF>, | ||
| <&clk ADSP_SC598_CLK_CGU0_OCLK>, | ||
|
|
Author
There was a problem hiding this comment.
I have dropped all newlines in the CDU clock controller DT nodes.
Contributor
|
resolved all my fixed/stale comments |
Add separate CDU clock binding headers for the ADSP-SC573, ADSP-SC589, ADSP-SC594, and ADSP-SC598 SoCs. Define the clock IDs exported by each SoC's CDU controller in its corresponding header. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The half rate clock IDs do not consistently identify which CGU provides the source clock, unlike the other CGU clock IDs. Rename the half rate clock IDs to include the corresponding CGU index and update the ADSP clock drivers to use the new names. Keep the existing numeric clock IDs unchanged. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC598 clock driver does not currently register the CCLK1_0 clock from CGU0. Add a clock ID for CGU0 CCLK1_0 and register the clock as a gate from the CGU0 CDIV output. This makes CCLK1_0 available as an input clock to the SC598 CDU clock controller. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Add a binding for the CDU on the ADSP-SC573, ADSP-SC589, ADSP-SC594, and ADSP-SC598 SoCs. Describe the register region, input clocks, and clock outputs provided by the CDU. Add the new binding file to the MAINTAINERS entry for the ADI SC5xx platform. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC5xx clock controller binding currently describes the Clock Generation Units and Clock Distribution Unit as a single provider. The CDU is now described as a separate clock controller with its own register region and binding. Update the existing binding to describe only the CGU and PLL register blocks. Remove the CDU register region from the examples and adjust the allowed register count for the SC57x, SC58x, SC594, and SC598 clock controllers. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Introduce the ADSP CDU (Clock Distribution Unit) clock driver. The
current clock drivers model the core hardware blocks (CGUs/CDU/PLLs)
as a single clock controller even though each block has separate
register blocks, separate functionality and uses.
This driver improves on and introduces various changes:
- The current ADSP clock drivers incorrectly models each CDU output by
using the generic clk_register_mux() implementation. However, this is
not suitable for the ADSP CDU because the generic kernel mux
implementation does not account for hardware specific requirements
such as polling and waiting for the corresponding CLKOn bit or
checking whether the LOCK bit is set. Furthermore, the generic mux
set_parent operation performs the register update and returns success
without waiting for CDU configuration completion, checking the LOCK
bit, or verifying the selector readback. As a result, hardware
timeouts, locked registers, and failed selector updates cannot be
detected or reported to CCF.
- The current driver registers every CDU mux and gate clock as
an early clock, this is not needed because there are no early mux
clock consumers. Generally we should limit the number of early clocks
we register since the core driver model (and all the benefits which
come with it) is not available at early clock registration time, the
idiomatic thing to do is convert this to a platform driver, taking
advantage of modern kernel APIs and limiting which clocks are
registered early.
- Rate requests on CDU output clocks may need to propagate to their
upstream PLL and divider clocks. During development this was
observed to have side effects on the ADSP-SC598, where changing the
TRACE CDU clock rate also affected other clocks sharing the same
upstream parent and caused the system to become unusable.
Retain CLK_SET_RATE_PARENT so consumers can propagate rate requests
to the programmable upstream clock tree. Also add the
CLK_OPS_PARENT_ENABLE flag to keep the relevant parent clocks enabled
while parent operations are performed.
- Register the CDU CLKINSEL mux as a writable clock with its own
clk_ops so Linux can select the reference clock supplied to CGU1.
- Another useful addition is adding a debugfs interface to dump CDU
related registers, such as EN, SEL, LOCK, LWERR, etc. This can be
useful for general clock related debugging. Furthermore the CDU
revision IDs are printed during probe, before these were
simply ignored.
- The current clock driver models the grounded input clocks ("N/A")
as dummy clocks however this is not needed because the SoC docs
state "Unused input clocks are grounded internally and never
selected". Adding them is not needed because they are not used
or selected, a cleaner design is to simply expose the input clocks
which can be selected. Furthermore introduce MUX_TABLES which map
parent clocks to CLKOn input SELs, this nicely fits into what the
CCF mux APIs already expose and expect.
- The old driver utilises string based parents which is the "legacy"
way to define and let CCF know about clock parents, it has various
issues and can be brittle. The modern and more preferred way (especially
when the parent clock is owned by an external clock controller as in
our case) is to use struct clk_parent_data.fw_name which introduces
a closer coupling with the device tree where clocks are resolved
directly through DT phandles to clock controllers instead of relying
on globally unique internal clock names.
Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Select COMMON_CLK_ADI_ADSP_CDU from COMMON_CLK_ADI_SC598 so the common Clock Distribution Unit driver is built for the ADSP-SC598. Also correct the SC598 help text, which incorrectly referred to the SC594 clock driver. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC598 clock driver currently maps the CDU register block alongside CGU0, CGU1, and the third PLL. It also registers the CDU CLKINSEL mux and the mux and gate clocks for the CDU outputs. These clocks are now registered by the common ADSP CDU driver. Update the SC598 clock driver to: - remove the third MMIO resource mapping for the CDU, since the common CDU driver now owns that register block - update the third PLL resource index after removing the CDU resource - keep a private lock for the remaining SC598 CGU and PLL clock registrations - change the parents of the CGU1 and third PLL input dividers from the locally registered "cgu1_in_sel" clock to the "cdu_clkinsel" clock registered by the common CDU driver - remove the CDU CLKINSEL registration since this is registered by the common CDU driver - remove the SC598 CDU output mux and gate clock registrations since these are now registered by the common CDU driver - remove the parent arrays used by the legacy CDU mux registrations After this change, clk-adi-sc598 registers the SC598 CGU0, CGU1, and third PLL clocks, while the common ADSP CDU driver owns the CDU register block, CLKINSEL mux, and CDU output clocks. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC598 clock controller node currently describes both the Clock Generation Units and the Clock Distribution Unit as a single controller. The CGUs and CDU are distinct hardware blocks with separate register regions and functions. Describe the CDU as a separate clock controller so it can be managed by the common ADSP CDU driver. Remove the CDU register range from the existing CGU clock node and add a standalone CDU node with its required input clocks, including CCLK1_0. Update the CPU, SPI, OSPI, Ethernet, and eMMC consumers to use the new CDU clock provider. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC598 clock ID namespace currently represents each CDU output as two separate clocks: a _SEL clock for the input mux and a second clock for the output gate. The common ADSP CDU driver models the mux and gate as a single CCF clock and exports the CDU outputs through the SC598-specific CDU clock provider. Remove the legacy SC598 CDU mux and gate IDs from the main SC598 clock namespace. Renumber the remaining PLL3 and DDR clock IDs, place the new CGU0 CCLK1 clock after them, and update ADSP_SC598_CLK_END accordingly. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Select COMMON_CLK_ADI_ADSP_CDU from COMMON_CLK_ADI_SC594 so the common Clock Distribution Unit driver is built for the ADSP-SC594. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC594 clock driver currently maps the CDU register block alongside CGU0 and CGU1. It also registers the CDU CLKINSEL mux and the mux and gate clocks for the CDU outputs. These clocks are now registered by the common ADSP CDU driver. Update the SC594 clock driver to: - remove the third MMIO resource mapping for the CDU, since the common CDU driver now owns that register block - change the parent of the CGU1 DF divider from the locally registered "cgu1_in_sel" clock to the "cdu_clkinsel" clock registered by the common CDU driver - remove the local CDU CLKINSEL registration - remove the SC594 CDU output mux and gate clock registrations - remove the parent arrays used by the old CDU output mux registrations After this change, clk-adi-sc594 registers the SC594 CGU clocks, while the CDU driver owns the CDU register block, CLKINSEL mux, and CDU output clocks. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC594 clock controller node currently describes both the Clock Generation Units and the Clock Distribution Unit as a single controller. This does not reflect the hardware layout because the CGUs and CDU are distinct hardware blocks with separate register blocks and different functions. Describe the CDU as a separate clock controller so that it can be managed by the common ADSP CDU driver shared across the ADSP SoC family. Remove the CDU register range from the existing clock node, add a standalone CDU node with its required CGU input clocks, and update the CPU, SPI, OSPI, and Ethernet nodes to consume clocks from the new CDU provider. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC594 clock ID namespace currently represents each CDU output as two separate clocks: a _SEL clock for the input mux and a second clock for the output gate. A CDU output is controlled by a single CDU_CFG register, with the SEL field selecting its parent and the EN field controlling the output gate. The common ADSP CDU driver therefore models each output as a single CCF clock. Remove the legacy SC594 CDU mux and gate IDs from the main SC594 clock namespace and update ADSP_SC594_CLK_END accordingly. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Select COMMON_CLK_ADI_ADSP_CDU from COMMON_CLK_ADI_SC58X so the common Clock Distribution Unit driver is built for the ADSP-SC589. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC589 clock driver currently maps the CDU register block alongside CGU0 and CGU1. It also registers the CDU CLKINSEL mux and the mux and gate clocks for the CDU outputs. These clocks are now registered by the common ADSP CDU driver. Update the SC589 clock driver to: - remove the third MMIO resource mapping for the CDU, since the common CDU driver now owns that register block - change the parent of the CGU1 DF divider from the locally registered "cgu1_in_sel" clock to the "cdu_clkinsel" clock registered by the common CDU driver - remove the local CDU CLKINSEL mux registration - remove the SC589 CDU output mux and gate clock registrations since these are now registered by the common CDU driver - remove the parent arrays used by the old CDU output mux registrations, since these are no longer needed and are handled by the common CDU driver After this change, clk-adi-sc58x registers the SC589 CGU clocks, while the CDU driver owns the CDU register block, CLKINSEL mux, and CDU output clocks. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC589 clock controller node currently describes both the Clock Generation Units and the Clock Distribution Unit as a single controller. This does not reflect the hardware layout because the CGUs and CDU are distinct hardware blocks with separate register blocks and functions. Describe the CDU as a separate clock controller so that it can be managed by the common ADSP CDU driver shared across the ADSP SoC family. Remove the CDU register range from the existing clock node and add a standalone CDU node with the required SYS_CLKIN and CGU input clocks. Update the CPU, Ethernet, and MMC nodes to consume their CDU output clocks from the new provider. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC58X clock ID namespace currently represents each CDU output as two separate clocks: a _SEL clock for the input mux and a second clock for the output gate. A CDU output is controlled by a single CDU_CFG register, with the SEL field selecting its parent and the EN field controlling the output gate. The common ADSP CDU driver therefore models each output as a single CCF clock. Remove the legacy SC58X CDU mux and gate IDs from the main SC58X clock namespace and update ADSP_SC58X_CLK_END accordingly. The CDU outputs are instead exported by the dedicated SC589 CDU provider using the IDs defined in adi,adsp-sc589-cdu.h. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Select COMMON_CLK_ADI_ADSP_CDU from COMMON_CLK_ADI_SC57X so the common Clock Distribution Unit driver is built for the ADSP-SC573. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC57X clock driver currently maps the CDU register block alongside CGU0 and CGU1. It also registers the CDU CLKINSEL mux and the mux and gate clocks for the CDU outputs. These clocks are now registered by the common ADSP CDU driver. Update the SC57X clock driver to: - remove the third MMIO resource mapping for the CDU, since the common CDU driver now owns that register block - change the parent of the CGU1 DF divider from the locally registered "cgu1_in_sel" clock to the "cdu_clkinsel" clock registered by the common CDU driver - remove the local CDU CLKINSEL mux registration - remove the SC57X CDU output mux and gate clock registrations since these are now handled by the common CDU driver - remove the parent arrays used by the old CDU output mux registrations After this change, clk-adi-sc57x registers the SC57X CGU clocks, while the CDU driver owns the CDU register block, CLKINSEL mux, and CDU output clocks. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The legacy ADI clock drivers previously used common CDU register offsets and helper functions to register the CDU mux and gate clocks. The common ADSP CDU driver now owns those clocks and calculates the CDU_CFG register address from the CDU output index, so the legacy definitions and helpers are no longer needed. Remove the obsolete CDU_CFG register offsets, CDU CLKINSEL and mux definitions, and the unused cdu_mux() and cdu_gate() helpers from the common ADI clock header. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC573 clock controller node currently describes both the Clock Generation Units and the Clock Distribution Unit as a single controller. The CGUs and CDU are distinct hardware blocks with separate register regions and functions. Describe the CDU as a separate clock controller so it can be managed by the common ADSP CDU driver. Remove the CDU register range from the existing clock node and add a standalone CDU node with the required SYS_CLKIN and CGU input clocks. Update the CPU, Ethernet, and MMC nodes to consume their CDU output clocks from the new provider. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The SC57X clock ID namespace currently represents each CDU output as two separate clocks: a _SEL clock for the input mux and a second clock for the output gate. A CDU output is controlled by a single CDU_CFG register, with the SEL field selecting its parent and the EN field controlling the output gate. The common ADSP CDU driver therefore models each output as a single CCF clock. Remove the legacy SC57X CDU mux and gate IDs from the main SC57X clock namespace and update ADSP_SC57X_CLK_END accordingly. The CDU outputs are instead exported by the dedicated SC573 CDU provider using the IDs defined in adi,adsp-sc573-cdu.h. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The legacy CDU mux clock registration used a zero rate dummy clock for
grounded ("N/A") mux input clocks.
The common ADSP CDU driver represents only valid parent clocks and no
longer requires the dummy clock.
Remove the unused dummy fixed rate clock registrations from the SC573,
SC589, SC594, and SC598 clock drivers. Keep the existing clock binding
IDs unchanged for now.
Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Author
|
v2:
v3:
v4:
|
Author
|
Looks like I need to add SC846 support |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Description
This PR introduces a common Clock Distribution Unit (CDU) driver for the Analog Devices ADSP-SC573, ADSP-SC589, ADSP-SC594, and ADSP-SC598 SoCs. The CDU is separated into its own platform driver, with each configurable output modelled as a single CCF clock combining the input selector mux and output gate. New Device Tree nodes describe the CDU controller, its input clocks, and how those clocks are routed to peripherals across the SoC.
These changes also modernise the ADI clock infrastructure and align it with current upstream APIs and practices, making future Linux kernel upstream submission easier (which will come soon hopefully).
The driver was boot tested on ADSP-SC573, ADSP-SC589, ADSP-SC594, and ADSP-SC598 hardware. Before and after clk_summary dumps were compared on each platform. The new CDU clocks seem to preserve the existing clock rates, selected parents, enable states, and peripheral consumers, the clk_summary pre and post changes can be found below
PR Type
PR Checklist