Re: [PATCH 1/3] mmc: k1: handle a missing max-frequency and clk_set_rate()'s return value

[Kathpalia, Tanmay]

On 03-10-2026 02:59, Dave Klotz wrote:

The driver sets the io clock to the max-frequency the device tree gives
and fails the probe on any non-zero return from clk_set_rate(). Two
problems follow:

- clk_set_rate() returns the new rate on success and a negative error
   code on failure. The SpacemiT clock driver happens to return 0, but
   any clock driver that reports the rate makes the probe fail.
- A node without max-frequency asks for 0 Hz. On the K1 clock driver
   that picks no divider at all and writes whatever the calculation left
   behind into the io clock's divider field; host->max_clk is then 0,
   the capabilities register gives no base clock either, and the probe
   fails in sdhci_setup_cfg().

Treat only a negative return as an error. Without max-frequency, leave
the io clock as an earlier stage set it and run from its current rate.

Signed-off-by: Dave Klotz<davewk...@protonmail.com>
---
  drivers/mmc/spacemit_sdhci.c | 26 ++++++++++++++++++++++----
  1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/spacemit_sdhci.c b/drivers/mmc/spacemit_sdhci.c
index f5578f38bd..bbd49b76c6 100644
--- a/drivers/mmc/spacemit_sdhci.c
+++ b/drivers/mmc/spacemit_sdhci.c
@@ -606,10 +606,28 @@ static int spacemit_sdhci_probe(struct udevice *dev)
                goto err_reset;
        }

- ret = clk_set_rate(&clk, plat->cfg.f_max);

-       if (ret) {
-               log_err("Failed to set io clk: %d\n", ret);
-               goto err_reset;
+       /*
+        * Without max-frequency there is no rate to ask for: leave the io
+        * clock as an earlier stage set it and run from its current rate. The
+        * capabilities register gives no base clock on the K1, so it cannot
+        * stand in. clk_set_rate() returns the new rate on success, so only a
+        * negative value is an error.
+        */
+       if (plat->cfg.f_max) {
+               long rate = clk_set_rate(&clk, plat->cfg.f_max);

The clock API returns ulong; please use ulong rate with
IS_ERR_VALUE(rate) for consistency.

+
+               if (rate < 0) {
+                       ret = rate;
+                       log_err("Failed to set io clk: %d\n", ret);
+                       goto err_reset;
+               }
+       } else {
+               plat->cfg.f_max = clk_get_rate(&clk);

clk_get_rate() returns ulong, but f_max is uint. On RV64, assigning a
negative error first truncates it, so IS_ERR_VALUE(f_max) no longer
detects the error. Please validate the ulong rate before assigning it:

    rate = clk_get_rate(&clk);
    if (IS_ERR_VALUE(rate) || !rate) {
        ret = rate ? (int)rate : -EINVAL;
        goto err_reset;
    }
    plat->cfg.f_max = rate;