[PATCH 0/4] mmc: spacemit: Introduce two DT properties

[PATCH 0/4] mmc: spacemit: Introduce two DT properties

[Yixun Lan]

In this series, we try to introduce two DT properties for SpacemiT
SDHCI driver, A clock-frequency property to configure the rate
of io clock, which needed to configure for SDIO mode. The tx delay
property is used to configure the step count of TX delay line,
which is needed due to varous hardware layout has different
tuning settings.

Signed-off-by: Yixun Lan <dlan@kernel.org>
---
Long Wan (4):
      dt-bindings: mmc: spacemit-sdhci: Add clock-frequency property
      dt-bindings: mmc: spacemit,sdhci: Add tx delay property
      mmc: sdhci-of-k1: Set clock-frequency of io clock
      mmc: sdhci-of-k1: Add TX tuning parameter

 .../devicetree/bindings/mmc/spacemit,sdhci.yaml    | 17 ++++++++++++
 drivers/mmc/host/sdhci-of-k1.c                     | 30 ++++++++++++++++++++--
 2 files changed, 45 insertions(+), 2 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260901-07-k3-sdhci-dts-579b17b1e205

Best regards,
--  
Yixun Lan <dlan@kernel.org>

[PATCH 1/4] dt-bindings: mmc: spacemit-sdhci: Add clock-frequency property

[Yixun Lan]

From: Long Wan <long.wan@linux.spacemit.com>

Add a clock-frequency property to set the rate of io clock, according
to various host controller mode it require different initial clock
frequency, and also match to the board's layout actually validated at.

Signed-off-by: Long Wan <long.wan@linux.spacemit.com>
Signed-off-by: Yixun Lan <dlan@kernel.org>
---
 Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
index 34d202af909f..f35bf7ce08dd 100644
--- a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
@@ -29,6 +29,13 @@ properties:
       - description: core clock, used by internal controller
       - description: io clock, output for SD, SDIO, eMMC device
 
+  clock-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Rate of the io clock in Hz. When present the io clock is programmed to
+      this rate, allowing a board to select the rate its layout was validated
+      at.
+
   clock-names:
     items:
       - const: core

-- 
2.55.0

[PATCH 2/4] dt-bindings: mmc: spacemit,sdhci: Add tx delay property

[Yixun Lan]

From: Long Wan <long.wan@linux.spacemit.com>

Introduce a vendor specific property to configure the step count of TX
delay line which should be applied before RX tuning, compensating for
board level trace differences. The TX delay line result comes from test
out of real board which vary from different hardware layout.

Signed-off-by: Long Wan <long.wan@linux.spacemit.com>
Signed-off-by: Yixun Lan <dlan@kernel.org>
---
 Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
index f35bf7ce08dd..3399a2d076dc 100644
--- a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
@@ -63,6 +63,16 @@ properties:
   pinctrl-1:
     description: Optional pinctrl state for 1.8V UHS operation with "uhs" name
 
+  spacemit,tx-delay:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 255
+    description:
+      TX delay line setting applied before RX tuning, compensating for
+      board level trace differences. This is a delay line step count with
+      no fixed time-per-step conversion; determined empirically per board
+      specific, used on SD/SDIO hosts.
+
 required:
   - compatible
   - reg

-- 
2.55.0

[PATCH 3/4] mmc: sdhci-of-k1: Set clock-frequency of io clock

[Yixun Lan]

From: Long Wan <long.wan@linux.spacemit.com>

Add a clock-frequency property to set the rate of io clock, for SDIO
case, a clock frequency need to propagate down early.

Signed-off-by: Long Wan <long.wan@linux.spacemit.com>
Signed-off-by: Yixun Lan <dlan@kernel.org>
---
 drivers/mmc/host/sdhci-of-k1.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
index 37b0911e7cf2..e6ab40c9a555 100644
--- a/drivers/mmc/host/sdhci-of-k1.c
+++ b/drivers/mmc/host/sdhci-of-k1.c
@@ -438,6 +438,8 @@ static inline int spacemit_sdhci_get_clocks(struct device *dev,
 					    struct sdhci_pltfm_host *pltfm_host)
 {
 	struct spacemit_sdhci_host *sdhst = sdhci_pltfm_priv(pltfm_host);
+	struct device_node *np = dev->of_node;
+	u32 freq;
 
 	sdhst->clk_core = devm_clk_get_enabled(dev, "core");
 	if (IS_ERR(sdhst->clk_core))
@@ -447,6 +449,9 @@ static inline int spacemit_sdhci_get_clocks(struct device *dev,
 	if (IS_ERR(sdhst->clk_io))
 		return -EINVAL;
 
+	if (!of_property_read_u32(np, "clock-frequency", &freq))
+		clk_set_rate(sdhst->clk_io, freq);
+
 	pltfm_host->clk = sdhst->clk_io;
 
 	return 0;

-- 
2.55.0

[PATCH 4/4] mmc: sdhci-of-k1: Add TX tuning parameter

[Yixun Lan]

From: Long Wan <long.wan@linux.spacemit.com>

Instead of setting the TX delay code via hardcoded parameter, configuring
it via device tree property, which make it able to adapt according to
board specific tuning requirement.

Signed-off-by: Long Wan <long.wan@linux.spacemit.com>
Signed-off-by: Yixun Lan <dlan@kernel.org>
---
 drivers/mmc/host/sdhci-of-k1.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
index e6ab40c9a555..6cab06332747 100644
--- a/drivers/mmc/host/sdhci-of-k1.c
+++ b/drivers/mmc/host/sdhci-of-k1.c
@@ -97,6 +97,7 @@ struct spacemit_sdhci_host {
 	struct pinctrl *pinctrl;
 	struct pinctrl_state *pinctrl_default;
 	struct pinctrl_state *pinctrl_uhs;
+	u8 tx_delaycode;
 };
 
 /* All helper functions will update clr/set while preserve rest bits */
@@ -263,6 +264,7 @@ static int spacemit_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 	int max_pass_len = 0, max_pass_start = 0;
 	struct mmc_host *mmc = host->mmc;
 	struct mmc_ios ios = mmc->ios;
+	struct spacemit_sdhci_host *sdhst = sdhci_pltfm_priv(sdhci_priv(host));
 	u8 final_delay;
 	int ret = 0;
 	int i;
@@ -279,11 +281,11 @@ static int spacemit_sdhci_execute_tuning(struct sdhci_host *host, u32 opcode)
 
 	if (mmc->caps2 & MMC_CAP2_NO_MMC) {
 		spacemit_sdhci_set_tx_dline_reg(host, SPACEMIT_TX_TUNING_DLINE_REG);
-		spacemit_sdhci_set_tx_delay(host, SPACEMIT_TX_TUNING_DELAYCODE);
+		spacemit_sdhci_set_tx_delay(host, sdhst->tx_delaycode);
 		spacemit_sdhci_tx_tuning_prepare(host);
 
 		dev_dbg(mmc_dev(host->mmc), "TX tuning: dline_reg=%d, delaycode=%d\n",
-			SPACEMIT_TX_TUNING_DLINE_REG, SPACEMIT_TX_TUNING_DELAYCODE);
+			SPACEMIT_TX_TUNING_DLINE_REG, sdhst->tx_delaycode);
 	}
 
 	spacemit_sdhci_prepare_tuning(host);
@@ -496,6 +498,23 @@ static inline void spacemit_sdhci_get_pins(struct device *dev,
 		sdhst->pinctrl_default, sdhst->pinctrl_uhs);
 }
 
+static void spacemit_sdhci_get_tuning_params(struct device *dev,
+					     struct sdhci_pltfm_host *pltfm_host)
+{
+	struct spacemit_sdhci_host *sdhst = sdhci_pltfm_priv(pltfm_host);
+	u32 val;
+
+	sdhst->tx_delaycode = SPACEMIT_TX_TUNING_DELAYCODE;
+
+	/* Override from DT property */
+	if (!device_property_read_u32(dev, "spacemit,tx-delay", &val)) {
+		if (val <= U8_MAX)
+			sdhst->tx_delaycode = val;
+		else
+			dev_warn(dev, "ignoring invalid tx delay: %u\n", val);
+	}
+}
+
 static const struct sdhci_ops spacemit_sdhci_ops = {
 	.get_max_clock		= spacemit_sdhci_clk_get_max_clock,
 	.reset			= spacemit_sdhci_reset,
@@ -571,6 +590,8 @@ static int spacemit_sdhci_probe(struct platform_device *pdev)
 
 	spacemit_sdhci_get_pins(dev, pltfm_host);
 
+	spacemit_sdhci_get_tuning_params(dev, pltfm_host);
+
 	host->mmc_host_ops.start_signal_voltage_switch = spacemit_sdhci_start_signal_voltage_switch;
 
 	ret = spacemit_sdhci_get_clocks(dev, pltfm_host);

-- 
2.55.0

Re: [PATCH 1/4] dt-bindings: mmc: spacemit-sdhci: Add clock-frequency property

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Introduction of a redundant custom clock configuration property, bypassing standard Device Tree mechanisms.
--

commit 1745887406b566692891c256f7b9ab7126926f90
Author: Long Wan <long.wan@linux.spacemit.com>

dt-bindings: mmc: spacemit-sdhci: Add clock-frequency property

This commit introduces a custom clock-frequency property to the
spacemit-sdhci device tree binding. The property is intended to define the
initial rate of the IO clock based on board layout validation.

> diff --git a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
> index 34d202af909f2..f35bf7ce08dd8 100644
> --- a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
> @@ -29,6 +29,13 @@ properties:
>        - description: core clock, used by internal controller
>        - description: io clock, output for SD, SDIO, eMMC device
>  
> +  clock-frequency:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Rate of the io clock in Hz. When present the io clock is programmed to
> +      this rate, allowing a board to select the rate its layout was validated
> +      at.

[Severity: Medium]
Does this property reinvent standard clock configuration mechanisms?

In modern Device Tree design, setting the initial rate of a clock provider
consumed by a device is typically achieved using the standard assigned-clocks
and assigned-clock-rates properties, which the core clock framework processes
automatically.

Alternatively, if the goal is to cap the bus frequency for signal integrity,
the standard MMC max-frequency property should be used. Repurposing
clock-frequency to manually parse and call clk_set_rate() creates a
non-standard ABI for clock configuration and duplicates core functionality.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-07-k3-sdhci-dts-v1-0-4e7909cf4233@kernel.org?part=1

Re: [PATCH 3/4] mmc: sdhci-of-k1: Set clock-frequency of io clock

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Missing check for `clk_set_rate()` return value, masking configuration failures.

Pre-existing issues:
- [High] Breaking probe deferral by obscuring `devm_clk_get_enabled()` error codes with `-EINVAL`.
- [High] Probe deferral is broken for pinctrl due to unconditionally ignoring all errors.
--

commit 9748f01c4f206e03365b12c3c2ceed5bed26c9c0
Author: Long Wan <long.wan@linux.spacemit.com>

mmc: sdhci-of-k1: Set clock-frequency of io clock

Add a clock-frequency property to set the rate of io clock, for SDIO
case, a clock frequency need to propagate down early.

> diff --git a/drivers/mmc/host/sdhci-of-k1.c b/drivers/mmc/host/sdhci-of-k1.c
> index 37b0911e7cf20..e6ab40c9a5551 100644
> --- a/drivers/mmc/host/sdhci-of-k1.c
> +++ b/drivers/mmc/host/sdhci-of-k1.c
> @@ -438,6 +438,8 @@ static inline int spacemit_sdhci_get_clocks(struct device *dev,
>  					    struct sdhci_pltfm_host *pltfm_host)
>  {
>  	struct spacemit_sdhci_host *sdhst = sdhci_pltfm_priv(pltfm_host);
> +	struct device_node *np = dev->of_node;
> +	u32 freq;
>  
>  	sdhst->clk_core = devm_clk_get_enabled(dev, "core");
>  	if (IS_ERR(sdhst->clk_core))
>  		return -EINVAL;

[Severity: High]
This isn't a bug introduced by this patch, but does returning -EINVAL here
break probe deferral? If devm_clk_get_enabled() in spacemit_sdhci_get_clocks()
(drivers/mmc/host/sdhci-of-k1.c) returns -EPROBE_DEFER, returning -EINVAL will
cause the probe to fail permanently. Should this propagate
PTR_ERR(sdhst->clk_core) instead?

>  
>  	sdhst->clk_io = devm_clk_get_enabled(dev, "io");
>  	if (IS_ERR(sdhst->clk_io))
>  		return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does this also break probe deferral for the
same reason? Should we propagate PTR_ERR(sdhst->clk_io) in
spacemit_sdhci_get_clocks() instead of returning -EINVAL?

>  
> +	if (!of_property_read_u32(np, "clock-frequency", &freq))
> +		clk_set_rate(sdhst->clk_io, freq);

[Severity: Medium]
Does this code silently ignore clock configuration failures? If clk_set_rate()
in spacemit_sdhci_get_clocks() (drivers/mmc/host/sdhci-of-k1.c) fails and
returns an error code (for instance, if the frequency is unsupported or
rejected by CLK_SET_RATE_GATE), the failure is masked. Should we check the
return value and handle potential configuration failures?

> +
>  	pltfm_host->clk = sdhst->clk_io;
>  
>  	return 0;

While reviewing the file, I noticed another pre-existing issue in
spacemit_sdhci_get_pins():

drivers/mmc/host/sdhci-of-k1.c:spacemit_sdhci_get_pins() {
    [ ... ]
	sdhst->pinctrl = devm_pinctrl_get(dev);
	if (IS_ERR(sdhst->pinctrl)) {
		sdhst->pinctrl = NULL;
		dev_dbg(dev, "pinctrl not available, voltage switching will work without it\n");
		return;
	}
    [ ... ]
}

[Severity: High]
This is a pre-existing issue, but does this code break probe deferral for
pinctrl by unconditionally ignoring all errors? If devm_pinctrl_get() in
spacemit_sdhci_get_pins() (drivers/mmc/host/sdhci-of-k1.c) returns
-EPROBE_DEFER, the driver will treat it as if no pin controller is present.
Should we check for and propagate -EPROBE_DEFER?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902-07-k3-sdhci-dts-v1-0-4e7909cf4233@kernel.org?part=3

Re: [PATCH 1/4] dt-bindings: mmc: spacemit-sdhci: Add clock-frequency property

[Yixun Lan]

Hi All,

On 07:11 Wed 02 Sep     , sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Introduction of a redundant custom clock configuration property, bypassing standard Device Tree mechanisms.
> --
> 
> commit 1745887406b566692891c256f7b9ab7126926f90
> Author: Long Wan <long.wan@linux.spacemit.com>
> 
> dt-bindings: mmc: spacemit-sdhci: Add clock-frequency property
> 
> This commit introduces a custom clock-frequency property to the
> spacemit-sdhci device tree binding. The property is intended to define the
> initial rate of the IO clock based on board layout validation.
> 
> > diff --git a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
> > index 34d202af909f2..f35bf7ce08dd8 100644
> > --- a/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
> > +++ b/Documentation/devicetree/bindings/mmc/spacemit,sdhci.yaml
> > @@ -29,6 +29,13 @@ properties:
> >        - description: core clock, used by internal controller
> >        - description: io clock, output for SD, SDIO, eMMC device
> >  
> > +  clock-frequency:
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +    description:
> > +      Rate of the io clock in Hz. When present the io clock is programmed to
> > +      this rate, allowing a board to select the rate its layout was validated
> > +      at.
> 
> [Severity: Medium]
> Does this property reinvent standard clock configuration mechanisms?
> 
> In modern Device Tree design, setting the initial rate of a clock provider
> consumed by a device is typically achieved using the standard assigned-clocks
> and assigned-clock-rates properties, which the core clock framework processes
> automatically.
> 
Ok, using assigned-clock-rates works here, will just drop this patch

-- 
Yixun Lan (dlan)