U-Boot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1
@ 2026-08-14 18:30 Ralph Siemsen
  2026-08-14 22:21 ` Marek Vasut via U-Boot
  0 siblings, 1 reply; 6+ messages in thread
From: Ralph Siemsen @ 2026-08-14 18:30 UTC (permalink / raw)
  To: u-boot
  Cc: Peng Fan, Jaehoon Chung, Tom Rini, Marek Vasut, Michal Simek,
	Bastien Curutchet, Anshul Dalal, Johan Jonker, Ralph Siemsen

The Renesas RZ/N1 SoC contains a MMC controller using the Arasan IP.
It is supported by the existing driver, with the following changes:

1) Multiple clocks need to be enabled. This is handled using
   clk_get_bulk() and clock_enable_bulk().

2) It is necessary to defer the sdhci_probe(), similar to what is
   done in drivers/mmc/am654_sdhci.c

Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
---
Changes in v2:
- split from the series "Renesas RZ/N1 additional drivers"
  https://lore.kernel.org/u-boot/20260731-rzn1-2026-07-v1-8-af2ce80db9d8@linaro.org/
- fix typo in commit message
---
 drivers/mmc/Kconfig      |  2 +-
 drivers/mmc/zynq_sdhci.c | 39 ++++++++++++++++++++++++++++++++-------
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index f9f7aa5cf97..7649fc8f53f 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -778,7 +778,7 @@ config TEGRA124_MMC_DISABLE_EXT_LOOPBACK
 config MMC_SDHCI_ZYNQ
 	bool "Arasan SDHCI controller support"
 	depends on OF_CONTROL
-	depends on ARCH_VERSAL || ARCH_VERSAL_NET || ARCH_VERSAL2 || ARCH_ZYNQ || ARCH_ZYNQMP
+	depends on ARCH_RENESAS || ARCH_VERSAL || ARCH_VERSAL_NET || ARCH_VERSAL2 || ARCH_ZYNQ || ARCH_ZYNQMP
 	depends on MMC_SDHCI
 	help
 	  Support for Arasan SDHCI host controller on Zynq/ZynqMP ARM SoCs platform
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
index eda95b72f49..351e7d56569 100644
--- a/drivers/mmc/zynq_sdhci.c
+++ b/drivers/mmc/zynq_sdhci.c
@@ -1012,6 +1012,31 @@ static const struct sdhci_ops arasan_ops = {
 	.config_dll = &arasan_sdhci_config_dll,
 #endif
 };
+
+#elif defined(CONFIG_ARCH_RENESAS)
+/*
+ * It is unknown whether the IP in RZ/N1 supports these clock phase
+ * adjustments. The devicetree does not include these properties.
+ * So this is a no-op for now.
+ */
+static void arasan_dt_parse_clk_phases(struct udevice *dev)
+{
+}
+
+/*
+ * Renesas RZ/N1 needs deferred sdhci_probe(), otherwise the
+ * first CMD_00 fails with "Timeout for status update"
+ */
+static int arasan_sdhci_deferred_probe(struct sdhci_host *host)
+{
+	struct udevice *dev = host->mmc->dev;
+
+	return sdhci_probe(dev);
+}
+
+static const struct sdhci_ops arasan_ops = {
+	.deferred_probe = arasan_sdhci_deferred_probe,
+};
 #endif
 
 #if defined(CONFIG_ARCH_ZYNQMP) && defined(CONFIG_ZYNQMP_FIRMWARE)
@@ -1110,7 +1135,7 @@ static int arasan_sdhci_probe(struct udevice *dev)
 	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
 	struct arasan_sdhci_priv *priv = dev_get_priv(dev);
 	struct sdhci_host *host;
-	struct clk clk;
+	struct clk_bulk clks;
 	unsigned long clock;
 	int ret;
 
@@ -1152,13 +1177,13 @@ static int arasan_sdhci_probe(struct udevice *dev)
 		}
 	}
 
-	ret = clk_get_by_index(dev, 0, &clk);
+	ret = clk_get_bulk(dev, &clks);
 	if (ret < 0) {
-		dev_err(dev, "failed to get clock\n");
+		dev_err(dev, "failed to get clock(s)\n");
 		return ret;
 	}
 
-	clock = clk_get_rate(&clk);
+	clock = clk_get_rate(&clks.clks[0]);
 	if (IS_ERR_VALUE(clock)) {
 		dev_err(dev, "failed to get rate\n");
 		return clock;
@@ -1166,9 +1191,9 @@ static int arasan_sdhci_probe(struct udevice *dev)
 
 	dev_dbg(dev, "%s: CLK %ld\n", __func__, clock);
 
-	ret = clk_enable(&clk);
+	ret = clk_enable_bulk(&clks);
 	if (ret) {
-		dev_err(dev, "failed to enable clock\n");
+		dev_err(dev, "failed to enable clock(s)\n");
 		return ret;
 	}
 
@@ -1239,7 +1264,7 @@ static int arasan_sdhci_of_to_plat(struct udevice *dev)
 	priv->host->name = dev->name;
 
 #if defined(CONFIG_ARCH_ZYNQMP) || defined(CONFIG_ARCH_VERSAL) || defined(CONFIG_ARCH_VERSAL_NET) || \
-    defined(CONFIG_ARCH_VERSAL2)
+    defined(CONFIG_ARCH_VERSAL2) || defined(CONFIG_ARCH_RENESAS)
 	priv->host->ops = &arasan_ops;
 	arasan_dt_parse_clk_phases(dev);
 #endif

---
base-commit: 36c377b9859ffb53eb1e39ea31e8d96d1e0fe1e5
change-id: 20260811-rzn1-2026-10-mmc-91cd47670763

Best regards,
--  
Ralph Siemsen <ralph.siemsen@linaro.org>


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1
  2026-08-14 18:30 [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1 Ralph Siemsen
@ 2026-08-14 22:21 ` Marek Vasut via U-Boot
  2026-08-25 19:47   ` Ralph Siemsen
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut via U-Boot @ 2026-08-14 22:21 UTC (permalink / raw)
  To: Ralph Siemsen, u-boot
  Cc: Peng Fan, Jaehoon Chung, Tom Rini, Marek Vasut, Michal Simek,
	Bastien Curutchet, Anshul Dalal, Johan Jonker

On 8/14/26 8:30 PM, Ralph Siemsen wrote:

[...]

> +++ b/drivers/mmc/zynq_sdhci.c

[...]

> @@ -1152,13 +1177,13 @@ static int arasan_sdhci_probe(struct udevice *dev)
>   		}
>   	}
>   
> -	ret = clk_get_by_index(dev, 0, &clk);
> +	ret = clk_get_bulk(dev, &clks);
>   	if (ret < 0) {
> -		dev_err(dev, "failed to get clock\n");
> +		dev_err(dev, "failed to get clock(s)\n");


"failed to get clock(s) (%d)\n", ret);

Please fix globally.

>   		return ret;
>   	}
>   
> -	clock = clk_get_rate(&clk);
> +	clock = clk_get_rate(&clks.clks[0]);

Can the clock ever be non-first clock , i.e. do you need to handle 
clock-names DT property somehow ?

>   	if (IS_ERR_VALUE(clock)) {
>   		dev_err(dev, "failed to get rate\n");
>   		return clock;
[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1
  2026-08-14 22:21 ` Marek Vasut via U-Boot
@ 2026-08-25 19:47   ` Ralph Siemsen
  2026-08-25 20:44     ` Marek Vasut
  2026-08-27 14:12     ` Ralph Siemsen
  0 siblings, 2 replies; 6+ messages in thread
From: Ralph Siemsen @ 2026-08-25 19:47 UTC (permalink / raw)
  To: Marek Vasut
  Cc: u-boot, Peng Fan, Jaehoon Chung, Tom Rini, Marek Vasut,
	Michal Simek, Bastien Curutchet, Anshul Dalal, Johan Jonker

On Sat, Aug 15, 2026 at 12:21:11AM +0200, Marek Vasut wrote:
>On 8/14/26 8:30 PM, Ralph Siemsen wrote:
>
>[...]
>
>>+++ b/drivers/mmc/zynq_sdhci.c
>
>[...]
>
>>@@ -1152,13 +1177,13 @@ static int arasan_sdhci_probe(struct udevice *dev)
>>  		}
>>  	}
>>-	ret = clk_get_by_index(dev, 0, &clk);
>>+	ret = clk_get_bulk(dev, &clks);
>>  	if (ret < 0) {
>>-		dev_err(dev, "failed to get clock\n");
>>+		dev_err(dev, "failed to get clock(s)\n");
>
>"failed to get clock(s) (%d)\n", ret);
>
>Please fix globally.

Will do in v3. I'll just do the clock-related debug prints, since I am 
modifying them already.

There are other (non-clock) debug prints that could benefit from same 
treatment, but that would be better done as a separate cleanup patch.

>>  		return ret;
>>  	}
>>-	clock = clk_get_rate(&clk);
>>+	clock = clk_get_rate(&clks.clks[0]);
>
>Can the clock ever be non-first clock , i.e. do you need to handle 
>clock-names DT property somehow ?

It's not clear to me if the DT binding specifies the order. It says 
there must be either 2 or 3 clocks, as well as clock-names properties.

Existing DTS files (rzn1 and zynqmp) have it as the first clock.

We could try using the name ("clk_xin") and if that fails, fall back on 
using the first clock.

Regards,
Ralph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1
  2026-08-25 19:47   ` Ralph Siemsen
@ 2026-08-25 20:44     ` Marek Vasut
  2026-08-27 14:12     ` Ralph Siemsen
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2026-08-25 20:44 UTC (permalink / raw)
  To: Ralph Siemsen, Michal Simek
  Cc: u-boot, Peng Fan, Jaehoon Chung, Tom Rini, Marek Vasut,
	Bastien Curutchet, Anshul Dalal, Johan Jonker

On 8/25/26 9:47 PM, Ralph Siemsen wrote:

Hi,

>>>          return ret;
>>>      }
>>> -    clock = clk_get_rate(&clk);
>>> +    clock = clk_get_rate(&clks.clks[0]);
>>
>> Can the clock ever be non-first clock , i.e. do you need to handle 
>> clock-names DT property somehow ?
> 
> It's not clear to me if the DT binding specifies the order. It says 
> there must be either 2 or 3 clocks, as well as clock-names properties.
> 
> Existing DTS files (rzn1 and zynqmp) have it as the first clock.
> 
> We could try using the name ("clk_xin") and if that fails, fall back on 
> using the first clock.
Sounds good.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1
  2026-08-25 19:47   ` Ralph Siemsen
  2026-08-25 20:44     ` Marek Vasut
@ 2026-08-27 14:12     ` Ralph Siemsen
  2026-08-27 16:53       ` Marek Vasut
  1 sibling, 1 reply; 6+ messages in thread
From: Ralph Siemsen @ 2026-08-27 14:12 UTC (permalink / raw)
  To: Marek Vasut
  Cc: u-boot, Peng Fan, Jaehoon Chung, Tom Rini, Marek Vasut,
	Michal Simek, Bastien Curutchet, Anshul Dalal, Johan Jonker

On Tue, Aug 25, 2026 at 03:47:41PM -0400, Ralph Siemsen wrote:
>On Sat, Aug 15, 2026 at 12:21:11AM +0200, Marek Vasut wrote:
>>On 8/14/26 8:30 PM, Ralph Siemsen wrote:
>>> 		return ret;
>>> 	}
>>>-	clock = clk_get_rate(&clk);
>>>+	clock = clk_get_rate(&clks.clks[0]);
>>
>>Can the clock ever be non-first clock , i.e. do you need to handle 
>>clock-names DT property somehow ?
>
>It's not clear to me if the DT binding specifies the order. It says 
>there must be either 2 or 3 clocks, as well as clock-names properties.

I had another look at the clock ordering. According to [1]
  "Order in lists is significant, order in dicts is not."

The binding [2] uses a list for the clock names:

   clock-names:
     minItems: 2
     items:
       - const: clk_xin
       - const: clk_ahb
       - const: gate

And as we can see, the reference clock ("clk_xin") is listed first.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/example-schema.yaml#n47

[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml#n116

As a further check, I reversed the order of the clocks, and then ran 
"make dtbs_check" to verify against DT schema. It complains:

arch/arm/dts/r9a06g032-rzn1-snarc.dtb: mmc@40100000 (renesas,r9a06g032-sdhci): clock-names:0: 'clk_xin' was expected
	from schema $id: http://devicetree.org/schemas/mmc/arasan,sdhci.yaml
arch/arm/dts/r9a06g032-rzn1-snarc.dtb: mmc@40100000 (renesas,r9a06g032-sdhci): clock-names:1: 'clk_ahb' was expected
	from schema $id: http://devicetree.org/schemas/mmc/arasan,sdhci.yaml

No complaint when the clocks are in the correct order.

>We could try using the name ("clk_xin") and if that fails, fall back on 
>using the first clock.

So I think we don't need this extra complexity after all. It should be 
fine to rely on the first clock being the correct one.

Ralph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1
  2026-08-27 14:12     ` Ralph Siemsen
@ 2026-08-27 16:53       ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2026-08-27 16:53 UTC (permalink / raw)
  To: Ralph Siemsen
  Cc: u-boot, Peng Fan, Jaehoon Chung, Tom Rini, Marek Vasut,
	Michal Simek, Bastien Curutchet, Anshul Dalal, Johan Jonker

On 8/27/26 4:12 PM, Ralph Siemsen wrote:
> On Tue, Aug 25, 2026 at 03:47:41PM -0400, Ralph Siemsen wrote:
>> On Sat, Aug 15, 2026 at 12:21:11AM +0200, Marek Vasut wrote:
>>> On 8/14/26 8:30 PM, Ralph Siemsen wrote:
>>>>         return ret;
>>>>     }
>>>> -    clock = clk_get_rate(&clk);
>>>> +    clock = clk_get_rate(&clks.clks[0]);
>>>
>>> Can the clock ever be non-first clock , i.e. do you need to handle 
>>> clock-names DT property somehow ?
>>
>> It's not clear to me if the DT binding specifies the order. It says 
>> there must be either 2 or 3 clocks, as well as clock-names properties.
> 
> I had another look at the clock ordering. According to [1]
>   "Order in lists is significant, order in dicts is not."
> 
> The binding [2] uses a list for the clock names:
> 
>    clock-names:
>      minItems: 2
>      items:
>        - const: clk_xin
>        - const: clk_ahb
>        - const: gate
> 
> And as we can see, the reference clock ("clk_xin") is listed first.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ 
> tree/Documentation/devicetree/bindings/example-schema.yaml#n47
> 
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ 
> tree/Documentation/devicetree/bindings/mmc/arasan,sdhci.yaml#n116
> 
> As a further check, I reversed the order of the clocks, and then ran 
> "make dtbs_check" to verify against DT schema. It complains:
> 
> arch/arm/dts/r9a06g032-rzn1-snarc.dtb: mmc@40100000 (renesas,r9a06g032- 
> sdhci): clock-names:0: 'clk_xin' was expected
>      from schema $id: http://devicetree.org/schemas/mmc/arasan,sdhci.yaml
> arch/arm/dts/r9a06g032-rzn1-snarc.dtb: mmc@40100000 (renesas,r9a06g032- 
> sdhci): clock-names:1: 'clk_ahb' was expected
>      from schema $id: http://devicetree.org/schemas/mmc/arasan,sdhci.yaml
> 
> No complaint when the clocks are in the correct order.
> 
>> We could try using the name ("clk_xin") and if that fails, fall back 
>> on using the first clock.
> 
> So I think we don't need this extra complexity after all. It should be 
> fine to rely on the first clock being the correct one.
I'd still go for the safer option -- look up by name, and if it fails, 
fall back to look up by ordering.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-27 17:01 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 18:30 [PATCH v2] mmc: zynq_sdhci: support Renesas RZ/N1 Ralph Siemsen
2026-08-14 22:21 ` Marek Vasut via U-Boot
2026-08-25 19:47   ` Ralph Siemsen
2026-08-25 20:44     ` Marek Vasut
2026-08-27 14:12     ` Ralph Siemsen
2026-08-27 16:53       ` Marek Vasut

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox