Add SC846 EMMC support by qasim-ijaz · Pull Request #164 · analogdevicesinc/u-boot

qasim-ijaz · 2026-09-08T15:20:36Z

Introduce eMMC support for the ADSP-SC846. This PR extends the current adi_sdhci mmc driver to now support the ADSP-SC846. The driver currently only supports the ADSP-SC598. The changes here have been tested in U-Boot and relevant U-Boot commands such as mmc list and mmc info seem to work as expected. I am also able to read/write to eMMC.

sipraga

I know it's a draft but here's some driveby comments since I was curious

github-actions · 2026-10-06T06:47:15Z

LLM review

This series adds eMMC support for ADSP-SC846 (MISCREG timer clock enable, SDHCI capability-based
clock discovery, DWC MSHC PHY/pad configuration) and the corresponding sc846.dtsi/sc846-som.dts
nodes.

Session: 37423885987

Review comments

All four comments from @sipraga on arch/arm/dts/sc846.dtsi (syscon node name/compatible,
0x400 register size, adi,miscreg-syscon property name) are addressed by f8dbc397.

CI warnings

The three checkpatch warnings reported by checks/checks were reproduced locally
(scripts/checkpatch.pl --strict with the same ignore list):

9f06e476 and 2fa82056: unwrapped commit description lines (>75 chars).
fabdb7fe: missing commit description (title only).

Fixup patches for these are attached below.

Verification data

No public ADSP-SC846 datasheet exists yet (checked the ADI PDF sitemap — the SC5xx family
entries stop at sc598/sc834). The new PHY/pad register offsets and bitfields in
drivers/mmc/adi_sdhci.c were instead cross-checked against the in-tree
drivers/mmc/snps_sdhci.c and against upstream Linux's drivers/mmc/host/sdhci-of-dwcmshc.c
(fetched from raw.githubusercontent.com/torvalds/linux): all offsets (0x300 PHY base,
0x52c EMMC_CTL, pad register offsets, GENMASK/BIT fields) match the generic Synopsys
DWC MSHC layout, and the "hold PHY in reset while configuring pads, release last" sequence
matches the pattern used by the eic7700/k230 vendors in that driver. sdhci_setup_cfg()
deriving max_clk from SDHCI_CAPABILITIES when host->max_clk == 0 (claimed by 2fa82056)
was confirmed by reading drivers/mmc/sdhci.c:939-957.

Builds were run locally with gcc-aarch64-linux-gnu for sc846-som_defconfig and
sc598-som-ezlite-spl_defconfig (both SC5xx targets use the aarch64 toolchain): u-boot
and u-boot.dtb build cleanly, no compiler warnings in adi_sdhci.c, and nm confirms the
new SC846 callbacks are linked in. The decompiled sc846-som.dtb (scripts/dtc/dtc) matches
the source .dts.

Two items worth double-checking before merge, in light of 3 hardware-test CI jobs failing
(sc598 ezkit, sc598 ezlite, sc846 ezkit — only "exit code 1" available in these logs, no
detail):

DWCMSHC_PHY_CNFG_PWRGOOD (bit 1) is defined but never read/polled; Linux's k230 driver
polls the equivalent bit before releasing PHY reset. May be fine for SC846, but currently
dead code.
The new mmc0 node in sc846.dtsi has no pinctrl-* properties and sc846.dtsi has no
eMMC pin group at all, unlike sc598-som.dtsi's mmc0 (pinctrl-0 = <&mmc_defaults>).
Could be intentional (dedicated, non-muxed eMMC pins on SC846) but there's no public HRM to
confirm this.

Suggested patches

0001-amend-mmc-adi_sdhci-add-SC846-eMMC-timer-clock-defin.patch (9f06e476) — rewraps the
commit description to ≤75 chars/line.
0002-amend-mmc-adi_sdhci-get-base-clock-from-capabilities.patch (2fa82056) — rewraps the
commit description to ≤75 chars/line.
0003-amend-arm-dts-sc846-add-eMMC-controller-nodes.patch (fabdb7fe) — adds a missing
commit description.

These are message-only --fixup=reword commits (empty diffs); no code fixups are suggested
since the two open items above require SC846-specific hardware knowledge we don't have access
to.

Apply with:

cd path/to/repository
export GITHUB_TOKEN=ghp_***
apply-patches --repo=analogdevicesinc/u-boot 37423885987

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.

pamolloy · 2026-10-06T08:35:00Z

A did a local LLM review with different prompting. I'd have to look more carefully, but I think some of this can go upstream once reviewed here. Note how the series is split and the commit messages:

Definition-only commits. Commits 1 and 6 add macros that have no user until
commits 2 and 7. Upstream mmc maintainers will ask for these to be squashed:
after commit 1 the tree carries three unreferenced macros, and after commit 6 it
carries nineteen. Split patches should each be independently meaningful, not
"defines" + "code".

Commit 5 is a fixup of commit 4. The body literally says "Address review
feedback around the SC846 MISCREG syscon". A posted series must not contain its
own review history; squash 5 into 4 and drop the reference. It also fixes up
commit 2 (adi,misc-reg to adi,miscreg-syscon), so the renamed property should
never appear in the posted series at all.

Commit 4 has no commit message body (checkpatch flags this). It also enables
eMMC on sc846-som before the PHY configuration lands in commit 7, so there is a
bisect window where the board probes the controller with an unconfigured PHY. DT
enablement should be the last patch.

Commit 3 does two unrelated things (base clock sourcing, and removing a dead
clk_get_by_index()); split it.

Suggested shape (4 patches):

mmc: adi_sdhci: drop unused clock handle

mmc: adi_sdhci: take the base clock from the capabilities register

mmc: adi_sdhci: add ADSP-SC846 support (match data, timer clock, PHY) or
split into timer + PHY, each with its defines

arm: dts: sc846: add MISCREG syscon and eMMC controller (+ binding doc)

pamolloy · 2026-10-06T08:37:19Z

BASE_CLK_FREQ is documented (R/NW), i.e. a hardwired constant. The actual
MSHC input is mux-selected in CDU_CFG14 (mshc_sels[] = {"sclk0_0", "sysclk_1", ...} in clk-adi-sc846.c). If the CDU is
ever programmed so the
real input is not 200 MHz, sdhci_set_clock() computes the wrong divider and
the card clock is silently wrong. Linux's dwcmshc path takes max_clk from
clk_get_rate() for exactly this reason. Is clk_get_rate() not the
authoritative source here?

Dropping max-frequency removes the board-level cap. sc598-som.dtsi still
carries max-frequency = <50000000>, which this patch makes a no-op; either
honor it or remove it in the same series. The idiomatic U-Boot form is what
zynq_sdhci.c, am654_sdhci.c and rockchip_sdhci.c do:
     ret = mmc_of_parse(dev, &plat->cfg);      /* fills cfg->f_max, bus-width, caps */                                       
     ...                                                                                                                     
     ret = sdhci_setup_cfg(&plat->cfg, host, plat->cfg.f_max, EMMC_MIN_FREQ);                                                
That also makes the hand-rolled bus-width parsing in of_to_plat()
redundant, and makes the non-removable property from commit 4 actually do
something (see below).

pamolloy · 2026-10-06T08:38:44Z

U-Boot already has two DWCMSHC drivers (snps_sdhci.c for TH1520,
cv1800b_sdhci.c), and this makes three. The new macros rename everything:

 ┌──────────────────────────────────────────────────────────────┬──────────────────────────┐                                  
 │ snps_sdhci.c (existing)                                      │ adi_sdhci.c (new)        │                                  
 ├──────────────────────────────────────────────────────────────┼──────────────────────────┤                                  
 │ DWC_MSHC_PTR_PHY_R                                           │ DWCMSHC_PHY_BASE         │                                  
 ├──────────────────────────────────────────────────────────────┼──────────────────────────┤                                  
 │ PHY_CNFG_R                                                   │ DWCMSHC_PHY_CNFG         │                                  
 ├──────────────────────────────────────────────────────────────┼──────────────────────────┤                                  
 │ PHY_DATAPAD_CNFG_R                                           │ DWCMSHC_PHY_DATPAD_CNFG  │                                  
 ├──────────────────────────────────────────────────────────────┼──────────────────────────┤                                  
 │ PHY_PAD_TXSLEW_CTRL_N_MASK                                   │ DWCMSHC_PHY_PAD_TXSLEW_N │                                  
 ├──────────────────────────────────────────────────────────────┼──────────────────────────┤                                  
 │ PHY_CNFG_RSTN_DEASSERT                                       │ DWCMSHC_PHY_CNFG_RSTN    │                                  
 ├──────────────────────────────────────────────────────────────┼──────────────────────────┤                                  
 │ P_VENDOR_SPECIFIC_AREA + DWCMSHC_EMMC_CONTROL (0x500 + 0x2c) │ DWCMSHC_EMMC_CTL (0x52c) │                                  
 └──────────────────────────────────────────────────────────────┴──────────────────────────┘

Upstream will likely ask either to match the existing names or to factor the
shared DWCMSHC defines into a common header. Flattening 0x500 + 0x2c to 0x52c
also hides the vendor-area structure; note that Linux does not hardcode it at
all, it reads the area base from DWCMSHC_P_VENDOR_AREA1 (0xe8).

qasim-ijaz · 2026-10-20T13:09:43Z

It seems the frequency of the eMMC card clock is:
eMMC card clock = mshc clock / (10 bit freq_select from CLK_CTRL_R)

It seems right now it's clocked at 31.25 MHz via:
125MHz / 4 = 31.25MHz

nunojsa

Just one comment about the compatible as I just realized I was messing with the same block (syscon) for the xspi reset lines

nunojsa · 2026-10-23T11:34:47Z

 		};
+
+		miscreg_syscon: syscon@310a9000 {
+			compatible = "adi,adsp-sc846-miscreg-syscon", "syscon";


Looks to be the same block I have for XSPI 1. Good idea to have the compatible consistent with linux I guess.

should be fixed

I would go with adi,adsp-... per the comments I'm always leaving, but you guys have to agree. anyway we can also figure it out later

ah yes! sure im fine with adi,adsp- its been used quite a bit iirc (cdu driver, pinctrl+gpio driver and probably some other places im missing) but yea would be good to be consistent @nunojsa what do you think?

I have to be honest that I don't love much the adsp prefix everywhere. Just adds up while the real name of the platform is sc846! No strong feelings though :)

I mean in a driver we typically have the driver (device) name as prefix. At least in there I'm not really leaning in having adsp_sc846_foo()

Add SC846 specific eMMC miscellaneous register definitions for enabling the timer clock and configuring its divider. Also introduce SC846 SDHCI match data and initialise the eMMC timer through the MISCREG syscon before probing. This enables the eMMC timer clock and programs the CQE engine clock with a divider. The CQE engine clock has the following frequency equation: CQE_freq = EMSI Source clock (CGU0 SYSCLK) / MISCREG_EMMC_TMR_CKDIV Our current SYSCLK is clocked at 500MHz, meaning the CQE engine is clocked at 10MHz via a divider of 50. If SYSCLK changes (unlikely) an appropriate divider value will be chosen. avoiding the following error message during mmc setup: mmc0: Internal clock never stabilised. It should also be noted that the max CQE_freq is 10MHz. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>

The driver previously used the max-frequency device tree property to determine the SDHCI base clock frequency. This works on ADSP-SC598 because its core clock runs at 50 MHz, matching the max-frequency property (and the underlying capabilities register). On ADSP-SC846, the MSHC core clock currently runs at 125 MHz, while the capabilities register reports 200 MHz. To generalise the clock handling for both platforms, obtain the "core" clock rate using clk_get_rate() and use it as host->max_clk. Also remove the now unused max-frequency property from the ADSP-SC598 device tree. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>

The ADSP-SC846 DWC MSHC requires additional PHY configuration for the eMMC interface. Add the SC846 specific setup and invoke it from the SDHCI set_ios_post callback. The generic SDHCI set_ios path programs controller state such as the card clock, bus width, etc. before invoking set_ios_post. Using this hook allows the SC846 specific PHY setup to run after the generic SDHCI I/O configuration has been applied, including when the MMC core changes the I/O configuration during card initialisation. Set CARD_IS_EMMC in the DWC MSHC EMMC_CTL register as per the Synopsis HRM and configure the integrated PHY and PADs with the recommended 1.8V mode settings used by SC846. This configures PHY specific fields such as PHY strength controls, pull up/down settings, slew controls, etc. To allow SC846 to perform its PHY configuration from the generic SDHCI set_ios path, add a per SoC set_ios_post callback to adi_sdhci_data and dispatch the generic SDHCI set_ios_post hook to the appropriate SoC implementation. This keeps the PHY setup specific to ADSP-SC846 while preserving the existing behaviour for other ADI DWC MSHC platforms such as ADSP-SC598. Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>

Add the Synopsys DWC MSHC eMMC controller node to the ADSP-SC846 device tree, including its register range, clock, and bus width configuration, etc. Also add the ADSP-SC846 MISCREG syscon node and reference it from the eMMC controller. The MISCREG block contains the eMMC timer clock control and divider registers required by the controller. Signed-off-by: Vasileios Bimpikas <Vasileios.Bimpikas@analog.com> Signed-off-by: Qasim Ijaz <Qasim.Ijaz@analog.com>

qasim-ijaz self-assigned this Sep 8, 2026

qasim-ijaz added the sc846 Support for the ADSP-SC846 label Sep 8, 2026

qasim-ijaz added this to ADSP Sep 8, 2026

qasim-ijaz requested a review from a team September 8, 2026 15:21

qasim-ijaz force-pushed the sc846-emmc branch 3 times, most recently from dfcfc05 to 8c08623 Compare September 9, 2026 10:19

vasbimpikasadi force-pushed the sc846-emmc branch from 68eaa0f to 4e65c38 Compare September 9, 2026 12:09

sipraga reviewed Sep 9, 2026

View reviewed changes

Comment thread arch/arm/dts/sc846.dtsi Outdated

Comment thread arch/arm/dts/sc846.dtsi Outdated

Comment thread arch/arm/dts/sc846.dtsi Outdated

Comment thread arch/arm/dts/sc846.dtsi Outdated

Comment thread drivers/mmc/adi_sdhci.c Outdated

vasbimpikasadi requested a review from sipraga September 9, 2026 13:51

qasim-ijaz force-pushed the sc846-emmc branch from 0898ba4 to 3d6312f Compare October 5, 2026 13:46

qasim-ijaz marked this pull request as ready for review October 5, 2026 13:58

pamolloy added the llm review Request a review from a LLM Reviewer label Oct 6, 2026

pamolloy requested review from nunojsa and swinchen October 6, 2026 06:28

qasim-ijaz force-pushed the sc846-emmc branch 3 times, most recently from 1a542a4 to c8c39b7 Compare October 8, 2026 13:31

qasim-ijaz force-pushed the sc846-emmc branch from c8c39b7 to f7f4a3b Compare October 19, 2026 13:12

qasim-ijaz force-pushed the sc846-emmc branch from f7f4a3b to 1505ea4 Compare October 21, 2026 11:30

swinchen reviewed Oct 21, 2026

View reviewed changes

Comment thread drivers/mmc/adi_sdhci.c

swinchen approved these changes Oct 21, 2026

View reviewed changes

sipraga reviewed Oct 21, 2026

View reviewed changes

Comment thread arch/arm/dts/sc846.dtsi Outdated

sipraga reviewed Oct 21, 2026

View reviewed changes

Comment thread arch/arm/dts/sc846.dtsi Outdated

qasim-ijaz force-pushed the sc846-emmc branch from 1505ea4 to 6c55907 Compare October 22, 2026 11:02

qasim-ijaz requested review from sipraga, swinchen and vasbimpikasadi October 22, 2026 11:17

sipraga approved these changes Oct 22, 2026

View reviewed changes

nunojsa reviewed Oct 23, 2026

View reviewed changes

qasim-ijaz and others added 4 commits October 23, 2026 14:44

qasim-ijaz force-pushed the sc846-emmc branch from 6c55907 to a719705 Compare October 23, 2026 13:48

qasim-ijaz requested review from nunojsa and sipraga October 23, 2026 13:48

sipraga approved these changes Oct 23, 2026

View reviewed changes

Conversation

qasim-ijaz commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sipraga left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Oct 6, 2026

LLM review

Review comments

CI warnings

Verification data

Suggested patches

Uh oh!

pamolloy commented Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pamolloy commented Oct 6, 2026

Uh oh!

pamolloy commented Oct 6, 2026

Uh oh!

qasim-ijaz commented Oct 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nunojsa left a comment

Choose a reason for hiding this comment

Uh oh!

nunojsa Oct 23, 2026 • 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!

qasim-ijaz Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

sipraga Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

qasim-ijaz Oct 23, 2026 • 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!

nunojsa Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

nunojsa Oct 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

qasim-ijaz commented Sep 8, 2026 •

edited

Loading

pamolloy commented Oct 6, 2026 •

edited

Loading

qasim-ijaz commented Oct 20, 2026 •

edited

Loading

nunojsa Oct 23, 2026 •

edited

Loading

qasim-ijaz Oct 23, 2026 •

edited

Loading