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.
> > > /*
> > > * 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
> > >
Regards,
Angelo Dureghello
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
