Re: [PATCH 6/7] mmc: sdhci-of-k1: Improve RX tuning window

[Hung-Chun Tseng <alan.tseng.cs@gmail.com>]

On Wed, Sep 02, 2026 at 08:04:27AM +0000, Yixun Lan wrote:
> Raise the minimum delay codes of RX tuning window from 3 to 50, to more
> accurately retrieve a valid configuration.
>
> A window of 3 codes wide leaves no sampling margin, which will result
> tuning tests reporting success on a configuration that drifts out of the
> window under thermal or power variation.

I agree with the motivation, and I have some data from a K1 board that
supports it. But I would like to ask about making 50 a compile-time
constant.

Caveat up front: my board runs the vendor sdhci-spacemit driver
(6.6.63, compatible "spacemit,k1-x-sdhci"), not sdhci-of-k1.c, so the
numbers below are observations from that driver rather than a test of
this series. I could not test the series itself: rootfs on this board
is on the SD card driven by this controller, and there is no eMMC, so
a tuning regression means it does not boot.

Measured RX tuning windows, Milk-V Jupiter (K1), SDR104 SD card,
across three boots (the vendor driver already logs these):

  mmc0 (SD, rootfs):
    boot 0:  [0,55) [77,255)         -> widest 178
    boot -1: [0,54) [77,255)         -> widest 178
    boot -2: [0,50) [71,76) [79,255) -> widest 176

  mmc1 (SDIO):
    boot 0:  [0,73) [81,106) [137,255) -> widest 118
    boot -1: [0,74) [81,106) [107,108) -> widest 74
    boot -2: [0,76) [82,100)           -> widest 76

So a threshold of 50 is comfortable here. It also supports your
rationale directly: boot -2 produced a 5-code window and boot -1
produced a 1-code window on mmc1, so the narrow-window case this
patch guards against does occur in practice.

Relevant to the delay-line question in 5/7: this board's DT already
sets spacemit,rx_dline_reg = 0, so the windows above are already at
the finest step size, i.e. they should be comparable to post-5/7
behaviour rather than to the current mainline default of 9.

My question is about the form rather than the value. The vendor
driver takes this same limit from DT, per host:

  sdh@d4280000: spacemit,rx_tuning_limit = <0x32>;  /* 50 */
  sdh@d4280800: spacemit,rx_tuning_limit = <0x32>;  /* 50 */

So 50 matches what SpacemiT already ships -- but there it is a
per-controller DT property, and this patch turns it into a global
compile-time constant. Was that deliberate? The vendor design implies
the value is expected to need per-board adjustment, and with a Fixes:
tag this will land in stable, where a board with a narrower window
would go from "adjust the DT" to "patch and rebuild the kernel".

Two options, if you think the concern is real: keep it as a DT
property (matching the existing binding), or keep the constant as a
default that DT can override.

One more thing on 5/7 and 6/7: since patch 5 changes the delay-line
step from 9 to 0, the same physical timing window spans a different
number of delay codes with and without it. If 50 is calibrated against
the finest step, then backporting 6/7 without 5/7 could reject
configurations that currently work. Both carry Fixes: tags pointing at
e9cb83c10071, so they may well be picked up separately -- might be
worth making the dependency explicit for the stable maintainers.

Thanks,
Hung-Chun Tseng <alan.tseng.cs@gmail.com>