[U-Boot] [PATCH 2/2] PXAMCI Monahans support

[Marek Vasut]

Dne So 24. dubna 2010 03:54:19 Andy Fleming napsal(a):
> On Fri, Apr 23, 2010 at 8:37 PM, Marek Vasut <marek.vasut at gmail.com> wrote:
> > Dne So 24. dubna 2010 03:22:08 Andy Fleming napsal(a):
> >> On Fri, Mar 26, 2010 at 12:57 AM, Marek Vasut <marek.vasut at gmail.com> 
wrote:
> >> > ---
> >> >  drivers/mmc/pxa_mmc.c |   13 ++++++++-----
> >> >  1 files changed, 8 insertions(+), 5 deletions(-)
> >> > 
> >> > diff --git a/drivers/mmc/pxa_mmc.c b/drivers/mmc/pxa_mmc.c
> >> > index 8225235..18d5df9 100644
> >> > --- a/drivers/mmc/pxa_mmc.c
> >> > +++ b/drivers/mmc/pxa_mmc.c
> >> > @@ -126,7 +126,7 @@ mmc_block_read(uchar * dst, ulong src, ulong len)
> >> >        MMC_I_MASK = ~MMC_I_MASK_RXFIFO_RD_REQ;
> >> >        while (len) {
> >> >                if (MMC_I_REG & MMC_I_REG_RXFIFO_RD_REQ) {
> >> > -#ifdef CONFIG_PXA27X
> >> > +#if defined(CONFIG_PXA27X) || defined(CONFIG_CPU_MONAHANS)
> >> >                        int i;
> >> >                        for (i = min(len, 32); i; i--) {
> >> >                                *dst++ = *((volatile uchar
> >> > *)&MMC_RXFIFO); @@ -558,8 +558,11 @@ mmc_legacy_init(int verbose)
> >> >        set_GPIO_mode(GPIO6_MMCCLK_MD);
> >> >        set_GPIO_mode(GPIO8_MMCCS0_MD);
> >> >  #endif
> >> > +#ifdef CONFIG_CPU_MONAHANS     /* pxa3xx */
> >> > +       CKENA |= CKENA_12_MMC0 | CKENA_13_MMC1;
> >> > +#else  /* pxa2xx */
> >> 
> >> Let's try to find a solution to this that doesn't require filling this
> >> driver with more #ifdefs.
> > 
> > That won't make too much sense. You won't run the same binary on both
> > pxa2xx and pxa3xx ... ever. Increasing the bootloader size only because
> > you want to avoid ifdef makes no sense either.
> 
> Just to be clear, I'm not objecting to #ifdefs.  I'm objecting to
> #ifdefs that cause the driver to become more and more rigidly tied to
> the system.  As more and more systems use this driver, such #ifdefs
> become impossible to manage, and very hard to read.  I don't want the
> code to be binary-compatible.  I just want the code to be independent
> of the board and cpu.

And how do you expect a CPU series-specific driver be CPU-independent?

btw. please don't remove the u-bootML from the mail.
> 
> I'd rather the #ifdef were something that could be reused by a
> non-monahans cpu, unless the fields being set are specific to the
> monahans cpu and only will ever be used by it.  Otherwise, come up
> with another name, please.

This will only ever be used by PXA3xx aka monahans. PXA168 might use it as well, 
but the TRM is still NDAed.
> 
> > There are other ifdefs in that driver that need to go away (will in
> > further patches), this one is the least harmful one.
> 
> Yeah, I suspected that was the case.  However, doesn't this patch
> conflict with the other one which removes the #ifdef CONFIG_PXA27X  ?

No, the one removing ifdef PXA27X fixes a problem with the card timing out on 
both monahans and pxa27x because the delay was too short (and it didn't make 
sense why it even was there that way, probably someone measured it on his board, 
it worked for him and that value was board-specific!). The PXAMCI haven't 
changed much ever since pxa21x. The clock management did change since pxa27x, 
that's why there's the new ifdef.

PXA21x,PXA25x,PXA26x,PXA27x -- CKEN clock enable reg
PXA3xx -- CKENA/CKENB clock enable regs

Makes the ifdef clear finally I hope.
> 
> Andy