diff mbox series

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

Message ID 20190521085215.6263-6-yangbo.lu@nxp.com
State Superseded
Delegated to: Peng Fan
Headers show
Series Split fsl_esdhc driver for i.MX | expand

Commit Message

Yangbo Lu May 21, 2019, 8:53 a.m. UTC
Dropped useless code for i.MX eSDHC driver.

Signed-off-by: Yangbo Lu <yangbo.lu@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(-)

Comments

Peng Fan May 29, 2019, 1:53 a.m. UTC | #1
> Subject: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> 
> Dropped useless code for i.MX eSDHC driver.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@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(-)
> 

[.....]

> -	{ .compatible = "fsl,esdhc", },

Please keep this, the following 2 dts using this compatible.
./arch/arm/dts/vf.dtsi
./arch/arm/dts/imx53-ppd.dts

With this addressed, you could have my
`Reviewed-by: Peng Fan <peng.fan@nxp.com>`

>  	{ /* 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
Yangbo Lu May 29, 2019, 6:09 a.m. UTC | #2
> -----Original Message-----
> From: Peng Fan
> Sent: 2019年5月29日 9:53
> To: Y.b. Lu <yangbo.lu@nxp.com>; u-boot@lists.denx.de
> Cc: Stefano Babic <sbabic@denx.de>; Fabio Estevam <festevam@gmail.com>;
> dl-uboot-imx <uboot-imx@nxp.com>; Albert Aribaud
> <albert.u.boot@aribaud.net>; Eddy Petrișor <eddy.petrisor@gmail.com>;
> Akshay Bhat <akshaybhat@timesys.com>; Ken Lin
> <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>; Christian
> Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese <sr@denx.de>; Patrick
> Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> <troy.kisky@boundarydevices.com>; Uri Mashiach
> <uri.mashiach@compulab.co.il>; Nikita Kiryanov <nikita@compulab.co.il>;
> Otavio Salvador <otavio@ossystems.com.br>; Andreas Geisreiter
> <ageisreiter@dh-electronics.de>; Ludwig Zenz <lzenz@dh-electronics.de>; Eric
> Bénard <eric@eukrea.com>; Jason Liu <jason.hui.liu@nxp.com>; Ye Li
> <ye.li@nxp.com>; Adrian Alonso <adrian.alonso@nxp.com>; Alison Wang
> <alison.wang@nxp.com>; tharvey@gateworks.com; Ian Ray
> <ian.ray@ge.com>; Marcin Niestroj <m.niestroj@grinn-global.com>; Andrej
> Rosano <andrej@inversepath.com>; Marek Vasut <marex@denx.de>; Lukasz
> Majewski <lukma@denx.de>; Adam Ford <aford173@gmail.com>; Olaf
> Mandel <o.mandel@menlosystems.com>; Martyn Welch
> <martyn.welch@collabora.com>; Ingo Schroeck <open-source@samtec.de>;
> Boris Brezillon <boris.brezillon@free-electrons.com>; Soeren Moch
> <smoch@web.de>; Richard Hu <richard.hu@technexion.com>; Vanessa
> Maegima <vanessa.maegima@nxp.com>; Max Krummenacher
> <max.krummenacher@toradex.com>; Stefan Agner
> <stefan.agner@toradex.com>; Markus Niebel <Markus.Niebel@tq-group.com>;
> Breno Matheus Lima <breno.lima@nxp.com>; Francesco Montefoschi
> <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>; Jagan
> Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> <raffaele.recalcati@bticino.it>; Simone CIANNI <simone.cianni@bticino.it>;
> Bhaskar Upadhaya <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> <sebastien.szymanski@armadeus.com>; Lucile Quirion
> <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> <denis.zalevskiy@ge.com>; Fabien Lahoudere
> <fabien.lahoudere@collabora.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; James Byrne
> <james.byrne@origamienergy.com>; Angelo Dureghello <angelo@sysam.it>
> Subject: RE: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> 
> 
> > Subject: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> >
> > Dropped useless code for i.MX eSDHC driver.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@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(-)
> >
> 
> [.....]
> 
> > -	{ .compatible = "fsl,esdhc", },
> 
> Please keep this, the following 2 dts using this compatible.
> ./arch/arm/dts/vf.dtsi
> ./arch/arm/dts/imx53-ppd.dts
> 
> With this addressed, you could have my
> `Reviewed-by: Peng Fan <peng.fan@nxp.com>`

[Y.b. Lu] Ok, thanks Peng.
May I know which u-boot subtree and branch should I use to rebase these patches?
Thanks!

> 
> >  	{ /* 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
Peng Fan May 29, 2019, 6:16 a.m. UTC | #3
> >
> > > Subject: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> > >
> > > Dropped useless code for i.MX eSDHC driver.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu@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(-)
> > >
> >
> > [.....]
> >
> > > -	{ .compatible = "fsl,esdhc", },
> >
> > Please keep this, the following 2 dts using this compatible.
> > ./arch/arm/dts/vf.dtsi
> > ./arch/arm/dts/imx53-ppd.dts
> >
> > With this addressed, you could have my
> > `Reviewed-by: Peng Fan <peng.fan@nxp.com>`
> 
> [Y.b. Lu] Ok, thanks Peng.
> May I know which u-boot subtree and branch should I use to rebase these
> patches?

I do not see Stefano update his tree, so use Tom's tree should be fine.

Regards,
Peng.

> Thanks!
> 
> >
> > >  	{ /* 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
Lukasz Majewski May 29, 2019, 6:42 a.m. UTC | #4
On Tue, 21 May 2019 08:53:04 +0000
"Y.b. Lu" <yangbo.lu@nxp.com> wrote:

> Dropped useless code for i.MX eSDHC driver.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@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
> -
>  /*
>   * 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);
> -#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
> -
>  #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", },

Please keep the above line (as Peng mentioned) - it allows re-using
this driver on VF610 and i.MX53 devices.

>  	{ /* 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




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Yangbo Lu May 29, 2019, 7:37 a.m. UTC | #5
> -----Original Message-----
> From: Lukasz Majewski <lukma@denx.de>
> Sent: 2019年5月29日 14:43
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio Estevam
> <festevam@gmail.com>; dl-uboot-imx <uboot-imx@nxp.com>; Albert Aribaud
> <albert.u.boot@aribaud.net>; Eddy Petrișor <eddy.petrisor@gmail.com>;
> Akshay Bhat <akshaybhat@timesys.com>; Ken Lin
> <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>; Christian
> Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese <sr@denx.de>; Patrick
> Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> <troy.kisky@boundarydevices.com>; Uri Mashiach
> <uri.mashiach@compulab.co.il>; Nikita Kiryanov <nikita@compulab.co.il>;
> Otavio Salvador <otavio@ossystems.com.br>; Andreas Geisreiter
> <ageisreiter@dh-electronics.de>; Ludwig Zenz <lzenz@dh-electronics.de>; Eric
> Bénard <eric@eukrea.com>; Peng Fan <peng.fan@nxp.com>; Jason Liu
> <jason.hui.liu@nxp.com>; Ye Li <ye.li@nxp.com>; Adrian Alonso
> <adrian.alonso@nxp.com>; Alison Wang <alison.wang@nxp.com>;
> tharvey@gateworks.com; Ian Ray <ian.ray@ge.com>; Marcin Niestroj
> <m.niestroj@grinn-global.com>; Andrej Rosano <andrej@inversepath.com>;
> Marek Vasut <marex@denx.de>; Adam Ford <aford173@gmail.com>; Olaf
> Mandel <o.mandel@menlosystems.com>; Martyn Welch
> <martyn.welch@collabora.com>; Ingo Schroeck <open-source@samtec.de>;
> Boris Brezillon <boris.brezillon@free-electrons.com>; Soeren Moch
> <smoch@web.de>; Richard Hu <richard.hu@technexion.com>; Vanessa
> Maegima <vanessa.maegima@nxp.com>; Max Krummenacher
> <max.krummenacher@toradex.com>; Stefan Agner
> <stefan.agner@toradex.com>; Markus Niebel <Markus.Niebel@tq-group.com>;
> Breno Matheus Lima <breno.lima@nxp.com>; Francesco Montefoschi
> <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>; Jagan
> Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> <raffaele.recalcati@bticino.it>; Simone CIANNI <simone.cianni@bticino.it>;
> Bhaskar Upadhaya <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> <sebastien.szymanski@armadeus.com>; Lucile Quirion
> <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> <denis.zalevskiy@ge.com>; Fabien Lahoudere
> <fabien.lahoudere@collabora.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; James Byrne
> <james.byrne@origamienergy.com>; Angelo Dureghello <angelo@sysam.it>
> Subject: Re: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> 
> On Tue, 21 May 2019 08:53:04 +0000
> "Y.b. Lu" <yangbo.lu@nxp.com> wrote:
> 
> > Dropped useless code for i.MX eSDHC driver.
> >
> > Signed-off-by: Yangbo Lu <yangbo.lu@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
> > -
> >  /*
> >   * 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);
> > -#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
> > -
> >  #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", },
> 
> Please keep the above line (as Peng mentioned) - it allows re-using this driver
> on VF610 and i.MX53 devices.
> 

[Y.b. Lu] Thanks. Will do that.

> >  	{ /* 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
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma@denx.de
Angelo Dureghello May 30, 2019, 6:23 p.m. UTC | #6
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@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.

>  /*
>   * 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
>
Peng Fan May 31, 2019, 5:58 a.m. UTC | #7
Hi,

> 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@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.

The mcf code is still in fsl_esdhc.c, this patch is to modify fsl_esdhc_imx.c.

Regards,
Peng.

> 
> >  /*
> >   * 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
> >
Yangbo Lu May 31, 2019, 6:12 a.m. UTC | #8
Hi Angelo,

> -----Original Message-----
> From: Angelo Dureghello <angelo@sysam.it>
> Sent: 2019年5月31日 2:23
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio Estevam
> <festevam@gmail.com>; dl-uboot-imx <uboot-imx@nxp.com>; Albert Aribaud
> <albert.u.boot@aribaud.net>; Eddy Petrișor <eddy.petrisor@gmail.com>;
> Akshay Bhat <akshaybhat@timesys.com>; Ken Lin
> <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>; Christian
> Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese <sr@denx.de>; Patrick
> Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> <troy.kisky@boundarydevices.com>; Uri Mashiach
> <uri.mashiach@compulab.co.il>; Nikita Kiryanov <nikita@compulab.co.il>;
> Otavio Salvador <otavio@ossystems.com.br>; Andreas Geisreiter
> <ageisreiter@dh-electronics.de>; Ludwig Zenz <lzenz@dh-electronics.de>; Eric
> Bénard <eric@eukrea.com>; Peng Fan <peng.fan@nxp.com>; Jason Liu
> <jason.hui.liu@nxp.com>; Ye Li <ye.li@nxp.com>; Adrian Alonso
> <adrian.alonso@nxp.com>; Alison Wang <alison.wang@nxp.com>;
> tharvey@gateworks.com; Ian Ray <ian.ray@ge.com>; Marcin Niestroj
> <m.niestroj@grinn-global.com>; Andrej Rosano <andrej@inversepath.com>;
> Marek Vasut <marex@denx.de>; Lukasz Majewski <lukma@denx.de>; Adam
> Ford <aford173@gmail.com>; Olaf Mandel <o.mandel@menlosystems.com>;
> Martyn Welch <martyn.welch@collabora.com>; Ingo Schroeck
> <open-source@samtec.de>; Boris Brezillon
> <boris.brezillon@free-electrons.com>; Soeren Moch <smoch@web.de>;
> Richard Hu <richard.hu@technexion.com>; Vanessa Maegima
> <vanessa.maegima@nxp.com>; Max Krummenacher
> <max.krummenacher@toradex.com>; Stefan Agner
> <stefan.agner@toradex.com>; Markus Niebel <Markus.Niebel@tq-group.com>;
> Breno Matheus Lima <breno.lima@nxp.com>; Francesco Montefoschi
> <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>; Jagan
> Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> <raffaele.recalcati@bticino.it>; Simone CIANNI <simone.cianni@bticino.it>;
> Bhaskar Upadhaya <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> <sebastien.szymanski@armadeus.com>; Lucile Quirion
> <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> <denis.zalevskiy@ge.com>; Fabien Lahoudere
> <fabien.lahoudere@collabora.com>; Joe Hershberger
> <joe.hershberger@ni.com>; Simon Goldschmidt
> <simon.k.r.goldschmidt@gmail.com>; James Byrne
> <james.byrne@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@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.
> 
> >  /*
> >   * 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
> >
Angelo Dureghello May 31, 2019, 7:15 a.m. UTC | #9
Hi Lu,

On Fri, May 31, 2019 at 06:12:12AM +0000, Y.b. Lu wrote:
> Hi Angelo,
> 
> > -----Original Message-----
> > From: Angelo Dureghello <angelo@sysam.it>
> > Sent: 2019年5月31日 2:23
> > To: Y.b. Lu <yangbo.lu@nxp.com>
> > Cc: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio Estevam
> > <festevam@gmail.com>; dl-uboot-imx <uboot-imx@nxp.com>; Albert Aribaud
> > <albert.u.boot@aribaud.net>; Eddy Petrișor <eddy.petrisor@gmail.com>;
> > Akshay Bhat <akshaybhat@timesys.com>; Ken Lin
> > <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>; Christian
> > Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese <sr@denx.de>; Patrick
> > Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> > <troy.kisky@boundarydevices.com>; Uri Mashiach
> > <uri.mashiach@compulab.co.il>; Nikita Kiryanov <nikita@compulab.co.il>;
> > Otavio Salvador <otavio@ossystems.com.br>; Andreas Geisreiter
> > <ageisreiter@dh-electronics.de>; Ludwig Zenz <lzenz@dh-electronics.de>; Eric
> > Bénard <eric@eukrea.com>; Peng Fan <peng.fan@nxp.com>; Jason Liu
> > <jason.hui.liu@nxp.com>; Ye Li <ye.li@nxp.com>; Adrian Alonso
> > <adrian.alonso@nxp.com>; Alison Wang <alison.wang@nxp.com>;
> > tharvey@gateworks.com; Ian Ray <ian.ray@ge.com>; Marcin Niestroj
> > <m.niestroj@grinn-global.com>; Andrej Rosano <andrej@inversepath.com>;
> > Marek Vasut <marex@denx.de>; Lukasz Majewski <lukma@denx.de>; Adam
> > Ford <aford173@gmail.com>; Olaf Mandel <o.mandel@menlosystems.com>;
> > Martyn Welch <martyn.welch@collabora.com>; Ingo Schroeck
> > <open-source@samtec.de>; Boris Brezillon
> > <boris.brezillon@free-electrons.com>; Soeren Moch <smoch@web.de>;
> > Richard Hu <richard.hu@technexion.com>; Vanessa Maegima
> > <vanessa.maegima@nxp.com>; Max Krummenacher
> > <max.krummenacher@toradex.com>; Stefan Agner
> > <stefan.agner@toradex.com>; Markus Niebel <Markus.Niebel@tq-group.com>;
> > Breno Matheus Lima <breno.lima@nxp.com>; Francesco Montefoschi
> > <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> > <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>; Jagan
> > Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> > <raffaele.recalcati@bticino.it>; Simone CIANNI <simone.cianni@bticino.it>;
> > Bhaskar Upadhaya <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> > <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> > <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>;
> > Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> > <sebastien.szymanski@armadeus.com>; Lucile Quirion
> > <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> > <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> > Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> > <denis.zalevskiy@ge.com>; Fabien Lahoudere
> > <fabien.lahoudere@collabora.com>; Joe Hershberger
> > <joe.hershberger@ni.com>; Simon Goldschmidt
> > <simon.k.r.goldschmidt@gmail.com>; James Byrne
> > <james.byrne@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@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
Yangbo Lu June 3, 2019, 4:28 a.m. UTC | #10
Hi,

> -----Original Message-----
> From: Angelo Dureghello <angelo@sysam.it>
> Sent: 2019年5月31日 15:15
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: u-boot@lists.denx.de
> Subject: Re: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> 
> Hi Lu,
> 
> On Fri, May 31, 2019 at 06:12:12AM +0000, Y.b. Lu wrote:
> > Hi Angelo,
> >
> > > -----Original Message-----
> > > From: Angelo Dureghello <angelo@sysam.it>
> > > Sent: 2019年5月31日 2:23
> > > To: Y.b. Lu <yangbo.lu@nxp.com>
> > > Cc: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio
> > > Estevam <festevam@gmail.com>; dl-uboot-imx <uboot-imx@nxp.com>;
> > > Albert Aribaud <albert.u.boot@aribaud.net>; Eddy Petrișor
> > > <eddy.petrisor@gmail.com>; Akshay Bhat <akshaybhat@timesys.com>;
> Ken
> > > Lin <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>;
> > > Christian Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese
> > > <sr@denx.de>; Patrick Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> > > <troy.kisky@boundarydevices.com>; Uri Mashiach
> > > <uri.mashiach@compulab.co.il>; Nikita Kiryanov
> > > <nikita@compulab.co.il>; Otavio Salvador <otavio@ossystems.com.br>;
> > > Andreas Geisreiter <ageisreiter@dh-electronics.de>; Ludwig Zenz
> > > <lzenz@dh-electronics.de>; Eric Bénard <eric@eukrea.com>; Peng Fan
> > > <peng.fan@nxp.com>; Jason Liu <jason.hui.liu@nxp.com>; Ye Li
> > > <ye.li@nxp.com>; Adrian Alonso <adrian.alonso@nxp.com>; Alison Wang
> > > <alison.wang@nxp.com>; tharvey@gateworks.com; Ian Ray
> > > <ian.ray@ge.com>; Marcin Niestroj <m.niestroj@grinn-global.com>;
> > > Andrej Rosano <andrej@inversepath.com>; Marek Vasut
> <marex@denx.de>;
> > > Lukasz Majewski <lukma@denx.de>; Adam Ford <aford173@gmail.com>;
> > > Olaf Mandel <o.mandel@menlosystems.com>; Martyn Welch
> > > <martyn.welch@collabora.com>; Ingo Schroeck
> <open-source@samtec.de>;
> > > Boris Brezillon <boris.brezillon@free-electrons.com>; Soeren Moch
> > > <smoch@web.de>; Richard Hu <richard.hu@technexion.com>; Vanessa
> > > Maegima <vanessa.maegima@nxp.com>; Max Krummenacher
> > > <max.krummenacher@toradex.com>; Stefan Agner
> > > <stefan.agner@toradex.com>; Markus Niebel
> > > <Markus.Niebel@tq-group.com>; Breno Matheus Lima
> > > <breno.lima@nxp.com>; Francesco Montefoschi
> > > <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> > > <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>;
> > > Jagan Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> > > <raffaele.recalcati@bticino.it>; Simone CIANNI
> > > <simone.cianni@bticino.it>; Bhaskar Upadhaya
> > > <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> > > <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> > > <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat
> <rajesh.bhagat@nxp.com>;
> > > Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> > > <sebastien.szymanski@armadeus.com>; Lucile Quirion
> > > <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> > > <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> > > Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> > > <denis.zalevskiy@ge.com>; Fabien Lahoudere
> > > <fabien.lahoudere@collabora.com>; Joe Hershberger
> > > <joe.hershberger@ni.com>; Simon Goldschmidt
> > > <simon.k.r.goldschmidt@gmail.com>; James Byrne
> > > <james.byrne@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@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.
> 

[Y.b. Lu] I have sent out v5 patch-set with changes applying esdhc imx driver to mcf5441x.
Please help to verify your platforms.
Travis-CI link for build test:
https://travis-ci.org/yangbolu1991/u-boot-test/builds/540574527

Thanks.

> > > >  /*
> > > >   * 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
Angelo Dureghello June 22, 2019, 9:42 p.m. UTC | #11
Hi Lu,

On Mon, Jun 03, 2019 at 04:28:24AM +0000, Y.b. Lu wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Angelo Dureghello <angelo@sysam.it>
> > Sent: 2019年5月31日 15:15
> > To: Y.b. Lu <yangbo.lu@nxp.com>
> > Cc: u-boot@lists.denx.de
> > Subject: Re: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> > 
> > Hi Lu,
> > 
> > On Fri, May 31, 2019 at 06:12:12AM +0000, Y.b. Lu wrote:
> > > Hi Angelo,
> > >
> > > > -----Original Message-----
> > > > From: Angelo Dureghello <angelo@sysam.it>
> > > > Sent: 2019年5月31日 2:23
> > > > To: Y.b. Lu <yangbo.lu@nxp.com>
> > > > Cc: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio
> > > > Estevam <festevam@gmail.com>; dl-uboot-imx <uboot-imx@nxp.com>;
> > > > Albert Aribaud <albert.u.boot@aribaud.net>; Eddy Petrișor
> > > > <eddy.petrisor@gmail.com>; Akshay Bhat <akshaybhat@timesys.com>;
> > Ken
> > > > Lin <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>;
> > > > Christian Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese
> > > > <sr@denx.de>; Patrick Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> > > > <troy.kisky@boundarydevices.com>; Uri Mashiach
> > > > <uri.mashiach@compulab.co.il>; Nikita Kiryanov
> > > > <nikita@compulab.co.il>; Otavio Salvador <otavio@ossystems.com.br>;
> > > > Andreas Geisreiter <ageisreiter@dh-electronics.de>; Ludwig Zenz
> > > > <lzenz@dh-electronics.de>; Eric Bénard <eric@eukrea.com>; Peng Fan
> > > > <peng.fan@nxp.com>; Jason Liu <jason.hui.liu@nxp.com>; Ye Li
> > > > <ye.li@nxp.com>; Adrian Alonso <adrian.alonso@nxp.com>; Alison Wang
> > > > <alison.wang@nxp.com>; tharvey@gateworks.com; Ian Ray
> > > > <ian.ray@ge.com>; Marcin Niestroj <m.niestroj@grinn-global.com>;
> > > > Andrej Rosano <andrej@inversepath.com>; Marek Vasut
> > <marex@denx.de>;
> > > > Lukasz Majewski <lukma@denx.de>; Adam Ford <aford173@gmail.com>;
> > > > Olaf Mandel <o.mandel@menlosystems.com>; Martyn Welch
> > > > <martyn.welch@collabora.com>; Ingo Schroeck
> > <open-source@samtec.de>;
> > > > Boris Brezillon <boris.brezillon@free-electrons.com>; Soeren Moch
> > > > <smoch@web.de>; Richard Hu <richard.hu@technexion.com>; Vanessa
> > > > Maegima <vanessa.maegima@nxp.com>; Max Krummenacher
> > > > <max.krummenacher@toradex.com>; Stefan Agner
> > > > <stefan.agner@toradex.com>; Markus Niebel
> > > > <Markus.Niebel@tq-group.com>; Breno Matheus Lima
> > > > <breno.lima@nxp.com>; Francesco Montefoschi
> > > > <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> > > > <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>;
> > > > Jagan Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> > > > <raffaele.recalcati@bticino.it>; Simone CIANNI
> > > > <simone.cianni@bticino.it>; Bhaskar Upadhaya
> > > > <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> > > > <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> > > > <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat
> > <rajesh.bhagat@nxp.com>;
> > > > Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> > > > <sebastien.szymanski@armadeus.com>; Lucile Quirion
> > > > <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> > > > <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> > > > Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> > > > <denis.zalevskiy@ge.com>; Fabien Lahoudere
> > > > <fabien.lahoudere@collabora.com>; Joe Hershberger
> > > > <joe.hershberger@ni.com>; Simon Goldschmidt
> > > > <simon.k.r.goldschmidt@gmail.com>; James Byrne
> > > > <james.byrne@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@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.
> > 
> 
> [Y.b. Lu] I have sent out v5 patch-set with changes applying esdhc imx driver to mcf5441x.
> Please help to verify your platforms.
> Travis-CI link for build test:
> https://travis-ci.org/yangbolu1991/u-boot-test/builds/540574527
> 
> Thanks.
> 
> > > > >  /*
> > > > >   * 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
> > > > >
> > 

Sorry for the late testing.

Tested, it works on ColdFire mcf54415. But please add if possible the _IMX 
(CONFIG_FSL_ESDHC_IMX) for the sysam/stmark2 board too. 


Tested-by: Angelo Dureghello <angelo@sysam.it>


> > Regards,
> > Angelo Dureghello

Thanks,
Regards
Angelo Dureghello
Yangbo Lu June 24, 2019, 1:23 a.m. UTC | #12
Hi Angelo,

> -----Original Message-----
> From: Angelo Dureghello <angelo@sysam.it>
> Sent: 2019年6月23日 5:43
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: u-boot@lists.denx.de
> Subject: Re: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> 
> Hi Lu,
> 
> On Mon, Jun 03, 2019 at 04:28:24AM +0000, Y.b. Lu wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Angelo Dureghello <angelo@sysam.it>
> > > Sent: 2019年5月31日 15:15
> > > To: Y.b. Lu <yangbo.lu@nxp.com>
> > > Cc: u-boot@lists.denx.de
> > > Subject: Re: [v3, 5/5] mmc: fsl_esdhc_imx: drop useless code
> > >
> > > Hi Lu,
> > >
> > > On Fri, May 31, 2019 at 06:12:12AM +0000, Y.b. Lu wrote:
> > > > Hi Angelo,
> > > >
> > > > > -----Original Message-----
> > > > > From: Angelo Dureghello <angelo@sysam.it>
> > > > > Sent: 2019年5月31日 2:23
> > > > > To: Y.b. Lu <yangbo.lu@nxp.com>
> > > > > Cc: u-boot@lists.denx.de; Stefano Babic <sbabic@denx.de>; Fabio
> > > > > Estevam <festevam@gmail.com>; dl-uboot-imx
> <uboot-imx@nxp.com>;
> > > > > Albert Aribaud <albert.u.boot@aribaud.net>; Eddy Petrișor
> > > > > <eddy.petrisor@gmail.com>; Akshay Bhat <akshaybhat@timesys.com>;
> > > Ken
> > > > > Lin <Ken.Lin@advantech.com.tw>; Heiko Schocher <hs@denx.de>;
> > > > > Christian Gmeiner <christian.gmeiner@gmail.com>; Stefan Roese
> > > > > <sr@denx.de>; Patrick Bruenn <p.bruenn@beckhoff.com>; Troy Kisky
> > > > > <troy.kisky@boundarydevices.com>; Uri Mashiach
> > > > > <uri.mashiach@compulab.co.il>; Nikita Kiryanov
> > > > > <nikita@compulab.co.il>; Otavio Salvador
> > > > > <otavio@ossystems.com.br>; Andreas Geisreiter
> > > > > <ageisreiter@dh-electronics.de>; Ludwig Zenz
> > > > > <lzenz@dh-electronics.de>; Eric Bénard <eric@eukrea.com>; Peng
> > > > > Fan <peng.fan@nxp.com>; Jason Liu <jason.hui.liu@nxp.com>; Ye Li
> > > > > <ye.li@nxp.com>; Adrian Alonso <adrian.alonso@nxp.com>; Alison
> > > > > Wang <alison.wang@nxp.com>; tharvey@gateworks.com; Ian Ray
> > > > > <ian.ray@ge.com>; Marcin Niestroj <m.niestroj@grinn-global.com>;
> > > > > Andrej Rosano <andrej@inversepath.com>; Marek Vasut
> > > <marex@denx.de>;
> > > > > Lukasz Majewski <lukma@denx.de>; Adam Ford
> <aford173@gmail.com>;
> > > > > Olaf Mandel <o.mandel@menlosystems.com>; Martyn Welch
> > > > > <martyn.welch@collabora.com>; Ingo Schroeck
> > > <open-source@samtec.de>;
> > > > > Boris Brezillon <boris.brezillon@free-electrons.com>; Soeren
> > > > > Moch <smoch@web.de>; Richard Hu <richard.hu@technexion.com>;
> > > > > Vanessa Maegima <vanessa.maegima@nxp.com>; Max Krummenacher
> > > > > <max.krummenacher@toradex.com>; Stefan Agner
> > > > > <stefan.agner@toradex.com>; Markus Niebel
> > > > > <Markus.Niebel@tq-group.com>; Breno Matheus Lima
> > > > > <breno.lima@nxp.com>; Francesco Montefoschi
> > > > > <francesco.montefoschi@udoo.org>; Parthiban Nallathambi
> > > > > <parthitce@gmail.com>; Albert ARIBAUD <albert.aribaud@3adev.fr>;
> > > > > Jagan Teki <jagan@amarulasolutions.com>; Raffaele RECALCATI
> > > > > <raffaele.recalcati@bticino.it>; Simone CIANNI
> > > > > <simone.cianni@bticino.it>; Bhaskar Upadhaya
> > > > > <bhaskar.upadhaya@nxp.com>; Vinitha V Pillai
> > > > > <vinitha.pillai@nxp.com>; Prabhakar Kushwaha
> > > > > <prabhakar.kushwaha@nxp.com>; Rajesh Bhagat
> > > <rajesh.bhagat@nxp.com>;
> > > > > Antti Mäentausta <antti.maentausta@ge.com>; Sébastien Szymanski
> > > > > <sebastien.szymanski@armadeus.com>; Lucile Quirion
> > > > > <lucile.quirion@savoirfairelinux.com>; Alexey Brodkin
> > > > > <abrodkin@synopsys.com>; Trevor Woerner <trevor@toganlabs.com>;
> > > > > Anatolij Gustschin <agust@denx.de>; Denis Zalevskiy
> > > > > <denis.zalevskiy@ge.com>; Fabien Lahoudere
> > > > > <fabien.lahoudere@collabora.com>; Joe Hershberger
> > > > > <joe.hershberger@ni.com>; Simon Goldschmidt
> > > > > <simon.k.r.goldschmidt@gmail.com>; James Byrne
> > > > > <james.byrne@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@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.
> > >
> >
> > [Y.b. Lu] I have sent out v5 patch-set with changes applying esdhc imx driver
> to mcf5441x.
> > Please help to verify your platforms.
> > Travis-CI link for build test:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrav
> >
> is-ci.org%2Fyangbolu1991%2Fu-boot-test%2Fbuilds%2F540574527&amp;dat
> a=0
> >
> 2%7C01%7Cyangbo.lu%40nxp.com%7Cc1ccc4bbbeb14f275f2208d6f75a8cf4%
> 7C686e
> >
> a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636968365631078109&am
> p;sdata=1W
> >
> lanMwum7TfEzNHJwIKifUnYU%2Fw9%2FOeNNKI39TGRuU%3D&amp;reserve
> d=0
> >
> > Thanks.
> >
> > > > > >  /*
> > > > > >   * 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
> > > > > >
> > >
> 
> Sorry for the late testing.
> 
> Tested, it works on ColdFire mcf54415. But please add if possible the _IMX
> (CONFIG_FSL_ESDHC_IMX) for the sysam/stmark2 board too.

[Y.b. Lu] Thanks a lot for your testing.
In default, CONFIG_FSL_ESDHC wasn't enabled. We can send another patch to enable CONFIG_FSL_ESDHC_IMX for ColdFire mcf54415 after this patch-set was applied.

> 
> 
> Tested-by: Angelo Dureghello <angelo@sysam.it>
> 
> 
> > > Regards,
> > > Angelo Dureghello
> 
> Thanks,
> Regards
> Angelo Dureghello
Marek Vasut June 24, 2019, 2:05 a.m. UTC | #13
On 5/31/19 8:12 AM, Y.b. Lu wrote:
> Hi Angelo,
> 
>> -----Original Message-----
>> From: Angelo Dureghello <...>
>> Sent: 2019年5月31日 2:23
>> To: Y.b. Lu <yangbo.lu@nxp.com>
>> Cc: ; Stefano Babic <...>; Fabio Estevam <...>
[...]

Can you please fix your mailer and stop quoting everyone's email in the
body of your reply ? It only feeds the spambots and has no value.
In fact, this whole "-- Original Message --" stuff quoting the headers
is useless, quote only the message body.
Angelo Dureghello June 24, 2019, 9:26 a.m. UTC | #14
Hi Lu,


-snip-

> > > >
> > 
> > Sorry for the late testing.
> > 
> > Tested, it works on ColdFire mcf54415. But please add if possible the _IMX
> > (CONFIG_FSL_ESDHC_IMX) for the sysam/stmark2 board too.
> 
> [Y.b. Lu] Thanks a lot for your testing.
> In default, CONFIG_FSL_ESDHC wasn't enabled. We can send another patch to enable CONFIG_FSL_ESDHC_IMX for ColdFire mcf54415 after this patch-set was applied.
> 

Sorry, realized CONFIG_FSL_ESDHC was enabled only here locally.
Don't worry about, sent a patch for it, in case i'll refix it later on
to CONFIG_FSL_ESDHC_IMX.

> > 
> > 
> > Tested-by: Angelo Dureghello <angelo@sysam.it>
> > 
> > 
> > > > Regards,
> > > > Angelo Dureghello
> > 
> > Thanks,
> > Regards
> > Angelo Dureghello

Thanks,
Regards,
Angelo Dureghello
diff mbox series

Patch

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
-
 /*
  * 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);
-#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
-
 #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