Skip to content

clk: adi: sc846: Fix and cleanup ADSP-SC846 clock tree - #145

Open
qasim-ijaz wants to merge 6 commits into
adi-u-boot-2025.10.yfrom
sc846-clk-fix
Open

clk: adi: sc846: Fix and cleanup ADSP-SC846 clock tree#145
qasim-ijaz wants to merge 6 commits into
adi-u-boot-2025.10.yfrom
sc846-clk-fix

Conversation

@qasim-ijaz

@qasim-ijaz qasim-ijaz commented Aug 5, 2026

Copy link
Copy Markdown

This PR introduces a few changes to correct the ADSP-SC846 U-Boot clock driver. The driver had a few issues with the clock tree being incorrect (some parent clocks were wrong, some clocks were not even registered, duplicate clock registrations exist, etc.). The changes also include some small fixes like return value checks, cleaning up the clock driver header and basically aligning it a bit more with the Linux clock driver . I have compiled the changes, but hardware testing is still needed (since I don't have a SC846 board right now).

PR Type

  • Bug fix (a change that fixes an issue)
  • New feature (a change that adds new functionality)
  • Breaking change (a change that affects other repos or cause CIs to fail)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have compiled my changes, including the documentation
  • I have tested the changes on the relevant hardware
  • I have updated the documentation outside this repo accordingly
  • I have provided links for the relevant upstream lore

The SC846 clock driver was derived from the existing ADI clock support, but
its SoC specific clock topology was implemented and is maintained by Analog Devices.

Remove the stale Timesys authorship and porting annotations, and describe the file
as the SC846 clock driver. Lastly update the Analog Devices copyright year from
2022 to 2026.

Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The PLLCLK fixed factor clock currently divides its VCO parent rate by
four. The HRM specifies that PLLCLK divides its parent rate by two.

Set the PLLCLK divisor to two.

Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The current driver uses the cguX_pllclk clock as parents for the divider clocks
but this is incorrect because the divider clocks should have a parent which divides
the rate of pllclk by 2 according to the HRM. To account for this introduce a fixed
factor clock cguX_pllclk_half which does that and make that the parent of the divider
clocks. As part of this add the needed clock binding IDs: ADSP_SC846_CLK_CGU0_PLLCLK_HALF
and ADSP_SC846_CLK_CGU1_PLLCLK_HALF.

Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
Check the return values from clk_get_by_name() when obtaining the dummy
and sys_clkin0 input clocks. Return the lookup error instead of
continuing with invalid clock handles.

Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
CCLK2_1 is registered twice also it is a CGU1 clock not a CGU0 clock.
Remove the incorrect CGU0 registration so CCLK2_1. Also remove the unused
clock binding and renumber the remaining SC846 clock IDs.

Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
The driver registers the CGU0 DCLK1_0 fixed factor clock but is missing
the corresponding DCLK1_1 clock from CGU1.

Add DCLK1_1 as a fixed factor clock derived directly from the CGU1 VCO
with a mult and div value of 1 and 5.

Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>
@qasim-ijaz
qasim-ijaz requested review from a team August 5, 2026 13:27
@qasim-ijaz

qasim-ijaz commented Aug 5, 2026

Copy link
Copy Markdown
Author

Another small thing i noticed is that pwm_sels references clkpwm which is not a registered clock, I decided to keep it in there for now (we could make it into a dummy clock i guess but no point imo, since no one uses it) I think we would need to further develop out the clock driver to support clkpwm because it looks like its related to the frac-n-pll which we don't support atm.

Edit:

Pre clock changes Uboot

=> clk dump
 Rate               Usecnt      Name
------------------------------------------
 0                    0        |-- dummy
 0                    0        |   |-- arm0_sel
 0                    0        |   |   `-- arm0
 0                    0        |   `-- arm1_sel
 0                    0        |       `-- arm1
 25000000             0        |-- sys_clkin0
 25000000             1        |   |-- cgu0_df
 4000000000           1        |   |   `-- cgu0_vco
 1000000000           1        |   |       |-- cgu0_pllclk
 1000000000           0        |   |       |   |-- cgu0_cdiv
 1000000000           0        |   |       |   |   `-- cclk0_0
 1000000000           0        |   |       |   |       `-- sharc_sel
 1000000000           0        |   |       |   |           `-- sharc
 500000000            1        |   |       |   |-- sysclk_0
 125000000            1        |   |       |   |   |-- cgu0_s0seldiv
 125000000            1        |   |       |   |   |   `-- sclk0_0
 125000000            0        |   |       |   |   |       |-- spi_sel
 125000000            0        |   |       |   |   |       |   `-- spi
 125000000            0        |   |       |   |   |       |-- gige_sel
 125000000            0        |   |       |   |   |       |   `-- gige
 125000000            0        |   |       |   |   |       |-- xspi_sel
 125000000            0        |   |       |   |   |       |   `-- xspi
 125000000            0        |   |       |   |   |       |-- trace_sel
 125000000            0        |   |       |   |   |       |   `-- trace
 125000000            0        |   |       |   |   |       `-- mshc_sel
 125000000            0        |   |       |   |   |           `-- mshc
 250000000            0        |   |       |   |   |-- cgu0_s1seldiv
 500000000            0        |   |       |   |   `-- pwm_sel
 500000000            0        |   |       |   |       `-- pwm
 500000000            0        |   |       |   |-- cgu0_ddiv
 500000000            0        |   |       |   |   `-- dclk_0
 500000000            0        |   |       |   |       `-- cdu_ddr_sel
 500000000            0        |   |       |   |           `-- cdu_ddr
 100000000            0        |   |       |   |-- cgu0_odiv
 100000000            0        |   |       |   |   `-- oclk_0
 100000000            0        |   |       |   |       `-- can_sel
 100000000            0        |   |       |   |           `-- can
 333333334            0        |   |       |   `-- cgu0_s1selexdiv
 333333334            0        |   |       |       `-- cgu0_sclk1sel
 333333334            0        |   |       |           `-- sclk1_0
 333333334            0        |   |       |               `-- spdif_sel
 333333334            0        |   |       |                   `-- spdif
 1333333333           0        |   |       |-- cclk2_0
 1333333333           0        |   |       |-- cclk2_1
 800000000            0        |   |       `-- dclk1_0
 25000000             0        |   `-- cgu1_df
 3600000000           0        |       `-- cgu1_vco
 900000000            0        |           |-- cgu1_pllclk
 450000000            0        |           |   |-- cgu1_cdiv
 450000000            0        |           |   |   `-- cclk0_1
 180000000            0        |           |   |-- sysclk_1
 45000000             0        |           |   |   |-- cgu1_s0seldiv
 90000000             0        |           |   |   `-- cgu1_s1seldiv
 300000000            0        |           |   |-- cgu1_ddiv
 300000000            0        |           |   |   `-- dclk_1
 300000000            0        |           |   |       `-- lp_sel
 300000000            0        |           |   |           `-- lp
 112500000            0        |           |   |-- cgu1_odiv
 112500000            0        |           |   |   `-- oclk_1
 112500000            0        |           |   |-- cgu1_s0selexdiv
 112500000            0        |           |   |   `-- cgu1_sclk0sel
 112500000            0        |           |   |       `-- sclk0_1
 180000000            0        |           |   `-- cgu1_s1selexdiv
 180000000            0        |           |       `-- cgu1_sclk1sel
 180000000            0        |           |           `-- sclk1_1
 180000000            0        |           |               `-- xspi2_sel
 180000000            0        |           |                   `-- xspi2
 1200000000           0        |           `-- cclk2_1
 25000000             0        |-- sys_clkin1

Post clock changes Uboot

=> clk dump
 Rate               Usecnt      Name
------------------------------------------
 0                    0        |-- dummy
 0                    0        |   |-- arm0_sel
 0                    0        |   |   `-- arm0
 0                    0        |   `-- arm1_sel
 0                    0        |       `-- arm1
 25000000             0        |-- sys_clkin0
 25000000             1        |   |-- cgu0_df
 4000000000           1        |   |   `-- cgu0_vco
 2000000000           1        |   |       |-- cgu0_pllclk
 1000000000           1        |   |       |   `-- cgu0_pllclk_half
 1000000000           0        |   |       |       |-- cgu0_cdiv
 1000000000           0        |   |       |       |   `-- cclk0_0
 1000000000           0        |   |       |       |       `-- sharc_sel
 1000000000           0        |   |       |       |           `-- sharc
 500000000            1        |   |       |       |-- sysclk_0
 125000000            1        |   |       |       |   |-- cgu0_s0seldiv
 125000000            1        |   |       |       |   |   `-- sclk0_0
 125000000            0        |   |       |       |   |       |-- spi_sel
 125000000            0        |   |       |       |   |       |   `-- spi
 125000000            0        |   |       |       |   |       |-- gige_sel
 125000000            0        |   |       |       |   |       |   `-- gige
 125000000            0        |   |       |       |   |       |-- xspi_sel
 125000000            0        |   |       |       |   |       |   `-- xspi
 125000000            0        |   |       |       |   |       |-- trace_sel
 125000000            0        |   |       |       |   |       |   `-- trace
 125000000            0        |   |       |       |   |       `-- mshc_sel
 125000000            0        |   |       |       |   |           `-- mshc
 250000000            0        |   |       |       |   |-- cgu0_s1seldiv
 500000000            0        |   |       |       |   `-- pwm_sel
 500000000            0        |   |       |       |       `-- pwm
 500000000            0        |   |       |       |-- cgu0_ddiv
 500000000            0        |   |       |       |   `-- dclk_0
 500000000            0        |   |       |       |       `-- cdu_ddr_sel
 500000000            0        |   |       |       |           `-- cdu_ddr
 100000000            0        |   |       |       |-- cgu0_odiv
 100000000            0        |   |       |       |   `-- oclk_0
 100000000            0        |   |       |       |       `-- can_sel
 100000000            0        |   |       |       |           `-- can
 333333334            0        |   |       |       `-- cgu0_s1selexdiv
 333333334            0        |   |       |           `-- cgu0_sclk1sel
 333333334            0        |   |       |               `-- sclk1_0
 333333334            0        |   |       |                   `-- spdif_sel
 333333334            0        |   |       |                       `-- spdif
 1333333333           0        |   |       |-- cclk2_0
 800000000            0        |   |       `-- dclk1_0
 25000000             0        |   `-- cgu1_df
 3600000000           0        |       `-- cgu1_vco
 1800000000           0        |           |-- cgu1_pllclk
 900000000            0        |           |   `-- cgu1_pllclk_half
 450000000            0        |           |       |-- cgu1_cdiv
 450000000            0        |           |       |   `-- cclk0_1
 180000000            0        |           |       |-- sysclk_1
 45000000             0        |           |       |   |-- cgu1_s0seldiv
 90000000             0        |           |       |   `-- cgu1_s1seldiv
 300000000            0        |           |       |-- cgu1_ddiv
 300000000            0        |           |       |   `-- dclk_1
 300000000            0        |           |       |       `-- lp_sel
 300000000            0        |           |       |           `-- lp
 112500000            0        |           |       |-- cgu1_odiv
 112500000            0        |           |       |   `-- oclk_1
 112500000            0        |           |       |-- cgu1_s0selexdiv
 112500000            0        |           |       |   `-- cgu1_sclk0sel
 112500000            0        |           |       |       `-- sclk0_1
 180000000            0        |           |       `-- cgu1_s1selexdiv
 180000000            0        |           |           `-- cgu1_sclk1sel
 180000000            0        |           |               `-- sclk1_1
 180000000            0        |           |                   `-- xspi2_sel
 180000000            0        |           |                       `-- xspi2
 1200000000           0        |           |-- cclk2_1
 720000000            0        |           `-- dclk1_1
 25000000             0        |-- sys_clkin1

@qasim-ijaz qasim-ijaz added this to ADSP Aug 5, 2026
@qasim-ijaz qasim-ijaz self-assigned this Aug 5, 2026
@qasim-ijaz qasim-ijaz moved this to In Development in ADSP Aug 5, 2026
@qasim-ijaz qasim-ijaz added the sc846 Support for the ADSP-SC846 label Aug 5, 2026
@qasim-ijaz qasim-ijaz removed the status in ADSP Aug 5, 2026
@qasim-ijaz qasim-ijaz moved this to In Development in ADSP Aug 5, 2026
@qasim-ijaz qasim-ijaz added the llm review Request a review from a LLM Reviewer label Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

LLM review

This series fixes the ADI SC846 clock driver: corrects the PLLCLK divisor, adds a
missing PLLCLK_HALF clock that feeds the CGU dividers, hardens input-clock
lookup error handling, removes a duplicated CCLK2_1 clock, and adds the missing
CGU1 DCLK1_1 clock.

run: 31021481969

Verification data

No public SC846 HRM exists yet (not on the ADI doctools sitemap), so the clock
topology claims in these commit messages were instead cross-checked against
arch/arm/mach-sc5xx/init/EHP2_LP4_1D_2000/adi_pwr_SC846_Family_{1000,1200}MHz_config.h,
which is already in this tree and documents the intended fVCO/fPLL/CCLK/
SYSCLK/OCLK/DCLK values and MSEL/CSEL/SYSSEL/OSEL/DSEL/S0SEL/S1SEL/ S0SELEX/S1SELEX register settings for both CGUs.

Using sc5xx_cgu_pll_get_rate()'s VCO = parent * MSEL * 2 formula together with
the config header's MSEL values (CGU0=80, CGU1=72, SYS_CLKIN0=25MHz), every
downstream frequency (CCLK, SYSCLK, DCLK, OCLK, SCLK0/1, SCLK0EX/1EX, CCLK2, and
the new DCLK1_1) was recomputed and matched exactly, but only when PLLCLK_HALF
(not PLLCLK) is used as the parent of CDIV/SYSSEL/DSEL/OSEL/S0SELEX/
S1SELEX — confirming f5dcc675 and 0bdb4c3a are correct. e26f0ede
(dclk1_1 = cgu1_vco/5 = 720MHz) and 5766ba5a (cclk2_1 = cgu1_vco/3 = 1200MHz,
kept only under CGU1) were verified the same way.

Also checked: the dt-bindings renumbering in 5766ba5a leaves no gaps/duplicates
in IDs 0..69 (every ID defined once, assigned once), and
arch/arm/dts/sc846-som.dts/sc846-som-ezkit.dts only reference the symbolic
macros (never raw numbers), so the renumbering is safe.

Build

gcc-aarch64-linux-gnu was installed (not present in the sandbox) to build
sc846-som-ezkit_defconfig with ARCH=arm CROSS_COMPILE=aarch64-linux-gnu-. The
u-boot ELF links cleanly; the only failure is the final, unrelated
u-boot.ldr step (aarch64-linux-gnu-ldr binary not available in this sandbox).
drivers/clk/adi/clk-adi-sc846.o built standalone with W=1 and no warnings.

CI warnings

checkpatch --strict (same ignore list as CI) was re-run and reproduces the
reported warnings exactly:

  • "DT binding docs and includes should be a separate patch" on 0bdb4c3a,
    5766ba5a, e26f0ede — style preference only; the shared bindings header is
    updated together with the single driver that owns these IDs, which is normal
    practice in this file. Non-blocking.
  • "Possible unwrapped commit description" on eee2fdf9 and 0bdb4c3a — a couple
    of commit-message lines exceed 75 columns. Cosmetic only.

Notes

  • pwm_sels[] still lists the unregistered "clkpwm" clock (frac-N PLL, not yet
    supported) — already called out and accepted in the PR discussion, no action
    needed here.
  • Out of scope for this range: arm_sels[] (unchanged by this series) selects
    "cclk2_0" for the ARM CDU mux, while the vendor config header's summary table
    names the ARM target clock CCLK2_1 @ 1200MHz (cclk2_0 would be 1333.33MHz).
    Worth a follow-up check against ADI's internal HRM, but not introduced or
    touched by these commits.

No functional issues found; no fixup patches are proposed for this range.

Install instructions

The following one-liner installs the script if not present already:

curl -fSsL "https://raw.githubusercontent.com/analogdevicesinc/doctools/refs/heads/main/ci/scripts/apply-patches.sh"      -o ~/.local/bin/apply-patches.sh &&   grep -q "/apply-patches.sh" ~/.bashrc || echo "source ~/.local/bin/apply-patches.sh" >> $_ ; . $_

More information at AI Usage.

@qasim-ijaz qasim-ijaz changed the title clk: adi: Fix and cleanup ADSP-SC846 clock tree clk: adi: sc846: Fix and cleanup ADSP-SC846 clock tree Aug 5, 2026
@pamolloy

pamolloy commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

@qasim-ijaz there should be a SC846 reference board in Edinburgh that @artursartamonovsadi is using.

Note the error in the LLM review. An HRM does exist. Please update ADI doctools sitemap accordingly.

@qasim-ijaz

qasim-ijaz commented Aug 10, 2026

Copy link
Copy Markdown
Author

I have now tested this series on the ADSP-SC846, Above I have captured U-Boot clk dump output before and after the changes:

  • CGU0 PLLCLK changes from 1 GHz to 2 GHz, with PLLCLK_HALF at 1 GHz, matching the existing Linux clock tree.
  • CGU1 PLLCLK changes from 900 MHz to 1.8 GHz, with PLLCLK_HALF at 900 MHz, also matching the Linux clock tree.
  • The affected divider clocks retain their expected output rates after being reparented to PLLCLK_HALF.
  • The duplicate CCLK2_1 clock is removed and the missing CGU1 DCLK1_1 clock is now present.

Linux also boots successfully with U-Boot built from these changes.

@qasim-ijaz

Copy link
Copy Markdown
Author

Note the error in the LLM review. An HRM does exist. Please update ADI doctools sitemap accordingly.

@pamolloy where can i find this?

@ozan956

ozan956 commented Aug 11, 2026

Copy link
Copy Markdown
Member

Note the error in the LLM review. An HRM does exist. Please update ADI doctools sitemap accordingly.

@pamolloy where can i find this?

https://github.com/analogdevicesinc/doctools

https://www.analog.com/media/en/dsp-documentation/processor-manuals/adsp-2184x-adsp-sc84x-hrm.pdf

e: you should be eligible to udpate the wiki, but idk about the analog.com

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llm review Request a review from a LLM Reviewer sc846 Support for the ADSP-SC846

Projects

Status: In Development

Development

Successfully merging this pull request may close these issues.

3 participants