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(&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
> >
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to