mbox series

[0/3] Fix issues with command queuing in arasan controllers

Message ID 20191230092343.30692-1-faiz_abbas@ti.com (mailing list archive)
Headers show
Series Fix issues with command queuing in arasan controllers | expand

Message

Faiz Abbas Dec. 30, 2019, 9:23 a.m. UTC
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.

Faiz Abbas (3):
  mmc: sdhci: Add Quirk to reset data lines after tuning
  mmc: sdhci_am654: Enable Quirk to reset data after tuning
  mmc: sdhci-of-arasan: Fix Command Queuing enable handling

 drivers/mmc/host/sdhci-of-arasan.c | 21 ++++-----------------
 drivers/mmc/host/sdhci.c           |  3 +++
 drivers/mmc/host/sdhci.h           |  4 ++++
 drivers/mmc/host/sdhci_am654.c     |  9 ++++++---
 4 files changed, 17 insertions(+), 20 deletions(-)

Comments

Faiz Abbas Jan. 8, 2020, 11:30 a.m. UTC | #1
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
Adrian Hunter Jan. 8, 2020, 11:42 a.m. UTC | #2
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?
Faiz Abbas Jan. 8, 2020, 11:49 a.m. UTC | #3
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
Adrian Hunter Jan. 8, 2020, 11:59 a.m. UTC | #4
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().
Faiz Abbas Jan. 8, 2020, 12:04 p.m. UTC | #5
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