[U-Boot] [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code

Angelo Dureghello angelo at sysam.it
Fri May 31 07:15:09 UTC 2019 

Hi Lu,

On Fri, May 31, 2019 at 06:12:12AM +0000, Y.b. Lu wrote:
> Hi Angelo,
> 
> > -----Original Message-----
> > From: Angelo Dureghello <angelo at sysam.it>
> > Sent: 2019年5月31日 2:23
> > To: Y.b. Lu <yangbo.lu at nxp.com>
> > Cc: u-boot at lists.denx.de; Stefano Babic <sbabic at denx.de>; Fabio Estevam
> > <festevam at gmail.com>; dl-uboot-imx <uboot-imx at nxp.com>; Albert Aribaud
> > <albert.u.boot at aribaud.net>; Eddy Petrișor <eddy.petrisor at gmail.com>;
> > Akshay Bhat <akshaybhat at timesys.com>; Ken Lin
> > <Ken.Lin at advantech.com.tw>; Heiko Schocher <hs at denx.de>; Christian
> > Gmeiner <christian.gmeiner at gmail.com>; Stefan Roese <sr at denx.de>; Patrick
> > Bruenn <p.bruenn at beckhoff.com>; Troy Kisky
> > <troy.kisky at boundarydevices.com>; Uri Mashiach
> > <uri.mashiach at compulab.co.il>; Nikita Kiryanov <nikita at compulab.co.il>;
> > Otavio Salvador <otavio at ossystems.com.br>; Andreas Geisreiter
> > <ageisreiter at dh-electronics.de>; Ludwig Zenz <lzenz at dh-electronics.de>; Eric
> > Bénard <eric at eukrea.com>; Peng Fan <peng.fan at nxp.com>; Jason Liu
> > <jason.hui.liu at nxp.com>; Ye Li <ye.li at nxp.com>; Adrian Alonso
> > <adrian.alonso at nxp.com>; Alison Wang <alison.wang at nxp.com>;
> > tharvey at gateworks.com; Ian Ray <ian.ray at ge.com>; Marcin Niestroj
> > <m.niestroj at grinn-global.com>; Andrej Rosano <andrej at inversepath.com>;
> > Marek Vasut <marex at denx.de>; Lukasz Majewski <lukma at denx.de>; Adam
> > Ford <aford173 at gmail.com>; Olaf Mandel <o.mandel at menlosystems.com>;
> > Martyn Welch <martyn.welch at collabora.com>; Ingo Schroeck
> > <open-source at samtec.de>; Boris Brezillon
> > <boris.brezillon at free-electrons.com>; Soeren Moch <smoch at web.de>;
> > Richard Hu <richard.hu at technexion.com>; Vanessa Maegima
> > <vanessa.maegima at nxp.com>; Max Krummenacher
> > <max.krummenacher at toradex.com>; Stefan Agner
> > <stefan.agner at toradex.com>; Markus Niebel <Markus.Niebel at tq-group.com>;
> > Breno Matheus Lima <breno.lima at nxp.com>; Francesco Montefoschi
> > <francesco.montefoschi at udoo.org>; Parthiban Nallathambi
> > <parthitce at gmail.com>; Albert ARIBAUD <albert.aribaud at 3adev.fr>; Jagan
> > Teki <jagan at amarulasolutions.com>; Raffaele RECALCATI
> > <raffaele.recalcati at bticino.it>; Simone CIANNI <simone.cianni at bticino.it>;
> > Bhaskar Upadhaya <bhaskar.upadhaya at nxp.com>; Vinitha V Pillai
> > <vinitha.pillai at nxp.com>; Prabhakar Kushwaha
> > <prabhakar.kushwaha at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> > Antti Mäentausta <antti.maentausta at ge.com>; Sébastien Szymanski
> > <sebastien.szymanski at armadeus.com>; Lucile Quirion
> > <lucile.quirion at savoirfairelinux.com>; Alexey Brodkin
> > <abrodkin at synopsys.com>; Trevor Woerner <trevor at toganlabs.com>;
> > Anatolij Gustschin <agust at denx.de>; Denis Zalevskiy
> > <denis.zalevskiy at ge.com>; Fabien Lahoudere
> > <fabien.lahoudere at collabora.com>; Joe Hershberger
> > <joe.hershberger at ni.com>; Simon Goldschmidt
> > <simon.k.r.goldschmidt at gmail.com>; James Byrne
> > <james.byrne at origamienergy.com>
> > Subject: Re: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> > 
> > Hi Lu,
> > 
> > On Tue, May 21, 2019 at 08:53:04AM +0000, Y.b. Lu wrote:
> > > Dropped useless code for i.MX eSDHC driver.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
> > > ---
> > > Changes for v2:
> > > 	- Added this patch.
> > > Changes for v3:
> > > 	- None.
> > > ---
> > >  drivers/mmc/fsl_esdhc_imx.c | 96 ++-----------------------------------
> > >  include/fsl_esdhc_imx.h     |  4 --
> > >  2 files changed, 4 insertions(+), 96 deletions(-)
> > >
> > > diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
> > > index faf133390f..1c02e0eef1 100644
> > > --- a/drivers/mmc/fsl_esdhc_imx.c
> > > +++ b/drivers/mmc/fsl_esdhc_imx.c
> > > @@ -259,8 +259,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv
> > > *priv, struct mmc *mmc,  {
> > >  	int timeout;
> > >  	struct fsl_esdhc *regs = priv->esdhc_regs; -#if
> > > defined(CONFIG_FSL_LAYERSCAPE) || defined(CONFIG_S32V234) || \
> > > -	defined(CONFIG_IMX8) || defined(CONFIG_IMX8M)
> > > +#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) ||
> > > +defined(CONFIG_IMX8M)
> > >  	dma_addr_t addr;
> > >  #endif
> > >  	uint wml_value;
> > > @@ -273,8 +272,7 @@ static int esdhc_setup_data(struct fsl_esdhc_priv
> > > *priv, struct mmc *mmc,
> > >
> > >  		esdhc_clrsetbits32(&regs->wml, WML_RD_WML_MASK, wml_value);
> > > #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO -#if
> > > defined(CONFIG_FSL_LAYERSCAPE) || defined(CONFIG_S32V234) || \
> > > -	defined(CONFIG_IMX8) || defined(CONFIG_IMX8M)
> > > +#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) ||
> > > +defined(CONFIG_IMX8M)
> > >  		addr = virt_to_phys((void *)(data->dest));
> > >  		if (upper_32_bits(addr))
> > >  			printf("Error found for upper 32 bits\n"); @@ -310,8 +308,7
> > @@
> > > static int esdhc_setup_data(struct fsl_esdhc_priv *priv, struct mmc *mmc,
> > >  		esdhc_clrsetbits32(&regs->wml, WML_WR_WML_MASK,
> > >  					wml_value << 16);
> > >  #ifndef CONFIG_SYS_FSL_ESDHC_USE_PIO
> > > -#if defined(CONFIG_FSL_LAYERSCAPE) || defined(CONFIG_S32V234) || \
> > > -	defined(CONFIG_IMX8) || defined(CONFIG_IMX8M)
> > > +#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) ||
> > > +defined(CONFIG_IMX8M)
> > >  		addr = virt_to_phys((void *)(data->src));
> > >  		if (upper_32_bits(addr))
> > >  			printf("Error found for upper 32 bits\n"); @@ -376,8 +373,7
> > @@
> > > static void check_and_invalidate_dcache_range
> > >  	unsigned end = 0;
> > >  	unsigned size = roundup(ARCH_DMA_MINALIGN,
> > >  				data->blocks*data->blocksize);
> > > -#if defined(CONFIG_FSL_LAYERSCAPE) || defined(CONFIG_S32V234) || \
> > > -	defined(CONFIG_IMX8) || defined(CONFIG_IMX8M)
> > > +#if defined(CONFIG_S32V234) || defined(CONFIG_IMX8) ||
> > > +defined(CONFIG_IMX8M)
> > >  	dma_addr_t addr;
> > >
> > >  	addr = virt_to_phys((void *)(data->dest)); @@ -392,25 +388,6 @@
> > > static void check_and_invalidate_dcache_range
> > >  	invalidate_dcache_range(start, end);  }
> > >
> > > -#ifdef CONFIG_MCF5441x
> > > -/*
> > > - * Swaps 32-bit words to little-endian byte order.
> > > - */
> > > -static inline void sd_swap_dma_buff(struct mmc_data *data) -{
> > > -	int i, size = data->blocksize >> 2;
> > > -	u32 *buffer = (u32 *)data->dest;
> > > -	u32 sw;
> > > -
> > > -	while (data->blocks--) {
> > > -		for (i = 0; i < size; i++) {
> > > -			sw = __sw32(*buffer);
> > > -			*buffer++ = sw;
> > > -		}
> > > -	}
> > > -}
> > > -#endif
> > > -
> > 
> > Doing so you remove the ColdFire family code (mcf5441x) i just added recently.
> > Since they are big-endian, and dma hw has no options to swap, swap is
> > needed.
> > 
> > Please don't remove it.
> 
> [Y.b. Lu] I didn’t remove mcf5441x support. The code is still in fsl_esdhc.c, but dropped in fsl_esdhc_imx.c.
> I'm surprised there was other platforms using eSDHC besides QorIQ and i.MX, because eSDHC is an IP of Freescale/NXP.
> Since I didn't know whether mcf5441x eSDHC is same with QorIQ or i.MX, I just left it in fsl_esdhc.c
> So, which driver should apply to mcf5441x eSDHC?
> 
> Thanks.
> > 

Many thanks for clarifications, looks like i lost some detail of the patch,
sorry. 
Well, Freescale did some HW modules that has re-used built-in into 
i.MX, ColdFire and also sometimes ppc families. They are nearly the same
(same register set) but with some minimal differences on some bit field. 
Those are i2c, dspi, edma, and as for this case, the eSDHC. You can apply 
ColdFire code for the i.MX driver, should be fine. Then i should be able to
test that it works properly. Just see the CONFIG_MCF5441x for the needed 
ColdFire code. Thanks for the cleanup.

> > >  /*
> > >   * Sends a command out on the bus.  Takes the mmc pointer,
> > >   * a command pointer, and an optional data pointer.
> > > @@ -575,9 +552,6 @@ static int esdhc_send_cmd_common(struct
> > fsl_esdhc_priv *priv, struct mmc *mmc,
> > >  		 */
> > >  		if (data->flags & MMC_DATA_READ) {
> > >  			check_and_invalidate_dcache_range(cmd, data); -#ifdef
> > > CONFIG_MCF5441x
> > > -			sd_swap_dma_buff(data);
> > 
> > Same here.
> > 
> > > -#endif
> > >  		}
> > >  #endif
> > >  	}
> > > @@ -1073,12 +1047,8 @@ static int esdhc_init_common(struct
> > fsl_esdhc_priv *priv, struct mmc *mmc)
> > >  	/* Disable the BRR and BWR bits in IRQSTAT */
> > >  	esdhc_clrbits32(&regs->irqstaten, IRQSTATEN_BRR | IRQSTATEN_BWR);
> > >
> > > -#ifdef CONFIG_MCF5441x
> > > -	esdhc_write32(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
> > > -#else
> > >  	/* Put the PROCTL reg back to the default */
> > >  	esdhc_write32(&regs->proctl, PROCTL_INIT); -#endif
> > >
> > >  	/* Set timout to the maximum value */
> > >  	esdhc_clrsetbits32(&regs->sysctl, SYSCTL_TIMEOUT_MASK, 14 << 16);
> > @@
> > > -1186,11 +1156,6 @@ 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(&regs->proctl, PROCTL_INIT | PROCTL_D3CD);
> > > -#endif
> > > -
> > >  #ifndef CONFIG_FSL_USDHC
> > >  	esdhc_setbits32(&regs->sysctl, SYSCTL_PEREN | SYSCTL_HCKEN
> > >  				| SYSCTL_IPGEN | SYSCTL_CKEN);
> > > @@ -1215,15 +1180,6 @@ static int fsl_esdhc_init(struct fsl_esdhc_priv
> > *priv,
> > >  	voltage_caps = 0;
> > >  	caps = esdhc_read32(&regs->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
> > 
> > Same here.
> > 
> > 
> > > -
> > >  #ifdef CONFIG_SYS_FSL_ERRATUM_ESDHC135
> > >  	caps = caps & ~(ESDHC_HOSTCAPBLT_SRS |
> > >  			ESDHC_HOSTCAPBLT_VS18 | ESDHC_HOSTCAPBLT_VS30); @@
> > -1375,45
> > > +1331,6 @@ int fsl_esdhc_mmc_init(bd_t *bis)  }  #endif
> > >
> > > -#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT -void
> > > mmc_adapter_card_type_ident(void) -{
> > > -	u8 card_id;
> > > -	u8 value;
> > > -
> > > -	card_id = QIXIS_READ(present) & QIXIS_SDID_MASK;
> > > -	gd->arch.sdhc_adapter = card_id;
> > > -
> > > -	switch (card_id) {
> > > -	case QIXIS_ESDHC_ADAPTER_TYPE_EMMC45:
> > > -		value = QIXIS_READ(brdcfg[5]);
> > > -		value |= (QIXIS_DAT4 | QIXIS_DAT5_6_7);
> > > -		QIXIS_WRITE(brdcfg[5], value);
> > > -		break;
> > > -	case QIXIS_ESDHC_ADAPTER_TYPE_SDMMC_LEGACY:
> > > -		value = QIXIS_READ(pwr_ctl[1]);
> > > -		value |= QIXIS_EVDD_BY_SDHC_VS;
> > > -		QIXIS_WRITE(pwr_ctl[1], value);
> > > -		break;
> > > -	case QIXIS_ESDHC_ADAPTER_TYPE_EMMC44:
> > > -		value = QIXIS_READ(brdcfg[5]);
> > > -		value |= (QIXIS_SDCLKIN | QIXIS_SDCLKOUT);
> > > -		QIXIS_WRITE(brdcfg[5], value);
> > > -		break;
> > > -	case QIXIS_ESDHC_ADAPTER_TYPE_RSV:
> > > -		break;
> > > -	case QIXIS_ESDHC_ADAPTER_TYPE_MMC:
> > > -		break;
> > > -	case QIXIS_ESDHC_ADAPTER_TYPE_SD:
> > > -		break;
> > > -	case QIXIS_ESDHC_NO_ADAPTER:
> > > -		break;
> > > -	default:
> > > -		break;
> > > -	}
> > > -}
> > > -#endif
> > > -
> > >  #ifdef CONFIG_OF_LIBFDT
> > >  __weak int esdhc_status_fixup(void *blob, const char *compat)  { @@
> > > -1441,10 +1358,6 @@ void fdt_fixup_esdhc(void *blob, bd_t *bd)
> > >  	do_fixup_by_compat_u32(blob, compat, "clock-frequency",
> > >  			       gd->arch.sdhc_clk, 1);
> > >  #endif
> > > -#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT
> > > -	do_fixup_by_compat_u32(blob, compat, "adapter-type",
> > > -			       (u32)(gd->arch.sdhc_adapter), 1);
> > > -#endif
> > >  }
> > >  #endif
> > >
> > > @@ -1654,7 +1567,6 @@ static const struct udevice_id fsl_esdhc_ids[] = {
> > >  	{ .compatible = "fsl,imx6q-usdhc", },
> > >  	{ .compatible = "fsl,imx7d-usdhc", .data = (ulong)&usdhc_imx7d_data,},
> > >  	{ .compatible = "fsl,imx7ulp-usdhc", },
> > > -	{ .compatible = "fsl,esdhc", },
> > >  	{ /* sentinel */ }
> > >  };
> > >
> > > diff --git a/include/fsl_esdhc_imx.h b/include/fsl_esdhc_imx.h index
> > > e05b24e7e8..8abd28ea50 100644
> > > --- a/include/fsl_esdhc_imx.h
> > > +++ b/include/fsl_esdhc_imx.h
> > > @@ -17,10 +17,6 @@
> > >  /* needed for the mmc_cfg definition */  #include <mmc.h>
> > >
> > > -#ifdef CONFIG_FSL_ESDHC_ADAPTER_IDENT -#include
> > > "../board/freescale/common/qixis.h"
> > > -#endif
> > > -
> > >  /* FSL eSDHC-specific constants */
> > >  #define SYSCTL			0x0002e02c
> > >  #define SYSCTL_INITA		0x08000000
> > > --
> > > 2.17.1
> > >

Regards,
Angelo Dureghello

More information about the U-Boot mailing list