* [PATCH] mmc: sdhci-of-arasan: Use standard mmc_clk_phase_map infrastructure @ 2026-03-17 2:17 Shawn Lin 2026-03-19 10:09 ` Michal Simek 0 siblings, 1 reply; 4+ messages in thread From: Shawn Lin @ 2026-03-17 2:17 UTC (permalink / raw) To: Ulf Hansson Cc: linux-mmc, Adrian Hunter, Michal Simek, linux-kernel, Shawn Lin 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Use standard mmc_clk_phase_map infrastructure 2026-03-17 2:17 [PATCH] mmc: sdhci-of-arasan: Use standard mmc_clk_phase_map infrastructure Shawn Lin @ 2026-03-19 10:09 ` Michal Simek 2026-03-23 9:27 ` Sai Krishna Potthuri 0 siblings, 1 reply; 4+ messages in thread From: Michal Simek @ 2026-03-19 10:09 UTC (permalink / raw) To: Shawn Lin, Ulf Hansson, Sai Krishna Potthuri Cc: linux-mmc, Adrian Hunter, linux-kernel +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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Use standard mmc_clk_phase_map infrastructure 2026-03-19 10:09 ` Michal Simek @ 2026-03-23 9:27 ` Sai Krishna Potthuri 2026-03-24 8:34 ` Shawn Lin 0 siblings, 1 reply; 4+ messages in thread From: Sai Krishna Potthuri @ 2026-03-23 9:27 UTC (permalink / raw) To: Simek, Michal, Shawn Lin, Ulf Hansson Cc: linux-mmc@vger.kernel.org, Adrian Hunter, linux-kernel@vger.kernel.org > -----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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mmc: sdhci-of-arasan: Use standard mmc_clk_phase_map infrastructure 2026-03-23 9:27 ` Sai Krishna Potthuri @ 2026-03-24 8:34 ` Shawn Lin 0 siblings, 0 replies; 4+ messages in thread From: Shawn Lin @ 2026-03-24 8:34 UTC (permalink / raw) To: Sai Krishna Potthuri, Simek, Michal, Ulf Hansson Cc: shawn.lin, linux-mmc@vger.kernel.org, Adrian Hunter, linux-kernel@vger.kernel.org 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 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-24 8:34 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-17 2:17 [PATCH] mmc: sdhci-of-arasan: Use standard mmc_clk_phase_map infrastructure Shawn Lin 2026-03-19 10:09 ` Michal Simek 2026-03-23 9:27 ` Sai Krishna Potthuri 2026-03-24 8:34 ` Shawn Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox