[PATCH 1/3] powerpc/esdhc: disable CMD23 for some Freescale SoCs

[PATCH 1/3] powerpc/esdhc: disable CMD23 for some Freescale SoCs

[Chang-Ming.Huang]

From: Jerry Huang <Chang-Ming.Huang@freescale.com>

CMD23 causes lots of errors in kernel on some freescale SoCs
when mmc card used, which is because these controllers does
not support CMD23, but even on SoCs which declare CMD23 is supported,
so we'll not use CMD23.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/mmc/host/sdhci-pltfm.c |    3 +++
 drivers/mmc/host/sdhci.c       |    3 +++
 include/linux/mmc/sdhci.h      |    1 +
 3 files changed, 7 insertions(+)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index d9a4ef4..1e532dd 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -83,6 +83,9 @@ void sdhci_get_of_property(struct platform_device *pdev)
 		    of_device_is_compatible(np, "fsl,mpc8536-esdhc"))
 			host->quirks |= SDHCI_QUIRK_BROKEN_TIMEOUT_VAL;
 
+		if (of_get_property(np, "sdhci,no-cmd23", NULL))
+			host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
+
 		clk = of_get_property(np, "clock-frequency", &size);
 		if (clk && size == sizeof(*clk) && *clk)
 			pltfm_host->clock = be32_to_cpup(clk);
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d98b199..a7ec2a0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2829,6 +2829,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (!(host->quirks & SDHCI_QUIRK_FORCE_1_BIT_DATA))
 		mmc->caps |= MMC_CAP_4_BIT_DATA;
 
+	if (host->quirks2 & SDHCI_QUIRK2_HOST_NO_CMD23)
+		mmc->caps &= ~MMC_CAP_CMD23;
+
 	if (caps[0] & SDHCI_CAN_DO_HISPD)
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
index fa8529a..1edcb4d 100644
--- a/include/linux/mmc/sdhci.h
+++ b/include/linux/mmc/sdhci.h
@@ -91,6 +91,7 @@ struct sdhci_host {
 	unsigned int quirks2;	/* More deviations from spec. */
 
 #define SDHCI_QUIRK2_HOST_OFF_CARD_ON			(1<<0)
+#define SDHCI_QUIRK2_HOST_NO_CMD23			(1<<1)
 
 	int irq;		/* Device IRQ */
 	void __iomem *ioaddr;	/* Mapped address */
-- 
1.7.9.5

[PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Chang-Ming.Huang]

From: Jerry Huang <Chang-Ming.Huang@freescale.com>

Below SOCs don't support the cmd23 command for MMC card,
therefore, disable it in device tree:
P1020, P1021, P1022, P1024, P1025 and P4080

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
 arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |    1 +
 arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |    1 +
 arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |    1 +
 arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |    1 +
 4 files changed, 4 insertions(+)

diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
index 68cc5e7..793a30b 100644
--- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
@@ -154,6 +154,7 @@
 	sdhc@2e000 {
 		compatible = "fsl,p1020-esdhc", "fsl,esdhc";
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 /include/ "pq3-sec3.3-0.dtsi"
 
diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
index adb82fd..2b7fd2a 100644
--- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
@@ -149,6 +149,7 @@
 /include/ "pq3-esdhc-0.dtsi"
 	sdhc@2e000 {
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 
 /include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
index 06216b8..2334a52 100644
--- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
@@ -215,6 +215,7 @@
 	sdhc@2e000 {
 		compatible = "fsl,p1022-esdhc", "fsl,esdhc";
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 
 /include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
index 8d35d2c..5b39952 100644
--- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
@@ -337,6 +337,7 @@
 	sdhc@114000 {
 		voltage-ranges = <3300 3300>;
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 
 /include/ "qoriq-i2c-0.dtsi"
-- 
1.7.9.5

[PATCH 3/3] MMC/esdhc: introduce the 'sdhci,no-cmd23'

[Chang-Ming.Huang]

From: Jerry Huang <Chang-Ming.Huang@freescale.com>

Some controller can't support CMD23, therefore introduce this property.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
 .../devicetree/bindings/mmc/fsl-esdhc.txt          |    1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
index bd9be0b..52286df9 100644
--- a/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
+++ b/Documentation/devicetree/bindings/mmc/fsl-esdhc.txt
@@ -19,6 +19,7 @@ Optional properties:
     "bus-width = <1>" property.
   - sdhci,auto-cmd12: specifies that a controller can only handle auto
     CMD12.
+  - sdhci,no-cmd23: specifies that a controller can't support cmd23.
 
 Example:
 
-- 
1.7.9.5

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Anton Vorontsov]

On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-Ming.Huang@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Below SOCs don't support the cmd23 command for MMC card,
> therefore, disable it in device tree:
> P1020, P1021, P1022, P1024, P1025 and P4080
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>

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

Thanks!

Re: [PATCH 3/3] MMC/esdhc: introduce the 'sdhci,no-cmd23'

[Anton Vorontsov]

On Tue, Sep 11, 2012 at 03:12:45PM +0800, Chang-Ming.Huang@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Some controller can't support CMD23, therefore introduce this property.
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>

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

Thank you!

Re: [PATCH 1/3] powerpc/esdhc: disable CMD23 for some Freescale SoCs

[Anton Vorontsov]

On Tue, Sep 11, 2012 at 03:12:43PM +0800, Chang-Ming.Huang@freescale.com wrote:
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> CMD23 causes lots of errors in kernel on some freescale SoCs
> when mmc card used, which is because these controllers does
> not support CMD23, but even on SoCs which declare CMD23 is supported,
> so we'll not use CMD23.
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@freescale.com>

This looks OK. For sdhci-pltfm,

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

Thanks.

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Anton Vorontsov]

On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-Ming.Huang@freescale.com wrote:
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > 
> > Below SOCs don't support the cmd23 command for MMC card,
> > therefore, disable it in device tree:
> > P1020, P1021, P1022, P1024, P1025 and P4080
> > 
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

Btw, although the patch is trivial, I guess you still want to let know
PowerPC folks about it. Adding Cc and copying the patch:

- - - -
From: Jerry Huang <Chang-Ming.Huang@freescale.com>

Below SOCs don't support the cmd23 command for MMC card,
therefore, disable it in device tree:
P1020, P1021, P1022, P1024, P1025 and P4080

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
 arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |    1 +
 arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |    1 +
 arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |    1 +
 arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |    1 +
 4 files changed, 4 insertions(+)

diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
index 68cc5e7..793a30b 100644
--- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
@@ -154,6 +154,7 @@
 	sdhc@2e000 {
 		compatible = "fsl,p1020-esdhc", "fsl,esdhc";
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 /include/ "pq3-sec3.3-0.dtsi"
 
diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
index adb82fd..2b7fd2a 100644
--- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
@@ -149,6 +149,7 @@
 /include/ "pq3-esdhc-0.dtsi"
 	sdhc@2e000 {
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 
 /include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
index 06216b8..2334a52 100644
--- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
@@ -215,6 +215,7 @@
 	sdhc@2e000 {
 		compatible = "fsl,p1022-esdhc", "fsl,esdhc";
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 
 /include/ "pq3-sec3.3-0.dtsi"
diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
index 8d35d2c..5b39952 100644
--- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
@@ -337,6 +337,7 @@
 	sdhc@114000 {
 		voltage-ranges = <3300 3300>;
 		sdhci,auto-cmd12;
+		sdhci,no-cmd23;
 	};
 
 /include/ "qoriq-i2c-0.dtsi"
-- 
1.7.9.5

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

Thanks, Anton.
If it is necessary, I will resend this patch to linuxppc-dev@lists.ozlabs.org.

Best Regards
Jerry Huang


> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Tuesday, September 11, 2012 4:05 PM
> To: Huang Changming-R66093
> Cc: linux-mmc@vger.kernel.org; Kumar Gala; linuxppc-dev@lists.ozlabs.org
> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
> 
> On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
> > On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-
> Ming.Huang@freescale.com wrote:
> > > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > >
> > > Below SOCs don't support the cmd23 command for MMC card, therefore,
> > > disable it in device tree:
> > > P1020, P1021, P1022, P1024, P1025 and P4080
> > >
> > > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
> Btw, although the patch is trivial, I guess you still want to let know
> PowerPC folks about it. Adding Cc and copying the patch:
> 
> - - - -
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Below SOCs don't support the cmd23 command for MMC card, therefore,
> disable it in device tree:
> P1020, P1021, P1022, P1024, P1025 and P4080
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
>  arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |    1 +
>  arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |    1 +
>  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |    1 +
>  arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |    1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> index 68cc5e7..793a30b 100644
> --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> @@ -154,6 +154,7 @@
>  	sdhc@2e000 {
>  		compatible = "fsl,p1020-esdhc", "fsl,esdhc";
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
>  /include/ "pq3-sec3.3-0.dtsi"
> 
> diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> index adb82fd..2b7fd2a 100644
> --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> @@ -149,6 +149,7 @@
>  /include/ "pq3-esdhc-0.dtsi"
>  	sdhc@2e000 {
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
> 
>  /include/ "pq3-sec3.3-0.dtsi"
> diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> index 06216b8..2334a52 100644
> --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> @@ -215,6 +215,7 @@
>  	sdhc@2e000 {
>  		compatible = "fsl,p1022-esdhc", "fsl,esdhc";
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
> 
>  /include/ "pq3-sec3.3-0.dtsi"
> diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> index 8d35d2c..5b39952 100644
> --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> @@ -337,6 +337,7 @@
>  	sdhc@114000 {
>  		voltage-ranges = <3300 3300>;
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
> 
>  /include/ "qoriq-i2c-0.dtsi"
> --
> 1.7.9.5

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

On Sep 11, 2012, at 4:36 AM, Huang Changming-R66093 wrote:

> Thanks, Anton.
> If it is necessary, I will resend this patch to linuxppc-dev@lists.ozlabs.org.
> 
> Best Regards
> Jerry Huang

I'm still not convinced this is the way to handle this issue.  It seems as if the linux driver code makes some assumptions about CMD23 support that it shouldn't.

- k

> 
> 
>> -----Original Message-----
>> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
>> Sent: Tuesday, September 11, 2012 4:05 PM
>> To: Huang Changming-R66093
>> Cc: linux-mmc@vger.kernel.org; Kumar Gala; linuxppc-dev@lists.ozlabs.org
>> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
>> 
>> On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
>>> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-
>> Ming.Huang@freescale.com wrote:
>>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>> 
>>>> Below SOCs don't support the cmd23 command for MMC card, therefore,
>>>> disable it in device tree:
>>>> P1020, P1021, P1022, P1024, P1025 and P4080
>>>> 
>>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>> 
>>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
>> 
>> Btw, although the patch is trivial, I guess you still want to let know
>> PowerPC folks about it. Adding Cc and copying the patch:
>> 
>> - - - -
>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>> 
>> Below SOCs don't support the cmd23 command for MMC card, therefore,
>> disable it in device tree:
>> P1020, P1021, P1022, P1024, P1025 and P4080
>> 
>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>> CC: Anton Vorontsov <cbouatmailru@gmail.com>
>> ---
>> arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |    1 +
>> arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |    1 +
>> arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |    1 +
>> arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |    1 +
>> 4 files changed, 4 insertions(+)
>> 
>> diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> index 68cc5e7..793a30b 100644
>> --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
>> @@ -154,6 +154,7 @@
>> 	sdhc@2e000 {
>> 		compatible = "fsl,p1020-esdhc", "fsl,esdhc";
>> 		sdhci,auto-cmd12;
>> +		sdhci,no-cmd23;
>> 	};
>> /include/ "pq3-sec3.3-0.dtsi"
>> 
>> diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> index adb82fd..2b7fd2a 100644
>> --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
>> @@ -149,6 +149,7 @@
>> /include/ "pq3-esdhc-0.dtsi"
>> 	sdhc@2e000 {
>> 		sdhci,auto-cmd12;
>> +		sdhci,no-cmd23;
>> 	};
>> 
>> /include/ "pq3-sec3.3-0.dtsi"
>> diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> index 06216b8..2334a52 100644
>> --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
>> @@ -215,6 +215,7 @@
>> 	sdhc@2e000 {
>> 		compatible = "fsl,p1022-esdhc", "fsl,esdhc";
>> 		sdhci,auto-cmd12;
>> +		sdhci,no-cmd23;
>> 	};
>> 
>> /include/ "pq3-sec3.3-0.dtsi"
>> diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> index 8d35d2c..5b39952 100644
>> --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
>> @@ -337,6 +337,7 @@
>> 	sdhc@114000 {
>> 		voltage-ranges = <3300 3300>;
>> 		sdhci,auto-cmd12;
>> +		sdhci,no-cmd23;
>> 	};
>> 
>> /include/ "qoriq-i2c-0.dtsi"
>> --
>> 1.7.9.5
> 

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

In sdhci_add_host()

We have the following

...
        mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;

        if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
                host->flags |= SDHCI_AUTO_CMD12;

        /* Auto-CMD23 stuff only works in ADMA or PIO. */
        if ((host->version >= SDHCI_SPEC_300) &&
            ((host->flags & SDHCI_USE_ADMA) ||
             !(host->flags & SDHCI_USE_SDMA))) {
                host->flags |= SDHCI_AUTO_CMD23;
                DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
        } else {
                DBG("%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
        }

...

I'm not clear what the difference is between mmc->caps & host->flags, but shouldn't we move setting MMC_CAP_CMD23 inside the 'Auto-CMD23' if check?

- k

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Chris Ball]

Hi,

On Tue, Sep 11 2012, Kumar Gala wrote:
> In sdhci_add_host()
>
> We have the following
>
> ...
>         mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
>
>         if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
>                 host->flags |= SDHCI_AUTO_CMD12;
>
>         /* Auto-CMD23 stuff only works in ADMA or PIO. */
>         if ((host->version >= SDHCI_SPEC_300) &&
>             ((host->flags & SDHCI_USE_ADMA) ||
>              !(host->flags & SDHCI_USE_SDMA))) {
>                 host->flags |= SDHCI_AUTO_CMD23;
>                 DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
>         } else {
>                 DBG("%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
>         }
>
> ...
>
> I'm not clear what the difference is between mmc->caps & host->flags, but shouldn't we move setting MMC_CAP_CMD23 inside the 'Auto-CMD23' if check?

The main answer is:  No, because CMD23 is distinct from Auto-CMD23.

Multiblock transfers (CMD23) date back from MMC cards (which is why
they're an MMC host capability) and can also be used in SDHCI.

Auto-CMD23 is a new feature in SDHCI 3.0 that reduces the overhead
of sending CMD23.  It doesn't work if we're using SDMA, though.

As for capability vs. flags, the capability is more of an inherent
property of the controller, and flags are runtime decisions on whether
to use that capability, based on e.g. the presence of a quirk.

So, I think the code's correct as written.  Feel free to ask more
questions if you're investigating a specific problem that you haven't
mentioned yet.

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Scott Wood]

On 09/11/2012 03:04 AM, Anton Vorontsov wrote:
> On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
>> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-Ming.Huang@freescale.com wrote:
>>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>>
>>> Below SOCs don't support the cmd23 command for MMC card,
>>> therefore, disable it in device tree:
>>> P1020, P1021, P1022, P1024, P1025 and P4080
>>>
>>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
>>
>> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
> Btw, although the patch is trivial, I guess you still want to let know
> PowerPC folks about it. Adding Cc and copying the patch:
> 
> - - - -
> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> 
> Below SOCs don't support the cmd23 command for MMC card,
> therefore, disable it in device tree:
> P1020, P1021, P1022, P1024, P1025 and P4080
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
>  arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |    1 +
>  arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |    1 +
>  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |    1 +
>  arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |    1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> index 68cc5e7..793a30b 100644
> --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> @@ -154,6 +154,7 @@
>  	sdhc@2e000 {
>  		compatible = "fsl,p1020-esdhc", "fsl,esdhc";
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
>  /include/ "pq3-sec3.3-0.dtsi"
>  
> diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> index adb82fd..2b7fd2a 100644
> --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> @@ -149,6 +149,7 @@
>  /include/ "pq3-esdhc-0.dtsi"
>  	sdhc@2e000 {
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
>  
>  /include/ "pq3-sec3.3-0.dtsi"
> diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> index 06216b8..2334a52 100644
> --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> @@ -215,6 +215,7 @@
>  	sdhc@2e000 {
>  		compatible = "fsl,p1022-esdhc", "fsl,esdhc";
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
>  
>  /include/ "pq3-sec3.3-0.dtsi"
> diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> index 8d35d2c..5b39952 100644
> --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> @@ -337,6 +337,7 @@
>  	sdhc@114000 {
>  		voltage-ranges = <3300 3300>;
>  		sdhci,auto-cmd12;
> +		sdhci,no-cmd23;
>  	};
>  
>  /include/ "qoriq-i2c-0.dtsi"
> 

This won't help people with old device trees (forked for a custom board,
tied to an old U-Boot, etc).  The driver should infer this from the
compatible string or version registers (ideally block-specific version
registers, but if these are absent or inconclusive, SVR can be used).

-Scott

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

On Sep 11, 2012, at 9:44 AM, Chris Ball wrote:

> Hi,
> 
> On Tue, Sep 11 2012, Kumar Gala wrote:
>> In sdhci_add_host()
>> 
>> We have the following
>> 
>> ...
>>        mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
>> 
>>        if (host->quirks & SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12)
>>                host->flags |= SDHCI_AUTO_CMD12;
>> 
>>        /* Auto-CMD23 stuff only works in ADMA or PIO. */
>>        if ((host->version >= SDHCI_SPEC_300) &&
>>            ((host->flags & SDHCI_USE_ADMA) ||
>>             !(host->flags & SDHCI_USE_SDMA))) {
>>                host->flags |= SDHCI_AUTO_CMD23;
>>                DBG("%s: Auto-CMD23 available\n", mmc_hostname(mmc));
>>        } else {
>>                DBG("%s: Auto-CMD23 unavailable\n", mmc_hostname(mmc));
>>        }
>> 
>> ...
>> 
>> I'm not clear what the difference is between mmc->caps & host->flags, but shouldn't we move setting MMC_CAP_CMD23 inside the 'Auto-CMD23' if check?
> 
> The main answer is:  No, because CMD23 is distinct from Auto-CMD23.
> 
> Multiblock transfers (CMD23) date back from MMC cards (which is why
> they're an MMC host capability) and can also be used in SDHCI.
> 
> Auto-CMD23 is a new feature in SDHCI 3.0 that reduces the overhead
> of sending CMD23.  It doesn't work if we're using SDMA, though.
> 
> As for capability vs. flags, the capability is more of an inherent
> property of the controller, and flags are runtime decisions on whether
> to use that capability, based on e.g. the presence of a quirk.
> 
> So, I think the code's correct as written.  Feel free to ask more
> questions if you're investigating a specific problem that you haven't
> mentioned yet.

Chris,

thanks for the info.  Do you know what's required on controller side to handle cards that support CMD23?

I'm trying to figure out if older controller's on FSL SoCs are missing some feature to allow CMD23 to work (vs Auto-CMD23).


- k

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Chris Ball]

Hi,

On Tue, Sep 11 2012, Kumar Gala wrote:
> thanks for the info.  Do you know what's required on controller side
> to handle cards that support CMD23?
>
> I'm trying to figure out if older controller's on FSL SoCs are missing
> some feature to allow CMD23 to work (vs Auto-CMD23).

It seems plausible that it's just not implemented on these controllers.
It's a little strange, since the command's been specified for so long
and we haven't seen any other controllers with problems.  The patch
would be correct if this is true.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

Best Regards
Jerry Huang


> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Wednesday, September 12, 2012 2:28 AM
> To: Anton Vorontsov
> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org; linux-
> mmc@vger.kernel.org
> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
> 
> On 09/11/2012 03:04 AM, Anton Vorontsov wrote:
> > On Tue, Sep 11, 2012 at 12:54:29AM -0700, Anton Vorontsov wrote:
> >> On Tue, Sep 11, 2012 at 03:12:44PM +0800, Chang-
> Ming.Huang@freescale.com wrote:
> >>> From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>>
> >>> Below SOCs don't support the cmd23 command for MMC card, therefore,
> >>> disable it in device tree:
> >>> P1020, P1021, P1022, P1024, P1025 and P4080
> >>>
> >>> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >>
> >> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> >
> > Btw, although the patch is trivial, I guess you still want to let know
> > PowerPC folks about it. Adding Cc and copying the patch:
> >
> > - - - -
> > From: Jerry Huang <Chang-Ming.Huang@freescale.com>
> >
> > Below SOCs don't support the cmd23 command for MMC card, therefore,
> > disable it in device tree:
> > P1020, P1021, P1022, P1024, P1025 and P4080
> >
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > CC: Anton Vorontsov <cbouatmailru@gmail.com>
> > ---
> >  arch/powerpc/boot/dts/fsl/p1020si-post.dtsi |    1 +
> >  arch/powerpc/boot/dts/fsl/p1021si-post.dtsi |    1 +
> >  arch/powerpc/boot/dts/fsl/p1022si-post.dtsi |    1 +
> >  arch/powerpc/boot/dts/fsl/p4080si-post.dtsi |    1 +
> >  4 files changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> > b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> > index 68cc5e7..793a30b 100644
> > --- a/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/p1020si-post.dtsi
> > @@ -154,6 +154,7 @@
> >  	sdhc@2e000 {
> >  		compatible = "fsl,p1020-esdhc", "fsl,esdhc";
> >  		sdhci,auto-cmd12;
> > +		sdhci,no-cmd23;
> >  	};
> >  /include/ "pq3-sec3.3-0.dtsi"
> >
> > diff --git a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> > b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> > index adb82fd..2b7fd2a 100644
> > --- a/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/p1021si-post.dtsi
> > @@ -149,6 +149,7 @@
> >  /include/ "pq3-esdhc-0.dtsi"
> >  	sdhc@2e000 {
> >  		sdhci,auto-cmd12;
> > +		sdhci,no-cmd23;
> >  	};
> >
> >  /include/ "pq3-sec3.3-0.dtsi"
> > diff --git a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> > b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> > index 06216b8..2334a52 100644
> > --- a/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/p1022si-post.dtsi
> > @@ -215,6 +215,7 @@
> >  	sdhc@2e000 {
> >  		compatible = "fsl,p1022-esdhc", "fsl,esdhc";
> >  		sdhci,auto-cmd12;
> > +		sdhci,no-cmd23;
> >  	};
> >
> >  /include/ "pq3-sec3.3-0.dtsi"
> > diff --git a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> > b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> > index 8d35d2c..5b39952 100644
> > --- a/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/p4080si-post.dtsi
> > @@ -337,6 +337,7 @@
> >  	sdhc@114000 {
> >  		voltage-ranges = <3300 3300>;
> >  		sdhci,auto-cmd12;
> > +		sdhci,no-cmd23;
> >  	};
> >
> >  /include/ "qoriq-i2c-0.dtsi"
> >
> 
> This won't help people with old device trees (forked for a custom board,
> tied to an old U-Boot, etc).  The driver should infer this from the
> compatible string or version registers (ideally block-specific version
> registers, but if these are absent or inconclusive, SVR can be used).
> 

I don't think it is the best way to do it.
For the VVN2.2 or older, some silicon support this feature (mpc8536 and p2020), but other silicones don't support it (e.g. p4080, p102x).
Though, the current p5/p4/p3 has supported this feature, can we sure the future silicon support it?
So I think the best way is to specify it in device tree as 'sdhci,auto-cmd12'

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Anton Vorontsov]

On Wed, Sep 12, 2012 at 03:19:18AM +0000, Huang Changming-R66093 wrote:
[...]
> I don't think it is the best way to do it.  For the VVN2.2 or older,
> some silicon support this feature (mpc8536 and p2020), but other
> silicones don't support it (e.g. p4080, p102x).  Though, the current
> p5/p4/p3 has supported this feature, can we sure the future silicon
> support it?  So I think the best way is to specify it in device tree
> as 'sdhci,auto-cmd12'

In addition to your current patches, you could just add another patch
that blacklists affected SOC revisions based on the info from PVR/SVR.

For example, see gfar_detect_errata() in
drivers/net/ethernet/freescale/gianfar.c.

That way you could help users that don't have the newest device trees.

Thanks,
Anton.

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

Best Regards
Jerry Huang


> -----Original Message-----
> From: Chris Ball [mailto:cjb@laptop.org]
> Sent: Wednesday, September 12, 2012 4:59 AM
> To: Kumar Gala
> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org list; linux-
> mmc@vger.kernel.org; Anton Vorontsov
> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
> 
> Hi,
> 
> On Tue, Sep 11 2012, Kumar Gala wrote:
> > thanks for the info.  Do you know what's required on controller side
> > to handle cards that support CMD23?
> >
> > I'm trying to figure out if older controller's on FSL SoCs are missing
> > some feature to allow CMD23 to work (vs Auto-CMD23).
> 
> It seems plausible that it's just not implemented on these controllers.
> It's a little strange, since the command's been specified for so long and
> we haven't seen any other controllers with problems.  The patch would be
> correct if this is true.
> 

I didn't find any description about it, but after testing on FSL silicones, I got this result:
Some silicones support this command, and some silicones don't support it, which will cause I/O error.

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

On Sep 12, 2012, at 1:18 AM, Huang Changming-R66093 wrote:

> 
> 
> Best Regards
> Jerry Huang
> 
> 
>> -----Original Message-----
>> From: Chris Ball [mailto:cjb@laptop.org]
>> Sent: Wednesday, September 12, 2012 4:59 AM
>> To: Kumar Gala
>> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org list; linux-
>> mmc@vger.kernel.org; Anton Vorontsov
>> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
>> 
>> Hi,
>> 
>> On Tue, Sep 11 2012, Kumar Gala wrote:
>>> thanks for the info.  Do you know what's required on controller side
>>> to handle cards that support CMD23?
>>> 
>>> I'm trying to figure out if older controller's on FSL SoCs are missing
>>> some feature to allow CMD23 to work (vs Auto-CMD23).
>> 
>> It seems plausible that it's just not implemented on these controllers.
>> It's a little strange, since the command's been specified for so long and
>> we haven't seen any other controllers with problems.  The patch would be
>> correct if this is true.
>> 
> 
> I didn't find any description about it, but after testing on FSL silicones, I got this result:
> Some silicones support this command, and some silicones don't support it, which will cause I/O error.

Can you list out which SoCs support it and which don't.  Having this list will be useful in understanding which controller versions supported it.

- k

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

> >
> >> -----Original Message-----
> >> From: Chris Ball [mailto:cjb@laptop.org]
> >> Sent: Wednesday, September 12, 2012 4:59 AM
> >> To: Kumar Gala
> >> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org list;
> >> linux- mmc@vger.kernel.org; Anton Vorontsov
> >> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the
> >> CMD23
> >>
> >> Hi,
> >>
> >> On Tue, Sep 11 2012, Kumar Gala wrote:
> >>> thanks for the info.  Do you know what's required on controller side
> >>> to handle cards that support CMD23?
> >>>
> >>> I'm trying to figure out if older controller's on FSL SoCs are
> >>> missing some feature to allow CMD23 to work (vs Auto-CMD23).
> >>
> >> It seems plausible that it's just not implemented on these controllers.
> >> It's a little strange, since the command's been specified for so long
> >> and we haven't seen any other controllers with problems.  The patch
> >> would be correct if this is true.
> >>
> >
> > I didn't find any description about it, but after testing on FSL
> silicones, I got this result:
> > Some silicones support this command, and some silicones don't support
> it, which will cause I/O error.
> 
> Can you list out which SoCs support it and which don't.  Having this list
> will be useful in understanding which controller versions supported it.
> 
P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041) support it.

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

Best Regards
Jerry Huang


> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Wednesday, September 12, 2012 11:38 AM
> To: Huang Changming-R66093
> Cc: Wood Scott-B07421; linuxppc-dev@lists.ozlabs.org; linux-
> mmc@vger.kernel.org
> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
> 
> On Wed, Sep 12, 2012 at 03:19:18AM +0000, Huang Changming-R66093 wrote:
> [...]
> > I don't think it is the best way to do it.  For the VVN2.2 or older,
> > some silicon support this feature (mpc8536 and p2020), but other
> > silicones don't support it (e.g. p4080, p102x).  Though, the current
> > p5/p4/p3 has supported this feature, can we sure the future silicon
> > support it?  So I think the best way is to specify it in device tree
> > as 'sdhci,auto-cmd12'
> 
> In addition to your current patches, you could just add another patch
> that blacklists affected SOC revisions based on the info from PVR/SVR.
> 
> For example, see gfar_detect_errata() in
> drivers/net/ethernet/freescale/gianfar.c.
> 
> That way you could help users that don't have the newest device trees.
> 
Hi, Anton
Thanks.
But, these patches are just for latest kernel, if the user don't have the latest device tree,
Then they don't have the latest kernel, and these patches can't be used for them directly,
Though provide the function like gfar_detect_errata, they must modify their codes.

And I don't know if the future silicon can support CMD23, if not, we must modify sdhc driver again.
When we use the device tree to identify it, if the silicon does not support this feature,
we just add this property into the new device tree, not need to modify SDHC driver.
I think, this is the best way.
 

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

On Sep 12, 2012, at 9:02 PM, Huang Changming-R66093 wrote:

>>> 
>>>> -----Original Message-----
>>>> From: Chris Ball [mailto:cjb@laptop.org]
>>>> Sent: Wednesday, September 12, 2012 4:59 AM
>>>> To: Kumar Gala
>>>> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org list;
>>>> linux- mmc@vger.kernel.org; Anton Vorontsov
>>>> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the
>>>> CMD23
>>>> 
>>>> Hi,
>>>> 
>>>> On Tue, Sep 11 2012, Kumar Gala wrote:
>>>>> thanks for the info.  Do you know what's required on controller side
>>>>> to handle cards that support CMD23?
>>>>> 
>>>>> I'm trying to figure out if older controller's on FSL SoCs are
>>>>> missing some feature to allow CMD23 to work (vs Auto-CMD23).
>>>> 
>>>> It seems plausible that it's just not implemented on these controllers.
>>>> It's a little strange, since the command's been specified for so long
>>>> and we haven't seen any other controllers with problems.  The patch
>>>> would be correct if this is true.
>>>> 
>>> 
>>> I didn't find any description about it, but after testing on FSL
>> silicones, I got this result:
>>> Some silicones support this command, and some silicones don't support
>> it, which will cause I/O error.
>> 
>> Can you list out which SoCs support it and which don't.  Having this list
>> will be useful in understanding which controller versions supported it.
>> 
> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041) support it.

Based on this, why don't we use the HOSTVER register to detect instead of device tree:


#define FSL_SDHC_VER_1_0	0x00
#define FSL_SDHC_VER_1_1	0x01
#define FSL_SDHC_VER_2_0	0x10
#define FSL_SDHC_VER_2_1	0x11
#define FSL_SDHC_VER_2_2	0x12
#define FSL_SDHC_VER_2_3	0x13

unsigned int vendor_version;

vendor_version = sdhci_readw(host, SDHCI_HOST_VERSION);
vendor_version = (vendor_version & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT;

if ((vendor_version == FSL_SDHC_VER_1_1) || (vendor_version == FSL_SDHC_VER_2_2))
	host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;

- k

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

Best Regards
Jerry Huang


> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]
> Sent: Thursday, September 13, 2012 8:48 PM
> To: Huang Changming-R66093
> Cc: Chris Ball; linuxppc-dev@lists.ozlabs.org list; linux-
> mmc@vger.kernel.org; Anton Vorontsov
> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23
> 
> 
> On Sep 12, 2012, at 9:02 PM, Huang Changming-R66093 wrote:
> 
> >>>
> >>>> -----Original Message-----
> >>>> From: Chris Ball [mailto:cjb@laptop.org]
> >>>> Sent: Wednesday, September 12, 2012 4:59 AM
> >>>> To: Kumar Gala
> >>>> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org list;
> >>>> linux- mmc@vger.kernel.org; Anton Vorontsov
> >>>> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the
> >>>> CMD23
> >>>>
> >>>> Hi,
> >>>>
> >>>> On Tue, Sep 11 2012, Kumar Gala wrote:
> >>>>> thanks for the info.  Do you know what's required on controller
> >>>>> side to handle cards that support CMD23?
> >>>>>
> >>>>> I'm trying to figure out if older controller's on FSL SoCs are
> >>>>> missing some feature to allow CMD23 to work (vs Auto-CMD23).
> >>>>
> >>>> It seems plausible that it's just not implemented on these
> controllers.
> >>>> It's a little strange, since the command's been specified for so
> >>>> long and we haven't seen any other controllers with problems.  The
> >>>> patch would be correct if this is true.
> >>>>
> >>>
> >>> I didn't find any description about it, but after testing on FSL
> >> silicones, I got this result:
> >>> Some silicones support this command, and some silicones don't
> >>> support
> >> it, which will cause I/O error.
> >>
> >> Can you list out which SoCs support it and which don't.  Having this
> >> list will be useful in understanding which controller versions
> supported it.
> >>
> > P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
> > Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041)
> support it.
> 
> Based on this, why don't we use the HOSTVER register to detect instead of
> device tree:
> 
> 
> #define FSL_SDHC_VER_1_0	0x00
> #define FSL_SDHC_VER_1_1	0x01
> #define FSL_SDHC_VER_2_0	0x10
> #define FSL_SDHC_VER_2_1	0x11
> #define FSL_SDHC_VER_2_2	0x12
> #define FSL_SDHC_VER_2_3	0x13
> 
> unsigned int vendor_version;
> 
> vendor_version = sdhci_readw(host, SDHCI_HOST_VERSION); vendor_version =
> (vendor_version & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT;
> 
> if ((vendor_version == FSL_SDHC_VER_1_1) || (vendor_version ==
> FSL_SDHC_VER_2_2))
> 	host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
> 

I once thought about it, but if the future silicon does not support this feature,
then we continue to modify these codes for new silicon?

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Chris Ball [mailto:cjb@laptop.org]
>>>>>> Sent: Wednesday, September 12, 2012 4:59 AM
>>>>>> To: Kumar Gala
>>>>>> Cc: Huang Changming-R66093; linuxppc-dev@lists.ozlabs.org list;
>>>>>> linux- mmc@vger.kernel.org; Anton Vorontsov
>>>>>> Subject: Re: [PATCH 2/3] powerpc/esdhc: add property to disable the
>>>>>> CMD23
>>>>>> 
>>>>>> Hi,
>>>>>> 
>>>>>> On Tue, Sep 11 2012, Kumar Gala wrote:
>>>>>>> thanks for the info.  Do you know what's required on controller
>>>>>>> side to handle cards that support CMD23?
>>>>>>> 
>>>>>>> I'm trying to figure out if older controller's on FSL SoCs are
>>>>>>> missing some feature to allow CMD23 to work (vs Auto-CMD23).
>>>>>> 
>>>>>> It seems plausible that it's just not implemented on these
>> controllers.
>>>>>> It's a little strange, since the command's been specified for so
>>>>>> long and we haven't seen any other controllers with problems.  The
>>>>>> patch would be correct if this is true.
>>>>>> 
>>>>> 
>>>>> I didn't find any description about it, but after testing on FSL
>>>> silicones, I got this result:
>>>>> Some silicones support this command, and some silicones don't
>>>>> support
>>>> it, which will cause I/O error.
>>>> 
>>>> Can you list out which SoCs support it and which don't.  Having this
>>>> list will be useful in understanding which controller versions
>> supported it.
>>>> 
>>> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
>>> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041)
>> support it.
>> 
>> Based on this, why don't we use the HOSTVER register to detect instead of
>> device tree:
>> 
>> 
>> #define FSL_SDHC_VER_1_0	0x00
>> #define FSL_SDHC_VER_1_1	0x01
>> #define FSL_SDHC_VER_2_0	0x10
>> #define FSL_SDHC_VER_2_1	0x11
>> #define FSL_SDHC_VER_2_2	0x12
>> #define FSL_SDHC_VER_2_3	0x13
>> 
>> unsigned int vendor_version;
>> 
>> vendor_version = sdhci_readw(host, SDHCI_HOST_VERSION); vendor_version =
>> (vendor_version & SDHCI_VENDOR_VER_MASK) >> SDHCI_VENDOR_VER_SHIFT;
>> 
>> if ((vendor_version == FSL_SDHC_VER_1_1) || (vendor_version ==
>> FSL_SDHC_VER_2_2))
>> 	host->quirks2 |= SDHCI_QUIRK2_HOST_NO_CMD23;
>> 
> 
> I once thought about it, but if the future silicon does not support this feature,
> then we continue to modify these codes for new silicon?

Yes, but it seems extremely unlikely that future versions of the controller will remove this feature now that it exists.

- k

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Chris Ball]

Hi,

On Thu, Sep 13 2012, Kumar Gala wrote:
>>> Can you list out which SoCs support it and which don't.  Having this list
>>> will be useful in understanding which controller versions supported it.
>>> 
>> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
>> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041) support it.
>
> Based on this, why don't we use the HOSTVER register to detect instead of device tree:

I've got a mild preference for handling quirk assignment in the DT
rather than in driver code, so I'd prefer to just push the original
patch to mmc-next as-is.  Does that sound okay?

(I think the argument that there isn't going to be any new hardware
with this problem is equally in favor of both methods.)

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

On Sep 17, 2012, at 7:36 AM, Chris Ball wrote:

> Hi,
> 
> On Thu, Sep 13 2012, Kumar Gala wrote:
>>>> Can you list out which SoCs support it and which don't.  Having this list
>>>> will be useful in understanding which controller versions supported it.
>>>> 
>>> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
>>> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041) support it.
>> 
>> Based on this, why don't we use the HOSTVER register to detect instead of device tree:
> 
> I've got a mild preference for handling quirk assignment in the DT
> rather than in driver code, so I'd prefer to just push the original
> patch to mmc-next as-is.  Does that sound okay?

Why?  I only ask because I agree with Scott that this means you have to update your device tree to get proper functionality.

> (I think the argument that there isn't going to be any new hardware
> with this problem is equally in favor of both methods.)

- k

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Chris Ball]

Hi,

On Mon, Sep 17 2012, Kumar Gala wrote:
>>>> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
>>>> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041) support it.
>>> 
>>> Based on this, why don't we use the HOSTVER register to detect instead of device tree:
>> 
>> I've got a mild preference for handling quirk assignment in the DT
>> rather than in driver code, so I'd prefer to just push the original
>> patch to mmc-next as-is.  Does that sound okay?
>
> Why?  I only ask because I agree with Scott that this means you have to update your device tree to get proper functionality.

Thanks, I'd missed that.  I withdraw my preference; I'll pick up
whichever method you all prefer.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

RE: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Huang Changming-R66093]

> On Sep 17, 2012, at 7:36 AM, Chris Ball wrote:
> 
> > Hi,
> >
> > On Thu, Sep 13 2012, Kumar Gala wrote:
> >>>> Can you list out which SoCs support it and which don't.  Having
> >>>> this list will be useful in understanding which controller versions
> supported it.
> >>>>
> >>> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
> >>> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041)
> support it.
> >>
> >> Based on this, why don't we use the HOSTVER register to detect instead
> of device tree:
> >
> > I've got a mild preference for handling quirk assignment in the DT
> > rather than in driver code, so I'd prefer to just push the original
> > patch to mmc-next as-is.  Does that sound okay?
> 
> Why?  I only ask because I agree with Scott that this means you have to
> update your device tree to get proper functionality.
> 
When the new silicon does not support CMD23,
if we don't update the device tree, then we must update the SDHC driver.
I prefer to add the property in device tree,
because we just add this property in new device tree, we don't need more effort to modify driver.

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Kumar Gala]

On Sep 17, 2012, at 8:09 PM, Huang Changming-R66093 wrote:

>> On Sep 17, 2012, at 7:36 AM, Chris Ball wrote:
>> 
>>> Hi,
>>> 
>>> On Thu, Sep 13 2012, Kumar Gala wrote:
>>>>>> Can you list out which SoCs support it and which don't.  Having
>>>>>> this list will be useful in understanding which controller versions
>> supported it.
>>>>>> 
>>>>> P1020, p1021, p1022, p1024, p1015 and p4080 can't support it.
>>>>> Mpc8536, p2020, and the other current DPAA silicon (e.g. p5020, p3041)
>> support it.
>>>> 
>>>> Based on this, why don't we use the HOSTVER register to detect instead
>> of device tree:
>>> 
>>> I've got a mild preference for handling quirk assignment in the DT
>>> rather than in driver code, so I'd prefer to just push the original
>>> patch to mmc-next as-is.  Does that sound okay?
>> 
>> Why?  I only ask because I agree with Scott that this means you have to
>> update your device tree to get proper functionality.
>> 
> When the new silicon does not support CMD23,
> if we don't update the device tree, then we must update the SDHC driver.
> I prefer to add the property in device tree,
> because we just add this property in new device tree, we don't need more effort to modify driver.
> 

Jerry,

I think doing it driver makes more sense because:

1. means older device tree's still work
2. odds that CMD23 not being supported in future devices is near 0%
   (Now that we support AutoCMD23 [and thus CMD23] we aren't likely to stop supporting it in future)
3. If IP changes you are going to have to update driver anyways for new features

I really think we should NOT utilize device tree for this.

- k

Re: [PATCH 2/3] powerpc/esdhc: add property to disable the CMD23

[Chris Ball]

Hi,

On Tue, Sep 18 2012, Kumar Gala wrote:
>>>> I've got a mild preference for handling quirk assignment in the DT
>>>> rather than in driver code, so I'd prefer to just push the original
>>>> patch to mmc-next as-is.  Does that sound okay?
>>> 
>>> Why?  I only ask because I agree with Scott that this means you have to
>>> update your device tree to get proper functionality.
>>> 
>> When the new silicon does not support CMD23,
>> if we don't update the device tree, then we must update the SDHC driver.
>> I prefer to add the property in device tree,
>> because we just add this property in new device tree, we don't need more effort to modify driver.
>
> Jerry,
>
> I think doing it driver makes more sense because:
>
> 1. means older device tree's still work
> 2. odds that CMD23 not being supported in future devices is near 0%
>    (Now that we support AutoCMD23 [and thus CMD23] we aren't likely to stop supporting it in future)
> 3. If IP changes you are going to have to update driver anyways for new features
>
> I really think we should NOT utilize device tree for this.

Of course, we could also make both (or perhaps neither) of you happy by
merging both:  if your DT says you don't support cmd23 *or* you hit the
driver's blacklist, we avoid it.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child