On 11/13/21 4:15 AM, Sean Anderson wrote:
> [ fsl_esdhc commit 52faec31827ec1a1837977e29c067424426634c5 ]
>
> Make the code cleaner and drop the old-style #ifdef constructs where it is
> possible.
>
> Signed-off-by: Michael Walle <mich...@walle.cc>
> Signed-off-by: Sean Anderson <sean.ander...@seco.com>
> ---
>
> (no changes since v1)
>
> drivers/mmc/fsl_esdhc_imx.c | 209 +++++++++++++++++-------------------
> include/fsl_esdhc_imx.h | 2 -
> 2 files changed, 100 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> index aa3d0877cb..89572509a7 100644
> --- a/drivers/mmc/fsl_esdhc_imx.c
> +++ b/drivers/mmc/fsl_esdhc_imx.c
> @@ -182,15 +182,15 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct
> mmc_data *data)
>
> if (data) {
> xfertyp |= XFERTYP_DPSEL;
> -#ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> - xfertyp |= XFERTYP_DMAEN;
> -#endif
> + if (!IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO) &&
> + cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK &&
> + cmd->cmdidx != MMC_CMD_SEND_TUNING_BLOCK_HS200)
> + xfertyp |= XFERTYP_DMAEN;
> if (data->blocks > 1) {
> xfertyp |= XFERTYP_MSBSEL;
> xfertyp |= XFERTYP_BCEN;
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
> - xfertyp |= XFERTYP_AC12EN;
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111))
> + xfertyp |= XFERTYP_AC12EN;
> }
>
> if (data->flags & MMC_DATA_READ)
> @@ -214,7 +214,6 @@ static uint esdhc_xfertyp(struct mmc_cmd *cmd, struct
> mmc_data *data)
> return XFERTYP_CMD(cmd->cmdidx) | xfertyp;
> }
>
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> /*
> * PIO Read/Write Mode reduce the performace as DMA is not used in this mode.
> */
> @@ -277,9 +276,7 @@ static void esdhc_pio_read_write(struct fsl_esdhc_priv
> *priv,
> }
> }
> }
> -#endif
>
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> static void esdhc_setup_watermark_level(struct fsl_esdhc_priv *priv,
> struct mmc_data *data)
> {
> @@ -299,7 +296,6 @@ static void esdhc_setup_watermark_level(struct
> fsl_esdhc_priv *priv,
> wml_value << 16);
> }
> }
> -#endif
>
> static void esdhc_setup_dma(struct fsl_esdhc_priv *priv, struct mmc_data
> *data)
> {
> @@ -342,11 +338,10 @@ static int esdhc_setup_data(struct fsl_esdhc_priv
> *priv, struct mmc *mmc,
> }
> }
>
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> - esdhc_setup_watermark_level(priv, data);
> -#else
> - esdhc_setup_dma(priv, data);
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO))
> + esdhc_setup_watermark_level(priv, data);
> + else
> + esdhc_setup_dma(priv, data);
>
> /* Calculate the timeout period for data transactions */
> /*
> @@ -379,14 +374,13 @@ static int esdhc_setup_data(struct fsl_esdhc_priv
> *priv, struct mmc *mmc,
> if (timeout < 0)
> timeout = 0;
>
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC_A001
> - if ((timeout == 4) || (timeout == 8) || (timeout == 12))
> + if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC_A001) &&
> + (timeout == 4 || timeout == 8 || timeout == 12))
> timeout++;
> -#endif
>
> -#ifdef ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE
> - timeout = 0xE;
> -#endif
> + if (IS_ENABLED(ESDHCI_QUIRK_BROKEN_TIMEOUT_VALUE))
> + timeout = 0xE;
> +
> esdhc_clrsetbits32(®s->sysctl, SYSCTL_TIMEOUT_MASK, timeout << 16);
>
> return 0;
> @@ -409,6 +403,11 @@ static inline void sd_swap_dma_buff(struct mmc_data
> *data)
> }
> }
> }
> +#else
> +static inline void sd_swap_dma_buff(struct mmc_data *data)
> +{
> + return;
> +}
> #endif
>
> /*
> @@ -425,10 +424,9 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv
> *priv, struct mmc *mmc,
> struct fsl_esdhc *regs = priv->esdhc_regs;
> unsigned long start;
>
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC111
> - if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> + if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC111) &&
> + cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> return 0;
> -#endif
>
> esdhc_write32(®s->irqstat, -1);
>
> @@ -526,42 +524,40 @@ static int esdhc_send_cmd_common(struct fsl_esdhc_priv
> *priv, struct mmc *mmc,
>
> /* Wait until all of the blocks are transferred */
> if (data) {
> -#ifdef CONFIG_SYS_FSL_ESDHC_USE_PIO
> - esdhc_pio_read_write(priv, data);
> -#else
> - flags = DATA_COMPLETE;
> - if ((cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK) ||
> - (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)) {
> - flags = IRQSTAT_BRR;
> + if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_USE_PIO)) {
> + esdhc_pio_read_write(priv, data);
> + } else {
> + flags = DATA_COMPLETE;
> + if (cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK ||
> + cmd->cmdidx == MMC_CMD_SEND_TUNING_BLOCK_HS200)
> + flags = IRQSTAT_BRR;
> +
> + do {
> + irqstat = esdhc_read32(®s->irqstat);
> +
> + if (irqstat & IRQSTAT_DTOE) {
> + err = -ETIMEDOUT;
> + goto out;
> + }
> +
> + if (irqstat & DATA_ERR) {
> + err = -ECOMM;
> + goto out;
> + }
> + } while ((irqstat & flags) != flags);
> +
> + /*
> + * Need invalidate the dcache here again to avoid any
> + * cache-fill during the DMA operations such as the
> + * speculative pre-fetching etc.
> + */
> + dma_unmap_single(priv->dma_addr,
> + data->blocks * data->blocksize,
> + mmc_get_dma_dir(data));
> + if (IS_ENABLED(CONFIG_MCF5441x) &&
> + (data->flags & MMC_DATA_READ))
> + sd_swap_dma_buff(data);
> }
> -
> - do {
> - irqstat = esdhc_read32(®s->irqstat);
> -
> - if (irqstat & IRQSTAT_DTOE) {
> - err = -ETIMEDOUT;
> - goto out;
> - }
> -
> - if (irqstat & DATA_ERR) {
> - err = -ECOMM;
> - goto out;
> - }
> - } while ((irqstat & flags) != flags);
> -
> - /*
> - * Need invalidate the dcache here again to avoid any
> - * cache-fill during the DMA operations such as the
> - * speculative pre-fetching etc.
> - */
> - dma_unmap_single(priv->dma_addr,
> - data->blocks * data->blocksize,
> - mmc_get_dma_dir(data));
> -#ifdef CONFIG_MCF5441x
> - if (data->flags & MMC_DATA_READ)
> - sd_swap_dma_buff(data);
> -#endif
> -#endif
> }
>
> out:
> @@ -595,21 +591,22 @@ static void set_sysctl(struct fsl_esdhc_priv *priv,
> struct mmc *mmc, uint clock)
> struct fsl_esdhc *regs = priv->esdhc_regs;
> int div = 1;
> u32 tmp;
> - int ret;
> -#ifdef ARCH_MXC
> -#ifdef CONFIG_MX53
> - /* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
> - int pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2 : 1;
> -#else
> - int pre_div = 1;
> -#endif
> -#else
> - int pre_div = 2;
> -#endif
> + int ret, pre_div;
> int ddr_pre_div = mmc->ddr_mode ? 2 : 1;
> int sdhc_clk = priv->sdhc_clk;
> uint clk;
>
> + if (IS_ENABLED(ARCH_MXC)) {
> +#ifdef CONFIG_MX53
Is there any reason not to use "IS_ENABLED()" ?
Best Regards,
Jaehoon Chung
> + /* For i.MX53 eSDHCv3, SYSCTL.SDCLKFS may not be set to 0. */
> + pre_div = (regs == (struct fsl_esdhc *)MMC_SDHC3_BASE_ADDR) ? 2
> : 1;
> +#else
> + pre_div = 1;
> +#endif
> + } else {
> + pre_div = 2;
> + }
> +
> while (sdhc_clk / (16 * pre_div * ddr_pre_div) > clock && pre_div < 256)
> pre_div *= 2;
>
> @@ -621,11 +618,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv,
> struct mmc *mmc, uint clock)
>
> clk = (pre_div << 8) | (div << 4);
>
> -#ifdef CONFIG_FSL_USDHC
> - esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN);
> -#else
> - esdhc_clrbits32(®s->sysctl, SYSCTL_CKEN);
> -#endif
> + if (IS_ENABLED(CONFIG_FSL_USDHC))
> + esdhc_clrbits32(®s->vendorspec, VENDORSPEC_CKEN);
> + else
> + esdhc_clrbits32(®s->sysctl, SYSCTL_CKEN);
>
> esdhc_clrsetbits32(®s->sysctl, SYSCTL_CLOCK_MASK, clk);
>
> @@ -633,11 +629,10 @@ static void set_sysctl(struct fsl_esdhc_priv *priv,
> struct mmc *mmc, uint clock)
> if (ret)
> pr_warn("fsl_esdhc_imx: Internal clock never stabilised.\n");
>
> -#ifdef CONFIG_FSL_USDHC
> - esdhc_setbits32(®s->vendorspec, VENDORSPEC_PEREN | VENDORSPEC_CKEN);
> -#else
> - esdhc_setbits32(®s->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
> -#endif
> + if (IS_ENABLED(CONFIG_FSL_USDHC))
> + esdhc_setbits32(®s->vendorspec, VENDORSPEC_PEREN |
> VENDORSPEC_CKEN);
> + else
> + esdhc_setbits32(®s->sysctl, SYSCTL_PEREN | SYSCTL_CKEN);
>
> mmc->clock = sdhc_clk / pre_div / div;
> priv->clock = clock;
> @@ -1145,22 +1140,21 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
> if (ret)
> return ret;
>
> -#ifdef CONFIG_MCF5441x
> /* ColdFire, using SDHC_DATA[3] for card detection */
> - esdhc_write32(®s->proctl, PROCTL_INIT | PROCTL_D3CD);
> -#endif
> + if (IS_ENABLED(CONFIG_MCF5441x))
> + esdhc_write32(®s->proctl, PROCTL_INIT | PROCTL_D3CD);
>
> -#ifndef CONFIG_FSL_USDHC
> - esdhc_setbits32(®s->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
> - | SYSCTL_IPGEN | SYSCTL_CKEN);
> - /* Clearing tuning bits in case ROM has set it already */
> - esdhc_write32(®s->mixctrl, 0);
> - esdhc_write32(®s->autoc12err, 0);
> - esdhc_write32(®s->clktunectrlstatus, 0);
> -#else
> - esdhc_setbits32(®s->vendorspec, VENDORSPEC_PEREN |
> - VENDORSPEC_HCKEN | VENDORSPEC_IPGEN | VENDORSPEC_CKEN);
> -#endif
> + if (IS_ENABLED(CONFIG_FSL_USDHC)) {
> + esdhc_setbits32(®s->vendorspec, VENDORSPEC_PEREN |
> + VENDORSPEC_HCKEN | VENDORSPEC_IPGEN |
> VENDORSPEC_CKEN);
> + } else {
> + esdhc_setbits32(®s->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
> + | SYSCTL_IPGEN | SYSCTL_CKEN);
> + /* Clearing tuning bits in case ROM has set it already */
> + esdhc_write32(®s->mixctrl, 0);
> + esdhc_write32(®s->autoc12err, 0);
> + esdhc_write32(®s->clktunectrlstatus, 0);
> + }
>
> if (priv->vs18_enable)
> esdhc_setbits32(®s->vendorspec, ESDHC_VENDORSPEC_VSELECT);
> @@ -1172,22 +1166,20 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
> #endif
>
> caps = esdhc_read32(®s->hostcapblt);
> -#ifdef CONFIG_MCF5441x
> +
> /*
> * MCF5441x RM declares in more points that sdhc clock speed must
> * never exceed 25 Mhz. From this, the HS bit needs to be disabled
> * from host capabilities.
> */
> - caps &= ~ESDHC_HOSTCAPBLT_HSS;
> -#endif
> + if (IS_ENABLED(CONFIG_MCF5441x))
> + caps &= ~HOSTCAPBLT_HSS;
>
> -#ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
> - caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_FSL_ERRATUM_ESDHC135))
> + caps &= ~(HOSTCAPBLT_SRS | HOSTCAPBLT_VS18 | HOSTCAPBLT_VS30);
>
> -#ifdef CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33
> - caps |= HOSTCAPBLT_VS33;
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_FSL_MMC_HAS_CAPBLT_VS33))
> + caps |= HOSTCAPBLT_VS33;
>
> if (caps & HOSTCAPBLT_VS18)
> cfg->voltages |= MMC_VDD_165_195;
> @@ -1197,12 +1189,13 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
> cfg->voltages |= MMC_VDD_32_33 | MMC_VDD_33_34;
>
> cfg->name = "FSL_SDHC";
> +
> #if !CONFIG_IS_ENABLED(DM_MMC)
> cfg->ops = &esdhc_ops;
> #endif
> -#ifdef CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE
> - cfg->host_caps |= MMC_MODE_DDR_52MHz;
> -#endif
> +
> + if (IS_ENABLED(CONFIG_SYS_FSL_ESDHC_HAS_DDR_MODE))
> + cfg->host_caps |= MMC_MODE_DDR_52MHz;
>
> if (caps & HOSTCAPBLT_HSS)
> cfg->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
> @@ -1286,10 +1279,8 @@ int fsl_esdhc_initialize(struct bd_info *bis, struct
> fsl_esdhc_cfg *cfg)
> return -EINVAL;
> }
>
> -#ifdef CONFIG_ESDHC_DETECT_8_BIT_QUIRK
> - if (CONFIG_ESDHC_DETECT_8_BIT_QUIRK)
> + if (IS_ENABLED(CONFIG_ESDHC_DETECT_8_BIT_QUIRK))
> mmc_cfg->host_caps &= ~MMC_MODE_8BIT;
> -#endif
>
> ret = fsl_esdhc_init(priv, plat);
> if (ret) {
> diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h
> index 1529b8bba3..b958a6b3bb 100644
> --- a/include/fsl_esdhc_imx.h
> +++ b/include/fsl_esdhc_imx.h
> @@ -24,12 +24,10 @@
> #define SYSCTL_INITA 0x08000000
> #define SYSCTL_TIMEOUT_MASK 0x000f0000
> #define SYSCTL_CLOCK_MASK 0x0000fff0
> -#if !defined(CONFIG_FSL_USDHC)
> #define SYSCTL_CKEN 0x00000008
> #define SYSCTL_PEREN 0x00000004
> #define SYSCTL_HCKEN 0x00000002
> #define SYSCTL_IPGEN 0x00000001
> -#endif
> #define SYSCTL_RSTA 0x01000000
> #define SYSCTL_RSTC 0x02000000
> #define SYSCTL_RSTD 0x04000000
>
