Hi Angelo,
> -----Original Message-----
> From: Angelo Dureghello <ang...@sysam.it>
> Sent: 2019年6月23日 5:43
> To: Y.b. Lu <yangbo...@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 <ang...@sysam.it>
> > > Sent: 2019年5月31日 15:15
> > > To: Y.b. Lu <yangbo...@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 <ang...@sysam.it>
> > > > > Sent: 2019年5月31日 2:23
> > > > > To: Y.b. Lu <yangbo...@nxp.com>
> > > > > Cc: u-boot@lists.denx.de; Stefano Babic <sba...@denx.de>; Fabio
> > > > > Estevam <feste...@gmail.com>; dl-uboot-imx
> <uboot-...@nxp.com>;
> > > > > Albert Aribaud <albert.u.b...@aribaud.net>; Eddy Petrișor
> > > > > <eddy.petri...@gmail.com>; Akshay Bhat <akshayb...@timesys.com>;
> > > Ken
> > > > > Lin <ken....@advantech.com.tw>; Heiko Schocher <h...@denx.de>;
> > > > > Christian Gmeiner <christian.gmei...@gmail.com>; Stefan Roese
> > > > > <s...@denx.de>; Patrick Bruenn <p.bru...@beckhoff.com>; Troy Kisky
> > > > > <troy.ki...@boundarydevices.com>; Uri Mashiach
> > > > > <uri.mashi...@compulab.co.il>; Nikita Kiryanov
> > > > > <nik...@compulab.co.il>; Otavio Salvador
> > > > > <ota...@ossystems.com.br>; Andreas Geisreiter
> > > > > <ageisrei...@dh-electronics.de>; Ludwig Zenz
> > > > > <lz...@dh-electronics.de>; Eric Bénard <e...@eukrea.com>; Peng
> > > > > Fan <peng....@nxp.com>; Jason Liu <jason.hui....@nxp.com>; Ye Li
> > > > > <ye...@nxp.com>; Adrian Alonso <adrian.alo...@nxp.com>; Alison
> > > > > Wang <alison.w...@nxp.com>; thar...@gateworks.com; Ian Ray
> > > > > <ian....@ge.com>; Marcin Niestroj <m.niest...@grinn-global.com>;
> > > > > Andrej Rosano <and...@inversepath.com>; Marek Vasut
> > > <ma...@denx.de>;
> > > > > Lukasz Majewski <lu...@denx.de>; Adam Ford
> <aford...@gmail.com>;
> > > > > Olaf Mandel <o.man...@menlosystems.com>; Martyn Welch
> > > > > <martyn.we...@collabora.com>; Ingo Schroeck
> > > <open-sou...@samtec.de>;
> > > > > Boris Brezillon <boris.brezil...@free-electrons.com>; Soeren
> > > > > Moch <sm...@web.de>; Richard Hu <richard...@technexion.com>;
> > > > > Vanessa Maegima <vanessa.maeg...@nxp.com>; Max Krummenacher
> > > > > <max.krummenac...@toradex.com>; Stefan Agner
> > > > > <stefan.ag...@toradex.com>; Markus Niebel
> > > > > <markus.nie...@tq-group.com>; Breno Matheus Lima
> > > > > <breno.l...@nxp.com>; Francesco Montefoschi
> > > > > <francesco.montefos...@udoo.org>; Parthiban Nallathambi
> > > > > <parthi...@gmail.com>; Albert ARIBAUD <albert.arib...@3adev.fr>;
> > > > > Jagan Teki <ja...@amarulasolutions.com>; Raffaele RECALCATI
> > > > > <raffaele.recalc...@bticino.it>; Simone CIANNI
> > > > > <simone.cia...@bticino.it>; Bhaskar Upadhaya
> > > > > <bhaskar.upadh...@nxp.com>; Vinitha V Pillai
> > > > > <vinitha.pil...@nxp.com>; Prabhakar Kushwaha
> > > > > <prabhakar.kushw...@nxp.com>; Rajesh Bhagat
> > > <rajesh.bha...@nxp.com>;
> > > > > Antti Mäentausta <antti.maentau...@ge.com>; Sébastien Szymanski
> > > > > <sebastien.szyman...@armadeus.com>; Lucile Quirion
> > > > > <lucile.quir...@savoirfairelinux.com>; Alexey Brodkin
> > > > > <abrod...@synopsys.com>; Trevor Woerner <tre...@toganlabs.com>;
> > > > > Anatolij Gustschin <ag...@denx.de>; Denis Zalevskiy
> > > > > <denis.zalevs...@ge.com>; Fabien Lahoudere
> > > > > <fabien.lahoud...@collabora.com>; Joe Hershberger
> > > > > <joe.hershber...@ni.com>; Simon Goldschmidt
> > > > > <simon.k.r.goldschm...@gmail.com>; James Byrne
> > > > > <james.by...@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...@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(®s->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(®s->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&dat
> a=0
> >
> 2%7C01%7Cyangbo.lu%40nxp.com%7Cc1ccc4bbbeb14f275f2208d6f75a8cf4%
> 7C686e
> >
> a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636968365631078109&am
> p;sdata=1W
> >
> lanMwum7TfEzNHJwIKifUnYU%2Fw9%2FOeNNKI39TGRuU%3D&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(®s->irqstaten, IRQSTATEN_BRR |
> > > > > > IRQSTATEN_BWR);
> > > > > >
> > > > > > -#ifdef CONFIG_MCF5441x
> > > > > > - esdhc_write32(®s->proctl, PROCTL_INIT | PROCTL_D3CD);
> > > > > > -#else
> > > > > > /* Put the PROCTL reg back to the default */
> > > > > > esdhc_write32(®s->proctl, PROCTL_INIT); -#endif
> > > > > >
> > > > > > /* Set timout to the maximum value */
> > > > > > esdhc_clrsetbits32(®s->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(®s->proctl, PROCTL_INIT | PROCTL_D3CD);
> > > > > > -#endif
> > > > > > -
> > > > > > #ifndef CONFIG_FSL_USDHC
> > > > > > esdhc_setbits32(®s->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(®s->hostcapblt);
> > > > > >
> > > > > > -#ifdef CONFIG_MCF5441x
> > > > > > - /*
> > > > > > - * MCF5441x RM declares in more points that sdhc clock speed
> > > must
> > > > > > - * never exceed 25 Mhz. From this, the HS bit needs to be
> > > > > > disabled
> > > > > > - * from host capabilities.
> > > > > > - */
> > > > > > - caps &= ~ESDHC_HOSTCAPBLT_HSS;
> > > > > > -#endif
> > > > >
> > > > > 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 <ang...@sysam.it>
>
>
> > > Regards,
> > > Angelo Dureghello
>
> Thanks,
> Regards
> Angelo Dureghello
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
