[U-Boot] [PATCH] fsl_esdhc: Add no-snoop config for default init

[Benoît Thébaudeau]

Hi Stefano,

On 08/11/2012 12:43, Stefano Babic wrote:
> On 10/08/2012 23:13, Benoît Thébaudeau wrote:
> > cfg->no_snoop was available only through custom fsl_esdhc_mmc init
> > functions.
> > Add a config options to make it available also through the default
> > init.
> > 
> 
> Hi Benoît,
> 
> > Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau at advansee.com>
> > Cc: Andy Fleming <afleming at gmail.com>
> > Cc: Stefano Babic <sbabic at denx.de>
> > Cc: Kim Phillips <kim.phillips at freescale.com>
> > ---
> >  .../drivers/mmc/fsl_esdhc.c                        |    3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git u-boot-4d3c95f.orig/drivers/mmc/fsl_esdhc.c
> > u-boot-4d3c95f/drivers/mmc/fsl_esdhc.c
> > index b6c969d..21154e5 100644
> > --- u-boot-4d3c95f.orig/drivers/mmc/fsl_esdhc.c
> > +++ u-boot-4d3c95f/drivers/mmc/fsl_esdhc.c
> > @@ -597,6 +597,9 @@ int fsl_esdhc_mmc_init(bd_t *bis)
> >  	cfg = malloc(sizeof(struct fsl_esdhc_cfg));
> >  	memset(cfg, 0, sizeof(struct fsl_esdhc_cfg));
> >  	cfg->esdhc_base = CONFIG_SYS_FSL_ESDHC_ADDR;
> > +#ifdef CONFIG_SYS_FSL_ESDHC_NO_SNOOP
> > +	cfg->no_snoop = 1;
> > +#endif
> >  	return fsl_esdhc_initialize(bis, cfg);
> >  }
> >  
> 
> Using a structure to setup the controller avoids to add further
> CONFIG_
> switch. I do not find the needs to have both. Why do you need ?

fsl_esdhc_mmc_init() exists, so it should give access to all features. Moreover,
it's shorter to #define a config option than to create custom fsl_esdhc_mmc init
functions when only a single eSDHC instance is needed.

Besides that, the cache snooping feature is not available on i.MX, which means
that without CONFIG_SYS_FSL_ESDHC_NO_SNOOP or an equivalent, boards using
fsl_esdhc_mmc_init() will make esdhc_init() access a reserved bit of a reserved
register, which is not really recommended.

On i.MX, the patch could also be improved by not making this option available,
but behaving as if it were set.

Best regards,
Benoît