On 8/3/21 2:20 PM, Ashok Reddy Soma wrote:
> Currently xilinx sdhci driver is using zynqmp_mmio_write() to set
> tapdelay values and DLL resets. Continue to use this for SPL and mini
> U-Boot where U-Boot will be executed at EL3 level.
>
> Use firmware call xilinx_pm_request() using appropriate arguments to
> set input/output tapdelays and also for DLL resets in regular flow(EL2).
>
> Host driver should explicitly request DLL reset before ITAP (assert DLL)
> and after OTAP (release DLL) to avoid issues in some cases. Also handle
> error return where possible.
>
> Signed-off-by: T Karthik Reddy <t.karthik.re...@xilinx.com>
> Signed-off-by: Ashok Reddy Soma <ashok.reddy.s...@xilinx.com>
Reviewed-by: Jaehoon Chung <jh80.ch...@samsung.com>
Best Regards,
Jaehoon Chung
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - As we are seeing issues with SPL flow, keep zynqmp_mmio_write method
> to set tapdelay's and DLL resets. Add xilinx_pm_request on top of it
> for regular flow.
> - Changed the patch title to reflect these changes
>
> Changes in v2:
> - Added comment for why 1ms delay is needed between DLL assert and
> release
> - Remove mmc->dev->seq_ and use priv->deviceid instead
>
> board/xilinx/zynqmp/tap_delays.c | 148 +++++++++++++++++--------------
> drivers/mmc/zynq_sdhci.c | 73 ++++++++++++---
> include/zynqmp_tap_delay.h | 25 ++++--
> 3 files changed, 159 insertions(+), 87 deletions(-)
>
> diff --git a/board/xilinx/zynqmp/tap_delays.c
> b/board/xilinx/zynqmp/tap_delays.c
> index d16bbb8eff..514f86a29a 100644
> --- a/board/xilinx/zynqmp/tap_delays.c
> +++ b/board/xilinx/zynqmp/tap_delays.c
> @@ -8,94 +8,108 @@
> #include <common.h>
> #include <zynqmp_tap_delay.h>
> #include <asm/arch/sys_proto.h>
> +#include <asm/cache.h>
> #include <linux/delay.h>
> #include <mmc.h>
> +#include <zynqmp_firmware.h>
>
> #define SD_DLL_CTRL 0xFF180358
> #define SD_ITAP_DLY 0xFF180314
> #define SD_OTAP_DLY 0xFF180318
> -#define SD0_DLL_RST_MASK 0x00000004
> -#define SD0_DLL_RST 0x00000004
> -#define SD1_DLL_RST_MASK 0x00040000
> -#define SD1_DLL_RST 0x00040000
> -#define SD0_ITAPCHGWIN_MASK 0x00000200
> -#define SD0_ITAPCHGWIN 0x00000200
> -#define SD1_ITAPCHGWIN_MASK 0x02000000
> -#define SD1_ITAPCHGWIN 0x02000000
> -#define SD0_ITAPDLYENA_MASK 0x00000100
> -#define SD0_ITAPDLYENA 0x00000100
> -#define SD1_ITAPDLYENA_MASK 0x01000000
> -#define SD1_ITAPDLYENA 0x01000000
> -#define SD0_ITAPDLYSEL_MASK 0x000000FF
> -#define SD1_ITAPDLYSEL_MASK 0x00FF0000
> -#define SD0_OTAPDLYSEL_MASK 0x0000003F
> -#define SD1_OTAPDLYSEL_MASK 0x003F0000
> +#define SD0_DLL_RST BIT(2)
> +#define SD1_DLL_RST BIT(18)
> +#define SD0_ITAPCHGWIN BIT(9)
> +#define SD1_ITAPCHGWIN BIT(25)
> +#define SD0_ITAPDLYENA BIT(8)
> +#define SD1_ITAPDLYENA BIT(24)
> +#define SD0_ITAPDLYSEL_MASK GENMASK(7, 0)
> +#define SD1_ITAPDLYSEL_MASK GENMASK(23, 16)
> +#define SD0_OTAPDLYSEL_MASK GENMASK(5, 0)
> +#define SD1_OTAPDLYSEL_MASK GENMASK(21, 16)
>
> -void zynqmp_dll_reset(u8 deviceid)
> +int zynqmp_dll_reset(u8 node_id, u32 type)
> {
> - /* Issue DLL Reset */
> - if (deviceid == 0)
> - zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK,
> - SD0_DLL_RST);
> - else
> - zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK,
> - SD1_DLL_RST);
> + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> + if (node_id == NODE_SD_0)
> + return zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST,
> + type == PM_DLL_RESET_ASSERT ?
> + SD0_DLL_RST : 0);
>
> - mdelay(1);
> -
> - /* Release DLL Reset */
> - if (deviceid == 0)
> - zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, 0x0);
> - else
> - zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, 0x0);
> + return zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST,
> + type == PM_DLL_RESET_ASSERT ?
> + SD1_DLL_RST : 0);
> + } else {
> + return xilinx_pm_request(PM_IOCTL, (u32)node_id,
> + IOCTL_SD_DLL_RESET, type, 0, NULL);
> + }
> }
>
> -void arasan_zynqmp_set_in_tapdelay(u8 deviceid, u32 itap_delay)
> +int arasan_zynqmp_set_in_tapdelay(u8 node_id, u32 itap_delay)
> {
> - if (deviceid == 0) {
> - zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, SD0_DLL_RST);
> + int ret;
>
> - /* Program ITAP delay */
> - zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN_MASK,
> - SD0_ITAPCHGWIN);
> - zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA_MASK,
> - SD0_ITAPDLYENA);
> - zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYSEL_MASK, itap_delay);
> - zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN_MASK, 0x0);
> + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> + if (node_id == NODE_SD_0) {
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN,
> + SD0_ITAPCHGWIN);
> + if (ret)
> + return ret;
>
> - zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, 0x0);
> - } else {
> - zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, SD1_DLL_RST);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPDLYENA,
> + SD0_ITAPDLYENA);
> + if (ret)
> + return ret;
>
> - /* Program ITAP delay */
> - zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN_MASK,
> - SD1_ITAPCHGWIN);
> - zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA_MASK,
> - SD1_ITAPDLYENA);
> - zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
> - (itap_delay << 16));
> - zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN_MASK, 0x0);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY,
> + SD0_ITAPDLYSEL_MASK,
> + itap_delay);
> + if (ret)
> + return ret;
>
> - zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, 0x0);
> - }
> -}
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD0_ITAPCHGWIN, 0);
> + if (ret)
> + return ret;
> + }
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN,
> + SD1_ITAPCHGWIN);
> + if (ret)
> + return ret;
>
> -void arasan_zynqmp_set_out_tapdelay(u8 deviceid, u32 otap_delay)
> -{
> - if (deviceid == 0) {
> - zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, SD0_DLL_RST);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYENA,
> + SD1_ITAPDLYENA);
> + if (ret)
> + return ret;
>
> - /* Program OTAP delay */
> - zynqmp_mmio_write(SD_OTAP_DLY, SD0_OTAPDLYSEL_MASK, otap_delay);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPDLYSEL_MASK,
> + (itap_delay << 16));
> + if (ret)
> + return ret;
>
> - zynqmp_mmio_write(SD_DLL_CTRL, SD0_DLL_RST_MASK, 0x0);
> + ret = zynqmp_mmio_write(SD_ITAP_DLY, SD1_ITAPCHGWIN, 0);
> + if (ret)
> + return ret;
> } else {
> - zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, SD1_DLL_RST);
> + return xilinx_pm_request(PM_IOCTL, (u32)node_id,
> + IOCTL_SET_SD_TAPDELAY,
> + PM_TAPDELAY_INPUT, itap_delay, NULL);
> + }
> +
> + return 0;
> +}
>
> - /* Program OTAP delay */
> - zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
> - (otap_delay << 16));
> +int arasan_zynqmp_set_out_tapdelay(u8 node_id, u32 otap_delay)
> +{
> + if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) {
> + if (node_id == NODE_SD_0)
> + return zynqmp_mmio_write(SD_OTAP_DLY,
> + SD0_OTAPDLYSEL_MASK,
> + otap_delay);
>
> - zynqmp_mmio_write(SD_DLL_CTRL, SD1_DLL_RST_MASK, 0x0);
> + return zynqmp_mmio_write(SD_OTAP_DLY, SD1_OTAPDLYSEL_MASK,
> + (otap_delay << 16));
> + } else {
> + return xilinx_pm_request(PM_IOCTL, (u32)node_id,
> + IOCTL_SET_SD_TAPDELAY,
> + PM_TAPDELAY_OUTPUT, otap_delay, NULL);
> }
> }
> diff --git a/drivers/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c
> index 1ecc2ec669..8397f24255 100644
> --- a/drivers/mmc/zynq_sdhci.c
> +++ b/drivers/mmc/zynq_sdhci.c
> @@ -18,6 +18,7 @@
> #include <malloc.h>
> #include <sdhci.h>
> #include <zynqmp_tap_delay.h>
> +#include <zynqmp_firmware.h>
>
> #define SDHCI_ARASAN_ITAPDLY_REGISTER 0xF0F8
> #define SDHCI_ARASAN_ITAPDLY_SEL_MASK GENMASK(7, 0)
> @@ -75,26 +76,40 @@ static const u8 mode2timing[] = {
> [MMC_HS_200] = MMC_TIMING_MMC_HS200,
> };
>
> -static void arasan_zynqmp_dll_reset(struct sdhci_host *host, u8 deviceid)
> +static int arasan_zynqmp_dll_reset(struct sdhci_host *host, u8 node_id)
> {
> - u16 clk;
> + struct mmc *mmc = (struct mmc *)host->mmc;
> + struct udevice *dev = mmc->dev;
> unsigned long timeout;
> + int ret;
> + u16 clk;
>
> clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL);
> clk &= ~(SDHCI_CLOCK_CARD_EN);
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
>
> /* Issue DLL Reset */
> - zynqmp_dll_reset(deviceid);
> + ret = zynqmp_dll_reset(node_id, PM_DLL_RESET_ASSERT);
> + if (ret) {
> + dev_err(dev, "dll_reset assert failed with err: %d\n", ret);
> + return ret;
> + }
> +
> + /* Allow atleast 1ms delay for proper DLL reset */
> + mdelay(1);
> + ret = zynqmp_dll_reset(node_id, PM_DLL_RESET_RELEASE);
> + if (ret) {
> + dev_err(dev, "dll_reset release failed with err: %d\n", ret);
> + return ret;
> + }
>
> /* Wait max 20 ms */
> timeout = 100;
> while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> & SDHCI_CLOCK_INT_STABLE)) {
> if (timeout == 0) {
> - dev_err(mmc_dev(host->mmc),
> - ": Internal clock never stabilised.\n");
> - return;
> + dev_err(dev, ": Internal clock never stabilised.\n");
> + return -EBUSY;
> }
> timeout--;
> udelay(1000);
> @@ -102,6 +117,8 @@ static void arasan_zynqmp_dll_reset(struct sdhci_host
> *host, u8 deviceid)
>
> clk |= SDHCI_CLOCK_CARD_EN;
> sdhci_writew(host, clk, SDHCI_CLOCK_CONTROL);
> +
> + return 0;
> }
>
> static int arasan_sdhci_execute_tuning(struct mmc *mmc, u8 opcode)
> @@ -112,12 +129,11 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc,
> u8 opcode)
> struct sdhci_host *host;
> struct arasan_sdhci_priv *priv = dev_get_priv(mmc->dev);
> char tuning_loop_counter = SDHCI_TUNING_LOOP_COUNT;
> - u8 deviceid;
> + u8 node_id = priv->deviceid ? NODE_SD_1 : NODE_SD_0;
>
> debug("%s\n", __func__);
>
> host = priv->host;
> - deviceid = priv->deviceid;
>
> ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> ctrl |= SDHCI_CTRL_EXEC_TUNING;
> @@ -125,7 +141,7 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc,
> u8 opcode)
>
> mdelay(1);
>
> - arasan_zynqmp_dll_reset(host, deviceid);
> + arasan_zynqmp_dll_reset(host, node_id);
>
> sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_INT_ENABLE);
> sdhci_writel(host, SDHCI_INT_DATA_AVAIL, SDHCI_SIGNAL_ENABLE);
> @@ -171,7 +187,7 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc,
> u8 opcode)
> }
>
> udelay(1);
> - arasan_zynqmp_dll_reset(host, deviceid);
> + arasan_zynqmp_dll_reset(host, node_id);
>
> /* Enable only interrupts served by the SD controller */
> sdhci_writel(host, SDHCI_INT_DATA_MASK | SDHCI_INT_CMD_MASK,
> @@ -194,10 +210,13 @@ static int arasan_sdhci_execute_tuning(struct mmc *mmc,
> u8 opcode)
> 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;
> + struct udevice *dev = mmc->dev;
> + struct arasan_sdhci_priv *priv = dev_get_priv(mmc->dev);
> + u8 node_id = priv->deviceid ? NODE_SD_1 : NODE_SD_0;
> u8 tap_delay, tap_max = 0;
> int timing = mode2timing[mmc->selected_mode];
> + int ret;
>
> /*
> * This is applicable for SDHCI_SPEC_300 and above
> @@ -233,7 +252,19 @@ static int sdhci_zynqmp_sdcardclk_set_phase(struct
> sdhci_host *host,
> /* Limit output tap_delay value to 6 bits */
> tap_delay &= SDHCI_ARASAN_OTAPDLY_SEL_MASK;
>
> - arasan_zynqmp_set_out_tapdelay(priv->deviceid, tap_delay);
> + /* Set the Clock Phase */
> + ret = arasan_zynqmp_set_out_tapdelay(node_id, tap_delay);
> + if (ret) {
> + dev_err(dev, "Error setting output Tap Delay\n");
> + return ret;
> + }
> +
> + /* Release DLL Reset */
> + ret = zynqmp_dll_reset(node_id, PM_DLL_RESET_RELEASE);
> + if (ret) {
> + dev_err(dev, "dll_reset release failed with err: %d\n", ret);
> + return ret;
> + }
>
> return 0;
> }
> @@ -250,10 +281,13 @@ static int sdhci_zynqmp_sdcardclk_set_phase(struct
> sdhci_host *host,
> 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;
> + struct udevice *dev = mmc->dev;
> + struct arasan_sdhci_priv *priv = dev_get_priv(mmc->dev);
> + u8 node_id = priv->deviceid ? NODE_SD_1 : NODE_SD_0;
> u8 tap_delay, tap_max = 0;
> int timing = mode2timing[mmc->selected_mode];
> + int ret;
>
> /*
> * This is applicable for SDHCI_SPEC_300 and above
> @@ -263,6 +297,13 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct
> sdhci_host *host,
> if (SDHCI_GET_VERSION(host) < SDHCI_SPEC_300)
> return 0;
>
> + /* Assert DLL Reset */
> + ret = zynqmp_dll_reset(node_id, PM_DLL_RESET_ASSERT);
> + if (ret) {
> + dev_err(dev, "dll_reset assert failed with err: %d\n", ret);
> + return ret;
> + }
> +
> switch (timing) {
> case MMC_TIMING_MMC_HS:
> case MMC_TIMING_SD_HS:
> @@ -289,7 +330,11 @@ static int sdhci_zynqmp_sampleclk_set_phase(struct
> sdhci_host *host,
> /* Limit input tap_delay value to 8 bits */
> tap_delay &= SDHCI_ARASAN_ITAPDLY_SEL_MASK;
>
> - arasan_zynqmp_set_in_tapdelay(priv->deviceid, tap_delay);
> + ret = arasan_zynqmp_set_in_tapdelay(node_id, tap_delay);
> + if (ret) {
> + dev_err(dev, "Error setting Input Tap Delay\n");
> + return ret;
> + }
>
> return 0;
> }
> diff --git a/include/zynqmp_tap_delay.h b/include/zynqmp_tap_delay.h
> index 1c1e3e7dee..7ef172f0fa 100644
> --- a/include/zynqmp_tap_delay.h
> +++ b/include/zynqmp_tap_delay.h
> @@ -8,14 +8,27 @@
> #ifndef __ZYNQMP_TAP_DELAY_H__
> #define __ZYNQMP_TAP_DELAY_H__
>
> +#include <zynqmp_firmware.h>
> +
> #ifdef CONFIG_ARCH_ZYNQMP
> -void zynqmp_dll_reset(u8 deviceid);
> -void arasan_zynqmp_set_in_tapdelay(u8 device_id, u32 itap_delay);
> -void arasan_zynqmp_set_out_tapdelay(u8 device_id, u32 otap_delay);
> +int zynqmp_dll_reset(u8 node_id, u32 type);
> +int arasan_zynqmp_set_in_tapdelay(u8 device_id, u32 itap_delay);
> +int arasan_zynqmp_set_out_tapdelay(u8 device_id, u32 otap_delay);
> #else
> -inline void zynqmp_dll_reset(u8 deviceid) {}
> -inline void arasan_zynqmp_set_in_tapdelay(u8 device_id, u32 itap_delay) {}
> -inline void arasan_zynqmp_set_out_tapdelay(u8 device_id, u32 otap_delay) {}
> +inline int zynqmp_dll_reset(u8 deviceid, u32 type)
> +{
> + return 0;
> +}
> +
> +inline int arasan_zynqmp_set_in_tapdelay(u8 device_id, u32 itap_delay)
> +{
> + return 0;
> +}
> +
> +inline int arasan_zynqmp_set_out_tapdelay(u8 device_id, u32 otap_delay)
> +{
> + return 0;
> +}
> #endif
>
> #endif
>
