Convert the Arasan SDHCI driver to use the mainline standard
mmc_clk_phase_map infrastructure instead of custom clk_phase_in/out
arrays as well as arasan_dt_read_clk_phase().

The phase values for ZynqMP, Versal, and Versal-NET platforms are
still initialized from the predefined tables, but now follow the
standard phase_map format with valid flag.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/sdhci-of-arasan.c | 79 +++++++++-----------------------------
 1 file changed, 18 insertions(+), 61 deletions(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index caf9723..b97d27c 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -152,8 +152,7 @@ struct sdhci_arasan_clk_ops {
  * @sdcardclk:		Pointer to normal 'struct clock' for sdcardclk_hw.
  * @sampleclk_hw:	Struct for the clock we might provide to a PHY.
  * @sampleclk:		Pointer to normal 'struct clock' for sampleclk_hw.
- * @clk_phase_in:	Array of Input Clock Phase Delays for all speed modes
- * @clk_phase_out:	Array of Output Clock Phase Delays for all speed modes
+ * @phase_map:		Struct for mmc_clk_phase_map provided.
  * @set_clk_delays:	Function pointer for setting Clock Delays
  * @clk_of_data:	Platform specific runtime clock data storage pointer
  */
@@ -162,8 +161,7 @@ struct sdhci_arasan_clk_data {
 	struct clk      *sdcardclk;
 	struct clk_hw	sampleclk_hw;
 	struct clk      *sampleclk;
-	int		clk_phase_in[MMC_TIMING_MMC_HS400 + 1];
-	int		clk_phase_out[MMC_TIMING_MMC_HS400 + 1];
+	struct mmc_clk_phase_map phase_map;
 	void		(*set_clk_delays)(struct sdhci_host *host);
 	void		*clk_of_data;
 };
@@ -1248,37 +1246,13 @@ static void sdhci_arasan_set_clk_delays(struct sdhci_host *host)
 	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
 	struct sdhci_arasan_clk_data *clk_data = &sdhci_arasan->clk_data;
 
-	clk_set_phase(clk_data->sampleclk,
-		      clk_data->clk_phase_in[host->timing]);
-	clk_set_phase(clk_data->sdcardclk,
-		      clk_data->clk_phase_out[host->timing]);
-}
-
-static void arasan_dt_read_clk_phase(struct device *dev,
-				     struct sdhci_arasan_clk_data *clk_data,
-				     unsigned int timing, const char *prop)
-{
-	struct device_node *np = dev->of_node;
-
-	u32 clk_phase[2] = {0};
-	int ret;
-
-	/*
-	 * Read Tap Delay values from DT, if the DT does not contain the
-	 * Tap Values then use the pre-defined values.
-	 */
-	ret = of_property_read_variable_u32_array(np, prop, &clk_phase[0],
-						  2, 0);
-	if (ret < 0) {
-		dev_dbg(dev, "Using predefined clock phase for %s = %d %d\n",
-			prop, clk_data->clk_phase_in[timing],
-			clk_data->clk_phase_out[timing]);
+	if (!clk_data->phase_map.phase[host->timing].valid)
 		return;
-	}
 
-	/* The values read are Input and Output Clock Delays in order */
-	clk_data->clk_phase_in[timing] = clk_phase[0];
-	clk_data->clk_phase_out[timing] = clk_phase[1];
+	clk_set_phase(clk_data->sampleclk,
+		      clk_data->phase_map.phase[host->timing].in_deg);
+	clk_set_phase(clk_data->sdcardclk,
+		      clk_data->phase_map.phase[host->timing].out_deg);
 }
 
 /**
@@ -1315,8 +1289,9 @@ static void arasan_dt_parse_clk_phases(struct device *dev,
 		}
 
 		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
-			clk_data->clk_phase_in[i] = zynqmp_iclk_phase[i];
-			clk_data->clk_phase_out[i] = zynqmp_oclk_phase[i];
+			clk_data->phase_map.phase[i].in_deg = zynqmp_iclk_phase[i];
+			clk_data->phase_map.phase[i].out_deg = zynqmp_oclk_phase[i];
+			clk_data->phase_map.phase[i].valid = true;
 		}
 	}
 
@@ -1327,8 +1302,9 @@ static void arasan_dt_parse_clk_phases(struct device *dev,
 			VERSAL_OCLK_PHASE;
 
 		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
-			clk_data->clk_phase_in[i] = versal_iclk_phase[i];
-			clk_data->clk_phase_out[i] = versal_oclk_phase[i];
+			clk_data->phase_map.phase[i].in_deg = versal_iclk_phase[i];
+			clk_data->phase_map.phase[i].out_deg = versal_oclk_phase[i];
+			clk_data->phase_map.phase[i].valid = true;
 		}
 	}
 	if (of_device_is_compatible(dev->of_node, "xlnx,versal-net-emmc")) {
@@ -1338,32 +1314,13 @@ static void arasan_dt_parse_clk_phases(struct device *dev,
 			VERSAL_NET_EMMC_OCLK_PHASE;
 
 		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
-			clk_data->clk_phase_in[i] = versal_net_iclk_phase[i];
-			clk_data->clk_phase_out[i] = versal_net_oclk_phase[i];
+			clk_data->phase_map.phase[i].in_deg = versal_net_iclk_phase[i];
+			clk_data->phase_map.phase[i].out_deg = versal_net_oclk_phase[i];
+			clk_data->phase_map.phase[i].valid = true;
 		}
 	}
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_LEGACY,
-				 "clk-phase-legacy");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS,
-				 "clk-phase-mmc-hs");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_SD_HS,
-				 "clk-phase-sd-hs");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR12,
-				 "clk-phase-uhs-sdr12");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR25,
-				 "clk-phase-uhs-sdr25");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR50,
-				 "clk-phase-uhs-sdr50");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR104,
-				 "clk-phase-uhs-sdr104");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_DDR50,
-				 "clk-phase-uhs-ddr50");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_DDR52,
-				 "clk-phase-mmc-ddr52");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS200,
-				 "clk-phase-mmc-hs200");
-	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS400,
-				 "clk-phase-mmc-hs400");
+
+	mmc_of_parse_clk_phase(dev, &clk_data->phase_map);
 }
 
 static const struct sdhci_pltfm_data sdhci_arasan_pdata = {
-- 
2.7.4

+Sai,

On 3/17/26 03:17, Shawn Lin wrote:
> Convert the Arasan SDHCI driver to use the mainline standard
> mmc_clk_phase_map infrastructure instead of custom clk_phase_in/out
> arrays as well as arasan_dt_read_clk_phase().
> 
> The phase values for ZynqMP, Versal, and Versal-NET platforms are
> still initialized from the predefined tables, but now follow the
> standard phase_map format with valid flag.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>   drivers/mmc/host/sdhci-of-arasan.c | 79 +++++++++-----------------------------
>   1 file changed, 18 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index caf9723..b97d27c 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -152,8 +152,7 @@ struct sdhci_arasan_clk_ops {
>    * @sdcardclk:		Pointer to normal 'struct clock' for sdcardclk_hw.
>    * @sampleclk_hw:	Struct for the clock we might provide to a PHY.
>    * @sampleclk:		Pointer to normal 'struct clock' for sampleclk_hw.
> - * @clk_phase_in:	Array of Input Clock Phase Delays for all speed modes
> - * @clk_phase_out:	Array of Output Clock Phase Delays for all speed modes
> + * @phase_map:		Struct for mmc_clk_phase_map provided.
>    * @set_clk_delays:	Function pointer for setting Clock Delays
>    * @clk_of_data:	Platform specific runtime clock data storage pointer
>    */
> @@ -162,8 +161,7 @@ struct sdhci_arasan_clk_data {
>   	struct clk      *sdcardclk;
>   	struct clk_hw	sampleclk_hw;
>   	struct clk      *sampleclk;
> -	int		clk_phase_in[MMC_TIMING_MMC_HS400 + 1];
> -	int		clk_phase_out[MMC_TIMING_MMC_HS400 + 1];
> +	struct mmc_clk_phase_map phase_map;
>   	void		(*set_clk_delays)(struct sdhci_host *host);
>   	void		*clk_of_data;
>   };
> @@ -1248,37 +1246,13 @@ static void sdhci_arasan_set_clk_delays(struct sdhci_host *host)
>   	struct sdhci_arasan_data *sdhci_arasan = sdhci_pltfm_priv(pltfm_host);
>   	struct sdhci_arasan_clk_data *clk_data = &sdhci_arasan->clk_data;
>   
> -	clk_set_phase(clk_data->sampleclk,
> -		      clk_data->clk_phase_in[host->timing]);
> -	clk_set_phase(clk_data->sdcardclk,
> -		      clk_data->clk_phase_out[host->timing]);
> -}
> -
> -static void arasan_dt_read_clk_phase(struct device *dev,
> -				     struct sdhci_arasan_clk_data *clk_data,
> -				     unsigned int timing, const char *prop)
> -{
> -	struct device_node *np = dev->of_node;
> -
> -	u32 clk_phase[2] = {0};
> -	int ret;
> -
> -	/*
> -	 * Read Tap Delay values from DT, if the DT does not contain the
> -	 * Tap Values then use the pre-defined values.
> -	 */
> -	ret = of_property_read_variable_u32_array(np, prop, &clk_phase[0],
> -						  2, 0);
> -	if (ret < 0) {
> -		dev_dbg(dev, "Using predefined clock phase for %s = %d %d\n",
> -			prop, clk_data->clk_phase_in[timing],
> -			clk_data->clk_phase_out[timing]);
> +	if (!clk_data->phase_map.phase[host->timing].valid)
>   		return;
> -	}
>   
> -	/* The values read are Input and Output Clock Delays in order */
> -	clk_data->clk_phase_in[timing] = clk_phase[0];
> -	clk_data->clk_phase_out[timing] = clk_phase[1];
> +	clk_set_phase(clk_data->sampleclk,
> +		      clk_data->phase_map.phase[host->timing].in_deg);
> +	clk_set_phase(clk_data->sdcardclk,
> +		      clk_data->phase_map.phase[host->timing].out_deg);
>   }
>   
>   /**
> @@ -1315,8 +1289,9 @@ static void arasan_dt_parse_clk_phases(struct device *dev,
>   		}
>   
>   		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
> -			clk_data->clk_phase_in[i] = zynqmp_iclk_phase[i];
> -			clk_data->clk_phase_out[i] = zynqmp_oclk_phase[i];
> +			clk_data->phase_map.phase[i].in_deg = zynqmp_iclk_phase[i];
> +			clk_data->phase_map.phase[i].out_deg = zynqmp_oclk_phase[i];
> +			clk_data->phase_map.phase[i].valid = true;
>   		}
>   	}
>   
> @@ -1327,8 +1302,9 @@ static void arasan_dt_parse_clk_phases(struct device *dev,
>   			VERSAL_OCLK_PHASE;
>   
>   		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
> -			clk_data->clk_phase_in[i] = versal_iclk_phase[i];
> -			clk_data->clk_phase_out[i] = versal_oclk_phase[i];
> +			clk_data->phase_map.phase[i].in_deg = versal_iclk_phase[i];
> +			clk_data->phase_map.phase[i].out_deg = versal_oclk_phase[i];
> +			clk_data->phase_map.phase[i].valid = true;
>   		}
>   	}
>   	if (of_device_is_compatible(dev->of_node, "xlnx,versal-net-emmc")) {
> @@ -1338,32 +1314,13 @@ static void arasan_dt_parse_clk_phases(struct device *dev,
>   			VERSAL_NET_EMMC_OCLK_PHASE;
>   
>   		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
> -			clk_data->clk_phase_in[i] = versal_net_iclk_phase[i];
> -			clk_data->clk_phase_out[i] = versal_net_oclk_phase[i];
> +			clk_data->phase_map.phase[i].in_deg = versal_net_iclk_phase[i];
> +			clk_data->phase_map.phase[i].out_deg = versal_net_oclk_phase[i];
> +			clk_data->phase_map.phase[i].valid = true;
>   		}
>   	}
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_LEGACY,
> -				 "clk-phase-legacy");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS,
> -				 "clk-phase-mmc-hs");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_SD_HS,
> -				 "clk-phase-sd-hs");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR12,
> -				 "clk-phase-uhs-sdr12");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR25,
> -				 "clk-phase-uhs-sdr25");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR50,
> -				 "clk-phase-uhs-sdr50");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR104,
> -				 "clk-phase-uhs-sdr104");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_DDR50,
> -				 "clk-phase-uhs-ddr50");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_DDR52,
> -				 "clk-phase-mmc-ddr52");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS200,
> -				 "clk-phase-mmc-hs200");
> -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS400,
> -				 "clk-phase-mmc-hs400");
> +
> +	mmc_of_parse_clk_phase(dev, &clk_data->phase_map);
>   }
>   
>   static const struct sdhci_pltfm_data sdhci_arasan_pdata = {

please take a look at this patch.
I was talking to him yesterday and he needs to finish two things before he has 
time to look at it. That's why please give him some time.

Thanks,
Michal

> -----Original Message-----
> From: Simek, Michal <michal.simek@amd.com>
> Sent: Thursday, March 19, 2026 3:39 PM
> To: Shawn Lin <shawn.lin@rock-chips.com>; Ulf Hansson
> <ulf.hansson@linaro.org>; Potthuri, Sai Krishna
> <sai.krishna.potthuri@amd.com>
> Cc: linux-mmc@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] mmc: sdhci-of-arasan: Use standard
> mmc_clk_phase_map infrastructure
> 
> +Sai,
> 
> On 3/17/26 03:17, Shawn Lin wrote:
> > Convert the Arasan SDHCI driver to use the mainline standard
> > mmc_clk_phase_map infrastructure instead of custom clk_phase_in/out
> > arrays as well as arasan_dt_read_clk_phase().
> >
> > The phase values for ZynqMP, Versal, and Versal-NET platforms are
> > still initialized from the predefined tables, but now follow the
> > standard phase_map format with valid flag.
> >
> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> > ---
> >
> >   drivers/mmc/host/sdhci-of-arasan.c | 79 +++++++++-----------------------------
> >   1 file changed, 18 insertions(+), 61 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-arasan.c
> > b/drivers/mmc/host/sdhci-of-arasan.c
> > index caf9723..b97d27c 100644
> > --- a/drivers/mmc/host/sdhci-of-arasan.c
> > +++ b/drivers/mmc/host/sdhci-of-arasan.c
> > @@ -152,8 +152,7 @@ struct sdhci_arasan_clk_ops {
> >    * @sdcardclk:		Pointer to normal 'struct clock' for
> sdcardclk_hw.
> >    * @sampleclk_hw:	Struct for the clock we might provide to a PHY.
> >    * @sampleclk:		Pointer to normal 'struct clock' for
> sampleclk_hw.
> > - * @clk_phase_in:	Array of Input Clock Phase Delays for all speed modes
> > - * @clk_phase_out:	Array of Output Clock Phase Delays for all speed
> modes
> > + * @phase_map:		Struct for mmc_clk_phase_map provided.
> >    * @set_clk_delays:	Function pointer for setting Clock Delays
> >    * @clk_of_data:	Platform specific runtime clock data storage pointer
> >    */
> > @@ -162,8 +161,7 @@ struct sdhci_arasan_clk_data {
> >   	struct clk      *sdcardclk;
> >   	struct clk_hw	sampleclk_hw;
> >   	struct clk      *sampleclk;
> > -	int		clk_phase_in[MMC_TIMING_MMC_HS400 + 1];
> > -	int		clk_phase_out[MMC_TIMING_MMC_HS400 + 1];
> > +	struct mmc_clk_phase_map phase_map;
> >   	void		(*set_clk_delays)(struct sdhci_host *host);
> >   	void		*clk_of_data;
> >   };
> > @@ -1248,37 +1246,13 @@ static void sdhci_arasan_set_clk_delays(struct
> sdhci_host *host)
> >   	struct sdhci_arasan_data *sdhci_arasan =
> sdhci_pltfm_priv(pltfm_host);
> >   	struct sdhci_arasan_clk_data *clk_data = &sdhci_arasan->clk_data;
> >
> > -	clk_set_phase(clk_data->sampleclk,
> > -		      clk_data->clk_phase_in[host->timing]);
> > -	clk_set_phase(clk_data->sdcardclk,
> > -		      clk_data->clk_phase_out[host->timing]);
> > -}
> > -
> > -static void arasan_dt_read_clk_phase(struct device *dev,
> > -				     struct sdhci_arasan_clk_data *clk_data,
> > -				     unsigned int timing, const char *prop)
> > -{
> > -	struct device_node *np = dev->of_node;
> > -
> > -	u32 clk_phase[2] = {0};
> > -	int ret;
> > -
> > -	/*
> > -	 * Read Tap Delay values from DT, if the DT does not contain the
> > -	 * Tap Values then use the pre-defined values.
> > -	 */
> > -	ret = of_property_read_variable_u32_array(np, prop, &clk_phase[0],
> > -						  2, 0);
> > -	if (ret < 0) {
> > -		dev_dbg(dev, "Using predefined clock phase for %s = %d
> %d\n",
> > -			prop, clk_data->clk_phase_in[timing],
> > -			clk_data->clk_phase_out[timing]);
> > +	if (!clk_data->phase_map.phase[host->timing].valid)
> >   		return;

This check breaks platforms relying on default phase values. When DT
properties are absent, mmc_of_parse_timing_phase() sets valid=false,
which causes sdhci_arasan_set_clk_delays() to skip phase configuration.

Regards
Sai Krishna

> > -	}
> >
> > -	/* The values read are Input and Output Clock Delays in order */
> > -	clk_data->clk_phase_in[timing] = clk_phase[0];
> > -	clk_data->clk_phase_out[timing] = clk_phase[1];
> > +	clk_set_phase(clk_data->sampleclk,
> > +		      clk_data->phase_map.phase[host->timing].in_deg);
> > +	clk_set_phase(clk_data->sdcardclk,
> > +		      clk_data->phase_map.phase[host->timing].out_deg);
> >   }
> >
> >   /**
> > @@ -1315,8 +1289,9 @@ static void arasan_dt_parse_clk_phases(struct
> device *dev,
> >   		}
> >
> >   		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
> > -			clk_data->clk_phase_in[i] = zynqmp_iclk_phase[i];
> > -			clk_data->clk_phase_out[i] = zynqmp_oclk_phase[i];
> > +			clk_data->phase_map.phase[i].in_deg =
> zynqmp_iclk_phase[i];
> > +			clk_data->phase_map.phase[i].out_deg =
> zynqmp_oclk_phase[i];
> > +			clk_data->phase_map.phase[i].valid = true;
> >   		}
> >   	}
> >
> > @@ -1327,8 +1302,9 @@ static void arasan_dt_parse_clk_phases(struct
> device *dev,
> >   			VERSAL_OCLK_PHASE;
> >
> >   		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
> > -			clk_data->clk_phase_in[i] = versal_iclk_phase[i];
> > -			clk_data->clk_phase_out[i] = versal_oclk_phase[i];
> > +			clk_data->phase_map.phase[i].in_deg =
> versal_iclk_phase[i];
> > +			clk_data->phase_map.phase[i].out_deg =
> versal_oclk_phase[i];
> > +			clk_data->phase_map.phase[i].valid = true;
> >   		}
> >   	}
> >   	if (of_device_is_compatible(dev->of_node, "xlnx,versal-net-emmc"))
> > { @@ -1338,32 +1314,13 @@ static void arasan_dt_parse_clk_phases(struct
> device *dev,
> >   			VERSAL_NET_EMMC_OCLK_PHASE;
> >
> >   		for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
> > -			clk_data->clk_phase_in[i] = versal_net_iclk_phase[i];
> > -			clk_data->clk_phase_out[i] =
> versal_net_oclk_phase[i];
> > +			clk_data->phase_map.phase[i].in_deg =
> versal_net_iclk_phase[i];
> > +			clk_data->phase_map.phase[i].out_deg =
> versal_net_oclk_phase[i];
> > +			clk_data->phase_map.phase[i].valid = true;
> >   		}
> >   	}
> > -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_LEGACY,
> > -				 "clk-phase-legacy");
> > -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS,
> > -				 "clk-phase-mmc-hs");
> > -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_SD_HS,
> > -				 "clk-phase-sd-hs");
> > -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR12,
> > -				 "clk-phase-uhs-sdr12");
> > -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR25,
> > -				 "clk-phase-uhs-sdr25");
> > -	arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR50,
> > -				 "clk-phase-uhs-sdr50");
> > -	arasan_dt_read_clk_phase(dev, clk_data,
> MMC_TIMING_UHS_SDR104,
> > -				 "clk-phase-uhs-sdr104");
> > -	arasan_dt_read_clk_phase(dev, clk_data,
> MMC_TIMING_UHS_DDR50,
> > -				 "clk-phase-uhs-ddr50");
> > -	arasan_dt_read_clk_phase(dev, clk_data,
> MMC_TIMING_MMC_DDR52,
> > -				 "clk-phase-mmc-ddr52");
> > -	arasan_dt_read_clk_phase(dev, clk_data,
> MMC_TIMING_MMC_HS200,
> > -				 "clk-phase-mmc-hs200");
> > -	arasan_dt_read_clk_phase(dev, clk_data,
> MMC_TIMING_MMC_HS400,
> > -				 "clk-phase-mmc-hs400");
> > +
> > +	mmc_of_parse_clk_phase(dev, &clk_data->phase_map);
> >   }
> >
> >   static const struct sdhci_pltfm_data sdhci_arasan_pdata = {
> 
> please take a look at this patch.
> I was talking to him yesterday and he needs to finish two things before he has
> time to look at it. That's why please give him some time.
> 
> Thanks,
> Michal

Hi Sai

在 2026/03/23 星期一 17:27, Sai Krishna Potthuri 写道:
> 
> 
>> -----Original Message-----
>> From: Simek, Michal <michal.simek@amd.com>
>> Sent: Thursday, March 19, 2026 3:39 PM
>> To: Shawn Lin <shawn.lin@rock-chips.com>; Ulf Hansson
>> <ulf.hansson@linaro.org>; Potthuri, Sai Krishna
>> <sai.krishna.potthuri@amd.com>
>> Cc: linux-mmc@vger.kernel.org; Adrian Hunter <adrian.hunter@intel.com>;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] mmc: sdhci-of-arasan: Use standard
>> mmc_clk_phase_map infrastructure
>>
>> +Sai,
>>
>> On 3/17/26 03:17, Shawn Lin wrote:
>> > Convert the Arasan SDHCI driver to use the mainline standard
>> > mmc_clk_phase_map infrastructure instead of custom clk_phase_in/out
>> > arrays as well as arasan_dt_read_clk_phase().
>> >
>> > The phase values for ZynqMP, Versal, and Versal-NET platforms are
>> > still initialized from the predefined tables, but now follow the
>> > standard phase_map format with valid flag.
>> >
>> > Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
>> > ---
>> >
>> >   drivers/mmc/host/sdhci-of-arasan.c | 79 
>> +++++++++-----------------------------
>> >   1 file changed, 18 insertions(+), 61 deletions(-)
>> >
>> > diff --git a/drivers/mmc/host/sdhci-of-arasan.c
>> > b/drivers/mmc/host/sdhci-of-arasan.c
>> > index caf9723..b97d27c 100644
>> > --- a/drivers/mmc/host/sdhci-of-arasan.c
>> > +++ b/drivers/mmc/host/sdhci-of-arasan.c
>> > @@ -152,8 +152,7 @@ struct sdhci_arasan_clk_ops {
>> >    * @sdcardclk:        Pointer to normal 'struct clock' for
>> sdcardclk_hw.
>> >    * @sampleclk_hw:    Struct for the clock we might provide to a PHY.
>> >    * @sampleclk:        Pointer to normal 'struct clock' for
>> sampleclk_hw.
>> > - * @clk_phase_in:    Array of Input Clock Phase Delays for all 
>> speed modes
>> > - * @clk_phase_out:    Array of Output Clock Phase Delays for all speed
>> modes
>> > + * @phase_map:        Struct for mmc_clk_phase_map provided.
>> >    * @set_clk_delays:    Function pointer for setting Clock Delays
>> >    * @clk_of_data:    Platform specific runtime clock data storage 
>> pointer
>> >    */
>> > @@ -162,8 +161,7 @@ struct sdhci_arasan_clk_data {
>> >       struct clk      *sdcardclk;
>> >       struct clk_hw    sampleclk_hw;
>> >       struct clk      *sampleclk;
>> > -    int        clk_phase_in[MMC_TIMING_MMC_HS400 + 1];
>> > -    int        clk_phase_out[MMC_TIMING_MMC_HS400 + 1];
>> > +    struct mmc_clk_phase_map phase_map;
>> >       void        (*set_clk_delays)(struct sdhci_host *host);
>> >       void        *clk_of_data;
>> >   };
>> > @@ -1248,37 +1246,13 @@ static void sdhci_arasan_set_clk_delays(struct
>> sdhci_host *host)
>> >       struct sdhci_arasan_data *sdhci_arasan =
>> sdhci_pltfm_priv(pltfm_host);
>> >       struct sdhci_arasan_clk_data *clk_data = &sdhci_arasan->clk_data;
>> >
>> > -    clk_set_phase(clk_data->sampleclk,
>> > -              clk_data->clk_phase_in[host->timing]);
>> > -    clk_set_phase(clk_data->sdcardclk,
>> > -              clk_data->clk_phase_out[host->timing]);
>> > -}
>> > -
>> > -static void arasan_dt_read_clk_phase(struct device *dev,
>> > -                     struct sdhci_arasan_clk_data *clk_data,
>> > -                     unsigned int timing, const char *prop)
>> > -{
>> > -    struct device_node *np = dev->of_node;
>> > -
>> > -    u32 clk_phase[2] = {0};
>> > -    int ret;
>> > -
>> > -    /*
>> > -     * Read Tap Delay values from DT, if the DT does not contain the
>> > -     * Tap Values then use the pre-defined values.
>> > -     */
>> > -    ret = of_property_read_variable_u32_array(np, prop, &clk_phase[0],
>> > -                          2, 0);
>> > -    if (ret < 0) {
>> > -        dev_dbg(dev, "Using predefined clock phase for %s = %d
>> %d\n",
>> > -            prop, clk_data->clk_phase_in[timing],
>> > -            clk_data->clk_phase_out[timing]);
>> > +    if (!clk_data->phase_map.phase[host->timing].valid)
>> >           return;
> 
> This check breaks platforms relying on default phase values. When DT
> properties are absent, mmc_of_parse_timing_phase() sets valid=false,
> which causes sdhci_arasan_set_clk_delays() to skip phase configuration.
> 

Ah, you're right, thanks for catching this.

I'll send a v2 patch that removes the validity check in
sdhci_arasan_set_clk_delays().

> Regards
> Sai Krishna
> 
>> > -    }
>> >
>> > -    /* The values read are Input and Output Clock Delays in order */
>> > -    clk_data->clk_phase_in[timing] = clk_phase[0];
>> > -    clk_data->clk_phase_out[timing] = clk_phase[1];
>> > +    clk_set_phase(clk_data->sampleclk,
>> > +              clk_data->phase_map.phase[host->timing].in_deg);
>> > +    clk_set_phase(clk_data->sdcardclk,
>> > +              clk_data->phase_map.phase[host->timing].out_deg);
>> >   }
>> >
>> >   /**
>> > @@ -1315,8 +1289,9 @@ static void arasan_dt_parse_clk_phases(struct
>> device *dev,
>> >           }
>> >
>> >           for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
>> > -            clk_data->clk_phase_in[i] = zynqmp_iclk_phase[i];
>> > -            clk_data->clk_phase_out[i] = zynqmp_oclk_phase[i];
>> > +            clk_data->phase_map.phase[i].in_deg =
>> zynqmp_iclk_phase[i];
>> > +            clk_data->phase_map.phase[i].out_deg =
>> zynqmp_oclk_phase[i];
>> > +            clk_data->phase_map.phase[i].valid = true;
>> >           }
>> >       }
>> >
>> > @@ -1327,8 +1302,9 @@ static void arasan_dt_parse_clk_phases(struct
>> device *dev,
>> >               VERSAL_OCLK_PHASE;
>> >
>> >           for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
>> > -            clk_data->clk_phase_in[i] = versal_iclk_phase[i];
>> > -            clk_data->clk_phase_out[i] = versal_oclk_phase[i];
>> > +            clk_data->phase_map.phase[i].in_deg =
>> versal_iclk_phase[i];
>> > +            clk_data->phase_map.phase[i].out_deg =
>> versal_oclk_phase[i];
>> > +            clk_data->phase_map.phase[i].valid = true;
>> >           }
>> >       }
>> >       if (of_device_is_compatible(dev->of_node, 
>> "xlnx,versal-net-emmc"))
>> > { @@ -1338,32 +1314,13 @@ static void arasan_dt_parse_clk_phases(struct
>> device *dev,
>> >               VERSAL_NET_EMMC_OCLK_PHASE;
>> >
>> >           for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) {
>> > -            clk_data->clk_phase_in[i] = versal_net_iclk_phase[i];
>> > -            clk_data->clk_phase_out[i] =
>> versal_net_oclk_phase[i];
>> > +            clk_data->phase_map.phase[i].in_deg =
>> versal_net_iclk_phase[i];
>> > +            clk_data->phase_map.phase[i].out_deg =
>> versal_net_oclk_phase[i];
>> > +            clk_data->phase_map.phase[i].valid = true;
>> >           }
>> >       }
>> > -    arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_LEGACY,
>> > -                 "clk-phase-legacy");
>> > -    arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_MMC_HS,
>> > -                 "clk-phase-mmc-hs");
>> > -    arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_SD_HS,
>> > -                 "clk-phase-sd-hs");
>> > -    arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR12,
>> > -                 "clk-phase-uhs-sdr12");
>> > -    arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR25,
>> > -                 "clk-phase-uhs-sdr25");
>> > -    arasan_dt_read_clk_phase(dev, clk_data, MMC_TIMING_UHS_SDR50,
>> > -                 "clk-phase-uhs-sdr50");
>> > -    arasan_dt_read_clk_phase(dev, clk_data,
>> MMC_TIMING_UHS_SDR104,
>> > -                 "clk-phase-uhs-sdr104");
>> > -    arasan_dt_read_clk_phase(dev, clk_data,
>> MMC_TIMING_UHS_DDR50,
>> > -                 "clk-phase-uhs-ddr50");
>> > -    arasan_dt_read_clk_phase(dev, clk_data,
>> MMC_TIMING_MMC_DDR52,
>> > -                 "clk-phase-mmc-ddr52");
>> > -    arasan_dt_read_clk_phase(dev, clk_data,
>> MMC_TIMING_MMC_HS200,
>> > -                 "clk-phase-mmc-hs200");
>> > -    arasan_dt_read_clk_phase(dev, clk_data,
>> MMC_TIMING_MMC_HS400,
>> > -                 "clk-phase-mmc-hs400");
>> > +
>> > +    mmc_of_parse_clk_phase(dev, &clk_data->phase_map);
>> >   }
>> >
>> >   static const struct sdhci_pltfm_data sdhci_arasan_pdata = {
>>
>> please take a look at this patch.
>> I was talking to him yesterday and he needs to finish two things 
>> before he has
>> time to look at it. That's why please give him some time.
>>
>> Thanks,
>> Michal
> 
>