Subject: Re: [PATCH 3/3] mmc: Add ESDHC weird register workaround

From: Anton Vorontsov <[email-masked]>
Date: Fri, 30 Jul 2010 11:37:31 +0400

On Wed, Jul 28, 2010 at 05:43:00PM +0800, Zang Roy-R61911 wrote:
[...]
> > > +	/* 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?

See drivers/mmc/host/sdhci-of-esdhc.c

struct sdhci_of_data sdhci_esdhc = {
...
        .ops = {
...
                .write_l = sdhci_be32bs_writel,
...
        },
};

You can implement your own write_l and put the quirk in
there, as we do for esdhc_readw, esdhc_writew, etc.

OTOH, as you say it has wrong default value, not behaviour,
so I think you can just initialize the HOST_CONTROL register
just once somewhere in sdhci-of-esdhc, e.g. ->enable_dma
callback?

> It makes more sense to define something like:
> SDHCI_QUIRK_QORIQ_PROCTL_WEIRD.

I don't think so.

Thanks,

-- 
Anton Vorontsov
email: [email-masked]
irc://irc.freenode.net/bd2