* [PATCH 1/3] mmc: sdhci: Add Quirk to reset data lines after tuning
2019-12-30 9:23 [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
@ 2019-12-30 9:23 ` Faiz Abbas
2019-12-30 9:23 ` [PATCH 2/3] mmc: sdhci_am654: Enable Quirk to reset data " Faiz Abbas
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Faiz Abbas @ 2019-12-30 9:23 UTC (permalink / raw)
To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, faiz_abbas, shawn.lin
Some arasan controllers have data leftover in the buffer after tuning
procedure is complete which interferes with future data commands. Add a
quirk to reset the data after tuning is finished.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
drivers/mmc/host/sdhci.c | 3 +++
drivers/mmc/host/sdhci.h | 4 ++++
2 files changed, 7 insertions(+)
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 1b1c26da3fe0..e4b478efb560 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2316,6 +2316,9 @@ EXPORT_SYMBOL_GPL(sdhci_start_tuning);
void sdhci_end_tuning(struct sdhci_host *host)
{
+ if (host->quirks2 & SDHCI_QUIRK2_RESET_DATA_POST_TUNING)
+ sdhci_reset(host, SDHCI_RESET_DATA);
+
sdhci_writel(host, host->ier, SDHCI_INT_ENABLE);
sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
}
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index fe83ece6965b..28826124f7c3 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -484,6 +484,10 @@ struct sdhci_host {
* block count.
*/
#define SDHCI_QUIRK2_USE_32BIT_BLK_CNT (1<<18)
+/*
+ * Controller needs to reset data lines once tuning is complete
+ */
+#define SDHCI_QUIRK2_RESET_DATA_POST_TUNING (1<<19)
int irq; /* Device IRQ */
void __iomem *ioaddr; /* Mapped address */
--
2.19.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] mmc: sdhci_am654: Enable Quirk to reset data after tuning
2019-12-30 9:23 [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
2019-12-30 9:23 ` [PATCH 1/3] mmc: sdhci: Add Quirk to reset data lines after tuning Faiz Abbas
@ 2019-12-30 9:23 ` Faiz Abbas
2019-12-30 9:23 ` [RFT PATCH 3/3] mmc: sdhci-of-arasan: Fix Command Queuing enable handling Faiz Abbas
2020-01-08 11:30 ` [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
3 siblings, 0 replies; 10+ messages in thread
From: Faiz Abbas @ 2019-12-30 9:23 UTC (permalink / raw)
To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, faiz_abbas, shawn.lin
Enable SDHCI_QUIRK2_RESET_DATA_POST_TUNING for all controller variants.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
drivers/mmc/host/sdhci_am654.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/sdhci_am654.c b/drivers/mmc/host/sdhci_am654.c
index b8e897e31e2e..1ac1caa2bd0c 100644
--- a/drivers/mmc/host/sdhci_am654.c
+++ b/drivers/mmc/host/sdhci_am654.c
@@ -255,7 +255,8 @@ static const struct sdhci_pltfm_data sdhci_am654_pdata = {
.ops = &sdhci_am654_ops,
.quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
- .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_RESET_DATA_POST_TUNING,
};
static const struct sdhci_am654_driver_data sdhci_am654_drvdata = {
@@ -292,7 +293,8 @@ static const struct sdhci_pltfm_data sdhci_j721e_8bit_pdata = {
.ops = &sdhci_j721e_8bit_ops,
.quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
- .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_RESET_DATA_POST_TUNING,
};
static const struct sdhci_am654_driver_data sdhci_j721e_8bit_drvdata = {
@@ -316,7 +318,8 @@ static const struct sdhci_pltfm_data sdhci_j721e_4bit_pdata = {
.ops = &sdhci_j721e_4bit_ops,
.quirks = SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
SDHCI_QUIRK_MULTIBLOCK_READ_ACMD12,
- .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_RESET_DATA_POST_TUNING,
};
static const struct sdhci_am654_driver_data sdhci_j721e_4bit_drvdata = {
--
2.19.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFT PATCH 3/3] mmc: sdhci-of-arasan: Fix Command Queuing enable handling
2019-12-30 9:23 [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
2019-12-30 9:23 ` [PATCH 1/3] mmc: sdhci: Add Quirk to reset data lines after tuning Faiz Abbas
2019-12-30 9:23 ` [PATCH 2/3] mmc: sdhci_am654: Enable Quirk to reset data " Faiz Abbas
@ 2019-12-30 9:23 ` Faiz Abbas
2020-01-03 0:36 ` Shawn Lin
2020-01-08 11:30 ` [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
3 siblings, 1 reply; 10+ messages in thread
From: Faiz Abbas @ 2019-12-30 9:23 UTC (permalink / raw)
To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, faiz_abbas, shawn.lin
There is a need to dump data from the buffer before enabling command
queuing because of leftover data from tuning. Reset the data lines to
fix this at the source.
Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
---
drivers/mmc/host/sdhci-of-arasan.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
index e49b44b4d82e..1495ae72b902 100644
--- a/drivers/mmc/host/sdhci-of-arasan.c
+++ b/drivers/mmc/host/sdhci-of-arasan.c
@@ -376,22 +376,8 @@ static void sdhci_arasan_dumpregs(struct mmc_host *mmc)
sdhci_dumpregs(mmc_priv(mmc));
}
-static void sdhci_arasan_cqe_enable(struct mmc_host *mmc)
-{
- struct sdhci_host *host = mmc_priv(mmc);
- u32 reg;
-
- reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
- while (reg & SDHCI_DATA_AVAILABLE) {
- sdhci_readl(host, SDHCI_BUFFER);
- reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
- }
-
- sdhci_cqe_enable(mmc);
-}
-
static const struct cqhci_host_ops sdhci_arasan_cqhci_ops = {
- .enable = sdhci_arasan_cqe_enable,
+ .enable = sdhci_cqe_enable,
.disable = sdhci_cqe_disable,
.dumpregs = sdhci_arasan_dumpregs,
};
@@ -410,8 +396,9 @@ static const struct sdhci_ops sdhci_arasan_cqe_ops = {
static const struct sdhci_pltfm_data sdhci_arasan_cqe_pdata = {
.ops = &sdhci_arasan_cqe_ops,
.quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
- .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
- SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
+ .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
+ SDHCI_QUIRK2_RESET_DATA_POST_TUNING |
+ SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
};
static struct sdhci_arasan_of_data sdhci_arasan_rk3399_data = {
--
2.19.2
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFT PATCH 3/3] mmc: sdhci-of-arasan: Fix Command Queuing enable handling
2019-12-30 9:23 ` [RFT PATCH 3/3] mmc: sdhci-of-arasan: Fix Command Queuing enable handling Faiz Abbas
@ 2020-01-03 0:36 ` Shawn Lin
0 siblings, 0 replies; 10+ messages in thread
From: Shawn Lin @ 2020-01-03 0:36 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, linux-mmc; +Cc: shawn.lin, ulf.hansson, adrian.hunter
On 2019/12/30 17:23, Faiz Abbas wrote:
> There is a need to dump data from the buffer before enabling command
> queuing because of leftover data from tuning. Reset the data lines to
> fix this at the source.
>
It seems to work for my platform by porting it to 4.19 LTS.
Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Faiz Abbas <faiz_abbas@ti.com>
> ---
> drivers/mmc/host/sdhci-of-arasan.c | 21 ++++-----------------
> 1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci-of-arasan.c b/drivers/mmc/host/sdhci-of-arasan.c
> index e49b44b4d82e..1495ae72b902 100644
> --- a/drivers/mmc/host/sdhci-of-arasan.c
> +++ b/drivers/mmc/host/sdhci-of-arasan.c
> @@ -376,22 +376,8 @@ static void sdhci_arasan_dumpregs(struct mmc_host *mmc)
> sdhci_dumpregs(mmc_priv(mmc));
> }
>
> -static void sdhci_arasan_cqe_enable(struct mmc_host *mmc)
> -{
> - struct sdhci_host *host = mmc_priv(mmc);
> - u32 reg;
> -
> - reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
> - while (reg & SDHCI_DATA_AVAILABLE) {
> - sdhci_readl(host, SDHCI_BUFFER);
> - reg = sdhci_readl(host, SDHCI_PRESENT_STATE);
> - }
> -
> - sdhci_cqe_enable(mmc);
> -}
> -
> static const struct cqhci_host_ops sdhci_arasan_cqhci_ops = {
> - .enable = sdhci_arasan_cqe_enable,
> + .enable = sdhci_cqe_enable,
> .disable = sdhci_cqe_disable,
> .dumpregs = sdhci_arasan_dumpregs,
> };
> @@ -410,8 +396,9 @@ static const struct sdhci_ops sdhci_arasan_cqe_ops = {
> static const struct sdhci_pltfm_data sdhci_arasan_cqe_pdata = {
> .ops = &sdhci_arasan_cqe_ops,
> .quirks = SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> - .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> - SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> + .quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
> + SDHCI_QUIRK2_RESET_DATA_POST_TUNING |
> + SDHCI_QUIRK2_CLOCK_DIV_ZERO_BROKEN,
> };
>
> static struct sdhci_arasan_of_data sdhci_arasan_rk3399_data = {
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix issues with command queuing in arasan controllers
2019-12-30 9:23 [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
` (2 preceding siblings ...)
2019-12-30 9:23 ` [RFT PATCH 3/3] mmc: sdhci-of-arasan: Fix Command Queuing enable handling Faiz Abbas
@ 2020-01-08 11:30 ` Faiz Abbas
2020-01-08 11:42 ` Adrian Hunter
3 siblings, 1 reply; 10+ messages in thread
From: Faiz Abbas @ 2020-01-08 11:30 UTC (permalink / raw)
To: linux-kernel, linux-mmc; +Cc: ulf.hansson, adrian.hunter, shawn.lin
Hi,
On 30/12/19 2:53 pm, Faiz Abbas wrote:
> In some Arasan SDHCI controllers, after tuning, the tuning pattern data
> is leftover in the sdhci buffer. This leads to issues with future data
> commands, especially when command queuing is enabled. The following
> patches help fix this issue by resetting data lines after tuning is
> finished. The first two patches have been tested with TI's am65x and
> j721e SoCs using the sdhci_am654 driver.
>
> I have a strong suspicion that this is the same issue with
> the sdhci-of-arasan driver where they are forced to dump data from the
> buffer before enabling command queuing. I need help from someone with a
> compatible platform to test this.
>
I had some discussions with our hardware team and they say we should be
asserting both SRC and SRD reset after tuning to start from a clean
state. Will update the patches to do that in v2.
Thanks,
Faiz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix issues with command queuing in arasan controllers
2020-01-08 11:30 ` [PATCH 0/3] Fix issues with command queuing in arasan controllers Faiz Abbas
@ 2020-01-08 11:42 ` Adrian Hunter
2020-01-08 11:49 ` Faiz Abbas
0 siblings, 1 reply; 10+ messages in thread
From: Adrian Hunter @ 2020-01-08 11:42 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson, shawn.lin
On 8/01/20 1:30 pm, Faiz Abbas wrote:
> Hi,
>
> On 30/12/19 2:53 pm, Faiz Abbas wrote:
>> In some Arasan SDHCI controllers, after tuning, the tuning pattern data
>> is leftover in the sdhci buffer. This leads to issues with future data
>> commands, especially when command queuing is enabled. The following
>> patches help fix this issue by resetting data lines after tuning is
>> finished. The first two patches have been tested with TI's am65x and
>> j721e SoCs using the sdhci_am654 driver.
>>
>> I have a strong suspicion that this is the same issue with
>> the sdhci-of-arasan driver where they are forced to dump data from the
>> buffer before enabling command queuing. I need help from someone with a
>> compatible platform to test this.
>>
>
> I had some discussions with our hardware team and they say we should be
> asserting both SRC and SRD reset after tuning to start from a clean
> state. Will update the patches to do that in v2.
Can you use the ->execute_tuning() for that instead of a quirk?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix issues with command queuing in arasan controllers
2020-01-08 11:42 ` Adrian Hunter
@ 2020-01-08 11:49 ` Faiz Abbas
2020-01-08 11:59 ` Adrian Hunter
0 siblings, 1 reply; 10+ messages in thread
From: Faiz Abbas @ 2020-01-08 11:49 UTC (permalink / raw)
To: Adrian Hunter, linux-kernel, linux-mmc; +Cc: ulf.hansson, shawn.lin
Adrian,
On 08/01/20 5:12 pm, Adrian Hunter wrote:
> On 8/01/20 1:30 pm, Faiz Abbas wrote:
>> Hi,
>>
>> On 30/12/19 2:53 pm, Faiz Abbas wrote:
>>> In some Arasan SDHCI controllers, after tuning, the tuning pattern data
>>> is leftover in the sdhci buffer. This leads to issues with future data
>>> commands, especially when command queuing is enabled. The following
>>> patches help fix this issue by resetting data lines after tuning is
>>> finished. The first two patches have been tested with TI's am65x and
>>> j721e SoCs using the sdhci_am654 driver.
>>>
>>> I have a strong suspicion that this is the same issue with
>>> the sdhci-of-arasan driver where they are forced to dump data from the
>>> buffer before enabling command queuing. I need help from someone with a
>>> compatible platform to test this.
>>>
>>
>> I had some discussions with our hardware team and they say we should be
>> asserting both SRC and SRD reset after tuning to start from a clean
>> state. Will update the patches to do that in v2.
>
> Can you use the ->execute_tuning() for that instead of a quirk?
>
->platform_execute_tuning() is called before __sdhci_execute_tuning(). I
need this to be done after that. Should I add a post_tuning() callback?
Thanks,
Faiz
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix issues with command queuing in arasan controllers
2020-01-08 11:49 ` Faiz Abbas
@ 2020-01-08 11:59 ` Adrian Hunter
2020-01-08 12:04 ` Faiz Abbas
0 siblings, 1 reply; 10+ messages in thread
From: Adrian Hunter @ 2020-01-08 11:59 UTC (permalink / raw)
To: Faiz Abbas, linux-kernel, linux-mmc; +Cc: ulf.hansson, shawn.lin
On 8/01/20 1:49 pm, Faiz Abbas wrote:
> Adrian,
>
> On 08/01/20 5:12 pm, Adrian Hunter wrote:
>> On 8/01/20 1:30 pm, Faiz Abbas wrote:
>>> Hi,
>>>
>>> On 30/12/19 2:53 pm, Faiz Abbas wrote:
>>>> In some Arasan SDHCI controllers, after tuning, the tuning pattern data
>>>> is leftover in the sdhci buffer. This leads to issues with future data
>>>> commands, especially when command queuing is enabled. The following
>>>> patches help fix this issue by resetting data lines after tuning is
>>>> finished. The first two patches have been tested with TI's am65x and
>>>> j721e SoCs using the sdhci_am654 driver.
>>>>
>>>> I have a strong suspicion that this is the same issue with
>>>> the sdhci-of-arasan driver where they are forced to dump data from the
>>>> buffer before enabling command queuing. I need help from someone with a
>>>> compatible platform to test this.
>>>>
>>>
>>> I had some discussions with our hardware team and they say we should be
>>> asserting both SRC and SRD reset after tuning to start from a clean
>>> state. Will update the patches to do that in v2.
>>
>> Can you use the ->execute_tuning() for that instead of a quirk?
>>
>
> ->platform_execute_tuning() is called before __sdhci_execute_tuning(). I
> need this to be done after that. Should I add a post_tuning() callback?
I meant hook host->mmc_host_ops.execute_tuning and call
sdhci_execute_tuning() and then sdhci_reset(), like in intel_execute_tuning().
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 0/3] Fix issues with command queuing in arasan controllers
2020-01-08 11:59 ` Adrian Hunter
@ 2020-01-08 12:04 ` Faiz Abbas
0 siblings, 0 replies; 10+ messages in thread
From: Faiz Abbas @ 2020-01-08 12:04 UTC (permalink / raw)
To: Adrian Hunter, linux-kernel, linux-mmc; +Cc: ulf.hansson, shawn.lin
Hi Adrian,
On 08/01/20 5:29 pm, Adrian Hunter wrote:
> On 8/01/20 1:49 pm, Faiz Abbas wrote:
>> Adrian,
>>
>> On 08/01/20 5:12 pm, Adrian Hunter wrote:
>>> On 8/01/20 1:30 pm, Faiz Abbas wrote:
>>>> Hi,
>>>>
>>>> On 30/12/19 2:53 pm, Faiz Abbas wrote:
>>>>> In some Arasan SDHCI controllers, after tuning, the tuning pattern data
>>>>> is leftover in the sdhci buffer. This leads to issues with future data
>>>>> commands, especially when command queuing is enabled. The following
>>>>> patches help fix this issue by resetting data lines after tuning is
>>>>> finished. The first two patches have been tested with TI's am65x and
>>>>> j721e SoCs using the sdhci_am654 driver.
>>>>>
>>>>> I have a strong suspicion that this is the same issue with
>>>>> the sdhci-of-arasan driver where they are forced to dump data from the
>>>>> buffer before enabling command queuing. I need help from someone with a
>>>>> compatible platform to test this.
>>>>>
>>>>
>>>> I had some discussions with our hardware team and they say we should be
>>>> asserting both SRC and SRD reset after tuning to start from a clean
>>>> state. Will update the patches to do that in v2.
>>>
>>> Can you use the ->execute_tuning() for that instead of a quirk?
>>>
>>
>> ->platform_execute_tuning() is called before __sdhci_execute_tuning(). I
>> need this to be done after that. Should I add a post_tuning() callback?
>
> I meant hook host->mmc_host_ops.execute_tuning and call
> sdhci_execute_tuning() and then sdhci_reset(), like in intel_execute_tuning().
>
Ok. Makes sense.
Thanks,
Faiz
^ permalink raw reply [flat|nested] 10+ messages in thread