On Mon, Feb 07, 2011 at 05:54:24PM +0000, Chris Ball wrote:
> Hi Wolfram, thanks for the review,
>
> On Mon, Feb 07, 2011 at 06:40:39PM +0100, Wolfram Sang wrote:
> > On Sun, Feb 06, 2011 at 01:13:10AM -0500, Chris Ball wrote:
> > > Part of a quirk cleanup run. This quirk was only used by sdhci-esdhc.
> > > This patch is untested.
> > >
> > > Signed-off-by: Chris Ball <cjb@xxxxxxxxxx>
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index 9e15f41..fcd6188 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -1962,8 +1962,8 @@ int sdhci_add_host(struct sdhci_host *host)
> > > * Maximum block size. This varies from controller to controller and
> > > * is specified in the capabilities register.
> > > */
> > > - if (host->quirks & SDHCI_QUIRK_FORCE_BLK_SZ_2048) {
> > > - mmc->max_blk_size = 2;
> > > + if (host->ops->get_max_blk_size) {
> > > + mmc->max_blk_size = host->ops->get_max_blk_size(host);
> > > } else {
> > > mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >>
> > > SDHCI_MAX_BLOCK_SHIFT;
> >
> > I tend to think this could be fixed using io-accessors when reading the
> > caps register?
>
> Hm, I agree that it would work, but I'm not sure it's going to be
> cleaner/more readable that way. I like that here we have one place
> for setting the max_blk_size, and it's obvious from the code exactly
> where you need to look to see if it's been overloaded by the driver.
Without the quirk and its handling, we lose the if-else-block which will
lead to:
mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >>
SDHCI_MAX_BLOCK_SHIFT;
if (mmc->max_blk_size >= 3) {
printk(KERN_WARNING "%s: Invalid maximum block size, "
"assuming 512 bytes\n", mmc_hostname(mmc));
mmc->max_blk_size = 0;
}
This is what I would prefer. Straight code, handling an sdhci-controller
adhering to the standard. A controller failing to do that, will have to take
care of that in its specific source file. For the sdhci-core, it would then
simply look like a "good" controller. We probably can't achieve this for all
quirks, but still...
Regards,
Wolfram
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Attachment:
signature.asc
Description: Digital signature
