> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmai...@gmail.com]
> Sent: Wednesday, July 28, 2010 16:03 PM
> To: Zang Roy-R61911
> Cc: linux-mmc-appro...@vger.kernel.org; linuxppc-...@ozlabs.org
> Subject: Re: [PATCH 3/3] mmc: Add ESDHC weird register workaround
>
> On Wed, Jul 28, 2010 at 01:54:21PM +0800, Roy Zang wrote:
> > P4080 ESDHC controller induces weird register setting.
> > This patch adds the workaround to correct the weird
> register setting.
> >
> > Signed-off-by: Roy Zang <tie-fei.z...@freescale.com>
> > ---
> > drivers/mmc/host/sdhci-of-core.c | 5 +++++
> > drivers/mmc/host/sdhci.c | 13 +++++++++++++
> > drivers/mmc/host/sdhci.h | 2 ++
> > 3 files changed, 20 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-core.c
> b/drivers/mmc/host/sdhci-of-core.c
> > index 0c30242..1b6945a 100644
> > --- a/drivers/mmc/host/sdhci-of-core.c
> > +++ b/drivers/mmc/host/sdhci-of-core.c
> > @@ -164,6 +164,11 @@ static int __devinit
> sdhci_of_probe(struct of_device *ofdev,
> > if (sdhci_of_wp_inverted(np))
> > host->quirks |= SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
> >
> > + if (of_device_is_compatible(np, "fsl,p4080-esdhc")) {
> > + host->quirks |= SDHCI_QUIRK_QORIQ_REG_WEIRD;
> > + host->quirks &= ~SDHCI_QUIRK_INVERTED_WRITE_PROTECT;
>
> You do not mention this change in the patch description, why?
This is not needed any more. Will be removed.
>
> > + }
> > +
> > clk = of_get_property(np, "clock-frequency", &size);
> > if (clk && size == sizeof(*clk) && *clk)
> > of_host->clock = *clk;
> > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > index 1424d08..b5b3627 100644
> > --- a/drivers/mmc/host/sdhci.c
> > +++ b/drivers/mmc/host/sdhci.c
> > @@ -788,6 +788,15 @@ static void sdhci_prepare_data(struct
> sdhci_host *host, struct mmc_data *data)
> > sdhci_writeb(host, ctrl, SDHCI_HOST_CONTROL);
> > }
> >
> > + /* The default value of DMAS bits of Protocol Control
> Register is not
> > + * correct. clear these two bits to use simple DMA */
> > +#define ESDHCI_CTRL_DMAS_MASK 0xFFFFFCFF
> > + if (host->quirks & SDHCI_QUIRK_QORIQ_REG_WEIRD) {
> > + ctrl = sdhci_readl(host, SDHCI_HOST_CONTROL);
> > + ctrl = ctrl & ESDHCI_CTRL_DMAS_MASK;
> > + sdhci_writel(host, ctrl, SDHCI_HOST_CONTROL);
> > + }
>
> You should implement register quirks via SDHCI IO accessors.
What does you mean "SDHCI IO accessors?
It makes more sense to define something like:
SDHCI_QUIRK_QORIQ_PROCTL_WEIRD.
Any comment?
>
> See esdhc_writew() for example.
>
> > if (!(host->flags & SDHCI_REQ_USE_DMA)) {
> > int flags;
> >
> > @@ -1699,6 +1708,10 @@ int sdhci_add_host(struct sdhci_host *host)
> >
> > caps = sdhci_readl(host, SDHCI_CAPABILITIES);
> >
> > + /* Workaround for P4080 host controller capabilities */
> > + if (host->quirks & SDHCI_QUIRK_QORIQ_REG_WEIRD)
> > + caps &= ~(SDHCI_CAN_VDD_180 | SDHCI_CAN_VDD_330);
>
> Ditto. Make a quirk for cap register. Or implement
> SDHCI_QUIRK_NO_VDD_180 and SDHCI_QUIRK_NO_VDD_300 quirks
> instead.
Second proposal more makes sense to me.
Thanks.
Roy
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
