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

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

[Roy Zang]

From: Jerry Huang <Chang-Ming.Huang@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@freescale.com>
Signed-off-by: Roy Zang <tie-fei.zang@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-of-core.c b/drivers/mmc/host/sdhci-of-core.c
index a2e9820..dd1bdd1 100644
--- a/drivers/mmc/host/sdhci-of-core.c
+++ b/drivers/mmc/host/sdhci-of-core.c
@@ -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;
+
+
 	if (of_get_property(np, "sdhci,1-bit-only", NULL))
 		host->quirks |= SDHCI_QUIRK_FORCE_1_BIT_DATA;
 
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;
+	}
 	if (data->flags & MMC_DATA_READ)
 		mode |= SDHCI_TRNS_READ;
 	if (host->flags & SDHCI_REQ_USE_DMA)
@@ -1108,6 +1112,12 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)
 #ifndef SDHCI_USE_LEDS_CLASS
 	sdhci_activate_led(host);
 #endif
+	if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12) {
+		if (mrq->stop) {
+			mrq->data->stop = NULL;
+			mrq->stop = NULL;
+		}
+	}
 
 	host->mrq = mrq;
 
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index c846813..8fb088c 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -240,6 +240,8 @@ struct sdhci_host {
 #define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<25)
 /* Controller cannot support End Attribute in NOP ADMA descriptor */
 #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
+/* Controller uses Auto CMD12 command to stop the transfer */
+#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<27)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */
-- 
1.5.6.5

[PATCH 2/3 v2] dts: Add sdhci,auto-cmd12 field for p4080 device tree

[Roy Zang]

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 Documentation/powerpc/dts-bindings/fsl/esdhc.txt |    2 ++
 arch/powerpc/boot/dts/p4080ds.dts                |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
index 8a00407..64bcb8b 100644
--- a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
+++ b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
@@ -14,6 +14,8 @@ Required properties:
     reports inverted write-protect state;
   - sdhci,1-bit-only : (optional) specifies that a controller can
     only handle 1-bit data transfers.
+  - sdhci,auto-cmd12: (optional) specifies that a controller can
+    only handle auto CMD12.
 
 Example:
 
diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts
index 6b29eab..efa0091 100644
--- a/arch/powerpc/boot/dts/p4080ds.dts
+++ b/arch/powerpc/boot/dts/p4080ds.dts
@@ -280,6 +280,7 @@
 			reg = <0x114000 0x1000>;
 			interrupts = <48 2>;
 			interrupt-parent = <&mpic>;
+			sdhci,auto-cmd12;
 		};
 
 		i2c@118000 {
-- 
1.5.6.5

[PATCH 3/3 v2] dts: Add ESDHC weird voltage bits workaround

[Roy Zang]

P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the
host controller capabilities register wrongly set the bits.
This patch adds the workaround to correct the weird voltage setting bits.
Only 3.3V voltage is supported for P4080 ESDHC controller.

Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
---
 arch/powerpc/boot/dts/p4080ds.dts |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts
index efa0091..2f0de24 100644
--- a/arch/powerpc/boot/dts/p4080ds.dts
+++ b/arch/powerpc/boot/dts/p4080ds.dts
@@ -280,6 +280,7 @@
 			reg = <0x114000 0x1000>;
 			interrupts = <48 2>;
 			interrupt-parent = <&mpic>;
+			voltage-ranges = <3300 3300>;
 			sdhci,auto-cmd12;
 		};
 
-- 
1.5.6.5

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

[Andrew Morton]

On Tue, 3 Aug 2010 11:11:10 +0800
Roy Zang <tie-fei.zang@freescale.com> wrote:

> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -240,6 +240,8 @@ struct sdhci_host {
>  #define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<25)
>  /* Controller cannot support End Attribute in NOP ADMA descriptor */
>  #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
> +/* Controller uses Auto CMD12 command to stop the transfer */
> +#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<27)

This becomes 1<<29 in my tree.

We're about to run out.  What happens then?

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

[Zang Roy-R61911]

=20

> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]=20
> Sent: Wednesday, August 04, 2010 7:44 AM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for=20
> eSDHC driver
>=20
> On Tue, 3 Aug 2010 11:11:10 +0800
> Roy Zang <tie-fei.zang@freescale.com> wrote:
>=20
> > --- a/drivers/mmc/host/sdhci.h
> > +++ b/drivers/mmc/host/sdhci.h
> > @@ -240,6 +240,8 @@ struct sdhci_host {
> >  #define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN		(1<<25)
> >  /* Controller cannot support End Attribute in NOP ADMA=20
> descriptor */
> >  #define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC		(1<<26)
> > +/* Controller uses Auto CMD12 command to stop the transfer */
> > +#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12		(1<<27)
>=20
> This becomes 1<<29 in my tree.
It also works.
>=20
> We're about to run out. =20
:-(
>What happens then?
Rewrite the code to extend some bits, I suppose.
Roy

Re: [PATCH 2/3 v2] dts: Add sdhci, auto-cmd12 field for p4080 device tree

[Grant Likely]

On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang <tie-fei.zang@freescale.com> wrote=
:
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> =A0Documentation/powerpc/dts-bindings/fsl/esdhc.txt | =A0 =A02 ++
> =A0arch/powerpc/boot/dts/p4080ds.dts =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0=
 =A01 +
> =A02 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt b/Documenta=
tion/powerpc/dts-bindings/fsl/esdhc.txt
> index 8a00407..64bcb8b 100644
> --- a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> +++ b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> @@ -14,6 +14,8 @@ Required properties:
> =A0 =A0 reports inverted write-protect state;
> =A0 - sdhci,1-bit-only : (optional) specifies that a controller can
> =A0 =A0 only handle 1-bit data transfers.
> + =A0- sdhci,auto-cmd12: (optional) specifies that a controller can
> + =A0 =A0only handle auto CMD12.

I read this, but I still don't understand what it means.  What does
"auto CMD12" mean?  Are there other kinds of CMD12?  Part of this
might be my ignorance about how eSDHC works, but it could be clearer.

Also, you can feel free to merge this patch with patch 1 in your
series.  I makes sense to update the documentation and the driver at
the same time.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

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

[Grant Likely]

On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang <tie-fei.zang@freescale.com> wrote=
:
> From: Jerry Huang <Chang-Ming.Huang@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@freescale.com>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
> =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A04 ++++
> =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 14 ++++++++++++--
> =A0drivers/mmc/host/sdhci.h =A0 =A0 =A0 =A0 | =A0 =A02 ++
> =A03 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_hos=
t *host,
> =A0 =A0 =A0 =A0WARN_ON(!host->data);
>
> =A0 =A0 =A0 =A0mode =3D SDHCI_TRNS_BLK_CNT_EN;
> - =A0 =A0 =A0 if (data->blocks > 1)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;
> + =A0 =A0 =A0 if (data->blocks > 1) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_R=
EAD_ACMD12)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI =
| SDHCI_TRNS_ACMD12;
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;

nit:
+               mode |=3D SDHCI_TRNS_MULTI;
+               if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
+                       mode |=3D SDHCI_TRNS_ACMD12;

Clearer, no?

> + =A0 =A0 =A0 }
> =A0 =A0 =A0 =A0if (data->flags & MMC_DATA_READ)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0mode |=3D SDHCI_TRNS_READ;
> =A0 =A0 =A0 =A0if (host->flags & SDHCI_REQ_USE_DMA)

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

[Zang Roy-R61911]

=20

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> Behalf Of Grant Likely
> Sent: Thursday, August 05, 2010 9:03 AM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@ozlabs.org;=20
> akpm@linux-foundation.org
> Subject: Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for=20
> eSDHC driver
>=20
> On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang=20
> <tie-fei.zang@freescale.com> wrote:
> > From: Jerry Huang <Chang-Ming.Huang@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=20
> violations on
> > these silicons.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> > =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A04 ++++
> > =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 14 ++++++++++++--
> > =A0drivers/mmc/host/sdhci.h =A0 =A0 =A0 =A0 | =A0 =A02 ++
> > =A03 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=20
> sdhci_set_transfer_mode(struct sdhci_host *host,
> > =A0 =A0 =A0 =A0WARN_ON(!host->data);
> >
> > =A0 =A0 =A0 =A0mode =3D SDHCI_TRNS_BLK_CNT_EN;
> > - =A0 =A0 =A0 if (data->blocks > 1)
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;
> > + =A0 =A0 =A0 if (data->blocks > 1) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks &=20
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D =
SDHCI_TRNS_MULTI |=20
> SDHCI_TRNS_ACMD12;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D =
SDHCI_TRNS_MULTI;
>=20
> nit:
> +               mode |=3D SDHCI_TRNS_MULTI;
> +               if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> +                       mode |=3D SDHCI_TRNS_ACMD12;
>=20
> Clearer, no?
why?
Roy

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

[Grant Likely]

On Wed, Aug 4, 2010 at 8:14 PM, Zang Roy-R61911 <r61911@freescale.com> wrot=
e:
>> > 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,
>> > =A0 =A0 =A0 =A0WARN_ON(!host->data);
>> >
>> > =A0 =A0 =A0 =A0mode =3D SDHCI_TRNS_BLK_CNT_EN;
>> > - =A0 =A0 =A0 if (data->blocks > 1)
>> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;
>> > + =A0 =A0 =A0 if (data->blocks > 1) {
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks &
>> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MUL=
TI |
>> SDHCI_TRNS_ACMD12;
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
>> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MUL=
TI;
>>
>> nit:
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_=
READ_ACMD12)
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_ACMD1=
2;
>>
>> Clearer, no?
> why?

Shorter lines, fewer lines, and the SDHCI_TRNS_MULTI is more obviously
unconditional.  But as I said, it is a nitpick.

g.

Re: [PATCH 3/3 v2] dts: Add ESDHC weird voltage bits workaround

[Anton Vorontsov]

On Tue, Aug 03, 2010 at 11:11:12AM +0800, Roy Zang wrote:
> P4080 ESDHC controller does not support 1.8V and 3.0V voltage. but the
> host controller capabilities register wrongly set the bits.
> This patch adds the workaround to correct the weird voltage setting bits.
> Only 3.3V voltage is supported for P4080 ESDHC controller.
> 
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

Btw, where is implementation for the voltage-ranges handling?

> ---
>  arch/powerpc/boot/dts/p4080ds.dts |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts
> index efa0091..2f0de24 100644
> --- a/arch/powerpc/boot/dts/p4080ds.dts
> +++ b/arch/powerpc/boot/dts/p4080ds.dts
> @@ -280,6 +280,7 @@
>  			reg = <0x114000 0x1000>;
>  			interrupts = <48 2>;
>  			interrupt-parent = <&mpic>;
> +			voltage-ranges = <3300 3300>;
>  			sdhci,auto-cmd12;
>  		};
>  
> -- 
> 1.5.6.5

Re: [PATCH 2/3 v2] dts: Add sdhci,auto-cmd12 field for p4080 device tree

[Anton Vorontsov]

On Tue, Aug 03, 2010 at 11:11:11AM +0800, Roy Zang wrote:
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> ---
>  Documentation/powerpc/dts-bindings/fsl/esdhc.txt |    2 ++
>  arch/powerpc/boot/dts/p4080ds.dts                |    1 +
>  2 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> index 8a00407..64bcb8b 100644
> --- a/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> +++ b/Documentation/powerpc/dts-bindings/fsl/esdhc.txt
> @@ -14,6 +14,8 @@ Required properties:
>      reports inverted write-protect state;
>    - sdhci,1-bit-only : (optional) specifies that a controller can
>      only handle 1-bit data transfers.
> +  - sdhci,auto-cmd12: (optional) specifies that a controller can
> +    only handle auto CMD12.

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

>  Example:
>  
> diff --git a/arch/powerpc/boot/dts/p4080ds.dts b/arch/powerpc/boot/dts/p4080ds.dts
> index 6b29eab..efa0091 100644
> --- a/arch/powerpc/boot/dts/p4080ds.dts
> +++ b/arch/powerpc/boot/dts/p4080ds.dts
> @@ -280,6 +280,7 @@
>  			reg = <0x114000 0x1000>;
>  			interrupts = <48 2>;
>  			interrupt-parent = <&mpic>;
> +			sdhci,auto-cmd12;
>  		};
>  
>  		i2c@118000 {

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

[Anton Vorontsov]

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@freescale.com> wrote:
> > From: Jerry Huang <Chang-Ming.Huang@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@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@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.

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.

>        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@gmail.com>

on the next resend (if any).

Thanks Roy!

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

[Michał Mirosław]

2010/8/3 Roy Zang <tie-fei.zang@freescale.com>:
[...]
> @@ -240,6 +240,8 @@ struct sdhci_host {
> =A0#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN =A0 =A0 =A0 =A0 =A0 =A0 =A0(=
1<<25)
> =A0/* Controller cannot support End Attribute in NOP ADMA descriptor */
> =A0#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC =A0 =A0 =A0 =A0 =A0 =A0 =A0(=
1<<26)
> +/* Controller uses Auto CMD12 command to stop the transfer */
> +#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 =A0 =A0 =A0 =A0 =A0 =A0 (1<<2=
7)
>
> =A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 irq; =A0 =A0 =
=A0 =A0 =A0 =A0/* Device IRQ */
> =A0 =A0 =A0 =A0void __iomem * =A0 =A0 =A0 =A0 =A0ioaddr; =A0 =A0 =A0 =A0 =
/* Mapped address */

Just a cosmetic hint: I suggest SDHCI_QUIRK_MULTIBLOCK_READ_AUTO_CMD12
or something for the quirk name, because ACMD12 part might be confused
with MMC/SD App CMD 12 (CMD55+CMD12 combo) if/whenever that gets used.

Best Regards,
Micha=B3 Miros=B3aw

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

[Zang Roy-R61911]

=20

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On=20
> Behalf Of Grant Likely
> Sent: Thursday, August 05, 2010 9:03 AM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@ozlabs.org;=20
> akpm@linux-foundation.org
> Subject: Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for=20
> eSDHC driver
>=20
> On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang=20
> <tie-fei.zang@freescale.com> wrote:
> > From: Jerry Huang <Chang-Ming.Huang@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=20
> violations on
> > these silicons.
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > ---
> > =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A04 ++++
> > =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 14 ++++++++++++--
> > =A0drivers/mmc/host/sdhci.h =A0 =A0 =A0 =A0 | =A0 =A02 ++
> > =A03 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=20
> sdhci_set_transfer_mode(struct sdhci_host *host,
> > =A0 =A0 =A0 =A0WARN_ON(!host->data);
> >
> > =A0 =A0 =A0 =A0mode =3D SDHCI_TRNS_BLK_CNT_EN;
> > - =A0 =A0 =A0 if (data->blocks > 1)
> > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;
> > + =A0 =A0 =A0 if (data->blocks > 1) {
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks &=20
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D =
SDHCI_TRNS_MULTI |=20
> SDHCI_TRNS_ACMD12;
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D =
SDHCI_TRNS_MULTI;
>=20
> nit:
> +               mode |=3D SDHCI_TRNS_MULTI;
> +               if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> +                       mode |=3D SDHCI_TRNS_ACMD12;
>=20
> Clearer, no?
It is clear.
Thanks.
Roy

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

[Zang Roy-R61911]

=20

> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]=20
> Sent: Tuesday, August 10, 2010 1:44 AM
> To: Grant Likely
> Cc: Zang Roy-R61911; linuxppc-dev@ozlabs.org;=20
> akpm@linux-foundation.org; linux-mmc@vger.kernel.org
> Subject: Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for=20
> eSDHC driver
>=20
> On Wed, Aug 04, 2010 at 07:02:56PM -0600, Grant Likely wrote:
> > On Mon, Aug 2, 2010 at 9:11 PM, Roy Zang=20
> <tie-fei.zang@freescale.com> wrote:
> > > From: Jerry Huang <Chang-Ming.Huang@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=20
> violations on
> > > these silicons.
> > >
> > > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > > Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
> > > ---
> > > =A0drivers/mmc/host/sdhci-of-core.c | =A0 =A04 ++++
> > > =A0drivers/mmc/host/sdhci.c =A0 =A0 =A0 =A0 | =A0 14 =
++++++++++++--
> > > =A0drivers/mmc/host/sdhci.h =A0 =A0 =A0 =A0 | =A0 =A02 ++
> > > =A03 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=20
> sdhci_set_transfer_mode(struct sdhci_host *host,
> > > =A0 =A0 =A0 =A0WARN_ON(!host->data);
> > >
> > > =A0 =A0 =A0 =A0mode =3D SDHCI_TRNS_BLK_CNT_EN;
> > > - =A0 =A0 =A0 if (data->blocks > 1)
> > > - =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D SDHCI_TRNS_MULTI;
> > > + =A0 =A0 =A0 if (data->blocks > 1) {
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (host->quirks &=20
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D =
SDHCI_TRNS_MULTI |=20
> SDHCI_TRNS_ACMD12;
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 else
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 mode |=3D =
SDHCI_TRNS_MULTI;
> >=20
> > nit:
> > +               mode |=3D SDHCI_TRNS_MULTI;
> > +               if (host->quirks &=20
> SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
> > +                       mode |=3D SDHCI_TRNS_ACMD12;
> >=20
> > Clearer, no?
>=20
> Much clearer. I would prefer the nit incorporated.
Agree.

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

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

[Zang Roy-R61911]

=20

> -----Original Message-----
> From: Micha=B3 Miros=B3aw [mailto:mirqus@gmail.com]=20
> Sent: Tuesday, August 10, 2010 2:25 AM
> To: Zang Roy-R61911
> Cc: linux-mmc@vger.kernel.org; linuxppc-dev@ozlabs.org;=20
> akpm@linux-foundation.org
> Subject: Re: [PATCH 1/3 v2] sdhci: Add auto CMD12 support for=20
> eSDHC driver
>=20
> 2010/8/3 Roy Zang <tie-fei.zang@freescale.com>:
> [...]
> > @@ -240,6 +240,8 @@ struct sdhci_host {
> > =A0#define SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN =A0 =A0 =A0 =A0 =A0 =A0 =
=A0(1<<25)
> > =A0/* Controller cannot support End Attribute in NOP ADMA=20
> descriptor */
> > =A0#define SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC =A0 =A0 =A0 =A0 =A0 =A0 =
=A0(1<<26)
> > +/* Controller uses Auto CMD12 command to stop the transfer */
> > +#define SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12 =A0 =A0 =A0 =A0 =A0 =A0 =
(1<<27)
> >
> > =A0 =A0 =A0 =A0int =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 irq; =A0 =
=A0 =A0 =A0 =A0 =A0/* Device IRQ */
> > =A0 =A0 =A0 =A0void __iomem * =A0 =A0 =A0 =A0 =A0ioaddr; =A0 =A0 =A0 =
=A0 /* Mapped address */
>=20
> Just a cosmetic hint: I suggest SDHCI_QUIRK_MULTIBLOCK_READ_AUTO_CMD12
> or something for the quirk name, because ACMD12 part might be confused
> with MMC/SD App CMD 12 (CMD55+CMD12 combo) if/whenever that gets used.
Thanks for the suggestion.
There are several ACMD12 needed to be updated.
Send a new patch to update it.
Roy