[PATCH 0/7] mmc: zynqmp_sdhci: Add support for Tap delay

Hi,
this patchset adding support for Tap delay programming for ZynqMP and Versal. Based on mainline discussion also DT properties have been introduced which are documented here. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/Do... The patchset is using these DT properties which are optional. Default values are also listed.
The patchset is based on: https://lists.denx.de/pipermail/u-boot/2020-May/410326.html
Also new code is adding checking for SOCs which are not needed at that time when patch is applied (For example "mmc: zynq_sdhci: Read clock phase delays from dt" and IS_ENABLED(CONFIG_ARCH_ZYNQMP)... But I kept it there for more cleaner patches built on the top.
Thanks, Michal
Ashok Reddy Soma (5): Revert "mmc: zynq: parse dt when probing" mmc: zynq_sdhci: Define timing macro's mmc: zynq_sdhci: Set tapdelays based on clk phase delays mmc: zynq_sdhci: Add clock phase delays for Versal mmc: zynq_sdhci: Extend UHS timings till hs200
Michal Simek (2): mmc: zynq_sdhci: Move macro to the top mmc: zynq_sdhci: Read clock phase delays from dt
drivers/mmc/sdhci.c | 3 +- drivers/mmc/zynq_sdhci.c | 406 ++++++++++++++++++++++++++++++++++++--- include/mmc.h | 13 ++ include/sdhci.h | 1 + 4 files changed, 398 insertions(+), 25 deletions(-)

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830.
This is partial revert of the above commit.
mmc_of_parse() is reading no-1-8-v from device tree and if set, it is clearing the UHS speed capabilities of cfg->host_caps. cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | MMC_MODE_HS400 | MMC_MODE_HS400_ES);
This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50.
Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), these speed flags are getting set again in cfg->host_caps in sdhci_setup_cfg().
The reason for this is, SDHCI_SUPPORT_XXX flags are cleared only if controller is not capable of supporting MMC_VDD_165_195 volts.
if (caps & SDHCI_CAN_VDD_180) cfg->voltages |= MMC_VDD_165_195;
if (!(cfg->voltages & MMC_VDD_165_195)) caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
It means "no-1-8-v", which is read from DT is not coming in to effect. So it is better we keep the host quirks(SDHCI_QUIRK_NO_1_8_V) to clear UHS speeds based on no-1-8-v from device tree.
Hence revert the functionality related to no-1-8-v only, rest is fine in the patch.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/sdhci.c | 3 ++- drivers/mmc/zynq_sdhci.c | 5 +++++ include/sdhci.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 92cc8434af24..099a007984a8 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -844,7 +844,8 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, cfg->host_caps &= ~MMC_MODE_HS_52MHz; }
- if (!(cfg->voltages & MMC_VDD_165_195)) + if (!(cfg->voltages & MMC_VDD_165_195) || + (host->quirks & SDHCI_QUIRK_NO_1_8_V)) caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 43b9f215229a..94c69cf1c1bd 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -28,6 +28,7 @@ struct arasan_sdhci_priv { struct sdhci_host *host; u8 deviceid; u8 bank; + u8 no_1p8; };
#if defined(CONFIG_ARCH_ZYNQMP) @@ -236,6 +237,9 @@ static int arasan_sdhci_probe(struct udevice *dev) host->quirks |= SDHCI_QUIRK_BROKEN_HISPD_MODE; #endif
+ if (priv->no_1p8) + host->quirks |= SDHCI_QUIRK_NO_1_8_V; + plat->cfg.f_max = CONFIG_ZYNQ_SDHCI_MAX_FREQ;
ret = mmc_of_parse(dev, &plat->cfg); @@ -277,6 +281,7 @@ static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
priv->deviceid = dev_read_u32_default(dev, "xlnx,device_id", -1); priv->bank = dev_read_u32_default(dev, "xlnx,mio_bank", -1); + priv->no_1p8 = dev_read_bool(dev, "no-1-8-v");
return 0; } diff --git a/include/sdhci.h b/include/sdhci.h index 94fc3ed56ace..ef84bebcb4aa 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -244,6 +244,7 @@ #define SDHCI_QUIRK_BROKEN_HISPD_MODE BIT(5) #define SDHCI_QUIRK_WAIT_SEND_CMD (1 << 6) #define SDHCI_QUIRK_USE_WIDE8 (1 << 8) +#define SDHCI_QUIRK_NO_1_8_V (1 << 9)
/* to make gcc happy */ struct sdhci_host;

Hi
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830.
This is partial revert of the above commit.
mmc_of_parse() is reading no-1-8-v from device tree and if set, it is clearing the UHS speed capabilities of cfg->host_caps. cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | MMC_MODE_HS400 | MMC_MODE_HS400_ES);
This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50.
Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), these speed flags are getting set again in cfg->host_caps in sdhci_setup_cfg().
The reason for this is, SDHCI_SUPPORT_XXX flags are cleared only if controller is not capable of supporting MMC_VDD_165_195 volts.
if (caps & SDHCI_CAN_VDD_180) cfg->voltages |= MMC_VDD_165_195;
if (!(cfg->voltages & MMC_VDD_165_195)) caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
I don't have a target to test with zynq_sdhci. If there is no effect no-1-8-v property, i think that it needs to fix main problem. I will check about this.
It means "no-1-8-v", which is read from DT is not coming in to effect. So it is better we keep the host quirks(SDHCI_QUIRK_NO_1_8_V) to clear UHS speeds based on no-1-8-v from device tree.
Hence revert the functionality related to no-1-8-v only, rest is fine in the patch.
Anyway, I don't have any objection about reverting it.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/sdhci.c | 3 ++- drivers/mmc/zynq_sdhci.c | 5 +++++ include/sdhci.h | 1 + 3 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/sdhci.c b/drivers/mmc/sdhci.c index 92cc8434af24..099a007984a8 100644 --- a/drivers/mmc/sdhci.c +++ b/drivers/mmc/sdhci.c @@ -844,7 +844,8 @@ int sdhci_setup_cfg(struct mmc_config *cfg, struct sdhci_host *host, cfg->host_caps &= ~MMC_MODE_HS_52MHz; }
- if (!(cfg->voltages & MMC_VDD_165_195))
- if (!(cfg->voltages & MMC_VDD_165_195) ||
caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);(host->quirks & SDHCI_QUIRK_NO_1_8_V))
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 43b9f215229a..94c69cf1c1bd 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -28,6 +28,7 @@ struct arasan_sdhci_priv { struct sdhci_host *host; u8 deviceid; u8 bank;
- u8 no_1p8;
};
#if defined(CONFIG_ARCH_ZYNQMP) @@ -236,6 +237,9 @@ static int arasan_sdhci_probe(struct udevice *dev) host->quirks |= SDHCI_QUIRK_BROKEN_HISPD_MODE; #endif
if (priv->no_1p8)
host->quirks |= SDHCI_QUIRK_NO_1_8_V;
plat->cfg.f_max = CONFIG_ZYNQ_SDHCI_MAX_FREQ;
ret = mmc_of_parse(dev, &plat->cfg);
@@ -277,6 +281,7 @@ static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
priv->deviceid = dev_read_u32_default(dev, "xlnx,device_id", -1); priv->bank = dev_read_u32_default(dev, "xlnx,mio_bank", -1);
priv->no_1p8 = dev_read_bool(dev, "no-1-8-v");
return 0;
} diff --git a/include/sdhci.h b/include/sdhci.h index 94fc3ed56ace..ef84bebcb4aa 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -244,6 +244,7 @@ #define SDHCI_QUIRK_BROKEN_HISPD_MODE BIT(5) #define SDHCI_QUIRK_WAIT_SEND_CMD (1 << 6) #define SDHCI_QUIRK_USE_WIDE8 (1 << 8) +#define SDHCI_QUIRK_NO_1_8_V (1 << 9)
/* to make gcc happy */ struct sdhci_host;

On 27. 05. 20 8:44, Jaehoon Chung wrote:
Hi
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830.
This is partial revert of the above commit.
mmc_of_parse() is reading no-1-8-v from device tree and if set, it is clearing the UHS speed capabilities of cfg->host_caps. cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | MMC_MODE_HS400 | MMC_MODE_HS400_ES);
This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50.
Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), these speed flags are getting set again in cfg->host_caps in sdhci_setup_cfg().
The reason for this is, SDHCI_SUPPORT_XXX flags are cleared only if controller is not capable of supporting MMC_VDD_165_195 volts.
if (caps & SDHCI_CAN_VDD_180) cfg->voltages |= MMC_VDD_165_195;
if (!(cfg->voltages & MMC_VDD_165_195)) caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
I don't have a target to test with zynq_sdhci. If there is no effect no-1-8-v property, i think that it needs to fix main problem. I will check about this.
Have you had any time to take a look at it?
Thanks, Michal

Hi,
On 6/8/20 10:16 PM, Michal Simek wrote:
On 27. 05. 20 8:44, Jaehoon Chung wrote:
Hi
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
This reverts commit 942b5fc03218d1c94468fc658e7dec65dabcc830.
This is partial revert of the above commit.
mmc_of_parse() is reading no-1-8-v from device tree and if set, it is clearing the UHS speed capabilities of cfg->host_caps. cfg->host_caps &= ~(UHS_CAPS | MMC_MODE_HS200 | MMC_MODE_HS400 | MMC_MODE_HS400_ES);
This is still missing to clear UHS speeds like SDHCI_SUPPORT_SDR104, SDHCI_SUPPORT_SDR50 and SDHCI_SUPPORT_DDR50.
Even if we clear the flags SDHCI_SUPPORT_XXX in mmc_of_parse(), these speed flags are getting set again in cfg->host_caps in sdhci_setup_cfg().
The reason for this is, SDHCI_SUPPORT_XXX flags are cleared only if controller is not capable of supporting MMC_VDD_165_195 volts.
if (caps & SDHCI_CAN_VDD_180) cfg->voltages |= MMC_VDD_165_195;
if (!(cfg->voltages & MMC_VDD_165_195)) caps_1 &= ~(SDHCI_SUPPORT_SDR104 | SDHCI_SUPPORT_SDR50 | SDHCI_SUPPORT_DDR50);
I don't have a target to test with zynq_sdhci. If there is no effect no-1-8-v property, i think that it needs to fix main problem. I will check about this.
Have you had any time to take a look at it?
Sorry for late. I think it was similar issue on my environment. I will share result within this week. If it's urgent, then after merging i will find solution to fix it.
Best Regards, Jaehoon Chung
Thanks, Michal

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5 - static const u8 mode2timing[] = { - [MMC_LEGACY] = UHS_SDR12_BUS_SPEED, - [MMC_HS] = HIGH_SPEED_BUS_SPEED, - [SD_HS] = HIGH_SPEED_BUS_SPEED, - [MMC_HS_52] = HIGH_SPEED_BUS_SPEED, - [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED, - [UHS_SDR12] = UHS_SDR12_BUS_SPEED, - [UHS_SDR25] = UHS_SDR25_BUS_SPEED, - [UHS_SDR50] = UHS_SDR50_BUS_SPEED, - [UHS_DDR50] = UHS_DDR50_BUS_SPEED, - [UHS_SDR104] = UHS_SDR104_BUS_SPEED, - [MMC_HS_200] = MMC_HS200_BUS_SPEED, + [MMC_LEGACY] = MMC_TIMING_LEGACY, + [MMC_HS] = MMC_TIMING_MMC_HS, + [SD_HS] = MMC_TIMING_SD_HS, + [MMC_HS_52] = MMC_TIMING_UHS_SDR50, + [MMC_DDR_52] = MMC_TIMING_UHS_DDR50, + [UHS_SDR12] = MMC_TIMING_UHS_SDR12, + [UHS_SDR25] = MMC_TIMING_UHS_SDR25, + [UHS_SDR50] = MMC_TIMING_UHS_SDR50, + [UHS_DDR50] = MMC_TIMING_UHS_DDR50, + [UHS_SDR104] = MMC_TIMING_UHS_SDR104, + [MMC_HS_200] = MMC_TIMING_MMC_HS200, };
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10 + /* Driver model support */
/**

On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Even though it's similar to linux, does it need to add new macro?
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5
static const u8 mode2timing[] = {
- [MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
- [MMC_HS] = HIGH_SPEED_BUS_SPEED,
- [SD_HS] = HIGH_SPEED_BUS_SPEED,
- [MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
- [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED,
- [UHS_SDR12] = UHS_SDR12_BUS_SPEED,
- [UHS_SDR25] = UHS_SDR25_BUS_SPEED,
- [UHS_SDR50] = UHS_SDR50_BUS_SPEED,
- [UHS_DDR50] = UHS_DDR50_BUS_SPEED,
- [UHS_SDR104] = UHS_SDR104_BUS_SPEED,
- [MMC_HS_200] = MMC_HS200_BUS_SPEED,
- [MMC_LEGACY] = MMC_TIMING_LEGACY,
- [MMC_HS] = MMC_TIMING_MMC_HS,
- [SD_HS] = MMC_TIMING_SD_HS,
- [MMC_HS_52] = MMC_TIMING_UHS_SDR50,
- [MMC_DDR_52] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR12] = MMC_TIMING_UHS_SDR12,
- [UHS_SDR25] = MMC_TIMING_UHS_SDR25,
- [UHS_SDR50] = MMC_TIMING_UHS_SDR50,
- [UHS_DDR50] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR104] = MMC_TIMING_UHS_SDR104,
- [MMC_HS_200] = MMC_TIMING_MMC_HS200,
};
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10
/* Driver model support */
/**

Michal,
On 27/05/20 12:17 pm, Jaehoon Chung wrote:
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Even though it's similar to linux, does it need to add new macro?
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5
static const u8 mode2timing[] = {
- [MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
- [MMC_HS] = HIGH_SPEED_BUS_SPEED,
- [SD_HS] = HIGH_SPEED_BUS_SPEED,
- [MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
- [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED,
- [UHS_SDR12] = UHS_SDR12_BUS_SPEED,
- [UHS_SDR25] = UHS_SDR25_BUS_SPEED,
- [UHS_SDR50] = UHS_SDR50_BUS_SPEED,
- [UHS_DDR50] = UHS_DDR50_BUS_SPEED,
- [UHS_SDR104] = UHS_SDR104_BUS_SPEED,
- [MMC_HS_200] = MMC_HS200_BUS_SPEED,
- [MMC_LEGACY] = MMC_TIMING_LEGACY,
- [MMC_HS] = MMC_TIMING_MMC_HS,
- [SD_HS] = MMC_TIMING_SD_HS,
- [MMC_HS_52] = MMC_TIMING_UHS_SDR50,
- [MMC_DDR_52] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR12] = MMC_TIMING_UHS_SDR12,
- [UHS_SDR25] = MMC_TIMING_UHS_SDR25,
- [UHS_SDR50] = MMC_TIMING_UHS_SDR50,
- [UHS_DDR50] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR104] = MMC_TIMING_UHS_SDR104,
- [MMC_HS_200] = MMC_TIMING_MMC_HS200,
};
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10
/* Driver model support */
There's already an
enum bus_mode { MMC_LEGACY, MMC_HS, SD_HS, MMC_HS_52, MMC_DDR_52, UHS_SDR12, UHS_SDR25, UHS_SDR50, UHS_DDR50, UHS_SDR104, MMC_HS_200, MMC_HS_400, MMC_HS_400_ES, MMC_MODES_END };
in this file. Thats what the mmc core uses to represent timing. Please use the same symbols.
Thanks, Faiz

Hi Faiz,
-----Original Message----- From: Faiz Abbas faiz_abbas@ti.com Sent: Wednesday, May 27, 2020 12:28 PM To: Jaehoon Chung jh80.chung@samsung.com; Michal Simek michals@xilinx.com; u-boot@lists.denx.de; git git@xilinx.com Cc: Ashok Reddy Soma ashokred@xilinx.com; Heinrich Schuchardt xypron.glpk@gmx.de; Lokesh Vutla lokeshvutla@ti.com; Marek Vasut marek.vasut@gmail.com; Masahiro Yamada masahiroy@kernel.org; Peng Fan peng.fan@nxp.com; Sam Protsenko semen.protsenko@linaro.org; Simon Glass sjg@chromium.org; Yann Gautier yann.gautier@st.com Subject: Re: [PATCH 2/7] mmc: zynq_sdhci: Define timing macro's
Michal,
On 27/05/20 12:17 pm, Jaehoon Chung wrote:
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Even though it's similar to linux, does it need to add new macro?
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5
static const u8 mode2timing[] = {
- [MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
- [MMC_HS] = HIGH_SPEED_BUS_SPEED,
- [SD_HS] = HIGH_SPEED_BUS_SPEED,
- [MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
- [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED,
- [UHS_SDR12] = UHS_SDR12_BUS_SPEED,
- [UHS_SDR25] = UHS_SDR25_BUS_SPEED,
- [UHS_SDR50] = UHS_SDR50_BUS_SPEED,
- [UHS_DDR50] = UHS_DDR50_BUS_SPEED,
- [UHS_SDR104] = UHS_SDR104_BUS_SPEED,
- [MMC_HS_200] = MMC_HS200_BUS_SPEED,
- [MMC_LEGACY] = MMC_TIMING_LEGACY,
- [MMC_HS] = MMC_TIMING_MMC_HS,
- [SD_HS] = MMC_TIMING_SD_HS,
- [MMC_HS_52] = MMC_TIMING_UHS_SDR50,
- [MMC_DDR_52] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR12] = MMC_TIMING_UHS_SDR12,
- [UHS_SDR25] = MMC_TIMING_UHS_SDR25,
- [UHS_SDR50] = MMC_TIMING_UHS_SDR50,
- [UHS_DDR50] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR104] = MMC_TIMING_UHS_SDR104,
- [MMC_HS_200] = MMC_TIMING_MMC_HS200,
};
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10
/* Driver model support */
There's already an
enum bus_mode { MMC_LEGACY, MMC_HS, SD_HS, MMC_HS_52, MMC_DDR_52, UHS_SDR12, UHS_SDR25, UHS_SDR50, UHS_DDR50, UHS_SDR104, MMC_HS_200, MMC_HS_400, MMC_HS_400_ES, MMC_MODES_END };
in this file. Thats what the mmc core uses to represent timing. Please use the same symbols.
The enum and macro differ in values. For example UHS_SDR12 macro value is 3 whereas enum will be 5. This is a problem when accessing below arrays. I take these reference values from linux driver. If the values change in future, it will be easy for u-boot driver to just copy and paste from linux driver if we use macro's.
/* Default settings for ZynqMP Clock Phases */ const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63, 0, 0, 183, 54, 0, 0}; const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0};
I also see that xenon_sdhci.c has defined these macro's locally. https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/mmc/xenon_sdhci.c...
So I have added these macro's in include/mmc.h for everyone's use.
Thanks, Faiz
Thanks, Ashok

Hi Ashok,
On 10/06/20 2:48 pm, Ashok Reddy Soma wrote:
Hi Faiz,
-----Original Message----- From: Faiz Abbas faiz_abbas@ti.com Sent: Wednesday, May 27, 2020 12:28 PM To: Jaehoon Chung jh80.chung@samsung.com; Michal Simek michals@xilinx.com; u-boot@lists.denx.de; git git@xilinx.com Cc: Ashok Reddy Soma ashokred@xilinx.com; Heinrich Schuchardt xypron.glpk@gmx.de; Lokesh Vutla lokeshvutla@ti.com; Marek Vasut marek.vasut@gmail.com; Masahiro Yamada masahiroy@kernel.org; Peng Fan peng.fan@nxp.com; Sam Protsenko semen.protsenko@linaro.org; Simon Glass sjg@chromium.org; Yann Gautier yann.gautier@st.com Subject: Re: [PATCH 2/7] mmc: zynq_sdhci: Define timing macro's
Michal,
On 27/05/20 12:17 pm, Jaehoon Chung wrote:
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Even though it's similar to linux, does it need to add new macro?
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5
static const u8 mode2timing[] = {
- [MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
- [MMC_HS] = HIGH_SPEED_BUS_SPEED,
- [SD_HS] = HIGH_SPEED_BUS_SPEED,
- [MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
- [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED,
- [UHS_SDR12] = UHS_SDR12_BUS_SPEED,
- [UHS_SDR25] = UHS_SDR25_BUS_SPEED,
- [UHS_SDR50] = UHS_SDR50_BUS_SPEED,
- [UHS_DDR50] = UHS_DDR50_BUS_SPEED,
- [UHS_SDR104] = UHS_SDR104_BUS_SPEED,
- [MMC_HS_200] = MMC_HS200_BUS_SPEED,
- [MMC_LEGACY] = MMC_TIMING_LEGACY,
- [MMC_HS] = MMC_TIMING_MMC_HS,
- [SD_HS] = MMC_TIMING_SD_HS,
- [MMC_HS_52] = MMC_TIMING_UHS_SDR50,
- [MMC_DDR_52] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR12] = MMC_TIMING_UHS_SDR12,
- [UHS_SDR25] = MMC_TIMING_UHS_SDR25,
- [UHS_SDR50] = MMC_TIMING_UHS_SDR50,
- [UHS_DDR50] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR104] = MMC_TIMING_UHS_SDR104,
- [MMC_HS_200] = MMC_TIMING_MMC_HS200,
};
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10
/* Driver model support */
There's already an
enum bus_mode { MMC_LEGACY, MMC_HS, SD_HS, MMC_HS_52, MMC_DDR_52, UHS_SDR12, UHS_SDR25, UHS_SDR50, UHS_DDR50, UHS_SDR104, MMC_HS_200, MMC_HS_400, MMC_HS_400_ES, MMC_MODES_END };
in this file. Thats what the mmc core uses to represent timing. Please use the same symbols.
The enum and macro differ in values. For example UHS_SDR12 macro value is 3 whereas enum will be 5. This is a problem when accessing below arrays. I take these reference values from linux driver. If the values change in future, it will be easy for u-boot driver to just copy and paste from linux driver if we use macro's.
/* Default settings for ZynqMP Clock Phases */ const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63, 0, 0, 183, 54, 0, 0}; const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0};
I also see that xenon_sdhci.c has defined these macro's locally. https://gitlab.denx.de/u-boot/u-boot/-/blob/master/drivers/mmc/xenon_sdhci.c...
So I have added these macro's in include/mmc.h for everyone's use.
A better approach would be to try to bring the U-boot macro in line with kernel. That way more drivers can take advantage of the similarities. Adding extra symbols just confuses people about which ones are being used in core and which ones in your driver.
Thanks, Faiz

On 6/10/20 6:18 PM, Ashok Reddy Soma wrote:
Hi Faiz,
-----Original Message----- From: Faiz Abbas faiz_abbas@ti.com Sent: Wednesday, May 27, 2020 12:28 PM To: Jaehoon Chung jh80.chung@samsung.com; Michal Simek michals@xilinx.com; u-boot@lists.denx.de; git git@xilinx.com Cc: Ashok Reddy Soma ashokred@xilinx.com; Heinrich Schuchardt xypron.glpk@gmx.de; Lokesh Vutla lokeshvutla@ti.com; Marek Vasut marek.vasut@gmail.com; Masahiro Yamada masahiroy@kernel.org; Peng Fan peng.fan@nxp.com; Sam Protsenko semen.protsenko@linaro.org; Simon Glass sjg@chromium.org; Yann Gautier yann.gautier@st.com Subject: Re: [PATCH 2/7] mmc: zynq_sdhci: Define timing macro's
Michal,
On 27/05/20 12:17 pm, Jaehoon Chung wrote:
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Even though it's similar to linux, does it need to add new macro?
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5
static const u8 mode2timing[] = {
- [MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
- [MMC_HS] = HIGH_SPEED_BUS_SPEED,
- [SD_HS] = HIGH_SPEED_BUS_SPEED,
- [MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
- [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED,
- [UHS_SDR12] = UHS_SDR12_BUS_SPEED,
- [UHS_SDR25] = UHS_SDR25_BUS_SPEED,
- [UHS_SDR50] = UHS_SDR50_BUS_SPEED,
- [UHS_DDR50] = UHS_DDR50_BUS_SPEED,
- [UHS_SDR104] = UHS_SDR104_BUS_SPEED,
- [MMC_HS_200] = MMC_HS200_BUS_SPEED,
- [MMC_LEGACY] = MMC_TIMING_LEGACY,
- [MMC_HS] = MMC_TIMING_MMC_HS,
- [SD_HS] = MMC_TIMING_SD_HS,
- [MMC_HS_52] = MMC_TIMING_UHS_SDR50,
- [MMC_DDR_52] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR12] = MMC_TIMING_UHS_SDR12,
- [UHS_SDR25] = MMC_TIMING_UHS_SDR25,
- [UHS_SDR50] = MMC_TIMING_UHS_SDR50,
- [UHS_DDR50] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR104] = MMC_TIMING_UHS_SDR104,
- [MMC_HS_200] = MMC_TIMING_MMC_HS200,
};
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10
/* Driver model support */
There's already an
enum bus_mode { MMC_LEGACY, MMC_HS, SD_HS, MMC_HS_52, MMC_DDR_52, UHS_SDR12, UHS_SDR25, UHS_SDR50, UHS_DDR50, UHS_SDR104, MMC_HS_200, MMC_HS_400, MMC_HS_400_ES, MMC_MODES_END };
in this file. Thats what the mmc core uses to represent timing. Please use the same symbols.
The enum and macro differ in values. For example UHS_SDR12 macro value is 3 whereas enum will be 5. This is a problem when accessing below arrays. I take these reference values from linux driver. If the values change in future, it will be easy for u-boot driver to just copy and paste from linux driver if we use macro's.
enum bus_mode is a specification which mode is. MMc_TIMING_xxx is a specification which timing is. As i know, its meaning is a little different. But it's not reason that can't use it. *specification* is using to make more readable. I don't have any objection to add or not. But i don't agree about copy and paste everything from linux driver.
Anyway, I don't know why you have to fix array value.
const u32 zynqmap_iclk_phases[] = {
[UHS_SDR12] = 0, ... }
Is it impossible to assign to proper value into array?
/* Default settings for ZynqMP Clock Phases */ const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63, 0, 0, 183, 54, 0, 0}; const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0};
I also see that xenon_sdhci.c has defined these macro's locally. https://protect2.fireeye.com/url?k=34df76b5-69117766-34defdfa-000babff317b-0...
So I have added these macro's in include/mmc.h for everyone's use.
If you want to use this for everyone, i think that it needs to change subject. Your subject looks like adding it only for zynq_sdhci.
I think Peng also has his opinion. let's wait for his opinion.
Best Regards, Jaehoon Chung
Thanks, Faiz
Thanks, Ashok

Hi Jaehoon,
-----Original Message----- From: Jaehoon Chung jh80.chung@samsung.com Sent: Wednesday, June 10, 2020 4:07 PM To: Ashok Reddy Soma ashokred@xilinx.com; Faiz Abbas faiz_abbas@ti.com; Michal Simek michals@xilinx.com; u- boot@lists.denx.de; git git@xilinx.com Cc: Heinrich Schuchardt xypron.glpk@gmx.de; Lokesh Vutla lokeshvutla@ti.com; Marek Vasut marek.vasut@gmail.com; Masahiro Yamada masahiroy@kernel.org; Peng Fan peng.fan@nxp.com; Sam Protsenko semen.protsenko@linaro.org; Simon Glass sjg@chromium.org; Yann Gautier yann.gautier@st.com Subject: Re: [PATCH 2/7] mmc: zynq_sdhci: Define timing macro's
On 6/10/20 6:18 PM, Ashok Reddy Soma wrote:
Hi Faiz,
-----Original Message----- From: Faiz Abbas faiz_abbas@ti.com Sent: Wednesday, May 27, 2020 12:28 PM To: Jaehoon Chung jh80.chung@samsung.com; Michal Simek michals@xilinx.com; u-boot@lists.denx.de; git git@xilinx.com Cc: Ashok Reddy Soma ashokred@xilinx.com; Heinrich Schuchardt xypron.glpk@gmx.de; Lokesh Vutla lokeshvutla@ti.com; Marek Vasut marek.vasut@gmail.com; Masahiro Yamada masahiroy@kernel.org;
Peng
Fan peng.fan@nxp.com; Sam Protsenko
Simon Glass sjg@chromium.org; Yann Gautier yann.gautier@st.com Subject: Re: [PATCH 2/7] mmc: zynq_sdhci: Define timing macro's
Michal,
On 27/05/20 12:17 pm, Jaehoon Chung wrote:
On 5/22/20 7:44 PM, Michal Simek wrote:
From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define timing macro's for all the available speeds of mmc. This is done similar to linux. Replace other macro's used in zynq_sdhci.c with these new macro's.
Even though it's similar to linux, does it need to add new macro?
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/mmc/zynq_sdhci.c | 24 +++++++++++------------- include/mmc.h | 13 +++++++++++++ 2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 94c69cf1c1bd..02583f76f936 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -32,20 +32,18 @@ struct arasan_sdhci_priv { };
#if defined(CONFIG_ARCH_ZYNQMP) -#define MMC_HS200_BUS_SPEED 5
static const u8 mode2timing[] = {
- [MMC_LEGACY] = UHS_SDR12_BUS_SPEED,
- [MMC_HS] = HIGH_SPEED_BUS_SPEED,
- [SD_HS] = HIGH_SPEED_BUS_SPEED,
- [MMC_HS_52] = HIGH_SPEED_BUS_SPEED,
- [MMC_DDR_52] = HIGH_SPEED_BUS_SPEED,
- [UHS_SDR12] = UHS_SDR12_BUS_SPEED,
- [UHS_SDR25] = UHS_SDR25_BUS_SPEED,
- [UHS_SDR50] = UHS_SDR50_BUS_SPEED,
- [UHS_DDR50] = UHS_DDR50_BUS_SPEED,
- [UHS_SDR104] = UHS_SDR104_BUS_SPEED,
- [MMC_HS_200] = MMC_HS200_BUS_SPEED,
- [MMC_LEGACY] = MMC_TIMING_LEGACY,
- [MMC_HS] = MMC_TIMING_MMC_HS,
- [SD_HS] = MMC_TIMING_SD_HS,
- [MMC_HS_52] = MMC_TIMING_UHS_SDR50,
- [MMC_DDR_52] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR12] = MMC_TIMING_UHS_SDR12,
- [UHS_SDR25] = MMC_TIMING_UHS_SDR25,
- [UHS_SDR50] = MMC_TIMING_UHS_SDR50,
- [UHS_DDR50] = MMC_TIMING_UHS_DDR50,
- [UHS_SDR104] = MMC_TIMING_UHS_SDR104,
- [MMC_HS_200] = MMC_TIMING_MMC_HS200,
};
#define SDHCI_TUNING_LOOP_COUNT 40 diff --git a/include/mmc.h b/include/mmc.h index 82562193cc48..05d8ab8eeac6 100644 --- a/include/mmc.h +++ b/include/mmc.h @@ -360,6 +360,19 @@ enum mmc_voltage { #define MMC_NUM_BOOT_PARTITION 2 #define MMC_PART_RPMB 3 /* RPMB partition number */
+/* timing specification used */ +#define MMC_TIMING_LEGACY 0 +#define MMC_TIMING_MMC_HS 1 +#define MMC_TIMING_SD_HS 2 +#define MMC_TIMING_UHS_SDR12 3 +#define MMC_TIMING_UHS_SDR25 4 +#define MMC_TIMING_UHS_SDR50 5 +#define MMC_TIMING_UHS_SDR104 6 +#define MMC_TIMING_UHS_DDR50 7 +#define MMC_TIMING_MMC_DDR52 8 +#define MMC_TIMING_MMC_HS200 9 +#define MMC_TIMING_MMC_HS400 10
/* Driver model support */
There's already an
enum bus_mode { MMC_LEGACY, MMC_HS, SD_HS, MMC_HS_52, MMC_DDR_52, UHS_SDR12, UHS_SDR25, UHS_SDR50, UHS_DDR50, UHS_SDR104, MMC_HS_200, MMC_HS_400, MMC_HS_400_ES, MMC_MODES_END };
in this file. Thats what the mmc core uses to represent timing. Please use the same symbols.
The enum and macro differ in values. For example UHS_SDR12 macro value
is 3 whereas enum will be 5.
This is a problem when accessing below arrays. I take these reference
values from linux driver.
If the values change in future, it will be easy for u-boot driver to just copy
and paste from linux driver if we use macro's.
enum bus_mode is a specification which mode is. MMc_TIMING_xxx is a specification which timing is.
Correct, these timing macro's are being used for setting phase delays. That was the background.
As i know, its meaning is a little different. But it's not reason that can't use it. *specification* is using to make more readable. I don't have any objection to add or not. But i don't agree about copy and paste everything from linux driver.
Anyway, I don't know why you have to fix array value.
const u32 zynqmap_iclk_phases[] = {
[UHS_SDR12] = 0, ... }
Is it impossible to assign to proper value into array?
It can be done, I just quoted a example.
/* Default settings for ZynqMP Clock Phases */ const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63, 0, 0, 183, 54, 0, 0}; const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0};
I also see that xenon_sdhci.c has defined these macro's locally. https://protect2.fireeye.com/url?k=34df76b5-69117766-34defdfa-000babff 317b-0937b081dc4534f6&q=1&u=https%3A%2F%2Fgitlab.denx.de%2Fu-
boot%2Fu-
boot%2F-%2Fblob%2Fmaster%2Fdrivers%2Fmmc%2Fxenon_sdhci.c%23L98
So I have added these macro's in include/mmc.h for everyone's use.
If you want to use this for everyone, i think that it needs to change subject. Your subject looks like adding it only for zynq_sdhci.
Sure, I will change the subject and send v2.
I think Peng also has his opinion. let's wait for his opinion.
Sure.
Best Regards, Jaehoon Chung
Thanks, Faiz
Thanks, Ashok

Just group macros below headers. Other patches will be using this location too.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/zynq_sdhci.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 02583f76f936..1c83aab84021 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -19,6 +19,8 @@ #include <sdhci.h> #include <zynqmp_tap_delay.h>
+#define SDHCI_TUNING_LOOP_COUNT 40 + struct arasan_sdhci_plat { struct mmc_config cfg; struct mmc mmc; @@ -46,8 +48,6 @@ static const u8 mode2timing[] = { [MMC_HS_200] = MMC_TIMING_MMC_HS200, };
-#define SDHCI_TUNING_LOOP_COUNT 40 - static void arasan_zynqmp_dll_reset(struct sdhci_host *host, u8 deviceid) { u16 clk;

Define input and output clock phase delays with pre-defined values.
Define arasan_sdhci_clk_data type structure and add it to priv structure and store these clock phase delays in it.
Read input and output clock phase delays from dt. If these values are not passed through dt, use pre-defined values.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/zynq_sdhci.c | 85 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 1c83aab84021..d3a32c98aa35 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -20,6 +20,12 @@ #include <zynqmp_tap_delay.h>
#define SDHCI_TUNING_LOOP_COUNT 40 +#define MMC_BANK2 0x2 + +struct arasan_sdhci_clk_data { + int clk_phase_in[MMC_TIMING_MMC_HS400 + 1]; + int clk_phase_out[MMC_TIMING_MMC_HS400 + 1]; +};
struct arasan_sdhci_plat { struct mmc_config cfg; @@ -28,12 +34,17 @@ struct arasan_sdhci_plat {
struct arasan_sdhci_priv { struct sdhci_host *host; + struct arasan_sdhci_clk_data clk_data; u8 deviceid; u8 bank; u8 no_1p8; };
#if defined(CONFIG_ARCH_ZYNQMP) +/* Default settings for ZynqMP Clock Phases */ +const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63, 0, 0, 183, 54, 0, 0}; +const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0}; + static const u8 mode2timing[] = { [MMC_LEGACY] = MMC_TIMING_LEGACY, [MMC_HS] = MMC_TIMING_MMC_HS, @@ -168,6 +179,79 @@ static void arasan_sdhci_set_tapdelay(struct sdhci_host *host) priv->bank); }
+static void arasan_dt_read_clk_phase(struct udevice *dev, unsigned char timing, + const char *prop) +{ + struct arasan_sdhci_priv *priv = dev_get_priv(dev); + struct arasan_sdhci_clk_data *clk_data = &priv->clk_data; + u32 clk_phase[2] = {0}; + + /* + * Read Tap Delay values from DT, if the DT does not contain the + * Tap Values then use the pre-defined values + */ + if (dev_read_u32_array(dev, prop, &clk_phase[0], 2)) { + 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]); + 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]; +} + +/** + * arasan_dt_parse_clk_phases - Read Tap Delay values from DT + * + * Called at initialization to parse the values of Tap Delays. + * + * @dev: Pointer to our struct udevice. + */ +static void arasan_dt_parse_clk_phases(struct udevice *dev) +{ + struct arasan_sdhci_priv *priv = dev_get_priv(dev); + struct arasan_sdhci_clk_data *clk_data = &priv->clk_data; + int i; + + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) && + device_is_compatible(dev, "xlnx,zynqmp-8.9a")) { + for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) { + clk_data->clk_phase_in[i] = zynqmp_iclk_phases[i]; + clk_data->clk_phase_out[i] = zynqmp_oclk_phases[i]; + } + + if (priv->bank == MMC_BANK2) { + clk_data->clk_phase_out[MMC_TIMING_UHS_SDR104] = 90; + clk_data->clk_phase_out[MMC_TIMING_MMC_HS200] = 90; + } + } + + arasan_dt_read_clk_phase(dev, MMC_TIMING_LEGACY, + "clk-phase-legacy"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_MMC_HS, + "clk-phase-mmc-hs"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_SD_HS, + "clk-phase-sd-hs"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_UHS_SDR12, + "clk-phase-uhs-sdr12"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_UHS_SDR25, + "clk-phase-uhs-sdr25"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_UHS_SDR50, + "clk-phase-uhs-sdr50"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_UHS_SDR104, + "clk-phase-uhs-sdr104"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_UHS_DDR50, + "clk-phase-uhs-ddr50"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_MMC_DDR52, + "clk-phase-mmc-ddr52"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_MMC_HS200, + "clk-phase-mmc-hs200"); + arasan_dt_read_clk_phase(dev, MMC_TIMING_MMC_HS400, + "clk-phase-mmc-hs400"); +} + static void arasan_sdhci_set_control_reg(struct sdhci_host *host) { struct mmc *mmc = (struct mmc *)host->mmc; @@ -271,6 +355,7 @@ static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
#if defined(CONFIG_ARCH_ZYNQMP) priv->host->ops = &arasan_ops; + arasan_dt_parse_clk_phases(dev); #endif
priv->host->ioaddr = (void *)dev_read_addr(dev);

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define and use functions for setting input and output tapdelays based on clk phase delays.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/zynq_sdhci.c | 128 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 123 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index d3a32c98aa35..1e7b79906f88 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -166,17 +166,135 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode) return 0; }
+/** + * sdhci_zynqmp_sdcardclk_set_phase - Set the SD Output Clock Tap Delays + * + * Set the SD Output Clock Tap Delays for Output path + * + * @host: Pointer to the sdhci_host structure. + * @degrees: The clock phase shift between 0 - 359. + * Return: 0 on success and error value on error + */ +static int sdhci_zynqmp_sdcardclk_set_phase(struct sdhci_host *host, + int degrees) +{ + struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev); + struct mmc *mmc = (struct mmc *)host->mmc; + u8 tap_delay, tap_max = 0; + int ret; + int timing = mode2timing[mmc->selected_mode]; + + /* + * This is applicable for SDHCI_SPEC_300 and above + * ZynqMP does not set phase for <=25MHz clock. + * If degrees is zero, no need to do anything. + */ + if (host->version < SDHCI_SPEC_300 || + timing == MMC_TIMING_LEGACY || + timing == MMC_TIMING_UHS_SDR12 || !degrees) + return 0; + + switch (timing) { + case MMC_TIMING_MMC_HS: + case MMC_TIMING_SD_HS: + case MMC_TIMING_UHS_SDR25: + case MMC_TIMING_UHS_DDR50: + case MMC_TIMING_MMC_DDR52: + /* For 50MHz clock, 30 Taps are available */ + tap_max = 30; + break; + case MMC_TIMING_UHS_SDR50: + /* For 100MHz clock, 15 Taps are available */ + tap_max = 15; + break; + case MMC_TIMING_UHS_SDR104: + case MMC_TIMING_MMC_HS200: + /* For 200MHz clock, 8 Taps are available */ + tap_max = 8; + default: + break; + } + + tap_delay = (degrees * tap_max) / 360; + + arasan_zynqmp_set_tapdelay(priv->deviceid, 0, tap_delay); + + return ret; +} + +/** + * sdhci_zynqmp_sampleclk_set_phase - Set the SD Input Clock Tap Delays + * + * Set the SD Input Clock Tap Delays for Input path + * + * @host: Pointer to the sdhci_host structure. + * @degrees: The clock phase shift between 0 - 359. + * Return: 0 on success and error value on error + */ +static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host, + int degrees) +{ + struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev); + struct mmc *mmc = (struct mmc *)host->mmc; + u8 tap_delay, tap_max = 0; + int ret; + int timing = mode2timing[mmc->selected_mode]; + + /* + * This is applicable for SDHCI_SPEC_300 and above + * ZynqMP does not set phase for <=25MHz clock. + * If degrees is zero, no need to do anything. + */ + if (host->version < SDHCI_SPEC_300 || + timing == MMC_TIMING_LEGACY || + timing == MMC_TIMING_UHS_SDR12 || !degrees) + return 0; + + switch (timing) { + case MMC_TIMING_MMC_HS: + case MMC_TIMING_SD_HS: + case MMC_TIMING_UHS_SDR25: + case MMC_TIMING_UHS_DDR50: + case MMC_TIMING_MMC_DDR52: + /* For 50MHz clock, 120 Taps are available */ + tap_max = 120; + break; + case MMC_TIMING_UHS_SDR50: + /* For 100MHz clock, 60 Taps are available */ + tap_max = 60; + break; + case MMC_TIMING_UHS_SDR104: + case MMC_TIMING_MMC_HS200: + /* For 200MHz clock, 30 Taps are available */ + tap_max = 30; + default: + break; + } + + tap_delay = (degrees * tap_max) / 360; + + arasan_zynqmp_set_tapdelay(priv->deviceid, tap_delay, 0); + + return ret; +} + static void arasan_sdhci_set_tapdelay(struct sdhci_host *host) { struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev); + struct arasan_sdhci_clk_data *clk_data = &priv->clk_data; struct mmc *mmc = (struct mmc *)host->mmc; - u8 uhsmode; + struct udevice *dev = mmc->dev; + u8 timing = mode2timing[mmc->selected_mode]; + u32 iclk_phase = clk_data->clk_phase_in[timing]; + u32 oclk_phase = clk_data->clk_phase_out[timing];
- uhsmode = mode2timing[mmc->selected_mode]; + dev_dbg(dev, "%s, host:%s, mode:%d\n", __func__, host->name, timing);
- if (uhsmode >= UHS_SDR25_BUS_SPEED) - arasan_zynqmp_set_tapdelay(priv->deviceid, uhsmode, - priv->bank); + if (IS_ENABLED(CONFIG_ARCH_ZYNQMP) && + device_is_compatible(dev, "xlnx,zynqmp-8.9a")) { + sdhci_zynqmp_sampleclk_set_phase(host, iclk_phase); + sdhci_zynqmp_sdcardclk_set_phase(host, oclk_phase); + } }
static void arasan_dt_read_clk_phase(struct udevice *dev, unsigned char timing,

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Define default values for input and output clock phase delays for Versal. Also define functions for setting tapdelays based on these clock phase delays.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/zynq_sdhci.c | 160 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 156 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 1e7b79906f88..91ec552119d1 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -19,6 +19,12 @@ #include <sdhci.h> #include <zynqmp_tap_delay.h>
+#define SDHCI_ARASAN_ITAPDLY_REGISTER 0xF0F8 +#define SDHCI_ARASAN_OTAPDLY_REGISTER 0xF0FC +#define SDHCI_ITAPDLY_CHGWIN 0x200 +#define SDHCI_ITAPDLY_ENABLE 0x100 +#define SDHCI_OTAPDLY_ENABLE 0x40 + #define SDHCI_TUNING_LOOP_COUNT 40 #define MMC_BANK2 0x2
@@ -40,11 +46,15 @@ struct arasan_sdhci_priv { u8 no_1p8; };
-#if defined(CONFIG_ARCH_ZYNQMP) +#if defined(CONFIG_ARCH_ZYNQMP) || defined(CONFIG_ARCH_VERSAL) /* Default settings for ZynqMP Clock Phases */ const u32 zynqmp_iclk_phases[] = {0, 63, 63, 0, 63, 0, 0, 183, 54, 0, 0}; const u32 zynqmp_oclk_phases[] = {0, 72, 60, 0, 60, 72, 135, 48, 72, 135, 0};
+/* Default settings for Versal Clock Phases */ +const u32 versal_iclk_phases[] = {0, 132, 132, 0, 132, 0, 0, 162, 90, 0, 0}; +const u32 versal_oclk_phases[] = {0, 60, 48, 0, 48, 72, 90, 36, 60, 90, 0}; + static const u8 mode2timing[] = { [MMC_LEGACY] = MMC_TIMING_LEGACY, [MMC_HS] = MMC_TIMING_MMC_HS, @@ -278,6 +288,138 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct sdhci_host *host, return ret; }
+/** + * sdhci_versal_sdcardclk_set_phase - Set the SD Output Clock Tap Delays + * + * Set the SD Output Clock Tap Delays for Output path + * + * @host: Pointer to the sdhci_host structure. + * @degrees: The clock phase shift between 0 - 359. + * Return: 0 on success and error value on error + */ +static int sdhci_versal_sdcardclk_set_phase(struct sdhci_host *host, + int degrees) +{ + struct mmc *mmc = (struct mmc *)host->mmc; + u8 tap_delay, tap_max = 0; + int ret; + int timing = mode2timing[mmc->selected_mode]; + + /* + * This is applicable for SDHCI_SPEC_300 and above + * Versal does not set phase for <=25MHz clock. + * If degrees is zero, no need to do anything. + */ + if (host->version < SDHCI_SPEC_300 || + timing == MMC_TIMING_LEGACY || + timing == MMC_TIMING_UHS_SDR12 || !degrees) + return 0; + + switch (timing) { + case MMC_TIMING_MMC_HS: + case MMC_TIMING_SD_HS: + case MMC_TIMING_UHS_SDR25: + case MMC_TIMING_UHS_DDR50: + case MMC_TIMING_MMC_DDR52: + /* For 50MHz clock, 30 Taps are available */ + tap_max = 30; + break; + case MMC_TIMING_UHS_SDR50: + /* For 100MHz clock, 15 Taps are available */ + tap_max = 15; + break; + case MMC_TIMING_UHS_SDR104: + case MMC_TIMING_MMC_HS200: + /* For 200MHz clock, 8 Taps are available */ + tap_max = 8; + default: + break; + } + + tap_delay = (degrees * tap_max) / 360; + + /* Set the Clock Phase */ + if (tap_delay) { + u32 regval; + + regval = sdhci_readl(host, SDHCI_ARASAN_OTAPDLY_REGISTER); + regval |= SDHCI_OTAPDLY_ENABLE; + sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER); + regval |= tap_delay; + sdhci_writel(host, regval, SDHCI_ARASAN_OTAPDLY_REGISTER); + } + + return ret; +} + +/** + * sdhci_versal_sampleclk_set_phase - Set the SD Input Clock Tap Delays + * + * Set the SD Input Clock Tap Delays for Input path + * + * @host: Pointer to the sdhci_host structure. + * @degrees: The clock phase shift between 0 - 359. + * Return: 0 on success and error value on error + */ +static int sdhci_versal_sampleclk_set_phase(struct sdhci_host *host, + int degrees) +{ + struct mmc *mmc = (struct mmc *)host->mmc; + u8 tap_delay, tap_max = 0; + int ret; + int timing = mode2timing[mmc->selected_mode]; + + /* + * This is applicable for SDHCI_SPEC_300 and above + * Versal does not set phase for <=25MHz clock. + * If degrees is zero, no need to do anything. + */ + if (host->version < SDHCI_SPEC_300 || + timing == MMC_TIMING_LEGACY || + timing == MMC_TIMING_UHS_SDR12 || !degrees) + return 0; + + switch (timing) { + case MMC_TIMING_MMC_HS: + case MMC_TIMING_SD_HS: + case MMC_TIMING_UHS_SDR25: + case MMC_TIMING_UHS_DDR50: + case MMC_TIMING_MMC_DDR52: + /* For 50MHz clock, 120 Taps are available */ + tap_max = 120; + break; + case MMC_TIMING_UHS_SDR50: + /* For 100MHz clock, 60 Taps are available */ + tap_max = 60; + break; + case MMC_TIMING_UHS_SDR104: + case MMC_TIMING_MMC_HS200: + /* For 200MHz clock, 30 Taps are available */ + tap_max = 30; + default: + break; + } + + tap_delay = (degrees * tap_max) / 360; + + /* Set the Clock Phase */ + if (tap_delay) { + u32 regval; + + regval = sdhci_readl(host, SDHCI_ARASAN_ITAPDLY_REGISTER); + regval |= SDHCI_ITAPDLY_CHGWIN; + sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER); + regval |= SDHCI_ITAPDLY_ENABLE; + sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER); + regval |= tap_delay; + sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER); + regval &= ~SDHCI_ITAPDLY_CHGWIN; + sdhci_writel(host, regval, SDHCI_ARASAN_ITAPDLY_REGISTER); + } + + return ret; +} + static void arasan_sdhci_set_tapdelay(struct sdhci_host *host) { struct arasan_sdhci_priv *priv = dev_get_priv(host->mmc->dev); @@ -294,6 +436,10 @@ static void arasan_sdhci_set_tapdelay(struct sdhci_host *host) device_is_compatible(dev, "xlnx,zynqmp-8.9a")) { sdhci_zynqmp_sampleclk_set_phase(host, iclk_phase); sdhci_zynqmp_sdcardclk_set_phase(host, oclk_phase); + } else if (IS_ENABLED(CONFIG_ARCH_VERSAL) && + device_is_compatible(dev, "xlnx,versal-8.9a")) { + sdhci_versal_sampleclk_set_phase(host, iclk_phase); + sdhci_versal_sdcardclk_set_phase(host, oclk_phase); } }
@@ -346,6 +492,14 @@ static void arasan_dt_parse_clk_phases(struct udevice *dev) } }
+ if (IS_ENABLED(CONFIG_ARCH_VERSAL) && + device_is_compatible(dev, "xlnx,versal-8.9a")) { + for (i = 0; i <= MMC_TIMING_MMC_HS400; i++) { + clk_data->clk_phase_in[i] = versal_iclk_phases[i]; + clk_data->clk_phase_out[i] = versal_oclk_phases[i]; + } + } + arasan_dt_read_clk_phase(dev, MMC_TIMING_LEGACY, "clk-phase-legacy"); arasan_dt_read_clk_phase(dev, MMC_TIMING_MMC_HS, @@ -388,9 +542,7 @@ static void arasan_sdhci_set_control_reg(struct sdhci_host *host) mmc->selected_mode <= UHS_DDR50) sdhci_set_uhs_timing(host); } -#endif
-#if defined(CONFIG_ARCH_ZYNQMP) const struct sdhci_ops arasan_ops = { .platform_execute_tuning = &arasan_sdhci_execute_tuning, .set_delay = &arasan_sdhci_set_tapdelay, @@ -471,7 +623,7 @@ static int arasan_sdhci_ofdata_to_platdata(struct udevice *dev)
priv->host->name = dev->name;
-#if defined(CONFIG_ARCH_ZYNQMP) +#if defined(CONFIG_ARCH_ZYNQMP) || defined(CONFIG_ARCH_VERSAL) priv->host->ops = &arasan_ops; arasan_dt_parse_clk_phases(dev); #endif

From: Ashok Reddy Soma ashok.reddy.soma@xilinx.com
Fix the condition to set UHS timings for speeds upto HS200.
Signed-off-by: Ashok Reddy Soma ashok.reddy.soma@xilinx.com Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/mmc/zynq_sdhci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index 91ec552119d1..55762a13395c 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -539,7 +539,7 @@ static void arasan_sdhci_set_control_reg(struct sdhci_host *host) }
if (mmc->selected_mode > SD_HS && - mmc->selected_mode <= UHS_DDR50) + mmc->selected_mode <= MMC_HS_200) sdhci_set_uhs_timing(host); }
participants (4)
-
Ashok Reddy Soma
-
Faiz Abbas
-
Jaehoon Chung
-
Michal Simek