[PATCH 1/3 v2] sdhci: Add auto CMD12 support for eSDHC driver

[Zang Roy-R61911]

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru at gmail.com] 
> Sent: Tuesday, August 10, 2010 1:44 AM
> To: Grant Likely
> Cc: Zang Roy-R61911; linuxppc-dev at ozlabs.org; 
> akpm at linux-foundation.org; linux-mmc at vger.kernel.org
> Subject: Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for 
> eSDHC driver
> 
> On Wed, Aug 04, 2010 at 07:02:56PM -0600, Grant Likely wrote:
> > On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang 
> <tie-fei.zang at freescale.com> wrote:
> > > From: Jerry Huang <Chang-Ming.Huang at freescale.com>
> > >
> > > Add auto CMD12 command support for eSDHC driver.
> > > This is needed by P4080 and P1022 for block read/write.
> > > Manual asynchronous CMD12 abort operation causes protocol 
> violations on
> > > these silicons.
> > >
> > > Signed-off-by: Jerry Huang <Chang-Ming.Huang at freescale.com>
> > > Signed-off-by: Roy Zang <tie-fei.zang at freescale.com>
> > > ---
> > >  drivers/mmc/host/sdhci-of-core.c |    4 ++++
> > >  drivers/mmc/host/sdhci.c         |   14 ++++++++++++--
> > >  drivers/mmc/host/sdhci.h         |    2 ++
> > >  3 files changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index c6d1bd8..a92566e 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -817,8 +817,12 @@ static void 
> sdhci_set_transfer_mode(struct sdhci_host *host,
> > >        WARN_ON(!host->data);
> > >
> > >        mode = SDHCI_TRNS_BLK_CNT_EN;
> > > -       if (data->blocks > 1)
> > > -               mode |= SDHCI_TRNS_MULTI;
> > > +       if (data->blocks > 1) {
> > > +               if (host->quirks & 
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > > +                       mode |= SDHCI_TRNS_MULTI | 
> SDHCI_TRNS_ACMD12;
> > > +               else
> > > +                       mode |= SDHCI_TRNS_MULTI;
> > 
> > nit:
> > +               mode |= SDHCI_TRNS_MULTI;
> > +               if (host->quirks & 
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > +                       mode |= SDHCI_TRNS_ACMD12;
> > 
> > Clearer, no?
> 
> Much clearer. I would prefer the nit incorporated.
Agree.

> 
> Another nit:
> 
> > @@ -154,6 +154,10 @@ static int __devinit 
> sdhci_of_probe(struct of_device *ofdev,
> >                 host->ops = &sdhci_of_data->ops;
> >         }
> > 
> > +       if (of_get_property(np, "sdhci,auto-cmd12", NULL))
> > +               host->quirks |= SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12;
> > +
> > +      
> 
> ^^ No need for the two empty lines.
Agree.
> 
> >        if (of_get_property(np, "sdhci,1-bit-only", NULL))
> 
> Though, technically the patch looks OK, feel free to add my
> 
>   Acked-by: Anton Vorontsov <cbouatmailru at gmail.com>
I send a new patch to fix the nip. you are Cced.
Thanks.
Roy