[LINUX PATCH 1/5] mmc: Workaround for the issue in auto tuning mode.

[LINUX PATCH 1/5] mmc: Workaround for the issue in auto tuning mode.

[P L Sai Krishna]

During the auto tuning mode of SDR104, a couple of transactions
on rx_tap_value which are not incremental or decremental by 1.
Since the DLL supports only increment/decrement by 1 during
dynamic change, observed unexpected delays during both these
transactions.
The first transaction occurs when the tap value
reached 0x1F, it will reset to 0x0 and go till 0x7. This
transaction can be avoided by changing the corecfg_dis1p5xtuningcnt
to 1'b1 which is currently tied to 1'b0 in the RTL.
The second transaction occurs after the tuning is completed.
Once the tuning is done, the tuning fsm in the host controller
calculates the average pattern match and will write the value
on the rx tap value. Therefore observed a transaction from 0x7 to
the final value which need not be a increment/decrement value.
Because of this issue DLL tuning will not be accurate and SDR50,
SDR104 & HS200 modes may not work.

This patch adds workaround to change the SD clock after
tuning done to provide accurate DLL tuning for SDR50,
SD104 & HS200 modes.

After receiving the tuning done, program "SDCLK Frequency Select"
of clock control register with a value different from the desired
value. Wait for the "Internal Clock Stable" bit of the clock
control register and program the desired frequency.

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 18 ++++++++++++++++++
 drivers/mmc/host/sdhci-pltfm.c     |  3 +++
 drivers/mmc/host/sdhci.c           |  5 +++++
 drivers/mmc/host/sdhci.h           |  4 ++++
 4 files changed, 30 insertions(+)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 75379cb..7f30577 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -52,6 +52,23 @@ static unsigned int sdhci_arasan_get_timeout_clock(struct sdhci_host *host)
 	return freq;
 }
 
+void arasan_tune_sdclk(struct sdhci_host *host)
+{
+	unsigned int clock;
+
+	clock = host->clock;
+
+	/*
+	 * As per controller erratum, program the SDCLK Frequency
+	 * Select of clock control register with a value, say
+	 * clock/2. Wait for the Internal clock stable and program
+	 * the desired frequency.
+	 */
+	host->ops->set_clock(host, clock/2);
+
+	host->ops->set_clock(host, host->clock);
+}
+
 static struct sdhci_ops sdhci_arasan_ops = {
 	.set_clock = sdhci_set_clock,
 	.get_max_clock = sdhci_pltfm_clk_get_max_clock,
@@ -59,6 +76,7 @@ static struct sdhci_ops sdhci_arasan_ops = {
 	.set_bus_width = sdhci_set_bus_width,
 	.reset = sdhci_reset,
 	.set_uhs_signaling = sdhci_set_uhs_signaling,
+	.tune_clk = arasan_tune_sdclk,
 };
 
 static struct sdhci_pltfm_data sdhci_arasan_pdata = {
diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 072bb27..223c5eb 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -90,6 +90,9 @@ void sdhci_get_of_property(struct platform_device *pdev)
 	if (of_get_property(np, "no-1-8-v", NULL))
 		host->quirks2 |= SDHCI_QUIRK2_NO_1_8_V;
 
+	if (of_get_property(np, "broken-tuning", NULL))
+		host->quirks2 |= SDHCI_QUIRK2_BROKEN_TUNING;
+
 	if (of_device_is_compatible(np, "fsl,p2020-rev1-esdhc"))
 		host->quirks |= SDHCI_QUIRK_BROKEN_DMA;
 
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d622435..8b064cd 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2043,6 +2043,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
 		err = -EIO;
 	}
 
+	if ((host->quirks2 & SDHCI_QUIRK2_BROKEN_TUNING) &&
+		(tuning_loop_counter >= 0) && (ctrl & SDHCI_CTRL_TUNED_CLK)) {
+		host->ops->tune_clk(host);
+	}
+
 out:
 	if (tuning_count) {
 		/*
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7654ae5..16419f0 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -417,6 +417,9 @@ struct sdhci_host {
 #define SDHCI_QUIRK2_ACMD23_BROKEN			(1<<14)
 /* Broken Clock divider zero in controller */
 #define SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN		(1<<15)
+/* Tuning Broken for HS200, SDR50 and SDR104 */
+#define SDHCI_QUIRK2_BROKEN_TUNING			(1<<16)
+
 /*
  * When internal clock is disabled, a delay is needed before modifying the
  * SD clock frequency or enabling back the internal clock.
@@ -548,6 +551,7 @@ struct sdhci_ops {
 	void	(*platform_init)(struct sdhci_host *host);
 	void    (*card_event)(struct sdhci_host *host);
 	void	(*voltage_switch)(struct sdhci_host *host);
+	void	(*tune_clk)(struct sdhci_host *host);
 	int	(*select_drive_strength)(struct sdhci_host *host,
 					 struct mmc_card *card,
 					 unsigned int max_dtr, int host_drv,
-- 
2.1.2

[LINUX PATCH 2/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[P L Sai Krishna]

This patch adds broken-tuning property to the binding
doc for Arasan SDHCI.

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index da541c3..2088d9f 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -17,6 +17,10 @@ Required Properties:
   - interrupt-parent: Phandle for the interrupt controller that services
 		      interrupts for this device.
 
+Optional Properties:
+  - broken-tuning: Indicates tuning broken in Silicon 1.0 of
+			  Zynq Ultrascale+ MPSoC.
+
 Example:
 	sdhci@e0100000 {
 		compatible = "arasan,sdhci-8.9a";
-- 
2.1.2

Re: [LINUX PATCH 2/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[Sören Brinkmann]

On Tue, 2016-01-19 at 07:47PM +0530, P L Sai Krishna wrote:
> This patch adds broken-tuning property to the binding
> doc for Arasan SDHCI.
> 
> Signed-off-by: P L Sai Krishna <lakshmis-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index da541c3..2088d9f 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -17,6 +17,10 @@ Required Properties:
>    - interrupt-parent: Phandle for the interrupt controller that services
>  		      interrupts for this device.
>  
> +Optional Properties:
> +  - broken-tuning: Indicates tuning broken in Silicon 1.0 of
> +			  Zynq Ultrascale+ MPSoC.

Describe the purpose/values of the property without reference to the
SoC. Other broken implementations would want to use this property too.

	Sören
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [LINUX PATCH 2/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[Rob Herring]

On Tue, Jan 19, 2016 at 07:47:32PM +0530, P L Sai Krishna wrote:
> This patch adds broken-tuning property to the binding
> doc for Arasan SDHCI.
> 
> Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
> ---
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index da541c3..2088d9f 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -17,6 +17,10 @@ Required Properties:
>    - interrupt-parent: Phandle for the interrupt controller that services
>  		      interrupts for this device.
>  
> +Optional Properties:
> +  - broken-tuning: Indicates tuning broken in Silicon 1.0 of
> +			  Zynq Ultrascale+ MPSoC.
> +

Make this a standard property.

Or use the compatible string to determine this. Si errata is exactly 
what they are for.

Rob

Re: [LINUX PATCH 2/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[Michal Simek]

On 19.1.2016 18:19, Rob Herring wrote:
> On Tue, Jan 19, 2016 at 07:47:32PM +0530, P L Sai Krishna wrote:
>> This patch adds broken-tuning property to the binding
>> doc for Arasan SDHCI.
>>
>> Signed-off-by: P L Sai Krishna <lakshmis-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
>> ---
>>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> index da541c3..2088d9f 100644
>> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
>> @@ -17,6 +17,10 @@ Required Properties:
>>    - interrupt-parent: Phandle for the interrupt controller that services
>>  		      interrupts for this device.
>>  
>> +Optional Properties:
>> +  - broken-tuning: Indicates tuning broken in Silicon 1.0 of
>> +			  Zynq Ultrascale+ MPSoC.
>> +
> 
> Make this a standard property.
> 
> Or use the compatible string to determine this. Si errata is exactly 
> what they are for.

I still think that having interface for runtime Silicon version
detection in generic way is much better than using compatible strings
for certain silicon version.
The same board revisions can have different silicon versions.

Asking secure monitor for this information and based on that setup
quirks for this case would be preferred solution.
I want to spend some time to share information about silicon via soc bus
to user space and there should be similar way for sharing this
information inside the kernel.

Thanks,
Michal


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[LINUX PATCH 3/5] mmc: host: Added DT support to use SDIO in standard speed.

[P L Sai Krishna]

If the controller is used in High Speed Mode (Zynq-7000 data output on
rising edge of the clock) the hold time requirement for the
JEDEC/MMC 4.41 Specification is NOT met.
In Standard Speed Mode (Zynq-7000 data output on falling edge of the clock)
an extra half clock period is added to the clock to out delay meeting the
hold time requirement.
This patch adds device tree property 'broken-mmc-highspeed' to sdhci node
to force the controller to use in standard speed as a workaround.

Signed-off-by: Emil Lenchak <emill@xilinx.com>
Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
 drivers/mmc/host/sdhci-pltfm.c | 3 +++
 drivers/mmc/host/sdhci.c       | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-pltfm.c b/drivers/mmc/host/sdhci-pltfm.c
index 223c5eb..37df5cb 100644
--- a/drivers/mmc/host/sdhci-pltfm.c
+++ b/drivers/mmc/host/sdhci-pltfm.c
@@ -102,6 +102,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, "broken-mmc-highspeed", NULL))
+		host->quirks |= SDHCI_QUIRK_NO_HISPD_BIT;
+
 	of_property_read_u32(np, "clock-frequency", &pltfm_host->clock);
 
 	if (of_find_property(np, "keep-power-in-suspend", NULL))
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 8b064cd..5cc07d6 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -3114,7 +3114,8 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks2 & SDHCI_QUIRK2_HOST_NO_CMD23)
 		mmc->caps &= ~MMC_CAP_CMD23;
 
-	if (caps[0] & SDHCI_CAN_DO_HISPD)
+	if ((caps[0] & SDHCI_CAN_DO_HISPD) &&
+		!(host->quirks & SDHCI_QUIRK_NO_HISPD_BIT))
 		mmc->caps |= MMC_CAP_SD_HIGHSPEED | MMC_CAP_MMC_HIGHSPEED;
 
 	if ((host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION) &&
-- 
2.1.2

[LINUX PATCH 4/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[P L Sai Krishna]

This patch adds broken-mmc-highspeed property to the binding
doc for Arasan SDHCI.

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
 Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
index 2088d9f..5884832 100644
--- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
+++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
@@ -20,7 +20,8 @@ Required Properties:
 Optional Properties:
   - broken-tuning: Indicates tuning broken in Silicon 1.0 of
 			  Zynq Ultrascale+ MPSoC.
-
+  - broken-mmc-highspeed: Indicates to force
+				the controller to use in standard speed.
 Example:
 	sdhci@e0100000 {
 		compatible = "arasan,sdhci-8.9a";
-- 
2.1.2

Re: [LINUX PATCH 4/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[Rob Herring]

On Tue, Jan 19, 2016 at 07:47:34PM +0530, P L Sai Krishna wrote:
> This patch adds broken-mmc-highspeed property to the binding
> doc for Arasan SDHCI.
> 
> Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
> ---
>  Documentation/devicetree/bindings/mmc/arasan,sdhci.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> index 2088d9f..5884832 100644
> --- a/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> +++ b/Documentation/devicetree/bindings/mmc/arasan,sdhci.txt
> @@ -20,7 +20,8 @@ Required Properties:
>  Optional Properties:
>    - broken-tuning: Indicates tuning broken in Silicon 1.0 of
>  			  Zynq Ultrascale+ MPSoC.
> -
> +  - broken-mmc-highspeed: Indicates to force
> +				the controller to use in standard speed.

We already have the inverse of this with "cap-mmc-highspeed".

The only potential problem is it will need to be added to existing DTs 
if you start disabling high speed by default. Maybe that's not an issue 
for users of the Arasan driver.

Otherwise, you should determine this with a more specific compatible 
string.

Rob

RE: [LINUX PATCH 4/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[Lakshmi Sai Krishna Potthuri]

Hi,

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: Tuesday, January 19, 2016 10:46 PM
> To: Lakshmi Sai Krishna Potthuri
> Cc: Michal Simek; Soren Brinkmann; Ulf Hansson; Kevin Hao; Emil P. Lenchak;
> Tobias Klauser; Sudeep Holla; Adrian Hunter; Jisheng Zhang; Ivan T. Ivanov;
> Scott Branden; Vincent Yang; Haibo Chen; Marek Vasut;
> ludovic.desroches@atmel.com; Pawel Moll; Mark Rutland; Ian Campbell;
> Kumar Gala; Suman Tripathi; Shawn Lin; devicetree@vger.kernel.org; Harini
> Katakam; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org;
> Lakshmi Sai Krishna Potthuri; Anirudha Sarangi; Punnaiah Choudary Kalluri;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [LINUX PATCH 4/5] Documentation: mmc: Updated the binding
> doc for Arasan SDHCI.
>
> On Tue, Jan 19, 2016 at 07:47:34PM +0530, P L Sai Krishna wrote:
> > This patch adds broken-mmc-highspeed property to the binding doc for
> > Arasan SDHCI.
> >
> > Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
> > ---
<snip>
> > -
> > +  - broken-mmc-highspeed: Indicates to force
> > +                           the controller to use in standard speed.
>
> We already have the inverse of this with "cap-mmc-highspeed".
>
> The only potential problem is it will need to be added to existing DTs if you
> start disabling high speed by default. Maybe that's not an issue for users of
> the Arasan driver.
>

It might be an issue for other users.
I know that it will be an issue for older Xilinx SoC (Zynq), for ex.

> Otherwise, you should determine this with a more specific compatible string.
>

Since this feature might be broken on any SoC, I thought it would be
good to provide a dt property.
Using a compatibility string will offer no flexibility as per board or silicon.

Regards,
Harini


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [LINUX PATCH 4/5] Documentation: mmc: Updated the binding doc for Arasan SDHCI.

[Rob Herring]

On Tue, Jan 19, 2016 at 11:51 PM, Lakshmi Sai Krishna Potthuri
<lakshmi.sai.krishna.potthuri@xilinx.com> wrote:
> Hi,
>
>> -----Original Message-----
>> From: Rob Herring [mailto:robh@kernel.org]
>> Sent: Tuesday, January 19, 2016 10:46 PM
>> To: Lakshmi Sai Krishna Potthuri
>> Cc: Michal Simek; Soren Brinkmann; Ulf Hansson; Kevin Hao; Emil P. Lenchak;
>> Tobias Klauser; Sudeep Holla; Adrian Hunter; Jisheng Zhang; Ivan T. Ivanov;
>> Scott Branden; Vincent Yang; Haibo Chen; Marek Vasut;
>> ludovic.desroches@atmel.com; Pawel Moll; Mark Rutland; Ian Campbell;
>> Kumar Gala; Suman Tripathi; Shawn Lin; devicetree@vger.kernel.org; Harini
>> Katakam; linux-mmc@vger.kernel.org; linux-kernel@vger.kernel.org;
>> Lakshmi Sai Krishna Potthuri; Anirudha Sarangi; Punnaiah Choudary Kalluri;
>> linux-arm-kernel@lists.infradead.org
>> Subject: Re: [LINUX PATCH 4/5] Documentation: mmc: Updated the binding
>> doc for Arasan SDHCI.
>>
>> On Tue, Jan 19, 2016 at 07:47:34PM +0530, P L Sai Krishna wrote:
>> > This patch adds broken-mmc-highspeed property to the binding doc for
>> > Arasan SDHCI.
>> >
>> > Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
>> > ---
> <snip>
>> > -
>> > +  - broken-mmc-highspeed: Indicates to force
>> > +                           the controller to use in standard speed.
>>
>> We already have the inverse of this with "cap-mmc-highspeed".
>>
>> The only potential problem is it will need to be added to existing DTs if you
>> start disabling high speed by default. Maybe that's not an issue for users of
>> the Arasan driver.
>>
>
> It might be an issue for other users.
> I know that it will be an issue for older Xilinx SoC (Zynq), for ex.
>
>> Otherwise, you should determine this with a more specific compatible string.
>>
>
> Since this feature might be broken on any SoC, I thought it would be
> good to provide a dt property.
> Using a compatibility string will offer no flexibility as per board or silicon.

I've thought about this some more. Since this is an override of the
SDHCI capabilities register, I'm okay with this property. However, it
should be common to all SDHCI controller and handled in the core SDHCI
code. Another way this could be handled is directly provide an
override value for the capability register in the DT. Then we don't
have to provide a property for every capability bit.

Additionally, the Arasan binding needs to have SOC specific compatible
strings. IP vendor strings alone have been proven often to be
insufficient.

Rob

[LINUX PATCH 5/5] mmc: host: Update the quirks for this controller.

[P L Sai Krishna]

This patch adds SDHCI_QUIRK2_STOP_WITH_TC quirk to remove
the following error message.
"mmc0: Got data interrupt 0x00000002 even though no data
operation was in progress"

Signed-off-by: P L Sai Krishna <lakshmis@xilinx.com>
---
 drivers/mmc/host/sdhci-of-arasan.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index 7f30577..751623f 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -83,7 +83,8 @@ static struct sdhci_pltfm_data sdhci_arasan_pdata = {
 	.ops = &sdhci_arasan_ops,
 	.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
 	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
-			SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
+			SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN |
+			SDHCI_QUIRK2_STOP_WITH_TC,
 };
 
 #ifdef CONFIG_PM_SLEEP
-- 
2.1.2

Re: [LINUX PATCH 5/5] mmc: host: Update the quirks for this controller.

[Holger Schurig]

Hi,

Don't write "of this controller" into subject, write "of sdhci-arasan"
or something like it.

Otherwise people will have to look into your patch description ...

> This patch adds SDHCI_QUIRK2_STOP_WITH_TC quirk to remove
> the following error message.
> "mmc0: Got data interrupt 0x00000002 even though no data
> operation was in progress"

But wait, it's not there, too. So people have to look into the patch
itself. :-(



Holger

Re: [LINUX PATCH 1/5] mmc: Workaround for the issue in auto tuning mode.

[Chen-Yu Tsai]

Hi,

On Tue, Jan 19, 2016 at 10:17 PM, P L Sai Krishna
<lakshmi.sai.krishna.potthuri-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> During the auto tuning mode of SDR104, a couple of transactions
> on rx_tap_value which are not incremental or decremental by 1.
> Since the DLL supports only increment/decrement by 1 during
> dynamic change, observed unexpected delays during both these
> transactions.
> The first transaction occurs when the tap value
> reached 0x1F, it will reset to 0x0 and go till 0x7. This
> transaction can be avoided by changing the corecfg_dis1p5xtuningcnt
> to 1'b1 which is currently tied to 1'b0 in the RTL.
> The second transaction occurs after the tuning is completed.
> Once the tuning is done, the tuning fsm in the host controller
> calculates the average pattern match and will write the value
> on the rx tap value. Therefore observed a transaction from 0x7 to
> the final value which need not be a increment/decrement value.
> Because of this issue DLL tuning will not be accurate and SDR50,
> SDR104 & HS200 modes may not work.
>
> This patch adds workaround to change the SD clock after
> tuning done to provide accurate DLL tuning for SDR50,
> SD104 & HS200 modes.
>
> After receiving the tuning done, program "SDCLK Frequency Select"
> of clock control register with a value different from the desired
> value. Wait for the "Internal Clock Stable" bit of the clock
> control register and program the desired frequency.

Does this series apply to any non-Arasan or non-sdhci mmc hosts?
The subject does not indicate a specific platform.

Thanks
ChenYu

> Signed-off-by: P L Sai Krishna <lakshmis-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/mmc/host/sdhci-of-arasan.c | 18 ++++++++++++++++++
>  drivers/mmc/host/sdhci-pltfm.c     |  3 +++
>  drivers/mmc/host/sdhci.c           |  5 +++++
>  drivers/mmc/host/sdhci.h           |  4 ++++
>  4 files changed, 30 insertions(+)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [LINUX PATCH 1/5] mmc: Workaround for the issue in auto tuning mode.

[Lakshmi Sai Krishna Potthuri]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3161 bytes --]

Hi,

> -----Original Message-----
> From: Chen-Yu Tsai [mailto:wens@csie.org]
> Sent: Tuesday, January 19, 2016 8:32 PM
> To: Lakshmi Sai Krishna Potthuri
> Cc: Michal Simek; Soren Brinkmann; Ulf Hansson; Kevin Hao; Emil P. Lenchak;
> Tobias Klauser; Sudeep Holla; Adrian Hunter; Jisheng Zhang; Ivan T. Ivanov;
> Scott Branden; Vincent Yang; Haibo Chen; Marek Vasut;
> ludovic.desroches@atmel.com; Rob Herring; Pawel Moll; Mark Rutland; Ian
> Campbell; Kumar Gala; Suman Tripathi; Shawn Lin; devicetree; Harini
> Katakam; linux-mmc@vger.kernel.org; linux-kernel; Lakshmi Sai Krishna
> Potthuri; Anirudha Sarangi; Punnaiah Choudary Kalluri; linux-arm-kernel
> Subject: Re: [LINUX PATCH 1/5] mmc: Workaround for the issue in auto
> tuning mode.
>
> Hi,
>
> On Tue, Jan 19, 2016 at 10:17 PM, P L Sai Krishna
> <lakshmi.sai.krishna.potthuri@xilinx.com> wrote:
> > During the auto tuning mode of SDR104, a couple of transactions on
> > rx_tap_value which are not incremental or decremental by 1.
> > Since the DLL supports only increment/decrement by 1 during dynamic
> > change, observed unexpected delays during both these transactions.
> > The first transaction occurs when the tap value reached 0x1F, it will
> > reset to 0x0 and go till 0x7. This transaction can be avoided by
> > changing the corecfg_dis1p5xtuningcnt to 1'b1 which is currently tied
> > to 1'b0 in the RTL.
> > The second transaction occurs after the tuning is completed.
> > Once the tuning is done, the tuning fsm in the host controller
> > calculates the average pattern match and will write the value on the
> > rx tap value. Therefore observed a transaction from 0x7 to the final
> > value which need not be a increment/decrement value.
> > Because of this issue DLL tuning will not be accurate and SDR50,
> > SDR104 & HS200 modes may not work.
> >
> > This patch adds workaround to change the SD clock after tuning done to
> > provide accurate DLL tuning for SDR50,
> > SD104 & HS200 modes.
> >
> > After receiving the tuning done, program "SDCLK Frequency Select"
> > of clock control register with a value different from the desired
> > value. Wait for the "Internal Clock Stable" bit of the clock control
> > register and program the desired frequency.
>
> Does this series apply to any non-Arasan or non-sdhci mmc hosts?
> The subject does not indicate a specific platform.

This series is applicable only for Zynq Ultrascal+ MPSoC which
uses Arasan SDHCI.
Sorry, I will change the description to indicate the same.
The DT property used is made generic but the tuning used is
Specific to this SoC.

Regards
Sai Krishna


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·zøœzÚÞz)í…æèw*\x1fjg¬±¨\x1e¶‰šŽŠÝ¢j.ïÛ°\½½MŽúgjÌæa×\x02››–' ™©Þ¢¸\f¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾\a«‘êçzZ+ƒùšŽŠÝ¢j"ú!¶i

Re: [LINUX PATCH 1/5] mmc: Workaround for the issue in auto tuning mode.

[Russell King - ARM Linux]

On Tue, Jan 19, 2016 at 07:47:31PM +0530, P L Sai Krishna wrote:
> +void arasan_tune_sdclk(struct sdhci_host *host)

static?

> +{
> +	unsigned int clock;
> +
> +	clock = host->clock;

Maybe combine the above two lines:

	unsigned int clock = host->clock;

?

> +
> +	/*
> +	 * As per controller erratum, program the SDCLK Frequency
> +	 * Select of clock control register with a value, say
> +	 * clock/2. Wait for the Internal clock stable and program
> +	 * the desired frequency.
> +	 */
> +	host->ops->set_clock(host, clock/2);

The comment above says "wait for the internal clock stable" - I see
no wait in here.  Does the code actually conform with the comment?

Please also use "clock / 2" as per coding style, thanks

> +
> +	host->ops->set_clock(host, host->clock);

Maybe replace host->clock with clock?

> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index d622435..8b064cd 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -2043,6 +2043,11 @@ static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode)
>  		err = -EIO;
>  	}
>  
> +	if ((host->quirks2 & SDHCI_QUIRK2_BROKEN_TUNING) &&
> +		(tuning_loop_counter >= 0) && (ctrl & SDHCI_CTRL_TUNED_CLK)) {
> +		host->ops->tune_clk(host);
> +	}

Do we need this "SDHCI_QUIRK2_BROKEN_TUNING" quirk at all?  What's wrong
with:

	if (host->ops->tune_clk && tuning_loop_counter >= 0 &&
	    ctrl & SDHCI_CTRL_TUNED_CLK)
		host->ops->tune_clk(host);

here?

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

RE: [LINUX PATCH 1/5] mmc: Workaround for the issue in auto tuning mode.

[Lakshmi Sai Krishna Potthuri]

Hi,

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux@arm.linux.org.uk]
> Sent: Tuesday, January 19, 2016 11:28 PM
> To: Lakshmi Sai Krishna Potthuri
> Cc: Michal Simek; Soren Brinkmann; Ulf Hansson; Kevin Hao; Emil P. Lenchak;
> Tobias Klauser; Sudeep Holla; Adrian Hunter; Jisheng Zhang; Ivan T. Ivanov;
> Scott Branden; Vincent Yang; Haibo Chen; Marek Vasut;
> ludovic.desroches@atmel.com; Rob Herring; Pawel Moll; Mark Rutland; Ian
> Campbell; Kumar Gala; Suman Tripathi; Shawn Lin;
> devicetree@vger.kernel.org; Harini Katakam; linux-mmc@vger.kernel.org;
> linux-kernel@vger.kernel.org; Lakshmi Sai Krishna Potthuri; Anirudha
> Sarangi; Punnaiah Choudary Kalluri; linux-arm-kernel@lists.infradead.org
> Subject: Re: [LINUX PATCH 1/5] mmc: Workaround for the issue in auto
> tuning mode.
>
<snip>
> > +
> > +   /*
> > +    * As per controller erratum, program the SDCLK Frequency
> > +    * Select of clock control register with a value, say
> > +    * clock/2. Wait for the Internal clock stable and program
> > +    * the desired frequency.
> > +    */
> > +   host->ops->set_clock(host, clock/2);
>
> The comment above says "wait for the internal clock stable" - I see no wait in
> here.  Does the code actually conform with the comment?

Wait for internal clock stable is taken care of inside
set_clock()

<snip>
> > +   if ((host->quirks2 & SDHCI_QUIRK2_BROKEN_TUNING) &&
> > +           (tuning_loop_counter >= 0) && (ctrl &
> SDHCI_CTRL_TUNED_CLK)) {
> > +           host->ops->tune_clk(host);
> > +   }
>
> Do we need this "SDHCI_QUIRK2_BROKEN_TUNING" quirk at all?  What's
> wrong
> with:
>
>       if (host->ops->tune_clk && tuning_loop_counter >= 0 &&
>           ctrl & SDHCI_CTRL_TUNED_CLK)
>               host->ops->tune_clk(host);
>
> here?
>

tune_clock is provided as part of ops and will always be present.
Other users of arasan and this driver might not have tuning broken
And won't use this manual tuning.

Regards
Sai Krishna


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.