Re: [PATCH] mmc: fsl_esdhc_imx: replace all readl/writel to esdhc_read32/esdhc_write32

18 Sep 2020

Hi Haibo,
On 01.09.20 09:34, haibo.chen@nxp.com wrote:
...
From: Haibo Chen haibo.chen@nxp.com
Currently, readl/writel and esdhc_read32/esdhc_write32 are used. To align
the usage, change to only use esdhc_read32/esdhc_write32.
Signed-off-by: Haibo Chen haibo.chen@nxp.com
drivers/mmc/fsl_esdhc_imx.c | 64 ++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 0c866b168f..a0a0903ae4 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -729,7 +729,7 @@ static void esdhc_set_strobe_dll(struct mmc *mmc)
   u32 val;
if (priv->clock > ESDHC_STROBE_DLL_CLK_FREQ) {

writel(ESDHC_STROBE_DLL_CTRL_RESET, &regs->strobe_dllctrl);




esdhc_write32(ESDHC_STROBE_DLL_CTRL_RESET, &regs->strobe_dllctrl);


/*

enable strobe dll ctrl and adjust the delay target



@@ -738,10 +738,10 @@ static void esdhc_set_strobe_dll(struct mmc *mmc)
   	val = ESDHC_STROBE_DLL_CTRL_ENABLE |
   		(priv->strobe_dll_delay_target <<
   		 ESDHC_STROBE_DLL_CTRL_SLV_DLY_TARGET_SHIFT);

writel(val, &regs->strobe_dllctrl);




esdhc_write32(val, &regs->strobe_dllctrl);

/* wait 1us to make sure strobe dll status register stable */
mdelay(1);


val = readl(&regs->strobe_dllstat);




val = esdhc_read32(&regs->strobe_dllstat);

if (!(val & ESDHC_STROBE_DLL_STS_REF_LOCK))
	pr_warn("HS400 strobe DLL status REF not lock!\n");
if (!(val & ESDHC_STROBE_DLL_STS_SLV_LOCK))

@@ -755,18 +755,18 @@ static int esdhc_set_timing(struct mmc *mmc)
   struct fsl_esdhc *regs = priv->esdhc_regs;
   u32 mixctrl;

mixctrl = readl(&regs->mixctrl);


mixctrl = esdhc_read32(&regs->mixctrl);
mixctrl &= ~(MIX_CTRL_DDREN | MIX_CTRL_HS400_EN);
switch (mmc->selected_mode) {
case MMC_LEGACY:
esdhc_reset_tuning(mmc);



writel(mixctrl, &regs->mixctrl);




esdhc_write32(mixctrl, &regs->mixctrl);

break;
case MMC_HS_400:
case MMC_HS_400_ES:
mixctrl |= MIX_CTRL_DDREN | MIX_CTRL_HS400_EN;


writel(mixctrl, &regs->mixctrl);




esdhc_write32(mixctrl, &regs->mixctrl);

esdhc_set_strobe_dll(mmc);
break;
case MMC_HS:

@@ -777,12 +777,12 @@ static int esdhc_set_timing(struct mmc *mmc)
   case UHS_SDR25:
   case UHS_SDR50:
   case UHS_SDR104:

writel(mixctrl, &regs->mixctrl);




esdhc_write32(mixctrl, &regs->mixctrl);

break;
case UHS_DDR50:
case MMC_DDR_52:
mixctrl |= MIX_CTRL_DDREN;


writel(mixctrl, &regs->mixctrl);




esdhc_write32(mixctrl, &regs->mixctrl);

break;
default:
printf("Not supported %d\n", mmc->selected_mode);

@@ -862,8 +862,8 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
   struct fsl_esdhc_priv *priv = dev_get_priv(dev);
   struct fsl_esdhc *regs = priv->esdhc_regs;
   struct mmc *mmc = &plat->mmc;

u32 irqstaten = readl(&regs->irqstaten);
u32 irqsigen = readl(&regs->irqsigen);


u32 irqstaten = esdhc_read32(&regs->irqstaten);
u32 irqsigen = esdhc_read32(&regs->irqsigen);
int i, ret = -ETIMEDOUT;
u32 val, mixctrl;

@@ -873,25 +873,25 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
/* This is readw/writew SDHCI_HOST_CONTROL2 when tuning */
   if (priv->flags & ESDHC_FLAG_STD_TUNING) {

val = readl(&regs->autoc12err);


mixctrl = readl(&regs->mixctrl);




val = esdhc_read32(&regs->autoc12err);


mixctrl = esdhc_read32(&regs->mixctrl);

val &= ~MIX_CTRL_SMPCLK_SEL;
mixctrl &= ~(MIX_CTRL_FBCLK_SEL | MIX_CTRL_AUTO_TUNE_EN);
val |= MIX_CTRL_EXE_TUNE;
mixctrl |= MIX_CTRL_FBCLK_SEL | MIX_CTRL_AUTO_TUNE_EN;



writel(val, &regs->autoc12err);


writel(mixctrl, &regs->mixctrl);




esdhc_write32(val, &regs->autoc12err);


esdhc_write32(mixctrl, &regs->mixctrl);

}
/* sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE); */



mixctrl = readl(&regs->mixctrl);


mixctrl = esdhc_read32(&regs->mixctrl);
mixctrl = MIX_CTRL_DTDSEL_READ | (mixctrl & ~MIX_CTRL_SDHCI_MASK);


writel(mixctrl, &regs->mixctrl);


esdhc_write32(mixctrl, &regs->mixctrl);


writel(IRQSTATEN_BRR, &regs->irqstaten);
writel(IRQSTATEN_BRR, &regs->irqsigen);


esdhc_write32(IRQSTATEN_BRR, &regs->irqstaten);

esdhc_write32(IRQSTATEN_BRR, &regs->irqsigen);
/*

Issue opcode repeatedly till Execute Tuning is set to 0 or the number



@@ -902,22 +902,22 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
if (opcode == MMC_CMD_SEND_TUNING_BLOCK_HS200) {
	if (mmc->bus_width == 8)


		writel(0x7080, &regs->blkattr);




		esdhc_write32(0x7080, &regs->blkattr);
else if (mmc->bus_width == 4)




		writel(0x7040, &regs->blkattr);




		esdhc_write32(0x7040, &regs->blkattr);

} else {


	writel(0x7040, &regs->blkattr);




	esdhc_write32(0x7040, &regs->blkattr);

}
/* sdhci_writew(host, SDHCI_TRNS_READ, SDHCI_TRANSFER_MODE) */



val = readl(&regs->mixctrl);




val = esdhc_read32(&regs->mixctrl);

val = MIX_CTRL_DTDSEL_READ | (val & ~MIX_CTRL_SDHCI_MASK);


writel(val, &regs->mixctrl);




esdhc_write32(val, &regs->mixctrl);


/* We are using STD tuning, no need to check return value */
mmc_send_tuning(mmc, opcode, NULL);



ctrl = readl(&regs->autoc12err);




ctrl = esdhc_read32(&regs->autoc12err);

if ((!(ctrl & MIX_CTRL_EXE_TUNE)) &&
   (ctrl & MIX_CTRL_SMPCLK_SEL)) {
	ret = 0;

@@ -925,8 +925,8 @@ static int fsl_esdhc_execute_tuning(struct udevice *dev, uint32_t opcode)
   	}
   }

writel(irqstaten, &regs->irqstaten);
writel(irqsigen, &regs->irqsigen);


esdhc_write32(irqstaten, &regs->irqstaten);

esdhc_write32(irqsigen, &regs->irqsigen);
esdhc_stop_tuning(mmc);


@@ -1179,7 +1179,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
   if (priv->vs18_enable)
   	esdhc_setbits32(&regs->vendorspec, ESDHC_VENDORSPEC_VSELECT);

writel(SDHCI_IRQ_EN_BITS, &regs->irqstaten);


esdhc_write32(SDHCI_IRQ_EN_BITS, &regs->irqstaten);
cfg = &plat->cfg;

#ifndef CONFIG_DM_MMC
   memset(cfg, '\0', sizeof(*cfg));
@@ -1260,10 +1260,10 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
cfg->b_max = CONFIG_SYS_MMC_MAX_BLK_COUNT;

writel(0, &regs->dllctrl);


esdhc_write32(0, &regs->dllctrl);
if (priv->flags & ESDHC_FLAG_USDHC) {
if (priv->flags & ESDHC_FLAG_STD_TUNING) {


	u32 val = readl(&regs->tuning_ctrl);




	u32 val = esdhc_read32(&regs->tuning_ctrl);

val |= ESDHC_STD_TUNING_EN;
val &= ~ESDHC_TUNING_START_TAP_MASK;



@@ -1282,7 +1282,7 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv *priv,
   		 * after the whole tuning procedure always can't get any response.
   		 */
   		val |= ESDHC_TUNING_CMD_CRC_CHECK_DISABLE;

	writel(val, &regs->tuning_ctrl);




	esdhc_write32(val, &regs->tuning_ctrl);

}
}

@@ -1648,9 +1648,9 @@ static int fsl_esdhc_set_enhanced_strobe(struct udevice *dev)
   struct fsl_esdhc *regs = priv->esdhc_regs;
   u32 m;

m = readl(&regs->mixctrl);


m = esdhc_read32(&regs->mixctrl);
m |= MIX_CTRL_HS400_ES;


writel(m, &regs->mixctrl);


esdhc_write32(m, &regs->mixctrl);
return 0;


}
In principle I find this cleanup good, but it breaks several board under
ARM64 (imx8 for example). You can take a look at my Travis' build :
https://travis-ci.org/github/sbabic/u-boot-imx/jobs/728000331
Can you take a look and then repost it ? Thanks !
Best regards,
Stefano Babic
-- 
=====================================================================
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
=====================================================================

    